Merge 20251016

.github/workflows/merge.yml

@@ -34,11 +34,11 @@ jobs:
           activate-environment: true
           enable-cache: true
           cache-dependency-glob: |
-            requirements_for_test.in
-            requirements_for_test.txt
+            requirements_nl_test.in
+            requirements_nl_test.txt
 
       - name: Install application requirements (pip)
-        run: uv pip install -r requirements_for_test.txt
+        run: uv pip install -r requirements_nl_test.txt
 
       - name: Set tag
         id: set-tag
@@ -88,7 +88,7 @@ jobs:
 
       - uses: docker/build-push-action@v6
         with:
-          file: docker/Dockerfile
+          file: docker/Dockerfile_nl
           push: true
           tags: ${{ env.IMAGE }}:latest,${{ env.IMAGE }}:${{ needs.build-tag-test.outputs.tag }}
           platforms: linux/amd64,linux/arm64

.github/workflows/pr.yml

@@ -31,11 +31,11 @@ jobs:
           activate-environment: true
           enable-cache: true
           cache-dependency-glob: |
-            requirements_for_test.in
-            requirements_for_test.txt
+            requirements_nl_test.in
+            requirements_nl_test.txt
 
       - name: Install application requirements (pip)
-        run: uv pip install -r requirements_for_test.txt
+        run: uv pip install -r requirements_nl_test.txt
 
       - uses: astral-sh/ruff-action@v3
 

.pre-commit-config.yaml

@@ -1,4 +1,4 @@
-# This file was automatically copied from notifications-utils@92.1.1
+# This file was automatically copied from notifications-utils@100.2.0
 
 repos:
 - repo: https://github.com/pre-commit/pre-commit-hooks
@@ -9,12 +9,8 @@ repos:
   - id: check-yaml
   - id: debug-statements
 - repo: https://github.com/charliermarsh/ruff-pre-commit
-  rev: 'v0.8.2'
+  rev: 'v0.11.4'
   hooks:
     - id: ruff
       args: [--fix, --exit-non-zero-on-fix]
-- repo: https://github.com/psf/black
-  rev: 24.10.0
-  hooks:
-    - id: black
-      name: black (python)
+    - id: ruff-format

Makefile

@@ -31,7 +31,7 @@ run-flask-with-docker: ## Run flask with docker
 .PHONY: test
 test: ## Run all tests
 	ruff check .
-	black --check .
+	ruff format --check .
 	py.test tests/
 
 .PHONY: test-with-docker

README.md

@@ -4,7 +4,7 @@
 
 ### Python version
 
-Check the version is [runtime.txt](runtime.txt)
+Check the version is the same as at the top of [Dockerfile](docker/Dockerfile)
 
 
 ### uv

app/__init__.py

@@ -5,11 +5,12 @@
 
 from flask import Flask, current_app, jsonify
 from gds_metrics import GDSMetrics
-from notifications_utils import logging, request_helper
+from notifications_utils import request_helper
 from notifications_utils.clients.antivirus.antivirus_client import AntivirusClient
 from notifications_utils.clients.redis.redis_client import RedisClient
 from notifications_utils.eventlet import EventletTimeout
 from notifications_utils.local_vars import LazyLocalGetter
+from notifications_utils.logging import flask as utils_logging
 from werkzeug.local import LocalProxy
 
 from app.config import Config, configs
@@ -48,6 +49,7 @@
 
 
 from app.download.views import download_blueprint
+from app.file_checks.views import file_checks_blueprint
 from app.upload.views import upload_blueprint
 
 mimetypes.init()
@@ -63,7 +65,7 @@ def create_app():
         application.config.from_object(Config)
 
     request_helper.init_app(application)
-    logging.init_app(application)
+    utils_logging.init_app(application)
 
     metrics.init_app(application)
     redis_client.init_app(application)
@@ -73,6 +75,7 @@ def create_app():
 
     application.register_blueprint(download_blueprint)
     application.register_blueprint(upload_blueprint)
+    application.register_blueprint(file_checks_blueprint)
 
     @application.errorhandler(EventletTimeout)
     def eventlet_timeout(error):

app/config.py

@@ -99,7 +99,57 @@ class Development(Config):
     REDIS_URL = os.getenv("REDIS_URL", "redis://localhost:6379/1")
 
 
