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

Jump to bottom

<WIP> Make user endpoints synchronous #5285

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

<WIP> Make user endpoints synchronous #5285

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 src/fides/api/api/v1/endpoints/user_endpoints.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def _validate_current_user(user_id: str, user_from_token: FidesUser) -> None:
status_code=HTTP_200_OK,
response_model=UserResponse,
)
async def update_user(
def update_user(
*,
db: Session = Depends(deps.get_db),
authorization: str = Security(oauth2_scheme),
Expand All @@ -136,7 +136,7 @@ async def update_user(

is_this_user = user.id == current_user.id
if not is_this_user:
await verify_oauth_client(
verify_oauth_client(
security_scopes=Security(verify_oauth_client, scopes=[USER_UPDATE]),
authorization=authorization,
db=db,
Expand Down Expand Up @@ -317,7 +317,7 @@ def update_managed_systems(
urls.SYSTEM_MANAGER,
response_model=List[SystemSchema],
)
async def get_managed_systems(
def get_managed_systems(
*,
db: Session = Depends(deps.get_db),
authorization: str = Security(oauth2_scheme),
Expand All @@ -337,7 +337,7 @@ async def get_managed_systems(

# User must have a specific scope to be able to read another user's systems
user = validate_user_id(db, user_id)
await verify_oauth_client(
verify_oauth_client(
security_scopes=Security(verify_oauth_client, scopes=[SYSTEM_MANAGER_READ]),
authorization=authorization,
db=db,
Expand All @@ -350,7 +350,7 @@ async def get_managed_systems(
urls.SYSTEM_MANAGER_DETAIL,
response_model=SystemSchema,
)
async def get_managed_system_details(
def get_managed_system_details(
*,
authorization: str = Security(oauth2_scheme),
db: Session = Depends(deps.get_db),
Expand All @@ -366,7 +366,7 @@ async def get_managed_system_details(
if current_user and current_user.id == user_id:
user = current_user
else:
await verify_oauth_client(
verify_oauth_client(
security_scopes=Security(verify_oauth_client, scopes=[SYSTEM_MANAGER_READ]),
authorization=authorization,
db=db,
Expand Down
16 changes: 8 additions & 8 deletions src/fides/api/api/v1/endpoints/user_permission_endpoints.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ def validate_user_id(db: Session, user_id: str) -> FidesUser:
return user


async def owner_role_permission_check(
def owner_role_permission_check(
db: Session, roles: List[RoleRegistryEnum], authorization: str
) -> None:
"""Extra permissions check to assert that the token possesses the USER_PERMISSION_ASSIGN_OWNERS scope
if attempting to make another user an owner.
"""
if OWNER in roles:
await verify_oauth_client(
verify_oauth_client(
security_scopes=SecurityScopes([USER_PERMISSION_ASSIGN_OWNERS]),
authorization=authorization,
db=db,
Expand All @@ -61,7 +61,7 @@ async def owner_role_permission_check(
status_code=HTTP_201_CREATED,
response_model=UserPermissionsResponse,
)
async def create_user_permissions(
def create_user_permissions(
*,
db: Session = Depends(deps.get_db),
user_id: str,
Expand All @@ -76,7 +76,7 @@ async def create_user_permissions(
detail="This user already has permissions set.",
)

await owner_role_permission_check(db, permissions.roles, authorization)
owner_role_permission_check(db, permissions.roles, authorization)
if user.client:
# Just in case - this shouldn't happen in practice.
user.client.update(db=db, data=permissions.model_dump(mode="json"))
Expand All @@ -91,7 +91,7 @@ async def create_user_permissions(
dependencies=[Security(verify_oauth_client, scopes=[USER_PERMISSION_UPDATE])],
response_model=UserPermissionsResponse,
)
async def update_user_permissions(
def update_user_permissions(
*,
db: Session = Depends(deps.get_db),
user_id: str,
Expand All @@ -106,7 +106,7 @@ async def update_user_permissions(
user = validate_user_id(db, user_id)
logger.info("Updated FidesUserPermission record")

await owner_role_permission_check(db, permissions.roles, authorization)
owner_role_permission_check(db, permissions.roles, authorization)

if user.client:
user.client.update(db=db, data={"roles": permissions.roles})
Expand All @@ -132,7 +132,7 @@ async def update_user_permissions(
urls.USER_PERMISSIONS,
response_model=UserPermissionsResponse,
)
async def get_user_permissions(
def get_user_permissions(
*,
db: Session = Depends(deps.get_db),
authorization: str = Security(oauth2_scheme),
Expand All @@ -156,7 +156,7 @@ async def get_user_permissions(
# To look up the permissions of another user, that user must exist and the current user must
# have permission to read users.
validate_user_id(db, user_id)
await verify_oauth_client(
verify_oauth_client(
security_scopes=SecurityScopes([USER_PERMISSION_READ]),
authorization=authorization,
db=db,
Expand Down
2 changes: 1 addition & 1 deletion src/fides/api/oauth/system_manager_oauth_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def _get_system_from_fides_key(
return resp


async def verify_oauth_client_for_system_from_request_body(
def verify_oauth_client_for_system_from_request_body(
security_scopes: SecurityScopes,
authorization: str = Security(oauth2_scheme),
db: Session = Depends(get_db),
Expand Down
6 changes: 3 additions & 3 deletions src/fides/api/oauth/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ def copy_func(source_function: Callable) -> Callable:
return updated_target_function


async def get_current_user(
def get_current_user(
security_scopes: SecurityScopes,
authorization: str = Security(oauth2_scheme),
db: Session = Depends(get_db),
) -> FidesUser:
"""A wrapper around verify_oauth_client that returns that client's user if one exists."""
client = await verify_oauth_client(
client = verify_oauth_client(
security_scopes=security_scopes,
authorization=authorization,
db=db,
Expand Down Expand Up @@ -253,7 +253,7 @@ async def get_root_client(
return client


async def verify_oauth_client(
def verify_oauth_client(
security_scopes: SecurityScopes,
authorization: str = Security(oauth2_scheme),
db: Session = Depends(get_db),
Expand Down
50 changes: 24 additions & 26 deletions tests/lib/test_oauth_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,16 @@ def test_is_token_expired(issued_at, token_duration_min, expected):
assert is_token_expired(issued_at, token_duration_min) is expected


async def test_verify_oauth_malformed_oauth_client(db):
def test_verify_oauth_malformed_oauth_client(db):
with pytest.raises(AuthorizationError):
await verify_oauth_client(
verify_oauth_client(
SecurityScopes([USER_READ]),
authorization="invalid",
db=db,
)


async def test_verify_oauth_client_no_issued_at(db, config, user):
def test_verify_oauth_client_no_issued_at(db, config, user):
payload = {
JWE_PAYLOAD_SCOPES: [USER_READ],
JWE_PAYLOAD_CLIENT_ID: user.client.id,
Expand All @@ -91,14 +91,14 @@ async def test_verify_oauth_client_no_issued_at(db, config, user):
config.security.app_encryption_key,
)
with pytest.raises(AuthorizationError):
await verify_oauth_client(
verify_oauth_client(
SecurityScopes([USER_READ]),
token,
db=db,
)


async def test_verify_oauth_client_expired(db, config, user):
def test_verify_oauth_client_expired(db, config, user):
scope = [USER_READ]
payload = {
JWE_PAYLOAD_SCOPES: scope,
Expand All @@ -111,14 +111,14 @@ async def test_verify_oauth_client_expired(db, config, user):
config.security.app_encryption_key,
)
with pytest.raises(AuthorizationError):
await verify_oauth_client(
verify_oauth_client(
SecurityScopes(scope),
token,
db=db,
)


async def test_verify_oauth_client_no_client_id(db, config):
def test_verify_oauth_client_no_client_id(db, config):
scope = [USER_READ]
payload = {
JWE_PAYLOAD_SCOPES: scope,
Expand All @@ -131,14 +131,14 @@ async def test_verify_oauth_client_no_client_id(db, config):
config.security.app_encryption_key,
)
with pytest.raises(AuthorizationError):
await verify_oauth_client(
verify_oauth_client(
SecurityScopes(scope),
token,
db=db,
)


async def test_verify_oauth_client_no_client(db, config, user):
def test_verify_oauth_client_no_client(db, config, user):
scopes = [USER_READ]
payload = {
JWE_PAYLOAD_SCOPES: scopes,
Expand All @@ -153,14 +153,14 @@ async def test_verify_oauth_client_no_client(db, config, user):
user.client.delete(db)
assert user.client is None
with pytest.raises(AuthorizationError):
await verify_oauth_client(
verify_oauth_client(
SecurityScopes(scopes),
token,
db=db,
)


async def test_verify_oauth_client_wrong_security_scope(db, config, user):
def test_verify_oauth_client_wrong_security_scope(db, config, user):
payload = {
JWE_PAYLOAD_SCOPES: [USER_DELETE],
JWE_PAYLOAD_CLIENT_ID: user.client.id,
Expand All @@ -172,14 +172,14 @@ async def test_verify_oauth_client_wrong_security_scope(db, config, user):
config.security.app_encryption_key,
)
with pytest.raises(AuthorizationError):
await verify_oauth_client(
verify_oauth_client(
SecurityScopes([USER_READ]),
token,
db=db,
)


async def test_verify_oauth_client_wrong_client_scope(db, config, user):
def test_verify_oauth_client_wrong_client_scope(db, config, user):
scopes = [USER_READ]
payload = {
JWE_PAYLOAD_SCOPES: scopes,
Expand All @@ -193,15 +193,15 @@ async def test_verify_oauth_client_wrong_client_scope(db, config, user):
)
user.client.scopes = [USER_DELETE]
with pytest.raises(AuthorizationError):
await verify_oauth_client(
verify_oauth_client(
SecurityScopes(scopes),
token,
db=db,
)


class TestVerifyOauthClientRoles:
async def test_token_does_not_have_roles(self, db, config):
def test_token_does_not_have_roles(self, db, config):
"""Test that roles aren't required to be on the token - scopes can still be assigned directly"""
client, _ = ClientDetail.create_client_and_secret(
db,
Expand All @@ -220,14 +220,14 @@ async def test_token_does_not_have_roles(self, db, config):
json.dumps(payload),
config.security.app_encryption_key,
)
verified_client = await verify_oauth_client(
verified_client = verify_oauth_client(
SecurityScopes([PRIVACY_REQUEST_REVIEW]),
token,
db=db,
)
assert client == verified_client

async def test_verify_oauth_client_roles(self, db, config, owner_user):
def test_verify_oauth_client_roles(self, db, config, owner_user):
"""Test token has a valid role and the client also has the matching role
Scopes aren't directly assigned but the user inherits the USER_READ scope
via the OWNER role.
Expand All @@ -241,14 +241,14 @@ async def test_verify_oauth_client_roles(self, db, config, owner_user):
json.dumps(payload),
config.security.app_encryption_key,
)
client = await verify_oauth_client(
client = verify_oauth_client(
SecurityScopes([PRIVACY_REQUEST_REVIEW]),
token,
db=db,
)
assert client == owner_user.client

async def test_no_roles_on_client(self, db, config, user):
def test_no_roles_on_client(self, db, config, user):
"""Test token has a role with the correct scopes but that role is not on the client"""
payload = {
JWE_PAYLOAD_ROLES: [OWNER],
Expand All @@ -260,13 +260,13 @@ async def test_no_roles_on_client(self, db, config, user):
config.security.app_encryption_key,
)
with pytest.raises(AuthorizationError):
await verify_oauth_client(
verify_oauth_client(
SecurityScopes([PRIVACY_REQUEST_REVIEW]),
token,
db=db,
)

async def test_no_roles_on_client_but_has_scopes_coverage(self, db, config, user):
def test_no_roles_on_client_but_has_scopes_coverage(self, db, config, user):
"""Test roles on token are outdated but token still has scopes coverage"""
user.client.scopes = [PRIVACY_REQUEST_REVIEW]
user.client.save(db)
Expand All @@ -280,16 +280,14 @@ async def test_no_roles_on_client_but_has_scopes_coverage(self, db, config, user
json.dumps(payload),
config.security.app_encryption_key,
)
client = await verify_oauth_client(
client = verify_oauth_client(
SecurityScopes([PRIVACY_REQUEST_REVIEW]),
token,
db=db,
)
assert client == user.client

async def test_token_does_not_have_role_with_coverage(
self, db, config, viewer_user
):
def test_token_does_not_have_role_with_coverage(self, db, config, viewer_user):
"""Test token only has a viewer role, which is not enough to view the particular endpoint
as it is missing DATASET_CREATE_OR_UPDATE scopes
"""
Expand All @@ -305,7 +303,7 @@ async def test_token_does_not_have_role_with_coverage(
)

with pytest.raises(AuthorizationError):
await verify_oauth_client(
verify_oauth_client(
SecurityScopes([DATASET_CREATE_OR_UPDATE]),
token,
db=db,
Expand Down
Loading
Loading