fix(auth): a host may receive a code at its own name, and a refusal is not a redirect

Two changes to who may start a flow here, and where a refusal is delivered.

A host reached at its own hostname sits on a different registrable domain from
this issuer, deliberately: that is what stops a cookie set there from ever
reaching this one. The default rule allows a redirect back to whatever hostname
the request arrived on, so it refused exactly the case the separation created.
Which is a real problem rather than a theoretical one, because a session cookie
without a Domain attribute is host-only, so a browser arriving at one of those
hostnames for the first time carries no cookie whether or not it is signed in,
and sending it here to sign in again changes nothing.

So a client id that is a single hostname under that zone, whose redirect_uri is
https and that same hostname at one reserved path, is allowed. Making the
client id the hostname is the load-bearing part: a token's audience is its
client id, so the session that comes back is bound to the host it will live on
and is not a credential anywhere else.

Separately, and worth its own paragraph: a refused client's redirect_uri was
still used to report the refusal. The check that approves that URI is the one
that just failed, so /authorize was an open redirector to anywhere at all --
no sign-in required, on the hostname people are asked to type a password into.
It is now a page here. Before:

  GET /authorize?client_id=web&redirect_uri=https://somewhere.example/callback
  -> 302 https://somewhere.example/callback?error=unauthorized_client

apps/auth/src/index.ts

@@ -2,6 +2,7 @@ import type { Hyperdrive } from '@cloudflare/workers-types';
 import { issuer } from '@nestri/auth/index';
 import { CodeProvider } from '@nestri/auth/provider/code';
 import { CodeUI } from '@nestri/auth/ui/code';
+import { isDomainMatch } from '@nestri/auth/util';
 import { Actor } from '@nestri/core/actor';
 import { PostgresCodeStore } from '@nestri/core/auth/authorization-code';
 import { PostgresDeviceStore } from '@nestri/core/auth/device-grant';
