Merge pull request #49 from geoadmin/develop

New Release v2.3.0 - #minor

.env.test

@@ -1 +1 @@
-ALLOWED_DOMAINS=some_random_domain
+ALLOWED_DOMAINS=some_random_domain,http://localhost

.github/workflows/pr-auto-semver.yml

@@ -0,0 +1,13 @@
+name: on-pr
+
+on:
+  pull_request:
+    types:
+      - opened
+      - reopened
+      - synchronize
+      - edited
+
+jobs:
+  pr-edit:
+    uses: geoadmin/.github/.github/workflows/pr-auto-semver.yml@master

.github/workflows/semver.yml

@@ -1,4 +1,4 @@
-name: Bump SemVer Version
+name: on-push
 
 on:
   push:
@@ -8,4 +8,4 @@ on:
 
 jobs:
   release:
-    uses: geoadmin/.github/.github/workflows/semver-release.yml@master
+    uses: geoadmin/.github/.github/workflows/semver-release.yml@master

.pylintrc

@@ -62,8 +62,6 @@ confidence=
 # --disable=W".
 disable=missing-docstring,
     missing-module-docstring,
-    bad-continuation,   # bad-continuation doesn't comply with yapf, so lets
-                        # yapf correct indentation continuation issue
     useless-object-inheritance,
 
 
@@ -198,13 +196,6 @@ max-line-length=100
 # Maximum number of lines in a module.
 max-module-lines=1000
 
-# List of optional constructs for which whitespace checking is disabled. `dict-
-# separator` is used to allow tabulation in dicts, etc.: {1  : 1,\n222: 2}.
-# `trailing-comma` allows a space between comma and closing bracket: (a, ).
-# `empty-line` allows space-only lines.
-no-space-check=trailing-comma,
-               dict-separator
-
 # Allow the body of a class to be on the same line as the declaration if body
 # contains single statement.
 single-line-class-stmt=no

Dockerfile

@@ -1,5 +1,5 @@
 # Buster slim python 3.7 base image.
-FROM python:3.7-slim-buster
+FROM python:3.9-slim-buster
 ENV HTTP_PORT 8080
 RUN groupadd -r geoadmin && useradd -r -s /bin/false -g geoadmin geoadmin
 

Pipfile

@@ -8,7 +8,7 @@ PyYAML = "~=5.4"
 gevent = "~=20.9.0"
 gunicorn = "~=19.9.0"
 Flask = "~=2.0"
-Pillow = "~=8.1" # required by qrcode but not in its dependencies
+Pillow = "~=9.1" # required by qrcode but not in its dependencies
 qrcode = "~=7.3"
 logging-utilities = ">=1.2.3,~=1.2"
 python-dotenv = "~=0.19"
@@ -21,4 +21,4 @@ pylint-flask = "*"
 pyzbar = "*"
 
 [requires]
-python_version = "3.7"
+python_version = "3.9"

app/__init__.py

@@ -36,18 +36,26 @@ def log_route():
 # Reject request from non allowed origins
 @app.before_request
 def validate_origin():
-    origin = request.headers.get('Origin')
-    referer = request.headers.get('Referer')
-    if origin is None and referer is None:
-        logger.error('Origin and/or Referer header(s) is/are not set')
+    # 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 None and referrer is None and sec_fetch_site is None:
+        logger.error('Referer and/or Origin and/or Sec-Fetch-Site headers not set')
         abort(403, 'Not allowed')
-    header = 'Origin'
-    value = origin
-    if origin is None:
-        header = 'Referer'
-        value = referer
-    if not re.match(ALLOWED_DOMAINS_PATTERN, value):
-        logger.error('%s=%s is not allowed', header, value)
+    if origin is not None and not re.match(ALLOWED_DOMAINS_PATTERN, origin):
+        logger.error('Origin=%s is not allowed', origin)
+        abort(403, 'Not allowed')
+    if referrer is not None and not re.match(ALLOWED_DOMAINS_PATTERN, referrer):
+        logger.error('Referer=%s is not allowed', referrer)
+        abort(403, 'Not allowed')
+    if sec_fetch_site is not None and sec_fetch_site != 'same-origin':
+        logger.error('Sec-Fetch-Site=%s is not allowed', sec_fetch_site)
         abort(403, 'Not allowed')
 
 
