Reduce duplication in federated auth APIs
The GET /v3/OS-FEDERATION/projects and GET /v3/OS-FEDERATION/domains
APIs were introduced to handle tokens from federated users, but now
that GET /v3/auth/projects and GET /v3/auth/domains know how to handle
federated tokens, they're just duplicate APIs.
In the past we deprecated these federated auth APIs, but they still
used separate code paths from GET /v3/auth/projects and GET
/v3/auth/domains. The two code paths are true duplication in that they
don't expect to differ over time and should provide the same user
experience.
Instead of running the risk that comes with two code paths that do the
same thing, we should consolidate them.
Conflicts:
keystone/federation/controllers.py due to the fact that pre-Queens
code used a different dependency framework. This was reworked in
the Queens release, causing a conflict with this patch since it
touches the same code.
Co-Authored-By: Kristi Nikolla <kristi@nikolla.me>
Closes-Bug: 1779205
Change-Id: Ib906c42e1dd2c2408ccd2e256ffd876af02af3fe
(cherry picked from commit df5d75571e
)
This commit is contained in:
parent
22af1d9f35
commit
ce46cc25dc
|
@ -447,13 +447,8 @@ class DomainV3(controller.V3Controller):
|
||||||
:returns: list of accessible domains
|
:returns: list of accessible domains
|
||||||
|
|
||||||
"""
|
"""
|
||||||
domains = self.assignment_api.list_domains_for_groups(
|
controller = auth_controllers.Auth()
|
||||||
request.auth_context['group_ids'])
|
return controller.get_auth_domains(request)
|
||||||
domains = domains + self.assignment_api.list_domains_for_user(
|
|
||||||
request.auth_context['user_id'])
|
|
||||||
# remove duplicates
|
|
||||||
domains = [dict(t) for t in set([tuple(d.items()) for d in domains])]
|
|
||||||
return DomainV3.wrap_collection(request.context_dict, domains)
|
|
||||||
|
|
||||||
|
|
||||||
@dependency.requires('assignment_api', 'resource_api')
|
@dependency.requires('assignment_api', 'resource_api')
|
||||||
|
@ -473,14 +468,8 @@ class ProjectAssignmentV3(controller.V3Controller):
|
||||||
:returns: list of accessible projects
|
:returns: list of accessible projects
|
||||||
|
|
||||||
"""
|
"""
|
||||||
projects = self.assignment_api.list_projects_for_groups(
|
controller = auth_controllers.Auth()
|
||||||
request.auth_context['group_ids'])
|
return controller.get_auth_projects(request)
|
||||||
projects = projects + self.assignment_api.list_projects_for_user(
|
|
||||||
request.auth_context['user_id'])
|
|
||||||
# remove duplicates
|
|
||||||
projects = [dict(t) for t in set([tuple(d.items()) for d in projects])]
|
|
||||||
return ProjectAssignmentV3.wrap_collection(request.context_dict,
|
|
||||||
projects)
|
|
||||||
|
|
||||||
|
|
||||||
@dependency.requires('federation_api')
|
@dependency.requires('federation_api')
|
||||||
|
|
|
@ -5126,6 +5126,59 @@ class TestAuthSpecificData(test_v3.RestfulTestCase):
|
||||||
def test_head_projects_with_project_scoped_token(self):
|
def test_head_projects_with_project_scoped_token(self):
|
||||||
self.head('/auth/projects', expected_status=http_client.OK)
|
self.head('/auth/projects', expected_status=http_client.OK)
|
||||||
|
|
||||||
|
def test_get_projects_matches_federated_get_projects(self):
|
||||||
|
# create at least one addition project to make sure it doesn't end up
|
||||||
|
# in the response, since the user doesn't have any authorization on it
|
||||||
|
ref = unit.new_project_ref(domain_id=CONF.identity.default_domain_id)
|
||||||
|
r = self.post('/projects', body={'project': ref})
|
||||||
|
unauthorized_project_id = r.json['project']['id']
|
||||||
|
|
||||||
|
r = self.get('/auth/projects', expected_status=http_client.OK)
|
||||||
|
self.assertThat(r.json['projects'], matchers.HasLength(1))
|
||||||
|
for project in r.json['projects']:
|
||||||
|
self.assertNotEqual(unauthorized_project_id, project['id'])
|
||||||
|
|
||||||
|
expected_project_id = r.json['projects'][0]['id']
|
||||||
|
|
||||||
|
# call GET /v3/OS-FEDERATION/projects
|
||||||
|
r = self.get('/OS-FEDERATION/projects', expected_status=http_client.OK)
|
||||||
|
|
||||||
|
# make sure the response is the same
|
||||||
|
self.assertThat(r.json['projects'], matchers.HasLength(1))
|
||||||
|
for project in r.json['projects']:
|
||||||
|
self.assertEqual(expected_project_id, project['id'])
|
||||||
|
|
||||||
|
def test_get_domains_matches_federated_get_domains(self):
|
||||||
|
# create at least one addition domain to make sure it doesn't end up
|
||||||
|
# in the response, since the user doesn't have any authorization on it
|
||||||
|
ref = unit.new_domain_ref()
|
||||||
|
r = self.post('/domains', body={'domain': ref})
|
||||||
|
unauthorized_domain_id = r.json['domain']['id']
|
||||||
|
|
||||||
|
ref = unit.new_domain_ref()
|
||||||
|
r = self.post('/domains', body={'domain': ref})
|
||||||
|
authorized_domain_id = r.json['domain']['id']
|
||||||
|
|
||||||
|
path = '/domains/%(domain_id)s/users/%(user_id)s/roles/%(role_id)s' % {
|
||||||
|
'domain_id': authorized_domain_id,
|
||||||
|
'user_id': self.user_id,
|
||||||
|
'role_id': self.role_id
|
||||||
|
}
|
||||||
|
self.put(path, expected_status=http_client.NO_CONTENT)
|
||||||
|
|
||||||
|
r = self.get('/auth/domains', expected_status=http_client.OK)
|
||||||
|
self.assertThat(r.json['domains'], matchers.HasLength(1))
|
||||||
|
self.assertEqual(authorized_domain_id, r.json['domains'][0]['id'])
|
||||||
|
self.assertNotEqual(unauthorized_domain_id, r.json['domains'][0]['id'])
|
||||||
|
|
||||||
|
# call GET /v3/OS-FEDERATION/domains
|
||||||
|
r = self.get('/OS-FEDERATION/domains', expected_status=http_client.OK)
|
||||||
|
|
||||||
|
# make sure the response is the same
|
||||||
|
self.assertThat(r.json['domains'], matchers.HasLength(1))
|
||||||
|
self.assertEqual(authorized_domain_id, r.json['domains'][0]['id'])
|
||||||
|
self.assertNotEqual(unauthorized_domain_id, r.json['domains'][0]['id'])
|
||||||
|
|
||||||
def test_get_domains_with_project_scoped_token(self):
|
def test_get_domains_with_project_scoped_token(self):
|
||||||
self.put(path='/domains/%s/users/%s/roles/%s' % (
|
self.put(path='/domains/%s/users/%s/roles/%s' % (
|
||||||
self.domain['id'], self.user['id'], self.role['id']))
|
self.domain['id'], self.user['id'], self.role['id']))
|
||||||
|
|
Loading…
Reference in New Issue