Skip to content

Commit b1c4b80

Browse files
miguel-heygenmiguel-heygen
committed
refactor(auth): simplify device authorization flow
1 parent 49bdaeb commit b1c4b80

3 files changed

Lines changed: 124 additions & 95 deletions

File tree

‎packages/cli/src/auth/index.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ export type { UserInfo } from "./client.js";
3333

3434
export {
3535
assertOAuthConfiguredOrExit,
36-
persistFreshOAuth,
3736
persistVerifiedOAuthSession,
3837
refreshTokens,
3938
revokeTokens,

‎packages/cli/src/auth/oauth.test.ts‎

Lines changed: 19 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,19 @@ vi.mock("./browser.js", () => ({
2929
openBrowser: vi.fn(async () => ({ opened: true })),
3030
}));
3131

32+
function tokenFetch(body: Record<string, unknown>): typeof fetch {
33+
return (async () =>
34+
new Response(JSON.stringify(body), {
35+
status: 200,
36+
headers: { "content-type": "application/json" },
37+
})) as unknown as typeof fetch;
38+
}
39+
40+
async function loginWithTokens(body: Record<string, unknown>) {
41+
await startAuthorizationCodeFlow({ fetchImpl: tokenFetch(body) });
42+
return (await readStore()).credentials;
43+
}
44+
3245
describe("auth/oauth", () => {
3346
let fixture: Awaited<ReturnType<typeof setupTempAuthEnv>>;
3447

@@ -167,11 +180,7 @@ describe("auth/oauth", () => {
167180
expires_at: "2026-01-01T00:00:00Z",
168181
},
169182
});
170-
const fetchImpl = (async () =>
171-
new Response(JSON.stringify({ access_token: "new_at", expires_in: 3600 }), {
172-
status: 200,
173-
headers: { "content-type": "application/json" },
174-
})) as unknown as typeof fetch;
183+
const fetchImpl = tokenFetch({ access_token: "new_at", expires_in: 3600 });
175184
await refreshTokens("keep_me_rt", { fetchImpl });
176185
const { credentials } = await readStore();
177186
expect(credentials.oauth?.access_token).toBe("new_at");
@@ -181,11 +190,7 @@ describe("auth/oauth", () => {
181190

182191
it("preserves an existing api_key when persisting refreshed oauth", async () => {
183192
await writeStore({ api_key: "hg_keep" });
184-
const fetchImpl = (async () =>
185-
new Response(JSON.stringify({ access_token: "new_at", expires_in: 60 }), {
186-
status: 200,
187-
headers: { "content-type": "application/json" },
188-
})) as unknown as typeof fetch;
193+
const fetchImpl = tokenFetch({ access_token: "new_at", expires_in: 60 });
189194
await refreshTokens("old_rt", { fetchImpl });
190195
const { credentials } = await readStore();
191196
expect(credentials.api_key).toBe("hg_keep");
@@ -211,11 +216,7 @@ describe("auth/oauth", () => {
211216
}),
212217
{ mode: 0o600 },
213218
);
214-
const fetchImpl = (async () =>
215-
new Response(JSON.stringify({ access_token: "new_at", expires_in: 3600 }), {
216-
status: 200,
217-
headers: { "content-type": "application/json" },
218-
})) as unknown as typeof fetch;
219+
const fetchImpl = tokenFetch({ access_token: "new_at", expires_in: 3600 });
219220
await refreshTokens("keep_me_rt", { fetchImpl });
220221

221222
const onDisk = JSON.parse(await fs.readFile(path, "utf8"));
@@ -310,14 +311,6 @@ describe("auth/oauth", () => {
310311
});
311312

312313
describe("startAuthorizationCodeFlow persistence", () => {
313-
function tokenFetch(body: Record<string, unknown>): typeof fetch {
314-
return (async () =>
315-
new Response(JSON.stringify(body), {
316-
status: 200,
317-
headers: { "content-type": "application/json" },
318-
})) as unknown as typeof fetch;
319-
}
320-
321314
it("overwrites the OAuth block on fresh login (no inherited refresh_token)", async () => {
322315
// Pre-seed a prior session whose refresh_token must NOT leak into
323316
// the new login when the new response omits one.
@@ -327,27 +320,21 @@ describe("auth/oauth", () => {
327320
refresh_token: "OLD_rt_should_not_survive",
328321
},
329322
});
330-
const fetchImpl = tokenFetch({
323+
const credentials = await loginWithTokens({
331324
access_token: "new_at",
332325
expires_in: 3600,
333326
});
334-
await startAuthorizationCodeFlow({ fetchImpl });
335-
336-
const { credentials } = await readStore();
337327
expect(credentials.oauth?.access_token).toBe("new_at");
338328
// Fresh login is a clean session — the old refresh_token is gone.
339329
expect(credentials.oauth?.refresh_token).toBeUndefined();
340330
});
341331

342332
it("preserves a co-located api_key across fresh login", async () => {
343333
await writeStore({ api_key: "hg_keep_me" });
344-
const fetchImpl = tokenFetch({
334+
const credentials = await loginWithTokens({
345335
access_token: "new_at",
346336
refresh_token: "new_rt",
347337
});
348-
await startAuthorizationCodeFlow({ fetchImpl });
349-
350-
const { credentials } = await readStore();
351338
expect(credentials.api_key).toBe("hg_keep_me");
352339
expect(credentials.oauth?.access_token).toBe("new_at");
353340
expect(credentials.oauth?.refresh_token).toBe("new_rt");
@@ -367,13 +354,10 @@ describe("auth/oauth", () => {
367354
}),
368355
{ mode: 0o600 },
369356
);
370-
const fetchImpl = tokenFetch({
357+
const credentials = await loginWithTokens({
371358
access_token: "new_at",
372359
expires_in: 3600,
373360
});
374-
await startAuthorizationCodeFlow({ fetchImpl });
375-
376-
const { credentials } = await readStore();
377361
expect(credentials.oauth?.access_token).toBe("new_at");
378362
expect(credentials.user).toEqual({
379363
email: "[email protected]",

‎packages/cli/src/auth/oauth.ts‎

Lines changed: 105 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -214,80 +214,115 @@ export async function startAuthorizationCodeFlow(
214214
export async function startDeviceAuthorizationFlow(
215215
opts: DeviceAuthorizationFlowOptions = {},
216216
): Promise<OAuthTokens> {
217-
const clientId = resolveClientId();
218-
const fetchImpl = opts.fetchImpl ?? fetch;
219-
const sleepImpl =
220-
opts.sleepImpl ?? ((ms: number) => new Promise((resolve) => setTimeout(resolve, ms)));
221-
const now = opts.now ?? Date.now;
222-
const scope = opts.scope ?? DEFAULT_SCOPES;
217+
const runtime: DeviceFlowRuntime = {
218+
clientId: resolveClientId(),
219+
fetchImpl: opts.fetchImpl ?? fetch,
220+
sleepImpl:
221+
opts.sleepImpl ?? ((ms: number) => new Promise((resolve) => setTimeout(resolve, ms))),
222+
now: opts.now ?? Date.now,
223+
};
224+
const issuance = await requestDeviceAuthorization(runtime, opts.scope ?? DEFAULT_SCOPES);
225+
await opts.onChallenge?.({
226+
userCode: issuance.userCode,
227+
verificationUri: issuance.verificationUri,
228+
});
229+
return await pollDeviceToken(runtime, issuance);
230+
}
223231

224-
let issuanceResponse: Response;
232+
interface DeviceFlowRuntime {
233+
clientId: string;
234+
fetchImpl: typeof fetch;
235+
sleepImpl: (ms: number) => Promise<void>;
236+
now: () => number;
237+
}
238+
239+
async function requestDeviceAuthorization(
240+
runtime: DeviceFlowRuntime,
241+
scope: string,
242+
): Promise<ParsedDeviceAuthorization> {
243+
let response: Response;
225244
try {
226-
issuanceResponse = await fetchImpl(deviceAuthorizationEndpoint(), {
245+
response = await runtime.fetchImpl(deviceAuthorizationEndpoint(), {
227246
method: "POST",
228247
headers: {
229248
"content-type": "application/x-www-form-urlencoded",
230249
accept: "application/json",
231250
},
232-
body: new URLSearchParams({ client_id: clientId, scope }).toString(),
251+
body: new URLSearchParams({ client_id: runtime.clientId, scope }).toString(),
233252
});
234253
} catch {
235254
throw ErrDeviceAuthFailed("could not reach the authorization server");
236255
}
237-
if (!issuanceResponse.ok) {
238-
throw ErrDeviceAuthFailed(`authorization server returned HTTP ${issuanceResponse.status}`);
256+
if (!response.ok) {
257+
throw ErrDeviceAuthFailed(`authorization server returned HTTP ${response.status}`);
239258
}
259+
return parseDeviceAuthorizationResponse(await readJsonOrDeviceError(response));
260+
}
240261

241-
const issuance = parseDeviceAuthorizationResponse(await readJsonOrDeviceError(issuanceResponse));
242-
await opts.onChallenge?.({
243-
userCode: issuance.userCode,
244-
verificationUri: issuance.verificationUri,
245-
});
246-
247-
const deadline = now() + Math.min(issuance.expiresIn, MAX_DEVICE_FLOW_SECONDS) * 1000;
262+
async function pollDeviceToken(
263+
runtime: DeviceFlowRuntime,
264+
issuance: ParsedDeviceAuthorization,
265+
): Promise<OAuthTokens> {
266+
const deadline = runtime.now() + Math.min(issuance.expiresIn, MAX_DEVICE_FLOW_SECONDS) * 1000;
248267
let intervalSeconds = issuance.interval;
249-
while (now() < deadline) {
250-
const remainingMs = deadline - now();
268+
while (runtime.now() < deadline) {
269+
const remainingMs = deadline - runtime.now();
251270
if (remainingMs <= 0) break;
252-
await sleepImpl(Math.min(intervalSeconds * 1000, remainingMs));
253-
254-
let pollResponse: Response;
255-
try {
256-
pollResponse = await fetchImpl(tokenEndpoint(), {
257-
method: "POST",
258-
headers: {
259-
"content-type": "application/x-www-form-urlencoded",
260-
accept: "application/json",
261-
},
262-
body: new URLSearchParams({
263-
grant_type: DEVICE_CODE_GRANT_TYPE,
264-
device_code: issuance.deviceCode,
265-
client_id: clientId,
266-
}).toString(),
267-
});
268-
} catch {
269-
throw ErrDeviceAuthFailed("lost contact with the authorization server");
270-
}
271-
272-
if (pollResponse.ok) {
273-
return parseTokenResponse(await readJsonOrDeviceError(pollResponse));
274-
}
271+
await runtime.sleepImpl(Math.min(intervalSeconds * 1000, remainingMs));
275272

276-
const error = await readDeviceOAuthError(pollResponse);
277-
if (error === "authorization_pending") continue;
278-
if (error === "slow_down") {
273+
const result = await evaluateDevicePollResponse(
274+
await requestDeviceToken(runtime, issuance.deviceCode),
275+
);
276+
if (result.tokens) return result.tokens;
277+
if (result.slowDown) {
279278
intervalSeconds = Math.min(intervalSeconds + 5, MAX_DEVICE_POLL_SECONDS);
280-
continue;
281279
}
282-
if (error === "access_denied") {
280+
}
281+
throw ErrDeviceAuthFailed("the code expired");
282+
}
283+
284+
async function requestDeviceToken(
285+
runtime: DeviceFlowRuntime,
286+
deviceCode: string,
287+
): Promise<Response> {
288+
try {
289+
return await runtime.fetchImpl(tokenEndpoint(), {
290+
method: "POST",
291+
headers: {
292+
"content-type": "application/x-www-form-urlencoded",
293+
accept: "application/json",
294+
},
295+
body: new URLSearchParams({
296+
grant_type: DEVICE_CODE_GRANT_TYPE,
297+
device_code: deviceCode,
298+
client_id: runtime.clientId,
299+
}).toString(),
300+
});
301+
} catch {
302+
throw ErrDeviceAuthFailed("lost contact with the authorization server");
303+
}
304+
}
305+
306+
async function evaluateDevicePollResponse(
307+
response: Response,
308+
): Promise<{ tokens?: OAuthTokens; slowDown?: boolean }> {
309+
if (response.ok) {
310+
return { tokens: parseTokenResponse(await readJsonOrDeviceError(response)) };
311+
}
312+
313+
const error = await readDeviceOAuthError(response);
314+
switch (error) {
315+
case "authorization_pending":
316+
return {};
317+
case "slow_down":
318+
return { slowDown: true };
319+
case "access_denied":
283320
throw ErrDeviceAuthFailed("access was denied");
284-
}
285-
if (error === "expired_token") {
321+
case "expired_token":
286322
throw ErrDeviceAuthFailed("the code expired");
287-
}
288-
throw ErrDeviceAuthFailed(`authorization server returned HTTP ${pollResponse.status}`);
323+
default:
324+
throw ErrDeviceAuthFailed(`authorization server returned HTTP ${response.status}`);
289325
}
290-
throw ErrDeviceAuthFailed("the code expired");
291326
}
292327

293328
export async function refreshTokens(
@@ -589,22 +624,22 @@ interface ParsedDeviceAuthorization {
589624
}
590625

591626
function parseDeviceAuthorizationResponse(payload: unknown): ParsedDeviceAuthorization {
592-
if (!payload || typeof payload !== "object" || Array.isArray(payload)) {
593-
throw ErrDeviceAuthFailed("authorization server returned an invalid response");
594-
}
595-
const data = payload as Record<string, unknown>;
627+
const data = requireDeviceAuthorizationRecord(payload);
596628
const deviceCode = stringField(data, "device_code");
597629
const userCode = stringField(data, "user_code");
598630
const verificationUri = stringField(data, "verification_uri");
599631
const expiresIn = strictNumericField(data, "expires_in");
600632
const interval = strictNumericField(data, "interval");
601-
if (!deviceCode || !isHeaderSafe(deviceCode) || !userCode || !verificationUri) {
633+
if (!deviceCode || !isHeaderSafe(deviceCode)) {
634+
throw ErrDeviceAuthFailed("authorization server returned an invalid response");
635+
}
636+
if (!userCode || !verificationUri) {
602637
throw ErrDeviceAuthFailed("authorization server returned an invalid response");
603638
}
604639
if (!isSafeVerificationUri(verificationUri)) {
605640
throw ErrDeviceAuthFailed("authorization server returned an unsafe verification URL");
606641
}
607-
if (!expiresIn || expiresIn <= 0 || !interval || interval <= 0) {
642+
if (!isPositiveNumber(expiresIn) || !isPositiveNumber(interval)) {
608643
throw ErrDeviceAuthFailed("authorization server returned invalid timing values");
609644
}
610645
return {
@@ -619,6 +654,17 @@ function parseDeviceAuthorizationResponse(payload: unknown): ParsedDeviceAuthori
619654
};
620655
}
621656

657+
function requireDeviceAuthorizationRecord(payload: unknown): Record<string, unknown> {
658+
if (!payload || typeof payload !== "object" || Array.isArray(payload)) {
659+
throw ErrDeviceAuthFailed("authorization server returned an invalid response");
660+
}
661+
return payload as Record<string, unknown>;
662+
}
663+
664+
function isPositiveNumber(value: number | undefined): value is number {
665+
return value !== undefined && value > 0;
666+
}
667+
622668
function isSafeVerificationUri(value: string): boolean {
623669
try {
624670
const url = new URL(value);

0 commit comments

Comments
 (0)