Implement scope_type checking for credentials

This change adds tests cases for the default roles keystone
supports at install time. It also modifies the policies for the
credentials API to be more self-service by properly checking
for various scopes.

Closes-Bug: 1788415
Partial-Bug: 968696

Change-Id: Ifedb7798c96930b6cc0f91159a14a21ac4b02f9f

keystone/api/credentials.py

@@ -21,16 +21,31 @@ from six.moves import http_client
 from keystone.common import provider_api
 from keystone.common import rbac_enforcer
 from keystone.common import validation
+import keystone.conf
 from keystone.credential import schema
 from keystone import exception
 from keystone.i18n import _
 from keystone.server import flask as ks_flask
 
-
+CONF = keystone.conf.CONF
 PROVIDERS = provider_api.ProviderAPIs
 ENFORCER = rbac_enforcer.RBACEnforcer
 
 
+def _build_target_enforcement():
+    target = {}
+    try:
+        target['credential'] = PROVIDERS.credential_api.get_credential(
+            flask.request.view_args.get('credential_id')
+        )
+    except exception.NotFound:  # nosec
+        # Defer existance in the event the credential doesn't exist, we'll
+        # check this later anyway.
+        pass
+
+    return target
+
+
 class CredentialResource(ks_flask.ResourceBase):
     collection_key = 'credentials'
     member_key = 'credential'
@@ -75,17 +90,34 @@ class CredentialResource(ks_flask.ResourceBase):
 
     def _list_credentials(self):
         filters = ['user_id', 'type']
+        if not self.oslo_context.system_scope:
+            target = {'credential': {'user_id': self.oslo_context.user_id}}
+        else:
+            target = None
         ENFORCER.enforce_call(action='identity:list_credentials',
-                              filters=filters)
+                              filters=filters, target_attr=target)
         hints = self.build_driver_hints(filters)
         refs = PROVIDERS.credential_api.list_credentials(hints)
+        # If the request was filtered, make sure to return only the
+        # credentials specific to that user. This makes it so that users with
+        # roles on projects can't see credentials that aren't theirs.
+        if (not self.oslo_context.system_scope and
+                CONF.oslo_policy.enforce_scope):
+            filtered_refs = []
+            for ref in refs:
+                if ref['user_id'] == target['credential']['user_id']:
+                    filtered_refs.append(ref)
+            refs = filtered_refs
         refs = [self._blob_to_json(r) for r in refs]
         return self.wrap_collection(refs, hints=hints)
 
     def _get_credential(self, credential_id):
-        ENFORCER.enforce_call(action='identity:get_credential')
-        ref = PROVIDERS.credential_api.get_credential(credential_id)
-        return self.wrap_member(self._blob_to_json(ref))
+        ENFORCER.enforce_call(
+            action='identity:get_credential',
+            build_target=_build_target_enforcement
+        )
+        credential = PROVIDERS.credential_api.get_credential(credential_id)
+        return self.wrap_member(self._blob_to_json(credential))
 
     def get(self, credential_id=None):
         # Get Credential or List of credentials.
@@ -97,8 +129,12 @@ class CredentialResource(ks_flask.ResourceBase):
 
     def post(self):
         # Create a new credential
-        ENFORCER.enforce_call(action='identity:create_credential')
         credential = flask.request.json.get('credential', {})
