diff --git a/apps/api/plane/authentication/middleware/proxy_auth.py b/apps/api/plane/authentication/middleware/proxy_auth.py index 4dce5939790..c4ce082f0de 100644 --- a/apps/api/plane/authentication/middleware/proxy_auth.py +++ b/apps/api/plane/authentication/middleware/proxy_auth.py @@ -37,6 +37,9 @@ "is_email_verified": True, } +# Session flag marking that this session's user is known to have a Profile. +_PROFILE_ENSURED_KEY = "proxy_auth_profile_ensured" + def _check_corporate_id(request) -> bool: """Verify the caller's access token contains a corporate_id matching this deployment. @@ -95,6 +98,18 @@ def __call__(self, request): # not pass through ForwardAuth — header absence is not a logout signal). current = _normalise_email(request.user.email or "") if not email or current == email: + # A user provisioned outside the signup path can already hold a + # session while still missing a Profile, and would otherwise + # keep 404ing on /api/users/me/profile/ until the session + # expires. Gate on a session flag so this runs once per session + # rather than once per request. A profile created here — or + # backfilled out of band — still has onboarding incomplete, + # which is what _auto_join_workspace finishes. + if not request.session.get(_PROFILE_ENSURED_KEY): + profile, _ = Profile.objects.get_or_create(user=request.user) + if not profile.is_onboarded: + self._auto_join_workspace(request.user) + request.session[_PROFILE_ENSURED_KEY] = True return self.get_response(request) # Mismatch detected: proxy asserts a different identity than the @@ -118,6 +133,9 @@ def __call__(self, request): return self.get_response(request) user_login(request=request, user=user, is_app=True) + # _resolve_user just guaranteed the profile, so the next request on this + # session can skip the check instead of re-running it once per login. + request.session[_PROFILE_ENSURED_KEY] = True return self.get_response(request) @staticmethod @@ -155,7 +173,7 @@ def _read_proxy_email(request): def _resolve_user(self, email): username_hint = email.split("@")[0] or uuid4().hex try: - user, created = User.objects.get_or_create( + user, _ = User.objects.get_or_create( email=email, defaults={ "username": username_hint, @@ -165,14 +183,14 @@ def _resolve_user(self, email): ) except IntegrityError: # Concurrent email insert race — fall back to get(). - try: - user = User.objects.get(email=email) - except User.DoesNotExist: - raise - created = False - - if created: - Profile.objects.get_or_create(user=user) + user = User.objects.get(email=email) + + # Run for every user, not just newly created ones. Users provisioned + # outside the signup path — e.g. inserted directly by the JIRA import — + # have no Profile row, which makes /api/users/me/profile/ return 404 and + # bounces the client back to the login page in a loop. get_or_create is + # idempotent, so an existing profile is left untouched. + Profile.objects.get_or_create(user=user) # Run for every user (new or existing) — idempotent, no-op if already joined. self._auto_join_workspace(user) diff --git a/apps/api/plane/authentication/tests/test_proxy_auth.py b/apps/api/plane/authentication/tests/test_proxy_auth.py index e4ca4d61c0c..f07b09e9ad7 100644 --- a/apps/api/plane/authentication/tests/test_proxy_auth.py +++ b/apps/api/plane/authentication/tests/test_proxy_auth.py @@ -18,14 +18,17 @@ - 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: - * proxy header absent or matches request.user.email → short-circuit + * proxy header absent or matches request.user.email → ensure a Profile exists + (once per session, gated on a session flag) then short-circuit * 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 -- 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 +- If identity can be derived from headers → get_or_create User, ensure a Profile exists + (for every user, not just newly created ones — the JIRA import inserts users + directly and leaves them profile-less), 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) - Email is normalised (lowercased + stripped) before DB lookup @@ -105,6 +108,98 @@ def test_skips_when_user_already_authenticated(self, django_user_model): mock_login.assert_not_called() assert User.objects.count() == count_before + @pytest.mark.django_db + def test_creates_missing_profile_for_authenticated_user(self, django_user_model): + """ + GIVEN an authenticated user with no Profile — e.g. inserted directly by + the JIRA import, which bypasses Plane's signup path + WHEN the middleware processes the request + THEN the missing Profile is created + AND the session is flagged so later requests skip the check + + Without this the user 404s on /api/users/me/profile/ for the life of + the session and the web client loops back to the login page. + """ + existing_user = django_user_model.objects.create_user( + email="noprofile@example.com", + username="noprofile_user", + password="irrelevant", + ) + Profile.objects.filter(user=existing_user).delete() + assert not Profile.objects.filter(user=existing_user).exists() + + middleware = make_middleware() + request = make_request( + meta={"HTTP_X_AUTH_REQUEST_EMAIL": "noprofile@example.com"}, + authenticated_user=existing_user, + ) + + with patch(PATCH_USER_LOGIN): + middleware(request) + + assert Profile.objects.filter(user=existing_user).exists() + assert request.session.get("proxy_auth_profile_ensured") is True + + @pytest.mark.django_db + def test_completes_onboarding_for_existing_unonboarded_profile(self, django_user_model): + """ + GIVEN an authenticated user whose Profile exists but is not onboarded — + the state left by an out-of-band backfill + WHEN the middleware processes the request + THEN onboarding is completed rather than skipped + + Gating this on "the middleware created the profile" would miss exactly + the backfilled users, leaving them stuck on /onboarding. + """ + existing_user = django_user_model.objects.create_user( + email="unonboarded@example.com", + username="unonboarded_user", + password="irrelevant", + ) + Profile.objects.update_or_create(user=existing_user, defaults={"is_onboarded": False}) + + middleware = make_middleware() + request = make_request( + meta={"HTTP_X_AUTH_REQUEST_EMAIL": "unonboarded@example.com"}, + authenticated_user=existing_user, + ) + + with patch(PATCH_USER_LOGIN), patch.object( + ProxyAuthMiddleware, "_auto_join_workspace" + ) as mock_join: + middleware(request) + + mock_join.assert_called_once_with(existing_user) + + @pytest.mark.django_db + def test_profile_check_is_cached_for_the_session(self, django_user_model): + """ + GIVEN a session already flagged as profile-checked + WHEN the middleware processes a further request on that session + THEN it does not re-check the profile + + Asserted by deleting the Profile after the first pass: if the flag were + ignored the second pass would recreate it, which would mean a SELECT on + every request rather than one per session. + """ + existing_user = django_user_model.objects.create_user( + email="cached@example.com", + username="cached_user", + password="irrelevant", + ) + middleware = make_middleware() + request = make_request( + meta={"HTTP_X_AUTH_REQUEST_EMAIL": "cached@example.com"}, + authenticated_user=existing_user, + ) + + with patch(PATCH_USER_LOGIN): + middleware(request) + Profile.objects.filter(user=existing_user).delete() + middleware(request) + + assert not Profile.objects.filter(user=existing_user).exists() + class TestProxyAuthMiddlewareUserSwitch: """Stale Django session must not survive an upstream identity change."""