openstack/cloudkitty
Code Issues Proposed changes

Update oslo.policy and oslo.context usage

Browse Source 
This changes the way oslo.policy and oslo.context are used. This aims at
delegating as much work as possible to these libraries in order to ease
maintainability.

Work items:

* Use oslo_context.RequestContext's from_environ() method instead of building
  the context manually.

* Replace "tenant_id" with "project_id" in policy rule definition/enforcement.

* Introduce a cloudkitty.context module allowing to easily tweak request
  context creation.

Change-Id: I91f38b9aded2dc6453507edaa55cfd9ddf7e995b
This commit is contained in:
Luka Peschke 2019-10-31 12:08:12 +01:00
parent 8a0f80ad91
commit a80d0e977d
11 changed files with 60 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
cloudkitty/api/v1/controllers/report.py
View File

@ -92,7 +92,7 @@ class ReportController(rest.RestController):
tenant_context = pecan.request.context.project_id
tenant_id = tenant_context if not tenant_id else tenant_id
policy.authorize(pecan.request.context, 'report:get_total',
{"tenant_id": tenant_id})
{"project_id": tenant_id})
storage = pecan.request.storage_backend
# FIXME(sheeprine): We should filter on user id.
@ -134,7 +134,7 @@ class ReportController(rest.RestController):
tenant_context = pecan.request.context.project_id
tenant_id = tenant_context if not tenant_id else tenant_id
policy.authorize(pecan.request.context, 'report:get_summary',
{"tenant_id": tenant_id})
{"project_id": tenant_id})
storage = pecan.request.storage_backend
scope_key = CONF.collect.scope_key

2
cloudkitty/api/v1/controllers/storage.py
View File

@ -53,7 +53,7 @@ class DataFramesController(rest.RestController):
project_id = tenant_id or pecan.request.context.project_id
policy.authorize(pecan.request.context, 'storage:list_data_frames', {
'tenant_id': project_id,
'project_id': project_id,
})
scope_key = CONF.collect.scope_key

23
cloudkitty/api/v1/hooks.py
View File

@ -13,10 +13,9 @@
# License for the specific language governing permissions and limitations
# under the License.
#
from oslo_context import context
from pecan import hooks
from cloudkitty.common import policy
from cloudkitty.common import context
from cloudkitty import messaging
@ -38,21 +37,5 @@ class StorageHook(hooks.PecanHook):
class ContextHook(hooks.PecanHook):
def on_route(self, state):
headers = state.request.headers
roles = headers.get('X-Roles', '').split(',')
is_admin = policy.check_is_admin(roles)
creds = {
'user_id': headers.get('X-User-Id', ''),
'tenant': headers.get('X-Tenant-Id', ''),
'auth_token': headers.get('X-Auth-Token', ''),
'is_admin': is_admin,
'roles': roles,
"user_name": headers.get('X-User-Name', ''),
"project_name": headers.get('X-Project-Name', ''),
"domain": headers.get('X-User-Domain-Id', ''),
"domain_name": headers.get('X-User-Domain-Name', ''),
}
state.request.context = context.RequestContext(**creds)
state.request.context = context.RequestContext.from_environ(
state.request.environ)

20
cloudkitty/api/v2/__init__.py
View File

