Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add user freeze feature #6298

Merged
merged 1 commit into from
Feb 22, 2024
Merged

Add user freeze feature #6298

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions docs/rfcs/1007-frozen-users.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,21 +101,21 @@ For parsec v2:
- Invalid administration token: `403` with JSON body `{"error": "not_allowed"}`
- Wrong request format: `400` with JSON body `{"error": "bad_data"}`

For parsec v3 (note the different `detail` field due to the migration to `FastAPI`):
For parsec v3, with an arbitrary JSON body only aimed at human consumption (and hence free to change at any time):

- Organization not found: `404` with JSON body `{"detail": "not_found"}`
- Invalid administration token: `403` with JSON body `{"detail": "not_allowed"}`
- Wrong request format: `400` with JSON body `{"detail": "bad_data"}`
- Organization not found: `404`
- Invalid administration token: `403`
- Wrong request format: `422`

On top of it, an extra error is handled when the `POST` request contains a user that does not exist in the organization.

For parsec v2:

- User not found: `404` with JSON body `{"error": "user_not_found"}`

For parsec v3 (note the different `detail` field due to the migration to `FastAPI`):
For parsec v3:

- User not found: `404` with JSON body `{"detail": "user_not_found"}`
- User not found: `404` (again with arbitrary JSON body)


## Implementation
Expand Down
134 changes: 113 additions & 21 deletions server/parsec/asgi/administration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
BootstrapToken,
DateTime,
OrganizationID,
UserID,
UserProfile,
)
from parsec.components.organization import (
Expand All @@ -33,6 +34,7 @@
OrganizationUpdateBadOutcome,
Unset,
)
from parsec.components.user import UserFreezeUserBadOutcome, UserInfo, UserListUsersBadOutcome
from parsec.events import OrganizationIDField

if TYPE_CHECKING:
Expand All @@ -50,6 +52,18 @@ def check_administration_auth(
raise HTTPException(status_code=403, detail="Bad authorization token")


# This function is a workaround for FastAPI's broken custom type in query parameters
# (see https://github.com/tiangolo/fastapi/issues/10259)
def parse_organization_id_or_die(raw_organization_id: str) -> OrganizationID:
try:
return OrganizationID(raw_organization_id)
except ValueError:
raise HTTPException(
status_code=404,
detail="Invalid organization ID",
)


class CreateOrganizationIn(BaseModel):
model_config = ConfigDict(arbitrary_types_allowed=True, strict=True)
organization_id: OrganizationIDField
Expand Down Expand Up @@ -122,13 +136,7 @@ async def administration_get_organization(
) -> GetOrganizationOut:
backend: Backend = request.app.state.backend

try:
organization_id = OrganizationID(raw_organization_id)
except ValueError:
raise HTTPException(
status_code=404,
detail="Invalid organization ID",
)
organization_id = parse_organization_id_or_die(raw_organization_id)

# Check whether the organization actually exists
outcome = await backend.organization.get(id=organization_id)
Expand Down Expand Up @@ -186,13 +194,7 @@ async def administration_patch_organization(
) -> PatchOrganizationOut:
backend: Backend = request.app.state.backend

try:
organization_id = OrganizationID(raw_organization_id)
except ValueError:
raise HTTPException(
status_code=404,
detail="Invalid organization ID",
)
organization_id = parse_organization_id_or_die(raw_organization_id)

outcome = await backend.organization.update(
id=organization_id,
Expand All @@ -219,13 +221,7 @@ async def administration_organization_stat(
) -> Response:
backend: Backend = request.app.state.backend

try:
organization_id = OrganizationID(raw_organization_id)
except ValueError:
raise HTTPException(
status_code=404,
detail="Invalid organization ID",
)
organization_id = parse_organization_id_or_die(raw_organization_id)

outcome = await backend.organization.organization_stats(organization_id)
match outcome:
Expand Down Expand Up @@ -357,3 +353,99 @@ async def administration_server_stats(

case unknown:
assert_never(unknown)


@administration_router.get("/administration/organizations/{raw_organization_id}/users")
async def administration_organization_users(
raw_organization_id: str,
auth: Annotated[None, Depends(check_administration_auth)],
request: Request,
) -> Response:
backend: Backend = request.app.state.backend

organization_id = parse_organization_id_or_die(raw_organization_id)

outcome = await backend.user.list_users(organization_id)
match outcome:
case list() as users:
pass
case UserListUsersBadOutcome.ORGANIZATION_NOT_FOUND:
raise HTTPException(status_code=404, detail="Organization not found")
case unknown:
assert_never(unknown)
Comment on lines +368 to +375
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a rationale for the "outcome" wording? (I noticed it was introduced in #6214)

I don't have strong arguments against it, it's just that it doesn't feel "standard" (which is usually better for readability). Usually this things are named result or response. Also suffix BadOutcome for enums which has the downside of using 2 words instead something like just Error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Outcome" comes from the outcome library, so an outcome wraps the result of a function.

Usually this things are named result or response

I think response is to be used when doing http request.

result would have been possible name, but it lacks the fact both ok and error are contained. Given in Python you expect a function to return the ok part and raise an exception for the errors, a dedicated name like outcome is good for readability.

Also suffix BadOutcome for enums which has the downside of using 2 words instead something like just Error.

This is a feature ! ^^
You have FooError which is an exception and FooBadOutcome which is an enum of possible errors.


return JSONResponse(
status_code=200,
content={
"users": [
{
"user_id": user.user_id.str,
"user_email": user.human_handle.email,
"user_name": user.human_handle.label,
"frozen": user.frozen,
touilleMan marked this conversation as resolved.
Show resolved Hide resolved
}
for user in users
]
},
)


class UserFreezeIn(BaseModel):
model_config = ConfigDict(arbitrary_types_allowed=True, strict=True)
frozen: bool
user_email: str | None = None
user_id: UserID | None = None

@field_validator("user_id", mode="plain")
@classmethod
def validate_user_id(cls, v: Any) -> UserID | None:
match v:
case UserID():
return v
case None:
return None
case raw:
return UserID(raw)
touilleMan marked this conversation as resolved.
Show resolved Hide resolved


@administration_router.post("/administration/organizations/{raw_organization_id}/users/freeze")
async def administration_organization_users_freeze(
raw_organization_id: str,
auth: Annotated[None, Depends(check_administration_auth)],
body: UserFreezeIn,
request: Request,
) -> Response:
backend: Backend = request.app.state.backend

organization_id = parse_organization_id_or_die(raw_organization_id)

outcome = await backend.user.freeze_user(
organization_id, user_id=body.user_id, user_email=body.user_email, frozen=body.frozen
)
match outcome:
case UserInfo() as user:
pass
case UserFreezeUserBadOutcome.ORGANIZATION_NOT_FOUND:
raise HTTPException(status_code=404, detail="Organization not found")
case UserFreezeUserBadOutcome.USER_NOT_FOUND:
raise HTTPException(status_code=404, detail="User not found")
case UserFreezeUserBadOutcome.BOTH_USER_ID_AND_EMAIL:
raise HTTPException(
status_code=400, detail="Both `user_id` and `user_email` fields are provided"
)
case UserFreezeUserBadOutcome.NO_USER_ID_NOR_EMAIL:
raise HTTPException(
status_code=400, detail="Missing either `user_id` or `user_email` field"
)
case unknown:
assert_never(unknown)

return JSONResponse(
status_code=200,
content={
"user_id": user.user_id.str,
"user_email": user.human_handle.email,
"user_name": user.human_handle.label,
"frozen": user.frozen,
},
)
Loading