From 23faaae48701f1fdf73d3dfcbebbf59a87ab8f9c Mon Sep 17 00:00:00 2001 From: Ratheesh kumar R <108045773+ratheesh-aot@users.noreply.github.com> Date: Wed, 7 Feb 2024 11:52:24 -0800 Subject: [PATCH] DESENG-447 : Convert keycloak groups to composite roles for permission levels (#2376) * DESENG-447 Removed references to EAO in groups, remove group check for AuthGate * DESENG-447 Remove or comment out references to groups * DESENG-447: Commented out checks related to groups * DESENG-447: Fixing linting issues and unit test * Updated Changelog * Removed console.log statements * Removed console.log --------- Co-authored-by: Alex --- CHANGELOG.MD | 6 + met-api/.DS_Store | Bin 6148 -> 6148 bytes met-api/src/met_api/models/membership.py | 2 +- .../met_api/resources/engagement_members.py | 23 +- met-api/src/met_api/resources/staff_user.py | 42 +- met-api/src/met_api/services/authorization.py | 6 +- met-api/src/met_api/services/keycloak.py | 276 ++++++------ .../met_api/services/membership_service.py | 141 +++--- .../services/staff_user_membership_service.py | 29 +- .../met_api/services/staff_user_service.py | 86 ++-- .../src/met_api/services/tenant_service.py | 2 +- met-api/src/met_api/utils/constants.py | 32 +- met-api/src/met_api/utils/enums.py | 24 +- .../unit/api/test_engagement_membership.py | 284 ++++++------ met-api/tests/unit/api/test_user.py | 414 ++++++++++-------- .../tests/unit/api/test_user_membership.py | 163 +++---- met-api/tests/unit/services/test_keycloak.py | 30 +- .../UserManagement/AddTeamMemberModal.tsx | 2 +- .../listing/UserManagementContext.tsx | 2 +- .../userDetails/UserDetailsContext.tsx | 2 +- met-web/src/models/user.ts | 10 +- met-web/src/routes/AuthGate.tsx | 3 +- .../src/services/userService/api/index.tsx | 4 +- 23 files changed, 793 insertions(+), 790 deletions(-) diff --git a/CHANGELOG.MD b/CHANGELOG.MD index 30f93e860..80c4e5899 100644 --- a/CHANGELOG.MD +++ b/CHANGELOG.MD @@ -1,3 +1,9 @@ +## February 06, 2024 +- **Task**Convert keycloak groups to composite roles for permission levels [DESENG-447](https://apps.itsm.gov.bc.ca/jira/browse/DESENG-447) + - Commented out unit test related to Keycloak groups + - Changed reference of Keycloak `groups` to `roles` + - Commented out code related to Keycloak groups + ## February 06, 2024 - **Task** Streamline CRON jobs [DESENG-493](https://apps.itsm.gov.bc.ca/jira/browse/DESENG-493) - Aligned the CRON configuration and sample environment files with the structure used in the Met API. diff --git a/met-api/.DS_Store b/met-api/.DS_Store index f21bee2e693e8d55ffb72c2c40a29d85d54da0f4..9b9604455f160d33a60e78e5ac0060f028193a31 100644 GIT binary patch delta 81 zcmZoMXfc=|#>CJ*u~2NHo}wrd0|Nsi1A_nqgD-B`mu~2NHo}w@_0|Nsi1A_oVesWSyeiD!;Fqx5Yr8-EQnZc2vfT6%M y2O$R(bp&F)|6sttu(9VV+h%qSeh#3Cn;99uGf(ChG2{Rm$j|`9n>|GKFarR&CmPNG diff --git a/met-api/src/met_api/models/membership.py b/met-api/src/met_api/models/membership.py index 812f58d90..37a2b0167 100644 --- a/met-api/src/met_api/models/membership.py +++ b/met-api/src/met_api/models/membership.py @@ -93,7 +93,7 @@ def find_by_user_id( @classmethod def find_by_engagement_and_user_id(cls, eng_id, userid, status=None) \ -> Membership: - """Get a survey.""" + """Get a membership by engagement and user ID.""" query = db.session.query(Membership) \ .join(StaffUser, StaffUser.id == Membership.user_id) \ .filter(and_(Membership.engagement_id == eng_id, diff --git a/met-api/src/met_api/resources/engagement_members.py b/met-api/src/met_api/resources/engagement_members.py index c827568a8..4939c76fc 100644 --- a/met-api/src/met_api/resources/engagement_members.py +++ b/met-api/src/met_api/resources/engagement_members.py @@ -48,17 +48,18 @@ def get(engagement_id): except BusinessException as err: return {'message': err.error}, err.status_code - @staticmethod - @cross_origin(origins=allowedorigins()) - @_jwt.requires_auth - def post(engagement_id): - """Create a new membership.""" - # TODO validate against a schema. - try: - member = MembershipService.create_membership(engagement_id, request.get_json()) - return MembershipSchema().dump(member), HTTPStatus.OK - except BusinessException as err: - return {'message': err.error}, err.status_code + # TODO: Create membership method that uses composite roles + # @staticmethod + # @cross_origin(origins=allowedorigins()) + # @_jwt.requires_auth + # def post(engagement_id): + # """Create a new membership.""" + # # TODO validate against a schema. + # try: + # member = MembershipService.create_membership(engagement_id, request.get_json()) + # return MembershipSchema().dump(member), HTTPStatus.OK + # except BusinessException as err: + # return {'message': err.error}, err.status_code @cors_preflight('GET,OPTIONS') diff --git a/met-api/src/met_api/resources/staff_user.py b/met-api/src/met_api/resources/staff_user.py index a4947bf30..bacaeafbf 100644 --- a/met-api/src/met_api/resources/staff_user.py +++ b/met-api/src/met_api/resources/staff_user.py @@ -72,7 +72,7 @@ def get(): users = StaffUserService.find_users( pagination_options=pagination_options, search_text=args.get('search_text', '', str), - include_groups=args.get('include_groups', default=False, type=lambda v: v.lower() == 'true'), + include_roles=args.get('include_roles', default=False, type=lambda v: v.lower() == 'true'), include_inactive=args.get('include_inactive', default=False, type=lambda v: v.lower() == 'true') ) return jsonify(users), HTTPStatus.OK @@ -91,7 +91,7 @@ def get(user_id): args = request.args user = StaffUserService.get_user_by_id( user_id, - include_groups=args.get('include_groups', default=False, type=lambda v: v.lower() == 'true'), + include_roles=args.get('include_roles', default=False, type=lambda v: v.lower() == 'true'), include_inactive=True, ) return user, HTTPStatus.OK @@ -121,44 +121,6 @@ def patch(user_id): return str(err), HTTPStatus.BAD_REQUEST -@cors_preflight('POST, PUT') -@API.route('//groups') -class UserGroup(Resource): - """Add user to group.""" - - @staticmethod - @cross_origin(origins=allowedorigins()) - @require_role([Role.CREATE_ADMIN_USER.value], skip_tenant_check_for_admin=True) - def post(user_id): - """Add user to group.""" - try: - args = request.args - user_schema = StaffUserService().add_user_to_group(user_id, args.get('group')) - return user_schema, HTTPStatus.OK - except KeyError as err: - return str(err), HTTPStatus.INTERNAL_SERVER_ERROR - except ValueError as err: - return str(err), HTTPStatus.INTERNAL_SERVER_ERROR - except BusinessException as err: - return {'message': err.error}, err.status_code - - @staticmethod - @cross_origin(origins=allowedorigins()) - @_jwt.has_one_of_roles([Role.UPDATE_USER_GROUP.value]) - def put(user_id): - """Update user group.""" - try: - args = request.args - user_schema = StaffUserMembershipService().reassign_user(user_id, args.get('group')) - return user_schema, HTTPStatus.OK - except KeyError as err: - return str(err), HTTPStatus.INTERNAL_SERVER_ERROR - except ValueError as err: - return str(err), HTTPStatus.INTERNAL_SERVER_ERROR - except BusinessException as err: - return {'message': err.error}, err.status_code - - @cors_preflight('GET,OPTIONS') @API.route('//engagements') class EngagementMemberships(Resource): diff --git a/met-api/src/met_api/services/authorization.py b/met-api/src/met_api/services/authorization.py index e3d1862be..bce03218c 100644 --- a/met-api/src/met_api/services/authorization.py +++ b/met-api/src/met_api/services/authorization.py @@ -31,14 +31,15 @@ def check_auth(**kwargs): has_valid_roles = token_roles & permitted_roles if has_valid_roles: if not skip_tenant_check: + user_tenant_id = user_from_db.tenant_id _validate_tenant(kwargs.get('engagement_id'), user_tenant_id) return - team_permitted_roles = {MembershipType.TEAM_MEMBER.name, MembershipType.REVIEWER.name} & permitted_roles if team_permitted_roles: # check if he is a member of particular engagement. + has_valid_team_access = _has_team_membership(kwargs, user_from_context, team_permitted_roles) if has_valid_team_access: return @@ -63,16 +64,19 @@ def _has_team_membership(kwargs, user_from_context, team_permitted_roles) -> boo eng_id = kwargs.get('engagement_id') if not eng_id: + return False user = StaffUserModel.get_user_by_external_id(user_from_context.sub) if not user: + return False membership = MembershipModel.find_by_engagement_and_user_id(eng_id, user.id, status=MembershipStatus.ACTIVE.value) if not membership: + return False skip_tenant_check = current_app.config.get('IS_SINGLE_TENANT_ENVIRONMENT') diff --git a/met-api/src/met_api/services/keycloak.py b/met-api/src/met_api/services/keycloak.py index 0e4bf3fe4..7ab9e0305 100644 --- a/met-api/src/met_api/services/keycloak.py +++ b/met-api/src/met_api/services/keycloak.py @@ -14,7 +14,6 @@ """Utils for keycloak administration.""" import json -from typing import List import requests from flask import current_app @@ -25,80 +24,80 @@ class KeycloakService: # pylint: disable=too-few-public-methods """Keycloak services.""" - @staticmethod - def get_user_groups(user_id): - """Get user group from Keycloak by userid.""" - keycloak = current_app.config['KEYCLOAK_CONFIG'] - timeout = keycloak['CONNECT_TIMEOUT'] - base_url = keycloak['BASE_URL'] - realm = keycloak['REALMNAME'] - admin_token = KeycloakService._get_admin_token() - headers = { - 'Content-Type': ContentType.JSON.value, - 'Authorization': f'Bearer {admin_token}' - } - - # Get the user and return - query_user_url = f'{base_url}/admin/realms/{realm}/users/{user_id}/groups' - response = requests.get(query_user_url, headers=headers, timeout=timeout) - response.raise_for_status() - return response.json() - - @staticmethod - def get_users_groups(user_ids: List): - """Get user groups from Keycloak by user ids.For bulk purposes.""" - # TODO if List is bigger than a number ; if so reject. - keycloak = current_app.config['KEYCLOAK_CONFIG'] - base_url = keycloak['BASE_URL'] - # TODO fix this during tests and remove below - if not base_url: - return {} - keycloak = current_app.config['KEYCLOAK_CONFIG'] - realm = keycloak['REALMNAME'] - timeout = keycloak['CONNECT_TIMEOUT'] - admin_token = KeycloakService._get_admin_token() - headers = { - 'Content-Type': ContentType.JSON.value, - 'Authorization': f'Bearer {admin_token}' - } - user_group_mapping = {} - # Get the user and return - for user_id in user_ids: - query_user_url = f'{base_url}/admin/realms/{realm}/users/{user_id}/groups' - response = requests.get(query_user_url, headers=headers, timeout=timeout) - - if response.status_code == 200: - if (groups := response.json()) is not None: - user_group_mapping[user_id] = [group.get('name') for group in groups] - else: - user_group_mapping[user_id] = [] - - return user_group_mapping - - @staticmethod - def _get_group_id(admin_token: str, group_name: str): - """Get a group id for the group name.""" - keycloak = current_app.config['KEYCLOAK_CONFIG'] - base_url = keycloak['BASE_URL'] - realm = keycloak['REALMNAME'] - timeout = keycloak['CONNECT_TIMEOUT'] - get_group_url = f'{base_url}/admin/realms/{realm}/groups?search={group_name}' - headers = { - 'Content-Type': ContentType.JSON.value, - 'Authorization': f'Bearer {admin_token}' - } - response = requests.get(get_group_url, headers=headers, timeout=timeout) - return KeycloakService._find_group_or_subgroup_id(response.json(), group_name) - - @staticmethod - def _find_group_or_subgroup_id(groups: list, group_name: str): - """Return group id by searching main and sub groups.""" - for group in groups: - if group['name'] == group_name: - return group['id'] - if group_id := KeycloakService._find_group_or_subgroup_id(group['subGroups'], group_name): - return group_id - return None + # @staticmethod + # def get_user_groups(user_id): + # """Get user group from Keycloak by userid.""" + # keycloak = current_app.config['KEYCLOAK_CONFIG'] + # timeout = keycloak['CONNECT_TIMEOUT'] + # base_url = keycloak['BASE_URL'] + # realm = keycloak['REALMNAME'] + # admin_token = KeycloakService._get_admin_token() + # headers = { + # 'Content-Type': ContentType.JSON.value, + # 'Authorization': f'Bearer {admin_token}' + # } + + # # Get the user and return + # query_user_url = f'{base_url}/admin/realms/{realm}/users/{user_id}/groups' + # response = requests.get(query_user_url, headers=headers, timeout=timeout) + # response.raise_for_status() + # return response.json() + + # @staticmethod + # def get_users_groups(user_ids: List): + # """Get user groups from Keycloak by user ids.For bulk purposes.""" + # # TODO if List is bigger than a number ; if so reject. + # keycloak = current_app.config['KEYCLOAK_CONFIG'] + # base_url = keycloak['BASE_URL'] + # # TODO fix this during tests and remove below + # if not base_url: + # return {} + # keycloak = current_app.config['KEYCLOAK_CONFIG'] + # realm = keycloak['REALMNAME'] + # timeout = keycloak['CONNECT_TIMEOUT'] + # admin_token = KeycloakService._get_admin_token() + # headers = { + # 'Content-Type': ContentType.JSON.value, + # 'Authorization': f'Bearer {admin_token}' + # } + # user_group_mapping = {} + # # Get the user and return + # for user_id in user_ids: + # query_user_url = f'{base_url}/admin/realms/{realm}/users/{user_id}/groups' + # response = requests.get(query_user_url, headers=headers, timeout=timeout) + + # if response.status_code == 200: + # if (groups := response.json()) is not None: + # user_group_mapping[user_id] = [group.get('name') for group in groups] + # else: + # user_group_mapping[user_id] = [] + + # return user_group_mapping + + # @staticmethod + # def _get_group_id(admin_token: str, group_name: str): + # """Get a group id for the group name.""" + # keycloak = current_app.config['KEYCLOAK_CONFIG'] + # base_url = keycloak['BASE_URL'] + # realm = keycloak['REALMNAME'] + # timeout = keycloak['CONNECT_TIMEOUT'] + # get_group_url = f'{base_url}/admin/realms/{realm}/groups?search={group_name}' + # headers = { + # 'Content-Type': ContentType.JSON.value, + # 'Authorization': f'Bearer {admin_token}' + # } + # response = requests.get(get_group_url, headers=headers, timeout=timeout) + # return KeycloakService._find_group_or_subgroup_id(response.json(), group_name) + + # @staticmethod + # def _find_group_or_subgroup_id(groups: list, group_name: str): + # """Return group id by searching main and sub groups.""" + # for group in groups: + # if group['name'] == group_name: + # return group['id'] + # if group_id := KeycloakService._find_group_or_subgroup_id(group['subGroups'], group_name): + # return group_id + # return None @staticmethod def _get_admin_token(): @@ -122,49 +121,49 @@ def _get_admin_token(): ) return response.json().get('access_token') - @staticmethod - def _remove_user_from_group(user_id: str, group_name: str): - """Remove user from the keycloak group.""" - keycloak = current_app.config['KEYCLOAK_CONFIG'] - base_url = keycloak['BASE_URL'] - realm = keycloak['REALMNAME'] - timeout = keycloak['CONNECT_TIMEOUT'] - # Create an admin token - admin_token = KeycloakService._get_admin_token() - # Get the '$group_name' group - group_id = KeycloakService._get_group_id(admin_token, group_name) - - # Add user to the keycloak group '$group_name' - headers = { - 'Content-Type': ContentType.JSON.value, - 'Authorization': f'Bearer {admin_token}' - } - remove_group_url = f'{base_url}/admin/realms/{realm}/users/{user_id}/groups/{group_id}' - response = requests.delete(remove_group_url, headers=headers, - timeout=timeout) - response.raise_for_status() - - @staticmethod - def add_user_to_group(user_id: str, group_name: str): - """Add user to the keycloak group.""" - keycloak = current_app.config['KEYCLOAK_CONFIG'] - base_url = keycloak['BASE_URL'] - realm = keycloak['REALMNAME'] - timeout = keycloak['CONNECT_TIMEOUT'] - # Create an admin token - admin_token = KeycloakService._get_admin_token() - # Get the '$group_name' group - group_id = KeycloakService._get_group_id(admin_token, group_name) - - # Add user to the keycloak group '$group_name' - headers = { - 'Content-Type': ContentType.JSON.value, - 'Authorization': f'Bearer {admin_token}' - } - add_to_group_url = f'{base_url}/admin/realms/{realm}/users/{user_id}/groups/{group_id}' - response = requests.put(add_to_group_url, headers=headers, - timeout=timeout) - response.raise_for_status() + # @staticmethod + # def _remove_user_from_group(user_id: str, group_name: str): + # """Remove user from the keycloak group.""" + # keycloak = current_app.config['KEYCLOAK_CONFIG'] + # base_url = keycloak['BASE_URL'] + # realm = keycloak['REALMNAME'] + # timeout = keycloak['CONNECT_TIMEOUT'] + # # Create an admin token + # admin_token = KeycloakService._get_admin_token() + # # Get the '$group_name' group + # group_id = KeycloakService._get_group_id(admin_token, group_name) + + # # Add user to the keycloak group '$group_name' + # headers = { + # 'Content-Type': ContentType.JSON.value, + # 'Authorization': f'Bearer {admin_token}' + # } + # remove_group_url = f'{base_url}/admin/realms/{realm}/users/{user_id}/groups/{group_id}' + # response = requests.delete(remove_group_url, headers=headers, + # timeout=timeout) + # response.raise_for_status() + + # @staticmethod + # def add_user_to_group(user_id: str, group_name: str): + # """Add user to the keycloak group.""" + # keycloak = current_app.config['KEYCLOAK_CONFIG'] + # base_url = keycloak['BASE_URL'] + # realm = keycloak['REALMNAME'] + # timeout = keycloak['CONNECT_TIMEOUT'] + # # Create an admin token + # admin_token = KeycloakService._get_admin_token() + # # Get the '$group_name' group + # group_id = KeycloakService._get_group_id(admin_token, group_name) + + # # Add user to the keycloak group '$group_name' + # headers = { + # 'Content-Type': ContentType.JSON.value, + # 'Authorization': f'Bearer {admin_token}' + # } + # add_to_group_url = f'{base_url}/admin/realms/{realm}/users/{user_id}/groups/{group_id}' + # response = requests.put(add_to_group_url, headers=headers, + # timeout=timeout) + # response.raise_for_status() @staticmethod def add_attribute_to_user(user_id: str, attribute_value: str, attribute_id: str = 'tenant_id'): @@ -186,26 +185,26 @@ def add_attribute_to_user(user_id: str, attribute_value: str, attribute_id: str requests.put(user_url, json=user_data, headers=headers) response.raise_for_status() - @staticmethod - def remove_user_from_group(user_id: str, group_name: str): - """Remove user from the keycloak group.""" - keycloak = current_app.config['KEYCLOAK_CONFIG'] - base_url = keycloak['BASE_URL'] - realm = keycloak['REALMNAME'] - timeout = keycloak['CONNECT_TIMEOUT'] - # Create an admin token - admin_token = KeycloakService._get_admin_token() - # Get the '$group_name' group - group_id = KeycloakService._get_group_id(admin_token, group_name) - - # Remove user from the keycloak group '$group_name' - headers = { - 'Content-Type': ContentType.JSON.value, - 'Authorization': f'Bearer {admin_token}' - } - remove_from_group_url = f'{base_url}/admin/realms/{realm}/users/{user_id}/groups/{group_id}' - response = requests.delete(remove_from_group_url, headers=headers, timeout=timeout) - response.raise_for_status() + # @staticmethod + # def remove_user_from_group(user_id: str, group_name: str): + # """Remove user from the keycloak group.""" + # keycloak = current_app.config['KEYCLOAK_CONFIG'] + # base_url = keycloak['BASE_URL'] + # realm = keycloak['REALMNAME'] + # timeout = keycloak['CONNECT_TIMEOUT'] + # # Create an admin token + # admin_token = KeycloakService._get_admin_token() + # # Get the '$group_name' group + # group_id = KeycloakService._get_group_id(admin_token, group_name) + + # # Remove user from the keycloak group '$group_name' + # headers = { + # 'Content-Type': ContentType.JSON.value, + # 'Authorization': f'Bearer {admin_token}' + # } + # remove_from_group_url = f'{base_url}/admin/realms/{realm}/users/{user_id}/groups/{group_id}' + # response = requests.delete(remove_from_group_url, headers=headers, timeout=timeout) + # response.raise_for_status() @staticmethod def add_user(user: dict): @@ -217,7 +216,6 @@ def add_user(user: dict): realm = keycloak['REALMNAME'] timeout = keycloak['CONNECT_TIMEOUT'] - # Add user to the keycloak group '$group_name' headers = { 'Content-Type': ContentType.JSON.value, 'Authorization': f'Bearer {admin_token}' diff --git a/met-api/src/met_api/services/membership_service.py b/met-api/src/met_api/services/membership_service.py index d8015d4b1..5eb6401b6 100644 --- a/met-api/src/met_api/services/membership_service.py +++ b/met-api/src/met_api/services/membership_service.py @@ -4,14 +4,10 @@ from met_api.constants.membership_type import MembershipType from met_api.exceptions.business_exception import BusinessException -from met_api.models import StaffUser as StaffUserModel from met_api.models.engagement import Engagement as EngagementModel from met_api.models.membership import Membership as MembershipModel -from met_api.schemas.staff_user import StaffUserSchema from met_api.services import authorization -from met_api.services.staff_user_service import KEYCLOAK_SERVICE, StaffUserService -from met_api.utils.constants import Groups -from met_api.utils.enums import KeycloakGroups, MembershipStatus +from met_api.utils.enums import MembershipStatus from met_api.utils.roles import Role from met_api.utils.token_info import TokenInfo @@ -19,30 +15,30 @@ class MembershipService: """Membership management service.""" - @staticmethod - def create_membership(engagement_id, request_json: dict): - """Create membership.""" - user_id = request_json.get('user_id') - user: StaffUserModel = StaffUserModel.get_user_by_external_id(user_id) - if not user: - raise BusinessException( - error='Invalid User.', - status_code=HTTPStatus.BAD_REQUEST) - - one_of_roles = ( - MembershipType.TEAM_MEMBER.name, - Role.EDIT_MEMBERS.value - ) - authorization.check_auth(one_of_roles=one_of_roles, engagement_id=engagement_id) - - user_details = StaffUserSchema().dump(user) - # attach and map groups - StaffUserService.attach_groups([user_details]) - MembershipService._validate_create_membership(engagement_id, user_details) - group_name, membership_type = MembershipService._get_membership_details(user_details) - MembershipService._add_user_group(user_details, group_name) - membership = MembershipService._create_membership_model(engagement_id, user.id, membership_type) - return membership + # TODO: Create membership method that uses composite roles + # @staticmethod + # def create_membership(engagement_id, request_json: dict): + # """Create membership.""" + # user_id = request_json.get('user_id') + # user: StaffUserModel = StaffUserModel.get_user_by_external_id(user_id) + # if not user: + # raise BusinessException( + # error='Invalid User.', + # status_code=HTTPStatus.BAD_REQUEST) + + # one_of_roles = ( + # MembershipType.TEAM_MEMBER.name, + # Role.EDIT_MEMBERS.value + # ) + # authorization.check_auth(one_of_roles=one_of_roles, engagement_id=engagement_id) + + # user_details = StaffUserSchema().dump(user) + + # MembershipService._validate_create_membership(engagement_id, user_details) + # group_name, membership_type = MembershipService._get_membership_details(user_details) + # MembershipService._add_user_group(user_details, group_name) + # membership = MembershipService._create_membership_model(engagement_id, user.id, membership_type) + # return membership @staticmethod def _validate_create_membership(engagement_id, user_details): @@ -55,11 +51,12 @@ def _validate_create_membership(engagement_id, user_details): user_id = user_details.get('id') - groups = user_details.get('groups') - if KeycloakGroups.EAO_IT_ADMIN.value in groups: - raise BusinessException( - error='This user is already an Administrator.', - status_code=HTTPStatus.CONFLICT.value) + # TODO: Check for permission level once composite role permission levels are added. + # roles = user_details.get('roles') + # if KeycloakPermissionLevels.IT_ADMIN.value in roles: + # raise BusinessException( + # error='This user is already a Administrator.', + # status_code=HTTPStatus.CONFLICT.value) existing_membership = MembershipModel.find_by_engagement_and_user_id( engagement_id, @@ -78,43 +75,45 @@ def _validate_create_membership(engagement_id, user_details): error='You cannot add yourself to an engagement.', status_code=HTTPStatus.FORBIDDEN.value) - @staticmethod - def _get_membership_details(user_details): - """Get the group name and membership type for the user based on their assigned groups.""" - default_group_name = Groups.EAO_TEAM_MEMBER.name - default_membership_type = MembershipType.TEAM_MEMBER - - is_reviewer = Groups.EAO_REVIEWER.value in user_details.get('groups') - is_team_member = Groups.EAO_TEAM_MEMBER.value in user_details.get('groups') - - if is_reviewer: - # If the user is assigned to the EAO_REVIEWER group, set the group name and membership type accordingly - group_name = Groups.EAO_REVIEWER.name - membership_type = MembershipType.REVIEWER - elif is_team_member: - # If the user is assigned to the EAO_TEAM_MEMBER group, set the group name and membership type accordingly - group_name = Groups.EAO_TEAM_MEMBER.name - membership_type = MembershipType.TEAM_MEMBER - else: - # If the user is not assigned to either group, return default values for group name and membership type - group_name = default_group_name - membership_type = default_membership_type - - return group_name, membership_type - - @staticmethod - def _add_user_group(user: StaffUserModel, group_name=Groups.EAO_TEAM_MEMBER.name): - valid_member_teams = [Groups.EAO_TEAM_MEMBER.name, Groups.EAO_REVIEWER.name] - if group_name not in valid_member_teams: - raise BusinessException( - error='Invalid Group name.', - status_code=HTTPStatus.BAD_REQUEST - ) - - KEYCLOAK_SERVICE.add_user_to_group( - user_id=user.get('external_id'), - group_name=group_name - ) + # TODO: Replace this method with one that checks membership type with composite roles + # @staticmethod + # def _get_membership_details(user_details): + # """Get the group name and membership type for the user based on their assigned groups.""" + # default_group_name = Groups.TEAM_MEMBER.name + # default_membership_type = MembershipType.TEAM_MEMBER + + # is_reviewer = Groups.REVIEWER.value in user_details.get('groups') + # is_team_member = Groups.TEAM_MEMBER.value in user_details.get('groups') + + # if is_reviewer: + # # If the user is assigned to the REVIEWER group, set the group name and membership type accordingly + # group_name = Groups.REVIEWER.name + # membership_type = MembershipType.REVIEWER + # elif is_team_member: + # # If the user is assigned to the TEAM_MEMBER group, set the group name and membership type accordingly + # group_name = Groups.TEAM_MEMBER.name + # membership_type = MembershipType.TEAM_MEMBER + # else: + # # If the user is not assigned to either group, return default values for group name and membership type + # group_name = default_group_name + # membership_type = default_membership_type + + # return group_name, membership_type + + # TODO: Replace this method with a method to add composite roles + # @staticmethod + # def _add_user_group(user: StaffUserModel, group_name=Groups.TEAM_MEMBER.name): + # valid_member_teams = [Groups.TEAM_MEMBER.name, Groups.REVIEWER.name] + # if group_name not in valid_member_teams: + # raise BusinessException( + # error='Invalid Group name.', + # status_code=HTTPStatus.BAD_REQUEST + # ) + + # KEYCLOAK_SERVICE.add_user_to_group( + # user_id=user.get('external_id'), + # group_name=group_name + # ) @staticmethod def _create_membership_model(engagement_id, user_id, membership_type=MembershipType.TEAM_MEMBER): diff --git a/met-api/src/met_api/services/staff_user_membership_service.py b/met-api/src/met_api/services/staff_user_membership_service.py index ebe470f06..e698e58a7 100644 --- a/met-api/src/met_api/services/staff_user_membership_service.py +++ b/met-api/src/met_api/services/staff_user_membership_service.py @@ -7,51 +7,40 @@ from met_api.services.membership_service import MembershipService from met_api.services.staff_user_service import KEYCLOAK_SERVICE, StaffUserService from met_api.utils.user_context import UserContext, user_context -from met_api.utils.constants import Groups from met_api.utils.enums import UserStatus class StaffUserMembershipService: """Staff User Membership management service.""" + # TODO: Restore a way to add users to composite roles. @classmethod @user_context - def reassign_user(cls, user_id, group_name, **kwargs): - """Add user to a new group and reassign memberships.""" - user = StaffUserService.get_user_by_id(user_id, include_groups=True) + def reassign_user(cls, user_id, **kwargs): + """Add user to a new composite role and reassign memberships.""" + user = StaffUserService.get_user_by_id(user_id, include_roles=True) if not user: raise BusinessException( error='Invalid User.', status_code=HTTPStatus.BAD_REQUEST) external_id = user.get('external_id', None) - main_group = user.get('main_group', None) - if any([not external_id, not main_group]): + # TODO: Put check for composite role membership into this conditional. + if not external_id: raise BusinessException( error='Invalid User.', status_code=HTTPStatus.BAD_REQUEST) - if group_name not in Groups.__members__: - raise BusinessException( - error='Invalid Group.', - status_code=HTTPStatus.BAD_REQUEST) - - if main_group == group_name: - raise BusinessException( - error='User is already a member of this group.', - status_code=HTTPStatus.BAD_REQUEST) - user_from_context: UserContext = kwargs['user_context'] + if external_id == user_from_context.sub: raise BusinessException( - error='User cannot change their own group.', + error='User cannot change their own permission level.', status_code=HTTPStatus.CONFLICT.value) - StaffUserService.remove_user_from_group(external_id, Groups.get_name_by_value(main_group)) - StaffUserService.add_user_to_group(external_id, group_name) MembershipService.revoke_memberships_bulk(user_id) - new_user = StaffUserService.get_user_by_id(user_id, include_groups=True) + new_user = StaffUserService.get_user_by_id(user_id, include_roles=True) return StaffUserSchema().dump(new_user) @staticmethod diff --git a/met-api/src/met_api/services/staff_user_service.py b/met-api/src/met_api/services/staff_user_service.py index e7f7ba83b..4092a6f9b 100644 --- a/met-api/src/met_api/services/staff_user_service.py +++ b/met-api/src/met_api/services/staff_user_service.py @@ -9,8 +9,6 @@ from met_api.schemas.staff_user import StaffUserSchema from met_api.services.keycloak import KeycloakService from met_api.utils import notification -from met_api.utils.constants import GROUP_NAME_MAPPING, Groups -from met_api.utils.enums import KeycloakGroupName from met_api.utils.template import Template KEYCLOAK_SERVICE = KeycloakService() @@ -20,13 +18,15 @@ class StaffUserService: """User management service.""" @classmethod - def get_user_by_id(cls, _user_id, include_groups=False, include_inactive=False): + def get_user_by_id(cls, _user_id, include_roles=False, include_inactive=False): """Get user by id.""" user_schema = StaffUserSchema() db_user = StaffUserModel.get_by_id(_user_id, include_inactive) user = user_schema.dump(db_user) - if include_groups: - cls.attach_groups([user]) + if include_roles: + # TODO: Replace this method with one that uses composite roles + # cls.attach_roles([user]) + pass return user @classmethod @@ -100,42 +100,47 @@ def _render_email_template(user: StaffUserModel): ) return subject, body, args - @staticmethod - def attach_groups(user_collection): - """Attach keycloak groups to user object.""" - group_user_details = KEYCLOAK_SERVICE.get_users_groups( - [user.get('external_id') for user in user_collection]) - - for user in user_collection: - # Transform group name from EAO_ADMINISTRATOR to Administrator - # TODO etc;Arrive at a better implementation than keeping a static list - # TODO Probably add a custom attribute in the keycloak as title against a group? - groups = group_user_details.get(user.get('external_id')) - user['groups'] = '' - if groups: - user['groups'] = [GROUP_NAME_MAPPING.get(group, '') for group in groups] - if Groups.EAO_IT_ADMIN.value in user['groups']: - user['main_group'] = Groups.EAO_IT_ADMIN.value - elif Groups.EAO_TEAM_MEMBER.value in user['groups']: - user['main_group'] = Groups.EAO_TEAM_MEMBER.value - elif Groups.EAO_REVIEWER.value in user['groups']: - user['main_group'] = Groups.EAO_REVIEWER.value - else: - user['main_group'] = user['groups'][0] + # TODO: Replace this method with one that uses composite roles, if necessary + # @staticmethod + # def attach_roles(user_collection): + # """Attach keycloak groups to user object.""" + # group_user_details = KEYCLOAK_SERVICE.get_users_groups( + # [user.get('external_id') for user in user_collection]) + + # for user in user_collection: + # # Transform group name from ADMINISTRATOR to Administrator + # # TODO etc;Arrive at a better implementation than keeping a static list + # # TODO Probably add a custom attribute in the keycloak as title against a group? + # groups = group_user_details.get(user.get('external_id')) + # user['groups'] = '' + # if groups: + # user['groups'] = [GROUP_NAME_MAPPING.get(group, '') for group in groups] + # if Groups.IT_ADMIN.value in user['groups']: + # user['main_group'] = Groups.IT_ADMIN.value + # elif Groups.TEAM_MEMBER.value in user['groups']: + # user['main_group'] = Groups.TEAM_MEMBER.value + # elif Groups.REVIEWER.value in user['groups']: + # user['main_group'] = Groups.REVIEWER.value + # else: + # user['main_group'] = user['groups'][0] @classmethod def find_users( cls, pagination_options: PaginationOptions = None, search_text='', - include_groups=False, + include_roles=False, include_inactive=False ): """Return a list of users.""" users, total = StaffUserModel.get_all_paginated(pagination_options, search_text, include_inactive) user_collection = StaffUserSchema(many=True).dump(users) - if include_groups: - cls.attach_groups(user_collection) + + if include_roles: + # TODO: Replace this method with one that uses composite roles + # cls.attach_roles(user_collection) + pass + return { 'items': user_collection, 'total': total @@ -161,7 +166,9 @@ def add_user_to_group(cls, external_id: str, group_name: str): cls.validate_user(db_user) - KEYCLOAK_SERVICE.add_user_to_group(user_id=external_id, group_name=group_name) + # TODO: Replace this method with one that uses composite roles + print(group_name) + # KEYCLOAK_SERVICE.add_user_to_group(user_id=external_id, group_name=group_name) KEYCLOAK_SERVICE.add_attribute_to_user(user_id=external_id, attribute_value=g.tenant_id) return StaffUserSchema().dump(db_user) @@ -174,7 +181,9 @@ def remove_user_from_group(cls, external_id: str, group_name: str): if db_user is None: raise KeyError('User not found') - KEYCLOAK_SERVICE.remove_user_from_group(user_id=external_id, group_name=group_name) + # TODO: Replace this method with one that uses composite roles + print(group_name) + # KEYCLOAK_SERVICE.remove_user_from_group(user_id=external_id, group_name=group_name) return StaffUserSchema().dump(db_user) @@ -184,9 +193,10 @@ def validate_user(db_user: StaffUserModel): if db_user is None: raise KeyError('User not found') - groups = KEYCLOAK_SERVICE.get_user_groups(user_id=db_user.external_id) - group_names = [group.get('name') for group in groups] - if KeycloakGroupName.EAO_IT_ADMIN.value in group_names: - raise BusinessException( - error='This user is already an Administrator.', - status_code=HTTPStatus.CONFLICT.value) + # TODO: Restore permission level functionality to replace "groups" later + # groups = KEYCLOAK_SERVICE.get_user_groups(user_id=db_user.external_id) + # group_names = [group.get('name') for group in groups] + # if KeycloakGroupName.IT_ADMIN.value in group_names: + # raise BusinessException( + # error='This user is already an Administrator.', + # status_code=HTTPStatus.CONFLICT.value) diff --git a/met-api/src/met_api/services/tenant_service.py b/met-api/src/met_api/services/tenant_service.py index 2992e79f8..0797bee97 100644 --- a/met-api/src/met_api/services/tenant_service.py +++ b/met-api/src/met_api/services/tenant_service.py @@ -27,5 +27,5 @@ def get(cls, tenant_id): """Get a tenant by id.""" tenant = TenantModel.find_by_short_name(tenant_id) if not tenant: - raise ValueError('Tenant not found.') + raise ValueError('Tenant not found.', cls, tenant_id) return TenantSchema().dump(tenant) diff --git a/met-api/src/met_api/utils/constants.py b/met-api/src/met_api/utils/constants.py index 07c82c28b..5a7202c87 100644 --- a/met-api/src/met_api/utils/constants.py +++ b/met-api/src/met_api/utils/constants.py @@ -13,30 +13,28 @@ # limitations under the License. """Constants definitions.""" -from enum import Enum +# from enum import Enum +# TODO Remove this +# class Groups(Enum): +# """Enumeration representing user groups.""" -class Groups(Enum): - """Enumeration representing user groups.""" +# IT_ADMIN = 'Administrator' +# TEAM_MEMBER = 'Team Member' +# REVIEWER = 'Reviewer' +# IT_VIEWER = 'Viewer' - EAO_IT_ADMIN = 'Administrator' - EAO_TEAM_MEMBER = 'Team Member' - EAO_REVIEWER = 'Reviewer' - EAO_IT_VIEWER = 'Viewer' - - @staticmethod - def get_name_by_value(value): - """Get the name of a group by its value.""" - for group in Groups: - if group.value == value: - return group.name - raise ValueError('No matching key found for the given value.') +# @staticmethod +# def get_name_by_value(value): +# """Get the name of a group by its value.""" +# for group in Groups: +# if group.value == value: +# return group.name +# raise ValueError('No matching key found for the given value.') TENANT_ID_HEADER = 'tenant-id' -GROUP_NAME_MAPPING = {group.name: group.value for group in Groups} - TENANT_ID_JWT_CLAIM = 'tenant_id' diff --git a/met-api/src/met_api/utils/enums.py b/met-api/src/met_api/utils/enums.py index 93fe76cd7..64de3e805 100644 --- a/met-api/src/met_api/utils/enums.py +++ b/met-api/src/met_api/utils/enums.py @@ -59,22 +59,22 @@ class LoginSource(Enum): IDIR = 'idir' -class KeycloakGroups(Enum): - """Login Source.""" +class KeycloakPermissionLevels(Enum): + """Keycloak permission levels.""" - EAO_IT_ADMIN = 'Administrator' - EAO_IT_VIEWER = 'Viewer' - EAO_TEAM_MEMBER = 'Member' - EAO_REVIEWER = 'Reviewer' + IT_ADMIN = 'Administrator' + IT_VIEWER = 'Viewer' + TEAM_MEMBER = 'Member' + REVIEWER = 'Reviewer' -class KeycloakGroupName(Enum): - """Keycloak group names.""" +class KeycloakCompositeRoleNames(Enum): + """Keycloak composite role names.""" - EAO_IT_ADMIN = 'EAO_IT_ADMIN' - EAO_IT_VIEWER = 'EAO_IT_VIEWER' - EAO_TEAM_MEMBER = 'EAO_TEAM_MEMBER' - EAO_REVIEWER = 'EAO_REVIEWER' + IT_ADMIN = 'IT_ADMIN' + IT_VIEWER = 'IT_VIEWER' + TEAM_MEMBER = 'TEAM_MEMBER' + REVIEWER = 'REVIEWER' class MembershipType(IntEnum): diff --git a/met-api/tests/unit/api/test_engagement_membership.py b/met-api/tests/unit/api/test_engagement_membership.py index 711ab3bd8..2c29dd40b 100644 --- a/met-api/tests/unit/api/test_engagement_membership.py +++ b/met-api/tests/unit/api/test_engagement_membership.py @@ -5,118 +5,116 @@ """ import json from http import HTTPStatus -from unittest.mock import MagicMock, patch -import pytest +from unittest.mock import patch -from met_api.constants.membership_type import MembershipType from met_api.exceptions.business_exception import BusinessException from met_api.services.membership_service import MembershipService -from met_api.utils.enums import ContentType, KeycloakGroupName, MembershipStatus +from met_api.utils.enums import ContentType, MembershipStatus from tests.utilities.factory_utils import ( factory_auth_header, factory_engagement_model, factory_membership_model, factory_staff_user_model) memberships_url = '/api/engagements/{}/members' - -def test_create_engagement_membership_team_member(mocker, client, jwt, session, - setup_admin_user_and_claims): - """Assert that a team member engagement membership can be created.""" - user, claims = setup_admin_user_and_claims - engagement = factory_engagement_model() - staff_user = factory_staff_user_model() - headers = factory_auth_header(jwt=jwt, claims=claims) - - mock_add_user_to_group_keycloak_response = MagicMock() - mock_add_user_to_group_keycloak_response.status_code = HTTPStatus.NO_CONTENT - mock_add_user_to_group_keycloak = mocker.patch( - 'met_api.services.keycloak.KeycloakService.add_user_to_group', - return_value=mock_add_user_to_group_keycloak_response - ) - mock_get_users_groups_keycloak = mocker.patch( - 'met_api.services.keycloak.KeycloakService.get_users_groups', - return_value={staff_user.external_id: [KeycloakGroupName.EAO_TEAM_MEMBER.value]} - ) - - data = {'user_id': staff_user.external_id} - - rv = client.post( - memberships_url.format(engagement.id), - data=json.dumps(data), - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == HTTPStatus.OK - assert rv.json.get('engagement_id') == engagement.id - assert rv.json.get('user_id') == staff_user.id - assert rv.json.get('type') == MembershipType.TEAM_MEMBER - assert rv.json.get('status') == MembershipStatus.ACTIVE.value - mock_add_user_to_group_keycloak.assert_called() - mock_get_users_groups_keycloak.assert_called() - - with patch.object(MembershipService, 'create_membership', - side_effect=BusinessException('Test error', status_code=HTTPStatus.INTERNAL_SERVER_ERROR)): - rv = client.post( - memberships_url.format(engagement.id), - data=json.dumps(data), - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == HTTPStatus.INTERNAL_SERVER_ERROR - - -def test_create_engagement_membership_reviewer(mocker, client, jwt, session, - setup_admin_user_and_claims): - """Assert that a reviewer engagement membership can be created.""" - user, claims = setup_admin_user_and_claims - engagement = factory_engagement_model() - staff_user = factory_staff_user_model() - headers = factory_auth_header(jwt=jwt, claims=claims) - - mock_add_user_to_group_keycloak_response = MagicMock() - mock_add_user_to_group_keycloak_response.status_code = HTTPStatus.NO_CONTENT - mock_add_user_to_group_keycloak = mocker.patch( - 'met_api.services.keycloak.KeycloakService.add_user_to_group', - return_value=mock_add_user_to_group_keycloak_response - ) - mock_get_users_groups_keycloak = mocker.patch( - 'met_api.services.keycloak.KeycloakService.get_users_groups', - return_value={staff_user.external_id: [KeycloakGroupName.EAO_REVIEWER.value]} - ) - - data = {'user_id': staff_user.external_id} - - rv = client.post( - memberships_url.format(engagement.id), - data=json.dumps(data), - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == HTTPStatus.OK - assert rv.json.get('engagement_id') == engagement.id - assert rv.json.get('user_id') == staff_user.id - assert rv.json.get('type') == MembershipType.REVIEWER - assert rv.json.get('status') == MembershipStatus.ACTIVE.value - mock_add_user_to_group_keycloak.assert_called() - mock_get_users_groups_keycloak.assert_called() - - -def test_create_engagement_membership_unauthorized(client, jwt, session, - setup_unprivileged_user_and_claims): - """Assert that creating an engagement membership without proper authorization fails.""" - user, claims = setup_unprivileged_user_and_claims - engagement = factory_engagement_model() - staff_user = factory_staff_user_model() - headers = factory_auth_header(jwt=jwt, claims=claims) - data = {'user_id': staff_user.external_id} - - rv = client.post( - memberships_url.format(engagement.id), - data=json.dumps(data), - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == HTTPStatus.FORBIDDEN +# TODO: Replace this test with one that adds composite roles to user +# def test_create_engagement_membership_team_member(mocker, client, jwt, session, +# setup_admin_user_and_claims): +# """Assert that a team member engagement membership can be created.""" +# user, claims = setup_admin_user_and_claims +# engagement = factory_engagement_model() +# staff_user = factory_staff_user_model() +# headers = factory_auth_header(jwt=jwt, claims=claims) + +# mock_add_user_to_group_keycloak_response = MagicMock() +# mock_add_user_to_group_keycloak_response.status_code = HTTPStatus.NO_CONTENT +# mock_add_user_to_group_keycloak = mocker.patch( +# 'met_api.services.keycloak.KeycloakService.add_user_to_group', +# return_value=mock_add_user_to_group_keycloak_response +# ) +# mock_get_users_groups_keycloak = mocker.patch( +# 'met_api.services.keycloak.KeycloakService.get_users_groups', +# return_value={staff_user.external_id: [KeycloakGroupName.TEAM_MEMBER.value]} +# ) + +# data = {'user_id': staff_user.external_id} + +# rv = client.post( +# memberships_url.format(engagement.id), +# data=json.dumps(data), +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == HTTPStatus.OK +# assert rv.json.get('engagement_id') == engagement.id +# assert rv.json.get('user_id') == staff_user.id +# assert rv.json.get('type') == MembershipType.TEAM_MEMBER +# assert rv.json.get('status') == MembershipStatus.ACTIVE.value +# mock_add_user_to_group_keycloak.assert_called() +# mock_get_users_groups_keycloak.assert_called() + +# with patch.object(MembershipService, 'create_membership', +# side_effect=BusinessException('Test error', status_code=HTTPStatus.INTERNAL_SERVER_ERROR)): +# rv = client.post( +# memberships_url.format(engagement.id), +# data=json.dumps(data), +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + +# TODO: Replace this test with one that adds composite roles to user +# def test_create_engagement_membership_reviewer(mocker, client, jwt, session, +# setup_admin_user_and_claims): +# """Assert that a reviewer engagement membership can be created.""" +# user, claims = setup_admin_user_and_claims +# engagement = factory_engagement_model() +# staff_user = factory_staff_user_model() +# headers = factory_auth_header(jwt=jwt, claims=claims) + +# mock_add_user_to_group_keycloak_response = MagicMock() +# mock_add_user_to_group_keycloak_response.status_code = HTTPStatus.NO_CONTENT +# mock_add_user_to_group_keycloak = mocker.patch( +# 'met_api.services.keycloak.KeycloakService.add_user_to_group', +# return_value=mock_add_user_to_group_keycloak_response +# ) +# mock_get_users_groups_keycloak = mocker.patch( +# 'met_api.services.keycloak.KeycloakService.get_users_groups', +# return_value={staff_user.external_id: [KeycloakGroupName.REVIEWER.value]} +# ) + +# data = {'user_id': staff_user.external_id} + +# rv = client.post( +# memberships_url.format(engagement.id), +# data=json.dumps(data), +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == HTTPStatus.OK +# assert rv.json.get('engagement_id') == engagement.id +# assert rv.json.get('user_id') == staff_user.id +# assert rv.json.get('type') == MembershipType.REVIEWER +# assert rv.json.get('status') == MembershipStatus.ACTIVE.value +# mock_add_user_to_group_keycloak.assert_called() +# mock_get_users_groups_keycloak.assert_called() + +# TODO: Replace this test with one that adds composite roles to user +# def test_create_engagement_membership_unauthorized(client, jwt, session, +# setup_unprivileged_user_and_claims): +# """Assert that creating an engagement membership without proper authorization fails.""" +# user, claims = setup_unprivileged_user_and_claims +# engagement = factory_engagement_model() +# staff_user = factory_staff_user_model() +# headers = factory_auth_header(jwt=jwt, claims=claims) +# data = {'user_id': staff_user.external_id} + +# rv = client.post( +# memberships_url.format(engagement.id), +# data=json.dumps(data), +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == HTTPStatus.FORBIDDEN def test_revoke_membership(client, jwt, session, @@ -265,43 +263,43 @@ def test_get_membership(client, jwt, session, assert rv.status_code == HTTPStatus.INTERNAL_SERVER_ERROR -@pytest.mark.parametrize('side_effect, expected_status', [ - (ValueError('Test error'), HTTPStatus.BAD_REQUEST), -]) -def test_get_all_engagements_by_user(mocker, client, jwt, session, side_effect, expected_status, - setup_admin_user_and_claims): - """Test that all engagements can be fetched for a member.""" - user, claims = setup_admin_user_and_claims - engagement = factory_engagement_model() - staff_user = factory_staff_user_model() - headers = factory_auth_header(jwt=jwt, claims=claims) - - mock_add_user_to_group_keycloak_response = MagicMock() - mock_add_user_to_group_keycloak_response.status_code = HTTPStatus.NO_CONTENT - mocker.patch( - 'met_api.services.keycloak.KeycloakService.add_user_to_group', - return_value=mock_add_user_to_group_keycloak_response - ) - mocker.patch( - 'met_api.services.keycloak.KeycloakService.get_users_groups', - return_value={staff_user.external_id: [KeycloakGroupName.EAO_TEAM_MEMBER.value]} - ) - - data = {'user_id': staff_user.external_id} - - rv = client.post( - memberships_url.format(engagement.id), - data=json.dumps(data), - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == HTTPStatus.OK - - rv = client.get( - f'/api/engagements/all/members/{staff_user.external_id}', - headers=headers, - content_type=ContentType.JSON.value - ) - - assert rv.status_code == HTTPStatus.OK - assert rv.json[0].get('engagement_id') == engagement.id +# @pytest.mark.parametrize('side_effect, expected_status', [ +# (ValueError('Test error'), HTTPStatus.BAD_REQUEST), +# ]) +# def test_get_all_engagements_by_user(mocker, client, jwt, session, side_effect, expected_status, +# setup_admin_user_and_claims): +# """Test that all engagements can be fetched for a member.""" +# user, claims = setup_admin_user_and_claims +# engagement = factory_engagement_model() +# staff_user = factory_staff_user_model() +# headers = factory_auth_header(jwt=jwt, claims=claims) + +# mock_add_user_to_group_keycloak_response = MagicMock() +# mock_add_user_to_group_keycloak_response.status_code = HTTPStatus.NO_CONTENT +# mocker.patch( +# 'met_api.services.keycloak.KeycloakService.add_user_to_group', +# return_value=mock_add_user_to_group_keycloak_response +# ) +# mocker.patch( +# 'met_api.services.keycloak.KeycloakService.get_users_groups', +# return_value={staff_user.external_id: [KeycloakGroupName.TEAM_MEMBER.value]} +# ) + +# data = {'user_id': staff_user.external_id} + +# rv = client.post( +# memberships_url.format(engagement.id), +# data=json.dumps(data), +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == HTTPStatus.OK + +# rv = client.get( +# f'/api/engagements/all/members/{staff_user.external_id}', +# headers=headers, +# content_type=ContentType.JSON.value +# ) + +# assert rv.status_code == HTTPStatus.OK +# assert rv.json[0].get('engagement_id') == engagement.id diff --git a/met-api/tests/unit/api/test_user.py b/met-api/tests/unit/api/test_user.py index d95e29556..4e35be2d8 100644 --- a/met-api/tests/unit/api/test_user.py +++ b/met-api/tests/unit/api/test_user.py @@ -17,70 +17,78 @@ Test-Suite to ensure that the /user endpoint is working as expected. """ -import copy from http import HTTPStatus from unittest.mock import MagicMock, patch + import pytest from flask import current_app -from met_api.exceptions.business_exception import BusinessException from met_api.models import Tenant as TenantModel from met_api.services.staff_user_membership_service import StaffUserMembershipService from met_api.services.staff_user_service import StaffUserService -from met_api.utils.enums import ContentType, KeycloakGroupName, UserStatus +from met_api.utils.enums import ContentType, UserStatus from tests.utilities.factory_scenarios import TestJwtClaims, TestUserInfo from tests.utilities.factory_utils import factory_auth_header, factory_staff_user_model, set_global_tenant -KEYCLOAK_SERVICE_MODULE = 'met_api.services.keycloak.KeycloakService' - - -def mock_add_user_to_group(mocker, mock_group_names): - """Mock the KeycloakService.add_user_to_group method.""" - mock_response = MagicMock() - mock_response.status_code = HTTPStatus.NO_CONTENT - - mock_add_user_to_group_keycloak = mocker.patch( - f'{KEYCLOAK_SERVICE_MODULE}.add_user_to_group', - return_value=mock_response - ) - mock_get_user_groups_keycloak = mocker.patch( - f'{KEYCLOAK_SERVICE_MODULE}.get_user_groups', - return_value=[{'name': group_name} for group_name in mock_group_names] - ) - mock_get_user_groups_keycloak = mocker.patch( - f'{KEYCLOAK_SERVICE_MODULE}.get_user_groups', - return_value=[{'name': group_name} for group_name in mock_group_names] - ) - - mock_add_attribute_to_user = mocker.patch( - f'{KEYCLOAK_SERVICE_MODULE}.add_attribute_to_user', - return_value=mock_response - ) - return mock_add_user_to_group_keycloak, mock_get_user_groups_keycloak, mock_add_attribute_to_user +KEYCLOAK_SERVICE_MODULE = 'met_api.services.keycloak.KeycloakService' -@pytest.mark.parametrize('side_effect, expected_status', [ - (KeyError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), - (ValueError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), -]) +# TODO: Replace this test with one that adds composite roles to user +# def mock_add_user_to_group(mocker, mock_group_names): +# """Mock the KeycloakService.add_user_to_group method.""" +# mock_response = MagicMock() +# mock_response.status_code = HTTPStatus.NO_CONTENT + +# mock_add_user_to_group_keycloak = mocker.patch( +# f'{KEYCLOAK_SERVICE_MODULE}.add_user_to_group', +# return_value=mock_response +# ) +# mock_get_user_groups_keycloak = mocker.patch( +# f'{KEYCLOAK_SERVICE_MODULE}.get_user_groups', +# return_value=[{'name': group_name} for group_name in mock_group_names] +# ) +# mock_get_user_groups_keycloak = mocker.patch( +# f'{KEYCLOAK_SERVICE_MODULE}.get_user_groups', +# return_value=[{'name': group_name} for group_name in mock_group_names] +# ) + +# mock_add_attribute_to_user = mocker.patch( +# f'{KEYCLOAK_SERVICE_MODULE}.add_attribute_to_user', +# return_value=mock_response +# ) +# return mock_add_user_to_group_keycloak, mock_get_user_groups_keycloak, mock_add_attribute_to_user + + +@pytest.mark.parametrize( + 'side_effect, expected_status', + [ + (KeyError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), + (ValueError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), + ], +) def test_create_staff_user(client, jwt, session, side_effect, expected_status): """Assert that a user can be POSTed.""" claims = TestJwtClaims.staff_admin_role headers = factory_auth_header(jwt=jwt, claims=claims) - rv = client.put('/api/user/', headers=headers, content_type=ContentType.JSON.value) + rv = client.put( + '/api/user/', headers=headers, content_type=ContentType.JSON.value + ) assert rv.status_code == HTTPStatus.OK assert rv.json.get('email_address') == claims.get('email') tenant_short_name = current_app.config.get('DEFAULT_TENANT_SHORT_NAME') tenant = TenantModel.find_by_short_name(tenant_short_name) assert rv.json.get('tenant_id') == str(tenant.id) - with patch.object(StaffUserService, 'create_or_update_user', side_effect=side_effect): - rv = client.put('/api/user/', headers=headers, content_type=ContentType.JSON.value) + with patch.object( + StaffUserService, 'create_or_update_user', side_effect=side_effect + ): + rv = client.put( + '/api/user/', headers=headers, content_type=ContentType.JSON.value + ) assert rv.status_code == expected_status -def test_get_staff_users(client, jwt, session, - setup_admin_user_and_claims): +def test_get_staff_users(client, jwt, session, setup_admin_user_and_claims): """Assert that a user can be POSTed.""" set_global_tenant() staff_1 = dict(TestUserInfo.user_staff_1) @@ -90,136 +98,142 @@ def test_get_staff_users(client, jwt, session, user, claims = setup_admin_user_and_claims headers = factory_auth_header(jwt=jwt, claims=claims) - rv = client.get('/api/user/', headers=headers, content_type=ContentType.JSON.value) + rv = client.get( + '/api/user/', headers=headers, content_type=ContentType.JSON.value + ) assert rv.status_code == HTTPStatus.OK assert rv.json.get('total') == 4 assert len(rv.json.get('items')) == 4 -@pytest.mark.parametrize('side_effect, expected_status', [ - (KeyError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), - (ValueError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), -]) -def test_add_user_to_admin_group(mocker, client, jwt, session, side_effect, expected_status, - setup_admin_user_and_claims): - """Assert that a user can be added to the admin group.""" - user = factory_staff_user_model() - - mock_add_user_to_group_keycloak, mock_get_user_groups_keycloak, mock_add_attribute_to_user = mock_add_user_to_group( - mocker, - [KeycloakGroupName.EAO_IT_VIEWER.value] - ) - - user, claims = setup_admin_user_and_claims - headers = factory_auth_header(jwt=jwt, claims=claims) - rv = client.post( - f'/api/user/{user.external_id}/groups?group=Administrator', - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == HTTPStatus.OK - mock_add_user_to_group_keycloak.assert_called() - mock_get_user_groups_keycloak.assert_called() - mock_add_attribute_to_user.assert_called() - - with patch.object(StaffUserService, 'add_user_to_group', side_effect=side_effect): - rv = client.post( - f'/api/user/{user.external_id}/groups?group=Administrator', - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == expected_status - - with patch.object(StaffUserService, 'add_user_to_group', - side_effect=BusinessException('Test error', status_code=HTTPStatus.INTERNAL_SERVER_ERROR)): - rv = client.post( - f'/api/user/{user.external_id}/groups?group=Administrator', - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == HTTPStatus.INTERNAL_SERVER_ERROR - - -def test_add_user_to_reviewer_group(mocker, client, jwt, session, - setup_admin_user_and_claims): - """Assert that a user can be added to the reviewer group.""" - user = factory_staff_user_model() - - mock_add_user_to_group_keycloak, mock_get_user_groups_keycloak, mock_add_attribute_to_user = mock_add_user_to_group( - mocker, - [KeycloakGroupName.EAO_IT_VIEWER.value] - ) - - user, claims = setup_admin_user_and_claims - headers = factory_auth_header(jwt=jwt, claims=claims) - rv = client.post( - f'/api/user/{user.external_id}/groups?group=Reviewer', - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == HTTPStatus.OK - mock_add_user_to_group_keycloak.assert_called() - mock_get_user_groups_keycloak.assert_called() - - -def test_add_user_to_team_member_group(mocker, client, jwt, session, - setup_admin_user_and_claims): - """Assert that a user can be added to the team member group.""" - user = factory_staff_user_model() - - mock_add_user_to_group_keycloak, mock_get_user_groups_keycloak, mock_add_attribute_to_user = mock_add_user_to_group( - mocker, - [KeycloakGroupName.EAO_IT_VIEWER.value] - ) - - user, claims = setup_admin_user_and_claims - headers = factory_auth_header(jwt=jwt, claims=claims) - rv = client.post( - f'/api/user/{user.external_id}/groups?group=TeamMember', - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == HTTPStatus.OK - mock_add_user_to_group_keycloak.assert_called() - mock_get_user_groups_keycloak.assert_called() - - -def test_add_user_to_team_member_group_across_tenants(mocker, client, jwt, session): - """Assert that a user can be added to the team member group.""" - set_global_tenant(tenant_id=1) - user = factory_staff_user_model() - - mock_add_user_to_group_keycloak, mock_get_user_groups_keycloak, mock_add_attribute_to_user = mock_add_user_to_group( - mocker, - [KeycloakGroupName.EAO_IT_VIEWER.value] - ) - - claims = copy.deepcopy(TestJwtClaims.staff_admin_role.value) - # sets a different tenant id in the request - claims['tenant_id'] = 2 - headers = factory_auth_header(jwt=jwt, claims=claims) - rv = client.post( - f'/api/user/{user.external_id}/groups?group=TeamMember', - headers=headers, - content_type=ContentType.JSON.value - ) - # assert staff admin cant do cross tenant operation - assert rv.status_code == HTTPStatus.FORBIDDEN - - claims = copy.deepcopy(TestJwtClaims.met_admin_role.value) - # sets a different tenant id in the request - claims['tenant_id'] = 2 - headers = factory_auth_header(jwt=jwt, claims=claims) - rv = client.post( - f'/api/user/{user.external_id}/groups?group=TeamMember', - headers=headers, - content_type=ContentType.JSON.value - ) - # assert MET admin can do cross tenant operation - assert rv.status_code == HTTPStatus.OK - - mock_add_user_to_group_keycloak.assert_called() - mock_get_user_groups_keycloak.assert_called() +# TODO: Replace/modify the next series of tests so they support composite roles instead of groups +# @pytest.mark.parametrize('side_effect, expected_status', [ +# (KeyError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), +# (ValueError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), +# ]) +# def test_add_user_to_admin_group(mocker, client, jwt, session, side_effect, expected_status, +# setup_admin_user_and_claims): +# """Assert that a user can be added to the admin group.""" +# user = factory_staff_user_model() + +# mock_add_user_to_group_keycloak, mock_get_user_groups_keycloak, mock_add_attribute_to_user = mock_add_user_to_group( # noqa: E501 +# mocker, +# [KeycloakGroupName.EAO_IT_VIEWER.value] +# ) + +# user, claims = setup_admin_user_and_claims +# headers = factory_auth_header(jwt=jwt, claims=claims) +# rv = client.post( +# f'/api/user/{user.external_id}/groups?group=Administrator', +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == HTTPStatus.OK +# mock_add_user_to_group_keycloak.assert_called() +# mock_get_user_groups_keycloak.assert_called() +# mock_add_attribute_to_user.assert_called() + +# with patch.object(StaffUserService, 'add_user_to_group', side_effect=side_effect): +# rv = client.post( +# f'/api/user/{user.external_id}/groups?group=Administrator', +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == expected_status + +# with patch.object(StaffUserService, 'add_user_to_group', +# side_effect=BusinessException('Test error', status_code=HTTPStatus.INTERNAL_SERVER_ERROR)): +# rv = client.post( +# f'/api/user/{user.external_id}/groups?group=Administrator', +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == HTTPStatus.INTERNAL_SERVER_ERROR + +# TODO: Replace/modify the next series of tests so they support composite roles instead of groups +# def test_add_user_to_reviewer_group(mocker, client, jwt, session, +# setup_admin_user_and_claims): +# """Assert that a user can be added to the reviewer group.""" +# user = factory_staff_user_model() + +# mock_add_user_to_group_keycloak, mock_get_user_groups_keycloak, mock_add_attribute_to_user = mock_add_user_to_group( # noqa: E501 +# mocker, +# [KeycloakGroupName.EAO_IT_VIEWER.value] +# ) + +# user, claims = setup_admin_user_and_claims +# headers = factory_auth_header(jwt=jwt, claims=claims) +# rv = client.post( +# f'/api/user/{user.external_id}/groups?group=Reviewer', +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == HTTPStatus.OK +# mock_add_user_to_group_keycloak.assert_called() +# mock_get_user_groups_keycloak.assert_called() + +# TODO: Replace/modify the next series of tests so they support composite +# roles instead of groups +# def test_add_user_to_team_member_group(mocker, client, jwt, session, +# setup_admin_user_and_claims): +# """Assert that a user can be added to the team member group.""" +# user = factory_staff_user_model() + +# mock_add_user_to_group_keycloak, mock_get_user_groups_keycloak, +# mock_add_attribute_to_user = mock_add_user_to_group( +# mocker, +# [KeycloakGroupName.EAO_IT_VIEWER.value] +# ) + +# user, claims = setup_admin_user_and_claims +# headers = factory_auth_header(jwt=jwt, claims=claims) +# rv = client.post( +# f'/api/user/{user.external_id}/groups?group=TeamMember', +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == HTTPStatus.OK +# mock_add_user_to_group_keycloak.assert_called() +# mock_get_user_groups_keycloak.assert_called() + +# TODO: Replace/modify the next series of tests so they support composite +# roles instead of groups +# def test_add_user_to_team_member_group_across_tenants(mocker, client, jwt, session): +# """Assert that a user can be added to the team member group.""" +# set_global_tenant(tenant_id=1) +# user = factory_staff_user_model() + +# mock_add_user_to_group_keycloak, mock_get_user_groups_keycloak, mock_add_attribute_to_user = mock_add_user_to_group( # noqa: E501 +# mocker, +# [KeycloakGroupName.EAO_IT_VIEWER.value] +# ) + +# claims = copy.deepcopy(TestJwtClaims.staff_admin_role.value) +# # sets a different tenant id in the request +# claims['tenant_id'] = 2 +# headers = factory_auth_header(jwt=jwt, claims=claims) +# rv = client.post( +# f'/api/user/{user.external_id}/groups?group=TeamMember', +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# # assert staff admin cant do cross tenant operation +# assert rv.status_code == HTTPStatus.FORBIDDEN + +# claims = copy.deepcopy(TestJwtClaims.met_admin_role.value) +# # sets a different tenant id in the request +# claims['tenant_id'] = 2 +# headers = factory_auth_header(jwt=jwt, claims=claims) +# rv = client.post( +# f'/api/user/{user.external_id}/groups?group=TeamMember', +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# # assert MET admin can do cross tenant operation +# assert rv.status_code == HTTPStatus.OK + +# mock_add_user_to_group_keycloak.assert_called() +# mock_get_user_groups_keycloak.assert_called() def mock_toggle_user_status(mocker): @@ -229,14 +243,15 @@ def mock_toggle_user_status(mocker): mock_toggle_user_status = mocker.patch( f'{KEYCLOAK_SERVICE_MODULE}.toggle_user_enabled_status', - return_value=mock_response + return_value=mock_response, ) return mock_toggle_user_status -def test_toggle_user_active_status(mocker, client, jwt, session, - setup_admin_user_and_claims): +def test_toggle_user_active_status( + mocker, client, jwt, session, setup_admin_user_and_claims +): """Assert that a user can be toggled.""" user = factory_staff_user_model() mocked_toggle_user_status = mock_toggle_user_status(mocker) @@ -248,15 +263,16 @@ def test_toggle_user_active_status(mocker, client, jwt, session, f'/api/user/{user.external_id}/status', headers=headers, json={'active': False}, - content_type=ContentType.JSON.value + content_type=ContentType.JSON.value, ) assert rv.status_code == HTTPStatus.OK assert rv.json.get('status_id') == UserStatus.INACTIVE.value mocked_toggle_user_status.assert_called() -def test_team_member_cannot_toggle_user_active_status(mocker, client, jwt, session, - setup_team_member_and_claims): +def test_team_member_cannot_toggle_user_active_status( + mocker, client, jwt, session, setup_team_member_and_claims +): """Assert that a team member cannot toggle user status.""" user = factory_staff_user_model() mocked_toggle_user_status = mock_toggle_user_status(mocker) @@ -268,14 +284,15 @@ def test_team_member_cannot_toggle_user_active_status(mocker, client, jwt, sessi f'/api/user/{user.external_id}/status', headers=headers, json={'active': False}, - content_type=ContentType.JSON.value + content_type=ContentType.JSON.value, ) assert rv.status_code == HTTPStatus.UNAUTHORIZED mocked_toggle_user_status.assert_not_called() -def test_reviewer_cannot_toggle_user_active_status(mocker, client, jwt, session, - setup_reviewer_and_claims): +def test_reviewer_cannot_toggle_user_active_status( + mocker, client, jwt, session, setup_reviewer_and_claims +): """Assert that a reviewer cannot toggle user status.""" user = factory_staff_user_model() mocked_toggle_user_status = mock_toggle_user_status(mocker) @@ -287,14 +304,15 @@ def test_reviewer_cannot_toggle_user_active_status(mocker, client, jwt, session, f'/api/user/{user.external_id}/status', headers=headers, json={'active': False}, - content_type=ContentType.JSON.value + content_type=ContentType.JSON.value, ) assert rv.status_code == HTTPStatus.UNAUTHORIZED mocked_toggle_user_status.assert_not_called() -def test_toggle_user_active_status_empty_body(mocker, client, jwt, session, - setup_admin_user_and_claims): +def test_toggle_user_active_status_empty_body( + mocker, client, jwt, session, setup_admin_user_and_claims +): """Assert that returns bad request if bad request body.""" user = factory_staff_user_model() mocked_toggle_user_status = mock_toggle_user_status(mocker) @@ -305,32 +323,48 @@ def test_toggle_user_active_status_empty_body(mocker, client, jwt, session, rv = client.patch( f'/api/user/{user.external_id}/status', headers=headers, - content_type=ContentType.JSON.value + content_type=ContentType.JSON.value, ) assert rv.status_code == HTTPStatus.BAD_REQUEST mocked_toggle_user_status.assert_not_called() -def test_get_staff_users_by_id(client, jwt, session, - setup_admin_user_and_claims): +def test_get_staff_users_by_id( + client, jwt, session, setup_admin_user_and_claims +): """Assert that a user can be fetched.""" user, claims = setup_admin_user_and_claims headers = factory_auth_header(jwt=jwt, claims=claims) - rv = client.put('/api/user/', headers=headers, content_type=ContentType.JSON.value) + rv = client.put( + '/api/user/', headers=headers, content_type=ContentType.JSON.value + ) assert rv.status_code == HTTPStatus.OK user_id = rv.json.get('id') - rv = client.get(f'/api/user/{user_id}', headers=headers, content_type=ContentType.JSON.value) + rv = client.get( + f'/api/user/{user_id}', + headers=headers, + content_type=ContentType.JSON.value, + ) assert rv.status_code == HTTPStatus.OK assert rv.json.get('id') == user_id -@pytest.mark.parametrize('side_effect, expected_status', [ - (KeyError('Test error'), HTTPStatus.BAD_REQUEST), - (ValueError('Test error'), HTTPStatus.BAD_REQUEST), -]) -def test_errors_on_toggle_user_active_status(client, jwt, session, side_effect, expected_status, - setup_admin_user_and_claims): +@pytest.mark.parametrize( + 'side_effect, expected_status', + [ + (KeyError('Test error'), HTTPStatus.BAD_REQUEST), + (ValueError('Test error'), HTTPStatus.BAD_REQUEST), + ], +) +def test_errors_on_toggle_user_active_status( + client, + jwt, + session, + side_effect, + expected_status, + setup_admin_user_and_claims, +): """Assert that a user can be toggled.""" user = factory_staff_user_model() @@ -338,11 +372,15 @@ def test_errors_on_toggle_user_active_status(client, jwt, session, side_effect, user, claims = setup_admin_user_and_claims headers = factory_auth_header(jwt=jwt, claims=claims) - with patch.object(StaffUserMembershipService, 'reactivate_deactivate_user', side_effect=side_effect): + with patch.object( + StaffUserMembershipService, + 'reactivate_deactivate_user', + side_effect=side_effect, + ): rv = client.patch( f'/api/user/{user.external_id}/status', headers=headers, json={'active': False}, - content_type=ContentType.JSON.value + content_type=ContentType.JSON.value, ) assert rv.status_code == expected_status diff --git a/met-api/tests/unit/api/test_user_membership.py b/met-api/tests/unit/api/test_user_membership.py index f91fb16c8..808b1428f 100644 --- a/met-api/tests/unit/api/test_user_membership.py +++ b/met-api/tests/unit/api/test_user_membership.py @@ -14,19 +14,19 @@ """Tests to verify the user membership operations. -Test-Suite to ensure that the user membership endpoints are working as expected. +Test-Suite to ensure that the user membership endpoints are working as expected. # noqa: E501 """ from http import HTTPStatus -from unittest.mock import MagicMock, patch -import pytest +from unittest.mock import MagicMock +# import pytest -from met_api.exceptions.business_exception import BusinessException -from met_api.models.membership import Membership as MembershipModel -from met_api.services.staff_user_membership_service import StaffUserMembershipService -from met_api.utils.enums import ContentType, KeycloakGroupName, MembershipStatus, UserStatus -from tests.utilities.factory_scenarios import TestJwtClaims -from tests.utilities.factory_utils import ( - factory_auth_header, factory_engagement_model, factory_membership_model, factory_staff_user_model) +# from met_api.exceptions.business_exception import BusinessException +# from met_api.models.membership import Membership as MembershipModel +# from met_api.services.staff_user_membership_service import StaffUserMembershipService # noqa: E501 +# from met_api.utils.enums import ContentType, KeycloakGroupName, MembershipStatus, UserStatus # noqa: E501 +# from tests.utilities.factory_scenarios import TestJwtClaims +# from tests.utilities.factory_utils import ( +# factory_auth_header, factory_engagement_model, factory_membership_model, factory_staff_user_model) # noqa: E501 KEYCLOAK_SERVICE_MODULE = 'met_api.services.keycloak.KeycloakService' @@ -42,10 +42,11 @@ def mock_keycloak_methods(mocker, mock_group_names): return_value=mock_response ) - mock_get_user_groups_keycloak = mocker.patch( - f'{KEYCLOAK_SERVICE_MODULE}.get_user_groups', - return_value=[{'name': group_name} for group_name in mock_group_names] - ) + # TODO: Restore this patch but for composite roles and not groups + # mock_get_user_groups_keycloak = mocker.patch( + # f'{KEYCLOAK_SERVICE_MODULE}.get_user_groups', + # return_value=[{'name': group_name} for group_name in mock_group_names] + # ) mock_add_attribute_to_user = mocker.patch( f'{KEYCLOAK_SERVICE_MODULE}.add_attribute_to_user', @@ -59,74 +60,74 @@ def mock_keycloak_methods(mocker, mock_group_names): return ( mock_add_user_to_group_keycloak, - mock_get_user_groups_keycloak, + # mock_get_user_groups_keycloak, mock_add_attribute_to_user, mock_remove_user_from_group_keycloak ) - -@pytest.mark.parametrize('side_effect, expected_status', [ - (KeyError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), - (ValueError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), -]) -def test_reassign_user_reviewer_team_member(mocker, client, jwt, session, side_effect, expected_status): - """Assert that returns bad request if bad request body.""" - user = factory_staff_user_model() - eng = factory_engagement_model() - current_membership = factory_membership_model(user_id=user.id, engagement_id=eng.id) - assert current_membership.status == MembershipStatus.ACTIVE.value - mock_response = MagicMock() - mock_response.status_code = HTTPStatus.NO_CONTENT - - ( - mock_add_user_to_group_keycloak, - mock_get_user_groups_keycloak, - mock_add_attribute_to_user, - mock_remove_user_from_group_keycloak - ) = mock_keycloak_methods( - mocker, - [KeycloakGroupName.EAO_REVIEWER.value] - ) - - mock_get_users_groups_keycloak = mocker.patch( - f'{KEYCLOAK_SERVICE_MODULE}.get_users_groups', - return_value={user.external_id: [KeycloakGroupName.EAO_REVIEWER.value]} - ) - - assert user.status_id == UserStatus.ACTIVE.value - claims = TestJwtClaims.staff_admin_role - headers = factory_auth_header(jwt=jwt, claims=claims) - - rv = client.put( - f'/api/user/{user.id}/groups?group=EAO_TEAM_MEMBER', - headers=headers, - content_type=ContentType.JSON.value - ) - - assert rv.status_code == HTTPStatus.OK - mock_add_user_to_group_keycloak.assert_called() - mock_get_user_groups_keycloak.assert_called() - mock_add_attribute_to_user.assert_called() - mock_remove_user_from_group_keycloak.assert_called() - mock_get_users_groups_keycloak.assert_called() - - memberships = MembershipModel.find_by_user_id(user.id) - assert len(memberships) == 1 - assert memberships[0].status == MembershipStatus.REVOKED.value - - with patch.object(StaffUserMembershipService, 'reassign_user', side_effect=side_effect): - rv = client.put( - f'/api/user/{user.id}/groups?group=EAO_TEAM_MEMBER', - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == expected_status - - with patch.object(StaffUserMembershipService, 'reassign_user', - side_effect=BusinessException('Test error', status_code=HTTPStatus.INTERNAL_SERVER_ERROR)): - rv = client.put( - f'/api/user/{user.id}/groups?group=EAO_TEAM_MEMBER', - headers=headers, - content_type=ContentType.JSON.value - ) - assert rv.status_code == HTTPStatus.INTERNAL_SERVER_ERROR +# TODO: Restore this test to support composite roles instead of groups +# @pytest.mark.parametrize('side_effect, expected_status', [ +# (KeyError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), +# (ValueError('Test error'), HTTPStatus.INTERNAL_SERVER_ERROR), +# ]) +# def test_reassign_user_reviewer_team_member(mocker, client, jwt, session, side_effect, expected_status): # noqa: E501 +# """Assert that returns bad request if bad request body.""" +# user = factory_staff_user_model() +# eng = factory_engagement_model() +# current_membership = factory_membership_model(user_id=user.id, engagement_id=eng.id) # noqa: E501 +# assert current_membership.status == MembershipStatus.ACTIVE.value +# mock_response = MagicMock() +# mock_response.status_code = HTTPStatus.NO_CONTENT + +# ( +# mock_add_user_to_group_keycloak, +# mock_get_user_groups_keycloak, +# mock_add_attribute_to_user, +# mock_remove_user_from_group_keycloak +# ) = mock_keycloak_methods( +# mocker, +# [KeycloakGroupName.EAO_REVIEWER.value] +# ) + +# mock_get_users_groups_keycloak = mocker.patch( +# f'{KEYCLOAK_SERVICE_MODULE}.get_users_groups', +# return_value={user.external_id: [KeycloakGroupName.EAO_REVIEWER.value]} # noqa: E501 +# ) + +# assert user.status_id == UserStatus.ACTIVE.value +# claims = TestJwtClaims.staff_admin_role +# headers = factory_auth_header(jwt=jwt, claims=claims) + +# rv = client.put( +# f'/api/user/{user.id}/groups?group=EAO_TEAM_MEMBER', +# headers=headers, +# content_type=ContentType.JSON.value +# ) + +# assert rv.status_code == HTTPStatus.OK +# mock_add_user_to_group_keycloak.assert_called() +# mock_get_user_groups_keycloak.assert_called() +# mock_add_attribute_to_user.assert_called() +# mock_remove_user_from_group_keycloak.assert_called() +# mock_get_users_groups_keycloak.assert_called() + +# memberships = MembershipModel.find_by_user_id(user.id) +# assert len(memberships) == 1 +# assert memberships[0].status == MembershipStatus.REVOKED.value + +# with patch.object(StaffUserMembershipService, 'reassign_user', side_effect=side_effect): +# rv = client.put( +# f'/api/user/{user.id}/groups?group=EAO_TEAM_MEMBER', +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == expected_status + +# with patch.object(StaffUserMembershipService, 'reassign_user', +# side_effect=BusinessException('Test error', status_code=HTTPStatus.INTERNAL_SERVER_ERROR)): +# rv = client.put( +# f'/api/user/{user.id}/groups?group=EAO_TEAM_MEMBER', +# headers=headers, +# content_type=ContentType.JSON.value +# ) +# assert rv.status_code == HTTPStatus.INTERNAL_SERVER_ERROR diff --git a/met-api/tests/unit/services/test_keycloak.py b/met-api/tests/unit/services/test_keycloak.py index 1266de9db..87cdfb842 100644 --- a/met-api/tests/unit/services/test_keycloak.py +++ b/met-api/tests/unit/services/test_keycloak.py @@ -36,18 +36,18 @@ def test_keycloak_get_user_by_username(session): user = KEYCLOAK_SERVICE.get_user_by_username(request.get('username')) assert user.get('username') == request.get('username') - -def test_keycloak_get_user_groups(session): - """Get user by username. Assert get a user with the same username as the username in request.""" - request = KeycloakScenario.create_user_request() - group_name = 'admins' - KEYCLOAK_SERVICE.add_user(request) - user = KEYCLOAK_SERVICE.get_user_by_username(request.get('username')) - user_id = user.get('id') - user_group = KEYCLOAK_SERVICE.get_users_groups([user_id]) - - assert group_name not in user_group.get(user_id) - # add the group - KEYCLOAK_SERVICE.add_user_to_group(user_id, '%s' % group_name) - user_group = KEYCLOAK_SERVICE.get_users_groups([user_id]) - assert group_name in user_group.get(user_id) +# TODO: Replace this test with one that gets user composite roles +# def test_keycloak_get_user_groups(session): +# """Get user by username. Assert get a user with the same username as the username in request.""" +# request = KeycloakScenario.create_user_request() +# group_name = 'admins' +# KEYCLOAK_SERVICE.add_user(request) +# user = KEYCLOAK_SERVICE.get_user_by_username(request.get('username')) +# user_id = user.get('id') +# user_group = KEYCLOAK_SERVICE.get_users_groups([user_id]) + +# assert group_name not in user_group.get(user_id) +# # add the group +# KEYCLOAK_SERVICE.add_user_to_group(user_id, '%s' % group_name) +# user_group = KEYCLOAK_SERVICE.get_users_groups([user_id]) +# assert group_name in user_group.get(user_id) diff --git a/met-web/src/components/engagement/form/EngagementFormTabs/UserManagement/AddTeamMemberModal.tsx b/met-web/src/components/engagement/form/EngagementFormTabs/UserManagement/AddTeamMemberModal.tsx index 4d5d1d552..ea27d66ad 100644 --- a/met-web/src/components/engagement/form/EngagementFormTabs/UserManagement/AddTeamMemberModal.tsx +++ b/met-web/src/components/engagement/form/EngagementFormTabs/UserManagement/AddTeamMemberModal.tsx @@ -63,7 +63,7 @@ export const AddTeamMemberModal = () => { setUsersLoading(true); const response = await getUserList({ search_text: searchText, - include_groups: false, + include_roles: false, }); setUsers(response.items); setUsersLoading(false); diff --git a/met-web/src/components/userManagement/listing/UserManagementContext.tsx b/met-web/src/components/userManagement/listing/UserManagementContext.tsx index eabef5a37..60b1c3dd8 100644 --- a/met-web/src/components/userManagement/listing/UserManagementContext.tsx +++ b/met-web/src/components/userManagement/listing/UserManagementContext.tsx @@ -101,7 +101,7 @@ export const UserManagementContextProvider = ({ children }: { children: JSX.Elem size, sort_key: nested_sort_key || sort_key, sort_order, - include_groups: true, + include_roles: true, search_text: searchText, include_inactive: true, }); diff --git a/met-web/src/components/userManagement/userDetails/UserDetailsContext.tsx b/met-web/src/components/userManagement/userDetails/UserDetailsContext.tsx index e97be33b3..72626dc03 100644 --- a/met-web/src/components/userManagement/userDetails/UserDetailsContext.tsx +++ b/met-web/src/components/userManagement/userDetails/UserDetailsContext.tsx @@ -76,7 +76,7 @@ export const UserDetailsContextProvider = ({ children }: { children: JSX.Element const getUserDetails = async () => { setUserLoading(true); - const fetchedUser = await getUser({ user_id: Number(userId), include_groups: true }); + const fetchedUser = await getUser({ user_id: Number(userId), include_roles: true }); setSavedUser(fetchedUser); setUserLoading(false); }; diff --git a/met-web/src/models/user.ts b/met-web/src/models/user.ts index 235e24dbb..53420ecde 100644 --- a/met-web/src/models/user.ts +++ b/met-web/src/models/user.ts @@ -1,20 +1,20 @@ -export type UserGroup = 'EAO_IT_ADMIN' | 'EAO_IT_VIEWER' | 'EAO_TEAM_MEMBER' | 'EAO_REVIEWER'; +export type UserGroup = 'IT_ADMIN' | 'IT_VIEWER' | 'TEAM_MEMBER' | 'REVIEWER'; export const USER_GROUP: { [x: string]: { value: UserGroup; label: string } } = { ADMIN: { - value: 'EAO_IT_ADMIN', + value: 'IT_ADMIN', label: 'Administrator', }, VIEWER: { - value: 'EAO_IT_VIEWER', + value: 'IT_VIEWER', label: 'Viewer', }, TEAM_MEMBER: { - value: 'EAO_TEAM_MEMBER', + value: 'TEAM_MEMBER', label: 'Team Member', }, REVIEWER: { - value: 'EAO_REVIEWER', + value: 'REVIEWER', label: 'Reviewer', }, }; diff --git a/met-web/src/routes/AuthGate.tsx b/met-web/src/routes/AuthGate.tsx index 46b35d0e8..c8be2ff32 100644 --- a/met-web/src/routes/AuthGate.tsx +++ b/met-web/src/routes/AuthGate.tsx @@ -5,7 +5,6 @@ import { USER_GROUP } from 'models/user'; const AuthGate = ({ allowedRoles }: { allowedRoles: string[] }) => { const permissions = useAppSelector((state) => state.user.roles); - const userGroups = useAppSelector((state) => state.user.userDetail.groups); const location = useLocation(); const scopesMap: { [scope: string]: boolean } = {}; @@ -14,7 +13,7 @@ const AuthGate = ({ allowedRoles }: { allowedRoles: string[] }) => { }); return permissions.some((permission) => scopesMap[permission]) || - userGroups?.includes('/' + USER_GROUP.TEAM_MEMBER.value) ? ( + permissions?.includes('/' + USER_GROUP.TEAM_MEMBER.value) ? ( ) : ( diff --git a/met-web/src/services/userService/api/index.tsx b/met-web/src/services/userService/api/index.tsx index 06fc5384d..8f1430352 100644 --- a/met-web/src/services/userService/api/index.tsx +++ b/met-web/src/services/userService/api/index.tsx @@ -12,7 +12,7 @@ interface GetUserListParams { sort_order?: 'asc' | 'desc'; search_text?: string; // If yes, user groups will be fetched as well from keycloak - include_groups?: boolean; + include_roles?: boolean; include_inactive?: boolean; } export const getUserList = async (params: GetUserListParams = {}): Promise> => { @@ -28,7 +28,7 @@ export const getUserList = async (params: GetUserListParams = {}): Promise => { const url = replaceUrl(Endpoints.User.GET, 'user_id', String(params.user_id));