Browse Source

REST API: improve tenant scoping of autohold, authorizations

Improve whitelabeling support of the REST API.

* autohold-info now checks that the tenant in the REST query matches the
  tenant of the autohold request looked up by id. Return a 404 Not Found if the
  autohold request's tenant doesn't match the REST query's tenant.
* autohold-delete: DELETE /api/tenant/{tenant}/autohold/{id}
  returns a 403 Forbidden if the autohold id does not match the tenant
* authorizations: /api/user/authorizations is deprecated in favor of
  the scoped endpoint: GET /api/tenant/{tenant}/authorizations which
  returns a list of authorized tenants scoped to {tenant}, ie either
  [tenant,] or [] depending on the user's authorizations.

Change-Id: Ibbe5e07a886d54ecd641bb64f02e28dbf8025659
changes/18/728118/33
Matthieu Huin 6 months ago
parent
commit
179775b839
3 changed files with 164 additions and 27 deletions
  1. +11
    -0
      releasenotes/notes/scoping_REST_API-866574c4d73c577a.yaml
  2. +89
    -12
      tests/unit/test_web.py
  3. +64
    -15
      zuul/web/__init__.py

+ 11
- 0
releasenotes/notes/scoping_REST_API-866574c4d73c577a.yaml View File

@@ -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.

+ 89
- 12
tests/unit/test_web.py View File

@@ -830,6 +830,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',
@@ -1513,14 +1517,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')

@@ -1539,13 +1536,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")
@@ -1701,12 +1736,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(
@@ -1876,9 +1915,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"""
@@ -1914,6 +1957,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)
@@ -1926,8 +1970,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)



+ 64
- 15
zuul/web/__init__.py View File

@@ -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,33 @@ 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 authorizations(self):
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': admin_tenants}, }

@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 tenant_authorizations(self, tenant):
basic_error = self._basic_auth_header_check()
if basic_error is not None:
return basic_error
@@ -650,12 +679,28 @@ 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': 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…
Cancel
Save