Skip to content

Commit 088cf1e

Browse files
committed
issue2551203 - Add support for CORS preflight request
Add support for unauthenticated CORS preflight and fix headers for CORS. client.py: pass through unauthenticated CORS preflight to rest backend. Normal rest OPTION handlers (including tracker defined extensions) can see and handle the request. make some error cases return error json with crrect mime type rather than plain text tracebacks. create new functions to verify origin and referer that filter using allowed origins setting. remove tracker base url from error message is referer is not at an allowed origin. rest.py: fix up OPTION methods handlers to include Access-Control-Allow-Methods that are the same as the Allow header. set cache to one week for all Access-Control headers for CORS preflight only. remove self.client.setHeader("Access-Control-Allow-Origin", "*") and set Access-Control-Allow-Origin to the client supplied origin if it passes allowed origin checks. Required for CORS otherwise data isn't available to caller. Set for all responses. set Vary header now includes Origin as responses can differ based on Origin for all responses. set Access-Control-Allow-Credentials to true on all responses. test_liveserver.py: run server with setting to enforce origin csrf header check run server with setting to enforce x-requested-with csrf header check run server with setting for allowed_api_origins requests now set required csrf headers test preflight request on collections check new headers and Origin is no longer '*' rewrite all compression checks to use a single method with argument to use different compression methods. Reduce a lot of code duplication and makes updating for new headers easier. test_cgi: test new error messages in client.py account for new headers test preflight and new code paths
1 parent 004b3de commit 088cf1e

File tree

5 files changed

+349
-357
lines changed

5 files changed

+349
-357
lines changed

CHANGES.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ Fixed:
9999
empty password to login.
100100
- issue2551207 - Fix sorting by order attribute if order attributes can
101101
be None. Add a test.
102+
- issue2551203 fix CORS requests by providing proper headers and allowing
103+
unauthenticted CORS preflight requests.
102104

103105
Features:
104106

roundup/cgi/client.py

Lines changed: 62 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,32 @@ def handle_rest(self):
620620
self.write(output)
621621
return
622622

623-
if not self.db.security.hasPermission('Rest Access', self.userid):
623+
# allow preflight request even if unauthenticated
624+
if ( self.env['REQUEST_METHOD'] == "OPTIONS"
625+
and self.request.headers.get ("Access-Control-Request-Headers")
626+
and self.request.headers.get ("Access-Control-Request-Method")
627+
and self.request.headers.get ("Origin")
628+
):
629+
# verify Origin is allowed
630+
if not self.is_origin_header_ok(api=True):
631+
# Use code 400 as 401 and 403 imply that authentication
632+
# is needed or authenticates person is not authorized.
633+
# Preflight doesn't do authentication.
634+
self.response_code = 400
635+
msg = self._("Client is not allowed to use Rest Interface.")
636+
output = s2b(
637+
'{ "error": { "status": 400, "msg": "%s" } }'%msg )
638+
self.setHeader("Content-Length", str(len(output)))
639+
self.setHeader("Content-Type", "application/json")
640+
self.write(output)
641+
return
642+
643+
# Call rest library to handle the pre-flight request
644+
handler = rest.RestfulInstance(self, self.db)
645+
output = handler.dispatch(self.env['REQUEST_METHOD'],
646+
self.path, self.form)
647+
648+
elif not self.db.security.hasPermission('Rest Access', self.userid):
624649
self.response_code = 403
625650
output = s2b('{ "error": { "status": 403, "msg": "Forbidden." } }')
626651
self.setHeader("Content-Length", str(len(output)))
@@ -636,14 +661,12 @@ def handle_rest(self):
636661
# raises exception on check failure.
637662
csrf_ok = self.handle_csrf(api=True)
638663
except (Unauthorised, UsageError) as msg:
639-
# report exception back to server
640-
exc_type, exc_value, exc_tb = sys.exc_info()
641664
# FIXME should return what the client requests
642665
# via accept header.
643-
output = s2b("%s: %s\n"%(exc_type, exc_value))
666+
output = s2b('{ "error": { "status": 400, "msg": "%s"}}'% str(msg))
644667
self.response_code = 400
645668
self.setHeader("Content-Length", str(len(output)))
646-
self.setHeader("Content-Type", "text/plain")
669+
self.setHeader("Content-Type", "application/json")
647670
self.write(output)
648671
csrf_ok = False # we had an error, failed check
649672
return
@@ -1223,9 +1246,39 @@ def is_origin_header_ok(self, api=False):
12231246
# Original spec says origin is case sensitive match.
12241247
# Living spec doesn't address Origin value's case or
12251248
# how to compare it. So implement case sensitive....
1226-
if allowed_origins[0] == '*' or origin in allowed_origins:
1249+
if allowed_origins:
1250+
if allowed_origins[0] == '*' or origin in allowed_origins:
1251+
return True
1252+
1253+
return False
1254+
1255+
def is_referer_header_ok(self, api=False):
1256+
referer = self.env['HTTP_REFERER']
1257+
# parse referer and create an origin
1258+
referer_comp = urllib_.urlparse(referer)
1259+
1260+
# self.base always has trailing /, so add trailing / to referer_origin
1261+
referer_origin = "%s://%s/"%(referer_comp[0], referer_comp[1])
1262+
foundat = self.base.find(referer_origin)
1263+
if foundat == 0:
12271264
return True
12281265

