Skip to content

Commit a3439c3

Browse files
Angeluz-07Angeluz-07
authored
fix: TT-131 remove feature toggle for user role field (#255)
* fix: TT-131 remove feature toggle from users endpoint * fix: TT-131 remove old endpoints for grant admin role * fix: TT-131 replace users by users_v2 method * fix: TT-131 replace to_azure_user_v2 method * fix: TT-131 remove role field * fix: TT-131 extract MSAL client to method
1 parent 058265c commit a3439c3

File tree

4 files changed

+40
-242
lines changed

4 files changed

+40
-242
lines changed

‎tests/time_tracker_api/users/users_namespace_test.py‎

Lines changed: 9 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -2,99 +2,33 @@
22
from flask import json
33
from flask.testing import FlaskClient
44
from flask_restplus._http import HTTPStatus
5-
from utils.azure_users import AzureConnection
65
from pytest import mark
76

87

9-
@patch('msal.ConfidentialClientApplication', Mock())
8+
@patch('utils.azure_users.AzureConnection.get_msal_client', Mock())
109
@patch('utils.azure_users.AzureConnection.get_token', Mock())
1110
@patch(
1211
'utils.azure_users.AzureConnection.is_test_user', Mock(return_value=True)
1312
)
14-
@patch(
15-
'commons.feature_toggles.feature_toggle_manager.FeatureToggleManager.get_azure_app_configuration_client'
16-
)
17-
@patch(
18-
'commons.feature_toggles.feature_toggle_manager.FeatureToggleManager.is_toggle_enabled_for_user'
19-
)
2013
@patch('utils.azure_users.AzureConnection.users')
21-
@patch('utils.azure_users.AzureConnection.users_v2')
22-
def test_feature_toggle_is_on_then_role_field_is_list(
23-
users_v2_mock,
24-
users_mock,
25-
is_toggle_enabled_for_user_mock,
26-
get_azure_app_configuration_client_mock,
27-
client: FlaskClient,
28-
valid_header: dict,
14+
def test_users_response_contains_expected_props(
15+
users_mock, client: FlaskClient, valid_header: dict,
2916
):
30-
31-
is_toggle_enabled_for_user_mock.return_value = True
32-
users_v2_mock.return_value = [
17+
users_mock.return_value = [
3318
{'name': 'dummy', 'email': 'dummy', 'roles': ['dummy-role']}
3419
]
3520
response = client.get('/users', headers=valid_header)
3621

37-
users_v2_mock.assert_called()
38-
users_mock.assert_not_called()
22+
users_mock.assert_called()
3923
assert HTTPStatus.OK == response.status_code
4024
assert 'name' in json.loads(response.data)[0]
4125
assert 'email' in json.loads(response.data)[0]
4226
assert 'roles' in json.loads(response.data)[0]
4327
assert ['dummy-role'] == json.loads(response.data)[0]['roles']
4428

4529

46-
@patch(
47-
'commons.feature_toggles.feature_toggle_manager.FeatureToggleManager.get_azure_app_configuration_client'
48-
)
49-
@patch(
50-
'commons.feature_toggles.feature_toggle_manager.FeatureToggleManager.is_toggle_enabled_for_user'
51-
)
52-
@patch('utils.azure_users.AzureConnection.users')
53-
@patch('utils.azure_users.AzureConnection.users_v2')
54-
def test_feature_toggle_is_off_then_role_field_is_string(
55-
users_v2_mock,
56-
users_mock,
57-
is_toggle_enabled_for_user_mock,
58-
get_azure_app_configuration_client_mock,
59-
client: FlaskClient,
60-
valid_header: dict,
61-
):
62-
is_toggle_enabled_for_user_mock.return_value = False
63-
users_mock.return_value = [
64-
{'name': 'dummy', 'email': 'dummy', 'role': 'dummy-role'}
65-
]
66-
67-
response = client.get('/users', headers=valid_header)
68-
69-
users_mock.assert_called()
70-
users_v2_mock.assert_not_called()
71-
assert HTTPStatus.OK == response.status_code
72-
assert 'name' in json.loads(response.data)[0]
73-
assert 'email' in json.loads(response.data)[0]
74-
assert 'role' in json.loads(response.data)[0]
75-
assert 'dummy-role' == json.loads(response.data)[0]['role']
76-
77-
78-
def test_update_user_role_response_contains_expected_props(
79-
client: FlaskClient, valid_header: dict, user_id: str,
80-
):
81-
valid_user_role_data = {'role': 'admin'}
82-
AzureConnection.update_user_role = Mock(
83-
return_value={'name': 'dummy', 'email': 'dummy', 'role': 'dummy'}
84-
)
85-
86-
response = client.post(
87-
f'/users/{user_id}/roles',
88-
headers=valid_header,
89-
json=valid_user_role_data,
90-
)
91-
92-
assert HTTPStatus.OK == response.status_code
93-
assert 'name' in json.loads(response.data)
94-
assert 'email' in json.loads(response.data)
95-
assert 'role' in json.loads(response.data)
96-
97-
30+
@patch('utils.azure_users.AzureConnection.get_msal_client', Mock())
31+
@patch('utils.azure_users.AzureConnection.get_token', Mock())
9832
@patch('utils.azure_users.AzureConnection.update_role')
9933
@mark.parametrize(
10034
'role_id,action', [('test', 'grant'), ('admin', 'revoke')],
@@ -121,43 +55,8 @@ def test_update_role_response_contains_expected_props(
12155
assert 'roles' in json.loads(response.data)
12256

12357

124-
@patch('utils.azure_users.AzureConnection.update_user_role', new_callable=Mock)
125-
def test_on_post_update_user_role_is_being_called_with_valid_arguments(
126-
update_user_role_mock,
127-
client: FlaskClient,
128-
valid_header: dict,
129-
user_id: str,
130-
):
131-
update_user_role_mock.return_value = {}
132-
valid_user_role_data = {'role': 'admin'}
133-
response = client.post(
134-
f'/users/{user_id}/roles',
135-
headers=valid_header,
136-
json=valid_user_role_data,
137-
)
138-
139-
assert HTTPStatus.OK == response.status_code
140-
update_user_role_mock.assert_called_once_with(
141-
user_id, valid_user_role_data['role']
142-
)
143-
144-
145-
@patch('utils.azure_users.AzureConnection.update_user_role', new_callable=Mock)
146-
def test_on_delete_update_user_role_is_being_called_with_valid_arguments(
147-
update_user_role_mock,
148-
client: FlaskClient,
149-
valid_header: dict,
150-
user_id: str,
151-
):
152-
update_user_role_mock.return_value = {}
153-
response = client.delete(
154-
f'/users/{user_id}/roles/time-tracker-admin', headers=valid_header,
155-
)
156-
157-
assert HTTPStatus.OK == response.status_code
158-
update_user_role_mock.assert_called_once_with(user_id, role=None)
159-
160-
58+
@patch('utils.azure_users.AzureConnection.get_msal_client', Mock())
59+
@patch('utils.azure_users.AzureConnection.get_token', Mock())
16160
@patch('utils.azure_users.AzureConnection.update_role', new_callable=Mock)
16261
@mark.parametrize(
16362
'role_id,action,is_grant',

‎tests/utils/azure_users_test.py‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
from unittest.mock import Mock, patch
2-
from utils.azure_users import AzureConnection, ROLE_FIELD_VALUES, AzureUser_v2
2+
from utils.azure_users import AzureConnection, ROLE_FIELD_VALUES, AzureUser
33
from pytest import mark
44

55

6-
@patch('msal.ConfidentialClientApplication', Mock())
6+
@patch('utils.azure_users.AzureConnection.get_msal_client', Mock())
77
@patch('utils.azure_users.AzureConnection.get_token', Mock())
88
@patch('requests.get')
99
@mark.parametrize(
@@ -26,7 +26,7 @@ def test_azure_connection_is_test_user(
2626
assert az_conn.is_test_user(test_user_id) == is_test_user_expected_value
2727

2828

29-
@patch('msal.ConfidentialClientApplication', Mock())
29+
@patch('utils.azure_users.AzureConnection.get_msal_client', Mock())
3030
@patch('utils.azure_users.AzureConnection.get_token', Mock())
3131
@patch('requests.get')
3232
def test_azure_connection_get_test_user_ids(get_mock):
@@ -42,16 +42,16 @@ def test_azure_connection_get_test_user_ids(get_mock):
4242
assert az_conn.get_test_user_ids() == ids
4343

4444

45-
@patch('msal.ConfidentialClientApplication', Mock())
45+
@patch('utils.azure_users.AzureConnection.get_msal_client', Mock())
4646
@patch('utils.azure_users.AzureConnection.get_token', Mock())
4747
@patch('utils.azure_users.AzureConnection.get_test_user_ids')
48-
@patch('utils.azure_users.AzureConnection.users_v2')
48+
@patch('utils.azure_users.AzureConnection.users')
4949
def test_azure_connection_get_non_test_users(
50-
users_v2_mock, get_test_user_ids_mock
50+
users_mock, get_test_user_ids_mock
5151
):
52-
test_user = AzureUser_v2('ID1', None, None, [])
53-
non_test_user = AzureUser_v2('ID2', None, None, [])
54-
users_v2_mock.return_value = [test_user, non_test_user]
52+
test_user = AzureUser('ID1', None, None, [])
53+
non_test_user = AzureUser('ID2', None, None, [])
54+
users_mock.return_value = [test_user, non_test_user]
5555
get_test_user_ids_mock.return_value = ['ID1']
5656
non_test_users = [non_test_user]
5757
az_conn = AzureConnection()

‎time_tracker_api/users/users_namespace.py‎

Lines changed: 10 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
from faker import Faker
22
from flask_restplus import fields, Resource
3-
from flask_restplus._http import HTTPStatus
43

5-
from time_tracker_api.api import common_fields, api, NullableString
4+
from time_tracker_api.api import common_fields, api
65
from time_tracker_api.security import current_user_id
76

87
from utils.azure_users import AzureConnection
9-
from commons.feature_toggles.feature_toggle_manager import FeatureToggleManager
108

119
ns = api.namespace('users', description='Namespace of the API for users')
1210

@@ -27,12 +25,6 @@
2725
description='Email of the user that belongs to the tenant',
2826
example=Faker().email(),
2927
),
30-
'role': NullableString(
31-
title="User's Role",
32-
max_length=50,
33-
description='Role assigned to the user by the tenant',
34-
example=Faker().word(['time-tracker-admin']),
35-
),
3628
'roles': fields.List(
3729
fields.String(
3830
title='Roles',
@@ -47,67 +39,22 @@
4739

4840
user_response_fields.update(common_fields)
4941

50-
user_role_input_fields = ns.model(
51-
'UserRoleInput',
52-
{
53-
'role': NullableString(
54-
title="User's Role",
55-
required=True,
56-
max_length=50,
57-
description='Role assigned to the user by the tenant',
58-
example=Faker().word(['time-tracker-admin']),
59-
),
60-
},
61-
)
62-
6342

6443
@ns.route('')
6544
class Users(Resource):
6645
@ns.doc('list_users')
6746
@ns.marshal_list_with(user_response_fields)
6847
def get(self):
6948
"""List all users"""
70-
user_role_field_toggle = FeatureToggleManager('bk-user-role-field')
71-
if user_role_field_toggle.is_toggle_enabled_for_user():
72-
azure_connection = AzureConnection()
73-
is_current_user_a_tester = azure_connection.is_test_user(
74-
current_user_id()
75-
)
76-
return (
77-
azure_connection.users_v2()
78-
if is_current_user_a_tester
79-
else azure_connection.get_non_test_users()
80-
)
81-
return AzureConnection().users()
82-
83-
84-
@ns.route('/<string:id>/roles')
85-
@ns.response(HTTPStatus.NOT_FOUND, 'User not found')
86-
@ns.response(HTTPStatus.UNPROCESSABLE_ENTITY, 'The id has an invalid format')
87-
@ns.param('id', 'The user identifier')
88-
class UserRoles(Resource):
89-
@ns.doc('create_user_role')
90-
@ns.expect(user_role_input_fields)
91-
@ns.response(
92-
HTTPStatus.BAD_REQUEST, 'Invalid format or structure of the user'
93-
)
94-
@ns.marshal_with(user_response_fields)
95-
def post(self, id):
96-
"""Create user's role"""
97-
return AzureConnection().update_user_role(id, ns.payload['role'])
98-
99-
100-
@ns.route('/<string:user_id>/roles/<string:role_id>')
101-
@ns.response(HTTPStatus.NOT_FOUND, 'User not found')
102-
@ns.response(HTTPStatus.UNPROCESSABLE_ENTITY, 'The id has an invalid format')
103-
@ns.param('user_id', 'The user identifier')
104-
@ns.param('role_id', 'The role name identifier')
105-
class UserRole(Resource):
106-
@ns.doc('delete_user_role')
107-
@ns.marshal_with(user_response_fields)
108-
def delete(self, user_id, role_id):
109-
"""Delete user's role"""
110-
return AzureConnection().update_user_role(user_id, role=None)
49+
azure_connection = AzureConnection()
50+
is_current_user_a_tester = azure_connection.is_test_user(
51+
current_user_id()
52+
)
53+
return (
54+
azure_connection.users()
55+
if is_current_user_a_tester
56+
else azure_connection.get_non_test_users()
57+
)
11158

11259

11360
@ns.route('/<string:user_id>/roles/<string:role_id>/grant')

0 commit comments

Comments
 (0)