+        target = {}
+        target['credential'] = credential
+        ENFORCER.enforce_call(
+            action='identity:create_credential', target_attr=target
+        )
         validation.lazy_validate(schema.credential_create, credential)
         trust_id = getattr(self.oslo_context, 'trust_id', None)
         ref = self._assign_unique_id(
@@ -108,7 +144,12 @@ class CredentialResource(ks_flask.ResourceBase):
 
     def patch(self, credential_id):
         # Update Credential
-        ENFORCER.enforce_call(action='identity:update_credential')
+        ENFORCER.enforce_call(
+            action='identity:update_credential',
+            build_target=_build_target_enforcement
+        )
+        PROVIDERS.credential_api.get_credential(credential_id)
+
         credential = flask.request.json.get('credential', {})
         validation.lazy_validate(schema.credential_update, credential)
         self._require_matching_id(credential)
@@ -118,7 +159,11 @@ class CredentialResource(ks_flask.ResourceBase):
 
     def delete(self, credential_id):
         # Delete credentials
-        ENFORCER.enforce_call(action='identity:delete_credential')
+        ENFORCER.enforce_call(
+            action='identity:delete_credential',
+            build_target=_build_target_enforcement
+        )
+
         return (PROVIDERS.credential_api.delete_credential(credential_id),
                 http_client.NO_CONTENT)
 

keystone/common/policies/credential.py

@@ -10,56 +10,109 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from oslo_log import versionutils
 from oslo_policy import policy
 
 from keystone.common.policies import base
 
+SYSTEM_READER_OR_CRED_OWNER = (
+    '(role:reader and system_scope:all) '
+    'or user_id:%(target.credential.user_id)s'
+)
+SYSTEM_MEMBER_OR_CRED_OWNER = (
+    '(role:member and system_scope:all) '
+    'or user_id:%(target.credential.user_id)s'
+)
+SYSTEM_ADMIN_OR_CRED_OWNER = (
+    '(role:admin and system_scope:all) '
+    'or user_id:%(target.credential.user_id)s'
+)
+
+DEPRECATED_REASON = (
+    'As of the Stein release, the credential API now understands how to '
+    'handle system-scoped tokens in addition to project-scoped tokens, making '
+    'the API more accessible to users without compromising security or '
+    'manageability for administrators. The new default policies for this API '
+    'account for these changes automatically.'
+)
+deprecated_get_credential = policy.DeprecatedRule(
+    name=base.IDENTITY % 'get_credential',
+    check_str=base.RULE_ADMIN_REQUIRED
+)
+deprecated_list_credentials = policy.DeprecatedRule(
+    name=base.IDENTITY % 'list_credentials',
+    check_str=base.RULE_ADMIN_REQUIRED
+)
+deprecated_create_credential = policy.DeprecatedRule(
+    name=base.IDENTITY % 'create_credential',
+    check_str=base.RULE_ADMIN_REQUIRED
+)
+deprecated_update_credential = policy.DeprecatedRule(
+    name=base.IDENTITY % 'update_credential',
+    check_str=base.RULE_ADMIN_REQUIRED
+)
+deprecated_delete_credential = policy.DeprecatedRule(
+    name=base.IDENTITY % 'delete_credential',
+    check_str=base.RULE_ADMIN_REQUIRED
+)
+
+
 credential_policies = [
     policy.DocumentedRuleDefault(
         name=base.IDENTITY % 'get_credential',
-        check_str=base.RULE_ADMIN_REQUIRED,
-        # FIXME(lbragstad): Credentials aren't really project-scoped or
-        # system-scoped. Instead, they are tied to a user. If this API is
-        # called with a system-scoped token, it's a system-administrator and
-        # they should be able to get any credential for management reasons. If
-        # this API is called with a project-scoped token, then extra
-        # enforcement needs to happen based on who created the credential, what
-        # projects they are members of, and the project the token is scoped to.
-        # When we fully support the second case, we can add `project` to the
-        # list of scope_types. This comment applies to the rest of the policies
-        # in this module.
-        # scope_types=['system', 'project'],
+        check_str=SYSTEM_READER_OR_CRED_OWNER,
+        scope_types=['system', 'project'],
         description='Show credentials details.',
         operations=[{'path': '/v3/credentials/{credential_id}',
-                     'method': 'GET'}]),
+                     'method': 'GET'}],
+        deprecated_rule=deprecated_get_credential,
+        deprecated_reason=DEPRECATED_REASON,
+        deprecated_since=versionutils.deprecated.STEIN
+    ),
     policy.DocumentedRuleDefault(
         name=base.IDENTITY % 'list_credentials',
-        check_str=base.RULE_ADMIN_REQUIRED,
-        # scope_types=['system', 'project'],
+        check_str=SYSTEM_READER_OR_CRED_OWNER,
+        scope_types=['system', 'project'],
         description='List credentials.',
         operations=[{'path': '/v3/credentials',
-                     'method': 'GET'}]),
+                     'method': 'GET'}],
+        deprecated_rule=deprecated_list_credentials,
+        deprecated_reason=DEPRECATED_REASON,
+        deprecated_since=versionutils.deprecated.STEIN
+    ),
     policy.DocumentedRuleDefault(
         name=base.IDENTITY % 'create_credential',
-        check_str=base.RULE_ADMIN_REQUIRED,
-        # scope_types=['system', 'project'],
+        check_str=SYSTEM_ADMIN_OR_CRED_OWNER,
+        scope_types=['system', 'project'],
         description='Create credential.',
         operations=[{'path': '/v3/credentials',
-                     'method': 'POST'}]),
+                     'method': 'POST'}],
+        deprecated_rule=deprecated_create_credential,
+        deprecated_reason=DEPRECATED_REASON,
+        deprecated_since=versionutils.deprecated.STEIN
+    ),
     policy.DocumentedRuleDefault(
         name=base.IDENTITY % 'update_credential',
-        check_str=base.RULE_ADMIN_REQUIRED,
-        # scope_types=['system', 'project'],
+        check_str=SYSTEM_MEMBER_OR_CRED_OWNER,
+        scope_types=['system', 'project'],
         description='Update credential.',
         operations=[{'path': '/v3/credentials/{credential_id}',
-                     'method': 'PATCH'}]),
+                     'method': 'PATCH'}],
+        deprecated_rule=deprecated_update_credential,
+        deprecated_reason=DEPRECATED_REASON,
+        deprecated_since=versionutils.deprecated.STEIN
+    ),
     policy.DocumentedRuleDefault(
         name=base.IDENTITY % 'delete_credential',
-        check_str=base.RULE_ADMIN_REQUIRED,
-        # scope_types=['system', 'project'],
+        check_str=SYSTEM_ADMIN_OR_CRED_OWNER,
+        scope_types=['system', 'project'],
         description='Delete credential.',
         operations=[{'path': '/v3/credentials/{credential_id}',
-                     'method': 'DELETE'}])
+                     'method': 'DELETE'}],
+        deprecated_rule=deprecated_delete_credential,
+        deprecated_reason=DEPRECATED_REASON,
+        deprecated_since=versionutils.deprecated.STEIN
+    )
 ]
 
 

