Add support for Oslo Policies to Trove

The Oslo Policy library provides support for RBAC policy
enforcement across all OpenStack services.

Update the devstack plugin to copy the default policy file
over to /etc/trove in the gate environments.

Note: Not adding a rule for 'reset-password' instance
action as that API was discontinued years ago
and is now just waiting for removal (Bug: 1645866).

DocImpact
Co-Authored-By: Ali Adil <aadil@tesora.com>

Change-Id: Ic443a4c663301840406cad537159eab7b0b5ed1c
Implements: blueprint trove-policy

devstack/plugin.sh

@@ -110,6 +110,9 @@ function configure_trove {
     # Copy api-paste file over to the trove conf dir
     cp $TROVE_LOCAL_API_PASTE_INI $TROVE_API_PASTE_INI
 
+    # Copy the default policy file over to the trove conf dir
+    cp $TROVE_LOCAL_POLICY_JSON $TROVE_POLICY_JSON
+
     # (Re)create trove conf files
     rm -f $TROVE_CONF
     rm -f $TROVE_TASKMANAGER_CONF

devstack/settings

@@ -21,9 +21,11 @@ TROVE_TASKMANAGER_CONF=${TROVE_TASKMANAGER_CONF:-${TROVE_CONF_DIR}/trove-taskman
 TROVE_CONDUCTOR_CONF=${TROVE_CONDUCTOR_CONF:-${TROVE_CONF_DIR}/trove-conductor.conf}
 TROVE_GUESTAGENT_CONF=${TROVE_GUESTAGENT_CONF:-${TROVE_CONF_DIR}/trove-guestagent.conf}
 TROVE_API_PASTE_INI=${TROVE_API_PASTE_INI:-${TROVE_CONF_DIR}/api-paste.ini}
+TROVE_POLICY_JSON=${TROVE_POLICY_JSON:-${TROVE_CONF_DIR}/policy.json}
 
 TROVE_LOCAL_CONF_DIR=${TROVE_LOCAL_CONF_DIR:-${TROVE_DIR}/etc/trove}
 TROVE_LOCAL_API_PASTE_INI=${TROVE_LOCAL_API_PASTE_INI:-${TROVE_LOCAL_CONF_DIR}/api-paste.ini}
+TROVE_LOCAL_POLICY_JSON=${TROVE_LOCAL_POLICY_JSON:-${TROVE_LOCAL_CONF_DIR}/policy.json}
 TROVE_AUTH_CACHE_DIR=${TROVE_AUTH_CACHE_DIR:-/var/cache/trove}
 TROVE_DATASTORE_TYPE=${TROVE_DATASTORE_TYPE:-"mysql"}
 TROVE_DATASTORE_VERSION=${TROVE_DATASTORE_VERSION:-"5.6"}

etc/trove/policy.json

@@ -0,0 +1,96 @@
+{
+    "admin": "role:admin or is_admin:True",
+    "admin_or_owner": "rule:admin or tenant:%(tenant)s",
+    "default": "rule:admin_or_owner",
+
+    "instance:create": "rule:admin_or_owner",
+    "instance:delete": "rule:admin_or_owner",
+    "instance:force_delete": "rule:admin_or_owner",
+    "instance:index": "rule:admin_or_owner",
+    "instance:show": "rule:admin_or_owner",
+    "instance:update": "rule:admin_or_owner",
+    "instance:edit": "rule:admin_or_owner",
+    "instance:restart": "rule:admin_or_owner",
+    "instance:resize_volume": "rule:admin_or_owner",
+    "instance:resize_flavor": "rule:admin_or_owner",
+    "instance:reset_status": "rule:admin",
+    "instance:promote_to_replica_source": "rule:admin_or_owner",
+    "instance:eject_replica_source": "rule:admin_or_owner",
+    "instance:configuration": "rule:admin_or_owner",
+    "instance:guest_log_list": "rule:admin_or_owner",
+    "instance:backups": "rule:admin_or_owner",
+    "instance:module_list": "rule:admin_or_owner",
+    "instance:module_apply": "rule:admin_or_owner",
+    "instance:module_remove": "rule:admin_or_owner",
+
+    "instance:extension:root:create": "rule:admin_or_owner",
+    "instance:extension:root:delete": "rule:admin_or_owner",
+    "instance:extension:root:index": "rule:admin_or_owner",
+
+    "instance:extension:user:create": "rule:admin_or_owner",
+    "instance:extension:user:delete": "rule:admin_or_owner",
+    "instance:extension:user:index": "rule:admin_or_owner",
+    "instance:extension:user:show": "rule:admin_or_owner",
+    "instance:extension:user:update": "rule:admin_or_owner",
+    "instance:extension:user:update_all": "rule:admin_or_owner",
+
+    "instance:extension:user_access:update": "rule:admin_or_owner",
+    "instance:extension:user_access:delete": "rule:admin_or_owner",
+    "instance:extension:user_access:index": "rule:admin_or_owner",
+
+    "instance:extension:database:create": "rule:admin_or_owner",
+    "instance:extension:database:delete": "rule:admin_or_owner",
+    "instance:extension:database:index": "rule:admin_or_owner",
+    "instance:extension:database:show": "rule:admin_or_owner",
+
+    "cluster:create": "rule:admin_or_owner",
+    "cluster:delete": "rule:admin_or_owner",
+    "cluster:force_delete": "rule:admin_or_owner",
+    "cluster:index": "rule:admin_or_owner",
+    "cluster:show": "rule:admin_or_owner",
+    "cluster:show_instance": "rule:admin_or_owner",
+    "cluster:action": "rule:admin_or_owner",
+    "cluster:reset-status": "rule:admin",
+
+    "cluster:extension:root:create": "rule:admin_or_owner",
+    "cluster:extension:root:delete": "rule:admin_or_owner",
+    "cluster:extension:root:index": "rule:admin_or_owner",
+
+    "backup:create": "rule:admin_or_owner",
+    "backup:delete": "rule:admin_or_owner",
+    "backup:index": "rule:admin_or_owner",
+    "backup:show": "rule:admin_or_owner",
+
+    "configuration:create": "rule:admin_or_owner",
+    "configuration:delete": "rule:admin_or_owner",
+    "configuration:index": "rule:admin_or_owner",
+    "configuration:show": "rule:admin_or_owner",
+    "configuration:instances": "rule:admin_or_owner",
+    "configuration:update": "rule:admin_or_owner",
+    "configuration:edit": "rule:admin_or_owner",
+
+    "configuration-parameter:index": "rule:admin_or_owner",
+    "configuration-parameter:show": "rule:admin_or_owner",
+    "configuration-parameter:index_by_version": "rule:admin_or_owner",
+    "configuration-parameter:show_by_version": "rule:admin_or_owner",
+
+    "datastore:index": "",
+    "datastore:show": "",
+    "datastore:version_show": "",
+    "datastore:version_show_by_uuid": "",
+    "datastore:version_index": "",
+    "datastore:list_associated_flavors": "",
+    "datastore:list_associated_volume_types": "",
+
+    "flavor:index": "",
+    "flavor:show": "",
+
+    "limits:index": "rule:admin_or_owner",
+
+    "module:create": "rule:admin_or_owner",
+    "module:delete": "rule:admin_or_owner",
+    "module:index": "rule:admin_or_owner",
+    "module:show": "rule:admin_or_owner",
+    "module:instances": "rule:admin_or_owner",
+    "module:update": "rule:admin_or_owner"
+}

releasenotes/notes/use-oslo-policy-bbd1b911e6487c36.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - Add RBAC (role-based access control)
+    enforcement on all trove APIs.
+    Allows to define a role-based access rule
+    for every trove API call
+    (rule definitions are available in
+     /etc/trove/policy.json).

requirements.txt

@@ -47,3 +47,4 @@ oslo.db!=4.13.1,!=4.13.2,>=4.11.0 # Apache-2.0
 enum34;python_version=='2.7' or python_version=='2.6' or python_version=='3.3' # BSD
 xmltodict>=0.10.1 # MIT
 pycrypto>=2.6 # Public Domain
+oslo.policy>=1.17.0 # Apache-2.0

trove/backup/service.py

@@ -22,6 +22,7 @@ from trove.common.i18n import _
 from trove.common import notification
 from trove.common.notification import StartNotification
 from trove.common import pagination
+from trove.common import policy
 from trove.common import wsgi
 
 LOG = logging.getLogger(__name__)
@@ -40,6 +41,7 @@ class BackupController(wsgi.Controller):
         LOG.debug("Listing backups for tenant %s" % tenant_id)
         datastore = req.GET.get('datastore')
         context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'backup:index')
         backups, marker = Backup.list(context, datastore)
         view = views.BackupViews(backups)
         paged = pagination.SimplePaginatedDataView(req.url, 'backups', view,
@@ -52,11 +54,14 @@ class BackupController(wsgi.Controller):
                   % (tenant_id, id))
         context = req.environ[wsgi.CONTEXT_KEY]
         backup = Backup.get_by_id(context, id)
+        policy.authorize_on_target(context, 'backup:show',
+                                   {'tenant': backup.tenant_id})
         return wsgi.Result(views.BackupView(backup).data(), 200)
 
     def create(self, req, body, tenant_id):
         LOG.info(_("Creating a backup for tenant %s"), tenant_id)
         context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'backup:create')
         data = body['backup']
         instance = data['instance']
         name = data['name']
