Merge "Implement scope_type checking for credentials"

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.