diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index 1fe5c46ed7..23934ab18c 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/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( diff --git a/keystone/token/providers/common.py b/keystone/token/providers/common.py index d29d6f8326..7b4e1b1f67 100644 --- a/keystone/token/providers/common.py +++ b/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: diff --git a/releasenotes/notes/bug-1701324-739a31f38037f77b.yaml b/releasenotes/notes/bug-1701324-739a31f38037f77b.yaml new file mode 100644 index 0000000000..d47e5aedd3 --- /dev/null +++ b/releasenotes/notes/bug-1701324-739a31f38037f77b.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + [`bug 1701324 `_] + Token bodies now contain only unique roles in the authentication response.