diff options
author | James E. Blair <jim@acmegating.com> | 2022-09-24 09:35:12 -0700 |
---|---|---|
committer | James E. Blair <jim@acmegating.com> | 2022-10-06 15:38:24 -0700 |
commit | 55ec721fa8f0ca8cdc33b80b04121fbc807b91d6 (patch) | |
tree | 8e368535b24f356f84dae30ae7fa5d5d8361982b | |
parent | 5e6dbf2001fdb4f99f616a33a0fe2f6b44613062 (diff) | |
download | zuul-55ec721fa8f0ca8cdc33b80b04121fbc807b91d6.tar.gz |
Simplify tenant_authorizatons check
This method iterates over all tenants but only needs to return
information about a single tenant. Simplify the calculation for
efficiency.
This includes a change in behavior for unknown tenants. Currently,
a request to /api/tenant/{name}/authorizations will always succeed
even if the tenant does not exist (it will return an authorization
entry indicating the user is not an admin of the unknown tenant).
This is unnecessary and confusing. It will now return a 404 for
the unknown tenant.
In the updated unit test, tenant-two was an unknown tenant; its name
has been updated to 'unknown' to make that clear.
(Since the test asserted that data were returned either way, it is
unclear whether the original author of the unit test expected
tenant-two to be unknown or known.)
Change-Id: I545575fb73ef555b34c207f8a5f2e70935c049aa
-rw-r--r-- | tests/unit/test_web.py | 15 | ||||
-rwxr-xr-x | zuul/web/__init__.py | 30 |
2 files changed, 9 insertions, 36 deletions
diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index 96bc503ff..2f414ad35 100644 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -2902,20 +2902,9 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb): data['zuul']['scope'], "%s got %s" % (authz['sub'], data)) - req = self.get_url('/api/tenant/tenant-two/authorizations', + req = self.get_url('/api/tenant/unknown/authorizations', headers={'Authorization': 'Bearer %s' % token}) - self.assertEqual(200, req.status_code, req.text) - data = req.json() - self.assertTrue('zuul' in data, - "%s got %s" % (authz['sub'], data)) - self.assertTrue('admin' in data['zuul'], - "%s got %s" % (authz['sub'], data)) - self.assertEqual('tenant-two' in test_user['zuul.admin'], - data['zuul']['admin'], - "%s got %s" % (authz['sub'], data)) - self.assertEqual(['tenant-two', ], - data['zuul']['scope'], - "%s got %s" % (authz['sub'], data)) + self.assertEqual(404, req.status_code, req.text) def test_authorizations_no_header(self): """Test that missing Authorization header results in HTTP 401""" diff --git a/zuul/web/__init__.py b/zuul/web/__init__.py index dfa47e431..adb0a7fc9 100755 --- a/zuul/web/__init__.py +++ b/zuul/web/__init__.py @@ -890,7 +890,7 @@ class ZuulWebAPI(object): @cherrypy.expose @cherrypy.tools.json_out(content_type='application/json; charset=utf-8') @cherrypy.tools.handle_options(allowed_methods=['GET', ]) - def tenant_authorizations(self, tenant): + def tenant_authorizations(self, tenant_name): basic_error = self._basic_auth_header_check() if basic_error is not None: return basic_error @@ -898,29 +898,12 @@ class ZuulWebAPI(object): claims, token_error = self._auth_token_check() if token_error is not None: return token_error - try: - admin_tenants = self._authorizations() - except exceptions.AuthTokenException as e: - for header, contents in e.getAdditionalHeaders().items(): - cherrypy.response.headers[header] = contents - cherrypy.response.status = e.HTTPError - return {'description': e.error_description, - 'error': e.error, - 'realm': e.realm} + tenant = self._getTenantOrRaise(tenant_name) + admin = self._is_authorized(tenant, claims) resp = cherrypy.response resp.headers['Access-Control-Allow-Origin'] = '*' - return {'zuul': {'admin': tenant in admin_tenants, - 'scope': [tenant, ]}, } - - def _authorizations(self): - rawToken = cherrypy.request.headers['Authorization'][len('Bearer '):] - claims = self.zuulweb.authenticators.authenticate(rawToken) - - if 'zuul' in claims and 'admin' in claims.get('zuul', {}): - return claims['zuul']['admin'] - - return [n for n, t in self.zuulweb.abide.tenants.items() - if self._is_authorized(t, claims)] + return {'zuul': {'admin': admin, + 'scope': [tenant_name, ]}, } def _tenants(self): result = [] @@ -1895,7 +1878,8 @@ class ZuulWeb(object): if self.authenticators.authenticators: # route order is important, put project actions before the more # generic tenant/{tenant}/project/{project} route - route_map.connect('api', '/api/tenant/{tenant}/authorizations', + route_map.connect('api', + '/api/tenant/{tenant_name}/authorizations', controller=api, action='tenant_authorizations') route_map.connect('api', '/api/tenant/{tenant_name}/promote', |