update POST /Users to use tenancy and not consider soft delete
This commit is contained in:
Jonathan Griffin
parent
bcfa421b8f
commit
4371fec38e
2 changed files with 92 additions and 110 deletions
|
|
@ -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())
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
}
|
||||
@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,
|
||||
)
|
||||
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
|
||||
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)
|
||||
)
|
||||
return JSONResponse(status_code=201, content=res.model_dump(mode='json'))
|
||||
|
||||
response.headers["Location"] = scim_user.meta.location
|
||||
return response
|
||||
|
||||
|
||||
@public_app.put("/Users/{user_id}", dependencies=[Depends(auth_required)])
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue