Recover from 403 on expired SSO session by re-authenticating

What does this MR do?

When a user's SAML SSO session expires, the Rails pages_access API returns 403 (not 401) for an otherwise valid cookie session. gitlab-pages previously recovered only from a 401 invalid_token response, so a 403 fell through to ServeNotFoundAuthFailed (404) without clearing the stale session cookie. Users stayed stuck on the dead cookie until it expired, which produced the intermittent errors reported by customers with SSO enforcement enabled (gitlab-com/request-for-help#4770).

This MR makes the page-serving auth flow destroy the session and restart OAuth on a 403 that carries a cookie-based session token, mirroring the existing 401 recovery path.

Behaviour change (scoped to the page-serving path)

• 403 from pages_access with a cookie session token: destroy session and redirect through OAuth to re-authenticate.
• A second consecutive 403 after a re-auth attempt (tracked via the sso_reauth_attempted session marker): falls through to the normal not-found handler, so genuinely unauthorized users are not trapped in a redirect loop.
• The marker is cleared on a successful (200) authorization, so a later legitimate expiry can recover again.
• 403 via the Authorization header: unchanged. There is no cookie session to refresh, so the user gets a genuine 404.
• 403 with no session token: unchanged.

Scope: what this does not cover

This fixes the 403-to-404 sequence confirmed in the server logs: pages_access returns 403, gitlab-pages tries a namespace 404 page, hits "domain does not exist", and serves a 404, all in ~36ms. The 503 symptoms in the ticket come from a different path. checkAuthenticationResponse returns 503 (httperrors.Serve503) when the OAuth token fetch itself fails during the callback, which this MR does not touch. Clearing the stale cookie on a 403 removes the most common trigger for the reported failures; any remaining 503s during the OAuth round-trip should be tracked separately.

Follow-up: Issue #1190 (closed) tracks the 503 from OAuth token-fetch failures during the /auth callback (out of scope for this MR).

Why the change is in checkAuthentication, not checkResponseForInvalidToken

checkResponseForInvalidToken is shared with the artifact handler (internal/handlers) through the exported CheckResponseForInvalidToken. In that path a 403-with-token legitimately means "project has no public pipelines" and must not re-authenticate, which TestForbiddenWithTokenIsNotHandled locks in. The 403 SSO re-auth therefore lives in checkAuthentication (the page-serving flow) only, leaving the artifact flow and its test untouched.

Accepted trade-off: extra OAuth round-trip for unauthorized users

Genuinely unauthorized users pay one extra OAuth round trip on their first 403: pages re-authenticates them (assuming an expired SSO session), they complete OAuth, and the second 403 then serves the not-found page. This is intentional — we cannot distinguish an expired SSO session from a genuine authorization failure on the first 403 alone. The marker is cleared on a 200, so if the same user later accesses a project that returns 200 on the same host, the marker is gone and a subsequent SSO expiry can recover again (at the cost of another extra round trip).

Concurrent requests

Several requests can observe the same first 403 at once, for example when a Teams link preview and the user's own click fire together. Each can redirect to OAuth before any marker is saved to the cookie, which costs one extra redirect per request. The flow does not loop: the first request to land a fresh session sets the marker, and a later 403 then fails the marker check and serves the not-found page. This behaviour is documented in a comment on reauthenticateExpiredSession.

ProjectPrefixCookiePath loop-guard gap (fixed in this MR)

Background: FF_ENABLE_PROJECT_PREFIX_COOKIE_PATH (default-enabled) scopes the authenticated session cookie to the project-prefix path (e.g. Path=/subgroup/project). getSessionFromStore resets session.Options.Path to / on every request, so a naive Save in the 403 reauth path would write the marker cookie at Path=/ while the stale project-prefix cookie remained. Browsers send the most-specific path first; gorilla's r.Cookie() would keep returning the stale cookie (no marker), and pages would redirect to OAuth on every request — an infinite loop.

Fix: reauthenticateExpiredSession now restores the project-prefix path on session.Options before calling destroySession, so the marker cookie is written at the same path as the stale cookie, overwriting it. The fix is guarded by the FF and only applies when the domain returns a non-trivial project prefix.

Verification:

• Unit: TestReauthenticateExpiredSessionSetsProjectPrefixCookiePath asserts the marker Set-Cookie is written at the project-prefix path.
• Acceptance: TestExpiredSSOSessionProjectPrefixCookiePathNoLoop drives the full re-auth against a running Pages process with a real cookiejar. To reproduce the browser's RFC 6265 path-specificity it addresses Pages by its real virtual host (routed to the listener via the existing test DialContext) rather than 127.0.0.1 with a Host override — the latter pools every host under one jar key and cannot reproduce the loop. It asserts the first 403 re-auths on the same host, the follow-up reaches the OAuth proxy (the marker cookie overwrote the stale one), and a second 403 serves 404 (loop guard). Validated by reverting the fix, which makes the test fail.

How it was built (double-loop TDD)

1. Outer (acceptance), RED: TestExpiredSSOSessionForbiddenRedirectsToReauth drives a running Pages process end-to-end and asserts a 302 re-auth instead of a silent 404.
2. Inner, GREEN: shouldReauthenticateOnForbidden and reauthenticateExpiredSession, invoked from checkAuthentication. A unit test TestCheckAuthenticationWhenForbiddenAndSessionExpiredReauthenticates covers the integration, and a table-driven TestShouldReauthenticateOnForbidden covers each decision branch.
3. Refactor: a self-review pass reduced the diff to the minimal fix. It dropped an unused accessTokenKey constant, moved ssoReauthAttemptedKey next to the existing projectPrefix session-key constant, and reverted an earlier saveSession/destroySession refactor that touched only one of several call sites. destroySession is reused as the re-auth primitive and is otherwise untouched.
4. ProjectPrefixCookiePath gap: identified, fixed, and covered with a unit test (Set-Cookie path) and a faithful cookie-jar acceptance test that reproduces the loop end-to-end and fails when the fix is reverted.

Test coverage (all branches of the new path)

• 403 with cookie session: re-auth (302).
• 403 after re-auth attempt (marker set): 404 (loop-guard).
• 403 via Authorization header: no re-auth, 404.
• 403 with no session token: shouldReauthenticateOnForbidden returns false.
• 200 with marker set: marker cleared, content served.
• ProjectPrefixCookiePath: marker cookie written at the project-prefix path (unit test); no re-auth loop plus loop-guard 404 driven end-to-end with a real cookie jar over real virtual hosts (acceptance test; fails when the fix is reverted).
• Existing 401 invalid_token, WhenNoAccess, and artifact TestForbiddenWithTokenIsNotHandled paths remain unchanged.

Related

• Investigation: gitlab-com/request-for-help#4770
• Follow-up (503 from OAuth token-fetch failures): #1190 (closed)

Author checklist

• Confirm CI is green (unit + acceptance).
• Changelog entry (Changelog: fixed).
• Concurrent-request behaviour documented in code and described above.
• ProjectPrefixCookiePath loop-guard gap verified and fixed.
• Accepted trade-off (extra OAuth round-trip for unauthorized users) documented.