Skip to content

Commit

Permalink
PB-934: Removed origin check
Browse files Browse the repository at this point in the history
Allow request if no origin/referer is set. If an origin is set, then returns
correct CORS headers.
  • Loading branch information
ltshb committed Aug 28, 2024
1 parent 0b689ce commit 40189d4
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 85 deletions.
69 changes: 14 additions & 55 deletions app/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from werkzeug.exceptions import HTTPException

from flask import Flask
from flask import abort
from flask import g
from flask import request

Expand Down Expand Up @@ -36,64 +35,24 @@ def log_route():
route_logger.info('%s %s', request.method, request.path)


# Reject request from non allowed origins
@app.before_request
def validate_origin():
# The Origin headers is automatically set by the browser and cannot be changed by the javascript
# application. Unfortunately this header is only set if the request comes from another origin.
# Sec-Fetch-Site header is set to `same-origin` by most of the browser except by Safari !
# The best protection would be to use the Sec-Fetch-Site and Origin header, however this is
# not supported by Safari. Therefore we added a fallback to the Referer header for Safari.
sec_fetch_site = request.headers.get('Sec-Fetch-Site', None)
origin = request.headers.get('Origin', None)
referrer = request.headers.get('Referer', None)

logger.debug(
"Validate origin: sec_fetch_site=%s, origin=%s, referrer=%s",
sec_fetch_site,
origin,
referrer
)

if origin is not None:
if is_domain_allowed(origin):
return
logger.error('Origin=%s does not match %s', origin, ALLOWED_DOMAINS_PATTERN)
abort(403, 'Permission denied')

# BGDIINF_SB-3115: Apparently IOS 16 has a bug and set Sec-Fetch-Site=cross-site even if the
# request is originated (same origin and/or referrer) from the same site ! Therefore to avoid
# issue on IOS we first checks the referrer before checking Sec-Fetch-Site even if this not
# correct.
if referrer is not None:
if is_domain_allowed(referrer):
return
logger.error('Referer=%s does not match %s', referrer, ALLOWED_DOMAINS_PATTERN)
abort(403, 'Permission denied')

if sec_fetch_site is not None:
if sec_fetch_site in ['same-origin', 'same-site']:
return
logger.error('Sec-Fetch-Site=%s is not allowed', sec_fetch_site)
abort(403, 'Permission denied')

logger.error('Referer and/or Origin and/or Sec-Fetch-Site headers not set')
abort(403, 'Permission denied')


# Add CORS Headers to all request
@app.after_request
def add_cors_header(response):
# Do not add CORS header to internal /checker endpoint.
if request.endpoint == 'checker':
return response

response.headers['Access-Control-Allow-Origin'] = request.host_url
# Only add CORS header if an origin or referer header is present in request
# is a host part of our whitelist
cors_origin = None
if 'Origin' in request.headers and is_domain_allowed(request.headers['Origin']):
response.headers['Access-Control-Allow-Origin'] = request.headers['Origin']
response.headers['Vary'] = 'Origin'
response.headers['Access-Control-Allow-Methods'] = 'GET, HEAD, OPTIONS'
response.headers['Access-Control-Allow-Headers'] = '*'
cors_origin = request.headers['Origin']

if 'Referer' in request.headers and is_domain_allowed(request.headers['Referer']):
cors_origin = request.headers['Referer']

if cors_origin:
response.headers['Access-Control-Allow-Origin'] = cors_origin
response.headers['Vary'] = 'Origin'
response.headers['Access-Control-Allow-Methods'] = 'GET, HEAD, OPTIONS'
response.headers['Access-Control-Allow-Headers'] = '*'

return response


Expand Down
46 changes: 16 additions & 30 deletions tests/unit_tests/test_qrcode.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ def assertCors(self, response): # pylint: disable=invalid-name
self.assertIn('Access-Control-Allow-Headers', response.headers)
self.assertEqual(response.headers['Access-Control-Allow-Headers'], '*')

def assertNoCors(self, response): # pylint: disable=invalid-name
self.assertNotIn('Access-Control-Allow-Origin', response.headers)
self.assertNotIn('Access-Control-Allow-Methods', response.headers)
self.assertNotIn('Access-Control-Allow-Headers', response.headers)

def test_checker(self):
response = self.app.get(url_for('checker'), headers=self.valid_origin_header)
self.assertEqual(response.status_code, 200)
Expand All @@ -54,21 +59,6 @@ def test_checker(self):
self.assertEqual(response.json, {"message": "OK", "success": True, "version": APP_VERSION})

def test_generate_errors(self):
response = self.app.get(url_for('generate_get'))
self.assertEqual(response.status_code, 403, msg="ORIGIN must be set")
self.assertCors(response)
self.assertIn('Cache-Control', response.headers, msg="Cache control header missing")
self.assertIn(
'max-age=', response.headers['Cache-Control'], msg="Cache Control max-age not set"
)
self.assertEqual(response.content_type, "application/json")
self.assertEqual(
response.json, {
"error": {
"code": 403, "message": "Permission denied"
}, "success": False
}
)
response = self.app.post(url_for('generate_get'), headers=self.valid_origin_header)
self.assertEqual(response.status_code, 405, msg="POST method is not allowed")
self.assertCors(response)
Expand Down Expand Up @@ -102,11 +92,15 @@ def test_generate_errors(self):
}
)

def test_referer_check(self):
response = self.app.get(url_for('generate_get'), headers={'Referer': 'not allowed'})
self.assertEqual(
response.status_code, 403, msg="Non allowed Referer did not returned an HTTP 403"
def test_no_origin_allowed(self):
response = self.app.get(
url_for('generate_get'),
query_string={'url': 'https://some_random_domain/test'},
)
self.assertEqual(response.status_code, 200)
self.assertNoCors(response)

def test_referer_check(self):
response = self.app.get(
url_for('generate_get'),
query_string={'url': 'https://some_random_domain/test'},
Expand All @@ -115,6 +109,7 @@ def test_referer_check(self):
self.assertEqual(
response.status_code, 200, msg="Allowed Referer did not returned an HTTP 200"
)
self.assertCors(response)

def test_generate_url_domain_restriction(self):
response = self.app.get(
Expand Down Expand Up @@ -157,20 +152,12 @@ def test_generate_origin_not_allowed(self, headers):
query_string={'url': 'https://some_random_domain/test'},
headers=headers
)
self.assertEqual(response.status_code, 403, msg="Domain restriction not applied")
self.assertCors(response)
self.assertNoCors(response)
self.assertIn('Cache-Control', response.headers, msg="Cache control header missing")
self.assertIn(
'max-age=', response.headers['Cache-Control'], msg="Cache Control max-age not set"
)
self.assertEqual(response.content_type, "application/json")
self.assertEqual(
response.json, {
"error": {
"code": 403, "message": "Permission denied"
}, "success": False
}
)
self.assertEqual(response.status_code, 200)

@params(
{'Origin': 'map.geo.admin.ch'},
Expand All @@ -183,7 +170,6 @@ def test_generate_origin_not_allowed(self, headers):
{
'Origin': 'http://localhost', 'Sec-Fetch-Site': 'cross-site'
},
{'Sec-Fetch-Site': 'same-origin'},
{'Referer': 'https://map.geo.admin.ch'},
)
def test_generate_origin_allowed(self, headers):
Expand Down

0 comments on commit 40189d4

Please sign in to comment.