Merge "Fix role_assignments role.id filter"

5 files changed, 51 insertions, 7 deletions

diff --git a/keystone/assignment/core.py b/keystone/assignment/core.py
index 9b50fd62a..6d2025d53 100644
--- a/keystone/assignment/core.py
+++ b/keystone/assignment/core.py
@@ -925,9 +925,11 @@ class Manager(manager.Manager):
                     a['system'] = {'all': True}
                     system_assignments.append(a)
 
-            for i, assignment in enumerate(system_assignments):
-                if role_id and role_id != assignment['role_id']:
-                    system_assignments.pop(i)
+            if role_id:
+                system_assignments = [
+                    sa for sa in system_assignments
+                    if role_id == sa['role_id']
+                ]
 
         assignments = []
         for assignment in itertools.chain(
diff --git a/keystone/tests/protection/v3/test_assignment.py b/keystone/tests/protection/v3/test_assignment.py
index c97a63b08..6441a5612 100644
--- a/keystone/tests/protection/v3/test_assignment.py
+++ b/keystone/tests/protection/v3/test_assignment.py
@@ -385,6 +385,8 @@ class _SystemUserTests(object):
 
     def test_user_can_filter_role_assignments_by_role(self):
         assignments = self._setup_test_role_assignments()
+        self.expected = [ra for ra in self.expected
+                         if ra['role_id'] == assignments['role_id']]
         self.expected.append({
             'user_id': assignments['user_id'],
             'project_id': assignments['project_id'],
@@ -484,6 +486,8 @@ class _SystemUserTests(object):
 
     def test_user_can_filter_role_assignments_by_system_and_role(self):
         assignments = self._setup_test_role_assignments()
+        self.expected = [ra for ra in self.expected
+                         if ra['role_id'] == assignments['role_id']]
         self.expected.append({
             'user_id': assignments['user_id'],
             'system': 'all',
diff --git a/keystone/tests/unit/test_v3.py b/keystone/tests/unit/test_v3.py
index 03a89d345..6c3eaf335 100644
--- a/keystone/tests/unit/test_v3.py
+++ b/keystone/tests/unit/test_v3.py
@@ -1402,6 +1402,8 @@ class AssignmentTestMixin(object):
         """
         if attribs.get('domain_id'):
             link = '/domains/' + attribs['domain_id']
+        elif attribs.get('system'):
+            link = '/system'
         else:
             link = '/projects/' + attribs['project_id']
 
@@ -1429,6 +1431,8 @@ class AssignmentTestMixin(object):
 
         if attribs.get('domain_id'):
             entity['scope'] = {'domain': {'id': attribs['domain_id']}}
+        elif attribs.get('system'):
+            entity['scope'] = {'system': {'all': True}}
         else:
             entity['scope'] = {'project': {'id': attribs['project_id']}}
 
diff --git a/keystone/tests/unit/test_v3_assignment.py b/keystone/tests/unit/test_v3_assignment.py
index a81d855ce..b6844cc77 100644
--- a/keystone/tests/unit/test_v3_assignment.py
+++ b/keystone/tests/unit/test_v3_assignment.py
@@ -920,7 +920,7 @@ class AssignmentTestCase(test_v3.RestfulTestCase,
         self.role2 = unit.new_role_ref()
         PROVIDERS.role_api.create_role(self.role2['id'], self.role2)
 
-        # Now add one of each of the four types of assignment
+        # Now add one of each of the six types of assignment
 
         gd_entity = self.build_role_assignment_entity(
             domain_id=self.domain_id, group_id=group1['id'],
@@ -944,6 +944,22 @@ class AssignmentTestCase(test_v3.RestfulTestCase,
             role_id=self.role2['id'])
         self.put(up_entity['links']['assignment'])
 
+        gs_entity = self.build_role_assignment_entity(
+            system='all',
+            group_id=group1['id'],
+            role_id=self.role1['id'])
+        self.put(gs_entity['links']['assignment'])
+        us_entity = self.build_role_assignment_entity(
+            system='all',
+            user_id=user1['id'],
+            role_id=self.role2['id'])
+        self.put(us_entity['links']['assignment'])
+        us2_entity = self.build_role_assignment_entity(
+            system='all',
+            user_id=user2['id'],
+            role_id=self.role2['id'])
+        self.put(us2_entity['links']['assignment'])
+
         # Now list by various filters to make sure we get back the right ones
 
         collection_url = ('/role_assignments?scope.project.id=%s' %
@@ -970,7 +986,7 @@ class AssignmentTestCase(test_v3.RestfulTestCase,
         r = self.get(collection_url, expected_status=http_client.OK)
         self.head(collection_url, expected_status=http_client.OK)
         self.assertValidRoleAssignmentListResponse(r,
-                                                   expected_length=2,
+                                                   expected_length=3,
                                                    resource_url=collection_url)
         self.assertRoleAssignmentInListResponse(r, up_entity)
         self.assertRoleAssignmentInListResponse(r, ud_entity)
@@ -979,7 +995,7 @@ class AssignmentTestCase(test_v3.RestfulTestCase,
         r = self.get(collection_url, expected_status=http_client.OK)
         self.head(collection_url, expected_status=http_client.OK)
         self.assertValidRoleAssignmentListResponse(r,
-                                                   expected_length=2,
+                                                   expected_length=3,
                                                    resource_url=collection_url)
         self.assertRoleAssignmentInListResponse(r, gd_entity)
         self.assertRoleAssignmentInListResponse(r, gp_entity)
@@ -988,10 +1004,21 @@ class AssignmentTestCase(test_v3.RestfulTestCase,
         r = self.get(collection_url, expected_status=http_client.OK)
         self.head(collection_url, expected_status=http_client.OK)
         self.assertValidRoleAssignmentListResponse(r,
-                                                   expected_length=2,
+                                                   expected_length=3,
                                                    resource_url=collection_url)
         self.assertRoleAssignmentInListResponse(r, gd_entity)
         self.assertRoleAssignmentInListResponse(r, gp_entity)
+        self.assertRoleAssignmentInListResponse(r, gs_entity)
+
+        collection_url = '/role_assignments?role.id=%s' % self.role2['id']
+        r = self.get(collection_url, expected_status=http_client.OK)
+        self.head(collection_url, expected_status=http_client.OK)
+        self.assertValidRoleAssignmentListResponse(r,
+                                                   expected_length=4,
+                                                   resource_url=collection_url)
+        self.assertRoleAssignmentInListResponse(r, ud_entity)
+        self.assertRoleAssignmentInListResponse(r, up_entity)
+        self.assertRoleAssignmentInListResponse(r, us_entity)
 
         # Let's try combining two filers together....
 
diff --git a/releasenotes/notes/bug-1858012-584267ada7e33f2c.yaml b/releasenotes/notes/bug-1858012-584267ada7e33f2c.yaml
new file mode 100644
index 000000000..175dd6c1a
--- /dev/null
+++ b/releasenotes/notes/bug-1858012-584267ada7e33f2c.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    [`bug 1858012 <https://bugs.launchpad.net/keystone/+bug/1858012>`_]
+    Fixes a bug in the /v3/role_assignments filtering where the `role.id` query
+    parameter didn't properly filter role assignments by role in cases where
+    there were multiple system role assignments.