# CSRF, Open-Redirect, and OAuth/OIDC Flow Review

Use this reference when the review scope includes a state-changing request's CSRF defenses, a redirect/return-URL parameter, or an OAuth/OIDC authorization flow. The OAuth/OIDC subsection applies only when an OAuth/OIDC flow is actually present in scope — do not load or apply it otherwise.

## CSRF defenses

### What people get wrong

The common bad assumption is:

> "We use cookies for sessions, so CSRF isn't really our problem — that's an old attack."

CSRF is specifically an attack against cookie-based (or any auto-attached-credential) session mechanisms: the browser automatically attaches the session cookie to a request the attacker's site triggers, without the user's knowledge. It remains directly relevant to any cookie-based session architecture, and is *not* automatically neutralized just because `SameSite` exists — `SameSite` is one layer, not a complete substitute for a token-based defense when the target action is sensitive.

### Grounded defense patterns (per OWASP CSRF Prevention Cheat Sheet)

- **Synchronizer token pattern** — server generates a unique, unpredictable, session-bound token; the client includes it in every state-changing request (hidden form field or custom header for AJAX/fetch); server validates it matches the session before processing. Requires server-side state. Preferred for traditional stateful (cookie-session) applications.
- **Double-submit cookie pattern** — a stateless alternative: the token is set as a cookie and also sent in the request body/header; the server compares the two. The *signed* double-submit variant, which cryptographically binds the token to the session, is the recommended variation — an unsigned token without session binding offers minimal protection and is vulnerable to cookie-injection attacks. Do not accept an unsigned double-submit implementation as adequate.
- **`SameSite` cookie attribute** — `Strict` or `Lax` prevents the browser from attaching the session cookie to most cross-site requests, providing meaningful CSRF mitigation as a primary layer per current OWASP guidance, but treat it as defense-in-depth alongside a token-based pattern for sensitive state-changing operations (password change, payment, account-deletion, permission grants), not as the sole control for those.
- **Custom request headers for API-only surfaces** — for a pure JSON API with no HTML forms, requiring a custom header (e.g., `X-Requested-With`) that only same-origin JS can set (browsers block cross-origin scripts from setting arbitrary headers without a permissive CORS policy) is an accepted lightweight defense, contingent on the API's CORS configuration not being permissive (`Access-Control-Allow-Origin: *` with credentials defeats this).

### Non-negotiables

