Merge pull request #77 from geoadmin/feat-PB-264_remove_origin_guard_…

…public_cors

PB-264 remove origin guard public cors

README.md

@@ -184,7 +184,6 @@ The service is configured by Environment Variable:
 | Env         | Default               | Description                |
 | ----------- | --------------------- | -------------------------- |
 | LOGGING_CFG | `logging-cfg-local.yml` | Logging configuration file |
-| ALLOWED_DOMAINS | `.*` | Comma separated list of regex that are allowed as domain in Origin header |
 | CACHE_CONTROL | `public, max-age=86400` | Cache Control header value of the `GET /*` endpoints |
 | CACHE_CONTROL_4XX | `public, max-age=3600` | Cache Control header for 4XX responses |
 | FORWARED_ALLOW_IPS | `*` | Sets the gunicorn `forwarded_allow_ips`. See [Gunicorn Doc](https://docs.gunicorn.org/en/stable/settings.html#forwarded-allow-ips). This setting is required in order to `secure_scheme_headers` to work. |

app/__init__.py

@@ -1,17 +1,14 @@
 import logging
-import re
 import time
 
 from werkzeug.exceptions import HTTPException
 
 from flask import Flask
-from flask import abort
 from flask import g
 from flask import request
 
 from app.helpers import make_error_msg
 from app.helpers.service_icon_custom_serializer import CustomJSONEncoder
-from app.settings import ALLOWED_DOMAINS_PATTERN
 from app.settings import CACHE_CONTROL
 from app.settings import CACHE_CONTROL_4XX
 
@@ -25,12 +22,6 @@
 app.json_encoder = CustomJSONEncoder
 
 
-def is_domain_allowed(domain):
-    return re.match(ALLOWED_DOMAINS_PATTERN, domain) is not None
-
-
-# NOTE it is better to have this method registered first (before validate_origin) otherwise
-# the route might not be logged if another method reject the request.
 @app.before_request
 def log_route():
     g.setdefault('started', time.time())
@@ -44,9 +35,7 @@ def add_cors_header(response):
     if request.endpoint == 'checker':
         return response
 
-    response.headers['Access-Control-Allow-Origin'] = request.host_url
-    if 'Origin' in request.headers and is_domain_allowed(request.headers['Origin']):
-        response.headers['Access-Control-Allow-Origin'] = request.headers['Origin']
+    response.headers['Access-Control-Allow-Origin'] = '*'
     response.headers['Vary'] = 'Origin'
     response.headers['Access-Control-Allow-Methods'] = 'GET, HEAD, OPTIONS'
     response.headers['Access-Control-Allow-Headers'] = '*'
@@ -63,44 +52,6 @@ def add_cache_control_header(response):
     return response
 
 
-# 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)
-
-    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')
-
-
 @app.after_request
 def log_response(response):
     logger.info(

app/settings.py

@@ -20,6 +20,5 @@
 
 # Definition of the allowed domains for CORS implementation
 ALLOWED_DOMAINS = os.getenv('ALLOWED_DOMAINS', r'.*').split(',')
-ALLOWED_DOMAINS_PATTERN = f"({'|'.join(ALLOWED_DOMAINS)})"
 CACHE_CONTROL = os.getenv('CACHE_CONTROL', 'public, max-age=86400')
 CACHE_CONTROL_4XX = os.getenv('CACHE_CONTROL_4XX', 'public, max-age=3600')

tests/unit_tests/base_test.py

@@ -1,10 +1,8 @@
-import re
 import unittest
 
 from flask import url_for
 
 from app import app
-from app.settings import ALLOWED_DOMAINS_PATTERN
 
 ORIGIN_FOR_TESTING = "some_random_domain"
 
@@ -28,11 +26,7 @@ def tearDown(self):
 
     def assertCors(self, response):  # pylint: disable=invalid-name
         self.assertIn('Access-Control-Allow-Origin', response.headers)
-        self.assertIsNotNone(
-            re.match(ALLOWED_DOMAINS_PATTERN, response.headers['Access-Control-Allow-Origin']),
-            msg=f"Access-Control-Allow-Origin={response.headers['Access-Control-Allow-Origin']} "
-            f"doesn't match {ALLOWED_DOMAINS_PATTERN}"
-        )
+        self.assertEqual(response.headers['Access-Control-Allow-Origin'], '*')
         self.assertIn('Access-Control-Allow-Methods', response.headers)
         self.assertListEqual(
             sorted(['GET', 'HEAD', 'OPTIONS']),

tests/unit_tests/test_all_icons.py

@@ -109,25 +109,6 @@ def check_image(
                         blue, average_color[2], delta=acceptable_color_delta, msg=error_message
                     )
 
-    @params(
-        None,
-        {'Origin': 'www.example'},
-        {
-            'Origin': 'www.example', 'Sec-Fetch-Site': 'cross-site'
-        },
-        {
-            'Origin': 'www.example', 'Sec-Fetch-Site': 'same-site'
-        },
-        {
-            'Origin': 'www.example', 'Sec-Fetch-Site': 'same-origin'
-        },
-        {'Referer': 'http://www.example'},
-    )
-    def test_icon_set_origin_not_allowed(self, headers):
-        response = self.app.get(url_for('all_icon_sets'), headers=headers)
-        self.assertEqual(response.status_code, 403, msg="Domain restriction not applied")
-        self.assertCors(response)
-
     @params(
         {'Origin': 'map.geo.admin.ch'},
         {

tests/unit_tests/test_checker.py

@@ -12,9 +12,3 @@ def test_checker(self):
         self.assertEqual(response.content_type, "application/json")
         self.assertNotIn('Cache-Control', response.headers)
         self.assertEqual(response.json, {"message": "OK", "success": True, "version": APP_VERSION})
-
-    def test_checker_denied_without_origin(self):
-        response = self.app.get(url_for('checker'), headers={"Origin": ""})
-        self.check_response_not_allowed(
-            response, msg="Should return HTTP 403 when origin is not authorized", is_checker=True
-        )

tests/unit_tests/test_icons.py

@@ -81,16 +81,6 @@ def test_colorized_icon_non_existent_icon_name(self):
             }
         )
 
-    def test_colorized_icon_http_header_origin_restriction(self):
-        response = self.request_colorized_icon(origin="www.dummy.com")
-        self.check_response_not_allowed(
-            response, msg="Should return HTTP 403 when origin is not authorized"
-        )
-        response = self.request_colorized_icon(origin="")
-        self.check_response_not_allowed(
-            response, msg="Should return HTTP 403 when origin is not authorized"
-        )
-
     def test_colorized_icon_no_http_post_method_allowed_on_endpoint(self):
         response = self.app.post(
             url_for(