Merge pull request girder#778 from girder/cors-origin-contains-path

cherrypy request.base can contain paths.

girder/api/rest.py

@@ -26,6 +26,7 @@
 import sys
 import traceback
 import types
+import urlparse
 
 from . import docs
 from girder import events, logger
@@ -366,11 +367,13 @@ def _setCommonCORSHeaders(isOptions=False):
 
     :param isOptions: True if this call is from the options method
     """
-    origin = cherrypy.request.headers.get('origin', '').rstrip('/')
+    origin = cherrypy.request.headers.get('origin', '')
     if not origin:
         if isOptions:
             raise cherrypy.HTTPError(405)
         return
+    originparse = urlparse.urlparse(origin)
+    origin = '%s://%s' % (originparse.scheme, originparse.netloc)
     cherrypy.response.headers['Access-Control-Allow-Origin'] = origin
     cherrypy.response.headers['Vary'] = 'Origin'
     # Some requests do not require further checking
@@ -381,14 +384,19 @@ def _setCommonCORSHeaders(isOptions=False):
                 'multipart/form-data', 'text/plain'))):
         return
     cors = ModelImporter.model('setting').corsSettingsDict()
-    base = cherrypy.request.base.rstrip('/')
+    baseparse = urlparse.urlparse(cherrypy.request.base)
+    base = '%s://%s' % (baseparse.scheme, baseparse.netloc)
+    baselist = [base]
     # We want to handle X-Forwarded-Host be default
     altbase = cherrypy.request.headers.get('X-Forwarded-Host', '')
     if altbase:
-        altbase = '%s://%s' % (cherrypy.request.scheme, altbase)
-        logAltBase = ', forwarded base origin is ' + altbase
+        altbase = altbase.strip('/').split('/')[0]
+        baselist.append('%s://%s' % (cherrypy.request.scheme, altbase))
+        logAltBase = ', forwarded base origin is ' + baselist[-1]
+        if baseparse.scheme != cherrypy.request.scheme:
+            baselist.append('%s://%s' % (baseparse.scheme, altbase))
+            logAltBase += ', also allowing ' + baselist[-1]
     else:
-        altbase = base
         logAltBase = ''
     # If we don't have any allowed origins, return that OPTIONS isn't a
     # valid method.  If the request specified an origin, fail.
@@ -397,14 +405,14 @@ def _setCommonCORSHeaders(isOptions=False):
             logger.info('CORS 405 error: no allowed origins (request origin '
                         'is %s, base origin is %s%s', origin, base, logAltBase)
             raise cherrypy.HTTPError(405)
-        if origin not in (base, altbase):
+        if origin not in baselist:
             logger.info('CORS 403 error: no allowed origins (request origin '
                         'is %s, base origin is %s%s', origin, base, logAltBase)
             raise cherrypy.HTTPError(403)
         return
     # If this origin is not allowed, return an error
     if ('*' not in cors['allowOrigin'] and origin not in cors['allowOrigin']
-            and origin not in (base, altbase)):
+            and origin not in baselist):
         if isOptions:
             logger.info('CORS 405 error: origin not allowed (request origin '
                         'is %s, base origin is %s%s', origin, base, logAltBase)
@@ -413,7 +421,7 @@ def _setCommonCORSHeaders(isOptions=False):
                     'is %s, base origin is %s%s', origin, base, logAltBase)
         raise cherrypy.HTTPError(403)
     # If possible, send back the requesting origin.
-    if origin not in (base, altbase) and not isOptions:
+    if origin not in baselist and not isOptions:
         _validateCORSMethodAndHeaders(cors)
 
 

tests/base.py

@@ -325,7 +325,7 @@ def _buildHeaders(self, headers, cookie, user, token, basicAuth):
     def request(self, path='/', method='GET', params=None, user=None,
                 prefix='/api/v1', isJson=True, basicAuth=None, body=None,
                 type=None, exception=False, cookie=None, token=None,
-                additionalHeaders=None):
+                additionalHeaders=None, useHttps=False):
         """
         Make an HTTP request.
 
@@ -347,6 +347,7 @@ def request(self, path='/', method='GET', params=None, user=None,
         :param additionalHeaders: a list of headers to add to the
                                   request.  Each item is a tuple of the form
                                   (header-name, header-value).
+        :param useHttps: if True, pretend to use https
         :returns: The cherrypy response object from the request.
         """
         if not params:
@@ -372,7 +373,8 @@ def request(self, path='/', method='GET', params=None, user=None,
             qs = urllib.urlencode(params)
 
         app = cherrypy.tree.apps['']
-        request, response = app.get_serving(local, remote, 'http', 'HTTP/1.1')
+        request, response = app.get_serving(
+            local, remote, 'http' if not useHttps else 'https', 'HTTP/1.1')
         request.show_tracebacks = True
 
         self._buildHeaders(headers, cookie, user, token, basicAuth)

tests/cases/routes_test.py

@@ -93,16 +93,17 @@ def testRouteSystem(self):
         self.assertRaises(RestException, dummy.handleRoute, 'DUMMY',
                           ('guid', 'dummy'), {})
 
-    def _testOrigin(self, origin=None, results={}, headers={}):
+    def _testOrigin(self, origin=None, results={}, headers={}, useHttps=False):
         """
         Test CORS responses by querying the dummy endpoints for each method.
 
         :param origin: the origin to use in the request, or None for no
                        origin header.
-        :param results: a dictionary.  The kesy are methods and the values are
+        :param results: a dictionary.  The keys are methods and the values are
                         the expected HTTP response code.  Any method that isn't
                         expected to return a 200 must be present.
         :param headers: a dictionary of additional headers to send.
+        :param useHttps: if True, pretend to use https.
         """
         methods = ['GET', 'PUT', 'POST', 'DELETE', 'PATCH', 'OPTIONS', 'HEAD']
         additionalHeaders = []
@@ -114,7 +115,8 @@ def _testOrigin(self, origin=None, results={}, headers={}):
         for method in methods:
             resp = self.request(
                 path='/dummy/test', method=method, isJson=False,
-                params={'key': 'value'}, additionalHeaders=additionalHeaders)
+                params={'key': 'value'}, additionalHeaders=additionalHeaders,
+                useHttps=useHttps)
             self.assertStatus(resp, results.get(method, 200))
 
     def testCORS(self):
@@ -156,11 +158,24 @@ def testCORS(self):
         # Set a list of allowed origins
         self.model('setting').set(
             SettingKey.CORS_ALLOW_ORIGIN,
-            'http://kitware.com,http://girder.kitware.com')
+            'http://kitware.com,http://girder.kitware.com,'
+            'https://secure.kitware.com')
         self._testOrigin(None, {'OPTIONS': 405})
         self._testOrigin('http://127.0.0.1')
         self._testOrigin('http://kitware.com')
         self._testOrigin('http://girder.kitware.com')
+
+        # Test origins with paths and https.  None should cause a problem
+        self._testOrigin('http://kitware.com/girder')
+        self._testOrigin('https://secure.kitware.com',
+                         headers={'X-Forwarded-Host': 'secure.kitware.com'},
+                         useHttps=True)
+        self._testOrigin('http://secure.kitware.com',
+                         headers={'X-Forwarded-Host': 'secure.kitware.com'},
+                         useHttps=False)
+        self._testOrigin('http://kitware.com',
+                         headers={'X-Forwarded-Host': 'kitware.com/girder'})
+
         # If we specify a different origin, everything should be refused
         self._testOrigin('http://girder2.kitware.com', {
             'PUT': 403, 'DELETE': 403, 'PATCH': 403, 'OPTIONS': 405})