@@ -58,11 +66,11 @@ def add_cors_header(response):
     if request.endpoint == 'checker':
         return response
 
-    if (
-        'Origin' in request.headers and
-        re.match(ALLOWED_DOMAINS_PATTERN, request.headers['Origin'])
-    ):
+    if 'Origin' in request.headers:
         response.headers['Access-Control-Allow-Origin'] = request.headers['Origin']
+    else:
+        response.headers['Access-Control-Allow-Origin'] = request.host_url
+    response.headers['Vary'] = 'Origin'
     response.headers['Access-Control-Allow-Methods'] = 'GET, HEAD, OPTIONS'
     response.headers['Access-Control-Allow-Headers'] = '*'
     return response

buildspec.yml

@@ -1,6 +1,7 @@
 version: 0.2
 
 env:
+  shell: bash
   variables:
     IMAGE_BASE_NAME: "service-qrcode"
     REGISTRY: "974517877189.dkr.ecr.eu-central-1.amazonaws.com"
@@ -14,7 +15,7 @@ env:
 phases:
   install:
     runtime-versions:
-      python: 3.7
+      python: 3.9
     commands:
       - aws --version
       - echo "Login to AWS ECR docker registry"
@@ -25,17 +26,30 @@ phases:
   pre_build:
     commands:
       - echo "export of the image tag for build and push purposes"
-      # Reading git branch (the utility in the deploy script is unable to read it automatically on CodeBuild)
-      # see https://stackoverflow.com/questions/47657423/get-github-git-branch-for-aws-codebuild
-      - export GITHUB_BRANCH="$(git symbolic-ref HEAD --short 2>/dev/null)"
+      - echo "CODEBUILD_WEBHOOK_HEAD_REF=${CODEBUILD_WEBHOOK_HEAD_REF} CODEBUILD_WEBHOOK_BASE_REF=${CODEBUILD_WEBHOOK_BASE_REF}"
       - |