@@ -45,6 +46,72 @@ type Env = {
  */
 const DEVICE_CLIENTS = new Set(['desktop']);
 
+/**
+ * The zone every user-owned host is reached under, and the one path on it that
+ * may receive an authorization code.
+ *
+ * A host is reached at `<id>.<zone>` through a proxy that authenticates
+ * browsers on its behalf. That proxy cannot be handed a session from here: a
+ * `__Host-` cookie is host-only by definition, so one set on this hostname is
+ * never sent to a different one, and a first request to a host's own name
+ * therefore arrives with no cookie whether or not the person is signed in.
+ *
+ * The proxy closes that by being an ordinary OAuth client — one per hostname —
+ * and exchanging a code for a session it can set on the hostname the browser is
+ * actually standing on. This is the rule that lets it: **the client id must be
+ * the hostname, and the redirect must be that same hostname at the reserved
+ * path below.**
+ *
+ * Making the client id the hostname is not a naming convention. A token is
+ * minted with its audience set to the client id, so it binds the session to the
+ * host it will live on — a cookie lifted off one host is not a credential on
+ * another, and it is not a credential here either. ref(d-0056)
+ */
+const HOST_ZONE = 'nestri.link';
+const HOST_CALLBACK_PATH = '/__nestri/callback';
+
+/**
+ * Whether `clientID` names a single host under {@link HOST_ZONE} and
+ * `redirectURI` is that same host's reserved callback.
+ *
+ * Every clause is load-bearing, because what is being decided is where this
+ * issuer will send an authorization code:
+ *
+ * - **`https` only.** A code is a one-time credential and belongs on a channel
+ *   that cannot be read.
+ * - **The host must equal the client id exactly**, so a client can only ever
+ *   receive a code at its own name.
+ * - **One label under the zone.** `a.b.<zone>` is not a host id, and must not
+ *   be treated as one because `b.<zone>` might be.
+ * - **The path must be exactly the reserved one**, with no query and no
+ *   fragment. A caller-chosen return address on a wildcard of hostnames is an
+ *   open redirector on every one of them, and this is the parameter that would
+ *   be it.
+ */
+function isHostCallback(clientID: string, redirectURI: string): boolean {
+	let url: URL;
+	try {
+		url = new URL(redirectURI);
+	} catch {
+		return false;
+	}
+
+	const label = clientID.toLowerCase().endsWith(`.${HOST_ZONE}`)
+		? clientID.toLowerCase().slice(0, -`.${HOST_ZONE}`.length)
+		: null;
+	if (!label || label.length === 0 || label.includes('.')) {
+		return false;
+	}
+
+	return (
+		url.protocol === 'https:' &&
+		url.host === clientID.toLowerCase() &&
+		url.pathname === HOST_CALLBACK_PATH &&
+		url.search === '' &&
+		url.hash === ''
+	);
+}
+
 /**
  * Enough of an address to be worth trying to deliver to.
  *
@@ -68,6 +135,40 @@ async function firstSteamLink(userID: string): Promise<string> {
 	return link?.id ?? '';
 }
 
+/**
+ * Which clients may start a flow here.
+ *
+ * The default rule allows a redirect back to whatever hostname the request
+ * arrived on, which is right for a site served beside this one and refuses the
+ * proxy in front of user-owned hosts — it redirects to a different registrable
+ * domain on purpose, so that no host's cookie can ever reach this one. That
+ * case is named here; everything else keeps the behaviour it had.
+ *
+ * Exported so it can be tested against a real `/authorize` request rather than
+ * by reading it.
+ */
+export const allowClient = async (
+	input: { clientID: string; redirectURI: string },
+	req: Request
+): Promise<boolean> => {
+	if (isHostCallback(input.clientID, input.redirectURI)) {
+		return true;
+	}
+
+	let redirect: string;
+	try {
+		redirect = new URL(input.redirectURI).hostname;
+	} catch {
+		return false;
+	}
+	if (redirect === 'localhost' || redirect === '127.0.0.1') {
+		return true;
+	}
+	const forwarded = req.headers.get('x-forwarded-host');
+	const host = forwarded ? new URL(`https://${forwarded}`).hostname : new URL(req.url).hostname;
+	return isDomainMatch(redirect, host);
+};
+
 export default {
 	async fetch(request: Request, env: Env, ctx?: ExecutionContext) {
 		Env.init(env as unknown as Record<string, unknown>);
@@ -90,6 +191,15 @@ export default {
 			refreshStore: PostgresRefreshStore(),
 			deviceStore: PostgresDeviceStore(),
 			allowDeviceClient: async (clientID) => DEVICE_CLIENTS.has(clientID),
+			// The default rule allows a redirect back to whatever hostname the
+			// request arrived on, which is right for a site served beside this
+			// one and refuses the proxy in front of user-owned hosts — it
+			// redirects to a different registrable domain on purpose, so that
+			// no host's cookie can ever reach this one.
+			//
+			// So that case is named, and everything else keeps the behaviour it
+			// had.
+			allow: allowClient,
 			// One provider, on purpose.
 			//
 			// Verifying an email address is the only thing that brings an

apps/auth/test/allow.test.ts

@@ -0,0 +1,150 @@
+import { describe, expect, test } from 'bun:test';
+
+import { issuer } from '@nestri/auth/index';
+import { CodeProvider } from '@nestri/auth/provider/code';
+import { MemoryStorage } from '@nestri/auth/storage/memory';
+import { CodeUI } from '@nestri/auth/ui/code';
+import { subjects } from '@nestri/core/auth/subjects';
+
+import { allowClient } from '../src/index.js';
+
+/**
+ * The same issuer the worker builds, with the database taken out and the real
+ * rule about which clients may start a flow left in.
+ *
+ * `allow` is the whole subject of this file, so unlike `worker.test.ts` it is
+ * not stubbed to `true`.
+ */
+const auth = issuer({
+	subjects,
+	storage: MemoryStorage(),
+	allow: allowClient,
+	providers: {
+		code: CodeProvider({
+			...CodeUI({ copy: { code_info: 'test' }, sendCode: async () => {} }),
+			sendCode: async () => {}
+		})
+	},
+	async success(context) {
+		return context.subject('user', { userID: 'usr_test123', linkedAccountID: '' });
+	}
+});
+
+/**
+ * Start an authorization and say only whether the client was allowed.
+ *
+ * An allowed client is redirected on towards the provider; a refused one is
+ * answered by the issuer itself. The distinction is the status, and nothing
+ * below cares about anything past it.
+ */
+async function allowed(clientID: string, redirectURI: string): Promise<boolean> {
+	const url = new URL('https://auth.internal/authorize');
+	url.searchParams.set('client_id', clientID);
+	url.searchParams.set('redirect_uri', redirectURI);
+	url.searchParams.set('response_type', 'code');
+	const response = await auth.request(url.toString());
+	if (response.status !== 302) {
+		return false;
+	}
+	// An allowed client is sent on to the provider, which is a path on this
+	// issuer. Anywhere else is not a sign-in beginning.
+	return (response.headers.get('location') ?? '').startsWith('/');
+}
+
+/**
+ * A browser that reaches one of these hostnames is standing on a different
+ * registrable domain from this issuer, and a session set here can never be
+ * sent there — a `__Host-` cookie has no `Domain` attribute and is host-only,
+ * which is exactly what it is for. The proxy in front of those hosts closes
+ * that by being an ordinary client and exchanging a code for a session it sets
+ * on the hostname the browser is actually on.
+ *
+ * Before this rule existed every case below was refused, including the first.
+ */
+describe('a host may receive a code at its own name', () => {
+	test('the reserved callback on the client id itself is allowed', async () => {
+		expect(await allowed('m123.nestri.link', 'https://m123.nestri.link/__nestri/callback')).toBe(
+			true
+		);
+	});
+
+	test('a code is never sent anywhere but the client id', async () => {
+		// The attack this refuses: a client that names itself as one host and
+		// asks for the code at another.
+		expect(await allowed('m123.nestri.link', 'https://evil.nestri.link/__nestri/callback')).toBe(
+			false
+		);
+		expect(await allowed('m123.nestri.link', 'https://evil.example/__nestri/callback')).toBe(false);
+	});
+
+	test('only the reserved path receives a code', async () => {
+		// Anything else under the hostname is served by the host itself, and a
+		// return address a caller chooses is an open redirector on every
+		// hostname in the zone.
+		expect(await allowed('m123.nestri.link', 'https://m123.nestri.link/')).toBe(false);
+		expect(
+			await allowed('m123.nestri.link', 'https://m123.nestri.link/__nestri/callback/../..')
+		).toBe(false);
+		expect(
+			await allowed('m123.nestri.link', 'https://m123.nestri.link/__nestri/callback?next=x')
+		).toBe(false);
+	});
+
+	test('a code goes over https or it does not go', async () => {
+		expect(await allowed('m123.nestri.link', 'http://m123.nestri.link/__nestri/callback')).toBe(
+			false
+		);
+	});
+
+	test('one label, because a deeper name is not a host id', async () => {
+		// `a.b.zone` must not be treated as a host id just because `b.zone`
+		// might be one.
+		expect(
+			await allowed('a.m123.nestri.link', 'https://a.m123.nestri.link/__nestri/callback')
+		).toBe(false);
+		expect(await allowed('nestri.link', 'https://nestri.link/__nestri/callback')).toBe(false);
+	});
+
+	test('another zone does not get in by using the path', async () => {
+		expect(await allowed('m123.example.com', 'https://m123.example.com/__nestri/callback')).toBe(
+			false
+		);
+	});
+});
+
+describe('everything else keeps the rule it had', () => {
+	test('a redirect back to where the request arrived is still allowed', async () => {
+		expect(await allowed('web', 'https://auth.internal/callback')).toBe(true);
+	});
+
+	test('local development is still allowed', async () => {
+		expect(await allowed('web', 'http://localhost:5173/callback')).toBe(true);
+	});
+
+	test('an unrelated domain is still refused', async () => {
+		expect(await allowed('web', 'https://somewhere.example/callback')).toBe(false);
+	});
+});
+
+/**
+ * A refusal is delivered here, not wherever the refused client asked.
+ *
+ * The issuer reports an error by redirecting to the caller's `redirect_uri`,
+ * which is right once that URI has been approved. This is the case where it has
+ * just been rejected — and honouring it there made `/authorize` an open
+ * redirector to anywhere at all, reachable without signing in, on the hostname
+ * people are asked to type a password into.
+ */
+describe('a refused client does not choose where the refusal goes', () => {
+	test('the refusal is a page here, not a redirect to the caller', async () => {
+		const url = new URL('https://auth.internal/authorize');
+		url.searchParams.set('client_id', 'web');
+		url.searchParams.set('redirect_uri', 'https://somewhere.example/callback');
+		url.searchParams.set('response_type', 'code');
+
+		const response = await auth.request(url.toString());
+
+		expect(response.status).toBe(400);
+		expect(response.headers.get('location')).toBeNull();
+	});
+});

packages/auth/src/issuer.ts

@@ -1702,6 +1702,16 @@ export function issuer<
 		if (err instanceof UnknownStateError) {
 			return auth.forward(c, await error(err, c.req.raw));
 		}
+		// A refused client does not get to choose where the refusal is delivered.
+		// Everything below reports an error by redirecting to the `redirect_uri`
+		// the caller supplied, which is correct once that URI has been approved
+		// and is an open redirector before it has: the check that approves it is
+		// the one that just failed, so honouring it here would turn every
+		// refusal into a redirect to anywhere at all — no sign-in required, on
+		// the hostname people are told to trust with a password.
+		if (err instanceof UnauthorizedClientError) {
+			return c.text(err.description || err.error, 400);
+		}
 		const authorization = await getAuthorization(c);
 		// A device grant has no redirect to carry the error back on, so it is
 		// said here instead. Without this the reporting path throws on a URL