Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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*
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
6 changes: 5 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
9 changes: 9 additions & 0 deletions render.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion src/core/ports/services/github-auth.port.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion src/core/ports/services/google-auth.port.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -22,7 +32,9 @@ export class OAuthExchangeUseCase {
private readonly refreshTokenRepository: IRefreshTokenRepository,
) {}

async execute(input: OAuthExchangeInput): Promise<LoginOutput> {
async execute(
input: OAuthExchangeInput,
): Promise<LoginOutput & { delivery: OAuthDelivery }> {
const cacheKey = `oauth:exchange:${input.code}`;

const raw = await this.cacheService.get(cacheKey);
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions src/core/use-cases/oauth/oauth-github/github-login.input.ts
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export class GithubLoginUseCase {
userId: user.id,
username: user.username,
isEmailVerified: user.isEmailVerified,
delivery: input.delivery,
};

await this.cacheService.set(
Expand Down
5 changes: 5 additions & 0 deletions src/core/use-cases/oauth/oauth-google/google-login.input.ts
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export class GoogleLoginUseCase {
userId: user.id,
username: user.username,
isEmailVerified: user.isEmailVerified,
delivery: input.delivery,
};

await this.cacheService.set(
Expand Down
21 changes: 21 additions & 0 deletions src/core/use-cases/oauth/oauth-state/index.ts
Original file line number Diff line number Diff line change
@@ -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";
112 changes: 112 additions & 0 deletions src/core/use-cases/oauth/oauth-state/oauth-redirect-target.ts
Original file line number Diff line number Diff line change
@@ -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;
}
Loading