@@ -76,6 +81,9 @@ class BackupController(wsgi.Controller):
                    'ID: %(backup_id)s') %
                  {'tenant_id': tenant_id, 'backup_id': id})
         context = req.environ[wsgi.CONTEXT_KEY]
+        backup = Backup.get_by_id(context, id)
+        policy.authorize_on_target(context, 'backup:delete',
+                                   {'tenant': backup.tenant_id})
         context.notification = notification.DBaaSBackupDelete(context,
                                                               request=req)
         with StartNotification(context, backup_id=id):

trove/cluster/service.py

@@ -25,6 +25,7 @@ from trove.common.i18n import _
 from trove.common import notification
 from trove.common.notification import StartNotification
 from trove.common import pagination
+from trove.common import policy
 from trove.common import utils
 from trove.common import wsgi
 from trove.datastore import models as datastore_models
@@ -39,6 +40,11 @@ class ClusterController(wsgi.Controller):
     """Controller for cluster functionality."""
     schemas = apischema.cluster.copy()
 
+    @classmethod
+    def authorize_cluster_action(cls, context, cluster_rule_name, cluster):
+        policy.authorize_on_target(context, 'cluster:%s' % cluster_rule_name,
+                                   {'tenant': cluster.tenant_id})
+
     @classmethod
     def get_action_schema(cls, body, action_schema):
         action_type = list(body.keys())[0]
@@ -58,15 +64,25 @@ class ClusterController(wsgi.Controller):
                   {"req": req, "id": id, "tenant_id": tenant_id})
         if not body:
             raise exception.BadRequest(_("Invalid request body."))
+
         if len(body) != 1:
             raise exception.BadRequest(_("Action request should have exactly"
                                          " one action specified in body"))
         context = req.environ[wsgi.CONTEXT_KEY]
         cluster = models.Cluster.load(context, id)
+        if ('reset-status' in body and
+                'force_delete' not in body['reset-status']):
+            self.authorize_cluster_action(context, 'reset-status', cluster)
+        elif ('reset-status' in body and
+                'force_delete' in body['reset-status']):
+            self.authorize_cluster_action(context, 'force_delete', cluster)
+        else:
+            self.authorize_cluster_action(context, 'action', cluster)
         cluster.action(context, req, *next(iter(body.items())))
 
         view = views.load_view(cluster, req=req, load_servers=False)
         wsgi_result = wsgi.Result(view.data(), 202)
+
         return wsgi_result
 
     def show(self, req, tenant_id, id):
@@ -77,6 +93,7 @@ class ClusterController(wsgi.Controller):
 
         context = req.environ[wsgi.CONTEXT_KEY]
         cluster = models.Cluster.load(context, id)
+        self.authorize_cluster_action(context, 'show', cluster)
         return wsgi.Result(views.load_view(cluster, req=req).data(), 200)
 
     def show_instance(self, req, tenant_id, cluster_id, instance_id):
@@ -92,6 +109,7 @@ class ClusterController(wsgi.Controller):
 
         context = req.environ[wsgi.CONTEXT_KEY]
         cluster = models.Cluster.load(context, cluster_id)
+        self.authorize_cluster_action(context, 'show_instance', cluster)
         instance = models.Cluster.load_instance(context, cluster.id,
                                                 instance_id)
         return wsgi.Result(views.ClusterInstanceDetailView(
@@ -105,6 +123,7 @@ class ClusterController(wsgi.Controller):
 
         context = req.environ[wsgi.CONTEXT_KEY]
         cluster = models.Cluster.load(context, id)
+        self.authorize_cluster_action(context, 'delete', cluster)
         context.notification = notification.DBaaSClusterDelete(context,
                                                                request=req)
         with StartNotification(context, cluster_id=id):
@@ -118,9 +137,19 @@ class ClusterController(wsgi.Controller):
                                              "tenant_id": tenant_id})
 
         context = req.environ[wsgi.CONTEXT_KEY]
+
+        # This theoretically allows the Admin tenant list clusters for
+        # only one particular tenant as opposed to listing all clusters for
+        # for all tenants.
+        # * As far as I can tell this is the only call which actually uses the
+        #   passed-in 'tenant_id' for anything.
         if not context.is_admin and context.tenant != tenant_id:
             raise exception.TroveOperationAuthError(tenant_id=context.tenant)
 
+        # The rule checks that the currently authenticated tenant can perform
+        # the 'cluster-list' action.
+        policy.authorize_on_tenant(context, 'cluster:index')
+
         # load all clusters and instances for the tenant
         clusters, marker = models.Cluster.load_all(context, tenant_id)
         view = views.ClustersView(clusters, req=req)
@@ -134,6 +163,8 @@ class ClusterController(wsgi.Controller):
                   {"tenant_id": tenant_id, "req": req, "body": body})
 
         context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'cluster:create')