+################
+### NotifyNL ###
+################
+NL_PREFIX = "notifynl"
+
+
+class ConfigNL(Config):
+    NOTIFY_APP_NAME = "document-download-api"
+
+
+class DevNL(ConfigNL):
+    NOTIFY_ENVIRONMENT = "development"
+
+    DEBUG = True
+    NOTIFY_REQUEST_LOG_LEVEL = "DEBUG"
+
+    DOCUMENTS_BUCKET = f"{NL_PREFIX}-{NOTIFY_ENVIRONMENT}-document-download"
+
+
+class TestNL(ConfigNL):
+    NOTIFY_ENVIRONMENT = "test"
+
+    DEBUG = True
+    NOTIFY_REQUEST_LOG_LEVEL = "DEBUG"
+
+    DOCUMENTS_BUCKET = f"{NL_PREFIX}-{NOTIFY_ENVIRONMENT}-document-download"
+
+
+class AccNL(ConfigNL):
+    NOTIFY_ENVIRONMENT = "acceptance"
+
+    DEBUG = False
+    NOTIFY_REQUEST_LOG_LEVEL = "INFO"
+
+    DOCUMENTS_BUCKET = f"{NL_PREFIX}-{NOTIFY_ENVIRONMENT}-document-download"
+
+
+class ProdNL(ConfigNL):
+    NOTIFY_ENVIRONMENT = "production"
+
+    DEBUG = False
+    NOTIFY_REQUEST_LOG_LEVEL = "ERROR"
+
+    DOCUMENTS_BUCKET = f"{NL_PREFIX}-{NOTIFY_ENVIRONMENT}-document-download"
+
+
 configs = {
-    "test": Test,
     "development": Development,
+    "devnl": DevNL,
+    "test": Test,
+    "testnl": TestNL,
+    "acceptance": AccNL,
+    "production": ProdNL
 }

app/download/views.py

@@ -77,7 +77,7 @@ def download_document(service_id, document_id, extension=None):
                 "document_id": document_id,
             },
         )
-        return jsonify(error=str(e)), 400
+        return jsonify(error=str(e)), e.suggested_status_code
 
     if redirect := get_redirect_url_if_user_not_authenticated(request, document):
         return redirect
@@ -130,24 +130,21 @@ def get_document_metadata(service_id, document_id):
                 "document_id": document_id,
             },
         )
-        return jsonify(error=str(e)), 400
-
-    if metadata:
-        document = {
-            "direct_file_url": get_direct_file_url(
-                service_id=service_id,
-                document_id=document_id,
-                key=key,
-                mimetype=metadata["mimetype"],
-            ),
-            "confirm_email": metadata["confirm_email"],
-            "size_in_bytes": metadata["size"],
-            "file_extension": current_app.config["MIME_TYPES_TO_FILE_EXTENSIONS"][metadata["mimetype"]],
-            "filename": metadata["filename"],
-            "available_until": metadata["available_until"],
-        }
-    else:
-        document = None
+        return jsonify(error=str(e)), e.suggested_status_code
+
+    document = {
+        "direct_file_url": get_direct_file_url(
+            service_id=service_id,
+            document_id=document_id,
+            key=key,
+            mimetype=metadata["mimetype"],
+        ),
+        "confirm_email": metadata["confirm_email"],
+        "size_in_bytes": metadata["size"],
+        "file_extension": current_app.config["MIME_TYPES_TO_FILE_EXTENSIONS"][metadata["mimetype"]],
+        "filename": metadata["filename"],
+        "available_until": metadata["available_until"],
+    }
 
     response = make_response({"document": document})
     response.headers["X-Robots-Tag"] = "noindex, nofollow"

app/file_checks/__init__.py

@@ -0,0 +1 @@
+from .errors import *  # noqa import errors module to register error handlers

app/file_checks/errors.py

@@ -0,0 +1,8 @@
+from flask import jsonify
+
+from .views import file_checks_blueprint
+
+
+@file_checks_blueprint.errorhandler(413)
+def request_entity_too_large(error):
+    return jsonify(error="Uploaded file exceeds file size limit"), 413

app/file_checks/views.py

@@ -0,0 +1,17 @@
+from flask import Blueprint, jsonify, request
+
+from app.utils.authentication import check_auth
+from app.utils.file_checks import AntivirusAndMimeTypeCheckError, UploadedFile
+
+file_checks_blueprint = Blueprint("file_checks", __name__, url_prefix="")
+file_checks_blueprint.before_request(check_auth)
+
+
+@file_checks_blueprint.route("/services/<uuid:service_id>/antivirus-and-mimetype-check", methods=["POST"])
+def get_mime_type_and_run_antivirus_scan(service_id):
+    try:
+        uploaded_file = UploadedFile.from_request_json(request.json, service_id=service_id)
+    except AntivirusAndMimeTypeCheckError as e:
+        return jsonify(error=e.message), e.status_code
+
+    return jsonify(mimetype=uploaded_file.mimetype), 200

app/upload/views.py

@@ -1,109 +1,28 @@
-import mimetypes
-from base64 import b64decode, binascii
-from io import BytesIO
+from flask import Blueprint, jsonify, request
 