keystone/tests/unit/base_classes.py

@@ -41,6 +41,7 @@ class TestCaseWithBootstrap(core.BaseTestCase):
         self.useFixture(database.Database())
         super(TestCaseWithBootstrap, self).setUp()
         self.config_fixture = self.useFixture(config_fixture.Config(CONF))
+        CONF(args=[], project='keystone')
         self.useFixture(
             ksfixtures.KeyRepository(
                 self.config_fixture,

keystone/tests/unit/test_v3_credential.py

@@ -113,6 +113,11 @@ class CredentialTestCase(CredentialBaseTestCase):
 
     def test_list_credentials_filtered_by_type(self):
         """Call ``GET  /credentials?type={type}``."""
+        PROVIDERS.assignment_api.create_system_grant_for_user(
+            self.user_id, self.role_id
+        )
+        token = self.get_system_scoped_token()
+
         # The type ec2 was chosen, instead of a random string,
         # because the type must be in the list of supported types
         ec2_credential = unit.new_credential_ref(user_id=uuid.uuid4().hex,
@@ -123,14 +128,14 @@ class CredentialTestCase(CredentialBaseTestCase):
             ec2_credential['id'], ec2_credential)
 
         # The type cert was chosen for the same reason as ec2
-        r = self.get('/credentials?type=cert')
+        r = self.get('/credentials?type=cert', token=token)
 
         # Testing the filter for two different types
         self.assertValidCredentialListResponse(r, ref=self.credential)
         for cred in r.result['credentials']:
             self.assertEqual('cert', cred['type'])
 
-        r_ec2 = self.get('/credentials?type=ec2')
+        r_ec2 = self.get('/credentials?type=ec2', token=token)
         self.assertThat(r_ec2.result['credentials'], matchers.HasLength(1))
         cred_ec2 = r_ec2.result['credentials'][0]
 
@@ -143,6 +148,11 @@ class CredentialTestCase(CredentialBaseTestCase):
         user1_id = uuid.uuid4().hex
         user2_id = uuid.uuid4().hex
 
+        PROVIDERS.assignment_api.create_system_grant_for_user(
+            self.user_id, self.role_id
+        )
+        token = self.get_system_scoped_token()
+
         # Creating credentials for two different users
         credential_user1_ec2 = unit.new_credential_ref(user_id=user1_id,
                                                        type=CRED_TYPE_EC2)
@@ -156,7 +166,9 @@ class CredentialTestCase(CredentialBaseTestCase):
         PROVIDERS.credential_api.create_credential(
             credential_user2_cert['id'], credential_user2_cert)
 
-        r = self.get('/credentials?user_id=%s&type=ec2' % user1_id)
+        r = self.get(
+            '/credentials?user_id=%s&type=ec2' % user1_id, token=token
+        )
         self.assertValidCredentialListResponse(r, ref=credential_user1_ec2)
         self.assertThat(r.result['credentials'], matchers.HasLength(1))
         cred = r.result['credentials'][0]

releasenotes/notes/bug-1788415-3190279e9c900f76.yaml

@@ -0,0 +1,25 @@
+---
+upgrade:
+  - |
+    [`bug 1788415 <https://bugs.launchpad.net/keystone/+bug/1788415>`_]
+    [`bug 968696 <https://bugs.launchpad.net/keystone/+bug/968696>`_]
+    Policies protecting the ``/v3/credentials`` API have changed defaults in
+    order to make the credentials API more accessible for all users and not
+    just operators or system administrator. Please consider these updates when
+    using this version of keystone since it could affect API behavior in your
+    deployment, especially if you're using a customized policy file.
+security:
+  - |
+    [`bug 1788415 <https://bugs.launchpad.net/keystone/+bug/1788415>`_]
+    [`bug 968696 <https://bugs.launchpad.net/keystone/+bug/968696>`_]
+    More granular policy checks have been applied to the credential API in
+    order to make it more self-service for users. By default, end users will
+    now have the ability to manage their credentials.
+fixes:
+  - |
+    [`bug 1788415 <https://bugs.launchpad.net/keystone/+bug/1788415>`_]
+    [`bug 968696 <https://bugs.launchpad.net/keystone/+bug/968696>`_]
+    Improved self-service support has been implemented in the credential API.
+    This means that end users have the ability to manage their own credentials
+    as opposed to filing tickets to have deployment administrators manage
+    credentials for users.