diff --git a/tests/time_tracker_api/users/users_namespace_test.py b/tests/time_tracker_api/users/users_namespace_test.py index 246de565..0e32ea41 100644 --- a/tests/time_tracker_api/users/users_namespace_test.py +++ b/tests/time_tracker_api/users/users_namespace_test.py @@ -10,14 +10,13 @@ @patch('utils.azure_users.AzureConnection.get_token', Mock()) @patch('utils.azure_users.AzureConnection.get_user') def test_get_user_response_contains_expected_props( - get_user_mock, - client: FlaskClient, - valid_header: dict, + get_user_mock, client: FlaskClient, valid_header: dict, ): get_user_mock.return_value = { 'name': 'dummy', 'email': 'dummy', 'roles': ['dummy-role'], + 'groups': ['dummy-group'], } user_id = (Faker().uuid4(),) response = client.get(f'/users/{user_id}', headers=valid_header) @@ -27,7 +26,9 @@ def test_get_user_response_contains_expected_props( assert 'name' in json.loads(response.data) assert 'email' in json.loads(response.data) assert 'roles' in json.loads(response.data) + assert 'groups' in json.loads(response.data) assert ['dummy-role'] == json.loads(response.data)['roles'] + assert ['dummy-group'] == json.loads(response.data)['groups'] @patch('utils.azure_users.AzureConnection.get_msal_client', Mock()) @@ -37,12 +38,15 @@ def test_get_user_response_contains_expected_props( ) @patch('utils.azure_users.AzureConnection.users') def test_users_response_contains_expected_props( - users_mock, - client: FlaskClient, - valid_header: dict, + users_mock, client: FlaskClient, valid_header: dict, ): users_mock.return_value = [ - {'name': 'dummy', 'email': 'dummy', 'roles': ['dummy-role']} + { + 'name': 'dummy', + 'email': 'dummy', + 'roles': ['dummy-role'], + 'groups': ['dummy-group'], + } ] response = client.get('/users', headers=valid_header) @@ -51,15 +55,16 @@ def test_users_response_contains_expected_props( assert 'name' in json.loads(response.data)[0] assert 'email' in json.loads(response.data)[0] assert 'roles' in json.loads(response.data)[0] + assert 'groups' in json.loads(response.data)[0] assert ['dummy-role'] == json.loads(response.data)[0]['roles'] + assert ['dummy-group'] == json.loads(response.data)[0]['groups'] @patch('utils.azure_users.AzureConnection.get_msal_client', Mock()) @patch('utils.azure_users.AzureConnection.get_token', Mock()) @patch('utils.azure_users.AzureConnection.update_role') @mark.parametrize( - 'role_id,action', - [('test', 'grant'), ('admin', 'revoke')], + 'role_id,action', [('test', 'grant'), ('admin', 'revoke')], ) def test_update_role_response_contains_expected_props( update_role_mock, @@ -73,15 +78,16 @@ def test_update_role_response_contains_expected_props( 'name': 'dummy', 'email': 'dummy', 'roles': [], + 'groups': [], } response = client.post( - f'/users/{user_id}/roles/{role_id}/{action}', - headers=valid_header, + f'/users/{user_id}/roles/{role_id}/{action}', headers=valid_header, ) assert HTTPStatus.OK == response.status_code assert 'name' in json.loads(response.data) assert 'email' in json.loads(response.data) assert 'roles' in json.loads(response.data) + assert 'groups' in json.loads(response.data) @patch('utils.azure_users.AzureConnection.get_msal_client', Mock()) @@ -107,8 +113,7 @@ def test_update_role_is_called_properly_on_each_action( ): update_role_mock.return_value = {} response = client.post( - f'/users/{user_id}/roles/{role_id}/{action}', - headers=valid_header, + f'/users/{user_id}/roles/{role_id}/{action}', headers=valid_header, ) assert HTTPStatus.OK == response.status_code diff --git a/tests/utils/azure_users_test.py b/tests/utils/azure_users_test.py index a1f69708..7c25f98d 100644 --- a/tests/utils/azure_users_test.py +++ b/tests/utils/azure_users_test.py @@ -14,10 +14,7 @@ ], ) def test_azure_connection_is_test_user( - get_mock, - field_name, - field_value, - is_test_user_expected_value, + get_mock, field_name, field_value, is_test_user_expected_value, ): response_mock = Mock() response_mock.status_code = 200 @@ -36,12 +33,7 @@ def test_azure_connection_get_test_user_ids(get_mock): response_mock = Mock() response_mock.status_code = 200 response_mock.json = Mock( - return_value={ - 'value': [ - {'objectId': 'ID1'}, - {'objectId': 'ID2'}, - ] - } + return_value={'value': [{'objectId': 'ID1'}, {'objectId': 'ID2'},]} ) get_mock.return_value = response_mock @@ -57,8 +49,8 @@ def test_azure_connection_get_test_user_ids(get_mock): def test_azure_connection_get_non_test_users( users_mock, get_test_user_ids_mock ): - test_user = AzureUser('ID1', None, None, []) - non_test_user = AzureUser('ID2', None, None, []) + test_user = AzureUser('ID1', None, None, [], []) + non_test_user = AzureUser('ID2', None, None, [], []) users_mock.return_value = [test_user, non_test_user] get_test_user_ids_mock.return_value = ['ID1'] non_test_users = [non_test_user] @@ -104,3 +96,83 @@ def test_is_user_in_group( azure_connection.is_user_in_group('user_id', payload_mock) == response_expected ) + + +@patch('utils.azure_users.AzureConnection.get_msal_client', Mock()) +@patch('utils.azure_users.AzureConnection.get_token', Mock()) +@patch('requests.get') +def test_get_groups_and_users(get_mock): + response_mock = Mock() + response_mock.status_code = 200 + return_value = { + 'value': [ + { + 'displayName': 'test-group-1', + 'members': [ + {'objectId': 'user-id1'}, + {'objectId': 'user-id2'}, + ], + }, + { + 'displayName': 'test-group-2', + 'members': [ + {'objectId': 'user-id3'}, + {'objectId': 'user-id1'}, + ], + }, + {'displayName': 'test-group-3', 'members': [],}, + ] + } + response_mock.json = Mock(return_value=return_value) + get_mock.return_value = response_mock + + expected_result = [ + ('test-group-1', ['user-id1', 'user-id2']), + ('test-group-2', ['user-id3', 'user-id1']), + ('test-group-3', []), + ] + + azure_connection = AzureConnection() + + assert azure_connection.get_groups_and_users() == expected_result + + +@patch('utils.azure_users.AzureConnection.get_msal_client', Mock()) +@patch('utils.azure_users.AzureConnection.get_token', Mock()) +@patch('utils.azure_users.AzureConnection.get_groups_and_users') +@mark.parametrize( + 'user_id,groups_expected_value', + [ + ('user-id1', ['test-group-1', 'test-group-2']), + ('user-id2', ['test-group-1']), + ('user-id3', ['test-group-2']), + ('user-id4', []), + ], +) +def test_get_groups_by_user_id( + get_groups_and_users_mock, user_id, groups_expected_value +): + get_groups_and_users_mock.return_value = [ + ('test-group-1', ['user-id1', 'user-id2']), + ('test-group-2', ['user-id3', 'user-id1']), + ] + + azure_connection = AzureConnection() + groups = azure_connection.get_groups_by_user_id(user_id) + assert groups == groups_expected_value + + +@patch('utils.azure_users.AzureConnection.get_msal_client', Mock()) +@patch('utils.azure_users.AzureConnection.get_token', Mock()) +@patch('utils.azure_users.AzureConnection.get_groups_and_users') +def test_get_groups_and_users_called_once_by_instance( + get_groups_and_users_mock, +): + get_groups_and_users_mock.return_value = [] + user_id = 'user-id1' + azure_connection = AzureConnection() + azure_connection.get_groups_by_user_id(user_id) + azure_connection.get_groups_by_user_id(user_id) + azure_connection.get_groups_by_user_id(user_id) + + get_groups_and_users_mock.assert_called_once() diff --git a/time_tracker_api/users/users_namespace.py b/time_tracker_api/users/users_namespace.py index 2c305837..d7a3da41 100644 --- a/time_tracker_api/users/users_namespace.py +++ b/time_tracker_api/users/users_namespace.py @@ -29,13 +29,23 @@ title='Roles', description='List of the roles assigned to the user by the tenant', ), + example=Faker().words( + 3, ['time-tracker-admin', 'test-user', 'guest',], unique=True + ), + ), + 'groups': fields.List( + fields.String( + title='Groups', + description='List of the groups the user belongs to, assigned by the tenant', + ), example=Faker().words( 3, [ 'time-tracker-admin', - 'test-user', - 'guest', + 'time-tracker-tester', + 'time-tracker-guest', ], + unique=True, ), ), }, diff --git a/utils/azure_users.py b/utils/azure_users.py index e6e88d26..3b0ed898 100644 --- a/utils/azure_users.py +++ b/utils/azure_users.py @@ -34,11 +34,12 @@ def __call__(self, r): class AzureUser: - def __init__(self, id, name, email, roles): + def __init__(self, id, name, email, roles, groups): self.id = id self.name = name self.email = email self.roles = roles + self.groups = groups HTTP_PATCH_HEADERS = { @@ -63,6 +64,7 @@ def __init__(self, config=MSConfig): self.config = config self.client = self.get_msal_client() self.access_token = self.get_token() + self.groups_and_users = None def get_msal_client(self): client = msal.ConfidentialClientApplication( @@ -115,7 +117,8 @@ def to_azure_user(self, item) -> AzureUser: for (field_name, field_value) in ROLE_FIELD_VALUES.values() if field_name in item ] - return AzureUser(id, name, email, roles) + groups = self.get_groups_by_user_id(id) + return AzureUser(id, name, email, roles, groups) def update_role(self, user_id, role_id, is_grant): endpoint = "{endpoint}/users/{user_id}?api-version=1.6".format( @@ -181,6 +184,28 @@ def get_group_id_by_group_name(self, group_name): return response.json()['value'][0]['objectId'] + def get_groups_by_user_id(self, user_id): + if self.groups_and_users is None: + self.groups_and_users = self.get_groups_and_users() + return [ + group_name + for (group_name, user_ids) in self.groups_and_users + if user_id in user_ids + ] + + def get_groups_and_users(self): + endpoint = "{endpoint}/groups?api-version=1.6&$select=displayName,members&$expand=members".format( + endpoint=self.config.ENDPOINT + ) + response = requests.get(endpoint, auth=BearerAuth(self.access_token)) + assert 200 == response.status_code + parse_item = lambda item: ( + item['displayName'], + [member['objectId'] for member in item['members']], + ) + result = list(map(parse_item, response.json()['value'])) + return result + def is_user_in_group(self, user_id, data: dict): group_id = self.get_group_id_by_group_name( group_name=data['group_name']