diff --git a/ee/api/chalicelib/core/users.py b/ee/api/chalicelib/core/users.py index 4fd1e8b09..de73bafd9 100644 --- a/ee/api/chalicelib/core/users.py +++ b/ee/api/chalicelib/core/users.py @@ -1,7 +1,7 @@ import json import logging import secrets -from typing import Optional +from typing import Any, Optional from decouple import config from fastapi import BackgroundTasks, HTTPException @@ -284,7 +284,7 @@ def get(user_id, tenant_id): r = cur.fetchone() return helper.dict_to_camel_case(r) -def get_by_uuid(user_uuid, tenant_id): +def get_scim_user_by_id(user_id, tenant_id): with pg_client.PostgresClient() as cur: cur.execute( cur.mogrify( @@ -292,19 +292,17 @@ def get_by_uuid(user_uuid, tenant_id): SELECT * FROM public.users WHERE - users.deleted_at IS NULL - AND users.user_id = %(user_id)s + users.user_id = %(user_id)s AND users.tenant_id = %(tenant_id)s LIMIT 1; """, { - "user_id": user_uuid, + "user_id": user_id, "tenant_id": tenant_id, }, ) ) - r = cur.fetchone() - return helper.dict_to_camel_case(r) + return helper.dict_to_camel_case(cur.fetchone()) def get_deleted_by_uuid(user_uuid, tenant_id): with pg_client.PostgresClient() as cur: @@ -440,6 +438,21 @@ def edit_member(user_id_to_update, tenant_id, changes: schemas.EditMemberSchema, return {"data": user} +def get_scim_user_by_unique_values(email): + with pg_client.PostgresClient() as cur: + cur.execute( + cur.mogrify( + """ + SELECT * + FROM public.users + WHERE users.email = %(email)s + """, + {"email": email} + ) + ) + return helper.dict_to_camel_case(cur.fetchone()) + + def get_by_email_only(email): with pg_client.PostgresClient() as cur: cur.execute( @@ -475,9 +488,7 @@ def get_users_paginated(start_index, tenant_id, count=None): """ SELECT * FROM public.users - WHERE - users.deleted_at IS NULL - AND users.tenant_id = %(tenant_id)s + WHERE users.tenant_id = %(tenant_id)s LIMIT %(limit)s OFFSET %(offset)s; """, @@ -1011,48 +1022,36 @@ def create_sso_user(tenant_id, email, admin, name, origin, role_id, internal_id= return helper.dict_to_camel_case(cur.fetchone()) def create_scim_user( - tenant_id, - user_uuid, email, - admin, - display_name, - full_name: dict, - emails, - origin, - locale, - role_id, - internal_id=None, + name, + tenant_id, ): - with pg_client.PostgresClient() as cur: - query = cur.mogrify(f"""\ - WITH u AS ( - INSERT INTO public.users (tenant_id, email, role, name, data, origin, internal_id, role_id) - VALUES (%(tenant_id)s, %(email)s, %(role)s, %(name)s, %(data)s, %(origin)s, %(internal_id)s, - (SELECT COALESCE((SELECT role_id FROM roles WHERE tenant_id = %(tenant_id)s AND role_id = %(role_id)s), - (SELECT role_id FROM roles WHERE tenant_id = %(tenant_id)s AND name = 'Member' LIMIT 1), - (SELECT role_id FROM roles WHERE tenant_id = %(tenant_id)s AND name != 'Owner' LIMIT 1)))) - RETURNING * - ), - au AS ( - INSERT INTO public.basic_authentication(user_id) - VALUES ((SELECT user_id FROM u)) - ) - SELECT u.user_id AS id, - u.email, - u.role, - u.name, - u.data, - (CASE WHEN u.role = 'owner' THEN TRUE ELSE FALSE END) AS super_admin, - (CASE WHEN u.role = 'admin' THEN TRUE ELSE FALSE END) AS admin, - (CASE WHEN u.role = 'member' THEN TRUE ELSE FALSE END) AS member, - origin - FROM u;""", - {"tenant_id": tenant_id, "email": email, "internal_id": internal_id, - "role": "admin" if admin else "member", "name": display_name, "origin": origin, - "role_id": role_id, "data": json.dumps({"lastAnnouncementView": TimeUTC.now(), "user_id": user_uuid, "locale": locale, "name": full_name, "emails": emails})}) cur.execute( - query + cur.mogrify( + """ + WITH u AS ( + INSERT INTO public.users ( + tenant_id, + email, + name + ) + VALUES ( + %(tenant_id)s, + %(email)s, + %(name)s + ) + RETURNING * + ) + SELECT * + FROM u; + """, + { + "tenant_id": tenant_id, + "email": email, + "name": name, + } + ) ) return helper.dict_to_camel_case(cur.fetchone()) diff --git a/ee/api/routers/scim.py b/ee/api/routers/scim.py index 61889c82e..b1c73a568 100644 --- a/ee/api/routers/scim.py +++ b/ee/api/routers/scim.py @@ -72,6 +72,18 @@ def _not_found_error_response(resource_id: str): ) +def _uniqueness_error_response(): + return JSONResponse( + status_code=409, + content={ + "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], + "detail": "One or more of the attribute values are already in use or are reserved.", + "status": "409", + "scimType": "uniqueness", + } + ) + + @public_app.get("/ResourceTypes", dependencies=[Depends(auth_required)]) async def get_resource_types(filter_param: str | None = Query(None, alias="filter")): if filter_param is not None: @@ -169,27 +181,8 @@ async def get_service_provider_config(r: Request, tenant_id: str | None = Depend """ User endpoints """ - -class Name(BaseModel): - givenName: str - familyName: str - -class Email(BaseModel): - primary: bool - value: str - type: str - class UserRequest(BaseModel): - schemas: list[str] userName: str - name: Name - emails: list[Email] - displayName: str - locale: str - externalId: str - groups: list[dict] - password: str = Field(default=None) - active: bool class PatchUserRequest(BaseModel): @@ -301,7 +294,7 @@ def get_user( attributes: list[str] | None = Query(None), excluded_attributes: list[str] | None = Query(None, alias="excludedAttributes"), ): - db_user = users.get_by_uuid(user_id, tenant_id) + db_user = users.get_scim_user_by_id(user_id, tenant_id) if not db_user: return _not_found_error_response(user_id) scim_user = _convert_db_user_to_scim_user(db_user, attributes, excluded_attributes) @@ -311,49 +304,39 @@ def get_user( ) -@public_app.post("/Users", dependencies=[Depends(auth_required)]) -async def create_user(r: UserRequest): - """Create SCIM User""" - tenant_id = 1 - existing_user = users.get_by_email_only(r.userName) - deleted_user = users.get_deleted_user_by_email(r.userName) - - if existing_user: - return JSONResponse( - status_code = 409, - content = { - "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"], - "detail": "User already exists in the database.", - "status": 409, - } - ) - elif deleted_user: - user_id = users.get_deleted_by_uuid(deleted_user["data"]["userId"], tenant_id) - user = users.restore_scim_user(user_id=user_id["userId"], tenant_id=tenant_id, user_uuid=uuid.uuid4().hex, email=r.emails[0].value, admin=False, - display_name=r.displayName, full_name=r.name.model_dump(mode='json'), emails=r.emails[0].model_dump(mode='json'), - origin="okta", locale=r.locale, role_id=None, internal_id=r.externalId) - else: - try: - user = users.create_scim_user(tenant_id=tenant_id, user_uuid=uuid.uuid4().hex, email=r.emails[0].value, admin=False, - display_name=r.displayName, full_name=r.name.model_dump(mode='json'), emails=r.emails[0].model_dump(mode='json'), - origin="okta", locale=r.locale, role_id=None, internal_id=r.externalId) - except Exception as e: - raise HTTPException(status_code=500, detail=str(e)) - - res = UserResponse( - schemas = ["urn:ietf:params:scim:schemas:core:2.0:User"], - id = user["data"]["userId"], - userName = r.userName, - name = r.name, - emails = r.emails, - displayName = r.displayName, - locale = r.locale, - externalId = r.externalId, - active = r.active, # ignore for now, since, can't insert actual timestamp - groups = [], # ignore +@public_app.post("/Users") +async def create_user(r: UserRequest, tenant_id = Depends(auth_required)): + existing_db_user = users.get_scim_user_by_unique_values(r.userName) + # todo(jon): we have a conflict in our db schema and the docs for SCIM. + # here is a quote from section 3.6 of RFC 7644: + # For example, if a User resource is deleted, a CREATE + # request for a User resource with the same userName as the previously + # deleted resource SHOULD NOT fail with a 409 error due to userName + # conflict. + # this doesn't work with our db schema as `public.users.email` is UNIQUE + # but not conditionaly unique based on deletion. this would be fine if + # we did a hard delete, but it seems like we do soft deletes. + # so, we need to figure out how to handle this: + # 1. we do hard deletes for scim users. + # 2. we change how we handle the unique constraint for users on the email field. + # i think the easiest thing to do here would be 1, since it wouldn't require + # updating any other parts of the codebase (potentially) + if existing_db_user: + return _uniqueness_error_response() + db_user = users.create_scim_user( + email=r.userName, + # note(jon): scim schema does not require the `name.formatted` attribute, but we require `name`. + # so, we have to define the value ourselves here + name="", + tenant_id=tenant_id, ) - return JSONResponse(status_code=201, content=res.model_dump(mode='json')) - + scim_user = _convert_db_user_to_scim_user(db_user) + response = JSONResponse( + status_code=201, + content=scim_user.model_dump(mode="json", exclude_none=True) + ) + response.headers["Location"] = scim_user.meta.location + return response @public_app.put("/Users/{user_id}", dependencies=[Depends(auth_required)])