feat: TT-156 add groups field in User model (#262)

Angeluz-07 · web-flow · commit 4c6f8ba3d910 · 2021-03-01T10:53:23.000-05:00
* feat: TT-156 add group fields in model

* feat: TT-156 cache groups_and_users response for perfomance

* test: TT-156 update tests

* test: TT-156 add get_grups_and_users test

* fix: TT-156 remove debug statements

* test: TT-156 add test for get_groups_by_user_id

* test: TT-156 parametrize test for get_groups_by_user_id

* test: TT-156 get_groups_and_users_called_once_by_instance
diff --git 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
@@ -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
@@ -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
@@ -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']
