Make collection_key and member_key raise if unset

Instead of populating with __UNUSED__ or other silly string, make
direct use of "collection_key" or "member_key" raise a ValueError
if they are unset and referenced.

Change-Id: Idf4f4df9d933317fff96a474cdf23d758ebdfa8c
Partial-Bug: #1776504

7 files changed, 52 insertions, 25 deletions

diff --git a/keystone/api/_shared/EC2_S3_Resource.py b/keystone/api/_shared/EC2_S3_Resource.py
index 36494b99e..f7d642d86 100644
--- a/keystone/api/_shared/EC2_S3_Resource.py
+++ b/keystone/api/_shared/EC2_S3_Resource.py
@@ -31,9 +31,6 @@ CRED_TYPE_EC2 = 'ec2'
 
 
 class ResourceBase(ks_flask.ResourceBase):
-    collection_key = '__UNUSED__'
-    member_key = '__UNUSED__'
-
     def get(self):
         # SPECIAL CASE: GET is not allowed, raise METHOD_NOT_ALLOWED
         raise exceptions.MethodNotAllowed(valid_methods=['POST'])
diff --git a/keystone/api/auth.py b/keystone/api/auth.py
index 568cccee2..da63ed9f8 100644
--- a/keystone/api/auth.py
+++ b/keystone/api/auth.py
@@ -101,9 +101,6 @@ def _get_sso_origin_host():
 
 
 class _AuthFederationWebSSOBase(ks_flask.ResourceBase):
-    collection_key = '__UNUSED__'
-    member_key = '__UNUSED__'
-
     @staticmethod
     def _render_template_response(host, token_id):
         with open(CONF.federation.sso_callback_template) as template:
diff --git a/keystone/api/ec2tokens.py b/keystone/api/ec2tokens.py
index f3b23ab3b..d10b429b9 100644
--- a/keystone/api/ec2tokens.py
+++ b/keystone/api/ec2tokens.py
@@ -30,9 +30,6 @@ CRED_TYPE_EC2 = 'ec2'
 
 
 class EC2TokensResource(EC2_S3_Resource.ResourceBase):
-    collection_key = '__UNUSED__'
-    member_key = '__UNUSED__'
-
     @staticmethod
     def _check_signature(creds_ref, credentials):
         signer = ec2_utils.Ec2Signer(creds_ref['secret'])
diff --git a/keystone/api/users.py b/keystone/api/users.py
index fed08a3c5..bd5a12536 100644
--- a/keystone/api/users.py
+++ b/keystone/api/users.py
@@ -193,9 +193,6 @@ class UserResource(ks_flask.ResourceBase):
 
 
 class UserChangePasswordResource(ks_flask.ResourceBase):
-    collection_key = '__UNUSED__'
-    member_key = '__UNUSED__'
-
     @ks_flask.unenforced_api
     def get(self, user_id):
         # Special case, GET is not allowed.
diff --git a/keystone/common/rbac_enforcer/enforcer.py b/keystone/common/rbac_enforcer/enforcer.py
index 032147d13..71fbba02a 100644
--- a/keystone/common/rbac_enforcer/enforcer.py
+++ b/keystone/common/rbac_enforcer/enforcer.py
@@ -168,7 +168,15 @@ class RBACEnforcer(object):
                 # here.
                 resource = flask.current_app.view_functions[
                     flask.request.endpoint].view_class
-                member_name = getattr(resource, 'member_key', None)
+                try:
+                    member_name = getattr(resource, 'member_key', None)
+                except ValueError:
+                    # NOTE(morgan): In the case that the ResourceBase keystone
+                    # class is used, we raise a value error when member_key
+                    # has not been set on the class. This is perfectly
+                    # normal and acceptable. Set member_name to None as though
+                    # it wasn't set.
+                    member_name = None
                 func = getattr(
                     resource, 'get_member_from_driver', None)
                 if member_name is not None and callable(func):
diff --git a/keystone/server/flask/common.py b/keystone/server/flask/common.py
index 520c17f44..f48e90c40 100644
--- a/keystone/server/flask/common.py
+++ b/keystone/server/flask/common.py
@@ -570,10 +570,22 @@ class APIBase(object):
         return inst
 
 
-class ResourceBase(flask_restful.Resource):
+class _AttributeRaisesError(object):
+    # NOTE(morgan): This is a special case class that exists to effectively
+    # create a @classproperty style function. We use __get__ to raise the
+    # exception.
+
+    def __init__(self, name):
+        self.__msg = 'PROGRAMMING ERROR: `self.{name}` is not set.'.format(
+            name=name)
+
+    def __get__(self, instance, owner):
+        raise ValueError(self.__msg)
 
-    collection_key = None
-    member_key = None
+
+class ResourceBase(flask_restful.Resource):
+    collection_key = _AttributeRaisesError(name='collection_key')
+    member_key = _AttributeRaisesError(name='member_key')
     _public_parameters = frozenset([])
     # NOTE(morgan): This must match the string on the API the resource is
     # registered to.
@@ -582,15 +594,6 @@ class ResourceBase(flask_restful.Resource):
 
     method_decorators = []
 
-    def __init__(self):
-        super(ResourceBase, self).__init__()
-        if self.collection_key is None:
-            raise ValueError('PROGRAMMING ERROR: `self.collection_key` '
-                             'cannot be `None`.')
-        if self.member_key is None:
-            raise ValueError('PROGRAMMING ERROR: `self.member_key` cannot '
-                             'be `None`.')
-
     @staticmethod
     def _assign_unique_id(ref):
         ref = ref.copy()
diff --git a/keystone/tests/unit/server/test_keystone_flask.py b/keystone/tests/unit/server/test_keystone_flask.py
index 94a7a00d8..147c76199 100644
--- a/keystone/tests/unit/server/test_keystone_flask.py
+++ b/keystone/tests/unit/server/test_keystone_flask.py
@@ -696,3 +696,31 @@ class TestKeystoneFlaskCommon(rest.RestfulTestCase):
                 headers={'Content-Type': 'unrecognized/content-type'}):
             # No exception should be raised, everything is happy.
             json_body.json_body_before_request()
+
+    def test_resource_collection_key_raises_exception_if_unset(self):
+        class TestResource(flask_common.ResourceBase):
+            """A Test Resource."""
+
+        class TestResourceWithKey(flask_common.ResourceBase):
+            collection_key = uuid.uuid4().hex
+
+        r = TestResource()
+        self.assertRaises(ValueError, getattr, r, 'collection_key')
+
+        r = TestResourceWithKey()
+        self.assertEqual(
+            TestResourceWithKey.collection_key, r.collection_key)
+
+    def test_resource_member_key_raises_exception_if_unset(self):
+        class TestResource(flask_common.ResourceBase):
+            """A Test Resource."""
+
+        class TestResourceWithKey(flask_common.ResourceBase):
+            member_key = uuid.uuid4().hex
+
+        r = TestResource()
+        self.assertRaises(ValueError, getattr, r, 'member_key')
+
+        r = TestResourceWithKey()
+        self.assertEqual(
+            TestResourceWithKey.member_key, r.member_key)