Improve database object access checking

When Mistral is configured with 'auth_enable=True', we need to check
the resource accessbility when operating db resources (i.e. resource
RBAC). Such code logic will spread across the db layer. The patch move
these logic to on single place.

Change-Id: Ia8509986c18b63ef4b338f1dffb935f3fed5ad06
This commit is contained in:
Lingxian Kong 2017-02-23 14:04:19 +13:00 committed by Renat Akhmerov
parent fb5961c07a
commit ce43812e90
3 changed files with 30 additions and 28 deletions

View File

@ -18,7 +18,9 @@ from oslo_db import exception as db_exc
from oslo_log import log as logging
from oslo_service import loopingcall
from mistral import context as ctx
from mistral import context
from mistral import exceptions as exc
from mistral.services import security
LOG = logging.getLogger(__name__)
@ -35,9 +37,9 @@ def _with_auth_context(auth_ctx, func, *args, **kw):
:param kw: Function keywork arguments.
:return: Function result.
"""
old_auth_ctx = ctx.ctx() if ctx.has_ctx() else None
old_auth_ctx = context.ctx() if context.has_ctx() else None
ctx.set_ctx(auth_ctx)
context.set_ctx(auth_ctx)
try:
return func(*args, **kw)
@ -48,7 +50,7 @@ def _with_auth_context(auth_ctx, func, *args, **kw):
raise e
finally:
ctx.set_ctx(old_auth_ctx)
context.set_ctx(old_auth_ctx)
def retry_on_deadlock(func):
@ -67,8 +69,26 @@ def retry_on_deadlock(func):
# auth context in the new thread that RetryDecorator spawns.
# In order to do that we need an additional helper function.
auth_ctx = ctx.ctx() if ctx.has_ctx() else None
auth_ctx = context.ctx() if context.has_ctx() else None
return _with_auth_context(auth_ctx, func, *args, **kw)
return decorate
def check_db_obj_access(db_obj):
"""Check accessbility to db object."""
ctx = context.ctx()
is_admin = ctx.is_admin
if not is_admin and db_obj.project_id != security.get_project_id():
raise exc.NotAllowedException(
"Can not access %s resource of other projects, ID: %s" %
(db_obj.__class__.__name__, db_obj.id)
)
if not is_admin and hasattr(db_obj, 'is_system') and db_obj.is_system:
raise exc.InvalidActionException(
"Can not modify a system %s resource, ID: %s" %
(db_obj.__class__.__name__, db_obj.id)
)

View File

@ -28,10 +28,10 @@ import sqlalchemy as sa
from sqlalchemy.ext.compiler import compiles
from sqlalchemy.sql.expression import Insert
from mistral import context as auth_ctx
from mistral.db.sqlalchemy import base as b
from mistral.db.sqlalchemy import model_base as mb
from mistral.db.sqlalchemy import sqlite_lock
from mistral.db import utils as mdb_utils
from mistral.db.v2.sqlalchemy import filters as db_filters
from mistral.db.v2.sqlalchemy import models
from mistral import exceptions as exc
@ -491,18 +491,8 @@ def create_workflow_definition(values, session=None):
@b.session_aware()
def update_workflow_definition(identifier, values, session=None):
wf_def = get_workflow_definition(identifier, insecure=True)
ctx = auth_ctx.ctx()
if not ctx.is_admin and wf_def.project_id != security.get_project_id():
raise exc.NotAllowedException(
"Can not update workflow of other tenants. "
"[workflow_identifier=%s]" % identifier
)
if not ctx.is_admin and wf_def.is_system:
raise exc.InvalidActionException(
"Attempt to modify a system workflow: %s" % identifier
)
mdb_utils.check_db_obj_access(wf_def)
if wf_def.scope == 'public' and values['scope'] == 'private':
# Check cron triggers.
@ -546,15 +536,7 @@ def create_or_update_workflow_definition(name, values, session=None):
def delete_workflow_definition(identifier, session=None):
wf_def = get_workflow_definition(identifier)
if wf_def.project_id != security.get_project_id():
raise exc.NotAllowedException(
"Can not delete workflow of other users. [workflow_identifier=%s]"
% identifier
)
if wf_def.is_system:
msg = "Attempt to delete a system workflow: %s" % identifier
raise exc.DataAccessException(msg)
mdb_utils.check_db_obj_access(wf_def)
cron_triggers = get_cron_triggers(insecure=True, workflow_id=wf_def.id)
if cron_triggers:

View File

@ -236,7 +236,7 @@ class TestWorkflowsController(base.APITest):
self.assertEqual(400, resp.status_int)
self.assertIn(
"Attempt to modify a system workflow",
"Can not modify a system",
resp.body.decode()
)
@ -395,7 +395,7 @@ class TestWorkflowsController(base.APITest):
self.assertEqual(400, resp.status_int)
self.assertIn(
"Attempt to delete a system workflow",
"Can not modify a system",
resp.body.decode()
)