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.

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

Move staff check above GOV ACCOUNT USER #3183

Merged
merged 3 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions auth-api/src/auth_api/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,15 +258,15 @@ def _get_type(cls, user_from_context: UserContext) -> str:
or user_from_context.login_source == LoginSource.BCROS.value
):
user_type = Role.ANONYMOUS_USER.name
elif user_from_context.is_staff():
user_type = Role.STAFF.name
elif Role.GOV_ACCOUNT_USER.value in user_from_context.roles:
user_type = Role.GOV_ACCOUNT_USER.name
elif Role.PUBLIC_USER.value in user_from_context.roles or user_from_context.login_source in [
LoginSource.BCEID.value,
LoginSource.BCSC.value,
]:
user_type = Role.PUBLIC_USER.name
elif user_from_context.is_staff():
user_type = Role.STAFF.name
elif user_from_context.is_system():
user_type = Role.SYSTEM.name

Expand Down
9 changes: 8 additions & 1 deletion auth-api/src/auth_api/resources/v1/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

from http import HTTPStatus

from flask import Blueprint, abort, g, jsonify, request
from flask import Blueprint, abort, current_app, g, jsonify, request
from flask_cors import cross_origin

from auth_api.exceptions import BusinessException
Expand All @@ -29,6 +29,7 @@
from auth_api.services.org import Org as OrgService
from auth_api.services.user import User as UserService
from auth_api.utils.auth import jwt as _jwt
from auth_api.utils.constants import GROUP_GOV_ACCOUNT_USERS
from auth_api.utils.endpoints_enums import EndpointEnum
from auth_api.utils.enums import LoginSource, Status
from auth_api.utils.roles import Role
Expand Down Expand Up @@ -108,6 +109,12 @@ def post_user():
if not valid_format:
return {"message": schema_utils.serialize(errors)}, HTTPStatus.BAD_REQUEST

# Ensure STAFF doesn't have GOV_ACCOUNT_USER, otherwise they get extra permissions they shouldn't have.
roles = token.get("realm_access", {}).get("roles", [])
if Role.STAFF.name in roles and Role.GOV_ACCOUNT_USER.value in roles:
current_app.logger.info("Removing GOV_ACCOUNT_USER group from STAFF user")
KeycloakService.remove_user_from_group(token.get("sub"), GROUP_GOV_ACCOUNT_USERS)

user = UserService.save_from_jwt_token(request_json)
response, status = user.as_dict(), HTTPStatus.CREATED
# Add the user to public_users group if the user doesn't have public_user group
Expand Down
4 changes: 2 additions & 2 deletions auth-api/src/auth_api/services/keycloak.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ def remove_from_account_holders_group(keycloak_guid: str = None, **kwargs):
keycloak_guid: Dict = user_from_context.sub

if Role.ACCOUNT_HOLDER.value in user_from_context.roles:
KeycloakService._remove_user_from_group(keycloak_guid, GROUP_ACCOUNT_HOLDERS)
KeycloakService.remove_user_from_group(keycloak_guid, GROUP_ACCOUNT_HOLDERS)

@staticmethod
@user_context
Expand Down Expand Up @@ -339,7 +339,7 @@ def add_user_to_group(user_id: str, group_name: str):
response.raise_for_status()

@staticmethod
def _remove_user_from_group(user_id: str, group_name: str):
def remove_user_from_group(user_id: str, group_name: str):
"""Remove user from the keycloak group."""
config = current_app.config
base_url = config.get("KEYCLOAK_BASE_URL")
Expand Down
Loading