Draft: Enforce auth session cookie lifetime on the server side

Armin Pašalićrequested to merge
duo/fix/1184-enforce-auth-cookie-lifetime into master

What does this MR do?

Enforces the gitlab-pages auth session cookie lifetime (auth-cookie-session-timeout, default 10 minutes) on the server side. Until now it was only limited client-side through Max-Age.

Closes #1184 (closed)

Root cause

  • In internal/auth/auth.go, New() created the store with sessions.NewCookieStore(keys[0], keys[1]), which kept gorilla/securecookie's default codec MaxAge of 30 days.
  • In internal/auth/session.go, getSessionFromStore set only session.Options.MaxAge. That value goes into the browser Max-Age attribute and is not used when the server validates the cookie.
  • As a result, a copied cookie kept working after GitLab logout until the OAuth access token expired (about 2 hours), not the intended 10 minutes.

Fix

  1. Codec MaxAge: New() now calls store.MaxAge(CookieSessionTimeout in seconds). The securecookie codecs then reject cookies whose signed timestamp is older than the timeout. A zero (or sub-second) timeout keeps the old behavior. Zero means "browser-session cookie", so the codec keeps its 30-day default and existing defaults don't change.
  2. Re-save finding: The session is not re-saved on a normal authenticated request. checkAuthentication with a valid token and a 200 from pages_access doesn't call Save (the one exception is a single save to clear the SSO re-auth marker). However, a session that holds a token can be re-saved with the token kept, and each re-save refreshes the securecookie timestamp. For example, requesting /auth?domain=<allowed domain>&state=x with the cookie (handleProxyingAuth) saves the session with all its values before redirecting to GitLab. With only fix 1, that would turn the limit into a sliding window that is capped only by the OAuth token's own expiry. To prevent this:
    • The OAuth callback now stores access_token_issued_at next to access_token. This follows the existing state_issued_at pattern and uses a.now().
    • getSessionFromStore drops access_token once it is older than CookieSessionTimeout. Every consumer (checkTokenExists, GetTokenIfExists, the forbidden re-auth path) then treats the session as unauthenticated.
    • destroySession also clears the issued-at value.
  3. Re-auth, no 500:
    • When the codec rejects an expired cookie, the existing checkSession error path runs: it saves a fresh empty session and redirects to the same URL. That request has no token, so checkTokenExists redirects to the normal OAuth flow (/auth?domain=…&state=… and then GitLab authorize).
    • When only the issued-at check fails, the token is dropped and the request goes straight to the OAuth flow.

Tradeoffs / notes

  • Sessions created before this change have no access_token_issued_at. For those, only the codec MaxAge applies (time since the cookie was last saved). I didn't reject them outright. During a rolling deploy, a callback handled by an old pod would issue a cookie without the key, and a new pod would keep rejecting it, which could cause a redirect loop. This gap only lasts as long as the pre-deploy OAuth tokens (about 2 hours).
  • There is no feature flag. The change only enforces the existing configured timeout.
  • This is a bugfunctional defense-in-depth change, fixed in public as agreed with AppSec in the issue. It doesn't revoke the OAuth token on GitLab logout. It limits how long a copied cookie can be used to CookieSessionTimeout.

Tests (internal/auth/auth_test.go)

  • TestCheckAuthenticationEnforcesCookieSessionTimeoutServerSide:
    • A fresh cookie (9 min old) is accepted.
    • An 11-minute-old cookie is rejected without calling the API, is redirected to itself, and the next request redirects to the OAuth flow.
    • A zero timeout keeps the default lifetime (a 24h-old cookie is still accepted).
    • securecookie stamps cookies with the wall clock and has no hook to change it, so a test helper re-signs the cookie's date|value|mac payload with a backdated timestamp. The "fresh" case also confirms that the re-signed cookie is valid.
  • TestCheckAuthenticationRejectsAccessTokenOlderThanCookieSessionTimeout: uses a freshly saved cookie, which is the re-save scenario, and controls time through auth.now. The token is accepted 1s before the timeout and dropped at the timeout, with a redirect to OAuth.
  • TestTryAuthenticateStoresAccessTokenIssuedAt: the callback records the issued-at value.

TODO

Merge request reports