Remove duplicate roles from federated auth

We were using a one-liner to prune duplicate role references from a
list of roles, but it didn't work in all cases. This reworks the
logic to pass the existing test case. I also added a comment
explaining why the logic we used previously doesn't work so we can
hopefully avoid the pattern in the future.

Change-Id: Id786d6463364ad8f4f02c22bb83221baac4b83d0
Closes-Bug: 1701324

keystone/tests/unit/test_v3_federation.py

@@ -2087,6 +2087,29 @@ class FederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin):
             self._check_project_scoped_token_attributes(token_resp,
                                                         project_id_ref)
 
+    def test_scope_to_project_with_duplicate_roles_returns_single_role(self):
+        r = self.v3_create_token(self.TOKEN_SCOPE_PROJECT_EMPLOYEE_FROM_ADMIN)
+
+        # Even though the process of obtaining a token shows that there is a
+        # role assignment on a project, we should attempt to create a duplicate
+        # assignment somewhere. Do this by creating a direct role assignment
+        # with each role against the project the token was scoped to.
+        user_id = r.json_body['token']['user']['id']
+        project_id = r.json_body['token']['project']['id']
+        for role in r.json_body['token']['roles']:
+            self.assignment_api.create_grant(
+                role_id=role['id'], user_id=user_id, project_id=project_id
+            )
+
+        # Ensure all roles in the token are unique even though we know there
+        # should be duplicate role assignment from the assertions and the
+        # direct role assignments we just created.
+        r = self.v3_create_token(self.TOKEN_SCOPE_PROJECT_EMPLOYEE_FROM_ADMIN)
+        known_role_ids = []
+        for role in r.json_body['token']['roles']:
+            self.assertNotIn(role['id'], known_role_ids)
+            known_role_ids.append(role['id'])
+
     def test_scope_to_project_with_only_inherited_roles(self):
         """Try to scope token whose only roles are inherited."""
         r = self.v3_create_token(

keystone/token/providers/common.py

@@ -192,11 +192,25 @@ class V3TokenDataHelper(object):
         roles = roles + self._get_roles_for_user(user_id, domain_id,
                                                  project_id)
 
-        # remove duplicates
-        roles = [dict(t) for t in set([tuple(d.items()) for d in roles])]
+        # NOTE(lbragstad): Remove duplicate role references from a list of
+        # roles. It is often suggested that this be done with:
+        #
+        # roles = [dict(t) for t in set([tuple(d.items()) for d in roles])]
+        #
+        # But that doesn't actually remove duplicates in all cases and causes
+        # transient failures because dictionaries are unordered objects. This
+        # means {'id': 1, 'foo': 'bar'} and {'foo': 'bar', 'id': 1} won't
+        # actually resolve to a single entity in the above logic since they are
+        # both considered unique. By using `in` we're performing a containment
+        # check, which also does a deep comparison of the objects, which is
+        # what we want.
+        unique_roles = []
+        for role in roles:
+            if role not in unique_roles:
+                unique_roles.append(role)
 
-        check_roles(roles, user_id, project_id, domain_id)
-        token_data['roles'] = roles
+        check_roles(unique_roles, user_id, project_id, domain_id)
+        token_data['roles'] = unique_roles
 
     def _populate_user(self, token_data, user_id, trust):
         if 'user' in token_data:

releasenotes/notes/bug-1701324-739a31f38037f77b.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    [`bug 1701324 <https://bugs.launchpad.net/keystone/+bug/1701324>`_]
+    Token bodies now contain only unique roles in the authentication response.