@ -15,10 +15,9 @@
import importlib
import flask
from oslo_context import context
import voluptuous
from cloudkitty.common import policy
from cloudkitty.common import context
RESOURCE_SCHEMA = voluptuous.Schema({
@ -39,21 +38,8 @@ API_MODULES = [
def _extend_request_context():
headers = flask.request.headers
roles = headers.get('X-Roles', '').split(',')
is_admin = policy.check_is_admin(roles)
ctx = {
'user_id': headers.get('X-User-Id', ''),
'auth_token': headers.get('X-Auth-Token', ''),
'is_admin': is_admin,
'roles': roles,
'project_id': headers.get('X-Project-Id', ''),
'domain_id': headers.get('X-Domain-Id', ''),
}
flask.request.context = context.RequestContext(**ctx)
flask.request.context = context.RequestContext.from_environ(
flask.request.environ)
def get_api_app():

4
cloudkitty/api/v2/scope/state.py
View File

@ -62,7 +62,7 @@ class ScopeState(base.BaseResource):
policy.authorize(
flask.request.context,
'scope:get_state',
{'tenant_id': scope_id or flask.request.context.project_id}
{'project_id': scope_id or flask.request.context.project_id}
)
results = self._storage_state.get_all(
identifier=scope_id,
@ -110,7 +110,7 @@ class ScopeState(base.BaseResource):
policy.authorize(
flask.request.context,
'scope:reset_state',
{'tenant_id': scope_id or flask.request.context.project_id}
{'project_id': scope_id or flask.request.context.project_id}
)
if not all_scopes and scope_id is None:

2
cloudkitty/api/v2/summary/summary.py
View File

@ -40,7 +40,7 @@ class Summary(base.BaseResource):
policy.authorize(
flask.request.context,
'summary:get_summary',
{'tenant_id': flask.request.context.project_id})
{'project_id': flask.request.context.project_id})
begin = begin or tzutils.get_month_start()
end = end or tzutils.get_next_month()

26
cloudkitty/common/context.py Normal file
View File

@ -0,0 +1,26 @@
# Copyright 2019 Objectif Libre
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
#
from oslo_context import context
from cloudkitty.common import policy
class RequestContext(context.RequestContext):
def __init__(self, is_admin=None, **kwargs):
super(RequestContext, self).__init__(is_admin=is_admin, **kwargs)
if self.is_admin is None:
self.is_admin = policy.check_is_admin(self)

2
cloudkitty/common/policies/base.py
View File

@ -25,7 +25,7 @@ rules = [
check_str='role:admin'),
policy.RuleDefault(
name='admin_or_owner',
check_str='is_admin:True or tenant:%(tenant_id)s'),
check_str='is_admin:True or project_id:%(project_id)s'),
policy.RuleDefault(
name='default',
check_str=UNPROTECTED)

17
cloudkitty/common/policy.py
View File

@ -105,7 +105,9 @@ def authorize(context, action, target):
init()
try:
return _ENFORCER.authorize(action, target, context.to_dict(),
LOG.debug('Authenticating user with credentials %(credentials)s',
{'credentials': context.to_dict()})
return _ENFORCER.authorize(action, target, context,
do_raise=True,
exc=PolicyNotAuthorized,
action=action)
@ -120,7 +122,7 @@ def authorize(context, action, target):
{'action': action, 'credentials': context.to_dict()})
def check_is_admin(roles):
def check_is_admin(context):
"""Whether or not roles contains 'admin' role according to policy setting.
"""
@ -129,12 +131,11 @@ def check_is_admin(roles):
init()
# include project_id on target to avoid KeyError if context_is_admin
# policy definition is missing, and default admin_or_owner rule
# attempts to apply. Since our credentials dict does not include a
# project_id, this target can never match as a generic rule.
target = {'project_id': ''}
credentials = {'roles': roles}
target = {
'user_id': context.user_id,
'project_id': context.project_id,
}
credentials = context.to_policy_values()
return _ENFORCER.authorize('context_is_admin', target, credentials)

18
cloudkitty/tests/test_policy.py
View File

@ -16,9 +16,9 @@ import os.path
from oslo_config import cfg
from oslo_config import fixture as config_fixture
from oslo_context import context
from oslo_policy import policy as oslo_policy
from cloudkitty.common import context
from cloudkitty.common import policy
from cloudkitty import tests
from cloudkitty import utils
@ -31,9 +31,13 @@ class PolicyFileTestCase(tests.TestCase):
def setUp(self):
super(PolicyFileTestCase, self).setUp()
self.context = context.RequestContext('fake', 'fake', roles=['member'])
self.target = {}
self.fixture = self.useFixture(config_fixture.Config(CONF))
self.context = context.RequestContext(
user_id='fake',
project_id='fake',
roles=['member'],
is_admin=False)
self.target = {}
self.addCleanup(policy.reset)
CONF(args=[], project='cloudkitty', default_config_files=[])
@ -78,8 +82,8 @@ class PolicyTestCase(tests.TestCase):
policy.reset()
policy.init()
policy._ENFORCER.register_defaults(rules)
self.context = context.RequestContext('fake',
'fake',
self.context = context.RequestContext(user_id='fake',
project_id='fake',
roles=['member'])
self.target = {}
self.addCleanup(policy.reset)
@ -116,8 +120,8 @@ class PolicyTestCase(tests.TestCase):
def test_ignore_case_role_check(self):
lowercase_action = "test:lowercase_admin"
uppercase_action = "test:uppercase_admin"
admin_context = context.RequestContext('admin',
'fake',
admin_context = context.RequestContext(user_id='admin',
project_id='fake',
roles=['AdMiN'])
policy.authorize(admin_context, lowercase_action, self.target)
policy.authorize(admin_context, uppercase_action, self.target)

2
doc/source/_static/cloudkitty.policy.yaml.sample
View File

@ -2,7 +2,7 @@
#"context_is_admin": "role:admin"
#
#"admin_or_owner": "is_admin:True or tenant:%(tenant_id)s"
#"admin_or_owner": "is_admin:True or project_id:%(project_id)s"
#
#"default": ""