diff --git a/gnocchi/rest/api.py b/gnocchi/rest/api.py index 39d73b362..3b2bebdaf 100644 --- a/gnocchi/rest/api.py +++ b/gnocchi/rest/api.py @@ -801,7 +801,11 @@ def get_all(self): self.resource_type, self.resource_id) if not resource: abort(404, str(indexer.NoSuchResource(self.resource_id))) - enforce("get resource", resource) + pecan.request.auth_helper.enforce_resource_policy( + pecan.request, + "get resource", + self.resource_id, + resource) return pecan.request.indexer.list_metrics( attribute_filter={"=": {"resource_id": self.resource_id}}) @@ -822,7 +826,11 @@ def get(self, **kwargs): if not resource: abort(404, str(indexer.NoSuchResource(self.resource_id))) - enforce("get resource", resource) + pecan.request.auth_helper.enforce_resource_policy( + pecan.request, + "get resource", + self.resource_id, + resource) try: resources = pecan.request.indexer.list_resources( @@ -1080,7 +1088,11 @@ def get(self): resource = pecan.request.indexer.get_resource( self._resource_type, self.id, with_metrics=True) if resource: - enforce("get resource", resource) + pecan.request.auth_helper.enforce_resource_policy( + pecan.request, + "get resource", + self.id, + resource) etag_precondition_check(resource) etag_set_headers(resource) return resource diff --git a/gnocchi/rest/auth_helper.py b/gnocchi/rest/auth_helper.py index 67ab28c23..263df4aa7 100644 --- a/gnocchi/rest/auth_helper.py +++ b/gnocchi/rest/auth_helper.py @@ -19,6 +19,7 @@ import webob import werkzeug.http +from gnocchi import indexer from gnocchi.rest import api @@ -45,6 +46,83 @@ def get_auth_info(request): 'roles': request.headers.get("X-Roles", "").split(","), } + @staticmethod + def enforce_resource_policy(request, + rule, + resource_id, + resource, + prefix=None): + try: + # Check if the policy allows the user to get any resource + api.enforce(rule, {}) + except webob.exc.HTTPForbidden: + policy_matched = False + auth_info = KeystoneAuthHelper.get_auth_info(request) + project_id = auth_info["project_id"] + user_id = auth_info["user_id"] + + target = {} + if prefix: + target_resource = target[prefix] = {} + else: + target_resource = target + + target_resource["project_id"] = project_id + try: + # Check if the policy allows the user to get resources linked + # to their project + api.enforce(rule, target) + except webob.exc.HTTPForbidden: + pass + else: + policy_matched = True + # User is authenticated using the project that owns the + # resource and the policy allows access by project. + if resource.project_id == project_id: + return + + resource_creator_user_id, _, resource_creator_project_id = ( + resource.creator.partition(":")) + + del target_resource["project_id"] + target_resource["created_by_project_id"] = project_id + try: + # Check if the policy allows the user to get resources linked + # to their created_by_project + api.enforce(rule, target) + except webob.exc.HTTPForbidden: + pass + else: + policy_matched = True + # User is authenticated using the project that created the + # resource and the policy allows access by created_by_project. + if resource_creator_project_id == project_id: + return + + del target_resource["created_by_project_id"] + target_resource["creator"] = user_id + try: + # Check if the policy allows the user to get resources linked + # to their creator + api.enforce(rule, target) + except webob.exc.HTTPForbidden: + pass + else: + policy_matched = True + # The authenticated user is the user that created the resource + # and the policy allows access by creator. + if resource_creator_user_id == user_id: + return + + # If at least one of the policies matched but the user should not + # be allowed access to the resource, return 404 Not Found to + # prevent the user from enumerating the existence of the resource. + if policy_matched: + api.abort(404, str(indexer.NoSuchResource(resource_id))) + + # None of the above policies matched, return 403 Forbidden. + api.abort(403) + @staticmethod def get_resource_policy_filter(request, rule, resource_type, prefix=None): try: @@ -177,6 +255,14 @@ def get_auth_info(self, request): "system": 'all', } + @staticmethod + def enforce_resource_policy(request, + rule, + resource_id, + resource, + prefix=None): + pass + @staticmethod def get_resource_policy_filter(request, rule, resource_type, prefix=None): return None @@ -210,6 +296,14 @@ def get_auth_info(self, request): "system": 'all', } + @staticmethod + def enforce_resource_policy(request, + rule, + resource_id, + resource, + prefix=None): + pass + @staticmethod def get_resource_policy_filter(request, rule, resource_type, prefix=None): return None diff --git a/gnocchi/tests/test_rest.py b/gnocchi/tests/test_rest.py index 93dcc5c79..ef24411ee 100644 --- a/gnocchi/tests/test_rest.py +++ b/gnocchi/tests/test_rest.py @@ -1005,7 +1005,7 @@ def test_get_resource_unauthorized(self): + self.resource_type + "/" + self.attributes['id'], - status=403) + status=404) def test_get_resource_named_metric(self): self.attributes['metrics'] = {'foo': {'archive_policy_name': "high"}} @@ -1026,7 +1026,7 @@ def test_list_resource_metrics_unauthorized(self): self.app.get( "/v1/resource/" + self.resource_type + "/" + self.attributes['id'] + "/metric", - status=403) + status=404) def test_delete_resource_named_metric(self): self.attributes['metrics'] = {'foo': {'archive_policy_name': "high"}}