From 50fd6933e8ab5ccf4ef232837fbe582d90c5c913 Mon Sep 17 00:00:00 2001
From: Jeremy Freudberg <jeremyfreudberg@gmail.com>
Date: Tue, 19 Jun 2018 18:54:36 +0000
Subject: [PATCH] Fix duplicate role names in trusts bug

Closes-Bug: #1778109

Change-Id: Id0953190b3b1e0b6765430fbb10d16e7f53f53ee
---
 keystone/tests/unit/test_v3_auth.py                  | 7 -------
 keystone/token/providers/common.py                   | 7 +++++--
 releasenotes/notes/bug-1778109-ea15ce6a8207f857.yaml | 8 ++++++++
 3 files changed, 13 insertions(+), 9 deletions(-)
 create mode 100644 releasenotes/notes/bug-1778109-ea15ce6a8207f857.yaml

diff --git a/keystone/tests/unit/test_v3_auth.py b/keystone/tests/unit/test_v3_auth.py
index 1b6f6131c4..7b98bf5a69 100644
--- a/keystone/tests/unit/test_v3_auth.py
+++ b/keystone/tests/unit/test_v3_auth.py
@@ -44,7 +44,6 @@ from keystone.tests.common import auth as common_auth
 from keystone.tests import unit
 from keystone.tests.unit import ksfixtures
 from keystone.tests.unit import test_v3
-from keystone.tests.unit import utils as test_utils
 
 
 CONF = keystone.conf.CONF
@@ -3944,12 +3943,6 @@ class TrustAPIBehavior(test_v3.RestfulTestCase):
         role_id_set2 = set(r['id'] for r in trust2['roles'])
         self.assertThat(role_id_set1, matchers.GreaterThan(role_id_set2))
 
-    @test_utils.wip(
-        "Waiting on fix for duplicate role names in token data when trust has "
-        "implied roles",
-        expected_exception=matchers.MismatchError,
-        bug="#1778109"
-    )
     def test_trust_with_implied_roles(self):
         # Create some roles
         role1 = unit.new_role_ref()
diff --git a/keystone/token/providers/common.py b/keystone/token/providers/common.py
index a9b3f7c8f0..9105733e8f 100644
--- a/keystone/token/providers/common.py
+++ b/keystone/token/providers/common.py
@@ -372,6 +372,9 @@ class V3TokenDataHelper(provider_api.ProviderAPIMixin, object):
                 refs = [{'role_id': role['id']} for role in trust['roles']]
                 effective_trust_roles = (
                     PROVIDERS.assignment_api.add_implied_roles(refs))
+                effective_trust_role_ids = (
+                    set([r['role_id'] for r in effective_trust_roles])
+                )
                 # Now get the current role assignments for the trustor,
                 # including any domain specific roles.
                 assignments = PROVIDERS.assignment_api.list_role_assignments(
@@ -384,10 +387,10 @@ class V3TokenDataHelper(provider_api.ProviderAPIMixin, object):
                 # Go through each of the effective trust roles, making sure the
                 # trustor still has them, if any have been removed, then we
                 # will treat the trust as invalid
-                for trust_role in effective_trust_roles:
+                for trust_role_id in effective_trust_role_ids:
 
                     match_roles = [x for x in current_effective_trustor_roles
-                                   if x == trust_role['role_id']]
+                                   if x == trust_role_id]
                     if match_roles:
                         role = PROVIDERS.role_api.get_role(match_roles[0])
                         if role['domain_id'] is None:
diff --git a/releasenotes/notes/bug-1778109-ea15ce6a8207f857.yaml b/releasenotes/notes/bug-1778109-ea15ce6a8207f857.yaml
new file mode 100644
index 0000000000..b4c76c4b65
--- /dev/null
+++ b/releasenotes/notes/bug-1778109-ea15ce6a8207f857.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    [`bug 1778109 <https://bugs.launchpad.net/keystone/+bug/1778109>`_]
+    Previously the token data for a trust-scoped token may have contained
+    duplicate roles, when implied roles were present. This is no longer the
+    case, for the sake of accuracy and to prevent the breaking of applications
+    which may consume this role list.