-        if [ "${GITHUB_BRANCH}" = "" ] ; then
-          GITHUB_BRANCH="$(git branch -a --contains HEAD | sed -n 2p | awk '{ printf $1 }')";
-          export GITHUB_BRANCH=${GITHUB_BRANCH#remotes/origin/};
+        if [[ -n "${CODEBUILD_WEBHOOK_HEAD_REF}" ]]; then
+          export GITHUB_BRANCH="${CODEBUILD_WEBHOOK_HEAD_REF#refs/heads/}"
+        else
+          # NOTE: For manual build trigger, CODEBUILD_WEBHOOK_HEAD_REF is not set therefore get
+          # the branch name from git command. This is a bit hacky but did not find any other solution
+          export GITHUB_BRANCH=$(git show-ref --heads | grep $(git --no-pager show --format=%H) | head -1 | awk '{gsub("refs/heads/", ""); print $2}')
         fi
       - export GITHUB_COMMIT=$(echo $CODEBUILD_RESOLVED_SOURCE_VERSION | cut -c 1-7)
       - export GITHUB_TAG="$(git describe --tags 2>/dev/null)"
-      - echo "GITHUB_BRANCH=${GITHUB_BRANCH} GITHUB_COMMIT=${GITHUB_COMMIT} GITHUB_TAG=${GITHUB_TAG} DOCKER_IMG_TAG=${DOCKER_IMG_TAG}"
+      - |
+        if [[ -n "${GITHUB_TAG}" ]]; then
+          export DOCKER_IMG_TAG=${REGISTRY}/${IMAGE_BASE_NAME}:${GITHUB_TAG}
+        else
+          export DOCKER_IMG_TAG=${REGISTRY}/${IMAGE_BASE_NAME}:${GITHUB_BRANCH//\//_}.${GITHUB_COMMIT}
+          export GITHUB_TAG=${GITHUB_COMMIT}
+        fi
+      - export DOCKER_IMG_TAG_LATEST=${REGISTRY}/${IMAGE_BASE_NAME}:${GITHUB_BRANCH//\//_}.latest
+      - echo "GITHUB_BRANCH=${GITHUB_BRANCH}"
+      - echo "GITHUB_COMMIT=${GITHUB_COMMIT}"
+      - echo "GITHUB_TAG=${GITHUB_TAG}"
+      - echo "DOCKER_IMG_TAG=${DOCKER_IMG_TAG}"
+      - echo "DOCKER_IMG_TAG_LATEST=${DOCKER_IMG_TAG_LATEST}"
       - echo "creating a clean environment"
       - make ci
   build:
@@ -45,13 +59,6 @@ phases:
       - echo "Linting code..."
       - make lint
       - echo Build started on $(date)
-      - export DOCKER_IMG_TAG=${REGISTRY}/${IMAGE_BASE_NAME}:${GITHUB_TAG}
-      - export DOCKER_IMG_TAG_LATEST=${REGISTRY}/${IMAGE_BASE_NAME}:${GITHUB_BRANCH}.latest
-      - |-
-        if [ "${GITHUB_TAG}" = "" ] ; then
-          export DOCKER_IMG_TAG=${REGISTRY}/${IMAGE_BASE_NAME}:${GITHUB_BRANCH}.${GITHUB_COMMIT}
-          export GITHUB_TAG=${GITHUB_COMMIT}
-        fi
       - echo "Building docker image with tags ${DOCKER_IMG_TAG} and ${DOCKER_IMG_TAG_LATEST}"
       - >
         docker build

tests/unit_tests/test_qrcode.py

@@ -28,8 +28,10 @@ def setUp(self):
     def assertCors(self, response, check_origin=True):  # pylint: disable=invalid-name
         if check_origin:
             self.assertIn('Access-Control-Allow-Origin', response.headers)
-            self.assertTrue(
-                re.match(ALLOWED_DOMAINS_PATTERN, response.headers['Access-Control-Allow-Origin'])
+            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.assertIn('Access-Control-Allow-Methods', response.headers)
         self.assertListEqual(
@@ -128,6 +130,19 @@ def test_generate_domain_restriction(self):
             'max-age=', response.headers['Cache-Control'], msg="Cache Control max-age not set"
         )
 
+        response = self.app.get(
+            url_for('generate_get'),
+            query_string={'url': 'https://some_random_domain/test'},
+            headers={"Sec-Fetch-Site": "same-origin"}
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertCors(response)
+        self.assertEqual(response.content_type, "image/png")
+        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"
+        )
+
         response = self.app.get(
             url_for('generate_get'),
             query_string={'url': 'https://www.example.com/test'},
@@ -168,6 +183,22 @@ def test_generate_domain_restriction(self):
             }
         )
 
+        response = self.app.get(
+            url_for('generate_get'),
+            query_string={'url': 'https://www.example.com/test'},
+            headers={"Origin": ""}
+        )
+        self.assertEqual(response.status_code, 403, msg="Domain restriction not applied")
+        self.assertCors(response, check_origin=False)
+
+        response = self.app.get(
+            url_for('generate_get'),
+            query_string={'url': 'https://www.example.com/test'},
+            headers={"Sec-Fetch-Site": ""}
+        )
+        self.assertEqual(response.status_code, 403, msg="Domain restriction not applied")
+        self.assertCors(response, check_origin=False)
+
     def test_generate(self):
         long_string_quoted = "value with space & special character !?"
         url_orig = f'https://some_random_domain/test?arg1=value2&arg2={quote(long_string_quoted)}'