- Never use `GET` requests for state-changing operations — GET requests are trivially triggerable cross-site (an `<img>` tag, a bare link) with no token protection possible in the same way, and are logged/cached/exposed via browser history and Referer headers.
- Do not transmit CSRF tokens inside cookies for the synchronizer token pattern — tokens for that pattern belong in the response payload (hidden form field, JSON body) and are returned via form submission or a custom header, not round-tripped through a cookie (that would defeat the pattern's separation from the auto-attached credential it is meant to validate).
- Do not accept "we validate the Origin/Referer header" as a complete substitute for a token or `SameSite` defense unless you have confirmed the app also has a documented fallback for the (rare but real) cases where those headers are stripped by proxies or privacy tools — treat header-checking as defense-in-depth, not sole control, unless the app explicitly documents this as its chosen primary defense with that tradeoff acknowledged.
- XSS bypasses CSRF protections entirely (a script running in-origin can read a CSRF token and submit it). If a known, unremediated XSS finding exists in the same review scope, state explicitly that the CSRF defenses reviewed here are undermined by it — do not present the CSRF findings as if they stand alone.

## Open-redirect prevention

### Grounded pattern (per OWASP Unvalidated Redirects and Forwards Cheat Sheet)

- Best: avoid using user input to determine the destination URL at all. Use a short name, ID, or token that the server maps to a full target URL server-side (with care to avoid enumeration issues if the mapping itself is guessable).
- If user input for the destination is unavoidable, validate it server-side against an allow-list of trusted hosts/paths before redirecting. A relative-path-only allow-list (rejecting any input containing a scheme or `//` prefix that would make it absolute/protocol-relative) is a common safe pattern for "return to this page after login" flows.
- If using a regex or string-prefix check for validation, it must be anchored and must account for scheme and protocol-relative URLs (`//evil.example.com` is parsed by browsers as `https://evil.example.com` when used as a redirect target) — an unanchored or naive `startsWith`/`includes` check is bypassable.
- Ideally, force a user-confirmation interstitial ("You are leaving [app] and going to [external site]") for any redirect to a genuinely external destination, even an allow-listed one, for sensitive flows.

### Non-negotiables

- A client-side-only check (JavaScript validation before calling `window.location.assign`/`.href =`) is not a control — an attacker can hit the server-side redirect endpoint directly with the malicious parameter, bypassing any client-side JS entirely. The validation must exist server-side to count as a mitigation.
- Do not accept "we only redirect within our own app" as verified without checking what "within our own app" means in the actual validation code — a check for a substring match (e.g., `url.includes('myapp.com')`) is bypassable by an attacker-controlled URL like `https://myapp.com.evil.example.com` or `https://evil.example.com/?x=myapp.com`.

## OAuth/OIDC flow review (load only when an OAuth/OIDC flow is in scope)

### What people get wrong

The common bad assumption is:

> "OAuth is OAuth — any grant type is fine as long as we're using a real identity provider."

Grant-type choice materially changes the app's attack surface. The implicit grant returns the access token directly in the URL fragment, exposing it to browser history, Referer leakage (in older browsers/misconfigurations), and any script with access to the URL — with no refresh-token issuance. OAuth 2.1 (the current consolidated guidance, an IETF draft at the time of writing — verify against the current draft revision via Context7 before citing exact section numbers) removes the implicit grant and hybrid flows entirely, and requires PKCE for public clients using the authorization code flow.

### Non-negotiable design rules

1. **Public clients (SPA, mobile app — anything that cannot hold a client secret) must use the authorization code flow with PKCE.** Verify both `code_challenge`/`code_challenge_method` on the authorization request and `code_verifier` on the token exchange are present. A code flow without PKCE for a public client is a finding.
2. **Never recommend or approve `response_type=token` (implicit grant) for new work.** If found in an existing app, flag as HIGH and recommend migration to authorization code + PKCE — do not treat it as merely legacy-acceptable.
3. **The `redirect_uri` must be an exact match against a pre-registered value at the authorization server**, not a pattern/wildcard match validated only client-side. Confirm this is enforced server-side (at the identity provider / authorization server config), not assumed.
4. **`state` parameter must be present, unique per authorization request, and validated on callback** to prevent CSRF against the OAuth flow itself (an attacker tricking a victim into completing an auth flow bound to the attacker's session).
5. **Tokens received from the authorization server must be stored per the token-storage guidance** in `references/token-storage-and-cookies.md` — an otherwise-correct PKCE flow that then stores the resulting access/refresh token in `localStorage` still carries the storage-location finding.

### Verification targets

- The actual authorization-request URL construction (grep for `response_type=`, `code_challenge`) to confirm PKCE is wired, not merely available in the SDK but unused.
- The token-exchange (token endpoint) call to confirm `code_verifier` is sent and matches what generated the `code_challenge`.
- The `state`-parameter generation and validation code path.

## When to push back

Push back if the user asks to:

- skip CSRF token implementation because "`SameSite=Lax` covers it" for a sensitive operation (payment, password change, account deletion, permission/role grant) — recommend defense-in-depth for those specifically, per the non-negotiables above,
- add a redirect allow-list check only in client-side JavaScript "to keep it simple" — restate that this is not a control and must exist server-side,
- keep an implicit-grant OAuth flow because "migrating is a bigger lift than this review's scope" — flag it at HIGH regardless of migration-effort framing; effort is a planning input, not a reason to downgrade a structural finding,
- treat Origin/Referer header validation as a complete CSRF defense with no documented fallback for header-stripping scenarios.

# Token Storage and Cookie-Flag Review

Use this reference when the review scope includes where tokens or session IDs are stored client-side, or how session cookies are configured. Grounded in the OWASP Session Management Cheat Sheet (`/owasp/cheatsheetseries` via Context7, or `official_docs` fallback).

## What people get wrong

The common bad assumption is:

> "We use HTTPS, so cookies/tokens are safe."

TLS protects data in transit between the browser and server. It does nothing about:

- a script running in the page's own origin reading `document.cookie` (no `HttpOnly`) or `localStorage`,
- a cookie being sent to the wrong origin or subdomain (`SameSite`/`Domain` misconfiguration),
- a token being replayed after logout because nothing invalidated it server-side.

Storage location and cookie attributes are a separate control surface from transport security. Both must be correct.

## Storage location: the tradeoffs, stated plainly

- **`HttpOnly` cookie** — not readable by JavaScript. The strongest default for a session identifier or refresh token. Cannot be attached manually to cross-origin API calls (the browser does that automatically only for same-origin/configured-domain requests), which is why bearer-token SPAs calling a separate API origin often cannot use this alone.
- **`localStorage`/`sessionStorage`** — readable by any JavaScript running in the page's origin, including any successfully injected script (stored or reflected XSS, a compromised third-party script/CDN dependency, a malicious browser extension with page access). No same-origin isolation between "your code" and "any script that got a foothold." Never the default recommendation for a session token or refresh token.
- **In-memory (JS variable/closure, not persisted)** — not readable via `document.cookie` or storage APIs, and does not survive a page reload without a re-fetch (typically via an `HttpOnly` refresh-token cookie). This is the pattern OAuth 2.1 / current SPA guidance converges on: short-lived access token in memory, refresh mechanism anchored in an `HttpOnly` cookie.

## Cookie-flag compliance table (build this for every session cookie in scope)

| Attribute | Required value | Why |
|---|---|---|
| `HttpOnly` | present | Blocks `document.cookie` read access from JS; the primary XSS-exfiltration mitigation for the cookie. Does not stop the cookie being *sent* during a combined XSS+CSRF attack — pair with CSRF defenses. |
| `Secure` | present | Cookie is only sent over HTTPS; without it, a network attacker on an insecure network segment can observe or (on `SameSite=None` without `Secure`, which browsers reject) inject it. |
| `SameSite` | `Strict` (preferred) or `Lax`; never unset, never `None` without `Secure` | Prevents the browser from attaching the cookie to cross-site requests, mitigating CSRF and cross-origin leakage. Do not rely on an unset value — browser default behavior has changed across versions and should not be the enforcement mechanism. |
| `Domain` | omitted, or scoped to the narrowest subdomain that needs it | An overly broad `Domain` (e.g., a session cookie set on the parent domain when only one subdomain needs it) widens the blast radius if any sibling subdomain is compromised. |
| `Path` | `/` or the narrowest path needed | Broad by convention for session cookies; flag only if a narrower path was clearly intended and not applied. |
| `__Host-` prefix | recommended when `Domain` is omitted and `Path=/` | Browser-enforced: only accepted if `Secure`, no `Domain` attribute, and `Path=/`. Prevents subdomain-forgery and downgrade attacks on the cookie name itself. |

Extract this table from the actual `Set-Cookie` header construction in code (session-middleware config, a manual `res.cookie(...)`/`Set-Cookie` call) — do not infer flags from framework defaults without checking the actual configured options, since defaults vary by framework and version.

## Non-negotiables

- Do not accept "the session cookie is `HttpOnly`" as sufficient on its own without also checking `Secure` and `SameSite` — all three are independent controls addressing different attack vectors (XSS-read, network interception, cross-site request attachment).
- Do not accept a refresh token or long-lived credential in `localStorage`/`sessionStorage` under any framing ("just for this internal tool," "we'll migrate later") without flagging it as a finding — a refresh token grants renewable access and is a higher-value target than a short-lived access token.
- Treat an access token held only in memory (a JS variable, not persisted storage) as acceptable for that token alone, but check separately how the app re-obtains a new access token after a page reload — if that mechanism reads a token from `localStorage` instead of an `HttpOnly` refresh-cookie flow, the finding moves to that mechanism.
- Session ID/token entropy and length are a server-side generation concern (cryptographically secure random generation, sufficient bit length per OWASP Session Management guidance) — verify it if the generation code is in scope, but do not assume insufficient entropy without reading the actual generation call; do not guess.

## Verification targets

- Actual `Set-Cookie` header value (from server code, middleware config, or a captured response header if the user provides sanitized evidence) — not the framework's documented default.
- The token re-acquisition path after page reload/tab reopen, to catch a `localStorage` fallback hiding behind an otherwise-correct in-memory primary storage.
- The logout code path, to confirm it triggers server-side invalidation (see `references/workflow-and-output.md` decision tree) rather than only clearing client-side state.

# Review Workflow and Findings Contract

Use this reference for the step-by-step review procedure, the architecture-classification decision tree, and the required output shape. Load the other two references only for the specific defect class the auth/session code under review actually raises.

## Prerequisites

- Classify the app architecture before anything else:
  - **Cookie-based** — the server sets a session cookie on login and the browser sends it automatically on subsequent requests; no bearer token is manually attached to API calls.
  - **SPA/bearer-token** — the client receives a token (access token, ID token) after authentication and manually attaches it to API requests (typically an `Authorization: Bearer` header), whether via `fetch`/`axios` interceptor or a client SDK.
  - **Hybrid** — a bearer token is used for API calls but a refresh token or session anchor lives in an `HttpOnly` cookie (the current recommended pattern for browser-based apps per OAuth 2.1 guidance).
  - State this classification explicitly in the output before any other finding — cookie-flag findings do not apply to a pure bearer-token flow with no cookies, and token-storage findings about `localStorage` do not apply to a pure cookie-based flow with no client-readable token.
- Read `package.json` and any auth-config files first to confirm the actual auth library/pattern wired up (session middleware, an OAuth/OIDC SDK, a hand-rolled fetch-based flow) before recommending a fix.

## Workflow

1. **Classify the architecture** per the Prerequisites above and state it first.
2. **Locate every token/session-ID storage site.** Grep for `localStorage`, `sessionStorage`, `document.cookie`, cookie-library calls (`Set-Cookie`, `cookie.set`, session-middleware config), and in-memory storage (a JS variable/closure/module-scope singleton). For each, identify what is stored (session ID, access token, refresh token, ID token) and where. See `references/token-storage-and-cookies.md`.
3. **For every session cookie found, extract its attribute set** (`HttpOnly`, `Secure`, `SameSite`, `Domain`, `Path`, and whether a `__Host-`/`__Secure-` prefix is used). Compare against the compliance table in `references/token-storage-and-cookies.md`.
4. **Enumerate every state-changing request path** (form POST, fetch/axios mutation call) reachable from an authenticated session. For each, determine whether a CSRF defense is present (synchronizer token, double-submit cookie, `SameSite=Strict`/`Lax` reliance, or a custom-header-based defense for API-only surfaces). See `references/csrf-redirect-and-oauth.md`.
5. **Enumerate every redirect/return-URL parameter** (post-login redirect, OAuth `redirect_uri` handling, logout redirect, "return to" deep links). For each, trace whether validation happens server-side against an allow-list, client-side only, or not at all.
6. **If an OAuth/OIDC flow is present**, identify the `response_type` and grant type in use. Flag implicit grant (`response_type=token`) as a finding. Confirm PKCE (`code_challenge`/`code_verifier`) is present for the authorization code flow. Load the OAuth/OIDC subsection of `references/csrf-redirect-and-oauth.md` only in this case.
7. **Check logout behavior.** Confirm logout triggers a server-side session/token invalidation (session-store deletion, token-revocation endpoint call), not only a client-side storage clear.
8. **Produce ranked findings** using the output contract below.

## Decision tree

- Session ID or access/refresh token stored in `localStorage`/`sessionStorage` → **HIGH** finding (XSS-exposure risk: any injected script reads it via `document`/`window` APIs with no mitigation). Note whether a known XSS sink exists in the same codebase (raises severity further) or whether none was found in the scope reviewed (still HIGH — absence-of-observed-XSS is not evidence of safety).
- Session cookie missing `HttpOnly` → **HIGH** finding (defeats the cookie's primary XSS-mitigation purpose).
- Session cookie missing `Secure` → **HIGH** finding (cookie can be sent over plaintext HTTP; also blocks safe use of `__Host-`/`__Secure-` prefixes).
- Session cookie with `SameSite=None` and no `Secure` → **HIGH** finding (browsers increasingly reject this combination outright, but code relying on it is broken-by-design regardless).
- Session cookie with `SameSite` unset (relying on browser default) → **MEDIUM** finding — do not rely on browser defaults, which vary by browser and version; require an explicit value.
- State-changing request with no CSRF token, no `SameSite=Strict`/`Lax` cookie reliance, and no custom-header-based defense → **HIGH** finding.
- State-changing request relying solely on `SameSite=Lax`/`Strict` with no token-based defense-in-depth → **MEDIUM** finding — acceptable as a primary defense per current OWASP guidance, but flag the absence of defense-in-depth for sensitive operations (e.g., password change, payment).
- Redirect/return-URL parameter validated only client-side (a JS check before navigation, with no equivalent server-side check) → **HIGH** finding — bypassable by calling the server endpoint directly.
- Redirect/return-URL parameter with no validation at all → **HIGH** finding, open redirect.
- Redirect/return-URL parameter validated server-side against an allow-list of trusted hosts/paths → not a finding.
- OAuth/OIDC flow using `response_type=token` (implicit grant) → **HIGH** finding — current guidance requires authorization code flow with PKCE for public clients.
- OAuth/OIDC authorization code flow present but missing `code_challenge`/`code_verifier` (no PKCE) for a public client (SPA, mobile app with no client secret) → **HIGH** finding.
- OAuth/OIDC authorization code flow with PKCE correctly present → not a finding on flow choice (still check `redirect_uri` validation and token-storage location separately).
- Logout clears client-side storage only, with no server-side session/token invalidation call → **MEDIUM-to-HIGH** finding depending on token lifetime (a short-lived access token narrows the window; a long-lived or non-expiring token makes this HIGH).

## Output contract

Every response from this skill must return:

1. **Architecture classification** — cookie-based, SPA/bearer-token, or hybrid, stated first and driving which finding categories apply.
2. **Ranked findings** — each with file:line, defect category (`token-storage` / `cookie-flags` / `csrf` / `open-redirect` / `oauth-flow` / `logout-invalidation`), and a fix sketch grounded in the cited OWASP/OAuth guidance.
3. **Cookie-flag compliance table** — if session cookies are in scope, a table of cookie name → `HttpOnly`/`Secure`/`SameSite`/prefix status.
4. **Sanitized examples only** — every finding illustrated with placeholder/redacted values, never a real token, cookie, or secret observed in the reviewed code.
5. **Evidence level per finding** — `repo evidence`, `documentation-based`, or `inference`.
6. **Explicit no-live-testing statement** — this skill performs static review only; it never attempts session hijacking, token replay, or CSRF exploitation against a live system.
7. **Verdict** — approve / approve-with-notes / block.
8. **Open questions or out-of-scope items** — e.g., "confirming this CSRF gap is exploitable end-to-end requires a live request against a running instance, not static review," or "JWT signature/algorithm choice is out of scope for this skill — recommend a token-issuance/backend review."

## When to push back

Push back if the user asks to:

- accept `localStorage` token storage because "we don't have any XSS right now" — absence of a known XSS finding in this review's scope is not proof none exists; the storage choice itself is the risk being flagged,
- skip cookie-flag validation because "the framework sets sane defaults" — verify the actual `Set-Cookie` header or session-middleware config; frameworks vary and defaults change across versions,
- treat a client-side-only redirect check as sufficient because "the server also has auth on that route" — authentication on the target route does not validate that the redirect *destination* itself was vetted; these are separate controls,
- approve an implicit-grant OAuth flow because "it's simpler to implement" — current guidance has removed implicit grant from OAuth 2.1 specifically because of its token-leakage-via-URL-fragment and no-refresh-token exposure; simplicity does not offset this,
- treat "we cleared localStorage on logout" as equivalent to server-side session invalidation — a still-valid token can be replayed from a captured request or a second device/tab until it naturally expires.