-from flask import Blueprint, abort, current_app, jsonify, request
-from notifications_utils.clients.antivirus.antivirus_client import AntivirusError
-from notifications_utils.recipient_validation.errors import InvalidEmailError
-from werkzeug.exceptions import BadRequest
-
-from app import antivirus_client, document_store
-from app.utils import get_mime_type
+from app import document_store
 from app.utils.authentication import check_auth
-from app.utils.files import split_filename
+from app.utils.file_checks import AntivirusAndMimeTypeCheckError, UploadedFile
 from app.utils.urls import get_direct_file_url, get_frontend_download_url
-from app.utils.validation import (
-    clean_and_validate_email_address,
-    clean_and_validate_retention_period,
-    validate_filename,
-)
 
 upload_blueprint = Blueprint("upload", __name__, url_prefix="")
 upload_blueprint.before_request(check_auth)
 
 
-def _get_upload_document_request_data(data):  # noqa: C901
-    if "document" not in data:
-        raise BadRequest("No document upload")
-
-    try:
-        raw_content = b64decode(data["document"])
-    except (binascii.Error, ValueError) as e:
-        raise BadRequest("Document is not base64 encoded") from e
-
-    if len(raw_content) > current_app.config["MAX_DECODED_FILE_SIZE"]:
-        abort(413)
-    file_data = BytesIO(raw_content)
-    is_csv = data.get("is_csv", False)
-
-    if not isinstance(is_csv, bool):
-        raise BadRequest("Value for is_csv must be a boolean")
-
-    confirmation_email = data.get("confirmation_email", None)
-    if confirmation_email is not None:
-        try:
-            confirmation_email = clean_and_validate_email_address(confirmation_email)
-        except InvalidEmailError as e:
-            raise BadRequest(str(e)) from e
-
-    retention_period = data.get("retention_period", None)
-    if retention_period is not None:
-        try:
-            retention_period = clean_and_validate_retention_period(retention_period)
-        except ValueError as e:
-            raise BadRequest(str(e)) from e
-
-    filename = data.get("filename", None)
-    if filename:
-        try:
-            filename = validate_filename(filename)
-        except ValueError as e:
-            raise BadRequest(str(e)) from e
-
-    return file_data, is_csv, confirmation_email, retention_period, filename
-
-
 @upload_blueprint.route("/services/<uuid:service_id>/documents", methods=["POST"])
 def upload_document(service_id):
     try:
-        file_data, is_csv, confirmation_email, retention_period, filename = _get_upload_document_request_data(
-            request.json
-        )
-    except BadRequest as e:
-        return jsonify(error=e.description), 400
-
-    if current_app.config["ANTIVIRUS_ENABLED"]:
-        try:
-            virus_free = antivirus_client.scan(file_data)
-        except AntivirusError:
-            return jsonify(error="Antivirus API error"), 503
-
-        if not virus_free:
-            return jsonify(error="File did not pass the virus scan"), 400
-
-    if filename:
-        mimetype = mimetypes.types_map[split_filename(filename, dotted=True)[1]]
-    else:
-        mimetype = get_mime_type(file_data)
-
-        # Our mimetype auto-detection sometimes resolves CSV content as text/plain, so we use
-        # an explicit POST body parameter `is_csv` from the caller to resolve it as text/csv
-        if is_csv and mimetype == "text/plain":
-            mimetype = "text/csv"
-
-    if mimetype not in current_app.config["MIME_TYPES_TO_FILE_EXTENSIONS"]:
-        allowed_file_types = ", ".join(
-            sorted({f"'.{x}'" for x in current_app.config["FILE_EXTENSIONS_TO_MIMETYPES"].keys()})
-        )
-        return jsonify(error=f"Unsupported file type '{mimetype}'. Supported types are: {allowed_file_types}"), 400
+        uploaded_file = UploadedFile.from_request_json(request.json, service_id=service_id)
+    except AntivirusAndMimeTypeCheckError as e:
+        return jsonify(error=e.message), e.status_code
 
     document = document_store.put(
         service_id,
-        file_data,
-        mimetype=mimetype,
-        confirmation_email=confirmation_email,
-        retention_period=retention_period,
-        filename=filename,
+        uploaded_file.file_data,
+        mimetype=uploaded_file.mimetype,
+        confirmation_email=uploaded_file.confirmation_email,
+        retention_period=uploaded_file.retention_period,
+        filename=uploaded_file.filename,
     )
 
     return (
@@ -115,14 +34,14 @@ def upload_document(service_id):
                     service_id=service_id,
                     document_id=document["id"],
                     key=document["encryption_key"],
-                    mimetype=mimetype,
+                    mimetype=uploaded_file.mimetype,
                 ),
                 "url": get_frontend_download_url(
                     service_id=service_id,
                     document_id=document["id"],
                     key=document["encryption_key"],
                 ),
-                "mimetype": mimetype,
+                "mimetype": uploaded_file.mimetype,
             },
         ),
         201,