1266+
if not api:
1267+
return False
1268+
1269+
allowed_origins = self.db.config['WEB_ALLOWED_API_ORIGINS']
1270+
if allowed_origins[0] == '*':
1271+
return True
1272+
1273+
# For referer, loop over allowed_api_origins and
1274+
# see if any of them are a prefix to referer, case sensitive.
1275+
# Append / to each origin so that:
1276+
# an allowed_origin of https://my.host does not match
1277+
# a referer of https://my.host.com/my/path
1278+
for allowed_origin in allowed_origins:
1279+
foundat = referer_origin.find(allowed_origin + '/')
1280+
if foundat == 0:
1281+
return True
12291282
return False
12301283

12311284
def handle_csrf(self, api=False):
@@ -1335,13 +1388,11 @@ def handle_csrf(self, api=False):
13351388
# self.base always matches: ^https?://hostname
13361389
enforce=config['WEB_CSRF_ENFORCE_HEADER_REFERER']
13371390
if 'HTTP_REFERER' in self.env and enforce != "no":
1338-
referer = self.env['HTTP_REFERER']
1339-
# self.base always has trailing /
1340-
foundat = referer.find(self.base)
1341-
if foundat != 0:
1391+
if not self.is_referer_header_ok(api=api):
1392+
referer = self.env['HTTP_REFERER']
13421393
if enforce in ('required', 'yes'):
13431394
logger.error(self._("csrf Referer header check failed for user%s. Value=%s"), current_user, referer)
1344-
raise Unauthorised(self._("Invalid Referer %s, %s")%(referer,self.base))
1395+
raise Unauthorised(self._("Invalid Referer: %s")%(referer))
13451396
elif enforce == 'logfailure':
13461397
logger.warning(self._("csrf Referer header check failed for user%s. Value=%s"), current_user, referer)
13471398
else:

roundup/rest.py

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1751,6 +1751,10 @@ def options_element(self, class_name, item_id, input):
17511751
"Allow",
17521752
"OPTIONS, GET, PUT, DELETE, PATCH"
17531753
)
1754+
self.client.setHeader(
1755+
"Access-Control-Allow-Methods",
1756+
"OPTIONS, GET, PUT, DELETE, PATCH"
1757+
)
17541758
return 204, ""
17551759

17561760
@Routing.route("/data/<:class_name>/<:item_id>/<:attr_name>", 'OPTIONS')
@@ -1774,12 +1778,20 @@ def option_attribute(self, class_name, item_id, attr_name, input):
17741778
"Allow",
17751779
"OPTIONS, GET, PUT, DELETE, PATCH"
17761780
)
1781+
self.client.setHeader(
1782+
"Access-Control-Allow-Methods",
1783+
"OPTIONS, GET, PUT, DELETE, PATCH"
1784+
)
17771785
elif attr_name in class_obj.getprops(protected=True):
17781786
# It must match a protected prop. These can't be written.
17791787
self.client.setHeader(
17801788
"Allow",
17811789
"OPTIONS, GET"
17821790
)
1791+
self.client.setHeader(
1792+
"Access-Control-Allow-Methods",
1793+
"OPTIONS, GET"
1794+
)
17831795
else:
17841796
raise NotFound('Attribute %s not valid for Class %s' % (
17851797
attr_name, class_name))
@@ -1910,6 +1922,10 @@ def options_describe(self, input):
19101922
"Allow",
19111923
"OPTIONS, GET"
19121924
)
1925+
self.client.setHeader(
1926+
"Access-Control-Allow-Methods",
1927+
"OPTIONS, GET"
1928+
)
19131929
return 204, ""
19141930

19151931
@Routing.route("/data")
@@ -1939,6 +1955,10 @@ def options_data(self, input):
19391955
"Allow",
19401956
"OPTIONS, GET"
19411957
)
1958+
self.client.setHeader(
1959+
"Access-Control-Allow-Methods",
1960+
"OPTIONS, GET"
1961+
)
19421962
return 204, ""
19431963

