diff --git a/.env.example b/.env.example index f54417e7..81dfda8c 100644 --- a/.env.example +++ b/.env.example @@ -164,6 +164,15 @@ REPORT_DIGEST_MAX_REPORTS=50 REPORT_RETENTION_DAYS=180 REPORT_PURGE_CRON=0 5 * * * +# --- OAuth redirect targets --- +# Where an OAuth flow may return to, beyond the web app's own page. Comma +# separated and matched *exactly* - the target receives the exchange code, so a +# loose match hands whoever owns the address a session. +OAUTH_REDIRECT_ALLOWLIST= +# The app's own scheme, e.g. tdn://oauth-success. A flow returning here gets its +# refresh token in the exchange response body rather than in a cookie. +OAUTH_NATIVE_REDIRECT_ALLOWLIST= + # --- Mobile clients --- # How long after a rotation a retired refresh token is still accepted as a # retry rather than treated as a stolen one. Mobile clients lose the *response* diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 35fadf9b..71345ff2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,6 +28,9 @@ env: GITHUB_CLIENT_SECRET: dummy_github_secret GOOGLE_CLIENT_ID: dummy_google_id GOOGLE_CLIENT_SECRET: dummy_google_secret + # The app target the OAuth e2e cases start a flow for. Any value works as + # long as the tests use the same one; it is never dialled. + OAUTH_NATIVE_REDIRECT_ALLOWLIST: tdn://oauth-success DISABLE_RATE_LIMIT: true HUSKY: "0" diff --git a/CLAUDE.md b/CLAUDE.md index 407fd0af..1d568771 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,7 +75,11 @@ Auth decorators: `fastify.authenticate` (required) and `fastify.optionalAuthenti One API, two clients. There is no separate mobile endpoint set; the difference is which **channel** a session is delivered on. -**The channel rule:** a request is answered on the channel it arrived on. `/auth/refresh` and `/auth/logout` read the refresh token from the signed cookie *or* the request body, and refresh answers on whichever one carried it — so a browser, which always reaches us through the cookie, can never be answered with a refresh token in the body. That is the whole of the web's protection here and it is not conditional on anything the caller claims. Login has no incoming channel to mirror, so it takes `client: "web" | "native"` (absent means web); `native` returns `refreshToken`/`refreshTokenExpiresAt` in the body and sets no cookie. The flag grants nothing to an attacker — it only lets somebody who already has the password receive the token differently. **The OAuth exchange deliberately has no such flag:** the callback hands the exchange code to a web page today, so a native channel there would be reachable from page JavaScript, which could trade a code it can already see for a thirty-day refresh token. It gets one when the callback learns to redirect to the app's own scheme, and not before. +**The channel rule:** a request is answered on the channel it arrived on. `/auth/refresh` and `/auth/logout` read the refresh token from the signed cookie *or* the request body, and refresh answers on whichever one carried it — so a browser, which always reaches us through the cookie, can never be answered with a refresh token in the body. That is the whole of the web's protection here and it is not conditional on anything the caller claims. Login has no incoming channel to mirror, so it takes `client: "web" | "native"` (absent means web); `native` returns `refreshToken`/`refreshTokenExpiresAt` in the body and sets no cookie. The flag grants nothing to an attacker — it only lets somebody who already has the password receive the token differently. **The OAuth exchange takes no such flag** — and must not. The channel is recorded on the exchange code when the flow *starts*, from the redirect target it was started for, and read back in `OAuthExchangeUseCase`. Whoever calls the exchange endpoint chooses nothing: a browser holding a code it can see in its own URL would otherwise trade it for a thirty-day refresh token instead of a fifteen-minute access token. + +`GET /oauth/{github,google}?redirect=…` picks that target from an **exact-match** allow-list (`OAUTH_REDIRECT_ALLOWLIST` for browsers, `OAUTH_NATIVE_REDIRECT_ALLOWLIST` for the app's scheme; absent means the web app's own page). No prefix test, no host comparison — the target receives the exchange code, so a loose match hands a session to whoever owns the address. An unknown target is a 400, not a quiet fallback. + +The target is stored against a random `state` (`BeginOAuthUseCase`, 10-minute TTL in the cache) and spent by the callback (`ConsumeOAuthStateUseCase`, single use). That closes something that was open before this existed: with no `state`, an attacker could start a flow with their own account and have a victim's browser finish it, leaving them signed in as the attacker. A callback with no usable state completes nothing and is answered on the default web target with `?error=invalid_state` — every exit from a callback is a redirect, because there is no client left to read a problem document. **Rotation has a grace window.** Reuse detection is strict — presenting a retired token revokes every session — which is right on the web and hazardous on a phone, where a refresh whose *response* is lost leaves the client retrying with a token already retired. `RefreshToken.revokedAt` and `replacedById` let `RefreshUseCase.resolveRetry` tell the two apart: inside `REFRESH_ROTATION_GRACE_SECONDS` (30), with a successor that is still untouched, it is a retry — the successor is retired in turn and a fresh pair issued. Tokens are stored hashed, so the lost response cannot be replayed; the retry gets new tokens, not the old ones. Outside the window, or with a successor that has been used, it is the alarm it always was. diff --git a/render.yaml b/render.yaml index 0ca09a33..8e8056ac 100644 --- a/render.yaml +++ b/render.yaml @@ -169,6 +169,15 @@ projects: sync: false - key: DATABASE_URL sync: false + # Where an OAuth flow may return to. Both are exact-match lists; the + # native one also switches that flow's session onto the response body + # instead of a cookie, which is why it is a separate list rather than a + # rule about URL schemes. Empty means the web app's own page only, which + # is what the flow did before it could be asked. + - key: OAUTH_REDIRECT_ALLOWLIST + sync: false + - key: OAUTH_NATIVE_REDIRECT_ALLOWLIST + sync: false # Mobile clients. All four have defaults in env.schema.ts, so the service # boots without them; they are declared because the two build numbers are # what lets the API refuse a version that is too old to be talked to, and diff --git a/src/core/ports/services/github-auth.port.ts b/src/core/ports/services/github-auth.port.ts index 2254b8fd..03a9b7e2 100644 --- a/src/core/ports/services/github-auth.port.ts +++ b/src/core/ports/services/github-auth.port.ts @@ -22,9 +22,14 @@ export interface GithubAuthPort { /** * Generates the GitHub OAuth authorization URL to redirect the user to. * + * @param state - Opaque value the provider hands back on the callback. It + * is what ties a callback to the flow that started it: without one, an + * attacker can feed a victim's browser a callback of their own and have it + * complete a login as somebody else, and there is nowhere to record which + * client asked for the flow. * @returns The full authorization URL including required query parameters. */ - getAuthorizationUrl(): string; + getAuthorizationUrl(state: string): string; /** * Exchanges an authorization code for tokens and retrieves the authenticated user's profile. diff --git a/src/core/ports/services/google-auth.port.ts b/src/core/ports/services/google-auth.port.ts index 6c2a442d..1bc00c75 100644 --- a/src/core/ports/services/google-auth.port.ts +++ b/src/core/ports/services/google-auth.port.ts @@ -19,9 +19,14 @@ export interface GoogleAuthPort { /** * Generates the Google OAuth authorization URL to redirect the user to. * + * @param state - Opaque value the provider hands back on the callback. It + * is what ties a callback to the flow that started it: without one, an + * attacker can feed a victim's browser a callback of their own and have it + * complete a login as somebody else, and there is nowhere to record which + * client asked for the flow. * @returns The full authorization URL including required query parameters. */ - getAuthorizationUrl(): string; + getAuthorizationUrl(state: string): string; /** * Exchanges an authorization code for tokens and retrieves the authenticated user's profile. diff --git a/src/core/use-cases/oauth/oauth-exchange/oauth-exchange.usecase.ts b/src/core/use-cases/oauth/oauth-exchange/oauth-exchange.usecase.ts index 54b01360..7bb2fc89 100644 --- a/src/core/use-cases/oauth/oauth-exchange/oauth-exchange.usecase.ts +++ b/src/core/use-cases/oauth/oauth-exchange/oauth-exchange.usecase.ts @@ -8,11 +8,21 @@ import type { LoginOutput } from "@core/use-cases/auth/login/login.output"; import { UnauthorizedError } from "@core/errors"; import { AuthMapper } from "../../auth/auth.mapper"; import type { OAuthExchangeInput } from "./oauth-exchange.input"; +import type { OAuthDelivery } from "../oauth-state"; export interface OAuthExchangePayload { userId: string; username: string; isEmailVerified: boolean; + + /** + * Which channel the session belongs on, decided when the flow started. + * + * Carried on the code rather than asked of the caller: whoever holds the + * code decides nothing here, because a browser holding one could otherwise + * ask for the body channel and read out a thirty-day refresh token. + */ + delivery?: OAuthDelivery; } export class OAuthExchangeUseCase { @@ -22,7 +32,9 @@ export class OAuthExchangeUseCase { private readonly refreshTokenRepository: IRefreshTokenRepository, ) {} - async execute(input: OAuthExchangeInput): Promise { + async execute( + input: OAuthExchangeInput, + ): Promise { const cacheKey = `oauth:exchange:${input.code}`; const raw = await this.cacheService.get(cacheKey); @@ -55,6 +67,10 @@ export class OAuthExchangeUseCase { }); return { + // Absent on a code minted before this field existed, which can + // only be one already in flight: the cookie is what those flows + // expected. + delivery: payload.delivery ?? "cookie", user: { ...AuthMapper.toUserOutput(userPayload), isEmailVerified: payload.isEmailVerified, diff --git a/src/core/use-cases/oauth/oauth-github/github-login.input.ts b/src/core/use-cases/oauth/oauth-github/github-login.input.ts index d00082ab..01057e9c 100644 --- a/src/core/use-cases/oauth/oauth-github/github-login.input.ts +++ b/src/core/use-cases/oauth/oauth-github/github-login.input.ts @@ -1,3 +1,8 @@ +import type { OAuthDelivery } from "../oauth-state"; + export interface GithubLoginInput { code: string; + + /** Which channel the session this produces belongs on. */ + delivery: OAuthDelivery; } diff --git a/src/core/use-cases/oauth/oauth-github/github-login.usecase.ts b/src/core/use-cases/oauth/oauth-github/github-login.usecase.ts index 288cd0a9..21e44aee 100644 --- a/src/core/use-cases/oauth/oauth-github/github-login.usecase.ts +++ b/src/core/use-cases/oauth/oauth-github/github-login.usecase.ts @@ -62,6 +62,7 @@ export class GithubLoginUseCase { userId: user.id, username: user.username, isEmailVerified: user.isEmailVerified, + delivery: input.delivery, }; await this.cacheService.set( diff --git a/src/core/use-cases/oauth/oauth-google/google-login.input.ts b/src/core/use-cases/oauth/oauth-google/google-login.input.ts index b96fc72a..2f9bca29 100644 --- a/src/core/use-cases/oauth/oauth-google/google-login.input.ts +++ b/src/core/use-cases/oauth/oauth-google/google-login.input.ts @@ -1,3 +1,8 @@ +import type { OAuthDelivery } from "../oauth-state"; + export interface GoogleLoginInput { code: string; + + /** Which channel the session this produces belongs on. */ + delivery: OAuthDelivery; } diff --git a/src/core/use-cases/oauth/oauth-google/google.login.usecase.ts b/src/core/use-cases/oauth/oauth-google/google.login.usecase.ts index 598dac97..cd2bf6bc 100644 --- a/src/core/use-cases/oauth/oauth-google/google.login.usecase.ts +++ b/src/core/use-cases/oauth/oauth-google/google.login.usecase.ts @@ -62,6 +62,7 @@ export class GoogleLoginUseCase { userId: user.id, username: user.username, isEmailVerified: user.isEmailVerified, + delivery: input.delivery, }; await this.cacheService.set( diff --git a/src/core/use-cases/oauth/oauth-state/index.ts b/src/core/use-cases/oauth/oauth-state/index.ts new file mode 100644 index 00000000..86c24e41 --- /dev/null +++ b/src/core/use-cases/oauth/oauth-state/index.ts @@ -0,0 +1,21 @@ +/** + * This module exports the use cases that start an OAuth flow and read back + * what it was started for. + */ +export { + BeginOAuthUseCase, + ConsumeOAuthStateUseCase, +} from "./oauth-state.usecase"; +export type { OAuthProvider } from "./oauth-state.usecase"; +/** + * This module exports the redirect target rules an OAuth flow is bound to. + */ +export { + defaultRedirectTarget, + resolveRedirectTarget, +} from "./oauth-redirect-target"; +export type { + OAuthDelivery, + OAuthRedirectConfig, + OAuthRedirectTarget, +} from "./oauth-redirect-target"; diff --git a/src/core/use-cases/oauth/oauth-state/oauth-redirect-target.ts b/src/core/use-cases/oauth/oauth-state/oauth-redirect-target.ts new file mode 100644 index 00000000..378359c2 --- /dev/null +++ b/src/core/use-cases/oauth/oauth-state/oauth-redirect-target.ts @@ -0,0 +1,112 @@ +/** + * How the session that comes out of an OAuth flow reaches the client. + * + * Decided by the flow rather than by whoever calls the exchange endpoint. The + * exchange code is handed to whatever the callback redirected to, so the + * redirect target is the only thing that knows whether a browser or an app is + * on the other end - and a client that could simply ask for the body channel + * would let page JavaScript trade a code it can see for a thirty-day refresh + * token. + */ +export type OAuthDelivery = "cookie" | "body"; + +/** + * Where a finished OAuth flow sends the browser, and on which channel the + * session it produced should be delivered. + */ +export interface OAuthRedirectTarget { + /** Absolute URL the exchange code is appended to. */ + successUrl: string; + + /** Absolute URL failures are reported to. */ + errorUrl: string; + + delivery: OAuthDelivery; +} + +/** + * The redirect targets a deployment accepts. + */ +export interface OAuthRedirectConfig { + /** Origin the web app is served from. */ + frontendUrl: string; + + /** Extra browser targets, matched exactly. */ + webAllowList: string[]; + + /** App targets - a custom scheme, matched exactly. */ + nativeAllowList: string[]; +} + +/** + * Strips a trailing slash so two spellings of the same origin do not become + * two different allow-list entries. + * + * @param url - The URL to normalise + * @returns The URL without its trailing slashes + */ +function trimTrailingSlash(url: string): string { + return url.replace(/\/+$/, ""); +} + +/** + * The target used when a caller asks for nothing in particular. + * + * Exactly what the flow did before it could be asked: success lands on the + * web app's OAuth page, failure on its login page. + * + * @param config - The configured targets + * @returns The default browser target + */ +export function defaultRedirectTarget( + config: OAuthRedirectConfig, +): OAuthRedirectTarget { + const origin = trimTrailingSlash(config.frontendUrl); + + return { + successUrl: `${origin}/oauth-success`, + errorUrl: `${origin}/login`, + delivery: "cookie", + }; +} + +/** + * Resolves the target a caller asked to be returned to. + * + * Exact string matching against the configured lists, deliberately: no prefix + * test, no host comparison, no "starts with our domain". Every one of those is + * how an open redirect gets built, and here it would not just bounce a visitor + * somewhere unpleasant - it would hand an OAuth exchange code, and with it a + * whole session, to whoever owned the address. + * + * @param requested - The redirect the caller asked for, if any + * @param config - The configured targets + * @returns The resolved target, or null when the request named something that + * is not allow-listed + */ +export function resolveRedirectTarget( + requested: string | undefined, + config: OAuthRedirectConfig, +): OAuthRedirectTarget | null { + if (!requested) return defaultRedirectTarget(config); + + const candidate = trimTrailingSlash(requested); + + if (config.webAllowList.map(trimTrailingSlash).includes(candidate)) { + return { + successUrl: candidate, + errorUrl: candidate, + delivery: "cookie", + }; + } + + if (config.nativeAllowList.map(trimTrailingSlash).includes(candidate)) { + return { + successUrl: candidate, + errorUrl: candidate, + delivery: "body", + }; + } + + return null; +} diff --git a/src/core/use-cases/oauth/oauth-state/oauth-state.usecase.ts b/src/core/use-cases/oauth/oauth-state/oauth-state.usecase.ts new file mode 100644 index 00000000..bf407a01 --- /dev/null +++ b/src/core/use-cases/oauth/oauth-state/oauth-state.usecase.ts @@ -0,0 +1,159 @@ +import { BadRequestError } from "@core/errors"; +import type { CachePort } from "@core/ports/services/cache.port"; +import type { CryptoPort } from "@core/ports/services/crypto.port"; +import type { GithubAuthPort } from "@core/ports/services/github-auth.port"; +import type { GoogleAuthPort } from "@core/ports/services/google-auth.port"; +import { + defaultRedirectTarget, + resolveRedirectTarget, + type OAuthRedirectConfig, + type OAuthRedirectTarget, +} from "./oauth-redirect-target"; + +/** Which provider a flow is being started with. */ +export type OAuthProvider = "github" | "google"; + +/** + * How long a started flow may take to come back. + * + * Long enough for somebody to read a consent screen, find their password + * manager and pass a second factor; short enough that an abandoned flow does + * not leave a usable state value lying in the cache for the afternoon. + */ +const STATE_TTL_SECONDS = 600; + +const STATE_KEY_PREFIX = "oauth:state:"; + +/** + * Starts an OAuth flow and remembers what it was started for. + * + * Two things are recorded against a random `state` value: where the callback + * should return to, and which channel the resulting session belongs on. Both + * are decided here, when the flow begins, rather than read from the callback + * or from whoever later calls the exchange endpoint - neither of which can be + * trusted to describe the flow they are finishing. + * + * The state value also does what state is for. Without one, an attacker can + * start a flow with their own account, hand the resulting callback URL to a + * victim, and have the victim's browser quietly finish it - leaving them + * signed in as the attacker, typing into an account somebody else can read. + */ +export class BeginOAuthUseCase { + /** + * Creates a new instance of BeginOAuthUseCase. + * + * @param githubAuthService - Builds the GitHub authorization URL + * @param googleAuthService - Builds the Google authorization URL + * @param cryptoService - Source of the random state value + * @param cacheService - Where the state is held until the callback + * @param oauthRedirectConfig - The targets this deployment accepts + */ + constructor( + private readonly githubAuthService: GithubAuthPort, + private readonly googleAuthService: GoogleAuthPort, + private readonly cryptoService: CryptoPort, + private readonly cacheService: CachePort, + private readonly oauthRedirectConfig: OAuthRedirectConfig, + ) {} + + /** + * Mints a state value and returns the provider URL to send the user to. + * + * @param provider - Which provider to start with + * @param requestedRedirect - Where the caller wants to be returned to + * @returns The authorization URL to redirect to + * + * @throws BadRequestError - When the requested redirect is not allow-listed + */ + async execute( + provider: OAuthProvider, + requestedRedirect?: string, + ): Promise<{ authorizationUrl: string }> { + const target = resolveRedirectTarget( + requestedRedirect, + this.oauthRedirectConfig, + ); + + // Refused rather than quietly redirected somewhere safe: a caller + // asking for an address we do not know is either misconfigured or + // probing, and both are better answered plainly. + if (!target) { + throw new BadRequestError("Unknown OAuth redirect target."); + } + + const state = this.cryptoService.generateRandomHex(32); + + await this.cacheService.set( + `${STATE_KEY_PREFIX}${state}`, + JSON.stringify(target), + STATE_TTL_SECONDS, + ); + + const authorizationUrl = + provider === "github" + ? this.githubAuthService.getAuthorizationUrl(state) + : this.googleAuthService.getAuthorizationUrl(state); + + return { authorizationUrl }; + } +} + +/** + * Reads back what a flow was started for, and spends the state doing it. + */ +export class ConsumeOAuthStateUseCase { + /** + * Creates a new instance of ConsumeOAuthStateUseCase. + * + * @param cacheService - Where the state was held + * @param oauthRedirectConfig - The targets this deployment accepts + */ + constructor( + private readonly cacheService: CachePort, + private readonly oauthRedirectConfig: OAuthRedirectConfig, + ) {} + + /** + * Resolves the target a callback belongs to. + * + * Single use: the value is deleted before it is trusted, so a callback URL + * that is replayed - or handed to somebody else - finds nothing. + * + * A callback with no usable state is not treated as fatal. It is answered + * on the default web target with an error, because the alternative is a + * blank page: this runs in a browser being redirected back from a provider, + * and by then there is nobody left to read a JSON problem document. What it + * must not do is complete the sign-in, and it does not. + * + * @param state - The state value the provider handed back + * @returns The target the flow was started for, or null when the state is + * missing, expired or already spent + */ + async execute( + state: string | undefined, + ): Promise { + if (!state) return null; + + const key = `${STATE_KEY_PREFIX}${state}`; + const raw = await this.cacheService.get(key); + + if (!raw) return null; + + await this.cacheService.delete(key); + + try { + return JSON.parse(raw) as OAuthRedirectTarget; + } catch { + return null; + } + } + + /** + * The target a callback with no usable state has to be answered on. + * + * @returns The default browser target + */ + fallbackTarget(): OAuthRedirectTarget { + return defaultRedirectTarget(this.oauthRedirectConfig); + } +} diff --git a/src/http/controllers/oauth.controller.ts b/src/http/controllers/oauth.controller.ts index 57d3fcc1..ccb00941 100644 --- a/src/http/controllers/oauth.controller.ts +++ b/src/http/controllers/oauth.controller.ts @@ -1,12 +1,22 @@ import { BaseAuthController } from "./base-auth.controller"; -import type { FastifyReply, FastifyRequest, FastifyInstance } from "fastify"; -import type { GithubAuthPort } from "@core/ports/services/github-auth.port"; +import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify"; import { AccountPendingDeletionError } from "@core/errors"; +import type { GithubAuthPort } from "@core/ports/services/github-auth.port"; import type { GithubLoginUseCase } from "@core/use-cases/oauth/oauth-github"; import type { GoogleAuthPort } from "@core/ports/services/google-auth.port"; import type { GoogleLoginUseCase } from "@core/use-cases/oauth/oauth-google"; import type { OAuthExchangeUseCase } from "@core/use-cases/oauth/oauth-exchange"; +import type { + BeginOAuthUseCase, + ConsumeOAuthStateUseCase, + OAuthProvider, + OAuthRedirectTarget, +} from "@core/use-cases/oauth/oauth-state"; import type { OAuthExchangeBody } from "@typings/schemas/oauth/oauth-exchange.schema"; +import type { OAuthStartQuery } from "@typings/schemas/oauth/oauth-start.schema"; + +/** What a provider hands back on the callback. */ +type CallbackQuery = { code?: string; error?: string; state?: string }; export class OAuthController extends BaseAuthController { constructor( @@ -15,137 +25,204 @@ export class OAuthController extends BaseAuthController { private readonly googleAuthService: GoogleAuthPort, private readonly googleLoginUseCase: GoogleLoginUseCase, private readonly oauthExchangeUseCase: OAuthExchangeUseCase, + private readonly beginOAuthUseCase: BeginOAuthUseCase, + private readonly consumeOAuthStateUseCase: ConsumeOAuthStateUseCase, config: FastifyInstance["config"], ) { super(config); } - private get frontendUrl(): string { - return this.config.FRONTEND_URL; + /** + * Starts a GitHub flow. + * + * @param request - The request, optionally naming a redirect target + * @param reply - The reply to send + */ + async github( + request: FastifyRequest<{ Querystring: OAuthStartQuery }>, + reply: FastifyReply, + ): Promise { + await this.start("github", request, reply); } - github(_request: FastifyRequest, reply: FastifyReply): void { - const url = this.githubAuthService.getAuthorizationUrl(); - reply.redirect(url); + /** + * Starts a Google flow. + * + * @param request - The request, optionally naming a redirect target + * @param reply - The reply to send + */ + async google( + request: FastifyRequest<{ Querystring: OAuthStartQuery }>, + reply: FastifyReply, + ): Promise { + await this.start("google", request, reply); } + /** + * Finishes a GitHub flow. + * + * @param request - The callback, carrying the code and the state + * @param reply - The reply to send + */ async githubCallback( - request: FastifyRequest<{ - Querystring: { code?: string; error?: string }; - }>, + request: FastifyRequest<{ Querystring: CallbackQuery }>, reply: FastifyReply, ): Promise { - const { code, error } = request.query; - - if (error) { - return reply.redirect( - `${this.frontendUrl}/login?error=github_access_denied`, - ); - } + await this.finish("github", request, reply); + } - if (!code) { - return reply.redirect( - `${this.frontendUrl}/login?error=missing_code`, - ); - } + /** + * Finishes a Google flow. + * + * @param request - The callback, carrying the code and the state + * @param reply - The reply to send + */ + async googleCallback( + request: FastifyRequest<{ Querystring: CallbackQuery }>, + reply: FastifyReply, + ): Promise { + await this.finish("google", request, reply); + } - try { - const { exchangeCode } = await this.githubLoginUseCase.execute({ - code, - }); + /** + * Trades a single-use exchange code for a session. + * + * The channel is read off the code rather than asked of the caller. The + * flow that minted it is the only thing that knows whether a browser or an + * app is waiting, and a caller allowed to choose could trade a code it can + * already see - one sitting in a page's own URL - for a thirty-day refresh + * token instead of a fifteen-minute access token. + * + * @param request - The request carrying the exchange code + * @param reply - The reply to send + */ + async exchange( + request: FastifyRequest<{ Body: OAuthExchangeBody }>, + reply: FastifyReply, + ): Promise { + const response = await this.oauthExchangeUseCase.execute({ + code: request.body.code, + deviceIp: request.ip, + userAgent: request.headers["user-agent"] ?? "Unknown Device", + }); - reply.redirect( - `${this.frontendUrl}/oauth-success?code=${exchangeCode}`, - ); - } catch (err: unknown) { - if (err instanceof AccountPendingDeletionError) { - return reply.redirect( - `${this.frontendUrl}/oauth-success?error=account_pending_deletion&recoveryToken=${err.recoveryToken}`, - ); - } + const delivered = this.deliverRefreshToken(reply, { + channel: response.delivery, + refreshToken: response.tokens.refreshToken, + refreshTokenExpiresAt: response.tokens.refreshTokenExpiresAt, + }); - reply.redirect( - `${this.frontendUrl}/oauth-success?error=oauth_failed`, - ); - } + reply.status(200).send({ + data: { + accessToken: response.tokens.accessToken, + expiresAt: response.tokens.expiresAt, + ...delivered, + user: response.user, + }, + meta: { timestamp: new Date().toISOString() }, + }); } - google(_request: FastifyRequest, reply: FastifyReply): void { - const url = this.googleAuthService.getAuthorizationUrl(); - reply.redirect(url); + /** + * Sends the user to a provider, having recorded what the flow is for. + * + * @param provider - Which provider to start with + * @param request - The request, optionally naming a redirect target + * @param reply - The reply to send + */ + private async start( + provider: OAuthProvider, + request: FastifyRequest<{ Querystring: OAuthStartQuery }>, + reply: FastifyReply, + ): Promise { + const { authorizationUrl } = await this.beginOAuthUseCase.execute( + provider, + request.query.redirect, + ); + + reply.redirect(authorizationUrl); } - async googleCallback( - request: FastifyRequest<{ - Querystring: { code?: string; error?: string }; - }>, + /** + * Completes a flow and redirects to wherever it was started for. + * + * Every exit from here is a redirect. This runs in a browser being sent + * back from a provider, so there is nobody to read a problem document - + * the only way to report anything is to put it in the address the caller + * is being returned to. + * + * @param provider - Which provider is calling back + * @param request - The callback, carrying the code and the state + * @param reply - The reply to send + */ + private async finish( + provider: OAuthProvider, + request: FastifyRequest<{ Querystring: CallbackQuery }>, reply: FastifyReply, ): Promise { - const { code, error } = request.query; + const { code, error, state } = request.query; + + const target = await this.consumeOAuthStateUseCase.execute(state); + + // A callback with no usable state is a callback that cannot be tied to + // a flow anybody started here: a replay, an expired attempt, or a link + // an attacker built to sign somebody into an account of theirs. It is + // answered, on the default target, without completing anything. + if (!target) { + return this.fail( + reply, + this.consumeOAuthStateUseCase.fallbackTarget(), + "invalid_state", + ); + } if (error) { - return reply.redirect( - `${this.frontendUrl}/login?error=google_access_denied`, - ); + return this.fail(reply, target, `${provider}_access_denied`); } if (!code) { - return reply.redirect( - `${this.frontendUrl}/login?error=missing_code`, - ); + return this.fail(reply, target, "missing_code"); } try { - const { exchangeCode } = await this.googleLoginUseCase.execute({ - code, - }); + const { exchangeCode } = await (provider === "github" + ? this.githubLoginUseCase.execute({ + code, + delivery: target.delivery, + }) + : this.googleLoginUseCase.execute({ + code, + delivery: target.delivery, + })); reply.redirect( - `${this.frontendUrl}/oauth-success?code=${exchangeCode}`, + `${target.successUrl}?code=${encodeURIComponent(exchangeCode)}`, ); } catch (err: unknown) { if (err instanceof AccountPendingDeletionError) { return reply.redirect( - `${this.frontendUrl}/oauth-success?error=account_pending_deletion&recoveryToken=${err.recoveryToken}`, + `${target.successUrl}?error=account_pending_deletion&recoveryToken=${encodeURIComponent(err.recoveryToken)}`, ); } - reply.redirect( - `${this.frontendUrl}/oauth-success?error=oauth_failed`, - ); + return this.fail(reply, target, "oauth_failed"); } } - async exchange( - request: FastifyRequest<{ Body: OAuthExchangeBody }>, + /** + * Reports a failure on the target the flow was started for. + * + * @param reply - The reply to send + * @param target - Where this flow is being returned to + * @param reason - The error code the client renders + */ + private fail( reply: FastifyReply, - ): Promise { - const response = await this.oauthExchangeUseCase.execute({ - code: request.body.code, - deviceIp: request.ip, - userAgent: request.headers["user-agent"] ?? "Unknown Device", - }); - - // Cookie only, deliberately. The exchange code is handed to a *web - // page* today - the callback redirects to FRONTEND_URL with the code - // in the query string - so a native channel here would be reachable - // from page JavaScript, which could trade the code it can already see - // for a thirty-day refresh token instead of a fifteen-minute access - // token. The app gets its own channel when the callback learns to - // redirect to the app's scheme, and not before. - this.setRefreshTokenCookie( - reply, - response.tokens.refreshToken, - response.tokens.refreshTokenExpiresAt, + target: OAuthRedirectTarget, + reason: string, + ): void { + reply.redirect( + `${target.errorUrl}?error=${encodeURIComponent(reason)}`, ); - - reply.status(200).send({ - data: { - accessToken: response.tokens.accessToken, - expiresAt: response.tokens.expiresAt, - user: response.user, - }, - meta: { timestamp: new Date().toISOString() }, - }); } } diff --git a/src/http/plugins/di/use-cases.di.ts b/src/http/plugins/di/use-cases.di.ts index 17710ccd..73c5887a 100644 --- a/src/http/plugins/di/use-cases.di.ts +++ b/src/http/plugins/di/use-cases.di.ts @@ -13,6 +13,10 @@ import { ResetPasswordUseCase } from "@core/use-cases/auth/reset-password"; import { RecoverAccountUseCase } from "@core/use-cases/auth/recover-account"; import { GoogleLoginUseCase } from "@core/use-cases/oauth/oauth-google"; import { OAuthExchangeUseCase } from "@core/use-cases/oauth/oauth-exchange"; +import { + BeginOAuthUseCase, + ConsumeOAuthStateUseCase, +} from "@core/use-cases/oauth/oauth-state"; import { PurgeExpiredUsersUseCase } from "@core/use-cases/user/purge-expired-users"; import { PurgeExpiredTokensUseCase } from "@core/use-cases/auth/purge-expired-tokens"; import { GetMeUserUseCase } from "@core/use-cases/user/get-me"; @@ -109,6 +113,22 @@ import { * * shared dependencies across the application. */ +/** + * Splits a comma-separated environment list, dropping blanks. + * + * An empty variable must produce no entries rather than one empty string, + * which would otherwise allow-list the empty redirect. + * + * @param value - The raw environment value + * @returns The trimmed, non-empty entries + */ +function splitList(value: string): string[] { + return value + .split(",") + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0); +} + export const useCasesModule = { /** * Use case for the morning digest email @@ -149,6 +169,43 @@ export const useCasesModule = { ), ).singleton(), + /** + * Use case that starts an OAuth flow and records what it is for + */ + beginOAuthUseCase: asFunction( + ( + githubAuthService, + googleAuthService, + cryptoService, + cacheService, + oauthRedirectConfig, + ) => + new BeginOAuthUseCase( + githubAuthService, + googleAuthService, + cryptoService, + cacheService, + oauthRedirectConfig, + ), + ).singleton(), + + /** + * Use case that reads back, and spends, an OAuth flow's state + */ + consumeOAuthStateUseCase: asFunction( + (cacheService, oauthRedirectConfig) => + new ConsumeOAuthStateUseCase(cacheService, oauthRedirectConfig), + ).singleton(), + + /** + * The redirect targets an OAuth flow may be returned to + */ + oauthRedirectConfig: asFunction((config) => ({ + frontendUrl: config.FRONTEND_URL, + webAllowList: splitList(config.OAUTH_REDIRECT_ALLOWLIST), + nativeAllowList: splitList(config.OAUTH_NATIVE_REDIRECT_ALLOWLIST), + })).singleton(), + /** * Use case for reporting a post or a comment */ diff --git a/src/http/routes/oauth/oauth.route.ts b/src/http/routes/oauth/oauth.route.ts index 5a15b6de..95c96e3a 100644 --- a/src/http/routes/oauth/oauth.route.ts +++ b/src/http/routes/oauth/oauth.route.ts @@ -12,6 +12,10 @@ import { OAuthExchangeResponseSchema, type OAuthExchangeBody, } from "@typings/schemas/oauth/oauth-exchange.schema"; +import { + OAuthStartQuerySchema, + type OAuthStartQuery, +} from "@typings/schemas/oauth/oauth-start.schema"; /** * Sets up OAuth routes on the Fastify instance @@ -22,18 +26,21 @@ import { export function oauthRoutes(fastify: FastifyInstance): void { const oauthController = fastify.diContainer.cradle.oauthController; - fastify.get( + fastify.get<{ Querystring: OAuthStartQuery }>( "/github", { config: { rateLimit: RateLimitPolicies.STANDARD }, schema: { + querystring: OAuthStartQuerySchema, tags: ["OAuth"], }, }, oauthController.github.bind(oauthController), ); - fastify.get<{ Querystring: { code?: string; error?: string } }>( + fastify.get<{ + Querystring: { code?: string; error?: string; state?: string }; + }>( "/github/callback", { config: { rateLimit: RateLimitPolicies.STRICT }, @@ -44,18 +51,21 @@ export function oauthRoutes(fastify: FastifyInstance): void { oauthController.githubCallback.bind(oauthController), ); - fastify.get( + fastify.get<{ Querystring: OAuthStartQuery }>( "/google", { config: { rateLimit: RateLimitPolicies.STANDARD }, schema: { + querystring: OAuthStartQuerySchema, tags: ["OAuth"], }, }, oauthController.google.bind(oauthController), ); - fastify.get<{ Querystring: { code?: string; error?: string } }>( + fastify.get<{ + Querystring: { code?: string; error?: string; state?: string }; + }>( "/google/callback", { config: { rateLimit: RateLimitPolicies.STRICT }, diff --git a/src/http/types/schemas/env.schema.ts b/src/http/types/schemas/env.schema.ts index fae48016..e986fda9 100644 --- a/src/http/types/schemas/env.schema.ts +++ b/src/http/types/schemas/env.schema.ts @@ -236,6 +236,18 @@ export const EnvSchema = Type.Object({ // large file rather than a target - clients compress first. MEDIA_MAX_FILE_SIZE_MB: Type.Number({ default: 10, minimum: 1 }), + // Where an OAuth flow may return to, beyond the web app's own page. + // Comma-separated and matched *exactly* - no prefix test, no host + // comparison. The target receives the exchange code, so a loose match here + // hands whoever owns the address a session, which is how open redirects + // stop being cosmetic. + OAUTH_REDIRECT_ALLOWLIST: Type.String({ default: "" }), + // The same, for the app's own scheme (e.g. tdn://oauth-success). A flow + // returning to one of these delivers its refresh token in the exchange + // response body rather than in a cookie - which is why the two lists are + // separate rather than one list with a rule about schemes. + OAUTH_NATIVE_REDIRECT_ALLOWLIST: Type.String({ default: "" }), + // --- Mobile clients --- // A web client is whatever was served this morning; an app version lives on // phones for months. These let the API tell a build that it is too old to diff --git a/src/http/types/schemas/oauth/oauth-start.schema.ts b/src/http/types/schemas/oauth/oauth-start.schema.ts new file mode 100644 index 00000000..fbb2ffd3 --- /dev/null +++ b/src/http/types/schemas/oauth/oauth-start.schema.ts @@ -0,0 +1,17 @@ +import { type Static, Type } from "@fastify/type-provider-typebox"; + +/** + * Where the caller wants to be returned to when the flow finishes. + * + * Matched exactly against the configured allow-list; anything else is refused + * rather than redirected somewhere safe. The value is not a hint - the target + * receives the exchange code, and with it a session. + * + * Absent means the web app's own OAuth page, which is what every caller wanted + * before there was anything else to want. + */ +export const OAuthStartQuerySchema = Type.Object({ + redirect: Type.Optional(Type.String({ maxLength: 500 })), +}); + +export type OAuthStartQuery = Static; diff --git a/src/infrastructure/external/github-auth.service.ts b/src/infrastructure/external/github-auth.service.ts index ba8103c8..98cacd81 100644 --- a/src/infrastructure/external/github-auth.service.ts +++ b/src/infrastructure/external/github-auth.service.ts @@ -31,13 +31,14 @@ export class GithubAuthService implements GithubAuthPort { return s.slice(0, maxLen).replace(/_+$/g, ""); } - getAuthorizationUrl(): string { + getAuthorizationUrl(state: string): string { const rootUrl = "https://github.com/login/oauth/authorize"; const options = { client_id: this.config.clientId, redirect_uri: this.config.callbackUrl, scope: "user:email", + state, }; const qs = new URLSearchParams(options); diff --git a/src/infrastructure/external/google-auth.service.ts b/src/infrastructure/external/google-auth.service.ts index 6c949103..a56899e1 100644 --- a/src/infrastructure/external/google-auth.service.ts +++ b/src/infrastructure/external/google-auth.service.ts @@ -15,9 +15,10 @@ export interface GoogleAuthConfig { export class GoogleAuthService implements GoogleAuthPort { constructor(private readonly config: GoogleAuthConfig) {} - getAuthorizationUrl(): string { + getAuthorizationUrl(state: string): string { const rootUrl = "https://accounts.google.com/o/oauth2/v2/auth"; const options = { + state, redirect_uri: this.config.callbackUrl, client_id: this.config.clientId, access_type: "offline", diff --git a/tests/e2e/oauth/redirect.test.ts b/tests/e2e/oauth/redirect.test.ts index c4e3f454..61828f48 100644 --- a/tests/e2e/oauth/redirect.test.ts +++ b/tests/e2e/oauth/redirect.test.ts @@ -2,35 +2,97 @@ import { request } from "../setup"; import { describe, expect, it } from "vitest"; const FRONTEND_URL = "http://localhost:5173"; +const NATIVE_TARGET = "tdn://oauth-success"; /** - * E2E tests for OAuth redirect endpoints. - * Validates that GitHub and Google authorization endpoints redirect correctly, - * and that callback endpoints handle error/missing-code scenarios with proper redirects. + * E2E tests for the OAuth redirect endpoints. * - * Note: Full OAuth callback flows (valid authorization codes) require real external - * provider APIs and are therefore out of scope for e2e tests. + * Full callback flows need a real provider and are out of scope. What is in + * scope, and what these cover, is everything that happens around the provider: + * a flow is bound to a `state` when it starts, the callback spends that state + * exactly once, and a callback that cannot be tied to a flow completes nothing. + * + * Note: `tdn://oauth-success` has to be in `OAUTH_NATIVE_REDIRECT_ALLOWLIST` + * for the app-target cases; CI sets it. */ describe("OAuth Redirect Endpoints", () => { - describe("GET /oauth/github - GitHub Authorization Redirect", () => { - it("should redirect to GitHub authorization URL", async () => { + /** + * Starts a flow and returns the state the provider would hand back. + */ + const startFlow = async ( + provider: "github" | "google", + redirect?: string, + ): Promise<{ statusCode: number; location: string; state: string }> => { + const response = await request({ + method: "GET", + url: `/oauth/${provider}${redirect ? `?redirect=${encodeURIComponent(redirect)}` : ""}`, + }); + + const location = response.headers.location as string | undefined; + const state = location + ? (new URL(location).searchParams.get("state") ?? "") + : ""; + + return { statusCode: response.statusCode, location: location ?? "", state }; + }; + + describe("starting a flow", () => { + it("should redirect to GitHub with a state parameter", async () => { + const { statusCode, location, state } = await startFlow("github"); + + expect(statusCode).toBe(302); + expect(location).toMatch( + /^https:\/\/github\.com\/login\/oauth\/authorize/, + ); + expect(state.length).toBeGreaterThan(16); + }); + + it("should redirect to Google with a state parameter", async () => { + const { statusCode, location, state } = await startFlow("google"); + + expect(statusCode).toBe(302); + expect(location).toMatch( + /^https:\/\/accounts\.google\.com\/o\/oauth2\/v2\/auth/, + ); + expect(state.length).toBeGreaterThan(16); + }); + + it("should mint a different state each time", async () => { + const first = await startFlow("github"); + const second = await startFlow("github"); + + expect(first.state).not.toBe(second.state); + }); + + it("should refuse a redirect target that is not allow-listed", async () => { const response = await request({ method: "GET", - url: "/oauth/github", + url: "/oauth/github?redirect=https%3A%2F%2Fevil.example%2Fsteal", }); - expect(response.statusCode).toBe(302); - expect(response.headers.location).toMatch( - /^https:\/\/github\.com\/login\/oauth\/authorize/, + // Refused rather than quietly sent somewhere safe: the target + // receives the exchange code, so guessing is not an option. + expect(response.statusCode).toBe(400); + }); + + it("should accept an allow-listed app target", async () => { + const { statusCode, state } = await startFlow( + "github", + NATIVE_TARGET, ); + + expect(statusCode).toBe(302); + expect(state.length).toBeGreaterThan(16); }); }); - describe("GET /oauth/github/callback - GitHub Callback", () => { - it("should redirect to frontend login page when provider returns an error", async () => { + describe("finishing a flow", () => { + it("should report a provider error on the target the flow started for", async () => { + const { state } = await startFlow("github"); + const response = await request({ method: "GET", - url: "/oauth/github/callback?error=access_denied", + url: `/oauth/github/callback?error=access_denied&state=${state}`, }); expect(response.statusCode).toBe(302); @@ -39,10 +101,12 @@ describe("OAuth Redirect Endpoints", () => { ); }); - it("should redirect to frontend login page when authorization code is missing", async () => { + it("should report a missing code on the target the flow started for", async () => { + const { state } = await startFlow("google"); + const response = await request({ method: "GET", - url: "/oauth/github/callback", + url: `/oauth/google/callback?state=${state}`, }); expect(response.statusCode).toBe(302); @@ -50,44 +114,62 @@ describe("OAuth Redirect Endpoints", () => { `${FRONTEND_URL}/login?error=missing_code`, ); }); - }); - describe("GET /oauth/google - Google Authorization Redirect", () => { - it("should redirect to Google authorization URL", async () => { + it("should send an app flow's failure to the app", async () => { + const { state } = await startFlow("github", NATIVE_TARGET); + const response = await request({ method: "GET", - url: "/oauth/google", + url: `/oauth/github/callback?error=access_denied&state=${state}`, }); - expect(response.statusCode).toBe(302); - expect(response.headers.location).toMatch( - /^https:\/\/accounts\.google\.com\/o\/oauth2\/v2\/auth/, + expect(response.headers.location).toBe( + `${NATIVE_TARGET}?error=github_access_denied`, ); }); - }); - describe("GET /oauth/google/callback - Google Callback", () => { - it("should redirect to frontend login page when provider returns an error", async () => { + it("should complete nothing for a callback with no state", async () => { + // The shape of a forged callback: an attacker starts a flow with + // their own account and hands the victim the callback URL. const response = await request({ method: "GET", - url: "/oauth/google/callback?error=access_denied", + url: "/oauth/github/callback?code=whatever", }); expect(response.statusCode).toBe(302); expect(response.headers.location).toBe( - `${FRONTEND_URL}/login?error=google_access_denied`, + `${FRONTEND_URL}/login?error=invalid_state`, ); }); - it("should redirect to frontend login page when authorization code is missing", async () => { + it("should complete nothing for a state that was never issued", async () => { const response = await request({ method: "GET", - url: "/oauth/google/callback", + url: "/oauth/github/callback?code=whatever&state=made-up-state", }); - expect(response.statusCode).toBe(302); expect(response.headers.location).toBe( - `${FRONTEND_URL}/login?error=missing_code`, + `${FRONTEND_URL}/login?error=invalid_state`, + ); + }); + + it("should spend a state exactly once", async () => { + const { state } = await startFlow("github"); + + const first = await request({ + method: "GET", + url: `/oauth/github/callback?error=access_denied&state=${state}`, + }); + const replay = await request({ + method: "GET", + url: `/oauth/github/callback?error=access_denied&state=${state}`, + }); + + expect(first.headers.location).toBe( + `${FRONTEND_URL}/login?error=github_access_denied`, + ); + expect(replay.headers.location).toBe( + `${FRONTEND_URL}/login?error=invalid_state`, ); }); }); diff --git a/tests/unit/core/use-cases/oauth/oauth-state.test.ts b/tests/unit/core/use-cases/oauth/oauth-state.test.ts new file mode 100644 index 00000000..e72d5d25 --- /dev/null +++ b/tests/unit/core/use-cases/oauth/oauth-state.test.ts @@ -0,0 +1,186 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + BeginOAuthUseCase, + ConsumeOAuthStateUseCase, + resolveRedirectTarget, + type OAuthRedirectConfig, +} from "@core/use-cases/oauth/oauth-state"; +import { BadRequestError } from "@core/errors"; +import type { CachePort } from "@core/ports/services/cache.port"; +import type { CryptoPort } from "@core/ports/services/crypto.port"; +import type { GithubAuthPort } from "@core/ports/services/github-auth.port"; +import type { GoogleAuthPort } from "@core/ports/services/google-auth.port"; + +const CONFIG: OAuthRedirectConfig = { + frontendUrl: "https://tdn.example", + webAllowList: ["https://beta.tdn.example/oauth-success"], + nativeAllowList: ["tdn://oauth-success"], +}; + +describe("resolveRedirectTarget", () => { + it("should send a caller that asked for nothing to the web app", () => { + const target = resolveRedirectTarget(undefined, CONFIG); + + expect(target).toEqual({ + successUrl: "https://tdn.example/oauth-success", + errorUrl: "https://tdn.example/login", + delivery: "cookie", + }); + }); + + it("should deliver an allow-listed app target in the body", () => { + expect(resolveRedirectTarget("tdn://oauth-success", CONFIG)).toEqual({ + successUrl: "tdn://oauth-success", + errorUrl: "tdn://oauth-success", + delivery: "body", + }); + }); + + it("should keep an allow-listed web target on the cookie", () => { + expect( + resolveRedirectTarget( + "https://beta.tdn.example/oauth-success", + CONFIG, + )?.delivery, + ).toBe("cookie"); + }); + + it("should refuse anything not on a list", () => { + for (const attempt of [ + "https://evil.example/oauth-success", + // Prefix and suffix games: an exact match is the only match. + "https://tdn.example.evil.test/oauth-success", + "https://beta.tdn.example/oauth-success/../../steal", + "https://beta.tdn.example/oauth-success?next=https://evil.example", + "//evil.example", + "tdn://oauth-success-evil", + "javascript:alert(1)", + ]) { + expect(resolveRedirectTarget(attempt, CONFIG)).toBeNull(); + } + }); + + it("should treat a trailing slash as the same target", () => { + expect( + resolveRedirectTarget("tdn://oauth-success/", CONFIG)?.delivery, + ).toBe("body"); + }); +}); + +describe("BeginOAuthUseCase", () => { + let cache: Pick; + let crypto: Pick; + let github: Pick; + let google: Pick; + let useCase: BeginOAuthUseCase; + + beforeEach(() => { + cache = { + set: vi.fn().mockResolvedValue(undefined), + get: vi.fn(), + delete: vi.fn(), + }; + crypto = { generateRandomHex: vi.fn().mockReturnValue("state-1") }; + github = { + getAuthorizationUrl: vi.fn().mockReturnValue("https://github/auth"), + }; + google = { + getAuthorizationUrl: vi.fn().mockReturnValue("https://google/auth"), + }; + + useCase = new BeginOAuthUseCase( + github as GithubAuthPort, + google as GoogleAuthPort, + crypto as CryptoPort, + cache as CachePort, + CONFIG, + ); + }); + + it("should pass the state to the provider and record the target", async () => { + const result = await useCase.execute("github", "tdn://oauth-success"); + + expect(result.authorizationUrl).toBe("https://github/auth"); + expect(github.getAuthorizationUrl).toHaveBeenCalledWith("state-1"); + expect(cache.set).toHaveBeenCalledWith( + "oauth:state:state-1", + JSON.stringify({ + successUrl: "tdn://oauth-success", + errorUrl: "tdn://oauth-success", + delivery: "body", + }), + expect.any(Number), + ); + }); + + it("should start a Google flow through the Google service", async () => { + await useCase.execute("google"); + + expect(google.getAuthorizationUrl).toHaveBeenCalledWith("state-1"); + expect(github.getAuthorizationUrl).not.toHaveBeenCalled(); + }); + + it("should refuse a target that is not allow-listed", async () => { + await expect( + useCase.execute("github", "https://evil.example/steal"), + ).rejects.toThrow(BadRequestError); + + // Nothing is minted for a flow that will not be started. + expect(cache.set).not.toHaveBeenCalled(); + }); +}); + +describe("ConsumeOAuthStateUseCase", () => { + let cache: Pick; + let useCase: ConsumeOAuthStateUseCase; + + beforeEach(() => { + cache = { + get: vi.fn(), + delete: vi.fn().mockResolvedValue(undefined), + }; + useCase = new ConsumeOAuthStateUseCase(cache as CachePort, CONFIG); + }); + + it("should return the recorded target and spend the state", async () => { + vi.mocked(cache.get).mockResolvedValue( + JSON.stringify({ + successUrl: "tdn://oauth-success", + errorUrl: "tdn://oauth-success", + delivery: "body", + }), + ); + + const target = await useCase.execute("state-1"); + + expect(target?.delivery).toBe("body"); + expect(cache.delete).toHaveBeenCalledWith("oauth:state:state-1"); + }); + + it("should return null for a state that was never issued", async () => { + vi.mocked(cache.get).mockResolvedValue(null); + + expect(await useCase.execute("made-up")).toBeNull(); + }); + + it("should return null when the callback carries no state at all", async () => { + // This is the shape of a forged callback, and of every callback from + // before the state existed. Neither may complete a sign-in. + expect(await useCase.execute(undefined)).toBeNull(); + expect(cache.get).not.toHaveBeenCalled(); + }); + + it("should return null when the stored value is unreadable", async () => { + vi.mocked(cache.get).mockResolvedValue("not json"); + + expect(await useCase.execute("state-1")).toBeNull(); + }); + + it("should offer the web target as the fallback", () => { + expect(useCase.fallbackTarget()).toEqual({ + successUrl: "https://tdn.example/oauth-success", + errorUrl: "https://tdn.example/login", + delivery: "cookie", + }); + }); +});