Merge "zuul-web: support OPTIONS for protected endpoints"

tests/unit/test_web.py

@@ -95,6 +95,10 @@ class BaseTestWeb(ZuulTestCase):
         return requests.delete(
             urllib.parse.urljoin(self.base_url, url), *args, **kwargs)
 
+    def options_url(self, url, *args, **kwargs):
+        return requests.options(
+            urllib.parse.urljoin(self.base_url, url), *args, **kwargs)
+
     def tearDown(self):
         self.executor_server.hold_jobs_in_build = False
         self.executor_server.release()
@@ -1644,6 +1648,62 @@ class TestTenantScopedWebApi(BaseTestWeb):
         self.waitUntilSettled()
         self.assertEqual(self.countJobResults(self.history, 'ABORTED'), 1)
 
+    def test_OPTIONS(self):
+        """Ensure that protected endpoints handle CORS preflight requests
+        properly"""
+        # Note that %tenant, %project are not relevant here. The client is
+        # just checking what the endpoint allows.
+        endpoints = [
+            {'action': 'enqueue',
+             'path': 'api/tenant/my-tenant/project/my-project/enqueue',
+             'allowed_methods': ['POST', ]},
+            {'action': 'enqueue_ref',
+             'path': 'api/tenant/my-tenant/project/my-project/enqueue',
+             'allowed_methods': ['POST', ]},
+            {'action': 'autohold',
+             'path': 'api/tenant/my-tenant/project/my-project/autohold',
+             'allowed_methods': ['GET', 'POST', ]},
+            {'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', ]},
+        ]
+        for endpoint in endpoints:
+            preflight = self.options_url(
+                endpoint['path'],
+                headers={'Access-Control-Request-Method': 'GET',
+                         'Access-Control-Request-Headers': 'Authorization'})
+            self.assertEqual(
+                204,
+                preflight.status_code,
+                "%s failed: %s" % (endpoint['action'], preflight.text))
+            self.assertEqual(
+                '*',
+                preflight.headers.get('Access-Control-Allow-Origin'),
+                "%s failed: %s" % (endpoint['action'], preflight.headers))
+            self.assertEqual(
+                'Authorization, Content-Type',
+                preflight.headers.get('Access-Control-Allow-Headers'),
+                "%s failed: %s" % (endpoint['action'], preflight.headers))
+            allowed_methods = preflight.headers.get(
+                'Access-Control-Allow-Methods').split(', ')
+            self.assertTrue(
+                'OPTIONS' in allowed_methods,
+                "%s has allowed methods: %s" % (endpoint['action'],
+                                                allowed_methods))
+            for method in endpoint['allowed_methods']:
+                self.assertTrue(
+                    method in allowed_methods,
+                    "%s has allowed methods: %s,"
+                    " expected: %s" % (endpoint['action'],
+                                       allowed_methods,
+                                       endpoint['allowed_methods']))
+
 
 class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
     config_file = 'zuul-admin-web-no-override.conf'
@@ -1831,6 +1891,11 @@ class TestTenantScopedWebApiWithAuthRules(BaseTestWeb):
                              data['zuul']['admin'],
                              "%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/user/authorizations')
+        self.assertEqual(401, req.status_code, req.text)
+
 
 class TestTenantScopedWebApiTokenWithExpiry(BaseTestWeb):
     config_file = 'zuul-admin-web-token-expiry.conf'

zuul/web/__init__.py

@@ -68,6 +68,30 @@ class SaveParamsTool(cherrypy.Tool):
 cherrypy.tools.save_params = SaveParamsTool()
 
 
+def handle_options(allowed_methods=None):
+    if cherrypy.request.method == 'OPTIONS':
+        methods = allowed_methods or ['GET', 'OPTIONS']
+        if allowed_methods and 'OPTIONS' not in allowed_methods:
+            methods = methods + ['OPTIONS']
+        # discard decorated handler
+        request = cherrypy.serving.request
+        request.handler = None
+        # Set CORS response headers
+        resp = cherrypy.response
+        resp.headers['Access-Control-Allow-Origin'] = '*'
+        resp.headers['Access-Control-Allow-Headers'] =\
+            ', '.join(['Authorization', 'Content-Type'])
+        resp.headers['Access-Control-Allow-Methods'] =\
+            ', '.join(methods)
+        # Allow caching of the preflight response
+        resp.headers['Access-Control-Max-Age'] = 86400
+        resp.status = 204
+
+
+cherrypy.tools.handle_options = cherrypy.Tool('on_start_resource',
+                                              handle_options)
+
+
 class ChangeFilter(object):
     def __init__(self, desired):
         self.desired = desired
@@ -240,6 +264,7 @@ class ZuulWebAPI(object):
     @cherrypy.expose
     @cherrypy.tools.json_in()
     @cherrypy.tools.json_out(content_type='application/json; charset=utf-8')
+    @cherrypy.tools.handle_options(allowed_methods=['POST', ])
     def dequeue(self, tenant, project):
         basic_error = self._basic_auth_header_check()
         if basic_error is not None:
@@ -284,6 +309,7 @@ class ZuulWebAPI(object):
     @cherrypy.expose
     @cherrypy.tools.json_in()
     @cherrypy.tools.json_out(content_type='application/json; charset=utf-8')
+    @cherrypy.tools.handle_options(allowed_methods=['POST', ])
     def enqueue(self, tenant, project):
         basic_error = self._basic_auth_header_check()
         if basic_error is not None:
@@ -356,6 +382,7 @@ class ZuulWebAPI(object):
 
     @cherrypy.expose
     @cherrypy.tools.json_out(content_type='application/json; charset=utf-8')
+    @cherrypy.tools.handle_options(allowed_methods=['GET', 'POST', ])
     def autohold(self, tenant, project=None):
         # we don't use json_in because a payload is not mandatory with GET
         # Note: GET handling is redundant with autohold_list
@@ -443,6 +470,7 @@ class ZuulWebAPI(object):
 
     @cherrypy.expose
     @cherrypy.tools.json_out(content_type='application/json; charset=utf-8')
+    @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)
@@ -586,7 +614,11 @@ class ZuulWebAPI(object):
     # TODO good candidate for caching
     @cherrypy.expose
     @cherrypy.tools.json_out(content_type='application/json; charset=utf-8')
+    @cherrypy.tools.handle_options(allowed_methods=['GET', ])
     def user_authorizations(self):
+        basic_error = self._basic_auth_header_check()
+        if basic_error is not None:
+            return basic_error
         rawToken = cherrypy.request.headers['Authorization'][len('Bearer '):]
         try:
             claims = self.zuulweb.authenticators.authenticate(rawToken)