Fix credential list for project members

Without this patch, project members and readers can list any credentials
with the /v3/credentials API when enforce_scope is false. enforce_scope
is only applicable to project admins due to the admin-ness problem[1],
and this policy is not meant to allow project admins any access to users'
credentials (only system admins should be able to access them). However,
when enforce_scope is false, we need to preserve the old behavior of
project admins being able to list all credentials. This change mitigates
the problem by running the identity:get_credential policy check to
filter out credentials the user does not have access to. This will
impact performance.

Closes-bug: #1855080

[1] https://bugs.launchpad.net/keystone/+bug/968696

Change-Id: I5dd85a6b8368373a27aef2942a64499d020662ef
(cherry picked from commit 17c337dbdbfb9d548ad531c2ad0483c9bce5b98f)
(cherry picked from commit bd3f63787151183f4daa43578aa491856fefae5b)

3 files changed, 151 insertions, 7 deletions

diff --git a/keystone/api/credentials.py b/keystone/api/credentials.py
index 9487f1914..08a492c1d 100644
--- a/keystone/api/credentials.py
+++ b/keystone/api/credentials.py
@@ -101,13 +101,22 @@ class CredentialResource(ks_flask.ResourceBase):
         # 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
+        filtered_refs = []
+        for ref in refs:
+            # Check each credential again to make sure the user has access to
+            # it, either by owning it, being a project admin with
+            # enforce_scope=false, being a system user, or having some other
+            # custom policy that allows access.
+            try:
+                cred = PROVIDERS.credential_api.get_credential(ref['id'])
+                ENFORCER.enforce_call(
+                    action='identity:get_credential',
+                    target_attr={'credential': cred}
+                )
+                filtered_refs.append(ref)
+            except exception.Forbidden:
+                pass
+        refs = filtered_refs
         refs = [self._blob_to_json(r) for r in refs]
         return self.wrap_collection(refs, hints=hints)
 
diff --git a/keystone/tests/unit/protection/v3/test_credentials.py b/keystone/tests/unit/protection/v3/test_credentials.py
index b681e7ee5..695e76526 100644
--- a/keystone/tests/unit/protection/v3/test_credentials.py
+++ b/keystone/tests/unit/protection/v3/test_credentials.py
@@ -1138,3 +1138,115 @@ class ProjectAdminTests(base_classes.TestCaseWithBootstrap,
                 'identity:delete_credential': cp.SYSTEM_ADMIN_OR_CRED_OWNER
             }
             f.write(jsonutils.dumps(overridden_policies))
