From 825f5e14b02727114ba57b325b2c70eabe460cf6 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Tue, 15 Mar 2022 14:31:37 -0400 Subject: [PATCH 01/19] first draft patch permissions --- magpie/api/management/resource/__init__.py | 1 + .../api/management/resource/resource_views.py | 75 ++++++++++++++++++- magpie/api/schemas.py | 68 +++++++++++++++++ tests/interfaces.py | 67 +++++++++++++++++ 4 files changed, 209 insertions(+), 2 deletions(-) diff --git a/magpie/api/management/resource/__init__.py b/magpie/api/management/resource/__init__.py index e8582980a..448306d1a 100644 --- a/magpie/api/management/resource/__init__.py +++ b/magpie/api/management/resource/__init__.py @@ -7,6 +7,7 @@ def includeme(config): LOGGER.info("Adding API resource...") # Add all the rest api routes + config.add_route(**s.service_api_route_info(s.PermissionsAPI)) config.add_route(**s.service_api_route_info(s.ResourcesAPI)) config.add_route(**s.service_api_route_info(s.ResourceAPI)) config.add_route(**s.service_api_route_info(s.ResourcePermissionsAPI)) diff --git a/magpie/api/management/resource/resource_views.py b/magpie/api/management/resource/resource_views.py index d3a13259d..3e17d0dcd 100644 --- a/magpie/api/management/resource/resource_views.py +++ b/magpie/api/management/resource/resource_views.py @@ -1,6 +1,8 @@ -from pyramid.httpexceptions import HTTPBadRequest, HTTPConflict, HTTPForbidden, HTTPInternalServerError, HTTPOk +from pyramid.httpexceptions import HTTPBadRequest, HTTPConflict, HTTPForbidden, HTTPInternalServerError, HTTPNotFound, HTTPOk from pyramid.settings import asbool from pyramid.view import view_config +from ziggurat_foundations.models.services.group import GroupService +from ziggurat_foundations.models.services.user import UserService from magpie import models from magpie.api import exception as ax @@ -11,7 +13,7 @@ from magpie.api.management.service.service_formats import format_service_resources from magpie.api.management.service.service_utils import get_services_by_type from magpie.permissions import PermissionType, format_permissions -from magpie.register import sync_services_phoenix +from magpie.register import sync_services_phoenix, magpie_register_permissions_from_config from magpie.services import SERVICE_TYPE_DICT, get_resource_child_allowed @@ -162,3 +164,72 @@ def get_res_types(res): } return ax.valid_http(http_success=HTTPOk, content=data, detail=s.ResourceTypes_GET_OkResponseSchema.description) + + +@s.PermissionsAPI.patch(schema=s.Permissions_PATCH_RequestSchema, tags=[s.PermissionTag], + response_schema=s.Permissions_PATCH_responses) +@view_config(route_name=s.PermissionsAPI.name, request_method="PATCH") +def update_permissions(request): + """ + Update the requested permissions. + """ + permissions = ar.get_value_multiformat_body_checked(request, "permissions", check_type=list) + ax.verify_param(permissions, not_none=True, not_empty=True, + http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + + for entry in permissions: + # Check if user/group exists for each permission + if "permission" in entry.keys() and entry["permission"]: + if "user" in entry.keys(): + user = UserService.by_user_name(entry["user"], db_session=request.db) + if user is None: + raise RuntimeError(f"User {entry['user']} not found in the database.") + if "group" in entry.keys(): + group = GroupService.by_group_name(entry["group"], db_session=request.db) + if group is None: + raise RuntimeError(f"Group {entry['group']} not found in the database.") + + # Reformat permissions for function + permissions_cfg = {"permissions": []} + resource_full_path = "" + for i, entry in enumerate(permissions): + resource_name = entry.get("resource_name") + resource_type = entry.get("resource_type") + permission = entry.get("permission") # TODO: should verify if either dict or string, should maybe verify other fields? + user = entry.get("user") + group = entry.get("group") + action = entry.get("action", "create") + + ax.verify_param(resource_name, not_none=True, + http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + ax.verify_param(resource_type, not_none=True, + http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + if i == 0: + # First resource must be a service + ax.verify_param(resource_type, is_equal=True, param_compare="service", + http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + service_name = resource_name + else: + resource_full_path += "/" + resource_name + if permission: + # Check that a user and/or a group is defined + ax.verify_param(bool(user or group), is_true=True, + http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + cfg_entry = { + "service": service_name, + "resource": resource_full_path, + "type": resource_type, + "permission": permission, + "action": action + } + if user: + cfg_entry["user"] = user + if group: + cfg_entry["group"] = group + + permissions_cfg["permissions"].append(cfg_entry) + + # apply permission update + magpie_register_permissions_from_config(permissions_config=permissions_cfg, db_session=request.db) + + return ax.valid_http(http_success=HTTPOk, detail=s.Permissions_PATCH_OkResponseSchema.description) diff --git a/magpie/api/schemas.py b/magpie/api/schemas.py index 285f0d93d..baad76d4d 100644 --- a/magpie/api/schemas.py +++ b/magpie/api/schemas.py @@ -198,6 +198,9 @@ def service_api_route_info(service_api, **kwargs): GroupResourcePermissionAPI = Service( path="/groups/{group_name}/resources/{resource_id}/permissions/{permission_name}", name="GroupResourcePermission") +PermissionsAPI = Service( + path="/permissions", + name="Permissions") RegisterGroupsAPI = Service( path="/register/groups", name="RegisterGroups") @@ -777,6 +780,63 @@ class PermissionNameListSchema(colander.SequenceSchema): ) +class PermissionPatchObjectSchema(colander.MappingSchema): + resource_name = colander.SchemaNode( + colander.String(), + description="Name of the resource to create." + ) + resource_type = colander.SchemaNode( + colander.String(), + description="Type of the resource", + example="service" + ) + + user = UserNameParameter + user.missing = colander.drop + + group = GroupNameParameter + group.missing = colander.drop + + permission = PermissionObjectSchema( + missing=colander.drop + ) + + action = colander.SchemaNode( + colander.String(), + description="Action to apply on the permission.", + example="create", + default="create", + validator=colander.OneOf(["create", "delete"]) + ) + + +class PermissionPatchListSchema(colander.SequenceSchema): + permission = PermissionPatchObjectSchema() + + +class Permissions_PATCH_RequestBodySchema(colander.MappingSchema): + permissions = PermissionPatchListSchema() + + +class Permissions_PATCH_RequestSchema(BaseRequestSchemaAPI): + body = Permissions_PATCH_RequestBodySchema() + + +class Permissions_PATCH_OkResponseSchema(BaseResponseSchemaAPI): + description = "Update permissions successful." + body = BaseResponseBodySchema(code=HTTPOk.code, description=description) + + +class Permissions_PATCH_BadRequestResponseSchema(BaseResponseSchemaAPI): + description = "Missing parameters to update permissions or parameters do not provide any modification." + body = ErrorResponseBodySchema(code=HTTPBadRequest.code, description=description) + + +class Permissions_PATCH_ForbiddenResponseSchema(BaseResponseSchemaAPI): + description = "Failed to update requested permissions." + body = ErrorResponseBodySchema(code=HTTPForbidden.code, description=description) + + class BaseUserInfoSchema(colander.MappingSchema): user_name = UserNameParameter email = colander.SchemaNode( @@ -3490,6 +3550,14 @@ class SwaggerAPI_GET_OkResponseSchema(colander.MappingSchema): "422": UnprocessableEntityResponseSchema(), "500": InternalServerErrorResponseSchema(), } +Permissions_PATCH_responses = { + "200": Permissions_PATCH_OkResponseSchema(), + "400": Permissions_PATCH_BadRequestResponseSchema(), # FIXME: https://github.com/Ouranosinc/Magpie/issues/359 + "401": UnauthorizedResponseSchema(), + "403": Permissions_PATCH_ForbiddenResponseSchema(), + "406": NotAcceptableResponseSchema(), + "500": InternalServerErrorResponseSchema(), +} Users_GET_responses = { "200": Users_GET_OkResponseSchema(), "400": Users_GET_BadRequestSchema(), diff --git a/tests/interfaces.py b/tests/interfaces.py index 2e1e35006..79d5c0969 100644 --- a/tests/interfaces.py +++ b/tests/interfaces.py @@ -2271,6 +2271,73 @@ def test_DeleteUserServicePermission(self): body = utils.check_response_basic_info(resp, 200, expected_method="GET") utils.check_val_not_in(self.test_service_perm, body["permission_names"]) + @runner.MAGPIE_TEST_PERMISSIONS + def test_PatchPermissions(self): + utils.TestSetup.create_TestGroup(self) + utils.TestSetup.create_TestUser(self) + utils.TestSetup.create_TestService(self, override_service_type="thredds") + + resource_names = ["folder1", "folder2", "the-file"] + data = { + "permissions": [ + { + "resource_name": self.test_service_name, + "resource_type": "service", + "permission": None + }, + { + "resource_name": resource_names[0], + "resource_type": "directory" + }, + { + "resource_name": resource_names[1], + "resource_type": "directory", + # "permission": "browse", + # "group": self.test_group_name + }, + { + "resource_name": resource_names[2], + "resource_type": "file", + "permission": { + "name": "read", + "access": "deny", + "scope": "match" + }, + "user": self.test_user_name + } + ] + } + path = "/permissions" + resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, cookies=self.cookies, json=data) + utils.check_response_basic_info(resp, 200, expected_method="PATCH") + + path = f"/services/{self.test_service_name}/resources" + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + body = utils.check_response_basic_info(resp) + resources = body[self.test_service_name]["resources"] + for res_name in resource_names: + target_res = None + for res_id, res in resources.items(): + if res["resource_name"] == res_name: + target_res = res + break + # Check if resource was found + assert target_res + + # if res with permission, get permissions and check + if res_name == resource_names[1]: + path = f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions" + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + body = utils.check_response_basic_info(resp) + # TODO: check if the right permission is found + elif res_name == resource_names[2]: + path = f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions" + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + body = utils.check_response_basic_info(resp) + # TODO: check if the right permission is found + resources = target_res["children"] + + def create_validate_permissions(self, test_user_name, # type: Str test_resource_id, # type: int # resource to validate perm against From 283335f4372799008c7194a8f867cc915f26e77f Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 16 Mar 2022 08:36:08 -0400 Subject: [PATCH 02/19] permissions config now supports using a path of types corresponding to the path of resources --- config/permissions.cfg | 2 ++ magpie/api/management/resource/resource_views.py | 7 ++++++- magpie/register.py | 15 +++++++++++++-- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/config/permissions.cfg b/config/permissions.cfg index 32ee75253..54f405b58 100644 --- a/config/permissions.cfg +++ b/config/permissions.cfg @@ -14,6 +14,8 @@ # service: service name to receive the permission (directly on it if no 'resource' mentioned, must exist) # resource: (optional) tree path of the service's resource (ex: /res1/sub-res2/sub-sub-res3) # type: (optional) resource type employed in case of ambiguity between multiple choices (depends on service type) +# can either be a single type, or a full path of resource types corresponding to the types of each +# resource found in the path of the 'resource' parameter (ex: /directory/directory/file) # user: user for which to apply the modification of permission (skip if omitted or None) # group: group for which to apply the modification of permission (skip if omitted or None) # permission: name or object of the permission to be applied (see 'magpie.permissions' for supported values) diff --git a/magpie/api/management/resource/resource_views.py b/magpie/api/management/resource/resource_views.py index 3e17d0dcd..739c0c15a 100644 --- a/magpie/api/management/resource/resource_views.py +++ b/magpie/api/management/resource/resource_views.py @@ -192,6 +192,7 @@ def update_permissions(request): # Reformat permissions for function permissions_cfg = {"permissions": []} resource_full_path = "" + resource_full_type = "" for i, entry in enumerate(permissions): resource_name = entry.get("resource_name") resource_type = entry.get("resource_type") @@ -211,6 +212,10 @@ def update_permissions(request): service_name = resource_name else: resource_full_path += "/" + resource_name + # Other resources must not be services + ax.verify_param(resource_type, not_equal=True, param_compare="service", + http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + resource_full_type += "/" + resource_type if permission: # Check that a user and/or a group is defined ax.verify_param(bool(user or group), is_true=True, @@ -218,7 +223,7 @@ def update_permissions(request): cfg_entry = { "service": service_name, "resource": resource_full_path, - "type": resource_type, + "type": resource_type if resource_type == "service" else resource_full_type, "permission": permission, "action": action } diff --git a/magpie/register.py b/magpie/register.py index e3df6a286..dc7202f10 100644 --- a/magpie/register.py +++ b/magpie/register.py @@ -730,7 +730,7 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem resource = None resource_path = permission_config_entry.get("resource", "") - resource_type = permission_config_entry.get("type") + resource_type_config = permission_config_entry.get("type") if resource_path.startswith("/"): resource_path = resource_path[1:] if resource_path.endswith("/"): @@ -739,6 +739,17 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem try: svc_name = service_info["service_name"] svc_type = service_info["service_type"] + + # Prepare a list of types that fits with the list of resources + resource_type_list = resource_type_config.strip("/").split("/") if resource_type_config else [None] + resource_list = resource_path.split("/") + if len(resource_type_list) == 1: + # if only one type specified, assume every path of the resource uses the same resource type + resource_type_list = resource_type_list[0] * len(resource_list) + if len(resource_list) != len(resource_type_list): + raise RegistrationConfigurationError("Invalid resource type found in configuration : " + f"{permission_config_entry.get('type')}") + res_path = None if _use_request(cookies_or_session): res_path = get_magpie_url() + ServiceResourcesAPI.path.format(service_name=svc_name) @@ -750,7 +761,7 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem res_dict = format_service_resources(svc, show_all_children=True, db_session=cookies_or_session) parent = res_dict["resource_id"] child_resources = res_dict["resources"] - for res in resource_path.split("/"): + for res, resource_type in zip(resource_list, resource_type_list): # search in existing children resources if len(child_resources): res_id = list(filter(lambda r: res in [r, child_resources[r]["resource_name"]], child_resources)) From dd562c04f2c23e4c48a85f0ed7b6215ae45083d8 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 16 Mar 2022 08:46:34 -0400 Subject: [PATCH 03/19] clean up update_permissions function --- magpie/api/management/resource/resource_views.py | 14 ++++++-------- tests/interfaces.py | 6 +++--- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/magpie/api/management/resource/resource_views.py b/magpie/api/management/resource/resource_views.py index 739c0c15a..5ad8f91f5 100644 --- a/magpie/api/management/resource/resource_views.py +++ b/magpie/api/management/resource/resource_views.py @@ -178,25 +178,23 @@ def update_permissions(request): http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) for entry in permissions: - # Check if user/group exists for each permission + # Check if user/group exists for each permission found if "permission" in entry.keys() and entry["permission"]: if "user" in entry.keys(): - user = UserService.by_user_name(entry["user"], db_session=request.db) - if user is None: + if not UserService.by_user_name(entry["user"], db_session=request.db): raise RuntimeError(f"User {entry['user']} not found in the database.") if "group" in entry.keys(): - group = GroupService.by_group_name(entry["group"], db_session=request.db) - if group is None: + if not GroupService.by_group_name(entry["group"], db_session=request.db): raise RuntimeError(f"Group {entry['group']} not found in the database.") - # Reformat permissions for function + # Reformat permissions config permissions_cfg = {"permissions": []} resource_full_path = "" resource_full_type = "" for i, entry in enumerate(permissions): resource_name = entry.get("resource_name") resource_type = entry.get("resource_type") - permission = entry.get("permission") # TODO: should verify if either dict or string, should maybe verify other fields? + permission = entry.get("permission") user = entry.get("user") group = entry.get("group") action = entry.get("action", "create") @@ -234,7 +232,7 @@ def update_permissions(request): permissions_cfg["permissions"].append(cfg_entry) - # apply permission update + # Apply permission update magpie_register_permissions_from_config(permissions_config=permissions_cfg, db_session=request.db) return ax.valid_http(http_success=HTTPOk, detail=s.Permissions_PATCH_OkResponseSchema.description) diff --git a/tests/interfaces.py b/tests/interfaces.py index 79d5c0969..52001329a 100644 --- a/tests/interfaces.py +++ b/tests/interfaces.py @@ -2292,8 +2292,8 @@ def test_PatchPermissions(self): { "resource_name": resource_names[1], "resource_type": "directory", - # "permission": "browse", - # "group": self.test_group_name + "permission": "browse", + "group": self.test_group_name }, { "resource_name": resource_names[2], @@ -2303,7 +2303,7 @@ def test_PatchPermissions(self): "access": "deny", "scope": "match" }, - "user": self.test_user_name + "user": "blarg"#self.test_user_name } ] } From 497d6f8c9debe7ed56a8a6ad4de91b03f033cbfb Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 16 Mar 2022 10:32:24 -0400 Subject: [PATCH 04/19] add other test cases for patch permissions --- .../api/management/resource/resource_views.py | 10 +- magpie/api/schemas.py | 2 +- tests/interfaces.py | 144 ++++++++++++++++-- 3 files changed, 137 insertions(+), 19 deletions(-) diff --git a/magpie/api/management/resource/resource_views.py b/magpie/api/management/resource/resource_views.py index 5ad8f91f5..9ad1a3053 100644 --- a/magpie/api/management/resource/resource_views.py +++ b/magpie/api/management/resource/resource_views.py @@ -181,11 +181,13 @@ def update_permissions(request): # Check if user/group exists for each permission found if "permission" in entry.keys() and entry["permission"]: if "user" in entry.keys(): - if not UserService.by_user_name(entry["user"], db_session=request.db): - raise RuntimeError(f"User {entry['user']} not found in the database.") + ax.verify_param(UserService.by_user_name(entry["user"], db_session=request.db), + not_none=True, http_error=HTTPBadRequest, + msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) if "group" in entry.keys(): - if not GroupService.by_group_name(entry["group"], db_session=request.db): - raise RuntimeError(f"Group {entry['group']} not found in the database.") + ax.verify_param(GroupService.by_group_name(entry["group"], db_session=request.db), + not_none=True, http_error=HTTPBadRequest, + msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) # Reformat permissions config permissions_cfg = {"permissions": []} diff --git a/magpie/api/schemas.py b/magpie/api/schemas.py index baad76d4d..39503e4cb 100644 --- a/magpie/api/schemas.py +++ b/magpie/api/schemas.py @@ -828,7 +828,7 @@ class Permissions_PATCH_OkResponseSchema(BaseResponseSchemaAPI): class Permissions_PATCH_BadRequestResponseSchema(BaseResponseSchemaAPI): - description = "Missing parameters to update permissions or parameters do not provide any modification." + description = "Missing or invalid parameters for permissions update." body = ErrorResponseBodySchema(code=HTTPBadRequest.code, description=description) diff --git a/tests/interfaces.py b/tests/interfaces.py index 52001329a..8e1cfe584 100644 --- a/tests/interfaces.py +++ b/tests/interfaces.py @@ -2277,7 +2277,8 @@ def test_PatchPermissions(self): utils.TestSetup.create_TestUser(self) utils.TestSetup.create_TestService(self, override_service_type="thredds") - resource_names = ["folder1", "folder2", "the-file"] + resource_names = ["folder1", "folder2", "folder3", "the-file"] + data = { "permissions": [ { @@ -2293,17 +2294,27 @@ def test_PatchPermissions(self): "resource_name": resource_names[1], "resource_type": "directory", "permission": "browse", - "group": self.test_group_name + "group": self.test_group_name, + "action": "create" }, { "resource_name": resource_names[2], + "resource_type": "directory", + "permission": "browse", + "group": self.test_group_name, + "user": self.test_user_name, + "action": "create" + }, + { + "resource_name": resource_names[3], "resource_type": "file", "permission": { "name": "read", "access": "deny", "scope": "match" }, - "user": "blarg"#self.test_user_name + "user": self.test_user_name, + "action": "create" } ] } @@ -2313,30 +2324,135 @@ def test_PatchPermissions(self): path = f"/services/{self.test_service_name}/resources" resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) - body = utils.check_response_basic_info(resp) - resources = body[self.test_service_name]["resources"] + resources = utils.check_response_basic_info(resp)[self.test_service_name]["resources"] for res_name in resource_names: - target_res = None - for res_id, res in resources.items(): - if res["resource_name"] == res_name: - target_res = res - break - # Check if resource was found - assert target_res + target_res = [res for res in resources.values() if res["resource_name"] == res_name] + # target resource should exist + assert len(target_res) == 1 + target_res = target_res[0] # if res with permission, get permissions and check if res_name == resource_names[1]: path = f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions" resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) body = utils.check_response_basic_info(resp) - # TODO: check if the right permission is found + utils.check_val_is_in("browse", body["permission_names"]) elif res_name == resource_names[2]: + for path in [f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions", + f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions"]: + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + body = utils.check_response_basic_info(resp) + utils.check_val_is_in("browse", body["permission_names"]) + elif res_name == resource_names[3]: path = f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions" resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) body = utils.check_response_basic_info(resp) - # TODO: check if the right permission is found + utils.check_val_is_in("read-deny-match", body["permission_names"]) resources = target_res["children"] + # Delete permissions + for i in range(len(data["permissions"])): + data["permissions"][i]["action"] = "remove" + path = "/permissions" + resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, cookies=self.cookies, json=data) + utils.check_response_basic_info(resp, 200, expected_method="PATCH") + + path = f"/services/{self.test_service_name}/resources" + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + resources = utils.check_response_basic_info(resp)[self.test_service_name]["resources"] + for res_name in resource_names: + target_res = [res for res in resources.values() if res["resource_name"] == res_name] + # target resource should exist and should be unique + assert len(target_res) == 1 + target_res = target_res[0] + + # if res with permission, get permissions and check if permission was deleted + if res_name == resource_names[1]: + path = f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions" + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + body = utils.check_response_basic_info(resp) + utils.check_val_not_in("browse", body["permission_names"]) + elif res_name == resource_names[2]: + for path in [f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions", + f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions"]: + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + body = utils.check_response_basic_info(resp) + utils.check_val_not_in("browse", body["permission_names"]) + elif res_name == resource_names[3]: + path = f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions" + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + body = utils.check_response_basic_info(resp) + utils.check_val_not_in("read-deny-match", body["permission_names"]) + resources = target_res["children"] + + @runner.MAGPIE_TEST_PERMISSIONS + def test_PatchPermissions_InvalidUserGroup(self): + # Test with invalid group + data = { + "permissions": [ + { + "resource_name": self.test_service_name, + "resource_type": "service", + "permission": "browse", + "group": "invalid_group", + } + ] + } + path = "/permissions" + resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, expect_errors=True, + cookies=self.cookies, json=data) + utils.check_response_basic_info(resp, 400, expected_method="PATCH") + + # Test with invalid user + data = { + "permissions": [ + { + "resource_name": self.test_service_name, + "resource_type": "service", + "permission": "browse", + "user": "invalid_user", + } + ] + } + path = "/permissions" + resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, expect_errors=True, + cookies=self.cookies, json=data) + utils.check_response_basic_info(resp, 400, expected_method="PATCH") + + @runner.MAGPIE_TEST_PERMISSIONS + def test_PatchPermissions_InvalidResourceType(self): + # Test with invalid service + data = { + "permissions": [ + { + "resource_name": self.test_service_name, + "resource_type": "directory" # 1st resource should always have the `service` type + } + ] + } + path = "/permissions" + resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, expect_errors=True, + cookies=self.cookies, json=data) + utils.check_response_basic_info(resp, 400, expected_method="PATCH") + + # Test with invalid resource + data = { + "permissions": [ + { + "resource_name": self.test_service_name, + "resource_type": "service", + "permission": None + }, + { + "resource_name": "resource", + "resource_type": "service" # only the 1st resource should have the `service` type + } + ] + } + path = "/permissions" + resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, expect_errors=True, + cookies=self.cookies, json=data) + utils.check_response_basic_info(resp, 400, expected_method="PATCH") def create_validate_permissions(self, test_user_name, # type: Str From 8fd1fdf5591b44e13d42592dbf66c17faedcccfe Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 16 Mar 2022 10:58:32 -0400 Subject: [PATCH 05/19] add case in valid patch permission test --- tests/interfaces.py | 71 ++++++++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 24 deletions(-) diff --git a/tests/interfaces.py b/tests/interfaces.py index 8e1cfe584..cc8f1f5eb 100644 --- a/tests/interfaces.py +++ b/tests/interfaces.py @@ -2296,30 +2296,52 @@ def test_PatchPermissions(self): "permission": "browse", "group": self.test_group_name, "action": "create" - }, - { - "resource_name": resource_names[2], - "resource_type": "directory", - "permission": "browse", - "group": self.test_group_name, - "user": self.test_user_name, - "action": "create" - }, - { - "resource_name": resource_names[3], - "resource_type": "file", - "permission": { - "name": "read", - "access": "deny", - "scope": "match" - }, - "user": self.test_user_name, - "action": "create" } ] } - path = "/permissions" - resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, cookies=self.cookies, json=data) + resp = utils.test_request(self, "PATCH", "/permissions", headers=self.json_headers, + cookies=self.cookies, json=data) + utils.check_response_basic_info(resp, 200, expected_method="PATCH") + + path = f"/services/{self.test_service_name}/resources" + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + resources = utils.check_response_basic_info(resp)[self.test_service_name]["resources"] + for res_name in resource_names[0:2]: + target_res = [res for res in resources.values() if res["resource_name"] == res_name] + # target resource should exist + assert len(target_res) == 1 + target_res = target_res[0] + + # if res with permission, get permissions and check if permission was created + if res_name == resource_names[1]: + path = f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions" + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + body = utils.check_response_basic_info(resp) + utils.check_val_is_in("browse", body["permission_names"]) + resources = target_res["children"] + + # Test second patch request, with additional resources/permissions + data["permissions"].append({ + "resource_name": resource_names[2], + "resource_type": "directory", + "permission": "browse", + "group": self.test_group_name, + "user": self.test_user_name, + "action": "create" + }) + data["permissions"].append({ + "resource_name": resource_names[3], + "resource_type": "file", + "permission": { + "name": "read", + "access": "deny", + "scope": "match" + }, + "user": self.test_user_name, + "action": "create" + }) + resp = utils.test_request(self, "PATCH", "/permissions", headers=self.json_headers, + cookies=self.cookies, json=data) utils.check_response_basic_info(resp, 200, expected_method="PATCH") path = f"/services/{self.test_service_name}/resources" @@ -2331,8 +2353,9 @@ def test_PatchPermissions(self): assert len(target_res) == 1 target_res = target_res[0] - # if res with permission, get permissions and check + # if res with permission, get permissions and check if permission was created if res_name == resource_names[1]: + # This permission was already created in the preceding request, but should still exist. path = f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions" resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) body = utils.check_response_basic_info(resp) @@ -2353,8 +2376,8 @@ def test_PatchPermissions(self): # Delete permissions for i in range(len(data["permissions"])): data["permissions"][i]["action"] = "remove" - path = "/permissions" - resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, cookies=self.cookies, json=data) + resp = utils.test_request(self, "PATCH", "/permissions", headers=self.json_headers, + cookies=self.cookies, json=data) utils.check_response_basic_info(resp, 200, expected_method="PATCH") path = f"/services/{self.test_service_name}/resources" From 3b5e242f75ccc9246a6ab2f4202da410046eab98 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 16 Mar 2022 11:27:50 -0400 Subject: [PATCH 06/19] refactor test case --- tests/interfaces.py | 108 ++++++++++++++------------------------------ 1 file changed, 35 insertions(+), 73 deletions(-) diff --git a/tests/interfaces.py b/tests/interfaces.py index cc8f1f5eb..5c6ad029b 100644 --- a/tests/interfaces.py +++ b/tests/interfaces.py @@ -2303,22 +2303,39 @@ def test_PatchPermissions(self): cookies=self.cookies, json=data) utils.check_response_basic_info(resp, 200, expected_method="PATCH") - path = f"/services/{self.test_service_name}/resources" - resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) - resources = utils.check_response_basic_info(resp)[self.test_service_name]["resources"] - for res_name in resource_names[0:2]: - target_res = [res for res in resources.values() if res["resource_name"] == res_name] - # target resource should exist - assert len(target_res) == 1 - target_res = target_res[0] - - # if res with permission, get permissions and check if permission was created - if res_name == resource_names[1]: - path = f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions" - resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) - body = utils.check_response_basic_info(resp) - utils.check_val_is_in("browse", body["permission_names"]) - resources = target_res["children"] + def check_permissions(data): + """ + Utility function that checks if resource and permissions found in data were succesfully create/removed. + """ + path = f"/services/{self.test_service_name}/resources" + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + resources = utils.check_response_basic_info(resp)[self.test_service_name]["resources"] + + for perm_entry in data["permissions"][1:]: # skip first resource, which is the service + target_res = [res for res in resources.values() if res["resource_name"] == perm_entry["resource_name"]] + # target resource should exist + assert len(target_res) == 1 + target_res = target_res[0] + + if perm_entry.get("permission"): + perm = perm_entry["permission"] + if type(perm) is dict: + perm = f"{perm['name']}-{perm['access']}-{perm['scope']}" + check_paths = [] + if perm_entry.get("user"): + check_paths.append(f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions") + if perm_entry.get("group"): + check_paths.append(f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions") + for path in check_paths: + resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) + body = utils.check_response_basic_info(resp) + if perm_entry.get("action") and perm_entry["action"] == "remove": + utils.check_val_not_in(perm, body["permission_names"]) + else: + utils.check_val_is_in(perm, body["permission_names"]) + resources = target_res["children"] + + check_permissions(data) # Test second patch request, with additional resources/permissions data["permissions"].append({ @@ -2343,35 +2360,7 @@ def test_PatchPermissions(self): resp = utils.test_request(self, "PATCH", "/permissions", headers=self.json_headers, cookies=self.cookies, json=data) utils.check_response_basic_info(resp, 200, expected_method="PATCH") - - path = f"/services/{self.test_service_name}/resources" - resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) - resources = utils.check_response_basic_info(resp)[self.test_service_name]["resources"] - for res_name in resource_names: - target_res = [res for res in resources.values() if res["resource_name"] == res_name] - # target resource should exist - assert len(target_res) == 1 - target_res = target_res[0] - - # if res with permission, get permissions and check if permission was created - if res_name == resource_names[1]: - # This permission was already created in the preceding request, but should still exist. - path = f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions" - resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) - body = utils.check_response_basic_info(resp) - utils.check_val_is_in("browse", body["permission_names"]) - elif res_name == resource_names[2]: - for path in [f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions", - f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions"]: - resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) - body = utils.check_response_basic_info(resp) - utils.check_val_is_in("browse", body["permission_names"]) - elif res_name == resource_names[3]: - path = f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions" - resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) - body = utils.check_response_basic_info(resp) - utils.check_val_is_in("read-deny-match", body["permission_names"]) - resources = target_res["children"] + check_permissions(data) # Delete permissions for i in range(len(data["permissions"])): @@ -2379,34 +2368,7 @@ def test_PatchPermissions(self): resp = utils.test_request(self, "PATCH", "/permissions", headers=self.json_headers, cookies=self.cookies, json=data) utils.check_response_basic_info(resp, 200, expected_method="PATCH") - - path = f"/services/{self.test_service_name}/resources" - resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) - resources = utils.check_response_basic_info(resp)[self.test_service_name]["resources"] - for res_name in resource_names: - target_res = [res for res in resources.values() if res["resource_name"] == res_name] - # target resource should exist and should be unique - assert len(target_res) == 1 - target_res = target_res[0] - - # if res with permission, get permissions and check if permission was deleted - if res_name == resource_names[1]: - path = f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions" - resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) - body = utils.check_response_basic_info(resp) - utils.check_val_not_in("browse", body["permission_names"]) - elif res_name == resource_names[2]: - for path in [f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions", - f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions"]: - resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) - body = utils.check_response_basic_info(resp) - utils.check_val_not_in("browse", body["permission_names"]) - elif res_name == resource_names[3]: - path = f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions" - resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) - body = utils.check_response_basic_info(resp) - utils.check_val_not_in("read-deny-match", body["permission_names"]) - resources = target_res["children"] + check_permissions(data) @runner.MAGPIE_TEST_PERMISSIONS def test_PatchPermissions_InvalidUserGroup(self): From 578cb5e5e10a83a2e38be5ad0c262a5785e107da Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 16 Mar 2022 15:30:52 -0400 Subject: [PATCH 07/19] add raise error in log_permission --- .../api/management/resource/resource_views.py | 3 +- magpie/register.py | 81 +++++++++++-------- 2 files changed, 50 insertions(+), 34 deletions(-) diff --git a/magpie/api/management/resource/resource_views.py b/magpie/api/management/resource/resource_views.py index 9ad1a3053..e1ecff773 100644 --- a/magpie/api/management/resource/resource_views.py +++ b/magpie/api/management/resource/resource_views.py @@ -235,6 +235,7 @@ def update_permissions(request): permissions_cfg["permissions"].append(cfg_entry) # Apply permission update - magpie_register_permissions_from_config(permissions_config=permissions_cfg, db_session=request.db) + magpie_register_permissions_from_config(permissions_config=permissions_cfg, db_session=request.db, + raise_errors=True) return ax.valid_http(http_success=HTTPOk, detail=s.Permissions_PATCH_OkResponseSchema.description) diff --git a/magpie/register.py b/magpie/register.py index dc7202f10..0943f88bd 100644 --- a/magpie/register.py +++ b/magpie/register.py @@ -11,7 +11,7 @@ import six import transaction import yaml -from pyramid.httpexceptions import HTTPException +from pyramid.httpexceptions import HTTPBadRequest, HTTPException from sqlalchemy.orm.session import Session from ziggurat_foundations.models.services.group import GroupService from ziggurat_foundations.models.services.resource import ResourceService @@ -673,7 +673,8 @@ def magpie_register_services_from_config(service_config_path, push_to_phoenix=Fa return merged_service_configs -def _log_permission(message, permission_index, trail=", skipping...", detail=None, permission=None, level=logging.WARN): +def _log_permission(message, permission_index, trail=", skipping...", detail=None, permission=None, level=logging.WARN, + raise_errors=False): # type: (Str, int, Str, Optional[Str], Optional[Str], Union[Str, int]) -> None """ Logs a message related to a 'permission' entry. @@ -695,6 +696,7 @@ def _log_permission(message, permission_index, trail=", skipping...", detail=Non :param detail: additional details appended after the trailing message after moving to another line. :param permission: permission name to log just before the trailing message. :param level: logging level (default: ``logging.WARN``) + :param raise_errors: raises errors related to permissions, instead of just logging the info. .. seealso:: `magpie/config/permissions.cfg` @@ -703,6 +705,9 @@ def _log_permission(message, permission_index, trail=", skipping...", detail=Non permission = " [{!s}]".format(permission) if permission else "" LOGGER.log(level, "%s [permission #%d]%s%s", message, permission_index, permission, trail) + if raise_errors: + raise HTTPBadRequest(f"{message} [permission #{permission_index}]{permission}{trail}") + def _use_request(cookies_or_session): return not isinstance(cookies_or_session, Session) @@ -712,7 +717,8 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem entry_index, # type: int service_info, # type: JSON cookies_or_session=None, # type: CookiesOrSessionType - magpie_url=None, # type: Optional[Str] + magpie_url=None, # type: Optional[Str], + raise_errors=False # type: bool ): # type: (...) -> Tuple[Optional[int], bool] """ Parses the `resource` field of a permission config entry and retrieves the final resource id. Creates missing @@ -774,19 +780,19 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem svc_res_types = SERVICE_TYPE_DICT[svc_type].resource_type_names type_count = len(svc_res_types) if type_count == 0: - _log_permission("Cannot generate resource", entry_index, + _log_permission("Cannot generate resource", entry_index, raise_errors=raise_errors, detail="Service [{!s}] of type [{!s}] doesn't allows any sub-resource types. " .format(svc_name, svc_type)) raise RegistrationConfigurationError("Invalid resource not allowed to apply permission.") if type_count != 1 and not (isinstance(resource_type, six.string_types) and resource_type): - _log_permission("Cannot automatically generate resource", entry_index, + _log_permission("Cannot automatically generate resource", entry_index, raise_errors=raise_errors, detail="Service [{!s}] of type [{!s}] allows more than 1 sub-resource types ({}). " "Type must be explicitly specified for auto-creation. " "Available choices are: {}" .format(svc_name, svc_type, type_count, svc_res_types)) raise RegistrationConfigurationError("Missing resource 'type' to apply permission.") if type_count != 1 and resource_type not in svc_res_types: - _log_permission("Cannot generate resource", entry_index, + _log_permission("Cannot generate resource", entry_index, raise_errors=raise_errors, detail="Service [{!s}] of type [{!s}] allows more than 1 sub-resource types ({}). " "Specified type [{!s}] doesn't match any of the allowed resource types. " "Available choices are: {}" @@ -808,10 +814,10 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem raise RegistrationConfigurationError("Could not extract child resource from resource path.") except HTTPException as exc: detail = "{} ({}), {!s}".format(type(exc).__name__, exc.code, exc) - _log_permission("Failed resources parsing.", entry_index, detail=detail) + _log_permission("Failed resources parsing.", entry_index, detail=detail, raise_errors=raise_errors) return None, False except Exception as exc: - _log_permission("Failed resources parsing.", entry_index, detail=repr(exc)) + _log_permission("Failed resources parsing.", entry_index, detail=repr(exc), raise_errors=raise_errors) return None, False return resource, True @@ -823,6 +829,7 @@ def _apply_permission_entry(permission_config_entry, # type: PermissionConfig magpie_url, # type: Str users, # type: UsersSettings groups, # type: GroupsSettings + raise_errors=False, # type: bool ): # type: (...) -> None """ Applies the single permission entry retrieved from the permission configuration. @@ -909,7 +916,7 @@ def _apply_profile(_usr_name=None, _grp_name=None): from magpie.api.management.group.group_utils import create_group return create_group(**grp_data) - def _validate_response(operation, is_create, item_type="Permission"): + def _validate_response(operation, is_create, item_type="Permission", raise_errors=False): """ Validate action/operation applied and handles raised ``HTTPException`` as returned response. """ @@ -927,20 +934,24 @@ def _validate_response(operation, is_create, item_type="Permission"): # validation according to status code returned if is_create: if _resp.status_code in [200, 201]: # update/create - _log_permission("{} successfully created.".format(item_type), entry_index, level=logging.INFO, trail="") + _log_permission("{} successfully created.".format(item_type), entry_index, level=logging.INFO, trail="", + raise_errors=False) elif _resp.status_code == 409: - _log_permission("{} already exists.".format(item_type), entry_index, level=logging.INFO) + _log_permission("{} already exists.".format(item_type), entry_index, level=logging.INFO, + raise_errors=False) else: _log_permission("Unknown response [{}]".format(_resp.status_code), entry_index, - permission=permission_config_entry, level=logging.ERROR) + permission=permission_config_entry, level=logging.ERROR, raise_errors=raise_errors) else: if _resp.status_code == 200: - _log_permission("{} successfully removed.".format(item_type), entry_index, level=logging.INFO, trail="") + _log_permission("{} successfully removed.".format(item_type), entry_index, level=logging.INFO, trail="", + raise_errors=False) elif _resp.status_code == 404: - _log_permission("{} already removed.".format(item_type), entry_index, level=logging.INFO) + _log_permission("{} already removed.".format(item_type), entry_index, level=logging.INFO, + raise_errors=False) else: _log_permission("Unknown response [{}]".format(_resp.status_code), entry_index, - permission=permission_config_entry, level=logging.ERROR) + permission=permission_config_entry, level=logging.ERROR, raise_errors=raise_errors) create_perm = permission_config_entry["action"] == "create" perm_def = permission_config_entry["permission"] # name or object @@ -949,17 +960,17 @@ def _validate_response(operation, is_create, item_type="Permission"): perm = PermissionSet(perm_def) # process groups first as they can be referenced by user definitions - _validate_response(lambda: _apply_profile(None, grp_name), is_create=True) - _validate_response(lambda: _apply_profile(usr_name, None), is_create=True) + _validate_response(lambda: _apply_profile(None, grp_name), is_create=True, raise_errors=raise_errors) + _validate_response(lambda: _apply_profile(usr_name, None), is_create=True, raise_errors=raise_errors) if _use_request(cookies_or_session): - _validate_response(lambda: _apply_request(None, grp_name), is_create=create_perm) - _validate_response(lambda: _apply_request(usr_name, None), is_create=create_perm) + _validate_response(lambda: _apply_request(None, grp_name), is_create=create_perm, raise_errors=raise_errors) + _validate_response(lambda: _apply_request(usr_name, None), is_create=create_perm, raise_errors=raise_errors) else: - _validate_response(lambda: _apply_session(None, grp_name), is_create=create_perm) - _validate_response(lambda: _apply_session(usr_name, None), is_create=create_perm) + _validate_response(lambda: _apply_session(None, grp_name), is_create=create_perm, raise_errors=raise_errors) + _validate_response(lambda: _apply_session(usr_name, None), is_create=create_perm, raise_errors=raise_errors) -def magpie_register_permissions_from_config(permissions_config, magpie_url=None, db_session=None): +def magpie_register_permissions_from_config(permissions_config, magpie_url=None, db_session=None, raise_errors=False): # type: (Union[Str, PermissionsConfig], Optional[Str], Optional[Session]) -> None """ Applies `permissions` specified in configuration(s) defined as file, directory with files or literal configuration. @@ -967,6 +978,7 @@ def magpie_register_permissions_from_config(permissions_config, magpie_url=None, :param permissions_config: file/dir path to `permissions` config or JSON/YAML equivalent pre-loaded. :param magpie_url: URL to magpie instance (when using requests; default: `magpie.url` from this app's config). :param db_session: db session to use instead of requests to directly create/remove permissions with config. + :param raise_errors: raises errors related to permissions, instead of just logging the info. .. seealso:: `magpie/config/permissions.cfg` for specific parameters and operational details. @@ -996,7 +1008,7 @@ def magpie_register_permissions_from_config(permissions_config, magpie_url=None, groups_settings = _resolve_config_registry(groups, "name") or {} for i, perms in enumerate(permissions): LOGGER.info("Processing permissions from configuration (%s/%s).", i + 1, perms_cfg_count) - _process_permissions(perms, magpie_url, cookies_or_session, users_settings, groups_settings) + _process_permissions(perms, magpie_url, cookies_or_session, users_settings, groups_settings, raise_errors) LOGGER.info("All permissions processed.") @@ -1025,7 +1037,7 @@ def _resolve_config_registry(config_files, key): return config_map -def _process_permissions(permissions, magpie_url, cookies_or_session, users=None, groups=None): +def _process_permissions(permissions, magpie_url, cookies_or_session, users=None, groups=None, raise_errors=False): # type: (PermissionsConfig, Str, Session, Optional[UsersSettings], Optional[GroupsSettings]) -> None """ Processes a single `permissions` configuration. @@ -1040,25 +1052,25 @@ def _process_permissions(permissions, magpie_url, cookies_or_session, users=None for i, perm_cfg in enumerate(permissions): # parameter validation if not isinstance(perm_cfg, dict) or not all(f in perm_cfg for f in ["permission", "service"]): - _log_permission("Invalid permission format for [{!s}]".format(perm_cfg), i) + _log_permission("Invalid permission format for [{!s}]".format(perm_cfg), i, raise_errors=raise_errors) continue try: perm = PermissionSet(perm_cfg["permission"]) except (ValueError, TypeError): perm = None if not perm: - _log_permission("Unknown permission [{!s}]".format(perm_cfg["permission"]), i) + _log_permission("Unknown permission [{!s}]".format(perm_cfg["permission"]), i, raise_errors=raise_errors) continue usr_name = perm_cfg.get("user") grp_name = perm_cfg.get("group") if not any([usr_name, grp_name]): - _log_permission("Missing required user and/or group field.", i) + _log_permission("Missing required user and/or group field.", i, raise_errors=raise_errors) continue if "action" not in perm_cfg: - _log_permission("Unspecified action", i, trail="using default (create)...") + _log_permission("Unspecified action", i, trail="using default (create)...", raise_errors=raise_errors) perm_cfg["action"] = "create" if perm_cfg["action"] not in ["create", "remove"]: - _log_permission("Unknown action [{!s}]".format(perm_cfg["action"]), i) + _log_permission("Unknown action [{!s}]".format(perm_cfg["action"]), i, raise_errors=raise_errors) continue # retrieve service for permissions validation @@ -1067,24 +1079,27 @@ def _process_permissions(permissions, magpie_url, cookies_or_session, users=None svc_path = magpie_url + ServiceAPI.path.format(service_name=svc_name) svc_resp = requests.get(svc_path, cookies=cookies_or_session) if svc_resp.status_code != 200: - _log_permission("Unknown service [{!s}]".format(svc_name), i) + _log_permission("Unknown service [{!s}]".format(svc_name), i, raise_errors=raise_errors) continue service_info = get_json(svc_resp)[svc_name] else: transaction.commit() # force any pending transaction to be applied to find possible dependencies svc = models.Service.by_service_name(svc_name, db_session=cookies_or_session) if not svc: - _log_permission("Unknown service [{!s}]. Can't edit permissions without service.".format(svc_name), i) + _log_permission("Unknown service [{!s}]. Can't edit permissions without service.".format(svc_name), i, + raise_errors=raise_errors) continue from magpie.api.management.service.service_formats import format_service service_info = format_service(svc) # apply permission config - resource_id, found = _parse_resource_path(perm_cfg, i, service_info, cookies_or_session, magpie_url) + resource_id, found = _parse_resource_path(perm_cfg, i, service_info, cookies_or_session, magpie_url, + raise_errors) if found: if not resource_id: resource_id = service_info["resource_id"] - _apply_permission_entry(perm_cfg, i, resource_id, cookies_or_session, magpie_url, users, groups) + _apply_permission_entry(perm_cfg, i, resource_id, cookies_or_session, magpie_url, users, groups, + raise_errors) if not _use_request(cookies_or_session): transaction.commit() From e682d874337ce5a0e9900ee75960fb7293d4bfe4 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 16 Mar 2022 15:45:16 -0400 Subject: [PATCH 08/19] update changelog --- CHANGES.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 1aab5c790..5b3d07508 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -9,6 +9,12 @@ Changes `Unreleased `_ (latest) ------------------------------------------------------------------------------------ +Features / Changes +~~~~~~~~~~~~~~~~~~~~~ +* Add ``PATCH /permissions`` endpoint that updates permissions and creates related resources if necessary. +* ``permissions.cfg`` now supports a new format for the ``type`` parameter, using multiple types separated + by a slash character, matching each type with each resource found in the ``resource`` parameter. + Bug Fixes ~~~~~~~~~~~~~~~~~~~~~ * Update linting configuration rules to validate all migration scripts employed by ``alembic``. From 1d1c530719db6cf7853c420e8d0962c5a3283585 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 16 Mar 2022 15:54:04 -0400 Subject: [PATCH 09/19] fix lint --- magpie/api/management/resource/resource_views.py | 16 ++++++++-------- tests/interfaces.py | 8 +++++--- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/magpie/api/management/resource/resource_views.py b/magpie/api/management/resource/resource_views.py index e1ecff773..1f3c381a6 100644 --- a/magpie/api/management/resource/resource_views.py +++ b/magpie/api/management/resource/resource_views.py @@ -1,4 +1,4 @@ -from pyramid.httpexceptions import HTTPBadRequest, HTTPConflict, HTTPForbidden, HTTPInternalServerError, HTTPNotFound, HTTPOk +from pyramid.httpexceptions import HTTPBadRequest, HTTPConflict, HTTPForbidden, HTTPInternalServerError, HTTPOk from pyramid.settings import asbool from pyramid.view import view_config from ziggurat_foundations.models.services.group import GroupService @@ -13,7 +13,7 @@ from magpie.api.management.service.service_formats import format_service_resources from magpie.api.management.service.service_utils import get_services_by_type from magpie.permissions import PermissionType, format_permissions -from magpie.register import sync_services_phoenix, magpie_register_permissions_from_config +from magpie.register import magpie_register_permissions_from_config, sync_services_phoenix from magpie.services import SERVICE_TYPE_DICT, get_resource_child_allowed @@ -207,19 +207,19 @@ def update_permissions(request): http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) if i == 0: # First resource must be a service - ax.verify_param(resource_type, is_equal=True, param_compare="service", - http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + ax.verify_param(resource_type, is_equal=True, param_compare="service", http_error=HTTPBadRequest, + msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) service_name = resource_name else: resource_full_path += "/" + resource_name # Other resources must not be services - ax.verify_param(resource_type, not_equal=True, param_compare="service", - http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + ax.verify_param(resource_type, not_equal=True, param_compare="service", http_error=HTTPBadRequest, + msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) resource_full_type += "/" + resource_type if permission: # Check that a user and/or a group is defined - ax.verify_param(bool(user or group), is_true=True, - http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + ax.verify_param(bool(user or group), is_true=True, http_error=HTTPBadRequest, + msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) cfg_entry = { "service": service_name, "resource": resource_full_path, diff --git a/tests/interfaces.py b/tests/interfaces.py index 7cc586b6e..7916f10b4 100644 --- a/tests/interfaces.py +++ b/tests/interfaces.py @@ -2432,13 +2432,15 @@ def check_permissions(data): if perm_entry.get("permission"): perm = perm_entry["permission"] - if type(perm) is dict: + if isinstance(perm, dict): perm = f"{perm['name']}-{perm['access']}-{perm['scope']}" check_paths = [] if perm_entry.get("user"): - check_paths.append(f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions") + check_paths.append( + f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions") if perm_entry.get("group"): - check_paths.append(f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions") + check_paths.append( + f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions") for path in check_paths: resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) body = utils.check_response_basic_info(resp) From a3a66b765ba6947ba0ecd834b7a0d83b4838d585 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 16 Mar 2022 17:01:10 -0400 Subject: [PATCH 10/19] fix test_register errors --- magpie/register.py | 2 +- tests/test_register.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/magpie/register.py b/magpie/register.py index 857dba217..c0156d3da 100644 --- a/magpie/register.py +++ b/magpie/register.py @@ -751,7 +751,7 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem resource_list = resource_path.split("/") if len(resource_type_list) == 1: # if only one type specified, assume every path of the resource uses the same resource type - resource_type_list = resource_type_list[0] * len(resource_list) + resource_type_list = resource_type_list * len(resource_list) if len(resource_list) != len(resource_type_list): raise RegistrationConfigurationError("Invalid resource type found in configuration : " f"{permission_config_entry.get('type')}") diff --git a/tests/test_register.py b/tests/test_register.py index ee28fea61..3f25c8df1 100644 --- a/tests/test_register.py +++ b/tests/test_register.py @@ -531,12 +531,12 @@ def test_register_process_permissions_from_multiple_files(): expect_users = {"usr1": cfg1["users"][0], "usr2": cfg1["users"][1], "usr3": cfg2["users"][0]} expect_groups = {"grp1": cfg1["groups"][0], "grp2": cfg1["groups"][1]} - perms, _, _, users, groups = mock_process_perms.call_args_list[0].args + perms, _, _, users, groups, _ = mock_process_perms.call_args_list[0].args assert perms == cfg1["permissions"] assert users == expect_users assert groups == expect_groups - perms, _, _, users, groups = mock_process_perms.call_args_list[1].args + perms, _, _, users, groups, _ = mock_process_perms.call_args_list[1].args assert perms == cfg2["permissions"] assert users == expect_users assert groups == expect_groups From b0523928499d44b2f5bc6c927b6e905f61c75226 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 16 Mar 2022 17:15:38 -0400 Subject: [PATCH 11/19] fix py3.5 string format --- magpie/register.py | 6 +++--- tests/interfaces.py | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/magpie/register.py b/magpie/register.py index c0156d3da..78a9f9aa5 100644 --- a/magpie/register.py +++ b/magpie/register.py @@ -706,7 +706,7 @@ def _log_permission(message, permission_index, trail=", skipping...", detail=Non LOGGER.log(level, "%s [permission #%d]%s%s", message, permission_index, permission, trail) if raise_errors: - raise HTTPBadRequest(f"{message} [permission #{permission_index}]{permission}{trail}") + raise HTTPBadRequest("{} [permission #{}]{}{}".format(message, permission_index, permission, trail)) def _use_request(cookies_or_session): @@ -753,8 +753,8 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem # if only one type specified, assume every path of the resource uses the same resource type resource_type_list = resource_type_list * len(resource_list) if len(resource_list) != len(resource_type_list): - raise RegistrationConfigurationError("Invalid resource type found in configuration : " - f"{permission_config_entry.get('type')}") + raise RegistrationConfigurationError("Invalid resource type found in configuration : " + + permission_config_entry.get('type')) res_path = None if _use_request(cookies_or_session): diff --git a/tests/interfaces.py b/tests/interfaces.py index 7916f10b4..afcc67b54 100644 --- a/tests/interfaces.py +++ b/tests/interfaces.py @@ -2420,7 +2420,7 @@ def check_permissions(data): """ Utility function that checks if resource and permissions found in data were succesfully create/removed. """ - path = f"/services/{self.test_service_name}/resources" + path = "/services/{}/resources".format(self.test_service_name) resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) resources = utils.check_response_basic_info(resp)[self.test_service_name]["resources"] @@ -2433,14 +2433,14 @@ def check_permissions(data): if perm_entry.get("permission"): perm = perm_entry["permission"] if isinstance(perm, dict): - perm = f"{perm['name']}-{perm['access']}-{perm['scope']}" + perm = "{}-{}-{}".format(perm['name'], perm['access'], perm['scope']) check_paths = [] if perm_entry.get("user"): check_paths.append( - f"/users/{self.test_user_name}/resources/{target_res['resource_id']}/permissions") + "/users/{}/resources/{}/permissions".format(self.test_user_name, target_res['resource_id'])) if perm_entry.get("group"): - check_paths.append( - f"/groups/{self.test_group_name}/resources/{target_res['resource_id']}/permissions") + check_paths.append("/groups/{}/resources/{}/permissions".format( + self.test_group_name, target_res['resource_id'])) for path in check_paths: resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) body = utils.check_response_basic_info(resp) From 48c2e6561afe55617c43a88561ba54199bc662da Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 16 Mar 2022 17:23:31 -0400 Subject: [PATCH 12/19] fix lint --- magpie/register.py | 2 +- tests/interfaces.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/magpie/register.py b/magpie/register.py index 78a9f9aa5..8b8059e93 100644 --- a/magpie/register.py +++ b/magpie/register.py @@ -754,7 +754,7 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem resource_type_list = resource_type_list * len(resource_list) if len(resource_list) != len(resource_type_list): raise RegistrationConfigurationError("Invalid resource type found in configuration : " + - permission_config_entry.get('type')) + permission_config_entry.get("type")) res_path = None if _use_request(cookies_or_session): diff --git a/tests/interfaces.py b/tests/interfaces.py index afcc67b54..f9694862b 100644 --- a/tests/interfaces.py +++ b/tests/interfaces.py @@ -2433,14 +2433,14 @@ def check_permissions(data): if perm_entry.get("permission"): perm = perm_entry["permission"] if isinstance(perm, dict): - perm = "{}-{}-{}".format(perm['name'], perm['access'], perm['scope']) + perm = "{}-{}-{}".format(perm["name"], perm["access"], perm["scope"]) check_paths = [] if perm_entry.get("user"): check_paths.append( - "/users/{}/resources/{}/permissions".format(self.test_user_name, target_res['resource_id'])) + "/users/{}/resources/{}/permissions".format(self.test_user_name, target_res["resource_id"])) if perm_entry.get("group"): check_paths.append("/groups/{}/resources/{}/permissions".format( - self.test_group_name, target_res['resource_id'])) + self.test_group_name, target_res["resource_id"])) for path in check_paths: resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) body = utils.check_response_basic_info(resp) From dd367393d8dfad404ca836d626bdfd7c969ce5aa Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Mon, 21 Mar 2022 13:05:50 -0400 Subject: [PATCH 13/19] update error messages in update_permissions method with more details --- .../api/management/resource/resource_views.py | 54 +++++++++++-------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/magpie/api/management/resource/resource_views.py b/magpie/api/management/resource/resource_views.py index 1f3c381a6..ffe6f104c 100644 --- a/magpie/api/management/resource/resource_views.py +++ b/magpie/api/management/resource/resource_views.py @@ -12,6 +12,7 @@ from magpie.api.management.resource import resource_utils as ru from magpie.api.management.service.service_formats import format_service_resources from magpie.api.management.service.service_utils import get_services_by_type +from magpie.api.management.user import user_utils as uu from magpie.permissions import PermissionType, format_permissions from magpie.register import magpie_register_permissions_from_config, sync_services_phoenix from magpie.services import SERVICE_TYPE_DICT, get_resource_child_allowed @@ -171,23 +172,31 @@ def get_res_types(res): @view_config(route_name=s.PermissionsAPI.name, request_method="PATCH") def update_permissions(request): """ - Update the requested permissions. + Update the requested permissions and create missing related resources if necessary. """ permissions = ar.get_value_multiformat_body_checked(request, "permissions", check_type=list) - ax.verify_param(permissions, not_none=True, not_empty=True, - http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + ax.verify_param(permissions, not_none=True, not_empty=True, http_error=HTTPBadRequest, + msg_on_fail="No permissions to update (empty `permissions` parameter.)") for entry in permissions: - # Check if user/group exists for each permission found - if "permission" in entry.keys() and entry["permission"]: - if "user" in entry.keys(): - ax.verify_param(UserService.by_user_name(entry["user"], db_session=request.db), - not_none=True, http_error=HTTPBadRequest, - msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) - if "group" in entry.keys(): + ax.verify_param(entry, is_type=True, param_compare=dict, http_error=HTTPBadRequest, + msg_on_fail="Permission entry should be of `dict` type, but type `{}` was found instead".format( + type(entry)), + content={"param_content": entry}) + if "permission" in entry and entry["permission"]: + if "user" in entry: + user = UserService.by_user_name(entry["user"], db_session=request.db) + ax.verify_param(user, not_none=True, http_error=HTTPBadRequest, + msg_on_fail="Permission's user `{}` could not be found in the database.".format( + entry["user"]), + content={"param_content": entry}) + uu.check_user_editable(user, request) + if "group" in entry: ax.verify_param(GroupService.by_group_name(entry["group"], db_session=request.db), not_none=True, http_error=HTTPBadRequest, - msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + msg_on_fail="Permission's group `{}` could not be found in the database.".format( + entry["group"]), + content={"param_content": entry}) # Reformat permissions config permissions_cfg = {"permissions": []} @@ -201,25 +210,28 @@ def update_permissions(request): group = entry.get("group") action = entry.get("action", "create") - ax.verify_param(resource_name, not_none=True, - http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) - ax.verify_param(resource_type, not_none=True, - http_error=HTTPBadRequest, msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + ax.verify_param(resource_name, not_none=True, not_empty=True, http_error=HTTPBadRequest, + msg_on_fail="Missing `resource_name` parameter for permissions update.", + content={"param_content": entry}) + ax.verify_param(resource_type, not_none=True, not_empty=True, http_error=HTTPBadRequest, + msg_on_fail="Missing `resource_type` parameter for permissions update.", + content={"param_content": entry}) if i == 0: - # First resource must be a service ax.verify_param(resource_type, is_equal=True, param_compare="service", http_error=HTTPBadRequest, - msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + msg_on_fail="First resource in the permissions list should have a `service` type but has " + "a `{}` type instead.".format(resource_type), + content={"param_content": entry}) service_name = resource_name else: resource_full_path += "/" + resource_name - # Other resources must not be services ax.verify_param(resource_type, not_equal=True, param_compare="service", http_error=HTTPBadRequest, - msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + msg_on_fail="Only the first resource in the permissions list should be of `service` type.", + content={"param_content": entry}) resource_full_type += "/" + resource_type if permission: - # Check that a user and/or a group is defined ax.verify_param(bool(user or group), is_true=True, http_error=HTTPBadRequest, - msg_on_fail=s.Permissions_PATCH_BadRequestResponseSchema.description) + msg_on_fail="No user or group defined with the permission to update.", + content={"param_content": entry}) cfg_entry = { "service": service_name, "resource": resource_full_path, From ecfd963e62ad892488fd7c0bce0f509faf25fc06 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Mon, 21 Mar 2022 15:43:29 -0400 Subject: [PATCH 14/19] fix schema with PR feedback --- magpie/api/schemas.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/magpie/api/schemas.py b/magpie/api/schemas.py index eec5a730a..22df437c2 100644 --- a/magpie/api/schemas.py +++ b/magpie/api/schemas.py @@ -783,20 +783,30 @@ class PermissionNameListSchema(colander.SequenceSchema): class PermissionPatchObjectSchema(colander.MappingSchema): resource_name = colander.SchemaNode( colander.String(), - description="Name of the resource to create." + description="Name of the resource associated with the permission. This resource will be created if missing." ) resource_type = colander.SchemaNode( colander.String(), - description="Type of the resource", + description="Type of the resource. The first resource must be of the `service` type, and children " + "resources must have a relevant resource type that is not of the `service` type.", example="service" ) - user = UserNameParameter - user.missing = colander.drop - - group = GroupNameParameter - group.missing = colander.drop + user = colander.SchemaNode( + colander.String(), + description="Registered local user associated with the permission.", + example="toto", + missing=colander.drop + ) + group = colander.SchemaNode( + colander.String(), + description="Registered user group associated with the permission.", + example="users", + missing=colander.drop + ) + # FIXME: support oneOf(string, object), permission can actually be either a string or a PermissionObjectSchema(dict) + # Currently not possible to have multiple type options with colander. permission = PermissionObjectSchema( missing=colander.drop ) @@ -806,7 +816,7 @@ class PermissionPatchObjectSchema(colander.MappingSchema): description="Action to apply on the permission.", example="create", default="create", - validator=colander.OneOf(["create", "delete"]) + validator=colander.OneOf(["create", "remove"]) ) From e1e6fb3f6f18dfb0045afe6025146891b669c376 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Mon, 21 Mar 2022 15:47:13 -0400 Subject: [PATCH 15/19] fix test variable name --- tests/interfaces.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/interfaces.py b/tests/interfaces.py index f9694862b..d5213147e 100644 --- a/tests/interfaces.py +++ b/tests/interfaces.py @@ -2416,7 +2416,7 @@ def test_PatchPermissions(self): cookies=self.cookies, json=data) utils.check_response_basic_info(resp, 200, expected_method="PATCH") - def check_permissions(data): + def check_permissions(permissions_data): """ Utility function that checks if resource and permissions found in data were succesfully create/removed. """ @@ -2424,7 +2424,7 @@ def check_permissions(data): resp = utils.test_request(self, "GET", path, headers=self.json_headers, cookies=self.cookies) resources = utils.check_response_basic_info(resp)[self.test_service_name]["resources"] - for perm_entry in data["permissions"][1:]: # skip first resource, which is the service + for perm_entry in permissions_data["permissions"][1:]: # skip first resource, which is the service target_res = [res for res in resources.values() if res["resource_name"] == perm_entry["resource_name"]] # target resource should exist assert len(target_res) == 1 From 7a7ecd510c9b83d2c974512f6536e4be762ed1ea Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Mon, 21 Mar 2022 16:53:03 -0400 Subject: [PATCH 16/19] fix error logging code with pr feedback --- .../api/management/resource/resource_views.py | 6 ++-- magpie/register.py | 36 ++++++++----------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/magpie/api/management/resource/resource_views.py b/magpie/api/management/resource/resource_views.py index ffe6f104c..785f32d69 100644 --- a/magpie/api/management/resource/resource_views.py +++ b/magpie/api/management/resource/resource_views.py @@ -247,7 +247,9 @@ def update_permissions(request): permissions_cfg["permissions"].append(cfg_entry) # Apply permission update - magpie_register_permissions_from_config(permissions_config=permissions_cfg, db_session=request.db, - raise_errors=True) + ax.evaluate_call( + lambda: magpie_register_permissions_from_config( + permissions_config=permissions_cfg, db_session=request.db, raise_errors=True), + http_error=HTTPBadRequest, msg_on_fail="Failed to update requested permissions.") return ax.valid_http(http_success=HTTPOk, detail=s.Permissions_PATCH_OkResponseSchema.description) diff --git a/magpie/register.py b/magpie/register.py index 8b8059e93..180fafc87 100644 --- a/magpie/register.py +++ b/magpie/register.py @@ -675,7 +675,7 @@ def magpie_register_services_from_config(service_config_path, push_to_phoenix=Fa def _log_permission(message, permission_index, trail=", skipping...", detail=None, permission=None, level=logging.WARN, raise_errors=False): - # type: (Str, int, Str, Optional[Str], Optional[Str], Union[Str, int]) -> None + # type: (Str, int, Str, Optional[Str], Optional[Str], Union[Str, int], bool) -> None """ Logs a message related to a 'permission' entry. @@ -717,7 +717,7 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem entry_index, # type: int service_info, # type: JSON cookies_or_session=None, # type: CookiesOrSessionType - magpie_url=None, # type: Optional[Str], + magpie_url=None, # type: Optional[Str] raise_errors=False # type: bool ): # type: (...) -> Tuple[Optional[int], bool] """ @@ -735,12 +735,8 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem raise RegistrationValueError("cannot use cookies without corresponding request URL") resource = None - resource_path = permission_config_entry.get("resource", "") + resource_path = permission_config_entry.get("resource", "").strip("/") resource_type_config = permission_config_entry.get("type") - if resource_path.startswith("/"): - resource_path = resource_path[1:] - if resource_path.endswith("/"): - resource_path = resource_path[:-1] if resource_path: try: svc_name = service_info["service_name"] @@ -916,7 +912,7 @@ def _apply_profile(_usr_name=None, _grp_name=None): from magpie.api.management.group.group_utils import create_group return create_group(**grp_data) - def _validate_response(operation, is_create, item_type="Permission", raise_errors=False): + def _validate_response(operation, is_create, item_type="Permission"): """ Validate action/operation applied and handles raised ``HTTPException`` as returned response. """ @@ -934,21 +930,17 @@ def _validate_response(operation, is_create, item_type="Permission", raise_error # validation according to status code returned if is_create: if _resp.status_code in [200, 201]: # update/create - _log_permission("{} successfully created.".format(item_type), entry_index, level=logging.INFO, trail="", - raise_errors=False) + _log_permission("{} successfully created.".format(item_type), entry_index, level=logging.INFO, trail="") elif _resp.status_code == 409: - _log_permission("{} already exists.".format(item_type), entry_index, level=logging.INFO, - raise_errors=False) + _log_permission("{} already exists.".format(item_type), entry_index, level=logging.INFO) else: _log_permission("Unknown response [{}]".format(_resp.status_code), entry_index, permission=permission_config_entry, level=logging.ERROR, raise_errors=raise_errors) else: if _resp.status_code == 200: - _log_permission("{} successfully removed.".format(item_type), entry_index, level=logging.INFO, trail="", - raise_errors=False) + _log_permission("{} successfully removed.".format(item_type), entry_index, level=logging.INFO, trail="") elif _resp.status_code == 404: - _log_permission("{} already removed.".format(item_type), entry_index, level=logging.INFO, - raise_errors=False) + _log_permission("{} already removed.".format(item_type), entry_index, level=logging.INFO) else: _log_permission("Unknown response [{}]".format(_resp.status_code), entry_index, permission=permission_config_entry, level=logging.ERROR, raise_errors=raise_errors) @@ -960,14 +952,14 @@ def _validate_response(operation, is_create, item_type="Permission", raise_error perm = PermissionSet(perm_def) # process groups first as they can be referenced by user definitions - _validate_response(lambda: _apply_profile(None, grp_name), is_create=True, raise_errors=raise_errors) - _validate_response(lambda: _apply_profile(usr_name, None), is_create=True, raise_errors=raise_errors) + _validate_response(lambda: _apply_profile(None, grp_name), is_create=True) + _validate_response(lambda: _apply_profile(usr_name, None), is_create=True) if _use_request(cookies_or_session): - _validate_response(lambda: _apply_request(None, grp_name), is_create=create_perm, raise_errors=raise_errors) - _validate_response(lambda: _apply_request(usr_name, None), is_create=create_perm, raise_errors=raise_errors) + _validate_response(lambda: _apply_request(None, grp_name), is_create=create_perm) + _validate_response(lambda: _apply_request(usr_name, None), is_create=create_perm) else: - _validate_response(lambda: _apply_session(None, grp_name), is_create=create_perm, raise_errors=raise_errors) - _validate_response(lambda: _apply_session(usr_name, None), is_create=create_perm, raise_errors=raise_errors) + _validate_response(lambda: _apply_session(None, grp_name), is_create=create_perm) + _validate_response(lambda: _apply_session(usr_name, None), is_create=create_perm) def magpie_register_permissions_from_config(permissions_config, magpie_url=None, db_session=None, raise_errors=False): From cb45eb46d12030429cd8ad78d2a4a8a5f0dac011 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Mon, 21 Mar 2022 17:31:39 -0400 Subject: [PATCH 17/19] rearrange user/group/permissions checks in the update_permissions method --- .../api/management/resource/resource_views.py | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/magpie/api/management/resource/resource_views.py b/magpie/api/management/resource/resource_views.py index 785f32d69..6f1c32870 100644 --- a/magpie/api/management/resource/resource_views.py +++ b/magpie/api/management/resource/resource_views.py @@ -178,25 +178,39 @@ def update_permissions(request): ax.verify_param(permissions, not_none=True, not_empty=True, http_error=HTTPBadRequest, msg_on_fail="No permissions to update (empty `permissions` parameter.)") + required_users = set() + required_groups = set() + has_permission_to_update = False for entry in permissions: ax.verify_param(entry, is_type=True, param_compare=dict, http_error=HTTPBadRequest, msg_on_fail="Permission entry should be of `dict` type, but type `{}` was found instead".format( type(entry)), content={"param_content": entry}) if "permission" in entry and entry["permission"]: - if "user" in entry: - user = UserService.by_user_name(entry["user"], db_session=request.db) - ax.verify_param(user, not_none=True, http_error=HTTPBadRequest, - msg_on_fail="Permission's user `{}` could not be found in the database.".format( - entry["user"]), - content={"param_content": entry}) - uu.check_user_editable(user, request) - if "group" in entry: - ax.verify_param(GroupService.by_group_name(entry["group"], db_session=request.db), - not_none=True, http_error=HTTPBadRequest, - msg_on_fail="Permission's group `{}` could not be found in the database.".format( - entry["group"]), - content={"param_content": entry}) + user = entry.get("user") + group = entry.get("group") + ax.verify_param(bool(user or group), is_true=True, http_error=HTTPBadRequest, + msg_on_fail="No user or group defined with the permission to update.", + content={"param_content": entry}) + has_permission_to_update = True + if user: + required_users.add(user) + if group: + required_groups.add(group) + + for user_name in required_users: + user = UserService.by_user_name(user_name, db_session=request.db) + ax.verify_param(user, not_none=True, http_error=HTTPBadRequest, + msg_on_fail="Permission's user `{}` could not be found in the database.".format(user_name)) + uu.check_user_editable(user, request) + for group_name in required_groups: + ax.verify_param(GroupService.by_group_name(group_name, db_session=request.db), + not_none=True, http_error=HTTPBadRequest, + msg_on_fail="Permission's group `{}` could not be found in the database.".format(group_name)) + + ax.verify_param(has_permission_to_update, is_true=True, http_error=HTTPBadRequest, + msg_on_fail="No permissions to update (none of the input entries has a defined permission.)", + content={"param_content": permissions}) # Reformat permissions config permissions_cfg = {"permissions": []} @@ -229,9 +243,6 @@ def update_permissions(request): content={"param_content": entry}) resource_full_type += "/" + resource_type if permission: - ax.verify_param(bool(user or group), is_true=True, http_error=HTTPBadRequest, - msg_on_fail="No user or group defined with the permission to update.", - content={"param_content": entry}) cfg_entry = { "service": service_name, "resource": resource_full_path, From 75cf60a192582a4b8ff1d8682741bac9c5e4ec94 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Tue, 22 Mar 2022 17:02:39 -0400 Subject: [PATCH 18/19] change exception type in function that manages permission logs --- magpie/register.py | 80 +++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/magpie/register.py b/magpie/register.py index 180fafc87..12c9c2bc3 100644 --- a/magpie/register.py +++ b/magpie/register.py @@ -11,7 +11,7 @@ import six import transaction import yaml -from pyramid.httpexceptions import HTTPBadRequest, HTTPException +from pyramid.httpexceptions import HTTPException from sqlalchemy.orm.session import Session from ziggurat_foundations.models.services.group import GroupService from ziggurat_foundations.models.services.resource import ResourceService @@ -673,11 +673,11 @@ def magpie_register_services_from_config(service_config_path, push_to_phoenix=Fa return merged_service_configs -def _log_permission(message, permission_index, trail=", skipping...", detail=None, permission=None, level=logging.WARN, - raise_errors=False): +def _handle_permission(message, permission_index, trail=", skipping...", detail=None, permission=None, + level=logging.WARN, raise_errors=False): # type: (Str, int, Str, Optional[Str], Optional[Str], Union[Str, int], bool) -> None """ - Logs a message related to a 'permission' entry. + Logs a message related to a 'permission' entry and raises an error if required. Log message format is as follows (detail portion omitted if none provided):: @@ -706,7 +706,8 @@ def _log_permission(message, permission_index, trail=", skipping...", detail=Non LOGGER.log(level, "%s [permission #%d]%s%s", message, permission_index, permission, trail) if raise_errors: - raise HTTPBadRequest("{} [permission #{}]{}{}".format(message, permission_index, permission, trail)) + raise RegistrationConfigurationError( + "{} [permission #{}]{}{}".format(message, permission_index, permission, trail)) def _use_request(cookies_or_session): @@ -776,24 +777,21 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem svc_res_types = SERVICE_TYPE_DICT[svc_type].resource_type_names type_count = len(svc_res_types) if type_count == 0: - _log_permission("Cannot generate resource", entry_index, raise_errors=raise_errors, - detail="Service [{!s}] of type [{!s}] doesn't allows any sub-resource types. " - .format(svc_name, svc_type)) - raise RegistrationConfigurationError("Invalid resource not allowed to apply permission.") + _handle_permission("Cannot generate resource", entry_index, raise_errors=True, + detail="Service [{!s}] of type [{!s}] doesn't allows any sub-resource types. " + .format(svc_name, svc_type)) if type_count != 1 and not (isinstance(resource_type, six.string_types) and resource_type): - _log_permission("Cannot automatically generate resource", entry_index, raise_errors=raise_errors, - detail="Service [{!s}] of type [{!s}] allows more than 1 sub-resource types ({}). " - "Type must be explicitly specified for auto-creation. " - "Available choices are: {}" - .format(svc_name, svc_type, type_count, svc_res_types)) - raise RegistrationConfigurationError("Missing resource 'type' to apply permission.") + _handle_permission("Cannot automatically generate resource", entry_index, raise_errors=True, + detail="Service [{!s}] of type [{!s}] allows more than 1 sub-resource " + "types ({}). Type must be explicitly specified for auto-creation. " + "Available choices are: {}" + .format(svc_name, svc_type, type_count, svc_res_types)) if type_count != 1 and resource_type not in svc_res_types: - _log_permission("Cannot generate resource", entry_index, raise_errors=raise_errors, - detail="Service [{!s}] of type [{!s}] allows more than 1 sub-resource types ({}). " - "Specified type [{!s}] doesn't match any of the allowed resource types. " - "Available choices are: {}" - .format(svc_name, svc_type, type_count, resource_type, svc_res_types)) - raise RegistrationConfigurationError("Invalid resource 'type' not allowed to apply permission.") + _handle_permission("Cannot generate resource", entry_index, raise_errors=True, + detail="Service [{!s}] of type [{!s}] allows more than 1 sub-resource " + "types ({}). Specified type [{!s}] doesn't match any of the allowed " + "resource types. Available choices are: {}" + .format(svc_name, svc_type, type_count, resource_type, svc_res_types)) res_type = resource_type or svc_res_types[0] if _use_request(cookies_or_session): body = {"resource_name": res, "resource_type": res_type, "parent_id": parent} @@ -810,10 +808,10 @@ def _parse_resource_path(permission_config_entry, # type: PermissionConfigItem raise RegistrationConfigurationError("Could not extract child resource from resource path.") except HTTPException as exc: detail = "{} ({}), {!s}".format(type(exc).__name__, exc.code, exc) - _log_permission("Failed resources parsing.", entry_index, detail=detail, raise_errors=raise_errors) + _handle_permission("Failed resources parsing.", entry_index, detail=detail, raise_errors=raise_errors) return None, False except Exception as exc: - _log_permission("Failed resources parsing.", entry_index, detail=repr(exc), raise_errors=raise_errors) + _handle_permission("Failed resources parsing.", entry_index, detail=repr(exc), raise_errors=raise_errors) return None, False return resource, True @@ -930,20 +928,22 @@ def _validate_response(operation, is_create, item_type="Permission"): # validation according to status code returned if is_create: if _resp.status_code in [200, 201]: # update/create - _log_permission("{} successfully created.".format(item_type), entry_index, level=logging.INFO, trail="") + _handle_permission("{} successfully created.".format(item_type), entry_index, + level=logging.INFO, trail="") elif _resp.status_code == 409: - _log_permission("{} already exists.".format(item_type), entry_index, level=logging.INFO) + _handle_permission("{} already exists.".format(item_type), entry_index, level=logging.INFO) else: - _log_permission("Unknown response [{}]".format(_resp.status_code), entry_index, - permission=permission_config_entry, level=logging.ERROR, raise_errors=raise_errors) + _handle_permission("Unknown response [{}]".format(_resp.status_code), entry_index, + permission=permission_config_entry, level=logging.ERROR, raise_errors=raise_errors) else: if _resp.status_code == 200: - _log_permission("{} successfully removed.".format(item_type), entry_index, level=logging.INFO, trail="") + _handle_permission("{} successfully removed.".format(item_type), entry_index, + level=logging.INFO, trail="") elif _resp.status_code == 404: - _log_permission("{} already removed.".format(item_type), entry_index, level=logging.INFO) + _handle_permission("{} already removed.".format(item_type), entry_index, level=logging.INFO) else: - _log_permission("Unknown response [{}]".format(_resp.status_code), entry_index, - permission=permission_config_entry, level=logging.ERROR, raise_errors=raise_errors) + _handle_permission("Unknown response [{}]".format(_resp.status_code), entry_index, + permission=permission_config_entry, level=logging.ERROR, raise_errors=raise_errors) create_perm = permission_config_entry["action"] == "create" perm_def = permission_config_entry["permission"] # name or object @@ -1045,28 +1045,28 @@ def _process_permissions(permissions, magpie_url, cookies_or_session, users=None for i, perm_cfg in enumerate(permissions): # parameter validation if not isinstance(perm_cfg, dict) or not all(f in perm_cfg for f in ["permission", "service"]): - _log_permission("Invalid permission format for [{!s}]".format(perm_cfg), i, raise_errors=raise_errors) + _handle_permission("Invalid permission format for [{!s}]".format(perm_cfg), i, raise_errors=raise_errors) continue try: perm = PermissionSet(perm_cfg["permission"]) except (ValueError, TypeError): perm = None if not perm: - _log_permission("Unknown permission [{!s}]".format(perm_cfg["permission"]), i, raise_errors=raise_errors) + _handle_permission("Unknown permission [{!s}]".format(perm_cfg["permission"]), i, raise_errors=raise_errors) continue usr_name = perm_cfg.get("user") grp_name = perm_cfg.get("group") if not any([usr_name, grp_name]): - _log_permission("Missing required user and/or group field.", i, raise_errors=raise_errors) + _handle_permission("Missing required user and/or group field.", i, raise_errors=raise_errors) continue if usr_name == anon_user: - _log_permission("Skipping forbidden user permission (reserved special user: {}).".format(anon_user), i) + _handle_permission("Skipping forbidden user permission (reserved special user: {}).".format(anon_user), i) continue if "action" not in perm_cfg: - _log_permission("Unspecified action", i, trail="using default (create)...", raise_errors=raise_errors) + _handle_permission("Unspecified action", i, trail="using default (create)...", raise_errors=raise_errors) perm_cfg["action"] = "create" if perm_cfg["action"] not in ["create", "remove"]: - _log_permission("Unknown action [{!s}]".format(perm_cfg["action"]), i, raise_errors=raise_errors) + _handle_permission("Unknown action [{!s}]".format(perm_cfg["action"]), i, raise_errors=raise_errors) continue # retrieve service for permissions validation @@ -1075,15 +1075,15 @@ def _process_permissions(permissions, magpie_url, cookies_or_session, users=None svc_path = magpie_url + ServiceAPI.path.format(service_name=svc_name) svc_resp = requests.get(svc_path, cookies=cookies_or_session) if svc_resp.status_code != 200: - _log_permission("Unknown service [{!s}]".format(svc_name), i, raise_errors=raise_errors) + _handle_permission("Unknown service [{!s}]".format(svc_name), i, raise_errors=raise_errors) continue service_info = get_json(svc_resp)[svc_name] else: transaction.commit() # force any pending transaction to be applied to find possible dependencies svc = models.Service.by_service_name(svc_name, db_session=cookies_or_session) if not svc: - _log_permission("Unknown service [{!s}]. Can't edit permissions without service.".format(svc_name), i, - raise_errors=raise_errors) + _handle_permission("Unknown service [{!s}]. Can't edit permissions without service.".format(svc_name), + i, raise_errors=raise_errors) continue from magpie.api.management.service.service_formats import format_service service_info = format_service(svc) From d0b228990433a1668ea57bacf73b6acdc26386b8 Mon Sep 17 00:00:00 2001 From: Charles-William Cummings Date: Wed, 23 Mar 2022 09:31:48 -0400 Subject: [PATCH 19/19] small fixes after PR feedback --- CHANGES.rst | 2 +- .../api/management/resource/resource_views.py | 18 +++++++++--------- magpie/register.py | 6 +++--- tests/interfaces.py | 12 ++++++++---- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 12a35e6e5..68a9a1a9d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -12,7 +12,7 @@ Changes Features / Changes ~~~~~~~~~~~~~~~~~~~~~ * Add ``PATCH /permissions`` endpoint that updates permissions and creates related resources if necessary. -* ``permissions.cfg`` now supports a new format for the ``type`` parameter, using multiple types separated +* Add support of new format for ``permissions.cfg`` for the ``type`` parameter, using multiple types separated by a slash character, matching each type with each resource found in the ``resource`` parameter. .. _changes_3.23.0: diff --git a/magpie/api/management/resource/resource_views.py b/magpie/api/management/resource/resource_views.py index aff304249..f28649cff 100644 --- a/magpie/api/management/resource/resource_views.py +++ b/magpie/api/management/resource/resource_views.py @@ -226,7 +226,7 @@ def update_permissions(request): """ permissions = ar.get_value_multiformat_body_checked(request, "permissions", check_type=list) ax.verify_param(permissions, not_none=True, not_empty=True, http_error=HTTPBadRequest, - msg_on_fail="No permissions to update (empty `permissions` parameter.)") + msg_on_fail="No permissions to update (empty `permissions` parameter).") required_users = set() required_groups = set() @@ -235,13 +235,13 @@ def update_permissions(request): ax.verify_param(entry, is_type=True, param_compare=dict, http_error=HTTPBadRequest, msg_on_fail="Permission entry should be of `dict` type, but type `{}` was found instead".format( type(entry)), - content={"param_content": entry}) + param_content={"value": entry}) if "permission" in entry and entry["permission"]: user = entry.get("user") group = entry.get("group") ax.verify_param(bool(user or group), is_true=True, http_error=HTTPBadRequest, msg_on_fail="No user or group defined with the permission to update.", - content={"param_content": entry}) + param_content={"value": entry}) has_permission_to_update = True if user: required_users.add(user) @@ -259,8 +259,8 @@ def update_permissions(request): msg_on_fail="Permission's group `{}` could not be found in the database.".format(group_name)) ax.verify_param(has_permission_to_update, is_true=True, http_error=HTTPBadRequest, - msg_on_fail="No permissions to update (none of the input entries has a defined permission.)", - content={"param_content": permissions}) + msg_on_fail="No permissions to update (none of the input entries has a defined permission).", + param_content={"value": permissions}) # Reformat permissions config permissions_cfg = {"permissions": []} @@ -276,21 +276,21 @@ def update_permissions(request): ax.verify_param(resource_name, not_none=True, not_empty=True, http_error=HTTPBadRequest, msg_on_fail="Missing `resource_name` parameter for permissions update.", - content={"param_content": entry}) + param_name="resource_name", param_content={"value": entry}) ax.verify_param(resource_type, not_none=True, not_empty=True, http_error=HTTPBadRequest, msg_on_fail="Missing `resource_type` parameter for permissions update.", - content={"param_content": entry}) + param_name="resource_type", param_content={"value": entry}) if i == 0: ax.verify_param(resource_type, is_equal=True, param_compare="service", http_error=HTTPBadRequest, msg_on_fail="First resource in the permissions list should have a `service` type but has " "a `{}` type instead.".format(resource_type), - content={"param_content": entry}) + param_name="resource_type", param_content={"value": entry}) service_name = resource_name else: resource_full_path += "/" + resource_name ax.verify_param(resource_type, not_equal=True, param_compare="service", http_error=HTTPBadRequest, msg_on_fail="Only the first resource in the permissions list should be of `service` type.", - content={"param_content": entry}) + param_name="resource_type", param_content={"value": entry}) resource_full_type += "/" + resource_type if permission: cfg_entry = { diff --git a/magpie/register.py b/magpie/register.py index 12c9c2bc3..3929af628 100644 --- a/magpie/register.py +++ b/magpie/register.py @@ -703,11 +703,11 @@ def _handle_permission(message, permission_index, trail=", skipping...", detail= """ trail = "{}\nDetail: [{!s}]".format(trail, detail) if detail else (trail or "") permission = " [{!s}]".format(permission) if permission else "" - LOGGER.log(level, "%s [permission #%d]%s%s", message, permission_index, permission, trail) + msg = "{} [permission #{}]{}{}".format(message, permission_index, permission, trail) + LOGGER.log(level, msg) if raise_errors: - raise RegistrationConfigurationError( - "{} [permission #{}]{}{}".format(message, permission_index, permission, trail)) + raise RegistrationConfigurationError(msg) def _use_request(cookies_or_session): diff --git a/tests/interfaces.py b/tests/interfaces.py index ba1a83f7b..a88cdbd77 100644 --- a/tests/interfaces.py +++ b/tests/interfaces.py @@ -2501,7 +2501,8 @@ def test_PatchPermissions_InvalidUserGroup(self): path = "/permissions" resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, expect_errors=True, cookies=self.cookies, json=data) - utils.check_response_basic_info(resp, 400, expected_method="PATCH") + body = utils.check_response_basic_info(resp, 400, expected_method="PATCH") + utils.check_error_param_structure(body) # Test with invalid user data = { @@ -2517,7 +2518,8 @@ def test_PatchPermissions_InvalidUserGroup(self): path = "/permissions" resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, expect_errors=True, cookies=self.cookies, json=data) - utils.check_response_basic_info(resp, 400, expected_method="PATCH") + body = utils.check_response_basic_info(resp, 400, expected_method="PATCH") + utils.check_error_param_structure(body) @runner.MAGPIE_TEST_PERMISSIONS def test_PatchPermissions_InvalidResourceType(self): @@ -2533,7 +2535,8 @@ def test_PatchPermissions_InvalidResourceType(self): path = "/permissions" resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, expect_errors=True, cookies=self.cookies, json=data) - utils.check_response_basic_info(resp, 400, expected_method="PATCH") + body = utils.check_response_basic_info(resp, 400, expected_method="PATCH") + utils.check_error_param_structure(body, param_value=data["permissions"]) # Test with invalid resource data = { @@ -2552,7 +2555,8 @@ def test_PatchPermissions_InvalidResourceType(self): path = "/permissions" resp = utils.test_request(self, "PATCH", path, headers=self.json_headers, expect_errors=True, cookies=self.cookies, json=data) - utils.check_response_basic_info(resp, 400, expected_method="PATCH") + body = utils.check_response_basic_info(resp, 400, expected_method="PATCH") + utils.check_error_param_structure(body, param_value=data["permissions"]) def create_validate_permissions(self, test_user_name, # type: Str