From 3926e7e6f4d4325197917356c861c97f2b658054 Mon Sep 17 00:00:00 2001 From: awais786 Date: Fri, 15 May 2026 17:37:59 +0500 Subject: [PATCH 01/10] fix: re-auth when proxy identity differs from existing Django session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProxyAuthMiddleware short-circuited on any authenticated Django session, ignoring the X-Auth-Request-Email header. After a portal "log out of all apps" + login as a different user, the app-local Django session cookie survives (it's scoped to the app subdomain and not cleared by the shared _oauth2_proxy / Cognito logout), so refreshing the Plane tab kept serving the previous user. Now: short-circuit only when the upstream-asserted email matches the session's user email (or no header is present). On mismatch, fall through and re-authenticate — django.contrib.auth.login() flushes the stale session automatically when the user pk changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../authentication/middleware/proxy_auth.py | 60 +++++++++---- .../authentication/tests/test_proxy_auth.py | 90 ++++++++++++++++++- 2 files changed, 132 insertions(+), 18 deletions(-) diff --git a/apps/api/plane/authentication/middleware/proxy_auth.py b/apps/api/plane/authentication/middleware/proxy_auth.py index 3ff2bf71475..985e08d90a3 100644 --- a/apps/api/plane/authentication/middleware/proxy_auth.py +++ b/apps/api/plane/authentication/middleware/proxy_auth.py @@ -55,30 +55,33 @@ def __init__(self, get_response): ) def __call__(self, request): - # Layer 2 session already valid — nothing to do. - if request.user.is_authenticated: - return self.get_response(request) - # Bypass paths use their own auth (god-mode local login, instance admin). # TODO(mpass): Keep OPTIONS bypass at the proxy layer; add an app-level # fallback here only if preflight routing becomes inconsistent. if _is_bypass_path(request.path, self.bypass_paths): return self.get_response(request) - email = (request.META.get("HTTP_X_AUTH_REQUEST_EMAIL") or "").strip() - if email and "@" not in email: - # Header holds a bare username (user_id_claim=cognito:username). Synth email. - domain = getattr(settings, "DEFAULT_EMAIL_DOMAIN", "askii.ai") - email = f"{email}@{domain}" - if not email: - username = (request.META.get("HTTP_X_AUTH_REQUEST_USER") or "").strip() - domain = getattr(settings, "DEFAULT_EMAIL_DOMAIN", "askii.ai") - if username: - email = f"{username}@{domain}" - if not email: + proxy_email = self._read_proxy_email(request) + + if request.user.is_authenticated: + # Short-circuit only when the upstream-asserted identity matches the + # current Django session, or when no header is present (request did + # not pass through ForwardAuth — header absence is not a logout signal). + # + # If the proxy email differs (typical pattern: portal "log out of all + # apps" clears the shared _oauth2_proxy cookie + Cognito session but + # NOT this app's Django session cookie, then someone else logs in), + # fall through to re-authenticate. Django's login() flushes the stale + # session automatically when the user pk changes. + current = _normalise_email(request.user.email or "") + incoming = _normalise_email(proxy_email or "") + if not incoming or current == incoming: + return self.get_response(request) + + if not proxy_email: return self.get_response(request) - email = _normalise_email(email) + email = _normalise_email(proxy_email) if not email: return self.get_response(request) @@ -92,6 +95,31 @@ def __call__(self, request): user_login(request=request, user=user, is_app=True) return self.get_response(request) + @staticmethod + def _read_proxy_email(request): + """Extract the upstream-asserted email from oauth2-proxy headers. + + Handles two header shapes: + - X-Auth-Request-Email contains a real email → use as-is + - X-Auth-Request-Email contains a bare username (user_id_claim= + cognito:username) → synthesise @DEFAULT_EMAIL_DOMAIN + - X-Auth-Request-Email is empty but X-Auth-Request-User has a username + → synthesise the same way + + Returns the raw (un-normalised) email string, or "" if none could be + derived. Caller is responsible for `_normalise_email` before using. + """ + email = (request.META.get("HTTP_X_AUTH_REQUEST_EMAIL") or "").strip() + if email and "@" not in email: + domain = getattr(settings, "DEFAULT_EMAIL_DOMAIN", "askii.ai") + email = f"{email}@{domain}" + if not email: + username = (request.META.get("HTTP_X_AUTH_REQUEST_USER") or "").strip() + if username: + domain = getattr(settings, "DEFAULT_EMAIL_DOMAIN", "askii.ai") + email = f"{username}@{domain}" + return email + def _resolve_user(self, email): username_hint = email.split("@")[0] or uuid4().hex try: diff --git a/apps/api/plane/authentication/tests/test_proxy_auth.py b/apps/api/plane/authentication/tests/test_proxy_auth.py index 98bc6d6329c..3ef5c25f410 100644 --- a/apps/api/plane/authentication/tests/test_proxy_auth.py +++ b/apps/api/plane/authentication/tests/test_proxy_auth.py @@ -13,10 +13,14 @@ Design contract being tested ----------------------------- - Reads HTTP_X_AUTH_REQUEST_EMAIL from request.META -- If request.user.is_authenticated → pass through immediately (no DB, no login) - If path starts with a bypass prefix → pass through immediately (no DB, no login) Default bypass prefixes: ["/god-mode", "/api/instances"] -- If email header is absent → pass through unauthenticated +- If request.user.is_authenticated: + * proxy header absent or matches request.user.email → short-circuit + * proxy header asserts a DIFFERENT email → fall through and re-authenticate + (defends against the "stale Django session survives upstream logout" + class of bug — see TestProxyAuthMiddlewareUserSwitch) +- If email header is absent (and no existing session) → pass through unauthenticated - If email is present → get_or_create User, create Profile on first creation, then call user_login(request, user, is_app=True) to establish session - New users get: set_unusable_password(), is_password_autoset=True, is_email_verified=True @@ -98,6 +102,88 @@ def test_skips_when_user_already_authenticated(self, django_user_model): assert User.objects.count() == count_before +class TestProxyAuthMiddlewareUserSwitch: + """Stale Django session must not survive an upstream identity change.""" + + @pytest.mark.django_db + def test_logs_in_new_user_when_proxy_email_differs(self, django_user_model): + """ + GIVEN the current Django session belongs to alice + AND X-Auth-Request-Email = bob's email (oauth2-proxy now says bob) + WHEN the middleware processes the request + THEN user_login is called with bob (not short-circuited on alice) + + Real-world repro: portal "log out of all apps" clears the shared + _oauth2_proxy cookie + Cognito session but NOT Plane's own Django + session cookie. The next user logs in upstream; this tab refreshes + and must re-bind to the new identity. + """ + django_user_model.objects.create_user( + email="alice@example.com", username="alice", password="x", + ) + bob = django_user_model.objects.create_user( + email="bob@example.com", username="bob", password="x", + ) + alice = django_user_model.objects.get(email="alice@example.com") + middleware = make_middleware() + request = make_request( + meta={"HTTP_X_AUTH_REQUEST_EMAIL": "bob@example.com"}, + authenticated_user=alice, + ) + + with patch(PATCH_USER_LOGIN) as mock_login: + middleware(request) + + mock_login.assert_called_once() + assert mock_login.call_args.kwargs["user"].pk == bob.pk + + @pytest.mark.django_db + def test_no_logout_when_header_absent(self, django_user_model): + """ + GIVEN the current Django session belongs to alice + AND no X-Auth-Request-Email header is present + WHEN the middleware processes the request + THEN user_login is NOT called — header absence is not a logout signal. + + Header absence can occur on internal requests, bypass paths reached + via redirect, or test paths. Treating it as "log out" would break + every such call. + """ + alice = django_user_model.objects.create_user( + email="alice@example.com", username="alice", password="x", + ) + middleware = make_middleware() + request = make_request(authenticated_user=alice) + + with patch(PATCH_USER_LOGIN) as mock_login: + middleware(request) + + mock_login.assert_not_called() + + @pytest.mark.django_db + def test_match_is_case_and_whitespace_insensitive(self, django_user_model): + """ + GIVEN the current Django session belongs to alice@example.com + AND X-Auth-Request-Email = " ALICE@example.com " + WHEN the middleware processes the request + THEN user_login is NOT called — match comparison runs through the + same normalisation as user creation does. + """ + alice = django_user_model.objects.create_user( + email="alice@example.com", username="alice", password="x", + ) + middleware = make_middleware() + request = make_request( + meta={"HTTP_X_AUTH_REQUEST_EMAIL": " ALICE@example.com "}, + authenticated_user=alice, + ) + + with patch(PATCH_USER_LOGIN) as mock_login: + middleware(request) + + mock_login.assert_not_called() + + class TestProxyAuthMiddlewareNoHeader: """Middleware must pass through cleanly when the email header is absent.""" From 417b8849da198da13c9c37115eb3d1053e48b6b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 14:50:30 +0000 Subject: [PATCH 02/10] fix: update _read_proxy_email docstring from 'two' to 'three' cases Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/d7642184-5c71-4374-8cd4-b43cd039d0f5 Co-authored-by: awais786 <445320+awais786@users.noreply.github.com> --- apps/api/plane/authentication/middleware/proxy_auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/plane/authentication/middleware/proxy_auth.py b/apps/api/plane/authentication/middleware/proxy_auth.py index 985e08d90a3..398217e0b47 100644 --- a/apps/api/plane/authentication/middleware/proxy_auth.py +++ b/apps/api/plane/authentication/middleware/proxy_auth.py @@ -99,7 +99,7 @@ def __call__(self, request): def _read_proxy_email(request): """Extract the upstream-asserted email from oauth2-proxy headers. - Handles two header shapes: + Handles three cases: - X-Auth-Request-Email contains a real email → use as-is - X-Auth-Request-Email contains a bare username (user_id_claim= cognito:username) → synthesise @DEFAULT_EMAIL_DOMAIN From 5d0fc3f5c9a2971061be6bae4f4b70f5578bfb10 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 14:52:43 +0000 Subject: [PATCH 03/10] docs(tests): reflect email/user proxy header contract Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/e547eb55-f665-43bb-a581-fadb1224613b Co-authored-by: awais786 <445320+awais786@users.noreply.github.com> --- apps/api/plane/authentication/tests/test_proxy_auth.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/api/plane/authentication/tests/test_proxy_auth.py b/apps/api/plane/authentication/tests/test_proxy_auth.py index 3ef5c25f410..ed1046ab679 100644 --- a/apps/api/plane/authentication/tests/test_proxy_auth.py +++ b/apps/api/plane/authentication/tests/test_proxy_auth.py @@ -12,7 +12,9 @@ Design contract being tested ----------------------------- -- Reads HTTP_X_AUTH_REQUEST_EMAIL from request.META +- Reads identity headers from request.META: + * HTTP_X_AUTH_REQUEST_EMAIL + * HTTP_X_AUTH_REQUEST_USER (fallback when email header is empty) - If path starts with a bypass prefix → pass through immediately (no DB, no login) Default bypass prefixes: ["/god-mode", "/api/instances"] - If request.user.is_authenticated: @@ -20,8 +22,8 @@ * proxy header asserts a DIFFERENT email → fall through and re-authenticate (defends against the "stale Django session survives upstream logout" class of bug — see TestProxyAuthMiddlewareUserSwitch) -- If email header is absent (and no existing session) → pass through unauthenticated -- If email is present → get_or_create User, create Profile on first creation, +- If both identity headers are absent (and no existing session) → pass through unauthenticated +- If identity can be derived from headers → get_or_create User, create Profile on first creation, then call user_login(request, user, is_app=True) to establish session - New users get: set_unusable_password(), is_password_autoset=True, is_email_verified=True - username is always uuid4().hex (never the Cognito sub — avoids length/collision issues) From 732f5135b6c7b50d1e2a70c6efaa9e5019ee67d9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 14:56:39 +0000 Subject: [PATCH 04/10] fix: flush stale session on proxy/session identity mismatch Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/c9e438f9-104f-440c-9990-21bb292ed8ae Co-authored-by: awais786 <445320+awais786@users.noreply.github.com> --- .../authentication/middleware/proxy_auth.py | 14 +++--- .../authentication/tests/test_proxy_auth.py | 49 +++++++++++++++++-- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/apps/api/plane/authentication/middleware/proxy_auth.py b/apps/api/plane/authentication/middleware/proxy_auth.py index 398217e0b47..fb1a68050e0 100644 --- a/apps/api/plane/authentication/middleware/proxy_auth.py +++ b/apps/api/plane/authentication/middleware/proxy_auth.py @@ -5,6 +5,7 @@ from uuid import uuid4 from django.conf import settings +from django.contrib.auth import logout from django.contrib.auth.hashers import make_password from django.db import IntegrityError @@ -67,16 +68,17 @@ def __call__(self, request): # Short-circuit only when the upstream-asserted identity matches the # current Django session, or when no header is present (request did # not pass through ForwardAuth — header absence is not a logout signal). - # - # If the proxy email differs (typical pattern: portal "log out of all - # apps" clears the shared _oauth2_proxy cookie + Cognito session but - # NOT this app's Django session cookie, then someone else logs in), - # fall through to re-authenticate. Django's login() flushes the stale - # session automatically when the user pk changes. current = _normalise_email(request.user.email or "") incoming = _normalise_email(proxy_email or "") if not incoming or current == incoming: return self.get_response(request) + + # Mismatch detected: proxy asserts a different identity than the + # current session. Flush the stale session immediately so that if + # subsequent re-auth fails (e.g., incoming user is inactive), the + # request proceeds as unauthenticated rather than retaining the + # previous user's identity. + logout(request) if not proxy_email: return self.get_response(request) diff --git a/apps/api/plane/authentication/tests/test_proxy_auth.py b/apps/api/plane/authentication/tests/test_proxy_auth.py index ed1046ab679..406ea5ea52e 100644 --- a/apps/api/plane/authentication/tests/test_proxy_auth.py +++ b/apps/api/plane/authentication/tests/test_proxy_auth.py @@ -19,7 +19,8 @@ Default bypass prefixes: ["/god-mode", "/api/instances"] - If request.user.is_authenticated: * proxy header absent or matches request.user.email → short-circuit - * proxy header asserts a DIFFERENT email → fall through and re-authenticate + * proxy header asserts a DIFFERENT email → logout() to flush the stale session, + then fall through and re-authenticate (defends against the "stale Django session survives upstream logout" class of bug — see TestProxyAuthMiddlewareUserSwitch) - If both identity headers are absent (and no existing session) → pass through unauthenticated @@ -113,7 +114,8 @@ def test_logs_in_new_user_when_proxy_email_differs(self, django_user_model): GIVEN the current Django session belongs to alice AND X-Auth-Request-Email = bob's email (oauth2-proxy now says bob) WHEN the middleware processes the request - THEN user_login is called with bob (not short-circuited on alice) + THEN logout() is called to flush alice's stale session + AND user_login is called with bob (not short-circuited on alice) Real-world repro: portal "log out of all apps" clears the shared _oauth2_proxy cookie + Cognito session but NOT Plane's own Django @@ -133,9 +135,13 @@ def test_logs_in_new_user_when_proxy_email_differs(self, django_user_model): authenticated_user=alice, ) - with patch(PATCH_USER_LOGIN) as mock_login: + with patch(PATCH_USER_LOGIN) as mock_login, \ + patch("plane.authentication.middleware.proxy_auth.logout") as mock_logout: middleware(request) + # Session should be flushed when mismatch is detected + mock_logout.assert_called_once_with(request) + # Then re-auth with the new user mock_login.assert_called_once() assert mock_login.call_args.kwargs["user"].pk == bob.pk @@ -185,6 +191,43 @@ def test_match_is_case_and_whitespace_insensitive(self, django_user_model): mock_login.assert_not_called() + @pytest.mark.django_db + def test_flushes_session_when_incoming_user_is_inactive(self, django_user_model): + """ + GIVEN the current Django session belongs to alice (active) + AND X-Auth-Request-Email = bob's email + AND bob exists but is_active=False + WHEN the middleware processes the request + THEN logout() is called to flush alice's session + AND user_login is NOT called (bob is inactive) + AND the request proceeds as unauthenticated + + This prevents a stale session from surviving when re-auth fails. + Without the explicit logout(), alice's session would remain active + and the request would proceed as alice even though the proxy now + asserts bob's identity. + """ + alice = django_user_model.objects.create_user( + email="alice@example.com", username="alice", password="x", + ) + django_user_model.objects.create_user( + email="bob@example.com", username="bob", password="x", is_active=False, + ) + middleware = make_middleware() + request = make_request( + meta={"HTTP_X_AUTH_REQUEST_EMAIL": "bob@example.com"}, + authenticated_user=alice, + ) + + with patch(PATCH_USER_LOGIN) as mock_login, \ + patch("plane.authentication.middleware.proxy_auth.logout") as mock_logout: + middleware(request) + + # Session should be flushed when mismatch is detected + mock_logout.assert_called_once_with(request) + # user_login should NOT be called because bob is inactive + mock_login.assert_not_called() + class TestProxyAuthMiddlewareNoHeader: """Middleware must pass through cleanly when the email header is absent.""" From 432e06d604bf4c3048b26ba2190db05388c79644 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 14:58:13 +0000 Subject: [PATCH 05/10] style: fix trailing comma and whitespace-only line Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/c9e438f9-104f-440c-9990-21bb292ed8ae Co-authored-by: awais786 <445320+awais786@users.noreply.github.com> --- apps/api/plane/authentication/middleware/proxy_auth.py | 2 +- apps/api/plane/authentication/tests/test_proxy_auth.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/api/plane/authentication/middleware/proxy_auth.py b/apps/api/plane/authentication/middleware/proxy_auth.py index fb1a68050e0..b4557dde643 100644 --- a/apps/api/plane/authentication/middleware/proxy_auth.py +++ b/apps/api/plane/authentication/middleware/proxy_auth.py @@ -72,7 +72,7 @@ def __call__(self, request): incoming = _normalise_email(proxy_email or "") if not incoming or current == incoming: return self.get_response(request) - + # Mismatch detected: proxy asserts a different identity than the # current session. Flush the stale session immediately so that if # subsequent re-auth fails (e.g., incoming user is inactive), the diff --git a/apps/api/plane/authentication/tests/test_proxy_auth.py b/apps/api/plane/authentication/tests/test_proxy_auth.py index 406ea5ea52e..7371b736171 100644 --- a/apps/api/plane/authentication/tests/test_proxy_auth.py +++ b/apps/api/plane/authentication/tests/test_proxy_auth.py @@ -208,10 +208,10 @@ def test_flushes_session_when_incoming_user_is_inactive(self, django_user_model) asserts bob's identity. """ alice = django_user_model.objects.create_user( - email="alice@example.com", username="alice", password="x", + email="alice@example.com", username="alice", password="x" ) django_user_model.objects.create_user( - email="bob@example.com", username="bob", password="x", is_active=False, + email="bob@example.com", username="bob", password="x", is_active=False ) middleware = make_middleware() request = make_request( From aff9bf13b9cebdbd3582cc5391a32ca76236514a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 14:59:31 +0000 Subject: [PATCH 06/10] refactor: use PATCH_LOGOUT constant for consistency Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/c9e438f9-104f-440c-9990-21bb292ed8ae Co-authored-by: awais786 <445320+awais786@users.noreply.github.com> --- apps/api/plane/authentication/tests/test_proxy_auth.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/api/plane/authentication/tests/test_proxy_auth.py b/apps/api/plane/authentication/tests/test_proxy_auth.py index 7371b736171..496e2d1607d 100644 --- a/apps/api/plane/authentication/tests/test_proxy_auth.py +++ b/apps/api/plane/authentication/tests/test_proxy_auth.py @@ -43,6 +43,7 @@ class of bug — see TestProxyAuthMiddlewareUserSwitch) from plane.db.models import User, Profile PATCH_USER_LOGIN = "plane.authentication.middleware.proxy_auth.user_login" +PATCH_LOGOUT = "plane.authentication.middleware.proxy_auth.logout" # --------------------------------------------------------------------------- @@ -136,7 +137,7 @@ def test_logs_in_new_user_when_proxy_email_differs(self, django_user_model): ) with patch(PATCH_USER_LOGIN) as mock_login, \ - patch("plane.authentication.middleware.proxy_auth.logout") as mock_logout: + patch(PATCH_LOGOUT) as mock_logout: middleware(request) # Session should be flushed when mismatch is detected @@ -220,7 +221,7 @@ def test_flushes_session_when_incoming_user_is_inactive(self, django_user_model) ) with patch(PATCH_USER_LOGIN) as mock_login, \ - patch("plane.authentication.middleware.proxy_auth.logout") as mock_logout: + patch(PATCH_LOGOUT) as mock_logout: middleware(request) # Session should be flushed when mismatch is detected From 578e9a724ef762fc23a39c67bd850c2497186fb4 Mon Sep 17 00:00:00 2001 From: awais786 Date: Fri, 15 May 2026 21:57:09 +0500 Subject: [PATCH 07/10] refactor(auth): inline normalised email + bypass-dominates-mismatch test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small follow-ups on PR #29 from review: - Inline `_normalise_email(self._read_proxy_email(request))` once into `email` and reuse it for both the mismatch comparison and the DB lookup, removing the second `_normalise_email(proxy_email)` call. - Add `TODO(security)` on `_read_proxy_email` pointing at `fix/proxy-auth-reject-bare-username` for the bare-username synthesis vulnerability this PR preserves but does not address. - Add `test_bypass_dominates_mismatched_proxy_header` to lock in that the bypass check at the top of `__call__` runs before the new mismatch logout flow — guards god-mode local admin sessions from being kicked out by an unrelated mPass identity reaching the same browser. All 22 proxy_auth tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../authentication/middleware/proxy_auth.py | 16 ++++++---- .../authentication/tests/test_proxy_auth.py | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/apps/api/plane/authentication/middleware/proxy_auth.py b/apps/api/plane/authentication/middleware/proxy_auth.py index b4557dde643..e6a42e2ca51 100644 --- a/apps/api/plane/authentication/middleware/proxy_auth.py +++ b/apps/api/plane/authentication/middleware/proxy_auth.py @@ -62,15 +62,14 @@ def __call__(self, request): if _is_bypass_path(request.path, self.bypass_paths): return self.get_response(request) - proxy_email = self._read_proxy_email(request) + email = _normalise_email(self._read_proxy_email(request)) if request.user.is_authenticated: # Short-circuit only when the upstream-asserted identity matches the # current Django session, or when no header is present (request did # not pass through ForwardAuth — header absence is not a logout signal). current = _normalise_email(request.user.email or "") - incoming = _normalise_email(proxy_email or "") - if not incoming or current == incoming: + if not email or current == email: return self.get_response(request) # Mismatch detected: proxy asserts a different identity than the @@ -80,10 +79,6 @@ def __call__(self, request): # previous user's identity. logout(request) - if not proxy_email: - return self.get_response(request) - - email = _normalise_email(proxy_email) if not email: return self.get_response(request) @@ -110,6 +105,13 @@ def _read_proxy_email(request): Returns the raw (un-normalised) email string, or "" if none could be derived. Caller is responsible for `_normalise_email` before using. + + TODO(security): the bare-username synthesis paths let a Cognito + principal whose username collides with a real Plane user's email + local-part impersonate that user (e.g. `cognito:username=alice` → + synthesised to `alice@askii.ai` → resolves to an existing `alice@askii.ai` + Plane user). See branch `fix/proxy-auth-reject-bare-username` for the + defensive fix that drops these paths and requires a real email claim. """ email = (request.META.get("HTTP_X_AUTH_REQUEST_EMAIL") or "").strip() if email and "@" not in email: diff --git a/apps/api/plane/authentication/tests/test_proxy_auth.py b/apps/api/plane/authentication/tests/test_proxy_auth.py index 496e2d1607d..e070d80cbe6 100644 --- a/apps/api/plane/authentication/tests/test_proxy_auth.py +++ b/apps/api/plane/authentication/tests/test_proxy_auth.py @@ -439,6 +439,38 @@ def test_instances_path_is_bypassed(self): mock_login.assert_not_called() assert User.objects.count() == count_before + @pytest.mark.django_db + def test_bypass_dominates_mismatched_proxy_header(self, django_user_model): + """ + GIVEN the current Django session belongs to alice + AND the request targets a bypass path (/god-mode/setup/) + AND X-Auth-Request-Email = bob's email (mismatch) + WHEN the middleware processes the request + THEN logout() is NOT called — bypass dominates the mismatch flow + AND user_login() is NOT called + + The bypass check runs at the top of __call__, before the proxy header + is read or session identity is compared. This guards god-mode local + admin sessions against being kicked out by an unrelated mPass + identity reaching the same browser. + """ + alice = django_user_model.objects.create_user( + email="alice@example.com", username="alice", password="x", + ) + middleware = make_middleware() + request = make_request( + path="/god-mode/setup/", + meta={"HTTP_X_AUTH_REQUEST_EMAIL": "bob@example.com"}, + authenticated_user=alice, + ) + + with patch(PATCH_USER_LOGIN) as mock_login, \ + patch(PATCH_LOGOUT) as mock_logout: + middleware(request) + + mock_logout.assert_not_called() + mock_login.assert_not_called() + class TestProxyAuthMiddlewareEdgeCases: """Email normalisation and concurrent creation races.""" From f16e21af22ec4bdb72c668cd40e12affc857914e Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Fri, 15 May 2026 22:02:17 +0500 Subject: [PATCH 08/10] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- apps/api/plane/authentication/middleware/proxy_auth.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/api/plane/authentication/middleware/proxy_auth.py b/apps/api/plane/authentication/middleware/proxy_auth.py index e6a42e2ca51..c91dc5f84f1 100644 --- a/apps/api/plane/authentication/middleware/proxy_auth.py +++ b/apps/api/plane/authentication/middleware/proxy_auth.py @@ -110,8 +110,8 @@ def _read_proxy_email(request): principal whose username collides with a real Plane user's email local-part impersonate that user (e.g. `cognito:username=alice` → synthesised to `alice@askii.ai` → resolves to an existing `alice@askii.ai` - Plane user). See branch `fix/proxy-auth-reject-bare-username` for the - defensive fix that drops these paths and requires a real email claim. + Plane user). The defensive fix is to drop these synthesis paths and + require a real email claim from the upstream proxy. """ email = (request.META.get("HTTP_X_AUTH_REQUEST_EMAIL") or "").strip() if email and "@" not in email: From cc1107b496b13c5ad70ac3339c623f667b8bad1c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 17:05:11 +0000 Subject: [PATCH 09/10] test: assert logout() is called before user_login() on identity mismatch Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/850afff6-c51c-4bfa-8f10-56dfea5d6efa Co-authored-by: awais786 <445320+awais786@users.noreply.github.com> --- .../authentication/tests/test_proxy_auth.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/apps/api/plane/authentication/tests/test_proxy_auth.py b/apps/api/plane/authentication/tests/test_proxy_auth.py index e070d80cbe6..ccf8f0ad2e5 100644 --- a/apps/api/plane/authentication/tests/test_proxy_auth.py +++ b/apps/api/plane/authentication/tests/test_proxy_auth.py @@ -35,7 +35,7 @@ class of bug — see TestProxyAuthMiddlewareUserSwitch) """ import pytest -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, Mock, patch from django.contrib.auth.models import AnonymousUser from django.test import RequestFactory @@ -136,8 +136,15 @@ def test_logs_in_new_user_when_proxy_email_differs(self, django_user_model): authenticated_user=alice, ) + # Use a manager mock to track call order + manager = Mock() + with patch(PATCH_USER_LOGIN) as mock_login, \ patch(PATCH_LOGOUT) as mock_logout: + # Attach mocks to manager to track order + manager.attach_mock(mock_logout, 'logout') + manager.attach_mock(mock_login, 'login') + middleware(request) # Session should be flushed when mismatch is detected @@ -145,6 +152,13 @@ def test_logs_in_new_user_when_proxy_email_differs(self, django_user_model): # Then re-auth with the new user mock_login.assert_called_once() assert mock_login.call_args.kwargs["user"].pk == bob.pk + + # Verify logout happens before user_login (order matters for security) + call_names = [call[0] for call in manager.mock_calls] + assert 'logout' in call_names and 'login' in call_names, \ + "Both logout and login should be called" + assert call_names.index('logout') < call_names.index('login'), \ + "logout() must be called before user_login() to flush stale session" @pytest.mark.django_db def test_no_logout_when_header_absent(self, django_user_model): From 17e64ac0fe044c5dcd2ad2c7e2da36230cde4229 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 17:09:15 +0000 Subject: [PATCH 10/10] test: verify request proceeds after logout when incoming user is inactive Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/e3de1135-e926-4554-b4ad-23ad11cd16aa Co-authored-by: awais786 <445320+awais786@users.noreply.github.com> --- .../plane/authentication/tests/test_proxy_auth.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/apps/api/plane/authentication/tests/test_proxy_auth.py b/apps/api/plane/authentication/tests/test_proxy_auth.py index ccf8f0ad2e5..e4ca4d61c0c 100644 --- a/apps/api/plane/authentication/tests/test_proxy_auth.py +++ b/apps/api/plane/authentication/tests/test_proxy_auth.py @@ -215,7 +215,7 @@ def test_flushes_session_when_incoming_user_is_inactive(self, django_user_model) WHEN the middleware processes the request THEN logout() is called to flush alice's session AND user_login is NOT called (bob is inactive) - AND the request proceeds as unauthenticated + AND the request proceeds without re-authentication This prevents a stale session from surviving when re-auth fails. Without the explicit logout(), alice's session would remain active @@ -228,7 +228,15 @@ def test_flushes_session_when_incoming_user_is_inactive(self, django_user_model) django_user_model.objects.create_user( email="bob@example.com", username="bob", password="x", is_active=False ) - middleware = make_middleware() + + # Capture what request object get_response receives + received_request = None + def capture_get_response(req): + nonlocal received_request + received_request = req + return MagicMock(status_code=200) + + middleware = make_middleware(get_response=capture_get_response) request = make_request( meta={"HTTP_X_AUTH_REQUEST_EMAIL": "bob@example.com"}, authenticated_user=alice, @@ -242,6 +250,9 @@ def test_flushes_session_when_incoming_user_is_inactive(self, django_user_model) mock_logout.assert_called_once_with(request) # user_login should NOT be called because bob is inactive mock_login.assert_not_called() + # Verify get_response was called with the request (middleware didn't block it) + assert received_request is request, \ + "Middleware should pass the request to get_response after logout" class TestProxyAuthMiddlewareNoHeader: