Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions gnocchi/rest/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}})

Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down
94 changes: 94 additions & 0 deletions gnocchi/rest/auth_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import webob
import werkzeug.http

from gnocchi import indexer
from gnocchi.rest import api


Expand All @@ -45,6 +46,83 @@ def get_auth_info(request):
'roles': request.headers.get("X-Roles", "").split(","),
}

@staticmethod
def enforce_resource_policy(request,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need some logging in this method. This would help us when troubleshooting issues. I mean, so operators can easily spot such situations. The logs can be in DEBUG level, in my opinion.

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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions gnocchi/tests/test_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"}}
Expand All @@ -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"}}
Expand Down
Loading