19441964
@Routing.route("/summary")
@@ -2161,20 +2181,46 @@ def dispatch(self, method, uri, input):
21612181
# with invalid values.
21622182
data_type = ext_type or accept_type or headers.get('Accept') or "invalid"
21632183

2164-
# add access-control-allow-* to support CORS
2165-
self.client.setHeader("Access-Control-Allow-Origin", "*")
2184+
if method.upper() == 'OPTIONS':
2185+
# add access-control-allow-* access-control-max-age to support
2186+
# CORS preflight
2187+
self.client.setHeader(
2188+
"Access-Control-Allow-Headers",
2189+
"Content-Type, Authorization, X-Requested-With, X-HTTP-Method-Override"
2190+
)
2191+
# can be overridden by options handlers to provide supported
2192+
# methods for endpoint
2193+
self.client.setHeader(
2194+
"Access-Control-Allow-Methods",
2195+
"HEAD, OPTIONS, GET, POST, PUT, DELETE, PATCH"
2196+
)
2197+
# cache the Access headings for a week. Allows one CORS pre-flight
2198+
# request to be reused again and again.
2199+
self.client.setHeader("Access-Control-Max-Age", "86400")
2200+
2201+
# response may change based on Origin value.
2202+
self.client.setVary("Origin")
2203+
2204+
# Allow-Origin must match origin supplied by client. '*' doesn't
2205+
# work for authenticated requests.
21662206
self.client.setHeader(
2167-
"Access-Control-Allow-Headers",
2168-
"Content-Type, Authorization, X-Requested-With, X-HTTP-Method-Override"
2207+
"Access-Control-Allow-Origin",
2208+
self.client.request.headers.get("Origin")
21692209
)
2210+
2211+
# allow credentials
21702212
self.client.setHeader(
2171-
"Allow",
2172-
"OPTIONS, GET, POST, PUT, DELETE, PATCH"
2213+
"Access-Control-Allow-Credentials",
2214+
"true"
21732215
)
2216+
# set allow header in case of error. 405 handlers below should
2217+
# replace it with a custom version as will OPTIONS handler
2218+
# doing CORS.
21742219
self.client.setHeader(
2175-
"Access-Control-Allow-Methods",
2176-
"HEAD, OPTIONS, GET, POST, PUT, DELETE, PATCH"
2220+
"Allow",
2221+
"OPTIONS, GET, POST, PUT, DELETE, PATCH"
21772222
)
2223+
21782224
# Is there an input.value with format json data?
21792225
# If so turn it into an object that emulates enough
21802226
# of the FieldStorge methods/props to allow a response.

test/test_cgi.py

Lines changed: 75 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,6 +1169,20 @@ def hasPermission(s, p, classname=None, d=None, e=None, **kw):
11691169
del(cl.env['HTTP_ORIGIN'])
11701170
cl.db.config.WEB_ALLOWED_API_ORIGINS = ""
11711171
del(out[0])
1172+
1173+
# test by setting allowed api origins to *
1174+
# this should not redirect as it is not an API call.
1175+
cl.db.config.WEB_ALLOWED_API_ORIGINS = " * "
1176+
cl.env['HTTP_ORIGIN'] = 'http://whoami.com'
1177+
cl.env['HTTP_REFERER'] = 'https://baz.edu/path/'
1178+
cl.inner_main()
1179+
match_at=out[0].find('Invalid Referer: https://baz.edu/path/')
1180+
print("result of subtest invalid referer:", out[0])
1181+
self.assertEqual(match_at, 36)
1182+
del(cl.env['HTTP_ORIGIN'])
1183+
del(cl.env['HTTP_REFERER'])
1184+
cl.db.config.WEB_ALLOWED_API_ORIGINS = ""
1185+
del(out[0])
11721186

11731187
# clean up from email log
11741188
if os.path.exists(SENDMAILDEBUG):
@@ -1215,7 +1229,7 @@ def wh(s):
12151229
# Should return explanation because content type is text/plain
12161230
# and not text/xml
12171231
cl.handle_rest()
1218-
self.assertEqual(b2s(out[0]), "<class 'roundup.exceptions.UsageError'>: Required Header Missing\n")
1232+
self.assertEqual(b2s(out[0]), '{ "error": { "status": 400, "msg": "Required Header Missing"}}')
12191233
del(out[0])
12201234

