Merge "REST API: improve tenant scoping of autohold, authorizations"
This commit is contained in:
Zuul
Gerrit Code Review
commit
2986cc09cf
|
|
@ -0,0 +1,11 @@
|
|||
---
|
||||
features:
|
||||
- |
|
||||
REST API: authorizations: add a tenant-scoped endpoint at
|
||||
'/api/tenant/{tenant}/authorizations'. Calling the endpoint will return
|
||||
a list of admin tenants limited to the scoped tenant, if the user has admin
|
||||
privileges on it.
|
||||
deprecations:
|
||||
- |
|
||||
REST API: authorizations: the /api/user/authorizations endpoint is deprecated
|
||||
in favor of the tenant-scoped endpoint. It will be removed next release.
|
||||
|
|
@ -832,6 +832,10 @@ class TestWeb(BaseTestWeb):
|
|||
self.assertEqual("reason text", request['reason'])
|
||||
self.assertEqual([], request['nodes'])
|
||||
|
||||
# Scope the request to tenant-two, not found
|
||||
resp = self.get_url("api/tenant/tenant-two/autohold/%s" % request_id)
|
||||
self.assertEqual(404, resp.status_code, resp.text)
|
||||
|
||||
def test_autohold_list(self):
|
||||
"""test listing autoholds through zuul-web"""
|
||||
client = zuul.rpcclient.RPCClient('127.0.0.1',
|
||||
|
|
@ -1515,14 +1519,7 @@ class TestTenantScopedWebApi(BaseTestWeb):
|
|||
self.assertEqual("some reason", request['reason'])
|
||||
self.assertEqual(1, request['max_count'])
|
||||
|
||||
def test_autohold_delete(self):
|
||||
authz = {'iss': 'zuul_operator',
|
||||
'aud': 'zuul.example.com',
|
||||
'sub': 'testuser',
|
||||
'zuul': {
|
||||
'admin': ['tenant-one', ]
|
||||
},
|
||||
'exp': time.time() + 3600}
|
||||
def _init_autohold_delete(self, authz):
|
||||
token = jwt.encode(authz, key='NoDanaOnlyZuul',
|
||||
algorithm='HS256').decode('utf-8')
|
||||
|
||||
|
|
@ -1541,13 +1538,51 @@ class TestTenantScopedWebApi(BaseTestWeb):
|
|||
self.assertNotEqual([], autohold_requests)
|
||||
self.assertEqual(1, len(autohold_requests))
|
||||
request_id = autohold_requests[0]['id']
|
||||
return client, request_id, token
|
||||
|
||||
def test_autohold_delete_wrong_tenant(self):
|
||||
"""Make sure authorization rules are applied"""
|
||||
authz = {'iss': 'zuul_operator',
|
||||
'aud': 'zuul.example.com',
|
||||
'sub': 'testuser',
|
||||
'zuul': {
|
||||
'admin': ['tenant-one', ]
|
||||
},
|
||||
'exp': time.time() + 3600}
|
||||
client, request_id, _ = self._init_autohold_delete(authz)
|
||||
# now try the autohold-delete API
|
||||
bad_authz = {'iss': 'zuul_operator',
|
||||
'aud': 'zuul.example.com',
|
||||
'sub': 'testuser',
|
||||
'zuul': {
|
||||
'admin': ['tenant-two', ]
|
||||
},
|
||||
'exp': time.time() + 3600}
|
||||
bad_token = jwt.encode(bad_authz, key='NoDanaOnlyZuul',
|
||||
algorithm='HS256').decode('utf-8')
|
||||
resp = self.delete_url(
|
||||
"api/tenant/tenant-one/autohold/%s" % request_id,
|
||||
headers={'Authorization': 'Bearer %s' % bad_token})
|
||||
# Throw a "Forbidden" error, because user is authenticated but not
|
||||
# authorized for tenant-one
|
||||
self.assertEqual(403, resp.status_code, resp.text)
|
||||
# clean up
|
||||
r = client.autohold_delete(request_id)
|
||||
self.assertTrue(r)
|
||||
|
||||
def test_autohold_delete(self):
|
||||
authz = {'iss': 'zuul_operator',
|
||||
'aud': 'zuul.example.com',
|
||||
'sub': 'testuser',
|
||||
'zuul': {
|
||||
'admin': ['tenant-one', ]
|
||||
},
|
||||
'exp': time.time() + 3600}
|
||||
client, request_id, token = self._init_autohold_delete(authz)
|
||||
resp = self.delete_url(
|
||||
"api/tenant/tenant-one/autohold/%s" % request_id,
|
||||
headers={'Authorization': 'Bearer %s' % token})
|
||||
self.assertEqual(204, resp.status_code, resp.text)
|
||||
|
||||
# autohold-list should be empty now
|
||||
resp = self.get_url(
|
||||
"api/tenant/tenant-one/autohold")
|
||||
|
|
@ -1703,12 +1738,16 @@ class TestTenantScopedWebApi(BaseTestWeb):
|
|||
{'action': 'autohold_by_request_id',
|
||||
'path': 'api/tenant/my-tenant/autohold/123',
|
||||
'allowed_methods': ['GET', 'DELETE', ]},
|
||||
{'action': 'authorizations',
|
||||
'path': 'api/user/authorizations',
|
||||
'allowed_methods': ['GET', ]},
|
||||
{'action': 'dequeue',
|
||||
'path': 'api/tenant/my-tenant/project/my-project/enqueue',
|
||||
'allowed_methods': ['POST', ]},
|
||||
{'action': 'authorizations',
|
||||
'path': 'api/tenant/my-tenant/authorizations',
|
||||
'allowed_methods': ['GET', ]},
|
||||
# TODO (mhu) deprecated, remove in next version
|
||||
{'action': 'authorizations',
|
||||
'path': 'api/user/authorizations',
|
||||
'allowed_methods': ['GET', ]},
|
||||
]
|
||||
for endpoint in endpoints:
|
||||
preflight = self.options_url(
|
||||
|
|
@ -1878,9 +1917,13 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
|
|||
'exp': time.time() + 3600}
|
||||
token = jwt.encode(authz, key='NoDanaOnlyZuul',
|
||||
algorithm='HS256').decode('utf-8')
|
||||
# TODO(mhu) deprecated, remove after next release
|
||||
req = self.get_url('/api/user/authorizations',
|
||||
headers={'Authorization': 'Bearer %s' % token})
|
||||
self.assertEqual(401, req.status_code, req.text)
|
||||
req = self.get_url('/api/tenant/tenant-one/authorizations',
|
||||
headers={'Authorization': 'Bearer %s' % token})
|
||||
self.assertEqual(401, req.status_code, req.text)
|
||||
|
||||
def test_user_actions(self):
|
||||
"""Test that users get the right 'zuul.actions' trees"""
|
||||
|
|
@ -1916,6 +1959,7 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
|
|||
authz['exp'] = time.time() + 3600
|
||||
token = jwt.encode(authz, key='NoDanaOnlyZuul',
|
||||
algorithm='HS256').decode('utf-8')
|
||||
# TODO(mhu) deprecated, remove after next release
|
||||
req = self.get_url('/api/user/authorizations',
|
||||
headers={'Authorization': 'Bearer %s' % token})
|
||||
self.assertEqual(200, req.status_code, req.text)
|
||||
|
|
@ -1928,8 +1972,41 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
|
|||
data['zuul']['admin'],
|
||||
"%s got %s" % (authz['sub'], data))
|
||||
|
||||
req = self.get_url('/api/tenant/tenant-one/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-one' in test_user['zuul.admin'],
|
||||
data['zuul']['admin'],
|
||||
"%s got %s" % (authz['sub'], data))
|
||||
self.assertEqual(['tenant-one', ],
|
||||
data['zuul']['scope'],
|
||||
"%s got %s" % (authz['sub'], data))
|
||||
|
||||
req = self.get_url('/api/tenant/tenant-two/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))
|
||||
|
||||
def test_authorizations_no_header(self):
|
||||
"""Test that missing Authorization header results in HTTP 401"""
|
||||
req = self.get_url('/api/tenant/tenant-one/authorizations')
|
||||
self.assertEqual(401, req.status_code, req.text)
|
||||
# TODO(mhu) deprecated, remove after next release
|
||||
req = self.get_url('/api/user/authorizations')
|
||||
self.assertEqual(401, req.status_code, req.text)
|
||||
|
||||
|
|
|
|||
|
|
@ -503,13 +503,13 @@ class ZuulWebAPI(object):
|
|||
@cherrypy.tools.handle_options(allowed_methods=['GET', 'DELETE', ])
|
||||
def autohold_by_request_id(self, tenant, request_id):
|
||||
if cherrypy.request.method == 'GET':
|
||||
return self._autohold_info(request_id)
|
||||
return self._autohold_info(tenant, request_id)
|
||||
elif cherrypy.request.method == 'DELETE':
|
||||
return self._autohold_delete(request_id)
|
||||
return self._autohold_delete(tenant, request_id)
|
||||
else:
|
||||
raise cherrypy.HTTPError(405)
|
||||
|
||||
def _autohold_info(self, request_id):
|
||||
def _autohold_info(self, tenant, request_id):
|
||||
job = self.rpc.submitJob('zuul:autohold_info',
|
||||
{'request_id': request_id})
|
||||
if job.failure:
|
||||
|
|
@ -518,7 +518,11 @@ class ZuulWebAPI(object):
|
|||
request = json.loads(job.data[0])
|
||||
if not request:
|
||||
raise cherrypy.HTTPError(
|
||||
404, 'Hold request %s does not exist.' % request_id)
|
||||
404, 'Hold request %s not found.' % request_id)
|
||||
if tenant != request['tenant']:
|
||||
# return 404 rather than 403 to avoid leaking tenant info
|
||||
raise cherrypy.HTTPError(
|
||||
404, 'Hold request %s not found.' % request_id)
|
||||
resp = cherrypy.response
|
||||
resp.headers['Access-Control-Allow-Origin'] = '*'
|
||||
return {
|
||||
|
|
@ -535,10 +539,9 @@ class ZuulWebAPI(object):
|
|||
'nodes': request['nodes']
|
||||
}
|
||||
|
||||
def _autohold_delete(self, request_id):
|
||||
def _autohold_delete(self, tenant, request_id):
|
||||
# We need tenant info from the request for authz
|
||||
request = self._autohold_info(request_id)
|
||||
|
||||
request = self._autohold_info(tenant, request_id)
|
||||
basic_error = self._basic_auth_header_check()
|
||||
if basic_error is not None:
|
||||
return basic_error
|
||||
|
|
@ -589,12 +592,16 @@ class ZuulWebAPI(object):
|
|||
'buildsets': '/api/tenant/{tenant}/buildsets',
|
||||
'buildset': '/api/tenant/{tenant}/buildset/{uuid}',
|
||||
'config_errors': '/api/tenant/{tenant}/config-errors',
|
||||
# TODO(mhu) remove after next release
|
||||
'authorizations': '/api/user/authorizations',
|
||||
'tenant_authorizations': ('/api/tenant/{tenant}'
|
||||
'/authorizations'),
|
||||
'autohold': '/api/tenant/{tenant}/project/{project:.*}/autohold',
|
||||
'autohold_list': '/api/tenant/{tenant}/autohold',
|
||||
'autohold_by_request_id': '/api/tenant/{tenant}/'
|
||||
'autohold/{request_id}',
|
||||
'autohold_delete': '/api/tenant/{tenant}/autohold/{request_id}',
|
||||
'autohold_by_request_id': ('/api/tenant/{tenant}'
|
||||
'/autohold/{request_id}'),
|
||||
'autohold_delete': ('/api/tenant/{tenant}'
|
||||
'/autohold/{request_id}'),
|
||||
'enqueue': '/api/tenant/{tenant}/project/{project:.*}/enqueue',
|
||||
'dequeue': '/api/tenant/{tenant}/project/{project:.*}/dequeue',
|
||||
'promote': '/api/tenant/{tenant}/promote',
|
||||
|
|
@ -638,11 +645,11 @@ class ZuulWebAPI(object):
|
|||
raise cherrypy.HTTPError(403)
|
||||
return user_authz
|
||||
|
||||
# TODO good candidate for caching
|
||||
# TODO(mhu) deprecated, remove next version
|
||||
@cherrypy.expose
|
||||
@cherrypy.tools.json_out(content_type='application/json; charset=utf-8')
|
||||
@cherrypy.tools.handle_options(allowed_methods=['GET', ])
|
||||
def user_authorizations(self):
|
||||
def authorizations(self):
|
||||
basic_error = self._basic_auth_header_check()
|
||||
if basic_error is not None:
|
||||
return basic_error
|
||||
|
|
@ -650,12 +657,50 @@ 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}
|
||||
return {'zuul': {'admin': admin_tenants}, }
|
||||
|
||||
@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):
|
||||
basic_error = self._basic_auth_header_check()
|
||||
if basic_error is not None:
|
||||
return basic_error
|
||||
# AuthN/AuthZ
|
||||
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}
|
||||
return {'zuul': {'admin': tenant in admin_tenants,
|
||||
'scope': [tenant, ]}, }
|
||||
|
||||
# TODO good candidate for caching
|
||||
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 {'zuul': {'admin': claims['zuul']['admin']}, }
|
||||
job = self.rpc.submitJob('zuul:get_admin_tenants',
|
||||
{'claims': claims})
|
||||
admin_tenants = json.loads(job.data[0])
|
||||
return {'zuul': {'admin': admin_tenants}, }
|
||||
return admin_tenants
|
||||
|
||||
@cherrypy.expose
|
||||
@cherrypy.tools.json_out(content_type='application/json; charset=utf-8')
|
||||
|
|
@ -1222,7 +1267,10 @@ class ZuulWeb(object):
|
|||
# route order is important, put project actions before the more
|
||||
# generic tenant/{tenant}/project/{project} route
|
||||
route_map.connect('api', '/api/user/authorizations',
|
||||
controller=api, action='user_authorizations')
|
||||
controller=api, action='authorizations')
|
||||
route_map.connect('api', '/api/tenant/{tenant}/authorizations',
|
||||
controller=api,
|
||||
action='tenant_authorizations')
|
||||
route_map.connect(
|
||||
'api',
|
||||
'/api/tenant/{tenant}/project/{project:.*}/autohold',
|
||||
|
|
@ -1240,7 +1288,8 @@ class ZuulWeb(object):
|
|||
'/api/tenant/{tenant}/promote',
|
||||
controller=api, action='promote')
|
||||
route_map.connect('api', '/api/tenant/{tenant}/autohold/{request_id}',
|
||||
controller=api, action='autohold_by_request_id')
|
||||
controller=api,
|
||||
action='autohold_by_request_id')
|
||||
route_map.connect('api', '/api/tenant/{tenant}/autohold',
|
||||
controller=api, action='autohold_list')
|
||||
route_map.connect('api', '/api/tenant/{tenant}/projects',
|
||||
|
|
|
|||
Loading…
Reference in New Issue