Don't check context.system_scope to check project scope
During first attempt of the Secure RBAC implementation [1] to
function model_query_scope_is_project() there was added check
which was checking if context.system_scope is "all" in case when
scope enforcement was enabled. It was like that because that meant
that it is SYSTEM_* persona context (e.g. SYSTEM_ADMIN).
This is not needed now as later it was agreed to have only one ADMIN
user which will still behave like old, "legacy" ADMIN user.
[1] https://review.opendev.org/c/openstack/neutron-lib/+/781075
Conflicts:
neutron_lib/db/utils.py
neutron_lib/tests/unit/db/test_utils.py
Closes-bug: #1996150
Change-Id: If3a97c4d3a0f4cb6b4d06434f74cbe9d933a07a4
(cherry picked from commit 91759b17ea)
This commit is contained in:
Slawek Kaplonski
parent
0fbbc3c0ab
commit
0dadfca1ff
|
|
@ -12,7 +12,6 @@
|
|||
|
||||
import functools
|
||||
|
||||
from oslo_config import cfg
|
||||
from oslo_db import exception as db_exc
|
||||
from oslo_utils import excutils
|
||||
import sqlalchemy
|
||||
|
|
@ -168,24 +167,16 @@ def model_query_scope_is_project(context, model):
|
|||
has a project_id. False otherwise.
|
||||
"""
|
||||
if not hasattr(model, 'project_id'):
|
||||
# If model don't have project_id, there is no need to scope query to
|
||||
# If model doesn't have project_id, there is no need to scope query to
|
||||
# just one project
|
||||
return False
|
||||
if context.is_advsvc:
|
||||
# For context which has 'advanced-service' rights the
|
||||
# query will not be scoped to a single project_id
|
||||
return False
|
||||
# TODO(slaweq): Remove that old is_admin check and always check scopes
|
||||
# when old, deprecated rules will be removed and only rules with new
|
||||
# personas will be supported
|
||||
if cfg.CONF.oslo_policy.enforce_new_defaults:
|
||||
# Unless a context is a system_scope token, query should be scoped to a
|
||||
# single project_id
|
||||
return context.system_scope != 'all'
|
||||
else:
|
||||
# Unless context has 'admin' rights the
|
||||
# query will be scoped to a single project_id
|
||||
return not context.is_admin
|
||||
# Unless context has 'admin' rights the
|
||||
# query will be scoped to a single project_id
|
||||
return not context.is_admin
|
||||
|
||||
|
||||
def model_query(context, model):
|
||||
|
|
|
|||
|
|
@ -40,7 +40,11 @@ class FakeRouter(declarative.declarative_base(cls=models.ModelBase)):
|
|||
gw_port = orm.relationship(FakePort, lazy='joined')
|
||||
|
||||
|
||||
class TestUtils(base.BaseTestCase):
|
||||
class TestUtilsLegacyPolicies(base.BaseTestCase):
|
||||
|
||||
def setUp(self):
|
||||
cfg.CONF.set_override('enforce_scope', False, group='oslo_policy')
|
||||
super().setUp()
|
||||
|
||||
def test_get_sort_dirs(self):
|
||||
sorts = [(1, True), (2, False), (3, True)]
|
||||
|
|
@ -90,9 +94,7 @@ class TestUtils(base.BaseTestCase):
|
|||
utils.resource_fields(r, ['name'])
|
||||
mock_populate.assert_called_once_with({'name': 'n'})
|
||||
|
||||
def test_model_query_scope_is_project_admin_old_defaults(self):
|
||||
cfg.CONF.set_override(
|
||||
'enforce_new_defaults', False, group='oslo_policy')
|
||||
def test_model_query_scope_is_project_admin(self):
|
||||
ctx = context.Context(
|
||||
project_id='some project',
|
||||
is_admin=True,
|
||||
|
|
@ -107,9 +109,7 @@ class TestUtils(base.BaseTestCase):
|
|||
self.assertFalse(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
|
||||
def test_model_query_scope_is_project_advsvc_old_defaults(self):
|
||||
cfg.CONF.set_override(
|
||||
'enforce_new_defaults', False, group='oslo_policy')
|
||||
def test_model_query_scope_is_project_advsvc(self):
|
||||
ctx = context.Context(
|
||||
project_id='some project',
|
||||
is_admin=False,
|
||||
|
|
@ -124,9 +124,7 @@ class TestUtils(base.BaseTestCase):
|
|||
self.assertFalse(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
|
||||
def test_model_query_scope_is_project_regular_user_old_defaults(self):
|
||||
cfg.CONF.set_override(
|
||||
'enforce_new_defaults', False, group='oslo_policy')
|
||||
def test_model_query_scope_is_project_regular_user(self):
|
||||
ctx = context.Context(
|
||||
project_id='some project',
|
||||
is_admin=False,
|
||||
|
|
@ -141,9 +139,7 @@ class TestUtils(base.BaseTestCase):
|
|||
self.assertFalse(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
|
||||
def test_model_query_scope_is_project_system_scope_old_defaults(self):
|
||||
cfg.CONF.set_override(
|
||||
'enforce_new_defaults', False, group='oslo_policy')
|
||||
def test_model_query_scope_is_project_system_scope(self):
|
||||
ctx = context.Context(system_scope='all')
|
||||
model = mock.Mock(project_id='project')
|
||||
|
||||
|
|
@ -155,68 +151,9 @@ class TestUtils(base.BaseTestCase):
|
|||
self.assertFalse(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
|
||||
def test_model_query_scope_is_project_admin_new_defaults(self):
|
||||
cfg.CONF.set_override(
|
||||
'enforce_new_defaults', True, group='oslo_policy')
|
||||
ctx = context.Context(
|
||||
project_id='some project',
|
||||
is_admin=True,
|
||||
is_advsvc=False)
|
||||
model = mock.Mock(project_id='project')
|
||||
|
||||
self.assertTrue(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
class TestUtilsWithScopeEnforcement(TestUtilsLegacyPolicies):
|
||||
|
||||
# Ensure that project_id isn't mocked
|
||||
del model.project_id
|
||||
self.assertFalse(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
|
||||
def test_model_query_scope_is_project_advsvc_new_defaults(self):
|
||||
cfg.CONF.set_override(
|
||||
'enforce_new_defaults', True, group='oslo_policy')
|
||||
ctx = context.Context(
|
||||
project_id='some project',
|
||||
is_admin=False,
|
||||
is_advsvc=True)
|
||||
model = mock.Mock(project_id='project')
|
||||
|
||||
self.assertFalse(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
|
||||
# Ensure that project_id isn't mocked
|
||||
del model.project_id
|
||||
self.assertFalse(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
|
||||
def test_model_query_scope_is_project_regular_user_new_defaults(self):
|
||||
cfg.CONF.set_override(
|
||||
'enforce_new_defaults', True, group='oslo_policy')
|
||||
ctx = context.Context(
|
||||
project_id='some project',
|
||||
is_admin=False,
|
||||
is_advsvc=False)
|
||||
model = mock.Mock(project_id='project')
|
||||
|
||||
self.assertTrue(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
|
||||
# Ensure that project_id isn't mocked
|
||||
del model.project_id
|
||||
self.assertFalse(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
|
||||
def test_model_query_scope_is_project_system_scope_new_defaults(self):
|
||||
cfg.CONF.set_override(
|
||||
'enforce_new_defaults', True, group='oslo_policy')
|
||||
ctx = context.Context(
|
||||
system_scope='all')
|
||||
model = mock.Mock(project_id='project')
|
||||
|
||||
self.assertFalse(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
|
||||
# Ensure that project_id isn't mocked
|
||||
del model.project_id
|
||||
self.assertFalse(
|
||||
utils.model_query_scope_is_project(ctx, model))
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
cfg.CONF.set_override('enforce_scope', True, group='oslo_policy')
|
||||
|
|
|
|||
Loading…
Reference in New Issue