+
+
+class ProjectReaderTestsEnforceScopeFalse(base_classes.TestCaseWithBootstrap,
+                                          common_auth.AuthTestMixin,
+                                          _UserCredentialTests,
+                                          _ProjectUsersTests):
+
+    def setUp(self):
+        super(ProjectReaderTestsEnforceScopeFalse, self).setUp()
+        self.loadapp()
+        self.useFixture(ksfixtures.Policy(self.config_fixture))
+        self.config_fixture.config(group='oslo_policy', enforce_scope=False)
+
+        project_reader = unit.new_user_ref(
+            domain_id=CONF.identity.default_domain_id
+        )
+        self.user_id = PROVIDERS.identity_api.create_user(
+            project_reader
+        )['id']
+        project = unit.new_project_ref(
+            domain_id=CONF.identity.default_domain_id
+        )
+        self.project_id = PROVIDERS.resource_api.create_project(
+            project['id'], project
+        )['id']
+        PROVIDERS.assignment_api.create_grant(
+            self.bootstrapper.reader_role_id, user_id=self.user_id,
+            project_id=self.project_id
+        )
+
+        auth = self.build_authentication_request(
+            user_id=self.user_id,
+            password=project_reader['password'],
+            project_id=self.project_id
+        )
+
+        # Grab a token using the persona we're testing and prepare headers
+        # for requests we'll be making in the tests.
+        with self.test_client() as c:
+            r = c.post('/v3/auth/tokens', json=auth)
+            self.token_id = r.headers['X-Subject-Token']
+            self.headers = {'X-Auth-Token': self.token_id}
+
+
+class ProjectMemberTestsEnforceScopeFalse(base_classes.TestCaseWithBootstrap,
+                                          common_auth.AuthTestMixin,
+                                          _UserCredentialTests,
+                                          _ProjectUsersTests):
+
+    def setUp(self):
+        super(ProjectMemberTestsEnforceScopeFalse, self).setUp()
+        self.loadapp()
+        self.useFixture(ksfixtures.Policy(self.config_fixture))
+        self.config_fixture.config(group='oslo_policy', enforce_scope=False)
+
+        project_member = unit.new_user_ref(
+            domain_id=CONF.identity.default_domain_id
+        )
+        self.user_id = PROVIDERS.identity_api.create_user(
+            project_member
+        )['id']
+        project = unit.new_project_ref(
+            domain_id=CONF.identity.default_domain_id
+        )
+        self.project_id = PROVIDERS.resource_api.create_project(
+            project['id'], project
+        )['id']
+        PROVIDERS.assignment_api.create_grant(
+            self.bootstrapper.member_role_id, user_id=self.user_id,
+            project_id=self.project_id
+        )
+
+        auth = self.build_authentication_request(
+            user_id=self.user_id,
+            password=project_member['password'],
+            project_id=self.project_id
+        )
+
+        # Grab a token using the persona we're testing and prepare headers
+        # for requests we'll be making in the tests.
+        with self.test_client() as c:
+            r = c.post('/v3/auth/tokens', json=auth)
+            self.token_id = r.headers['X-Subject-Token']
+            self.headers = {'X-Auth-Token': self.token_id}
+
+
+class ProjectAdminTestsEnforceScopeFalse(base_classes.TestCaseWithBootstrap,
+                                         common_auth.AuthTestMixin,
+                                         _UserCredentialTests,
+                                         _SystemUserCredentialTests):
+
+    def setUp(self):
+        super(ProjectAdminTestsEnforceScopeFalse, self).setUp()
+        self.loadapp()
+        self.useFixture(ksfixtures.Policy(self.config_fixture))
+        self.config_fixture.config(group='oslo_policy', enforce_scope=False)
+
+        # Reuse the system administrator account created during
+        # ``keystone-manage bootstrap``
+        self.user_id = self.bootstrapper.admin_user_id
+        auth = self.build_authentication_request(
+            user_id=self.user_id,
+            password=self.bootstrapper.admin_password,
+            project_id=self.bootstrapper.project_id
+        )
+
+        # Grab a token using the persona we're testing and prepare headers
+        # for requests we'll be making in the tests.
+        with self.test_client() as c:
+            r = c.post('/v3/auth/tokens', json=auth)
+            self.token_id = r.headers['X-Subject-Token']
+            self.headers = {'X-Auth-Token': self.token_id}
diff --git a/releasenotes/notes/bug-1855080-08b28181b7cb2470.yaml b/releasenotes/notes/bug-1855080-08b28181b7cb2470.yaml
new file mode 100644
index 000000000..9e47134ac
--- /dev/null
+++ b/releasenotes/notes/bug-1855080-08b28181b7cb2470.yaml
@@ -0,0 +1,23 @@
+---
+critical:
+  - |
+    [`bug 1855080 <https://bugs.launchpad.net/keystone/+bug/1855080>`_]
+    An error in the policy target filtering inadvertently allowed any user to
+    list any credential object with the /v3/credentials API when
+    ``[oslo_policy]/enforce_scope`` was set to false, which is the default.
+    This has been addressed: users with non-admin roles on a project may not
+    list other users' credentials. However, users with the admin role on a
+    project may still list any users credentials when
+    ``[oslo_policy]/enforce_scope`` is false due to `bug 968696
+    <https://bugs.launchpad.net/keystone/+bug/968696>`_.
+security:
+  - |
+    [`bug 1855080 <https://bugs.launchpad.net/keystone/+bug/1855080>`_]
+    An error in the policy target filtering inadvertently allowed any user to
+    list any credential object with the /v3/credentials API when
+    ``[oslo_policy]/enforce_scope`` was set to false, which is the default.
+    This has been addressed: users with non-admin roles on a project may not
+    list other users' credentials. However, users with the admin role on a
+    project may still list any users credentials when
+    ``[oslo_policy]/enforce_scope`` is false due to `bug 968696
+    <https://bugs.launchpad.net/keystone/+bug/968696>`_.