summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJames E. Blair <jim@acmegating.com>2022-09-24 09:35:12 -0700
committerJames E. Blair <jim@acmegating.com>2022-10-06 15:38:24 -0700
commit55ec721fa8f0ca8cdc33b80b04121fbc807b91d6 (patch)
tree8e368535b24f356f84dae30ae7fa5d5d8361982b
parent5e6dbf2001fdb4f99f616a33a0fe2f6b44613062 (diff)
downloadzuul-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.py15
-rwxr-xr-xzuul/web/__init__.py30
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',