Auth: restart OAuth flow on token-fetch failure instead of serving 503

<🤖>

Fable 5.1, acting for @armin.pasalic.

Stack note, 2026-10-22. This MR is the base of a six-MR stack for #1190 (closed): !1309 (closed), !1334 (closed), !1335 (closed), !1336 (closed), !1337 (closed), !1338 (closed), each targeting the previous one's branch. Three statements below describe only the state after this MR merges and are revised later in the stack:

  • "Restart, not retry": !1336 (closed) adds one in-place retry before any restart.
  • "The restart is unconditional": !1337 (closed) gates it by failure class, bounds it to once a minute per host, and moves the marker from the session into a dedicated cookie.
  • "Friendlier error page: deferred": !1338 (closed) ships it at the requested page.

Rebased onto master on 2026-10-22: the token exchange keeps sending the PKCE code_verifier (!1354 (merged)) and wraps transport errors with logging.CleanURLError; the stack's contract stub now refuses an exchange without a verifier, as Doorkeeper does.

</🤖>


What does this MR do?

Related to #1190 (closed): when fetchAccessToken fails during the /auth callback, replace the unconditional Serve503 with a one-shot OAuth restart.

Problem

Authorization codes are single-use. The most likely non-200 from /oauth/token is invalid_grant (code already redeemed or expired). Serving 503 guarantees the user cannot recover; restarting the flow mints a fresh code and succeeds.

Additionally, fetchAccessToken previously discarded the response body on non-200, collapsing every failure into errResponseNotOk, which made production 503s indistinguishable between "code was spent" and "token endpoint is down".

Solution

Restart, not retry. On token-fetch failure, redirect the user to the originally requested URI (session.Values["uri"]). Visiting that URI runs checkTokenExists, finds no access_token, and starts a clean OAuth flow with a fresh code. The restart is a 302 back through the browser and /oauth/authorize, so it also acts as a delayed retry for transient 5xx and transport failures.

Loop guard. A session marker (token_fetch_restart_attempted) prevents a redirect loop when the token endpoint is persistently failing. The second failure falls through to the existing 503. The restart is unconditional: gating it on the failure reason was considered twice in review and rejected, because invalid_grant is the case the restart exists for and the marker already bounds the cost to one extra token request per session.

Observability. fetchAccessToken returns a typed tokenFetchError for every failure: transport errors keep their cause via Unwrap, non-200 responses carry the decoded OAuth error/error_description, and a 200 whose body is not the token JSON carries the decode failure. A new Prometheus counter gitlab_pages_auth_token_fetch_failures_total has two labels:

  • reason: transport_error, upstream_5xx, rate_limited, invalid_grant, oauth_error, decode_error (successful status, undecodable body), internal_error (request could not be built).
  • outcome: restarted (first failure, flow restarted transparently) or terminal (marker already set, 503 served).

Page on sum(rate(gitlab_pages_auth_token_fetch_failures_total{outcome="terminal"}[5m])); outcome="restarted" is the leading indicator. The failure log line carries the same reason and outcome fields.

Sentry noise. The token-fetch error is sent to error tracking only on the terminal (second) failure, not on the self-healing first one. Session-save failures on either path are still captured, as elsewhere in this file.

The restart marker is saved at the project-prefix cookie path (FF_ENABLE_PROJECT_PREFIX_COOKIE_PATH, default on), the same cookie that holds the access token and the sso_reauth_attempted marker from !1280 (merged). Browsers send the longest-path cookie first, so this is the cookie the next visit to the originally requested page reads; checkTokenExists then copies the marker into the Path=/auth cookie for the following callback. A first version saved the marker at Path=/, which a token-less project-prefix cookie left behind by an SSO re-authentication would shadow, so a persistently failing token endpoint could restart the flow repeatedly. TestRestartMarkerSurvivesTokenlessProjectPrefixCookie covers that case.

On a successful token fetch the marker is dropped from the session and the access token is saved at the same project-prefix path, so the marker cookie is overwritten. No session cookie is written at Path=/.

After a terminal 503 the marker stays in the project's cookie until it expires (auth-cookie-session-timeout, default 10m) or a later token fetch succeeds. A further failure for that project inside that window serves 503 directly. Other projects on the same host are unaffected.

Concurrent requests

Each concurrent request that hits a failing token endpoint independently sets the marker in its own session cookie. At most one extra redirect per user session; no shared state between sessions.

Review decisions

  • Terminal fallback status code: 503, unchanged, to preserve existing alerting.
  • Restart gating by reason: rejected, see above and the review threads.
  • Friendlier error page: deferred per refinement in #1190 (closed); no follow-up issue filed yet.
  • Security review: not requested. Token validation and storage are unchanged; the marker is a boolean in the existing encrypted session cookie at the same path as the access token, and the restart redirects to the same session URI the success path already uses.
  • MR size: Danger flagged the diff as large. About 70% of it is tests; the production change is one error path in checkAuthenticationResponse plus a metric, so it is kept as one MR.

How to validate

  • Unit: go test ./internal/auth/ -run 'TokenFetch|RestartMarker'
  • Acceptance: make acceptance ARGS='-run TestTokenFetchFailureRestartsOAuthFlow'
  • Manually, with access control enabled: complete a login to a private project, then open the callback URL from that flow again (https://<pages-host>/auth?code=<used-code>&state=<state>). Expect a 302 back to the page and the page served, instead of a 503. With a failing token endpoint (for example an invalid client_secret), expect one restart and then a 503, with gitlab_pages_auth_token_fetch_failures_total{outcome="terminal"} incremented.

TODO

  • Feature flag
    • This feature does not require a feature flag: the change only affects a path that previously served 503, and the restart is bounded to one per project cookie by the marker
  • I added the Changelog trailer to the commits that need to be included in the changelog (e.g. Changelog: fixed)
  • I added unit tests or they are not required
  • I added acceptance tests or they are not required
  • I added documentation (or it's not required): a docs MR against doc/administration/pages/troubleshooting.md is pending, to note the one-shot re-authentication, the reason/outcome log fields and the gitlab_pages_auth_token_fetch_failures_total counter in the "503 error: Client authentication failed due to unknown client" section. Pages metrics are not otherwise documented.
  • I followed code review guidelines
  • I followed Go Style guidelines
Edited by Armin Pašalić

Merge request reports

Loading
Loading