+
         name = body['cluster']['name']
         datastore_args = body['cluster'].get('datastore', {})
         datastore, datastore_version = (

trove/common/exception.py

@@ -236,11 +236,6 @@ class UnprocessableEntity(TroveError):
     message = _("Unable to process the contained request.")
 
 
-class UnauthorizedRequest(TroveError):
-
-    message = _("Unauthorized request.")
-
-
 class CannotResizeToSameSize(TroveError):
 
     message = _("No change was requested in the size of the instance.")
@@ -309,6 +304,11 @@ class Forbidden(TroveError):
     message = _("User does not have admin privileges.")
 
 
+class PolicyNotAuthorized(Forbidden):
+
+    message = _("Policy doesn't allow %(action)s to be performed.")
+
+
 class InvalidModelError(TroveError):
 
     message = _("The following values are invalid: %(errors)s.")
@@ -538,6 +538,10 @@ class ModuleInvalid(Forbidden):
     message = _("The module is invalid: %(reason)s")
 
 
+class InstanceNotFound(NotFound):
+    message = _("Instance '%(instance)s' cannot be found.")
+
+
 class ClusterNotFound(NotFound):
     message = _("Cluster '%(cluster)s' cannot be found.")
 
@@ -622,3 +626,8 @@ class ImageNotFound(NotFound):
 class DatastoreVersionAlreadyExists(BadRequest):
 
     message = _("A datastore version with the name '%(name)s' already exists.")
+
+
+class LogAccessForbidden(Forbidden):
+
+    message = _("You must be admin to %(action)s log '%(log)s'.")

trove/common/policy.py

@@ -0,0 +1,260 @@
+# Copyright 2016 Tesora Inc.
+# 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_config import cfg
+from oslo_policy import policy
+
+from trove.common import exception as trove_exceptions
+
+CONF = cfg.CONF
+_ENFORCER = None
+
+
+base_rules = [
+    policy.RuleDefault(
+        'admin',
+        'role:admin or is_admin:True',
+        description='Must be an administrator.'),
+    policy.RuleDefault(
+        'admin_or_owner',
+        'rule:admin or tenant:%(tenant)s',
+        description='Must be an administrator or owner of the object.'),
+    policy.RuleDefault(
+        'default',
+        'rule:admin_or_owner',
+        description='Must be an administrator or owner of the object.')
+]
+
+instance_rules = [
+    policy.RuleDefault(
+        'instance:create', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:force_delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:index', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:show', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:update', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:edit', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:restart', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:resize_volume', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:resize_flavor', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:reset_status', 'rule:admin'),
+    policy.RuleDefault(
+        'instance:promote_to_replica_source', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:eject_replica_source', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:configuration', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:guest_log_list', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:backups', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:module_list', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:module_apply', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:module_remove', 'rule:admin_or_owner'),
+
+    policy.RuleDefault(
+        'instance:extension:root:create', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:root:delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:root:index', 'rule:admin_or_owner'),
+
+    policy.RuleDefault(
+        'instance:extension:user:create', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:user:delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:user:index', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:user:show', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:user:update', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:user:update_all', 'rule:admin_or_owner'),
+
+    policy.RuleDefault(
+        'instance:extension:user_access:update', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:user_access:delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:user_access:index', 'rule:admin_or_owner'),
+
+    policy.RuleDefault(
+        'instance:extension:database:create', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:database:delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:database:index', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'instance:extension:database:show', 'rule:admin_or_owner'),
+
+    policy.RuleDefault(
+        'cluster:create', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'cluster:delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'cluster:force_delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'cluster:index', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'cluster:show', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'cluster:show_instance', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'cluster:action', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'cluster:reset-status', 'rule:admin'),
+
+    policy.RuleDefault(
+        'cluster:extension:root:create', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'cluster:extension:root:delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'cluster:extension:root:index', 'rule:admin_or_owner'),
+
+    policy.RuleDefault(
+        'backup:create', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'backup:delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'backup:index', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'backup:show', 'rule:admin_or_owner'),
+
+    policy.RuleDefault(
+        'configuration:create', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'configuration:delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'configuration:index', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'configuration:show', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'configuration:instances', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'configuration:update', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'configuration:edit', 'rule:admin_or_owner'),
+
+    policy.RuleDefault(
+        'configuration-parameter:index', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'configuration-parameter:show', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'configuration-parameter:index_by_version', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'configuration-parameter:show_by_version', 'rule:admin_or_owner'),
+
+    policy.RuleDefault(
+        'datastore:index', ''),
+    policy.RuleDefault(
+        'datastore:show', ''),
+    policy.RuleDefault(
+        'datastore:version_show', ''),
+    policy.RuleDefault(
+        'datastore:version_show_by_uuid', ''),
+    policy.RuleDefault(
+        'datastore:version_index', ''),
+    policy.RuleDefault(
+        'datastore:list_associated_flavors', ''),
+    policy.RuleDefault(
+        'datastore:list_associated_volume_types', ''),
+
+    policy.RuleDefault(
+        'flavor:index', ''),
+    policy.RuleDefault(
+        'flavor:show', ''),
+
+    policy.RuleDefault(
+        'limits:index', 'rule:admin_or_owner'),
+
+    policy.RuleDefault(
+        'module:create', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'module:delete', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'module:index', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'module:show', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'module:instances', 'rule:admin_or_owner'),
+    policy.RuleDefault(
+        'module:update', 'rule:admin_or_owner'),
+]
+
+
+def get_enforcer():
+    global _ENFORCER
+    if not _ENFORCER:
+        _ENFORCER = policy.Enforcer(CONF)
+        _ENFORCER.register_defaults(base_rules)
+        _ENFORCER.register_defaults(instance_rules)
+    return _ENFORCER
+
+
+def authorize_on_tenant(context, rule):
+    return __authorize(context, rule, target=None)
+
+
+def authorize_on_target(context, rule, target):
+    if target:
+        return __authorize(context, rule, target=target)
+    raise trove_exceptions.TroveError(
+        "BUG: Target must not evaluate to False.")
+
+
+def __authorize(context, rule, target=None):
+    """Checks authorization of a rule against the target in this context.
+
+    * This function is not to be called directly.
+      Calling the function with a target that evaluates to None may
+      result in policy bypass.
+      Use 'authorize_on_*' calls instead.
+
+       :param context   Trove context.
+       :type context    Context.
+
+       :param rule:     The rule to evaluate.
+                        e.g. ``instance:create_instance``,
+                             ``instance:resize_volume``
+
+       :param target    As much information about the object being operated on
+                        as possible.
+                        For object creation (target=None) this should be a
+                        dictionary representing the location of the object
+                        e.g. ``{'project_id': context.project_id}``
+       :type target     dict
+
+       :raises:         :class:`PolicyNotAuthorized` if verification fails.
+
+    """
+    target = target or {'tenant': context.tenant}
+    return get_enforcer().authorize(
+        rule, target, context.to_dict(), do_raise=True,
+        exc=trove_exceptions.PolicyNotAuthorized, action=rule)

trove/common/wsgi.py

@@ -322,6 +322,8 @@ class Controller(object):
             exception.BackupTooLarge,
             exception.ModuleAccessForbidden,
             exception.ModuleAppliedToInstance,
+            exception.PolicyNotAuthorized,
+            exception.LogAccessForbidden,
         ],
         webob.exc.HTTPBadRequest: [
             exception.InvalidModelError,
@@ -548,7 +550,8 @@ class ContextMiddleware(base_wsgi.Middleware):
                                           is_admin=is_admin,
                                           limit=limits.get('limit'),
                                           marker=limits.get('marker'),
-                                          service_catalog=service_catalog)
+                                          service_catalog=service_catalog,
+                                          roles=roles)
         request.environ[CONTEXT_KEY] = context
 
     @classmethod

trove/configuration/service.py

@@ -25,6 +25,7 @@ from trove.common.i18n import _
 from trove.common import notification
 from trove.common.notification import StartNotification, EndNotification
 from trove.common import pagination
+from trove.common import policy
 from trove.common import wsgi
 from trove.configuration import models
 from trove.configuration.models import DBConfigurationParameter
@@ -41,9 +42,16 @@ class ConfigurationsController(wsgi.Controller):
 
     schemas = apischema.configuration
 
+    @classmethod
+    def authorize_config_action(cls, context, config_rule_name, config):
+        policy.authorize_on_target(
+            context, 'configuration:%s' % config_rule_name,
+            {'tenant': config.tenant_id})
+
     def index(self, req, tenant_id):
         context = req.environ[wsgi.CONTEXT_KEY]
         configs, marker = models.Configurations.load(context)
+        policy.authorize_on_tenant(context, 'configuration:index')
         view = views.ConfigurationsView(configs)
         paged = pagination.SimplePaginatedDataView(req.url, 'configurations',
                                                    view, marker)
@@ -54,6 +62,7 @@ class ConfigurationsController(wsgi.Controller):
                   % {"tenant": tenant_id, "id": id})
         context = req.environ[wsgi.CONTEXT_KEY]
         configuration = models.Configuration.load(context, id)
+        self.authorize_config_action(context, 'show', configuration)
         configuration_items = models.Configuration.load_items(context, id)
 
         configuration.instance_count = instances_models.DBInstance.find_all(
@@ -68,6 +77,7 @@ class ConfigurationsController(wsgi.Controller):
     def instances(self, req, tenant_id, id):
         context = req.environ[wsgi.CONTEXT_KEY]
         configuration = models.Configuration.load(context, id)
+        self.authorize_config_action(context, 'instances', configuration)
         instances = instances_models.DBInstance.find_all(
             tenant_id=context.tenant,
             configuration_id=configuration.id,
@@ -89,6 +99,7 @@ class ConfigurationsController(wsgi.Controller):
         LOG.debug("body : '%s'\n\n" % req)
 
         context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'configuration:create')
         context.notification = notification.DBaaSConfigurationCreate(
             context, request=req)
         name = body['configuration']['name']
@@ -137,10 +148,11 @@ class ConfigurationsController(wsgi.Controller):
         LOG.info(msg % {"tenant_id": tenant_id, "cfg_id": id})
 
         context = req.environ[wsgi.CONTEXT_KEY]
+        group = models.Configuration.load(context, id)
+        self.authorize_config_action(context, 'delete', group)
         context.notification = notification.DBaaSConfigurationDelete(
             context, request=req)
         with StartNotification(context, configuration_id=id):
-            group = models.Configuration.load(context, id)
             instances = instances_models.DBInstance.find_all(
                 tenant_id=context.tenant,
                 configuration_id=id,
@@ -157,6 +169,15 @@ class ConfigurationsController(wsgi.Controller):
 
         context = req.environ[wsgi.CONTEXT_KEY]
         group = models.Configuration.load(context, id)
+        # Note that changing the configuration group will also
+        # indirectly affect all the instances which attach it.
+        #
+        # The Trove instance itself won't be changed (the same group is still
+        # attached) but the configuration values will.
+        #
+        # The operator needs to keep this in mind when defining the related
+        # policies.
+        self.authorize_config_action(context, 'update', group)
 
         # if name/description are provided in the request body, update the
         # model with these values as well.
@@ -181,10 +202,11 @@ class ConfigurationsController(wsgi.Controller):
 
     def edit(self, req, body, tenant_id, id):
         context = req.environ[wsgi.CONTEXT_KEY]
+        group = models.Configuration.load(context, id)
+        self.authorize_config_action(context, 'edit', group)
         context.notification = notification.DBaaSConfigurationEdit(
             context, request=req)
         with StartNotification(context, configuration_id=id):
-            group = models.Configuration.load(context, id)
             items = self._configuration_items_list(group,
                                                    body['configuration'])
             models.Configuration.save(group, items)
@@ -329,7 +351,18 @@ class ConfigurationsController(wsgi.Controller):
 
 class ParametersController(wsgi.Controller):
 
+    @classmethod
+    def authorize_request(cls, req, rule_name):
+        """Parameters (configuration templates) bind to a datastore.
+        Datastores are not owned by any particular tenant so we only check
+        the current tenant is allowed to perform the action.
+        """
+        context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'configuration-parameter:%s'
+                                   % rule_name)
+
     def index(self, req, tenant_id, datastore, id):
+        self.authorize_request(req, 'index')
         ds, ds_version = ds_models.get_datastore_version(
             type=datastore, version=id)
         rules = models.DatastoreConfigurationParameters.load_parameters(
@@ -338,6 +371,7 @@ class ParametersController(wsgi.Controller):
                            200)
 
     def show(self, req, tenant_id, datastore, id, name):
+        self.authorize_request(req, 'show')
         ds, ds_version = ds_models.get_datastore_version(
             type=datastore, version=id)
         rule = models.DatastoreConfigurationParameters.load_parameter_by_name(
@@ -345,6 +379,7 @@ class ParametersController(wsgi.Controller):
         return wsgi.Result(views.ConfigurationParameterView(rule).data(), 200)
 
     def index_by_version(self, req, tenant_id, version):
+        self.authorize_request(req, 'index_by_version')
         ds_version = ds_models.DatastoreVersion.load_by_uuid(version)
         rules = models.DatastoreConfigurationParameters.load_parameters(
             ds_version.id)
@@ -352,6 +387,7 @@ class ParametersController(wsgi.Controller):
                            200)
 
     def show_by_version(self, req, tenant_id, version, name):
+        self.authorize_request(req, 'show_by_version')
         ds_models.DatastoreVersion.load_by_uuid(version)
         rule = models.DatastoreConfigurationParameters.load_parameter_by_name(
             version, name)

trove/datastore/service.py

@@ -16,6 +16,7 @@
 #    under the License.
 #
 
+from trove.common import policy
 from trove.common import wsgi
 from trove.datastore import models, views
 from trove.flavor import views as flavor_views
@@ -23,7 +24,16 @@ from trove.flavor import views as flavor_views
 
 class DatastoreController(wsgi.Controller):
 
+    @classmethod
+    def authorize_request(cls, req, rule_name):
+        """Datastores are not owned by any particular tenant so we only check
+        the current tenant is allowed to perform the action.
+        """
+        context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'datastore:%s' % rule_name)
+
     def show(self, req, tenant_id, id):
+        self.authorize_request(req, 'show')
         datastore = models.Datastore.load(id)
         datastore_versions = (models.DatastoreVersions.load(datastore.id))
         return wsgi.Result(views.
@@ -31,6 +41,7 @@ class DatastoreController(wsgi.Controller):
                                          req).data(), 200)
 
     def index(self, req, tenant_id):
+        self.authorize_request(req, 'index')
         context = req.environ[wsgi.CONTEXT_KEY]
         only_active = True
         if context.is_admin:
@@ -42,17 +53,20 @@ class DatastoreController(wsgi.Controller):
                                           req).data(), 200)
 
     def version_show(self, req, tenant_id, datastore, id):
+        self.authorize_request(req, 'version_show')
         datastore = models.Datastore.load(datastore)
         datastore_version = models.DatastoreVersion.load(datastore, id)
         return wsgi.Result(views.DatastoreVersionView(datastore_version,
                                                       req).data(), 200)
 
     def version_show_by_uuid(self, req, tenant_id, uuid):
+        self.authorize_request(req, 'version_show_by_uuid')
         datastore_version = models.DatastoreVersion.load_by_uuid(uuid)
         return wsgi.Result(views.DatastoreVersionView(datastore_version,
                                                       req).data(), 200)
 
     def version_index(self, req, tenant_id, datastore):
+        self.authorize_request(req, 'version_index')
         context = req.environ[wsgi.CONTEXT_KEY]
         only_active = True
         if context.is_admin:
@@ -70,6 +84,7 @@ class DatastoreController(wsgi.Controller):
         one or more entries are found in datastore_version_metadata,
         in which case only those are returned.
         """
+        self.authorize_request(req, 'list_associated_flavors')
         context = req.environ[wsgi.CONTEXT_KEY]
         flavors = (models.DatastoreVersionMetadata.
                    list_datastore_version_flavor_associations(

trove/extensions/common/service.py

@@ -21,14 +21,17 @@ from oslo_log import log as logging
 from oslo_utils import importutils
 import six
 
+from trove.cluster import models as cluster_models
 from trove.cluster.models import DBCluster
 from trove.common import cfg
 from trove.common import exception
 from trove.common.i18n import _LI
+from trove.common import policy
 from trove.common import wsgi
 from trove.datastore import models as datastore_models
 from trove.extensions.common import models
 from trove.extensions.common import views
+from trove.instance import models as instance_models
 from trove.instance.models import DBInstance
 
 
@@ -37,8 +40,30 @@ import_class = importutils.import_class
 CONF = cfg.CONF
 
 
+class ExtensionController(wsgi.Controller):
+
+    @classmethod
+    def authorize_target_action(cls, context, target_rule_name,
+                                target_id, is_cluster=False):
+        target = None
+        if is_cluster:
+            target = cluster_models.Cluster.load(context, target_id)
+        else:
+            target = instance_models.Instance.load(context, target_id)
+
+        if not target:
+            if is_cluster:
+                raise exception.ClusterNotFound(cluster=target_id)
+            raise exception.InstanceNotFound(instance=target_id)
+
+        target_type = 'cluster' if is_cluster else 'instance'
+        policy.authorize_on_target(
+            context, '%s:extension:%s' % (target_type, target_rule_name),
+            {'tenant': target.tenant_id})
+
+
 @six.add_metaclass(abc.ABCMeta)
-class BaseDatastoreRootController(wsgi.Controller):
+class BaseDatastoreRootController(ExtensionController):
     """Base class that defines the contract for root controllers."""
 
     @abc.abstractmethod
@@ -174,13 +199,16 @@ class ClusterRootController(DefaultRootController):
         return single_instance_id, instance_ids
 
 
-class RootController(wsgi.Controller):
+class RootController(ExtensionController):
     """Controller for instance functionality."""
 
     def index(self, req, tenant_id, instance_id):
         """Returns True if root is enabled; False otherwise."""
         datastore_manager, is_cluster = self._get_datastore(tenant_id,
                                                             instance_id)
+        context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(context, 'root:index', instance_id,
+                                     is_cluster=is_cluster)
         root_controller = self.load_root_controller(datastore_manager)
         return root_controller.root_index(req, tenant_id, instance_id,
                                           is_cluster)
@@ -189,6 +217,9 @@ class RootController(wsgi.Controller):
         """Enable the root user for the db instance."""
         datastore_manager, is_cluster = self._get_datastore(tenant_id,
                                                             instance_id)
+        context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(context, 'root:create', instance_id,
+                                     is_cluster=is_cluster)
         root_controller = self.load_root_controller(datastore_manager)
         if root_controller is not None:
             return root_controller.root_create(req, body, tenant_id,
@@ -199,6 +230,9 @@ class RootController(wsgi.Controller):
     def delete(self, req, tenant_id, instance_id):
         datastore_manager, is_cluster = self._get_datastore(tenant_id,
                                                             instance_id)
+        context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(context, 'root:delete', instance_id,
+                                     is_cluster=is_cluster)
         root_controller = self.load_root_controller(datastore_manager)
         if root_controller is not None:
             return root_controller.root_delete(req, tenant_id,

trove/extensions/mysql/service.py

@@ -30,6 +30,7 @@ from trove.common import pagination
 from trove.common.utils import correct_id_with_req
 from trove.common import wsgi
 from trove.extensions.common.service import DefaultRootController
+from trove.extensions.common.service import ExtensionController
 from trove.extensions.mysql.common import populate_users
 from trove.extensions.mysql.common import populate_validated_databases
 from trove.extensions.mysql.common import unquote_user_host
@@ -42,7 +43,7 @@ import_class = importutils.import_class
 CONF = cfg.CONF
 
 
-class UserController(wsgi.Controller):
+class UserController(ExtensionController):
     """Controller for instance functionality."""
     schemas = apischema.user
 
@@ -60,6 +61,7 @@ class UserController(wsgi.Controller):
                    "req : '%(req)s'\n\n") %
                  {"id": instance_id, "req": req})
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(context, 'user:index', instance_id)
         users, next_marker = models.Users.load(context, instance_id)
         view = views.UsersView(users)
         paged = pagination.SimplePaginatedDataView(req.url, 'users', view,
@@ -75,6 +77,7 @@ class UserController(wsgi.Controller):
                   "req": strutils.mask_password(req),
                   "body": strutils.mask_password(body)})
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(context, 'user:create', instance_id)
         context.notification = notification.DBaaSUserCreate(context,
                                                             request=req)
         users = body['users']
@@ -94,6 +97,7 @@ class UserController(wsgi.Controller):
                    "req : '%(req)s'\n\n") %
                  {"id": instance_id, "req": req})
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(context, 'user:delete', instance_id)
         id = correct_id_with_req(id, req)
         username, host = unquote_user_host(id)
         user = None
@@ -122,6 +126,7 @@ class UserController(wsgi.Controller):
                    "req : '%(req)s'\n\n") %
                  {"id": instance_id, "req": req})
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(context, 'user:show', instance_id)
         id = correct_id_with_req(id, req)
         username, host = unquote_user_host(id)
         user = None
@@ -141,6 +146,7 @@ class UserController(wsgi.Controller):
                    "req : '%(req)s'\n\n") %
                  {"id": instance_id, "req": strutils.mask_password(req)})
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(context, 'user:update', instance_id)
         id = correct_id_with_req(id, req)
         username, hostname = unquote_user_host(id)
         user = None
@@ -171,6 +177,7 @@ class UserController(wsgi.Controller):
                    "req : '%(req)s'\n\n") %
                  {"id": instance_id, "req": strutils.mask_password(req)})
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(context, 'user:update_all', instance_id)
         context.notification = notification.DBaaSUserChangePassword(
             context, request=req)
         users = body['users']
@@ -203,7 +210,7 @@ class UserController(wsgi.Controller):
         return wsgi.Result(None, 202)
 
 
-class UserAccessController(wsgi.Controller):
+class UserAccessController(ExtensionController):
     """Controller for adding and removing database access for a user."""
     schemas = apischema.user
 
@@ -232,6 +239,8 @@ class UserAccessController(wsgi.Controller):
                  {"id": instance_id, "req": req})
 
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(
+            context, 'user_access:index', instance_id)
         # Make sure this user exists.
         user_id = correct_id_with_req(user_id, req)
         user = self._get_user(context, instance_id, user_id)
@@ -249,6 +258,8 @@ class UserAccessController(wsgi.Controller):
                    "req : '%(req)s'\n\n") %
                  {"id": instance_id, "req": req})
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(
+            context, 'user_access:update', instance_id)
         context.notification = notification.DBaaSUserGrant(
             context, request=req)
         user_id = correct_id_with_req(user_id, req)
@@ -270,6 +281,8 @@ class UserAccessController(wsgi.Controller):
                    "req : '%(req)s'\n\n") %
                  {"id": instance_id, "req": req})
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(
+            context, 'user_access:delete', instance_id)
         context.notification = notification.DBaaSUserRevoke(
             context, request=req)
         user_id = correct_id_with_req(user_id, req)
@@ -288,7 +301,7 @@ class UserAccessController(wsgi.Controller):
         return wsgi.Result(None, 202)
 
 
-class SchemaController(wsgi.Controller):
+class SchemaController(ExtensionController):
     """Controller for instance functionality."""
     schemas = apischema.dbschema
 
@@ -299,6 +312,8 @@ class SchemaController(wsgi.Controller):
                  {"id": instance_id, "req": req})
 
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(
+            context, 'database:index', instance_id)
         schemas, next_marker = models.Schemas.load(context, instance_id)
         view = views.SchemasView(schemas)
         paged = pagination.SimplePaginatedDataView(req.url, 'databases', view,
@@ -315,6 +330,8 @@ class SchemaController(wsgi.Controller):
                   "body": body})
 
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(
+            context, 'database:create', instance_id)
         schemas = body['databases']
         context.notification = notification.DBaaSDatabaseCreate(context,
                                                                 request=req)
@@ -334,6 +351,8 @@ class SchemaController(wsgi.Controller):
                    "req : '%(req)s'\n\n") %
                  {"id": instance_id, "req": req})
         context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(
+            context, 'database:delete', instance_id)
         context.notification = notification.DBaaSDatabaseDelete(
             context, request=req)
         with StartNotification(context, instance_id=instance_id, dbname=id):
@@ -349,6 +368,9 @@ class SchemaController(wsgi.Controller):
         return wsgi.Result(None, 202)
 
     def show(self, req, tenant_id, instance_id, id):
+        context = req.environ[wsgi.CONTEXT_KEY]
+        self.authorize_target_action(
+            context, 'database:show', instance_id)
         raise webob.exc.HTTPNotImplemented()
 
 

trove/flavor/service.py

@@ -17,6 +17,7 @@
 import six
 
 from trove.common import exception
+from trove.common import policy
 from trove.common import wsgi
 from trove.flavor import models
 from trove.flavor import views
@@ -30,12 +31,16 @@ class FlavorController(wsgi.Controller):
         context = req.environ[wsgi.CONTEXT_KEY]
         self._validate_flavor_id(id)
         flavor = models.Flavor(context=context, flavor_id=id)
+        # Flavors do not bind to a particular tenant.
+        # Only authorize the current tenant.
+        policy.authorize_on_tenant(context, 'flavor:show')
         # Pass in the request to build accurate links.
         return wsgi.Result(views.FlavorView(flavor, req).data(), 200)
 
     def index(self, req, tenant_id):
         """Return all flavors."""
         context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'flavor:index')
         flavors = models.Flavors(context=context)
         return wsgi.Result(views.FlavorsView(flavors, req).data(), 200)
 

trove/guestagent/guest_log.py

@@ -209,8 +209,7 @@ class GuestLog(object):
                 'metafile': self._metafile_name()
             }
         else:
-            raise exception.UnauthorizedRequest(_(
+            raise exception.LogAccessForbidden(action='show', log=self._name)
-                "Not authorized to show log '%s'.") % self._name)
 
     def _refresh_details(self):
 
@@ -310,16 +309,16 @@ class GuestLog(object):
                     self._file)
             return self.show()
         else:
-            raise exception.UnauthorizedRequest(_(
+            raise exception.LogAccessForbidden(
-                "Not authorized to publish log '%s'.") % self._name)
+                action='publish', log=self._name)
 
     def discard_log(self):
         if self.exposed:
             self._delete_log_components()
             return self.show()
         else:
-            raise exception.UnauthorizedRequest(_(
+            raise exception.LogAccessForbidden(
-                "Not authorized to discard log '%s'.") % self._name)
+                action='discard', log=self._name)
 
     def _delete_log_components(self):
         container_name = self.get_container_name(force=True)

trove/instance/service.py

@@ -27,6 +27,7 @@ from trove.common.i18n import _LI
 from trove.common import notification
 from trove.common.notification import StartNotification
 from trove.common import pagination
+from trove.common import policy
 from trove.common.remote import create_guest_client
 from trove.common import utils
 from trove.common import wsgi
@@ -47,6 +48,11 @@ class InstanceController(wsgi.Controller):
     """Controller for instance functionality."""
     schemas = apischema.instance.copy()
 
+    @classmethod
+    def authorize_instance_action(cls, context, instance_rule_name, instance):
+        policy.authorize_on_target(context, 'instance:%s' % instance_rule_name,
+                                   {'tenant': instance.tenant_id})
+
     @classmethod
     def get_action_schema(cls, body, action_schema):
         action_type = list(body.keys())[0]
@@ -106,6 +112,7 @@ class InstanceController(wsgi.Controller):
     def _action_restart(self, context, req, instance, body):
         context.notification = notification.DBaaSInstanceRestart(context,
                                                                  request=req)
+        self.authorize_instance_action(context, 'restart', instance)
         with StartNotification(context, instance_id=instance.id):
             instance.restart()
         return wsgi.Result(None, 202)
@@ -136,6 +143,8 @@ class InstanceController(wsgi.Controller):
     def _action_resize_volume(self, context, req, instance, volume):
         context.notification = notification.DBaaSInstanceResizeVolume(
             context, request=req)
+        self.authorize_instance_action(context, 'resize_volume', instance)
+
         with StartNotification(context, instance_id=instance.id,
                                new_size=volume['size']):
             instance.resize_volume(volume['size'])
@@ -144,6 +153,8 @@ class InstanceController(wsgi.Controller):
     def _action_resize_flavor(self, context, req, instance, flavorRef):
         context.notification = notification.DBaaSInstanceResizeInstance(
             context, request=req)
+        self.authorize_instance_action(context, 'resize_flavor', instance)
+
         new_flavor_id = utils.get_id_from_href(flavorRef)
         with StartNotification(context, instance_id=instance.id,
                                new_flavor_id=new_flavor_id):
@@ -154,6 +165,8 @@ class InstanceController(wsgi.Controller):
         raise webob.exc.HTTPNotImplemented()
 
     def _action_promote_to_replica_source(self, context, req, instance, body):
+        self.authorize_instance_action(
+            context, 'promote_to_replica_source', instance)
         context.notification = notification.DBaaSInstanceEject(context,
                                                                request=req)
         with StartNotification(context, instance_id=instance.id):
@@ -161,6 +174,8 @@ class InstanceController(wsgi.Controller):
         return wsgi.Result(None, 202)
 
     def _action_eject_replica_source(self, context, req, instance, body):
+        self.authorize_instance_action(
+            context, 'eject_replica_source', instance)
         context.notification = notification.DBaaSInstancePromote(context,
                                                                  request=req)
         with StartNotification(context, instance_id=instance.id):
@@ -168,6 +183,11 @@ class InstanceController(wsgi.Controller):
         return wsgi.Result(None, 202)
 
     def _action_reset_status(self, context, req, instance, body):
+        if 'force_delete' in body['reset_status']:
+            self.authorize_instance_action(context, 'force_delete', instance)
+        else:
+            self.authorize_instance_action(
+                context, 'reset_status', instance)
         context.notification = notification.DBaaSInstanceResetStatus(
             context, request=req)
         with StartNotification(context, instance_id=instance.id):
@@ -183,6 +203,7 @@ class InstanceController(wsgi.Controller):
         LOG.info(_LI("Listing database instances for tenant '%s'"), tenant_id)
         LOG.debug("req : '%s'\n\n", req)
         context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'instance:index')
         clustered_q = req.GET.get('include_clustered', '').lower()
         include_clustered = clustered_q == 'true'
         servers, marker = models.Instances.load(context, include_clustered)
@@ -197,6 +218,10 @@ class InstanceController(wsgi.Controller):
                  id)
         LOG.debug("req : '%s'\n\n", req)
         context = req.environ[wsgi.CONTEXT_KEY]
+
+        instance = models.Instance.load(context, id)
+        self.authorize_instance_action(context, 'backups', instance)
+
         backups, marker = backup_model.list_for_instance(context, id)
         view = backup_views.BackupViews(backups)
         paged = pagination.SimplePaginatedDataView(req.url, 'backups', view,
@@ -213,6 +238,7 @@ class InstanceController(wsgi.Controller):
         context = req.environ[wsgi.CONTEXT_KEY]
         server = models.load_instance_with_info(models.DetailInstance,
                                                 context, id)
+        self.authorize_instance_action(context, 'show', server)
         return wsgi.Result(views.InstanceDetailView(server,
                                                     req=req).data(), 200)
 
@@ -224,6 +250,7 @@ class InstanceController(wsgi.Controller):
         LOG.debug("req : '%s'\n\n", req)
         context = req.environ[wsgi.CONTEXT_KEY]
         instance = models.load_any_instance(context, id)
+        self.authorize_instance_action(context, 'delete', instance)
         context.notification = notification.DBaaSInstanceDelete(
             context, request=req)
         with StartNotification(context, instance_id=instance.id):
@@ -247,6 +274,7 @@ class InstanceController(wsgi.Controller):
         LOG.debug("req : '%s'\n\n", strutils.mask_password(req))
         LOG.debug("body : '%s'\n\n", strutils.mask_password(body))
         context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'instance:create')
         context.notification = notification.DBaaSInstanceCreate(context,
                                                                 request=req)
         datastore_args = body['instance'].get('datastore', {})
@@ -268,6 +296,25 @@ class InstanceController(wsgi.Controller):
         except ValueError as ve:
             raise exception.BadRequest(msg=ve)
 
+        modules = body['instance'].get('modules')
+
+        # The following operations have their own API calls.
+        # We need to make sure the same policies are enforced when
+        # creating an instance.
+        # i.e. if attaching configuration group to an existing instance is not
+        # allowed, it should not be possible to create a new instance with the
+        # group attached either
+        if configuration:
+            policy.authorize_on_tenant(context, 'instance:update')
+        if modules:
+            policy.authorize_on_tenant(context, 'instance:module_apply')
+        if users:
+            policy.authorize_on_tenant(
+                context, 'instance:extension:user:create')
+        if databases:
+            policy.authorize_on_tenant(
+                context, 'instance:extension:database:create')
+
         if 'volume' in body['instance']:
             volume_info = body['instance']['volume']
             volume_size = int(volume_info['size'])
@@ -289,7 +336,6 @@ class InstanceController(wsgi.Controller):
                                            # also check for older name
                                            body['instance'].get('slave_of'))
         replica_count = body['instance'].get('replica_count')
-        modules = body['instance'].get('modules')
         locality = body['instance'].get('locality')
         if locality:
             locality_domain = ['affinity', 'anti-affinity']
@@ -371,6 +417,7 @@ class InstanceController(wsgi.Controller):
         context = req.environ[wsgi.CONTEXT_KEY]
 
         instance = models.Instance.load(context, id)
+        self.authorize_instance_action(context, 'update', instance)
 
         # Make sure args contains a 'configuration_id' argument,
         args = {}
@@ -388,6 +435,7 @@ class InstanceController(wsgi.Controller):
         context = req.environ[wsgi.CONTEXT_KEY]
 
         instance = models.Instance.load(context, id)
+        self.authorize_instance_action(context, 'edit', instance)
 
         args = {}
         args['detach_replica'] = ('replica_of' in body['instance'] or
@@ -411,6 +459,8 @@ class InstanceController(wsgi.Controller):
         LOG.info(_LI("Getting default configuration for instance %s"), id)
         context = req.environ[wsgi.CONTEXT_KEY]
         instance = models.Instance.load(context, id)
+        self.authorize_instance_action(context, 'configuration', instance)
+
         LOG.debug("Server: %s", instance)
         config = instance.get_default_configuration_template()
         LOG.debug("Default config for instance %(instance_id)s is %(config)s",
@@ -425,6 +475,7 @@ class InstanceController(wsgi.Controller):
         instance = models.Instance.load(context, id)
         if not instance:
             raise exception.NotFound(uuid=id)
+        self.authorize_instance_action(context, 'guest_log_list', instance)
         client = create_guest_client(context, id)
         guest_log_list = client.guest_log_list()
         return wsgi.Result({'logs': guest_log_list}, 200)
@@ -454,6 +505,7 @@ class InstanceController(wsgi.Controller):
         instance = models.Instance.load(context, id)
         if not instance:
             raise exception.NotFound(uuid=id)
+        self.authorize_instance_action(context, 'module_list', instance)
         from_guest = bool(req.GET.get('from_guest', '').lower())
         include_contents = bool(req.GET.get('include_contents', '').lower())
         if from_guest:
@@ -481,6 +533,7 @@ class InstanceController(wsgi.Controller):
         instance = models.Instance.load(context, id)
         if not instance:
             raise exception.NotFound(uuid=id)
+        self.authorize_instance_action(context, 'module_apply', instance)
         module_ids = [mod['id'] for mod in body.get('modules', [])]
         modules = module_models.Modules.load_by_ids(context, module_ids)
         module_list = []
@@ -501,6 +554,7 @@ class InstanceController(wsgi.Controller):
         instance = models.Instance.load(context, id)
         if not instance:
             raise exception.NotFound(uuid=id)
+        self.authorize_instance_action(context, 'module_remove', instance)
         module = module_models.Module.load(context, module_id)
         module_info = module_views.DetailedModuleView(module).data()
         client = create_guest_client(context, id)

trove/limits/service.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from trove.common import policy
 from trove.common import wsgi
 from trove.limits import views
 from trove.quota.quota import QUOTAS
@@ -27,6 +28,8 @@ class LimitsController(wsgi.Controller):
         """
         Return all absolute and rate limit information.
         """
+        context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'limits:index')
         quotas = QUOTAS.get_all_quotas_by_tenant(tenant_id)
         abs_limits = {k: v['hard_limit'] for k, v in quotas.items()}
         rate_limits = req.environ.get("trove.limits", [])

trove/module/service.py

@@ -22,6 +22,7 @@ import trove.common.apischema as apischema
 from trove.common import exception
 from trove.common.i18n import _
 from trove.common import pagination
+from trove.common import policy
 from trove.common import wsgi
 from trove.datastore import models as datastore_models
 from trove.instance import models as instance_models
@@ -37,8 +38,20 @@ class ModuleController(wsgi.Controller):
 
     schemas = apischema.module
 
+    @classmethod
+    def authorize_module_action(cls, context, module_rule_name, module):
+        """If a modules in not owned by any particular tenant just check
+        the current tenant is allowed to perform the action.
+        """
+        if module.tenant_id is not None:
+            policy.authorize_on_target(context, 'module:%s' % module_rule_name,
+                                       {'tenant': module.tenant_id})
+        else:
+            policy.authorize_on_tenant(context, 'module:%s' % module_rule_name)
+
     def index(self, req, tenant_id):
         context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'module:index')
         datastore = req.GET.get('datastore', '')
         if datastore and datastore.lower() != models.Modules.MATCH_ALL_NAME:
             ds, ds_ver = datastore_models.get_datastore_version(
@@ -53,6 +66,7 @@ class ModuleController(wsgi.Controller):
 
         context = req.environ[wsgi.CONTEXT_KEY]
         module = models.Module.load(context, id)
+        self.authorize_module_action(context, 'show', module)
         module.instance_count = len(models.InstanceModules.load(
             context, module_id=module.id, md5=module.md5))
 
@@ -65,6 +79,7 @@ class ModuleController(wsgi.Controller):
         LOG.info(_("Creating module '%s'") % name)
 
         context = req.environ[wsgi.CONTEXT_KEY]
+        policy.authorize_on_tenant(context, 'module:create')
         module_type = body['module']['module_type']
         contents = body['module']['contents']
 
@@ -89,6 +104,7 @@ class ModuleController(wsgi.Controller):
 
         context = req.environ[wsgi.CONTEXT_KEY]
         module = models.Module.load(context, id)
+        self.authorize_module_action(context, 'delete', module)
         models.Module.delete(context, module)
         return wsgi.Result(None, 200)
 
@@ -97,6 +113,7 @@ class ModuleController(wsgi.Controller):
 
         context = req.environ[wsgi.CONTEXT_KEY]
         module = models.Module.load(context, id)
+        self.authorize_module_action(context, 'update', module)
         original_module = copy.deepcopy(module)
         if 'name' in body['module']:
             module.name = body['module']['name']
@@ -146,6 +163,10 @@ class ModuleController(wsgi.Controller):
         LOG.info(_("Getting instances for module %s") % id)
 
         context = req.environ[wsgi.CONTEXT_KEY]
+
+        module = models.Module.load(context, id)
+        self.authorize_module_action(context, 'instances', module)
+
         instance_modules, marker = models.InstanceModules.load(
             context, module_id=id)
         if instance_modules:

trove/tests/api/mgmt/instances_actions.py

@@ -18,6 +18,7 @@ from proboscis import after_class
 from proboscis.asserts import assert_equal
 from proboscis.asserts import assert_raises
 from proboscis import before_class
+from proboscis import SkipTest
 from proboscis import test
 
 from trove.backup import models as backup_models
@@ -30,6 +31,7 @@ from trove.extensions.mgmt.instances.service import MgmtInstanceController
 from trove.instance import models as imodels
 from trove.instance.models import DBInstance
 from trove.instance.tasks import InstanceTasks
+from trove.tests.config import CONFIG
 from trove.tests.util import create_dbaas_client
 from trove.tests.util import test_config
 from trove.tests.util.users import Requirements
@@ -79,6 +81,7 @@ class MgmtInstanceBase(object):
 
 @test(groups=[GROUP])
 class RestartTaskStatusTests(MgmtInstanceBase):
+
     @before_class
     def setUp(self):
         super(RestartTaskStatusTests, self).setUp()
@@ -137,6 +140,9 @@ class RestartTaskStatusTests(MgmtInstanceBase):
 
     @test
     def mgmt_reset_task_status_clears_backups(self):
+        if CONFIG.fake_mode:
+            raise SkipTest("Test requires an instance.")
+
         self.reset_task_status()
         self._reload_db_info()
         assert_equal(self.db_info.task_status, InstanceTasks.NONE)
@@ -201,5 +207,6 @@ class RestartTaskStatusTests(MgmtInstanceBase):
             found_backup.delete()
         admin = test_config.users.find_user(Requirements(is_admin=True))
         admin_dbaas = create_dbaas_client(admin)
+        if not CONFIG.fake_mode:
             result = admin_dbaas.instances.backups(self.db_info.id)
             assert_equal(0, len(result))

trove/tests/scenario/runners/instance_force_delete_runners.py

@@ -46,8 +46,9 @@ class InstanceForceDeleteRunner(TestRunner):
 
     def run_delete_build_instance(self, expected_http_code=202):
         if self.build_inst_id:
-            self.auth_client.instances.force_delete(self.build_inst_id)
+            self.admin_client.instances.force_delete(self.build_inst_id)
-            self.assert_client_code(expected_http_code)
+            self.assert_client_code(expected_http_code,
+                                    client=self.admin_client)
 
     def run_wait_for_force_delete(self):
         if self.build_inst_id:

trove/tests/unittests/api/common/test_limits.py

@@ -45,6 +45,7 @@ class BaseLimitTestSuite(trove_testtools.TestCase):
 
     def setUp(self):
         super(BaseLimitTestSuite, self).setUp()
+        self.context = trove_testtools.TroveTestContext(self)
         self.absolute_limits = {"max_instances": 55,
                                 "max_volumes": 100,
                                 "max_backups": 40}
@@ -60,7 +61,7 @@ class LimitsControllerTest(BaseLimitTestSuite):
         limit_controller = LimitsController()
 
         req = MagicMock()
-        req.environ = {}
+        req.environ = {'trove.context': self.context}
 
         view = limit_controller.index(req, "test_tenant_id")
         expected = {'limits': [{'verb': 'ABSOLUTE'}]}
@@ -122,7 +123,7 @@ class LimitsControllerTest(BaseLimitTestSuite):
                                        hard_limit=55)}
 
         req = MagicMock()
-        req.environ = {"trove.limits": limits}
+        req.environ = {"trove.limits": limits, 'trove.context': self.context}
 
         with patch.object(QUOTAS, 'get_all_quotas_by_tenant',
                           return_value=abs_limits):

trove/tests/unittests/cluster/test_cluster_controller.py

@@ -18,7 +18,6 @@ import jsonschema
 from mock import MagicMock
 from mock import Mock
 from mock import patch
-from testtools import TestCase
 from testtools.matchers import Is, Equals
 from trove.cluster import models
 from trove.cluster.models import Cluster, DBCluster
@@ -33,7 +32,8 @@ from trove.datastore import models as datastore_models
 from trove.tests.unittests import trove_testtools
 
 
-class TestClusterController(TestCase):
+class TestClusterController(trove_testtools.TestCase):
+
     def setUp(self):
         super(TestClusterController, self).setUp()
         self.controller = ClusterController()
@@ -248,7 +248,8 @@ class TestClusterController(TestCase):
         cluster.delete.assert_called_with()
 
 
-class TestClusterControllerWithStrategy(TestCase):
+class TestClusterControllerWithStrategy(trove_testtools.TestCase):
+
     def setUp(self):
         super(TestClusterControllerWithStrategy, self).setUp()
         self.controller = ClusterController()

trove/tests/unittests/common/test_common_extensions.py

@@ -24,6 +24,7 @@ from trove.extensions.common import models
 from trove.extensions.common.service import ClusterRootController
 from trove.extensions.common.service import DefaultRootController
 from trove.extensions.common.service import RootController
+from trove.instance import models as instance_models
 from trove.instance.models import DBInstance
 from trove.tests.unittests import trove_testtools
 
@@ -90,16 +91,20 @@ class TestRootController(trove_testtools.TestCase):
 
     def setUp(self):
         super(TestRootController, self).setUp()
+        self.context = trove_testtools.TroveTestContext(self)
         self.controller = RootController()
 
+    @patch.object(instance_models.Instance, "load")
     @patch.object(RootController, "load_root_controller")
     @patch.object(RootController, "_get_datastore")
-    def test_index(self, service_get_datastore, service_load_root_controller):
+    def test_index(self, service_get_datastore, service_load_root_controller,
+                   service_load_instance):
         req = Mock()
+        req.environ = {'trove.context': self.context}
         tenant_id = Mock()
         uuid = utils.generate_uuid()
         ds_manager = Mock()
-        is_cluster = Mock()
+        is_cluster = False
         service_get_datastore.return_value = (ds_manager, is_cluster)
         root_controller = Mock()
         ret = Mock()
@@ -112,15 +117,18 @@ class TestRootController(trove_testtools.TestCase):
         root_controller.root_index.assert_called_with(
             req, tenant_id, uuid, is_cluster)
 
+    @patch.object(instance_models.Instance, "load")
     @patch.object(RootController, "load_root_controller")
     @patch.object(RootController, "_get_datastore")
-    def test_create(self, service_get_datastore, service_load_root_controller):
+    def test_create(self, service_get_datastore, service_load_root_controller,
+                    service_load_instance):
         req = Mock()
+        req.environ = {'trove.context': self.context}
         body = Mock()
         tenant_id = Mock()
         uuid = utils.generate_uuid()
         ds_manager = Mock()
-        is_cluster = Mock()
+        is_cluster = False
         service_get_datastore.return_value = (ds_manager, is_cluster)
         root_controller = Mock()
         ret = Mock()
@@ -134,17 +142,20 @@ class TestRootController(trove_testtools.TestCase):
         root_controller.root_create.assert_called_with(
             req, body, tenant_id, uuid, is_cluster)
 
+    @patch.object(instance_models.Instance, "load")
     @patch.object(RootController, "load_root_controller")
     @patch.object(RootController, "_get_datastore")
     def test_create_with_no_root_controller(self,
                                             service_get_datastore,
-                                            service_load_root_controller):
+                                            service_load_root_controller,
+                                            service_load_instance):
         req = Mock()
+        req.environ = {'trove.context': self.context}
         body = Mock()
         tenant_id = Mock()
         uuid = utils.generate_uuid()
         ds_manager = Mock()
-        is_cluster = Mock()
+        is_cluster = False
         service_get_datastore.return_value = (ds_manager, is_cluster)
         service_load_root_controller.return_value = None
 
@@ -160,6 +171,7 @@ class TestClusterRootController(trove_testtools.TestCase):
 
     def setUp(self):
         super(TestClusterRootController, self).setUp()
+        self.context = trove_testtools.TroveTestContext(self)
         self.controller = ClusterRootController()
 
     @patch.object(ClusterRootController, "cluster_root_index")
@@ -204,22 +216,18 @@ class TestClusterRootController(trove_testtools.TestCase):
 
     @patch.object(models.ClusterRoot, "load")
     def test_instance_root_index(self, mock_cluster_root_load):
-        context = Mock()
         req = Mock()
-        req.environ = Mock()
+        req.environ = {'trove.context': self.context}
-        req.environ.__getitem__ = Mock(return_value=context)
         tenant_id = Mock()
         instance_id = utils.generate_uuid()
         self.controller.instance_root_index(req, tenant_id, instance_id)
-        mock_cluster_root_load.assert_called_with(context, instance_id)
+        mock_cluster_root_load.assert_called_with(self.context, instance_id)
 
     @patch.object(models.ClusterRoot, "load",
                   side_effect=exception.UnprocessableEntity())
     def test_instance_root_index_exception(self, mock_cluster_root_load):
-        context = Mock()
         req = Mock()
-        req.environ = Mock()
+        req.environ = {'trove.context': self.context}
-        req.environ.__getitem__ = Mock(return_value=context)
         tenant_id = Mock()
         instance_id = utils.generate_uuid()
         self.assertRaises(
@@ -227,7 +235,7 @@ class TestClusterRootController(trove_testtools.TestCase):
             self.controller.instance_root_index,
             req, tenant_id, instance_id
         )
-        mock_cluster_root_load.assert_called_with(context, instance_id)
+        mock_cluster_root_load.assert_called_with(self.context, instance_id)
 
     @patch.object(ClusterRootController, "instance_root_index")
     @patch.object(ClusterRootController, "_get_cluster_instance_id")
@@ -278,12 +286,10 @@ class TestClusterRootController(trove_testtools.TestCase):
     @patch.object(models.ClusterRoot, "create")
     def test_instance_root_create(self, mock_cluster_root_create):
         user = Mock()
-        context = Mock()
+        self.context.user = Mock()
-        context.user = Mock()
+        self.context.user.__getitem__ = Mock(return_value=user)
-        context.user.__getitem__ = Mock(return_value=user)
         req = Mock()
-        req.environ = Mock()
+        req.environ = {'trove.context': self.context}
-        req.environ.__getitem__ = Mock(return_value=context)
         password = Mock()
         body = {'password': password}
         instance_id = utils.generate_uuid()
@@ -291,17 +297,16 @@ class TestClusterRootController(trove_testtools.TestCase):
         self.controller.instance_root_create(
             req, body, instance_id, cluster_instances)
         mock_cluster_root_create.assert_called_with(
-            context, instance_id, context.user, password, cluster_instances)
+            self.context, instance_id, self.context.user, password,
+            cluster_instances)
 
     @patch.object(models.ClusterRoot, "create")
     def test_instance_root_create_no_body(self, mock_cluster_root_create):
         user = Mock()
-        context = Mock()
+        self.context.user = Mock()
-        context.user = Mock()
+        self.context.user.__getitem__ = Mock(return_value=user)
-        context.user.__getitem__ = Mock(return_value=user)
         req = Mock()
-        req.environ = Mock()
+        req.environ = {'trove.context': self.context}
-        req.environ.__getitem__ = Mock(return_value=context)
         password = None
         body = None
         instance_id = utils.generate_uuid()
@@ -309,4 +314,5 @@ class TestClusterRootController(trove_testtools.TestCase):
         self.controller.instance_root_create(
             req, body, instance_id, cluster_instances)
         mock_cluster_root_create.assert_called_with(
-            context, instance_id, context.user, password, cluster_instances)
+            self.context, instance_id, self.context.user, password,
+            cluster_instances)

trove/tests/unittests/common/test_policy.py

@@ -0,0 +1,53 @@
+# Copyright 2016 Tesora Inc.
+# 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 mock import MagicMock
+from mock import NonCallableMock
+from mock import patch
+
+from trove.common import exception as trove_exceptions
+from trove.common import policy as trove_policy
+from trove.tests.unittests import trove_testtools
+
+
+class TestPolicy(trove_testtools.TestCase):
+
+    def setUp(self):
+        super(TestPolicy, self).setUp()
+        self.context = trove_testtools.TroveTestContext(self)
+        self.mock_enforcer = MagicMock()
+        get_enforcer_patch = patch.object(trove_policy, 'get_enforcer',
+                                          return_value=self.mock_enforcer)
+        self.addCleanup(get_enforcer_patch.stop)
+        self.mock_get_enforcer = get_enforcer_patch.start()
+
+    def test_authorize_on_tenant(self):
+        test_rule = NonCallableMock()
+        trove_policy.authorize_on_tenant(self.context, test_rule)
+        self.mock_get_enforcer.assert_called_once_with()
+        self.mock_enforcer.authorize.assert_called_once_with(
+            test_rule, {'tenant': self.context.tenant}, self.context.to_dict(),
+            do_raise=True, exc=trove_exceptions.PolicyNotAuthorized,
+            action=test_rule)
+
+    def test_authorize_on_target(self):
+        test_rule = NonCallableMock()
+        test_target = NonCallableMock()
+        trove_policy.authorize_on_target(self.context, test_rule, test_target)
+        self.mock_get_enforcer.assert_called_once_with()
+        self.mock_enforcer.authorize.assert_called_once_with(
+            test_rule, test_target, self.context.to_dict(),
+            do_raise=True, exc=trove_exceptions.PolicyNotAuthorized,
+            action=test_rule)

trove/tests/unittests/trove_testtools.py

@@ -23,6 +23,7 @@ import testtools
 from trove.common import cfg
 from trove.common.context import TroveContext
 from trove.common.notification import DBaaSAPINotification
+from trove.common import policy
 from trove.tests import root_logger
 
 
@@ -101,6 +102,11 @@ class TestCase(testtools.TestCase):
         # Default manager used by all unittsest unless explicitly overridden.
         self.patch_datastore_manager('mysql')
 
+        policy_patcher = mock.patch.object(policy, 'get_enforcer',
+                                           return_value=mock.MagicMock())
+        self.addCleanup(policy_patcher.stop)
+        policy_patcher.start()
+
     def tearDown(self):
         # yes, this is gross and not thread aware.
         # but the only way to make it thread aware would require that