From 56d452373088425fb1f58fecd0b766ee5dd8bfb5 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 16 Mar 2021 15:12:07 +0100 Subject: [PATCH 1/5] add post endpoint for roles --- .../endpoints/role_and_permission_endpoint.py | 35 ++++++++- airflow/api_connexion/openapi/v1.yaml | 72 +++++++++++++++++++ airflow/security/permissions.py | 1 + .../test_role_and_permission_endpoint.py | 54 +++++++++++++- 4 files changed, 159 insertions(+), 3 deletions(-) diff --git a/airflow/api_connexion/endpoints/role_and_permission_endpoint.py b/airflow/api_connexion/endpoints/role_and_permission_endpoint.py index 9668e4d299ded..23eabb35eef48 100644 --- a/airflow/api_connexion/endpoints/role_and_permission_endpoint.py +++ b/airflow/api_connexion/endpoints/role_and_permission_endpoint.py @@ -14,12 +14,13 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from flask import current_app +from flask import current_app, request from flask_appbuilder.security.sqla.models import Permission, Role +from marshmallow import ValidationError from sqlalchemy import func from airflow.api_connexion import security -from airflow.api_connexion.exceptions import NotFound +from airflow.api_connexion.exceptions import AlreadyExists, BadRequest, NotFound from airflow.api_connexion.parameters import apply_sorting, check_limit, format_parameters from airflow.api_connexion.schemas.role_and_permission_schema import ( ActionCollection, @@ -66,3 +67,33 @@ def get_permissions(limit=None, offset=None): query = session.query(Permission) actions = query.offset(offset).limit(limit).all() return action_collection_schema.dump(ActionCollection(actions=actions, total_entries=total_entries)) + + +def delete_role(): + """Delete a role""" + + +def patch_role(): + """Update a role""" + + +@security.requires_access([(permissions.ACTION_CAN_ADD, permissions.RESOURCE_ROLE_MODEL_VIEW)]) +def post_role(): + """Create a new role""" + appbuilder = current_app.appbuilder + security_manager = appbuilder.sm + body = request.json + try: + data = role_schema.load(body) + except ValidationError as err: + raise BadRequest(detail=str(err.messages)) + role = security_manager.find_role(name=data['name']) + if not role: + perms = [ + (item['permission']['name'], item['view_menu']['name']) for item in data['permissions'] if item + ] + security_manager.init_role(role_name=data['name'], perms=perms) + return role_schema.dump(role) + raise AlreadyExists( + detail=f"Role with name `{role.name}` already exist. Please update with patch endpoint" + ) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 0d900592b441b..d8944bcfbab22 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -1412,6 +1412,31 @@ paths: '403': $ref: '#/components/responses/PermissionDenied' + post: + summary: Create a role + x-openapi-router-controller: airflow.api_connexion.endpoints.role_and_permission_endpoint + operationId: post_role + tags: [Role] + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/Role' + responses: + '200': + description: Success. + content: + application/json: + schema: + $ref: '#/components/schemas/Role' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthenticated' + '403': + $ref: '#/components/responses/PermissionDenied' + /roles/{role_name}: parameters: - $ref: '#/components/parameters/RoleName' @@ -1435,6 +1460,53 @@ paths: '404': $ref: '#/components/responses/NotFound' + patch: + summary: Update a role + x-openapi-router-controller: airflow.api_connexion.endpoints.role_and_permission_endpoint + operationId: patch_role + tags: [Role] + parameters: + - $ref: '#/components/parameters/UpdateMask' + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/Role' + + responses: + '200': + description: Success. + content: + application/json: + schema: + $ref: '#/components/schemas/Role' + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthenticated' + '403': + $ref: '#/components/responses/PermissionDenied' + '404': + $ref: '#/components/responses/NotFound' + + delete: + summary: Delete a role + x-openapi-router-controller: airflow.api_connexion.endpoints.role_and_permission_endpoint + operationId: delete_role + tags: [Role] + responses: + '204': + description: Success. + '400': + $ref: '#/components/responses/BadRequest' + '401': + $ref: '#/components/responses/Unauthenticated' + '403': + $ref: '#/components/responses/PermissionDenied' + '404': + $ref: '#/components/responses/NotFound' + /permissions: get: summary: List permissions diff --git a/airflow/security/permissions.py b/airflow/security/permissions.py index 1a49e11a67a6f..ffa4457b3b56f 100644 --- a/airflow/security/permissions.py +++ b/airflow/security/permissions.py @@ -50,6 +50,7 @@ RESOURCE_PERMISSION_MODEL_VIEW = "PermissionModelView" # Action Constants +ACTION_CAN_ADD = "can_add" ACTION_CAN_LIST = "can_list" ACTION_CAN_SHOW = "can_show" ACTION_CAN_CREATE = "can_create" diff --git a/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py b/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py index 4881bf4ecbdeb..f492f3b9ab465 100644 --- a/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py +++ b/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py @@ -21,7 +21,7 @@ from airflow.api_connexion.exceptions import EXCEPTIONS_LINK_MAP from airflow.security import permissions from airflow.www.security import EXISTING_ROLES -from tests.test_utils.api_connexion_utils import assert_401, create_user, delete_user +from tests.test_utils.api_connexion_utils import assert_401, create_user, delete_role, delete_user @pytest.fixture(scope="module") @@ -33,6 +33,7 @@ def configured_app(minimal_app_for_api): role_name="Test", permissions=[ (permissions.ACTION_CAN_LIST, permissions.RESOURCE_ROLE_MODEL_VIEW), + (permissions.ACTION_CAN_ADD, permissions.RESOURCE_ROLE_MODEL_VIEW), (permissions.ACTION_CAN_SHOW, permissions.RESOURCE_ROLE_MODEL_VIEW), (permissions.ACTION_CAN_LIST, permissions.RESOURCE_PERMISSION_MODEL_VIEW), ], @@ -154,3 +155,54 @@ def test_should_raise_403_forbidden(self): "/api/v1/permissions", environ_overrides={'REMOTE_USER': "test_no_permissions"} ) assert response.status_code == 403 + + +class TestPostRole(TestRoleEndpoint): + def test_post_should_respond_200(self): + payload = { + 'name': 'Test2', + 'actions': [{'resource': {'name': 'Connections'}, 'action': {'name': 'can_create'}}], + } + response = self.client.post("/api/v1/roles", json=payload, environ_overrides={'REMOTE_USER': "test"}) + assert response.status_code == 200 + role = self.app.appbuilder.sm.find_role('Test2') + assert role is not None + delete_role(self.app, "Test2") + + def test_post_should_respond_400_for_invalid_payload(self): + payload = { + 'actions': [{'resource': {'name': 'Connections'}, 'action': {'name': 'can_create'}}], + } + response = self.client.post("/api/v1/roles", json=payload, environ_overrides={'REMOTE_USER': "test"}) + assert response.status_code == 400 + assert response.json == { + 'detail': "{'name': ['Missing data for required field.']}", + 'status': 400, + 'title': 'Bad Request', + 'type': EXCEPTIONS_LINK_MAP[400], + } + + def test_post_should_respond_409_already_exist(self): + payload = { + 'name': 'Test', + 'actions': [{'resource': {'name': 'Connections'}, 'action': {'name': 'can_create'}}], + } + response = self.client.post("/api/v1/roles", json=payload, environ_overrides={'REMOTE_USER': "test"}) + assert response.status_code == 409 + assert response.json == { + 'detail': "Role with name `Test` already exist. Please update with patch endpoint", + 'status': 409, + 'title': 'Conflict', + 'type': EXCEPTIONS_LINK_MAP[409], + } + + def test_should_raises_401_unauthenticated(self): + response = self.client.post( + "/api/v1/roles", + json={ + 'name': 'Test2', + 'actions': [{'resource': {'name': 'Connections'}, 'action': {'name': 'can_create'}}], + }, + ) + + assert_401(response) From e09dc0a3ee5e2c2bd097412a87e31441647b00a2 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 16 Mar 2021 21:59:24 +0100 Subject: [PATCH 2/5] add patch and delete endpoint --- .../endpoints/role_and_permission_endpoint.py | 42 +++- airflow/security/permissions.py | 1 + .../test_role_and_permission_endpoint.py | 195 +++++++++++++++++- 3 files changed, 234 insertions(+), 4 deletions(-) diff --git a/airflow/api_connexion/endpoints/role_and_permission_endpoint.py b/airflow/api_connexion/endpoints/role_and_permission_endpoint.py index 23eabb35eef48..c82c9afbf7040 100644 --- a/airflow/api_connexion/endpoints/role_and_permission_endpoint.py +++ b/airflow/api_connexion/endpoints/role_and_permission_endpoint.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from connexion import NoContent from flask import current_app, request from flask_appbuilder.security.sqla.models import Permission, Role from marshmallow import ValidationError @@ -69,12 +70,49 @@ def get_permissions(limit=None, offset=None): return action_collection_schema.dump(ActionCollection(actions=actions, total_entries=total_entries)) -def delete_role(): +@security.requires_access([(permissions.ACTION_CAN_DELETE, permissions.RESOURCE_ROLE_MODEL_VIEW)]) +def delete_role(role_name): """Delete a role""" + ab_security_manager = current_app.appbuilder.sm + role = ab_security_manager.find_role(name=role_name) + if not role: + raise NotFound(title="Role not found", detail=f"The Role with name `{role_name}` was not found") + ab_security_manager.delete_role(role_name=role_name) + return NoContent, 204 -def patch_role(): +@security.requires_access([(permissions.ACTION_CAN_EDIT, permissions.RESOURCE_ROLE_MODEL_VIEW)]) +def patch_role(role_name, update_mask=None): """Update a role""" + appbuilder = current_app.appbuilder + security_manager = appbuilder.sm + body = request.json + try: + data = role_schema.load(body) + except ValidationError as err: + raise BadRequest(detail=str(err.messages)) + role = security_manager.find_role(name=role_name) + if not role: + raise NotFound(title="Role not found", detail=f"Role with name: `{role_name} was not found") + if update_mask: + update_mask = [i.strip() for i in update_mask] + data_ = {} + for field in update_mask: + if field in data and not field == "permissions": + data_[field] = data[field] + elif field == "actions": + data_["permissions"] = data['permissions'] + else: + raise BadRequest(detail=f"'{field}' in update_mask is unknown") + data = data_ + perms = data.get("permissions", []) + if perms: + perms = [ + (item['permission']['name'], item['view_menu']['name']) for item in data['permissions'] if item + ] + security_manager.update_role(pk=role.id, name=data['name']) + security_manager.init_role(role_name=data['name'], perms=perms or role.permissions) + return role_schema.dump(role) @security.requires_access([(permissions.ACTION_CAN_ADD, permissions.RESOURCE_ROLE_MODEL_VIEW)]) diff --git a/airflow/security/permissions.py b/airflow/security/permissions.py index ffa4457b3b56f..ac6cfc72a3611 100644 --- a/airflow/security/permissions.py +++ b/airflow/security/permissions.py @@ -53,6 +53,7 @@ ACTION_CAN_ADD = "can_add" ACTION_CAN_LIST = "can_list" ACTION_CAN_SHOW = "can_show" +ACTION_CAN_DELETE = "can_delete" ACTION_CAN_CREATE = "can_create" ACTION_CAN_READ = "can_read" ACTION_CAN_EDIT = "can_edit" diff --git a/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py b/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py index f492f3b9ab465..3b59a0d9e8dba 100644 --- a/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py +++ b/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py @@ -16,12 +16,19 @@ # under the License. import pytest +from flask_appbuilder.security.sqla.models import Role from parameterized import parameterized from airflow.api_connexion.exceptions import EXCEPTIONS_LINK_MAP from airflow.security import permissions from airflow.www.security import EXISTING_ROLES -from tests.test_utils.api_connexion_utils import assert_401, create_user, delete_role, delete_user +from tests.test_utils.api_connexion_utils import ( + assert_401, + create_role, + create_user, + delete_role, + delete_user, +) @pytest.fixture(scope="module") @@ -36,6 +43,8 @@ def configured_app(minimal_app_for_api): (permissions.ACTION_CAN_ADD, permissions.RESOURCE_ROLE_MODEL_VIEW), (permissions.ACTION_CAN_SHOW, permissions.RESOURCE_ROLE_MODEL_VIEW), (permissions.ACTION_CAN_LIST, permissions.RESOURCE_PERMISSION_MODEL_VIEW), + (permissions.ACTION_CAN_DELETE, permissions.RESOURCE_ROLE_MODEL_VIEW), + (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_ROLE_MODEL_VIEW), ], ) create_user(app, username="test_no_permissions", role_name="TestNoPermissions") # type: ignore @@ -51,6 +60,18 @@ def setup_attrs(self, configured_app) -> None: self.app = configured_app self.client = self.app.test_client() # type:ignore + def teardown_method(self): + """ + Delete all roles except these ones. + Test and TestNoPermissions are deleted by delete_user above + """ + session = self.app.appbuilder.get_session + existing_roles = set(EXISTING_ROLES) + existing_roles.update(['Test', 'TestNoPermissions']) + roles = session.query(Role).filter(~Role.name.in_(existing_roles)).all() + for role in roles: + delete_role(self.app, role.name) + class TestGetRoleEndpoint(TestRoleEndpoint): def test_should_response_200(self): @@ -167,7 +188,6 @@ def test_post_should_respond_200(self): assert response.status_code == 200 role = self.app.appbuilder.sm.find_role('Test2') assert role is not None - delete_role(self.app, "Test2") def test_post_should_respond_400_for_invalid_payload(self): payload = { @@ -206,3 +226,174 @@ def test_should_raises_401_unauthenticated(self): ) assert_401(response) + + def test_should_raise_403_forbidden(self): + response = self.client.post( + "/api/v1/roles", + json={ + "name": "mytest2", + "actions": [{"resource": {"name": "Connections"}, "action": {"name": "can_create"}}], + }, + environ_overrides={'REMOTE_USER': "test_no_permissions"}, + ) + assert response.status_code == 403 + + +class TestDeleteRole(TestRoleEndpoint): + def test_delete_should_respond_204(self, session): + role = create_role(self.app, "mytestrole") + response = self.client.delete(f"/api/v1/roles/{role.name}", environ_overrides={'REMOTE_USER': "test"}) + assert response.status_code == 204 + role_obj = session.query(Role).filter(Role.name == role.name).all() + assert len(role_obj) == 0 + + def test_delete_should_respond_404(self): + response = self.client.delete( + "/api/v1/roles/invalidrolename", environ_overrides={'REMOTE_USER': "test"} + ) + assert response.status_code == 404 + assert response.json == { + 'detail': "The Role with name `invalidrolename` was not found", + 'status': 404, + 'title': 'Role not found', + 'type': EXCEPTIONS_LINK_MAP[404], + } + + def test_should_raises_401_unauthenticated(self): + response = self.client.delete("/api/v1/roles/test") + + assert_401(response) + + def test_should_raise_403_forbidden(self): + response = self.client.delete( + "/api/v1/roles/test", environ_overrides={'REMOTE_USER': "test_no_permissions"} + ) + assert response.status_code == 403 + + +class TestPatchRole(TestRoleEndpoint): + @parameterized.expand( + [ + ({"name": "mytest"}, "mytest", []), + ( + { + "name": "mytest2", + "actions": [{"resource": {"name": "Connections"}, "action": {"name": "can_create"}}], + }, + "mytest2", + [{"resource": {"name": "Connections"}, "action": {"name": "can_create"}}], + ), + ] + ) + def test_patch_should_respond_200(self, payload, expected_name, expected_actions): + role = create_role(self.app, 'mytestrole') + response = self.client.patch( + f"/api/v1/roles/{role.name}", json=payload, environ_overrides={'REMOTE_USER': "test"} + ) + assert response.status_code == 200 + assert response.json['name'] == expected_name + assert response.json["actions"] == expected_actions + + @parameterized.expand( + [ + ( + "?update_mask=name", + { + "name": "mytest2", + "actions": [{"resource": {"name": "Connections"}, "action": {"name": "can_create"}}], + }, + "mytest2", + [], + ), + ( + "?update_mask=name, actions", # both name and actions in update mask + { + "name": "mytest2", + "actions": [{"resource": {"name": "Connections"}, "action": {"name": "can_create"}}], + }, + "mytest2", + [{"resource": {"name": "Connections"}, "action": {"name": "can_create"}}], + ), + ] + ) + def test_patch_should_respond_200_with_update_mask( + self, update_mask, payload, expected_name, expected_actions + ): + role = create_role(self.app, "mytestrole") + assert role.permissions == [] + response = self.client.patch( + f"/api/v1/roles/{role.name}{update_mask}", + json=payload, + environ_overrides={'REMOTE_USER': "test"}, + ) + assert response.status_code == 200 + assert response.json['name'] == expected_name + assert response.json['actions'] == expected_actions + + def test_patch_should_respond_400_for_invalid_fields_in_update_mask(self): + role = create_role(self.app, "mytestrole") + payload = {"name": "testme"} + response = self.client.patch( + f"/api/v1/roles/{role.name}?update_mask=invalid_name", + json=payload, + environ_overrides={'REMOTE_USER': "test"}, + ) + assert response.status_code == 400 + assert response.json['detail'] == "'invalid_name' in update_mask is unknown" + + @parameterized.expand( + [ + ( + { + "name": "testme", + "permissions": [ # Using permissions instead of actions should raise + {"resource": {"name": "Connections"}, "action": {"name": "can_create"}} + ], + }, + "{'permissions': ['Unknown field.']}", + ), + ( + { + "name": "testme", + "actions": [ + { + "view_menu": {"name": "Connections"}, # Using view_menu instead of resource + "action": {"name": "can_create"}, + } + ], + }, + "{'actions': {0: {'view_menu': ['Unknown field.']}}}", + ), + ] + ) + def test_patch_should_respond_400_for_invalid_update(self, payload, expected_error): + role = create_role(self.app, "mytestrole") + response = self.client.patch( + f"/api/v1/roles/{role.name}", + json=payload, + environ_overrides={'REMOTE_USER': "test"}, + ) + assert response.status_code == 400 + assert response.json['detail'] == expected_error + + def test_should_raises_401_unauthenticated(self): + response = self.client.patch( + "/api/v1/roles/test", + json={ + "name": "mytest2", + "actions": [{"resource": {"name": "Connections"}, "action": {"name": "can_create"}}], + }, + ) + + assert_401(response) + + def test_should_raise_403_forbidden(self): + response = self.client.patch( + "/api/v1/roles/test", + json={ + "name": "mytest2", + "actions": [{"resource": {"name": "Connections"}, "action": {"name": "can_create"}}], + }, + environ_overrides={'REMOTE_USER': "test_no_permissions"}, + ) + assert response.status_code == 403 From 178690f7ecc35cfb79f75fbf634b0a2b810ea6eb Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Tue, 16 Mar 2021 22:27:55 +0100 Subject: [PATCH 3/5] fixup! add patch and delete endpoint --- airflow/security/permissions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/airflow/security/permissions.py b/airflow/security/permissions.py index ac6cfc72a3611..ffa4457b3b56f 100644 --- a/airflow/security/permissions.py +++ b/airflow/security/permissions.py @@ -53,7 +53,6 @@ ACTION_CAN_ADD = "can_add" ACTION_CAN_LIST = "can_list" ACTION_CAN_SHOW = "can_show" -ACTION_CAN_DELETE = "can_delete" ACTION_CAN_CREATE = "can_create" ACTION_CAN_READ = "can_read" ACTION_CAN_EDIT = "can_edit" From a64b94190dc1cf616a0c9c9c0b6bd97bd8c65dae Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 22 Mar 2021 14:57:46 +0100 Subject: [PATCH 4/5] Improve code and test --- .../endpoints/role_and_permission_endpoint.py | 16 +++ .../test_role_and_permission_endpoint.py | 97 +++++++++++++++++-- 2 files changed, 107 insertions(+), 6 deletions(-) diff --git a/airflow/api_connexion/endpoints/role_and_permission_endpoint.py b/airflow/api_connexion/endpoints/role_and_permission_endpoint.py index c82c9afbf7040..b8e06915d8733 100644 --- a/airflow/api_connexion/endpoints/role_and_permission_endpoint.py +++ b/airflow/api_connexion/endpoints/role_and_permission_endpoint.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. + from connexion import NoContent from flask import current_app, request from flask_appbuilder.security.sqla.models import Permission, Role @@ -33,6 +34,19 @@ from airflow.security import permissions +def _check_action_and_resource(sm, perms): + """ + Checks if the action or resource exists and raise 400 if not + + This function is intended for use in the REST API because it raise 400 + """ + for item in perms: + if not sm.find_permission(item[0]): + raise BadRequest(detail=f"The specified action: '{item[0]}' was not found") + if not sm.find_view_menu(item[1]): + raise BadRequest(detail=f"The specified resource: '{item[1]}' was not found") + + @security.requires_access([(permissions.ACTION_CAN_SHOW, permissions.RESOURCE_ROLE_MODEL_VIEW)]) def get_role(role_name): """Get role""" @@ -110,6 +124,7 @@ def patch_role(role_name, update_mask=None): perms = [ (item['permission']['name'], item['view_menu']['name']) for item in data['permissions'] if item ] + _check_action_and_resource(security_manager, perms) security_manager.update_role(pk=role.id, name=data['name']) security_manager.init_role(role_name=data['name'], perms=perms or role.permissions) return role_schema.dump(role) @@ -130,6 +145,7 @@ def post_role(): perms = [ (item['permission']['name'], item['view_menu']['name']) for item in data['permissions'] if item ] + _check_action_and_resource(security_manager, perms) security_manager.init_role(role_name=data['name'], perms=perms) return role_schema.dump(role) raise AlreadyExists( diff --git a/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py b/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py index 3b59a0d9e8dba..c99304eed3488 100644 --- a/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py +++ b/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py @@ -40,7 +40,7 @@ def configured_app(minimal_app_for_api): role_name="Test", permissions=[ (permissions.ACTION_CAN_LIST, permissions.RESOURCE_ROLE_MODEL_VIEW), - (permissions.ACTION_CAN_ADD, permissions.RESOURCE_ROLE_MODEL_VIEW), + (permissions.ACTION_CAN_CREATE, permissions.RESOURCE_ROLE_MODEL_VIEW), (permissions.ACTION_CAN_SHOW, permissions.RESOURCE_ROLE_MODEL_VIEW), (permissions.ACTION_CAN_LIST, permissions.RESOURCE_PERMISSION_MODEL_VIEW), (permissions.ACTION_CAN_DELETE, permissions.RESOURCE_ROLE_MODEL_VIEW), @@ -189,14 +189,75 @@ def test_post_should_respond_200(self): role = self.app.appbuilder.sm.find_role('Test2') assert role is not None - def test_post_should_respond_400_for_invalid_payload(self): - payload = { - 'actions': [{'resource': {'name': 'Connections'}, 'action': {'name': 'can_create'}}], - } + @parameterized.expand( + [ + ( + { + 'actions': [{'resource': {'name': 'Connections'}, 'action': {'name': 'can_create'}}], + }, + "{'name': ['Missing data for required field.']}", + ), + ( + { + 'name': "TestRole", + 'actionss': [ + { + 'resource': {'name': 'Connections'}, # actionss not correct + 'action': {'name': 'can_create'}, + } + ], + }, + "{'actionss': ['Unknown field.']}", + ), + ( + { + 'name': "TestRole", + 'actions': [ + { + 'resources': {'name': 'Connections'}, # resources is invalid, should be resource + 'action': {'name': 'can_create'}, + } + ], + }, + "{'actions': {0: {'resources': ['Unknown field.']}}}", + ), + ( + { + 'name': "TestRole", + 'actions': [ + {'resource': {'name': 'Connections'}, 'actions': {'name': 'can_create'}} + ], # actions is invalid, should be action + }, + "{'actions': {0: {'actions': ['Unknown field.']}}}", + ), + ( + { + 'name': "TestRole", + 'actions': [ + { + 'resource': {'name': 'FooBars'}, # FooBars is not a resource + 'action': {'name': 'can_create'}, + } + ], + }, + "The specified resource: 'FooBars' was not found", + ), + ( + { + 'name': "TestRole", + 'actions': [ + {'resource': {'name': 'Connections'}, 'action': {'name': 'can_amend'}} + ], # can_amend is not an action + }, + "The specified action: 'can_amend' was not found", + ), + ] + ) + def test_post_should_respond_400_for_invalid_payload(self, payload, error_message): response = self.client.post("/api/v1/roles", json=payload, environ_overrides={'REMOTE_USER': "test"}) assert response.status_code == 400 assert response.json == { - 'detail': "{'name': ['Missing data for required field.']}", + 'detail': error_message, 'status': 400, 'title': 'Bad Request', 'type': EXCEPTIONS_LINK_MAP[400], @@ -364,6 +425,30 @@ def test_patch_should_respond_400_for_invalid_fields_in_update_mask(self): }, "{'actions': {0: {'view_menu': ['Unknown field.']}}}", ), + ( + { + "name": "testme", + "actions": [ + { + "resource": {"name": "FooBars"}, # Using wrong resource name + "action": {"name": "can_create"}, + } + ], + }, + "The specified resource: 'FooBars' was not found", + ), + ( + { + "name": "testme", + "actions": [ + { + "resource": {"name": "Connections"}, # Using wrong action name + "action": {"name": "can_invalid"}, + } + ], + }, + "The specified action: 'can_invalid' was not found", + ), ] ) def test_patch_should_respond_400_for_invalid_update(self, payload, expected_error): From ac1a194cdbd8a4081d0e6bcce17ed86d54b97fd9 Mon Sep 17 00:00:00 2001 From: EphraimBuddy Date: Mon, 22 Mar 2021 15:29:19 +0100 Subject: [PATCH 5/5] fixup! Improve code and test --- .../endpoints/test_role_and_permission_endpoint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py b/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py index c99304eed3488..6b0728258595f 100644 --- a/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py +++ b/tests/api_connexion/endpoints/test_role_and_permission_endpoint.py @@ -40,7 +40,7 @@ def configured_app(minimal_app_for_api): role_name="Test", permissions=[ (permissions.ACTION_CAN_LIST, permissions.RESOURCE_ROLE_MODEL_VIEW), - (permissions.ACTION_CAN_CREATE, permissions.RESOURCE_ROLE_MODEL_VIEW), + (permissions.ACTION_CAN_ADD, permissions.RESOURCE_ROLE_MODEL_VIEW), (permissions.ACTION_CAN_SHOW, permissions.RESOURCE_ROLE_MODEL_VIEW), (permissions.ACTION_CAN_LIST, permissions.RESOURCE_PERMISSION_MODEL_VIEW), (permissions.ACTION_CAN_DELETE, permissions.RESOURCE_ROLE_MODEL_VIEW),