Skip to content

✨(backend) make forward auth request uri header configurable#2241

Merged
lunika merged 1 commit intosuitenumerique:mainfrom
erincandescent:forward-auth-config
Apr 28, 2026
Merged

✨(backend) make forward auth request uri header configurable#2241
lunika merged 1 commit intosuitenumerique:mainfrom
erincandescent:forward-auth-config

Conversation

@erincandescent
Copy link
Copy Markdown
Contributor

Purpose

In our deployment we're using Traefik, not nginx, as an ingress. Traefik uses X-Forwarded-Uri instead of X-Original-Url. This adds a setting which lets users adapt Docs to their ingress proxy of choice

Proposal

Add a MEDIA_AUTH_URL_PARAMETER (I'm not 100% happy about the name) which lets you configure it. We set it to HTTP_X_FORWARDED_URI

External contributions

Thank you for your contribution! 🎉

Please ensure the following items are checked before submitting your pull request:

General requirements

  • I have read and followed the contributing guidelines
  • I have read and agreed to the Code of Conduct
  • I have added corresponding tests for new features or bug fixes (if applicable)
    • Not sure if applicable

CI requirements

  • I made sure that all existing tests are passing
  • I have signed off my commits with git commit --signoff (DCO compliance)
  • I have signed my commits with my SSH or GPG key (git commit -S)
  • My commit messages follow the required format: <gitmoji>(type) title description
  • I have added a changelog entry under ## [Unreleased] section (if noticeable change)

AI requirements

Skip the checkboxes below 👇 If you didn't use AI for your contribution

  • I used AI assistance to produce part or all of this contribution
  • I have read, reviewed, understood and can explain the code I am submitting
  • I can jump in a call or a chat to explain my work to a maintainer
  • This code was written by hand

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 23, 2026

Walkthrough

The PR makes the original-media URL header name configurable. It adds a new setting MEDIA_AUTH_ORIGINAL_URL_HEADER (default "HTTP_X_ORIGINAL_URL", overridable via environment) and updates _auth_get_original_url to read the original URL from request.META using that configured header key instead of hardcoding HTTP_X_ORIGINAL_URL. The method’s debug log and docstring were adjusted to report the configured header and reference reverse-proxy/nginx header mapping. Tests and documentation were updated to cover and document the new setting.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: making the forward auth request URI header configurable. The emoji and format are appropriate, and the message clearly conveys the primary objective of the PR.
Description check ✅ Passed The description is directly related to the changeset, explaining the business need (Traefik support instead of nginx), the proposed solution (configurable header setting), and context about the contributor's deployment. It provides sufficient context about the purpose and implementation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/backend/core/api/viewsets.py`:
- Around line 1798-1800: Replace the f-string in the logger call with lazy
%-style argument formatting so the interpolation is deferred when debug logging
is disabled; locate the block checking original_url (the conditional that raises
drf.exceptions.PermissionDenied) and change logger.debug(f"Missing
{settings.MEDIA_AUTH_URL_PARAMETER} header in subrequest") to something like
logger.debug("Missing %s header in subrequest",
settings.MEDIA_AUTH_URL_PARAMETER) to match the rest of the file.

In `@src/backend/impress/settings.py`:
- Around line 132-134: The constant MEDIA_AUTH_URL_PARAMETER should explicitly
document that it expects a Django request.META key (e.g. HTTP_X_ORIGINAL_URL)
and not the raw header name; add a short inline comment/help_text next to
MEDIA_AUTH_URL_PARAMETER listing common examples (nginx → HTTP_X_ORIGINAL_URL,
Traefik → HTTP_X_FORWARDED_URI) and clarify the expected HTTP_* form. Either
rename the constant to a clearer identifier like
MEDIA_AUTH_ORIGINAL_URL_META_KEY (update all references) or, if you prefer
operator-friendliness, accept raw header names in the setting and convert them
to a META key where the setting is read (implement mapping: uppercase, replace
'-'→'_', and prefix with 'HTTP_' if missing) before use (update the code that
reads MEDIA_AUTH_URL_PARAMETER accordingly).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a16583df-bf6c-4841-84d5-f3cc15727c8c

📥 Commits

Reviewing files that changed from the base of the PR and between e747e03 and 33fadea.

📒 Files selected for processing (2)
  • src/backend/core/api/viewsets.py
  • src/backend/impress/settings.py

Comment thread src/backend/core/api/viewsets.py
Comment thread src/backend/impress/settings.py Outdated
@lunika
Copy link
Copy Markdown
Member

lunika commented Apr 27, 2026

Hi,

Thank you for your contribution,

Add a MEDIA_AUTH_URL_PARAMETER (I'm not 100% happy about the name)

Yes, it does not reflect that this is a header name. MEDIA_AUTH_ORIGINAL_URL_HEADER suggested by coderabbit seems more appropriated.

You also have to add this new setting in the docs/env.md file, where you can provide more information about what this setting is.

@lunika
Copy link
Copy Markdown
Member

lunika commented Apr 27, 2026

@erincandescent is it possible to share the modification you made on the helm chart to use the Traefik ingress ? I opened an issue to investigate what should be done to switch to Traefik #2174
If you already made it, it would be great if you could give us some feedback.

@erincandescent
Copy link
Copy Markdown
Contributor Author

Locally I render the chart and then deploy the result through Kustomize, so I haven't edited the chart. But I generated the rules, ran them through ingress2gateway and then hand edited them (e.g. adding the Traefik custom middleware)

But here are our resources, it should be relatively easy to adapt them to the chart:

---
apiVersion: traefik.io/v1alpha1
kind: Middleware
metadata:
  name: docs-media-auth
spec:
  forwardAuth:
    address: http://suite-docs-backend.suite.svc.cluster.local/api/v1.0/documents/media-auth/
    authResponseHeaders:
      - Authorization
      - x-amz-content-sha256
      - x-amz-date

---
apiVersion: traefik.io/v1alpha1
kind: Middleware
metadata:
  name: docs-media-csp
spec:
  headers:
    contentSecurityPolicy: "default-src 'none'"
    customRequestHeaders:
      # Currently broken with gateway equiv.
      Host: "rook-ceph-rgw-deber01-s3.rook-ceph.svc.cluster.local"
      X-Forwarded-Host: "rook-ceph-rgw-deber01-s3.rook-ceph.svc.cluster.local"
---
apiVersion: traefik.io/v1alpha1
kind: Middleware
metadata:
  name: docs-media-path
spec:
  replacePathRegex:
    regex: "^/media/(.*)"
    replacement: "/suite-docs-486cea00-c2fd-439a-b5f8-db02268d9789/$1"
---
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: docs-http
spec:
  hostnames:
    - docs.services.alioth.systems
  parentRefs:
    - namespace: traefik
      name: traefik
      sectionName: http
  rules:
    - filters:
        - requestRedirect:
            scheme: https
            statusCode: 301
          type: RequestRedirect
      matches:
        - path:
            type: PathPrefix
            value: /
---
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: docs-https
spec:
  hostnames:
    - docs.services.alioth.systems
  parentRefs:
    - namespace: traefik
      name: traefik
      sectionName: https
  rules:
    - name: frontend
      matches:
        - path:
            type: PathPrefix
            value: /
      backendRefs:
        - name: suite-docs-frontend
          port: 80
    - name: backend
      matches:
        - path:
            value: /api
        - path:
            value: /external_api
        - path:
            value: /admin
        - path:
            value: /static
      backendRefs:
        - name: suite-docs-backend
          port: 80
    - name: y-provider-api
      matches:
        - path:
            type: PathPrefix
            value: /collaboration/api
      backendRefs:
        - name: suite-docs-y-provider
          port: 80
    - name: y-provider-ws
      matches:
        - path:
            type: PathPrefix
            value: /collaboration/ws/
      timeouts:
        request: 240h0m0s
      backendRefs:
        - name: suite-docs-y-provider
          port: 80
    - name: media
      matches:
        - path:
            type: PathPrefix
            value: /media/
      filters:
        - type: ExtensionRef
          extensionRef:
            group: traefik.io
            kind: Middleware
            name: docs-media-auth
        - type: ExtensionRef
          extensionRef:
            group: traefik.io
            kind: Middleware
            name: docs-media-csp
        - type: ExtensionRef
          extensionRef:
            group: traefik.io
            kind: Middleware
            name: docs-media-path
      backendRefs:
        - namespace: rook-ceph
          name: rook-ceph-rgw-deber01-s3
          port: 80

(Bit of a shame about the hardcoded bucket name. Haven't figured out a solution to that)

I'll try & tackle the review comments this evening

@lunika
Copy link
Copy Markdown
Member

lunika commented Apr 27, 2026

And the websocket is working correctly ?

@erincandescent
Copy link
Copy Markdown
Contributor Author

I will double check with someone else but I believe so, there are no connection errors and Traefik doesn't normally require special configuration for them

@erincandescent
Copy link
Copy Markdown
Contributor Author

I believe these changes fix the comments. I confirm that the websockets work without issues through Traefik - I opened a document in two browser windows simultaneously and saw edits flow from one to the other live.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/backend/impress/settings.py (1)

132-133: ⚠️ Potential issue | 🟠 Major

Fail fast (or normalize) for invalid MEDIA_AUTH_ORIGINAL_URL_HEADER values.

Line 132 accepts arbitrary strings, but Line 1797 in src/backend/core/api/viewsets.py uses this value directly as a request.META key. A common operator value like X-Forwarded-Uri will silently break media auth and return 403s for every request. Add startup validation and/or normalize to Django META form.

🔧 Proposed hardening approach
+import re
...
 class Base(Configuration):
...
     MEDIA_AUTH_ORIGINAL_URL_HEADER = values.Value(
         default="HTTP_X_ORIGINAL_URL", environ_name="MEDIA_AUTH_ORIGINAL_URL_HEADER", environ_prefix=None)
...
     `@classmethod`
     def post_setup(cls):
         super().post_setup()
+        raw_header = (cls.MEDIA_AUTH_ORIGINAL_URL_HEADER or "").strip()
+        normalized = raw_header.upper().replace("-", "_")
+        if normalized and not normalized.startswith("HTTP_"):
+            normalized = f"HTTP_{normalized}"
+        if not re.fullmatch(r"HTTP_[A-Z0-9_]+", normalized):
+            raise ValueError(
+                "MEDIA_AUTH_ORIGINAL_URL_HEADER must be a valid Django META key "
+                "(e.g. HTTP_X_ORIGINAL_URL or HTTP_X_FORWARDED_URI)."
+            )
+        cls.MEDIA_AUTH_ORIGINAL_URL_HEADER = normalized

Run this read-only check to confirm there is currently no validation/normalization path:

#!/bin/bash
set -euo pipefail

echo "== Setting definition and usages =="
rg -n "MEDIA_AUTH_ORIGINAL_URL_HEADER" src/backend/impress/settings.py src/backend/core/api/viewsets.py

echo
echo "== Look for normalization/validation logic around header key form =="
rg -n "replace\\(\"-\", \"_\"\\)|startswith\\(\"HTTP_\"\\)|fullmatch\\(|MEDIA_AUTH_ORIGINAL_URL_HEADER.*ValueError" \
  src/backend/impress/settings.py src/backend/core/api/viewsets.py
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backend/impress/settings.py` around lines 132 - 133,
MEDIA_AUTH_ORIGINAL_URL_HEADER accepts arbitrary strings which are later used
directly as a request.META key in viewsets (see MEDIA_AUTH_ORIGINAL_URL_HEADER
in settings.py and use in src/backend/core/api/viewsets.py), so add startup
validation/normalization in the settings layer: normalize the configured header
to Django META form by uppercasing, replacing '-' with '_' and ensuring it
starts with "HTTP_" (or raise django.core.exceptions.ImproperlyConfigured on
invalid/empty values) and export the normalized constant for consumers; update
any code that reads MEDIA_AUTH_ORIGINAL_URL_HEADER to use the normalized value
so request.META lookups cannot silently fail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/backend/impress/settings.py`:
- Around line 132-133: MEDIA_AUTH_ORIGINAL_URL_HEADER accepts arbitrary strings
which are later used directly as a request.META key in viewsets (see
MEDIA_AUTH_ORIGINAL_URL_HEADER in settings.py and use in
src/backend/core/api/viewsets.py), so add startup validation/normalization in
the settings layer: normalize the configured header to Django META form by
uppercasing, replacing '-' with '_' and ensuring it starts with "HTTP_" (or
raise django.core.exceptions.ImproperlyConfigured on invalid/empty values) and
export the normalized constant for consumers; update any code that reads
MEDIA_AUTH_ORIGINAL_URL_HEADER to use the normalized value so request.META
lookups cannot silently fail.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f00b2672-7048-4601-84ec-1d10b11e2db7

📥 Commits

Reviewing files that changed from the base of the PR and between 33fadea and 292eac9.

📒 Files selected for processing (3)
  • docs/env.md
  • src/backend/core/api/viewsets.py
  • src/backend/impress/settings.py

@erincandescent
Copy link
Copy Markdown
Contributor Author

By the way I have a port of this to Drive as well, which makes me think some of this logic should maybe be factored out into django-lasuite (but that's probably something that someone with maintainer context would be better thinking about how to do)

@lunika
Copy link
Copy Markdown
Member

lunika commented Apr 28, 2026

Thank you for your contribution,

I will fix the linting, rebase on the main branch, and merge it.

@lunika lunika force-pushed the forward-auth-config branch 2 times, most recently from 78f05d6 to dbe405d Compare April 28, 2026 06:17
@lunika lunika enabled auto-merge (squash) April 28, 2026 06:19
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/backend/core/tests/documents/test_api_documents_media_auth.py`:
- Around line 418-463: The test
test_api_documents_media_auth_anonymous_public_custom_origin_header duplicates
S3 signed-download verification logic used elsewhere; extract that repeated
verification into a reusable helper (e.g., assert_s3_signed_download or
verify_signed_s3_response) and call it from this test and other tests that
perform the same flow. The helper should accept the unique symbols produced by
the API (authorization header, x-amz-date, x-amz-content-sha256 and the S3 key
or expected URL) plus the expected response body, construct the file_url using
settings.AWS_S3_ENDPOINT_URL and the key, issue the requests.get with the same
headers (authorization, x-amz-date, x-amz-content-sha256, Host) and assert the
response content equals the expected body; replace the inline request/assert
block in test_api_documents_media_auth_anonymous_public_custom_origin_header
with a single call to that helper.

In `@src/backend/impress/settings.py`:
- Around line 133-137: MEDIA_AUTH_ORIGINAL_URL_HEADER currently accepts any
string and can hide misconfiguration until runtime; add startup validation that
the resolved value matches a valid Django META header key (e.g. regex
^HTTP_[A-Z0-9_]+$). After the values.Value declaration for
MEDIA_AUTH_ORIGINAL_URL_HEADER, read its resolved value and validate with
re.match; if invalid raise django.core.exceptions.ImproperlyConfigured (or
ValueError) with a clear message referencing MEDIA_AUTH_ORIGINAL_URL_HEADER so
the app fails fast on bad config.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fbb487d1-83cc-4baf-a05b-68199088d148

📥 Commits

Reviewing files that changed from the base of the PR and between 292eac9 and dbe405d.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • docs/env.md
  • src/backend/core/api/viewsets.py
  • src/backend/core/tests/documents/test_api_documents_media_auth.py
  • src/backend/impress/settings.py

Comment on lines +418 to +463
def test_api_documents_media_auth_anonymous_public_custom_origin_header(settings):
"""Changing the setting MEDIA_AUTH_ORIGINAL_URL_HEADER to match other header should work"""
settings.MEDIA_AUTH_ORIGINAL_URL_HEADER = "HTTP_X_FORWARDED_URI"
document_id = uuid4()
filename = f"{uuid4()!s}.jpg"
key = f"{document_id!s}/attachments/{filename:s}"
default_storage.connection.meta.client.put_object(
Bucket=default_storage.bucket_name,
Key=key,
Body=BytesIO(b"my prose"),
ContentType="text/plain",
Metadata={"status": DocumentAttachmentStatus.READY},
)

factories.DocumentFactory(id=document_id, link_reach="public", attachments=[key])

original_url = f"http://localhost/media/{key:s}"
now = timezone.now()
with freeze_time(now):
response = APIClient().get(
"/api/v1.0/documents/media-auth/", HTTP_X_FORWARDED_URI=original_url
)

assert response.status_code == 200

authorization = response["Authorization"]
assert "AWS4-HMAC-SHA256 Credential=" in authorization
assert (
"SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature="
in authorization
)
assert response["X-Amz-Date"] == now.strftime("%Y%m%dT%H%M%SZ")

s3_url = urlparse(settings.AWS_S3_ENDPOINT_URL)
file_url = f"{settings.AWS_S3_ENDPOINT_URL:s}/impress-media-storage/{key:s}"
response = requests.get(
file_url,
headers={
"authorization": authorization,
"x-amz-date": response["x-amz-date"],
"x-amz-content-sha256": response["x-amz-content-sha256"],
"Host": f"{s3_url.hostname:s}:{s3_url.port:d}",
},
timeout=1,
)
assert response.content.decode("utf-8") == "my prose"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Reduce duplicated signed-download assertion flow.

Lines 418-463 add good coverage for custom origin headers, but this block duplicates the same S3 signed-download verification logic used in several tests. Extracting a helper will reduce maintenance overhead.

♻️ Suggested helper extraction (example)
+def _assert_signed_media_download(settings, auth_response, key, expected_content="my prose"):
+    s3_url = urlparse(settings.AWS_S3_ENDPOINT_URL)
+    file_url = f"{settings.AWS_S3_ENDPOINT_URL:s}/impress-media-storage/{key:s}"
+    s3_response = requests.get(
+        file_url,
+        headers={
+            "authorization": auth_response["Authorization"],
+            "x-amz-date": auth_response["x-amz-date"],
+            "x-amz-content-sha256": auth_response["x-amz-content-sha256"],
+            "Host": f"{s3_url.hostname:s}:{s3_url.port:d}",
+        },
+        timeout=1,
+    )
+    assert s3_response.content.decode("utf-8") == expected_content
@@
-    s3_url = urlparse(settings.AWS_S3_ENDPOINT_URL)
-    file_url = f"{settings.AWS_S3_ENDPOINT_URL:s}/impress-media-storage/{key:s}"
-    response = requests.get(
-        file_url,
-        headers={
-            "authorization": authorization,
-            "x-amz-date": response["x-amz-date"],
-            "x-amz-content-sha256": response["x-amz-content-sha256"],
-            "Host": f"{s3_url.hostname:s}:{s3_url.port:d}",
-        },
-        timeout=1,
-    )
-    assert response.content.decode("utf-8") == "my prose"
+    _assert_signed_media_download(settings, response, key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backend/core/tests/documents/test_api_documents_media_auth.py` around
lines 418 - 463, The test
test_api_documents_media_auth_anonymous_public_custom_origin_header duplicates
S3 signed-download verification logic used elsewhere; extract that repeated
verification into a reusable helper (e.g., assert_s3_signed_download or
verify_signed_s3_response) and call it from this test and other tests that
perform the same flow. The helper should accept the unique symbols produced by
the API (authorization header, x-amz-date, x-amz-content-sha256 and the S3 key
or expected URL) plus the expected response body, construct the file_url using
settings.AWS_S3_ENDPOINT_URL and the key, issue the requests.get with the same
headers (authorization, x-amz-date, x-amz-content-sha256, Host) and assert the
response content equals the expected body; replace the inline request/assert
block in test_api_documents_media_auth_anonymous_public_custom_origin_header
with a single call to that helper.

Comment on lines +133 to +137
MEDIA_AUTH_ORIGINAL_URL_HEADER = values.Value(
default="HTTP_X_ORIGINAL_URL",
environ_name="MEDIA_AUTH_ORIGINAL_URL_HEADER",
environ_prefix=None,
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Fail fast on invalid header-key format.

Lines 133-137 accept any string; a malformed value will only surface as media-auth 403s at runtime. Consider validating this at startup to prevent silent misconfiguration.

♻️ Proposed validation approach
 class Base(Configuration):
@@
     MEDIA_AUTH_ORIGINAL_URL_HEADER = values.Value(
         default="HTTP_X_ORIGINAL_URL",
         environ_name="MEDIA_AUTH_ORIGINAL_URL_HEADER",
         environ_prefix=None,
     )
@@
     def post_setup(cls):
         """Post setup configuration.
@@
         super().post_setup()
+
+        if not (
+            isinstance(cls.MEDIA_AUTH_ORIGINAL_URL_HEADER, str)
+            and cls.MEDIA_AUTH_ORIGINAL_URL_HEADER.startswith("HTTP_")
+        ):
+            raise ValueError(
+                "MEDIA_AUTH_ORIGINAL_URL_HEADER must be a Django request.META key "
+                '(e.g. "HTTP_X_ORIGINAL_URL").'
+            )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backend/impress/settings.py` around lines 133 - 137,
MEDIA_AUTH_ORIGINAL_URL_HEADER currently accepts any string and can hide
misconfiguration until runtime; add startup validation that the resolved value
matches a valid Django META header key (e.g. regex ^HTTP_[A-Z0-9_]+$). After the
values.Value declaration for MEDIA_AUTH_ORIGINAL_URL_HEADER, read its resolved
value and validate with re.match; if invalid raise
django.core.exceptions.ImproperlyConfigured (or ValueError) with a clear message
referencing MEDIA_AUTH_ORIGINAL_URL_HEADER so the app fails fast on bad config.

In our deployment we're using Traefik, not nginx, as an ingress.
Traefik uses X-Forwarded-Uri instead of X-Original-Uri. This
adds a setting which lets users adapt Docs to their ingress
proxy of choice

Signed-off-by: Erin Shepherd <erin.shepherd@e43.eu>
@lunika lunika force-pushed the forward-auth-config branch from dbe405d to 8acd520 Compare April 28, 2026 08:38
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/backend/impress/settings.py (1)

133-137: ⚠️ Potential issue | 🟠 Major

Fail fast on invalid MEDIA_AUTH_ORIGINAL_URL_HEADER format.

Line 133 defines a free-form string; a malformed value won’t fail until media-auth requests start returning 403s. Add startup validation in post_setup (e.g., ^HTTP_[A-Z0-9_]+$) and raise a clear configuration error.

♻️ Proposed fix
+import re
 import os
 import tomllib
 from socket import gethostbyname, gethostname
@@
+from django.core.exceptions import ImproperlyConfigured
 from django.utils.translation import gettext_lazy as _
@@
     def post_setup(cls):
@@
         super().post_setup()
+
+        if not re.match(r"^HTTP_[A-Z0-9_]+$", cls.MEDIA_AUTH_ORIGINAL_URL_HEADER or ""):
+            raise ImproperlyConfigured(
+                "MEDIA_AUTH_ORIGINAL_URL_HEADER must be a Django request.META key "
+                '(example: "HTTP_X_ORIGINAL_URL" or "HTTP_X_FORWARDED_URI").'
+            )
#!/bin/bash
set -euo pipefail
# Verify whether startup validation exists for MEDIA_AUTH_ORIGINAL_URL_HEADER.
rg -n "MEDIA_AUTH_ORIGINAL_URL_HEADER|post_setup|ImproperlyConfigured|re\.match|^HTTP_" src/backend/impress/settings.py -C 3
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/backend/impress/settings.py` around lines 133 - 137, Validate
MEDIA_AUTH_ORIGINAL_URL_HEADER at startup by adding a check in the module/class
post_setup function: read the MEDIA_AUTH_ORIGINAL_URL_HEADER value and ensure it
matches the regex ^HTTP_[A-Z0-9_]+$ (use re.fullmatch or re.match anchored) and
if it doesn’t, raise django.core.exceptions.ImproperlyConfigured with a clear
message referencing MEDIA_AUTH_ORIGINAL_URL_HEADER; update any tests or import
paths as needed and keep the validation next to the existing post_setup logic so
misconfiguration fails fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/backend/impress/settings.py`:
- Around line 133-137: Validate MEDIA_AUTH_ORIGINAL_URL_HEADER at startup by
adding a check in the module/class post_setup function: read the
MEDIA_AUTH_ORIGINAL_URL_HEADER value and ensure it matches the regex
^HTTP_[A-Z0-9_]+$ (use re.fullmatch or re.match anchored) and if it doesn’t,
raise django.core.exceptions.ImproperlyConfigured with a clear message
referencing MEDIA_AUTH_ORIGINAL_URL_HEADER; update any tests or import paths as
needed and keep the validation next to the existing post_setup logic so
misconfiguration fails fast.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 79aec1e8-d40a-4a5d-a59b-3037255b53d0

📥 Commits

Reviewing files that changed from the base of the PR and between dbe405d and 8acd520.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • docs/env.md
  • src/backend/core/api/viewsets.py
  • src/backend/core/tests/documents/test_api_documents_media_auth.py
  • src/backend/impress/settings.py

@lunika lunika merged commit 394fbc5 into suitenumerique:main Apr 28, 2026
27 checks passed
This was referenced Apr 30, 2026
lunika added a commit that referenced this pull request Apr 30, 2026
Added

- ✨(backend) create a dedicated endpoint to update document content
- ⚡️(backend) stream s3 file content with a dedicated endpoint
- ✨(backend) allow to use new ai feature using mistral sdk

Changed

- ♻️(backend) rename documents content endpoint in `formatted-content` (BC)
- 🚸(frontend) show Crisp from the help menu #2222
- ♿️(frontend) structure correctly 5xx error alerts #2128
- ♿️(frontend) make doc search result labels uniquely identifiable #2212
- ⬆️(backend) upgrade docspec to v3.0.x and adapt converter API #2220
- ✨(backend) make forward auth request uri header configurable #2241
- ♿️(frontend) fix sidebar resize handle for screen readers #2122

Fixed

- 🚸(frontend) redirect on current url tab after 401 #2197
- 🐛(frontend) abort check media status unmount #2194
- ✨(backend) order pinned documents by last updated at #2028
- 🐛(frontend) fix app shallow reload #2231
- 🐛(frontend) fix interlinking modal clipping #2213
- 🛂(frontend) fix cannot manage member on small screen #2226
- 🐛(backend) load jwks url when OIDC_RS_PRIVATE_KEY_STR is set
- 🐛(backend) Prevent moving document to its own descendant or self #2208
- 🐛(backend) return 400 when restoring a non-deleted document #2225
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants