openstack
/
senlin
Code Issues Proposed changes

Enforce project_safe checking for admin users 

Admin users are designed to retrieve objects from all projects even
take actions on them, this will cause a lot problems, for example:
1. name_unique checking issue:
https://bugs.launchpad.net/senlin/+bug/1652742
2. cluster-node-add, admin user can add projectB's node to projectA's
  cluster
Admin users should respect the project concepts as well, this patch
enforces project_safe checking fro admin users. And as for some actions
that admin users should be authorized to retrieve things from all projects,
will be added in following patches.

Change-Id: I7103efabccbb4d10c2572e2df51a38ade009df84
This commit is contained in:
lvdongbing 2016-12-29 06:59:37 -05:00
parent a79fa034b0
commit 4d70b88d33
13 changed files with 63 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
senlin/db/sqlalchemy/api.py
View File

@ -82,7 +82,7 @@ def query_by_short_id(context, model, short_id, project_safe=True):
q = model_query(context, model)
q = q.filter(model.id.like('%s%%' % short_id))
if not context.is_admin and project_safe:
if project_safe:
q = q.filter_by(project=context.project)
if q.count() == 1:
@ -97,7 +97,7 @@ def query_by_name(context, model, name, project_safe=True):
q = model_query(context, model)
q = q.filter_by(name=name)
if not context.is_admin and project_safe:
if project_safe:
q = q.filter_by(project=context.project)
if q.count() == 1:
@ -123,7 +123,7 @@ def cluster_get(context, cluster_id, project_safe=True):
if cluster is None:
return None
if not context.is_admin and project_safe:
if project_safe:
if context.project != cluster.project:
return None
return cluster
@ -227,7 +227,7 @@ def node_get(context, node_id, project_safe=True):
if not node:
return None
if not context.is_admin and project_safe:
if project_safe:
if context.project != node.project:
return None
@ -517,7 +517,7 @@ def policy_get(context, policy_id, project_safe=True):
if policy is None:
return None
if not context.is_admin and project_safe:
if project_safe:
if context.project != policy.project:
return None
@ -737,7 +737,7 @@ def profile_get(context, profile_id, project_safe=True):
if profile is None:
return None
if not context.is_admin and project_safe:
if project_safe:
if context.project != profile.project:
return None
@ -850,7 +850,7 @@ def event_create(context, values):
def event_get(context, event_id, project_safe=True):
event = model_query(context, models.Event).get(event_id)
if not context.is_admin and project_safe and event is not None:
if project_safe and event is not None:
if event.project != context.project:
return None
@ -887,7 +887,7 @@ def event_get_all(context, limit=None, marker=None, sort=None, filters=None,
def event_count_by_cluster(context, cluster_id, project_safe=True):
query = model_query(context, models.Event)
if not context.is_admin and project_safe:
if project_safe:
query = query.filter_by(project=context.project)
count = query.filter_by(cluster_id=cluster_id).count()
@ -899,7 +899,7 @@ def event_get_all_by_cluster(context, cluster_id, limit=None, marker=None,
query = model_query(context, models.Event)
query = query.filter_by(cluster_id=cluster_id)
if not context.is_admin and project_safe:
if project_safe:
query = query.filter_by(project=context.project)
return _event_filter_paginate_query(context, query, filters=filters,
@ -910,7 +910,7 @@ def event_prune(context, cluster_id, project_safe=True):
with session_for_write() as session:
query = session.query(models.Event).with_for_update()
query = query.filter_by(cluster_id=cluster_id)
if not context.is_admin and project_safe:
if project_safe:
query = query.filter_by(project=context.project)
return query.delete(synchronize_session='fetch')
@ -941,7 +941,7 @@ def action_get(context, action_id, project_safe=True, refresh=False):
if action is None:
return None
if not context.is_admin and project_safe:
if project_safe:
if action.project != context.project:
return None
@ -1245,7 +1245,7 @@ def receiver_get(context, receiver_id, project_safe=True):
if not receiver:
return None
if not context.is_admin and project_safe:
if project_safe:
if context.project != receiver.project:
return None

7
senlin/engine/actions/base.py
View File

@ -215,7 +215,7 @@ class Action(object):
return cls(obj.target, obj.action, context, **kwargs)
@classmethod
def load(cls, context, action_id=None, db_action=None):
def load(cls, context, action_id=None, db_action=None, project_safe=True):
"""Retrieve an action from database.
:param context: Instance of request context.
@ -224,7 +224,8 @@ class Action(object):
:return: A `Action` object instance.
"""
if db_action is None:
db_action = ao.Action.get(context, action_id)
db_action = ao.Action.get(context, action_id,
project_safe=project_safe)
if db_action is None:
raise exception.ResourceNotFound(type='action', id=action_id)
@ -494,7 +495,7 @@ def ActionProc(context, action_id):
'''Action process.'''
# Step 1: materialize the action object
action = Action.load(context, action_id=action_id)
action = Action.load(context, action_id=action_id, project_safe=False)
if action is None:
LOG.error(_LE('Action "%s" could not be found.'), action_id)
return False

9
senlin/engine/scheduler.py
View File

@ -141,17 +141,20 @@ class ThreadGroupManager(object):
def cancel_action(self, action_id):
'''Cancel an action execution progress.'''
action = action_mod.Action.load(self.db_session, action_id)
action = action_mod.Action.load(self.db_session, action_id,
project_safe=False)
action.signal(action.SIG_CANCEL)
def suspend_action(self, action_id):
'''Suspend an action execution progress.'''
action = action_mod.Action.load(self.db_session, action_id)
action = action_mod.Action.load(self.db_session, action_id,
project_safe=False)
action.signal(action.SIG_SUSPEND)
def resume_action(self, action_id):
'''Resume an action execution progress.'''
action = action_mod.Action.load(self.db_session, action_id)
action = action_mod.Action.load(self.db_session, action_id,
project_safe=False)
action.signal(action.SIG_RESUME)
def add_timer(self, interval, func, *args, **kwargs):

8
senlin/engine/service.py
View File

@ -2189,7 +2189,13 @@ class EngineService(service.Service):
:return: A dictionary containing the details about a receiver or
an exception `ResourceNotFound` if no matching object found.
"""
receiver = receiver_obj.Receiver.find(ctx, req.identity)
# NOTE: Temporary code to make tempest tests about webhook_trigger
# pass, will remove in latter patches.
kwargs = {}
if ctx.is_admin is True:
kwargs['project_safe'] = False
receiver = receiver_obj.Receiver.find(ctx, req.identity, **kwargs)
return receiver.to_dict()
@request_context

2
senlin/tests/unit/db/test_action_api.py
View File

@ -101,6 +101,8 @@ class DBAPIActionTest(base.SenlinTestCase):
action = _create_action(self.ctx)
new_ctx = utils.dummy_context(project='another-project', is_admin=True)
retobj = db_api.action_get(new_ctx, action.id, project_safe=True)
self.assertIsNone(retobj)
retobj = db_api.action_get(new_ctx, action.id, project_safe=False)
self.assertIsNotNone(retobj)
def test_action_acquire_random_ready(self):

3
senlin/tests/unit/db/test_cluster_api.py
View File

@ -80,6 +80,9 @@ class DBAPIClusterTest(base.SenlinTestCase):
is_admin=True)
ret_cluster = db_api.cluster_get(admin_ctx, cluster.id,
project_safe=True)
self.assertIsNone(ret_cluster)
ret_cluster = db_api.cluster_get(admin_ctx, cluster.id,
project_safe=False)
self.assertEqual(cluster.id, ret_cluster.id)
self.assertEqual('db_test_cluster_name', ret_cluster.name)

8
senlin/tests/unit/db/test_event_api.py
View File

@ -102,6 +102,8 @@ class DBAPIEventTest(base.SenlinTestCase):
admin_ctx = utils.dummy_context(project='a-different-project',
is_admin=True)
res = db_api.event_get(admin_ctx, event.id, project_safe=True)
self.assertIsNone(res)
res = db_api.event_get(admin_ctx, event.id, project_safe=False)
self.assertIsNotNone(res)
def test_event_get_by_short_id(self):
@ -352,6 +354,9 @@ class DBAPIEventTest(base.SenlinTestCase):
is_admin=True)
events = db_api.event_get_all_by_cluster(admin_ctx, cluster1.id,
project_safe=True)
self.assertEqual(0, len(events))
events = db_api.event_get_all_by_cluster(admin_ctx, cluster1.id,
project_safe=False)
self.assertEqual(2, len(events))
def test_event_count_all_by_cluster(self):
@ -418,6 +423,9 @@ class DBAPIEventTest(base.SenlinTestCase):
res = db_api.event_count_by_cluster(admin_ctx, cluster1.id,
project_safe=True)
self.assertEqual(0, res)
res = db_api.event_count_by_cluster(admin_ctx, cluster1.id,
project_safe=False)
self.assertEqual(1, res)
def test_event_get_all_filtered(self):

5
senlin/tests/unit/db/test_node_api.py
View File

@ -83,6 +83,8 @@ class DBAPINodeTest(base.SenlinTestCase):
admin_ctx = utils.dummy_context(project='a_different_project',
is_admin=True)
node = db_api.node_get(admin_ctx, res.id, project_safe=True)
self.assertIsNone(node)
node = db_api.node_get(admin_ctx, res.id, project_safe=False)
self.assertIsNotNone(node)
def test_node_get_by_name(self):
@ -150,6 +152,9 @@ class DBAPINodeTest(base.SenlinTestCase):
is_admin=True)
res = db_api.node_get_by_short_id(admin_ctx, node_id[:11],
project_safe=True)
self.assertIsNone(res)
res = db_api.node_get_by_short_id(admin_ctx, node_id[:11],
project_safe=False)
self.assertIsNotNone(res)
def test_node_get_all(self):

2
senlin/tests/unit/db/test_policy_api.py
View File

@ -90,6 +90,8 @@ class DBAPIPolicyTest(base.SenlinTestCase):
admin_ctx = utils.dummy_context(project='a-different-project',
is_admin=True)
res = db_api.policy_get(admin_ctx, policy.id, project_safe=True)
self.assertIsNone(res)
res = db_api.policy_get(admin_ctx, policy.id, project_safe=False)
self.assertIsNotNone(res)
def test_policy_get_not_found(self):

2
senlin/tests/unit/db/test_profile_api.py
View File

@ -58,6 +58,8 @@ class DBAPIProfileTest(base.SenlinTestCase):
admin_ctx = utils.dummy_context(project='a-different-project',
is_admin=True)
res = db_api.profile_get(admin_ctx, profile.id, project_safe=True)
self.assertIsNone(res)
res = db_api.profile_get(admin_ctx, profile.id, project_safe=False)
self.assertIsNotNone(res)
def test_profile_get_not_found(self):

2
senlin/tests/unit/db/test_receiver_api.py
View File

@ -85,6 +85,8 @@ class DBAPIReceiverTest(base.SenlinTestCase):
r = self._create_receiver(self.ctx)
res = db_api.receiver_get(admin_ctx, r.id, project_safe=True)
self.assertIsNone(res)
res = db_api.receiver_get(admin_ctx, r.id, project_safe=False)
self.assertIsNotNone(res)
def test_receiver_get_by_short_id(self):

6
senlin/tests/unit/engine/actions/test_action_base.py
View File

@ -833,7 +833,8 @@ class ActionProcTest(base.SenlinTestCase):
res = ab.ActionProc(self.ctx, 'ACTION_ID')
self.assertTrue(res)
mock_load.assert_called_once_with(self.ctx, action_id='ACTION_ID')
mock_load.assert_called_once_with(self.ctx, action_id='ACTION_ID',
project_safe=False)
mock_event_info.assert_called_once_with(action, 'start')
mock_status.assert_called_once_with(action.RES_OK, 'BIG SUCCESS')
@ -851,6 +852,7 @@ class ActionProcTest(base.SenlinTestCase):
res = ab.ActionProc(self.ctx, 'ACTION')
self.assertFalse(res)
mock_load.assert_called_once_with(self.ctx, action_id='ACTION')
mock_load.assert_called_once_with(self.ctx, action_id='ACTION',
project_safe=False)
mock_info.assert_called_once_with(action, 'start')
mock_status.assert_called_once_with(action.RES_ERROR, 'Boom!')

9
senlin/tests/unit/engine/test_scheduler.py
View File

@ -188,7 +188,8 @@ class SchedulerTest(base.SenlinTestCase):
tgm = scheduler.ThreadGroupManager()
tgm.cancel_action('action0123')
mock_load.assert_called_once_with(tgm.db_session, 'action0123')
mock_load.assert_called_once_with(tgm.db_session, 'action0123',
project_safe=False)
mock_action.signal.assert_called_once_with(mock_action.SIG_CANCEL)
def test_suspend_action(self):
@ -198,7 +199,8 @@ class SchedulerTest(base.SenlinTestCase):
tgm = scheduler.ThreadGroupManager()
tgm.suspend_action('action0123')
mock_load.assert_called_once_with(tgm.db_session, 'action0123')
mock_load.assert_called_once_with(tgm.db_session, 'action0123',
project_safe=False)
mock_action.signal.assert_called_once_with(mock_action.SIG_SUSPEND)
def test_resume_action(self):
@ -208,7 +210,8 @@ class SchedulerTest(base.SenlinTestCase):
tgm = scheduler.ThreadGroupManager()
tgm.resume_action('action0123')
mock_load.assert_called_once_with(tgm.db_session, 'action0123')
mock_load.assert_called_once_with(tgm.db_session, 'action0123',
project_safe=False)
mock_action.signal.assert_called_once_with(mock_action.SIG_RESUME)
def test_add_timer(self):