12211235
cl = client.Client(self.instance, None,
@@ -1320,7 +1334,7 @@ def wh(s):
13201334
# Should return explanation because content type is text/plain
13211335
# and not text/xml
13221336
cl.handle_rest()
1323-
self.assertEqual(b2s(out[0]), "<class 'roundup.exceptions.Unauthorised'>: Invalid Origin httxs://bar.edu\n")
1337+
self.assertEqual(b2s(out[0]), '{ "error": { "status": 400, "msg": "Invalid Origin httxs://bar.edu"}}')
13241338
del(out[0])
13251339

13261340

@@ -1332,7 +1346,7 @@ def wh(s):
13321346
'HTTP_ORIGIN': 'httxs://bar.edu',
13331347
'HTTP_X_REQUESTED_WITH': 'rest',
13341348
'HTTP_AUTHORIZATION': 'Basic YWRtaW46YWRtaW4=',
1335-
'HTTP_REFERER': 'http://whoami.com/path/',
1349+
'HTTP_REFERER': 'httxp://bar.edu/path/',
13361350
'HTTP_ACCEPT': "application/json;version=1"
13371351
}, form)
13381352
cl.db = self.db
@@ -1346,11 +1360,68 @@ def wh(s):
13461360

13471361
cl.write = wh # capture output
13481362

1349-
# create third issue
1363+
# create fourth issue
13501364
cl.handle_rest()
13511365
self.assertIn('"id": "3"', b2s(out[0]))
13521366
del(out[0])
13531367

1368+
cl.db.config.WEB_ALLOWED_API_ORIGINS = "httxs://bar.foo.edu httxs://bar.edu"
1369+
for referer in [ 'httxs://bar.edu/path/foo',
1370+
'httxs://bar.edu/path/foo?g=zz',
1371+
'httxs://bar.edu']:
1372+
cl = client.Client(self.instance, None,
1373+
{'REQUEST_METHOD':'POST',
1374+
'PATH_INFO':'rest/data/issue',
1375+
'CONTENT_TYPE': 'application/x-www-form-urlencoded',
1376+
'HTTP_ORIGIN': 'httxs://bar.edu',
1377+
'HTTP_X_REQUESTED_WITH': 'rest',
1378+
'HTTP_AUTHORIZATION': 'Basic YWRtaW46YWRtaW4=',
1379+
'HTTP_REFERER': referer,
1380+
'HTTP_ACCEPT': "application/json;version=1"
1381+
}, form)
1382+
cl.db = self.db
1383+
cl.base = 'http://whoami.com/path/'
1384+
cl._socket_op = lambda *x : True
1385+
cl._error_message = []
1386+
cl.request = MockNull()
1387+
h = { 'content-type': 'application/json',
1388+
'accept': 'application/json' }
1389+
cl.request.headers = MockNull(**h)
1390+
1391+
cl.write = wh # capture output
1392+
1393+
# create fourth issue
1394+
cl.handle_rest()
1395+
self.assertIn('"id": "', b2s(out[0]))
1396+
del(out[0])
1397+
1398+
cl.db.config.WEB_ALLOWED_API_ORIGINS = "httxs://bar.foo.edu httxs://bar.edu"
1399+
cl = client.Client(self.instance, None,
1400+
{'REQUEST_METHOD':'POST',
1401+
'PATH_INFO':'rest/data/issue',
1402+
'CONTENT_TYPE': 'application/x-www-form-urlencoded',
1403+
'HTTP_ORIGIN': 'httxs://bar.edu',
1404+
'HTTP_X_REQUESTED_WITH': 'rest',
1405+
'HTTP_AUTHORIZATION': 'Basic YWRtaW46YWRtaW4=',
1406+
'HTTP_REFERER': 'httxp://bar.edu/path/',
1407+
'HTTP_ACCEPT': "application/json;version=1"
1408+
}, form)
1409+
cl.db = self.db
1410+
cl.base = 'http://whoami.com/path/'
1411+
cl._socket_op = lambda *x : True
1412+
cl._error_message = []
1413+
cl.request = MockNull()
1414+
h = { 'content-type': 'application/json',
1415+
'accept': 'application/json' }
1416+
cl.request.headers = MockNull(**h)
1417+
1418+
cl.write = wh # capture output
1419+
1420+
# create fourth issue
1421+
cl.handle_rest()
1422+
self.assertEqual(b2s(out[0]), '{ "error": { "status": 400, "msg": "Invalid Referer: httxp://bar.edu/path/"}}')
1423+
del(out[0])
1424+
13541425
def testXmlrpcCsrfProtection(self):
13551426
# set the password for admin so we can log in.
13561427
passwd=password.Password('admin')

0 commit comments

Comments
 (0)