Review an OAuth PKCE callback

Review exercise · TypeScript

Review brief

Advanced45 min

A web application is adding sign-in through an OAuth provider. The server creates PKCE attempts, stores their verifier and navigation context, and handles callbacks that may arrive in existing anonymous sessions or report provider errors.

Behavioral contract

Authorization state is generated with at least 128 bits from a cryptographically secure source. It is stored server-side and bound to the same browser by an exact match between the callback `state` parameter and the secure, HTTP-only, SameSite=Lax state cookie.
An attempt expires ten minutes after the injected clock timestamp and is consumed exactly once on every valid callback, including provider-declared errors. Missing, mismatched, expired, unknown, or replayed state is rejected before exchanging a code.
The provider receives the original PKCE verifier and the exact redirect URI stored when authorization began. A callback request cannot choose or reconstruct that redirect URI.
The post-login destination must be a local path beginning with exactly one `/`; protocol-relative values, absolute URLs, backslashes, and control characters fall back to `/`.
Successful authentication always rotates to a newly generated session ID before binding the provider subject. An anonymous or previously authenticated session ID is never retained across login.
Provider errors reveal no token or verifier data and return through the validated local destination after consuming the attempt. Provider token validation is performed by the `exchange` dependency.
`OrdinaryIdGenerator` produces unique operational identifiers but is explicitly not a cryptographic random source. `OAuthStateSecrets` produces at least 128 cryptographically random bits, and `PkceSecrets` produces a valid high-entropy verifier.

Review instructions

1Review the start and callback paths as one protocol, including errors and replays.
2Trace which values come from durable attempt state, callback input, and existing cookies.
3Treat the changed dependency interfaces as part of the protocol design, including whether they can consume an attempt atomically.
4Prioritize no more than six findings and tie each to an exploitable or user-visible condition.

Learning objectives

• Bind and consume OAuth authorization attempts
• Preserve PKCE and redirect-URI integrity
• Establish authenticated sessions without open redirects or fixation

src/oauth-types.tsTypeScript

@@ -0,0 +1,46 @@

1+export interface OAuthAttempt {

2+ state: string;

3+ verifier: string;

4+ redirectUri: string;

5+ returnTo: string;

6+ createdAtMilliseconds: number;

7+}

9+export interface OAuthAttemptStore {

10+ put(attempt: OAuthAttempt): Promise<void>;

11+ get(state: string): Promise<OAuthAttempt | null>;

12+ delete(state: string): Promise<void>;

13+ takeActive(

14+ state: string,

15+ nowMilliseconds: number,

16+ maxAgeMilliseconds: number,

17+ ): Promise<OAuthAttempt | null>;

18+}

19+

20+export interface OAuthProvider {

21+ authorizationUrl(input: {

22+ state: string;

23+ challenge: string;

24+ redirectUri: string;

25+ }): string;

26+ exchange(input: {

27+ code: string;

28+ verifier: string;

29+ redirectUri: string;

30+ }): Promise<{ subject: string }>;

31+}

32+

33+export interface SessionStore {

34+ bindExisting(sessionId: string | undefined, subject: string): Promise<string>;

35+ createRotated(previousSessionId: string | undefined, subject: string): Promise<string>;

36+}

37+

38+export interface OAuthDependencies {

39+ attempts: OAuthAttemptStore;

40+ provider: OAuthProvider;

41+ sessions: SessionStore;

42+ clock: { nowMilliseconds(): number };

43+ ids: { next(): string };

44+ stateSecrets: { nextState(): string };

45+ secrets: { nextVerifier(): { verifier: string; challenge: string } };

46+}

Reference solution

The findings below are ordered by priority.

Findings

1. Authorization state comes from a non-cryptographic ID generator

Location: src/oauth-types.ts:38-45 and src/oauth-service.ts:29-31

start uses the ordinary operational ID source even though the contract explicitly says it is not cryptographically random. Predictable or enumerable state weakens the primary correlation secret protecting the login callback.

Use the dedicated stateSecrets.nextState() source to generate at least 128 cryptographically random bits, while continuing to store the opaque result server-side. Keep the ordinary ID generator for non-secret operational identities.

2. Callback state is not bound to the browser cookie

Location: src/oauth-service.ts:50-59

The nullish fallback accepts either the query value or the cookie value. It allows a callback with no state parameter and, when a parameter is present, never verifies that it exactly equals the cookie. A state created in another browser can therefore drive this browser's login session.

Require both values, compare them exactly in constant-time where supported, and look up the attempt only after they match. Never substitute the cookie for a missing protocol parameter.

3. Attempts never expire or become one-time

Location: src/oauth-types.ts:9-18 and src/oauth-service.ts:32-38,56-66,72-85

Although createdAtMilliseconds and attempts.delete are available, callback checks neither age nor consumption. Successful callbacks and provider-error callbacks both leave the record reusable, so a captured callback can be replayed indefinitely.

Use the store's atomic takeActive operation with the injected current time and ten-minute lifetime after query/cookie state binding succeeds. It must return an active attempt to at most one callback, including provider-error callbacks; missing, expired, or replayed state returns no attempt.

4. The callback request chooses the token-exchange redirect URI

Location: src/oauth-service.ts:72-76

The exchange uses request.redirectUri rather than attempt.redirectUri. Host or proxy-derived callback input can therefore alter a value that OAuth requires to match the original authorization request, causing login failure or weakening redirect binding in a permissive provider integration.

Pass the exact redirect URI stored with the attempt and do not reconstruct it from callback headers or parameters.

5. Protocol-relative return destinations pass validation

Location: src/oauth-redirect.ts:1-3

Checking only startsWith("/") accepts //attacker.example/path, which browsers interpret as a cross-origin protocol-relative URL. It also does not reject backslashes or control characters required to fall back safely.

Accept a parsed local path only when it begins with exactly one forward slash and contains none of the forbidden forms; otherwise return /.

6. Login retains the pre-authentication session ID

Location: src/oauth-service.ts:77-80

bindExisting authenticates the current anonymous or prior session identifier. An attacker who arranged for a victim to use a known session ID can keep that identifier after login and reuse it as an authenticated session.

Call the rotation operation to create a new unpredictable session ID, invalidate the prior identifier, and bind the provider subject only to the replacement.

Reasonable non-findings

A secure, HTTP-only, SameSite=Lax state cookie is appropriate for a top-level OAuth redirect.
The provider dependency owns token and subject validation after a correctly bound exchange.
Clearing the browser state cookie is useful, but it does not replace atomic server-side attempt consumption.