Skip to content

Commit 107c1fa

Browse files
authored
Merge pull request #556 from Helm-OpenSource/claude/seat-device-e1-entry-links-20261113
feat(seat-device): entry links in the device authorization answer (E-1)
2 parents 103fb16 + 1638dbe commit 107c1fa

4 files changed

Lines changed: 72 additions & 5 deletions

File tree

‎lib/seat-device/contract.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
readSeatDeviceTrustedProxyHops,
2929
resolveSeatDeviceTrustedSource,
3030
seatDeviceCredentialExpiresAt,
31+
seatDeviceEntryLinks,
3132
seatDeviceHashesEqual,
3233
seatDeviceIdentity,
3334
seatDevicePreviousAccessExpiresAt,
@@ -249,6 +250,26 @@ describe("seat device request validation", () => {
249250
expect(parseSeatDeviceRevokeRequest({ refreshToken, reason: "device_logout", extra: 1 })).toEqual({ ok: false, code: "invalid_input" });
250251
});
251252

253+
it("builds entry links from the given origin only; activation follows its own switch; no sign-up link", () => {
254+
const origin = "https://seat.example.test";
255+
expect(seatDeviceEntryLinks(origin, {})).toEqual({ signIn: `${origin}/login`, activateMember: null, signUp: null });
256+
expect(seatDeviceEntryLinks(origin, { HELM_AUTH_MEMBER_ACTIVATION_ENABLED: "true" })).toEqual({
257+
signIn: `${origin}/login`,
258+
activateMember: `${origin}/activate-member`,
259+
signUp: null,
260+
});
261+
// Only the exact "true" opens the activation page (it answers 404 otherwise).
262+
for (const value of ["false", "TRUE", "1", "", undefined]) {
263+
expect(seatDeviceEntryLinks(origin, { HELM_AUTH_MEMBER_ACTIVATION_ENABLED: value }).activateMember).toBeNull();
264+
}
265+
// The origin passed in is the only source: APP_URL itself is not re-read here.
266+
expect(seatDeviceEntryLinks(origin, { ...on, APP_URL: "https://other.example.test" })).toEqual({
267+
signIn: `${origin}/login`,
268+
activateMember: null,
269+
signUp: null,
270+
});
271+
});
272+
252273
it("normalizes pairing codes in lookup and decision bodies", () => {
253274
expect(parseSeatDeviceRequest(SEAT_DEVICE_LOOKUP_REQUEST_SCHEMA, { pairingCode: "bcdf-ghjk" })).toEqual({ ok: true, value: { pairingCode: "BCDFGHJK" } });
254275
expect(parseSeatDeviceRequest(SEAT_DEVICE_LOOKUP_REQUEST_SCHEMA, { pairingCode: "BCDF-GHJA" }).ok).toBe(false);

‎lib/seat-device/contract.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ export type SeatDeviceClientType = (typeof SEAT_DEVICE_CLIENT_TYPES)[number];
2323
export const SEAT_DEVICE_PUBLIC_AUTH_BASE_PATH = "/api/public-auth/seat-device";
2424
// The verification URL never carries the pairing code: the member types it.
2525
export const SEAT_DEVICE_VERIFICATION_PATH = "/device";
26+
// Existing Core pages the client may point a member to (server design §8).
27+
export const SEAT_DEVICE_SIGN_IN_PATH = "/login";
28+
export const SEAT_DEVICE_ACTIVATE_MEMBER_PATH = "/activate-member";
2629

2730
export const SEAT_DEVICE_REQUEST_TTL_SECONDS = 600;
2831
export const SEAT_DEVICE_POLL_INTERVAL_SECONDS = 5;
@@ -242,6 +245,28 @@ export function isSeatDeviceWorkspaceEnabled(
242245
}
243246
}
244247

248+
export type SeatDeviceEntryLinks = {
249+
signIn: string;
250+
activateMember: string | null;
251+
signUp: string | null;
252+
};
253+
254+
// Entry links in the authorization answer (server design §8.3), for the
255+
// client's guidance text. `origin` is the already resolved APP_URL origin
256+
// (https outside local development), never anything from the request.
257+
// activateMember is null while its page answers 404. signUp stays null:
258+
// Core has no sign-up page and no sign-up address is configured.
259+
export function seatDeviceEntryLinks(
260+
origin: string,
261+
env: Readonly<Record<string, string | undefined>> = process.env,
262+
): SeatDeviceEntryLinks {
263+
return {
264+
signIn: `${origin}${SEAT_DEVICE_SIGN_IN_PATH}`,
265+
activateMember: env.HELM_AUTH_MEMBER_ACTIVATION_ENABLED === "true" ? `${origin}${SEAT_DEVICE_ACTIVATE_MEMBER_PATH}` : null,
266+
signUp: null,
267+
};
268+
}
269+
245270
// Number of proxies in front of the app that append to x-forwarded-for. Any
246271
// value other than a small non-negative integer means 0: source untrusted.
247272
export function readSeatDeviceTrustedProxyHops(

‎lib/seat-device/device-routes.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
parseSeatDeviceRevokeRequest,
3030
readSeatDeviceTrustedProxyHops,
3131
resolveSeatDeviceTrustedSource,
32+
seatDeviceEntryLinks,
3233
type SeatDeviceErrorCode,
3334
} from "@/lib/seat-device/contract";
3435
import {
@@ -118,7 +119,7 @@ const OPERATIONS: Record<SeatDeviceOperation, (request: Request) => Promise<Resp
118119
const input = rejectOnParse(parseSeatDeviceAuthorizationRequest(await readJson(request)));
119120
const grant = await requestSeatDeviceAuthorization({ ...input, source: trustedSource(request) });
120121
// The member types the pairing code; the URL never carries it.
121-
return ok({ ...grant, verificationUrl: `${origin}${SEAT_DEVICE_VERIFICATION_PATH}` });
122+
return ok({ ...grant, verificationUrl: `${origin}${SEAT_DEVICE_VERIFICATION_PATH}`, entryLinks: seatDeviceEntryLinks(origin) });
122123
},
123124
async cancel(request) {
124125
const input = rejectOnParse(parseSeatDeviceRequest(SEAT_DEVICE_POLL_REQUEST_SCHEMA, await readJson(request)));

‎lib/seat-device/seat-device-routes.mysql.test.ts‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// The five D1 device endpoints (design v0.3 §4.2, §15) through their real
33
// route handlers and the real store on an isolated MySQL database: answer
44
// shapes, the code map with its HTTP statuses, no-store on every answer,
5-
// Retry-After on every 429 and 503, the verification URL (APP_URL only, no
5+
// Retry-After on every 429 and 503, the verification URL and entry links (APP_URL only, no
66
// code), and a log probe that no token or pairing code reaches a log line.
77
//
88
// SEAT_DEVICE_DATABASE_URL=<disposable db> DATABASE_URL=<same>
@@ -160,7 +160,7 @@ describeMysql("seat device endpoints on MySQL", () => {
160160
async function authorize(source = freshSource()) {
161161
const answer = await post("authorize", DEVICE, source);
162162
expect(answer.status).toBe(200);
163-
return { grant: answer.body as Grant & { verificationUrl: string }, source };
163+
return { grant: answer.body as Grant & { verificationUrl: string; entryLinks: unknown }, source };
164164
}
165165

166166
// A claimed device of a fresh member, from a trusted source.
@@ -185,6 +185,8 @@ describeMysql("seat device endpoints on MySQL", () => {
185185
vi.stubEnv("HELM_SEAT_DEVICE_AUTH_ENABLED", "true");
186186
vi.stubEnv("HELM_SEAT_DEVICE_TRUSTED_PROXY_HOPS", "1");
187187
vi.stubEnv("APP_URL", APP_ORIGIN);
188+
// The activation page (and so its entry link) is off unless a test opens it.
189+
vi.stubEnv("HELM_AUTH_MEMBER_ACTIVATION_ENABLED", undefined);
188190
for (const method of CONSOLE_METHODS) {
189191
const original = console[method].bind(console);
190192
vi.spyOn(console, method).mockImplementation((...args: unknown[]) => {
@@ -209,9 +211,11 @@ describeMysql("seat device endpoints on MySQL", () => {
209211
it("authorizes with a URL built from APP_URL only, never carrying the code, and keeps the trusted source", async () => {
210212
nextDay();
211213
const { grant, source } = await authorize();
212-
expect(Object.keys(grant).sort()).toEqual(["expiresInSeconds", "intervalSeconds", "pairingCode", "pollToken", "requestId", "verificationUrl"]);
214+
expect(Object.keys(grant).sort()).toEqual(["entryLinks", "expiresInSeconds", "intervalSeconds", "pairingCode", "pollToken", "requestId", "verificationUrl"]);
213215
expect(grant).toMatchObject({ expiresInSeconds: 600, intervalSeconds: 5 });
214216
expect(grant.verificationUrl).toBe(`${APP_ORIGIN}/device`);
217+
// post() sends a hostile Host, x-forwarded-host and an http forwarded proto.
218+
expect(grant.entryLinks).toEqual({ signIn: `${APP_ORIGIN}/login`, activateMember: null, signUp: null });
215219
const url = new URL(grant.verificationUrl);
216220
expect(url.search + url.hash).toBe("");
217221
expect(grant.verificationUrl).not.toContain(grant.pairingCode.replace("-", ""));
@@ -222,13 +226,29 @@ describeMysql("seat device endpoints on MySQL", () => {
222226
expect(row.pollTokenHash).toBe(hashSeatDeviceToken(grant.pollToken));
223227
});
224228

229+
it("offers the activation link only while the activation page is on, and takes no address from the client", async () => {
230+
nextDay();
231+
vi.stubEnv("HELM_AUTH_MEMBER_ACTIVATION_ENABLED", "true");
232+
try {
233+
const { grant } = await authorize();
234+
expect(grant.entryLinks).toEqual({ signIn: `${APP_ORIGIN}/login`, activateMember: `${APP_ORIGIN}/activate-member`, signUp: null });
235+
} finally {
236+
vi.stubEnv("HELM_AUTH_MEMBER_ACTIVATION_ENABLED", undefined);
237+
}
238+
// A client cannot steer the links: any address field is an unknown field.
239+
for (const extra of [{ entryLinks: { signUp: "https://phish.example" } }, { signUpUrl: "https://phish.example" }, { verificationUrl: "https://phish.example/device" }]) {
240+
expect(only(await post("authorize", { ...DEVICE, ...extra }, freshSource()))).toEqual({ status: 400, body: errorOf("invalid_input") });
241+
}
242+
});
243+
225244
it("resolves the public origin before storing anything", async () => {
226245
nextDay();
227246
const before = await db.seatDeviceAuthorization.count();
228247
vi.stubEnv("NODE_ENV", "production");
229248
vi.stubEnv("APP_URL", "http://plain-http.example.test");
230249
try {
231-
expect(await post("authorize", DEVICE, freshSource())).toMatchObject({ status: 503, body: errorOf("temporarily_unavailable") });
250+
// Fail closed: no link of any kind leaves with a non-https origin.
251+
expect(only(await post("authorize", DEVICE, freshSource()))).toEqual({ status: 503, body: errorOf("temporarily_unavailable") });
232252
} finally {
233253
vi.stubEnv("NODE_ENV", "test");
234254
vi.stubEnv("APP_URL", APP_ORIGIN);

0 commit comments

Comments
 (0)