OAuth Security Review skill for Codex

Done well means: the implementation uses the flow current guidance recommends for its client type, every redirect and token is validated against spoofing and replay, and tokens live somewhere script and logs cannot read them. OAuth is authorization; OIDC adds authentication. Mixing them up (using an access token as proof of identity) is itself a bug.

Establish the shape first

Read the code and config before judging. Identify:

1. Client type: SPA/browser, native/mobile, confidential server-side, or machine-to-machine.
2. Flow in use: authorization code (+PKCE), client credentials, device code, or a legacy flow (implicit, ROPC) that should be replaced.
3. Role: are you the client (relying party) or operating the authorization server? This review centres on the client / relying-party side.
4. Library: prefer a maintained, certified library over hand-rolled endpoints. Hand-rolled OAuth is where most of these bugs live.

Client type	Use	Not
SPA / browser	Authorization Code + PKCE, tokens handled by a backend-for-frontend where possible	Implicit (removed in OAuth 2.1), tokens in localStorage
Native / mobile	Authorization Code + PKCE, system browser (AppAuth), claimed https redirect	Embedded webview, custom-scheme without PKCE
Server web app	Authorization Code + PKCE, client secret, tokens server-side	Tokens in the browser
Service-to-service	Client credentials	A user flow with a shared service user

OAuth 2.1 consolidates current best practice: PKCE for all authorization-code clients, exact redirect-URI matching, no implicit or password grant. Review against that baseline.

Checklist

Authorization request and redirect

• state present, unguessable, bound to the user’s session, and verified on return (CSRF defence). Reject if missing or mismatched.
• PKCE: code_challenge with S256 (not plain) sent, code_verifier checked. Required even for confidential clients now.
• redirect_uri registered and matched exactly by the server (scheme, host, path, no wildcards, no trailing-slash or subpath tolerance). Open-redirect-style matching is the classic account-takeover bug.
• OIDC: nonce sent in the request and verified in the ID token to bind it to this session (replay defence).
• No tokens or codes in the URL fragment reaching third-party scripts; no sensitive data in query strings that land in logs/referrers.

Token handling (relying party)

• Authorization code is exchanged server-side (or via BFF), single-use, short-lived.
• ID token validated fully: signature against the IdP’s JWKS (kid, rotate-aware), iss exact match, aud equals this client ID, exp/iat/nbf, and nonce. Never trust an unverified JWT or accept alg: none.
• Access tokens are treated as opaque by the client unless it is the intended audience; do not read claims from someone else’s access token to make decisions.
• The userinfo/ID-token sub (stable, per-issuer) is the account key, not email (email is mutable and can be unverified). Check email_verified before trusting email.
• Storage: web — HttpOnly, Secure, SameSite cookies, not localStorage/sessionStorage; mobile — Keychain/Keystore, not plaintext prefs.

Scopes, refresh, logout

• Request the least scope needed; review any admin/offline scope.
• Refresh tokens for public clients are rotated (new refresh token each use) with reuse detection that revokes the chain; confidential clients store them server-side.
• Token revocation on logout; back-channel logout or short access-token lifetime so revocation is effective. Clearing a cookie is not revocation.

Provider side (if you operate the AS)

• Exact redirect-URI registration, PKCE enforced, codes single-use and short TTL, consent for third-party clients, and no token leakage via error redirects.

High-severity patterns to grep for

rg -n -i "response_type=token|grant_type=password|implicit" .          # legacy flows
rg -n -i "localStorage|sessionStorage" . | rg -i "token|access|id_token|refresh"
rg -n -i "redirect_uri" . | rg -i "startswith|includes|indexOf|regex|\\*"  # loose matching
rg -n -i "state" . | rg -i "oauth|authorize" | rg -vi "verify|compare|===|equals"  # unchecked state
rg -n -i "alg.{0,10}none|verify=false|skipVerification|insecure"       # disabled validation
rg -n -i "decode(?!.*verify)|jwt\\.decode\\(" .                         # decode without verify

Each hit is a lead; confirm against the code path before calling it a finding.

Common account-takeover chains

• Loose redirect_uri matching + attacker-controlled path on the same host → code/token sent to the attacker.
• Missing state → login CSRF, attacker logs victim into the attacker’s account (or vice versa).
• Accepting an ID token whose aud is a different client, or skipping signature verification → forged identity.
• Trusting unverified email from the IdP to match an existing local account → takeover by registering that email at a sloppy IdP.
• Access token from one app replayed at another that does not check aud.

Verification

• Replay an authorization response with a tampered or absent state: the client must reject it.
• Register a lookalike redirect_uri (extra path, different case, trailing slash): the AS must refuse it.
• Present an ID token signed by a different key or with aud set to another client: rejected.
• Confirm tokens are absent from localStorage, URLs, and server/access logs (rg "access_token|id_token" logs).
• For refresh rotation: use a refresh token twice and confirm the second use revokes the session.

Output template

## OAuth/OIDC review: web app "Sign in with Acme" (authorization code + PKCE)
| ID | Severity | Finding | Location | Fix |
|---|---|---|---|---|
| O-1 | Critical | ID token signature not verified (decode only) | auth/callback.ts:38 | Verify against JWKS; check iss/aud/exp/nonce |
| O-2 | High | redirect_uri matched with startsWith | auth/config.ts:12 | Exact-match against the registered set |
| O-3 | Medium | Access token in localStorage | hooks/useAuth.ts:21 | Move to HttpOnly cookie via BFF |
Baseline: OAuth 2.1 / current BCP. PKCE: present (S256). Not reviewed: the IdP's own config.