From 1195416db810ae48b6483cac4aeb4392bcf1a665 Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Tue, 20 Oct 2026 12:32:06 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Stabilize=20device=20login=20and?= =?UTF-8?q?=20poll=20terminal=20authorization?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/auth/client.js | 5 +- src/auth/core.js | 93 ++++- src/auth/operations.js | 44 ++- src/commands/login.js | 253 +++++-------- .../src/components/views/builds-view.jsx | 105 ++---- src/reporter/src/hooks/use-device-login.js | 80 ++++ src/server/routers/auth.js | 76 ++-- src/utils/browser.js | 4 +- src/utils/login-poll-wait.js | 52 +++ tests/auth/core.test.js | 82 ++++ tests/commands/login-integration.test.js | 349 ++++++++++++++++++ tests/commands/login.test.js | 330 ----------------- tests/reporter/specs/builds-workflow.spec.js | 279 ++++++++++++++ tests/server/routers/auth-integration.test.js | 197 ++++++++++ tests/server/routers/auth.test.js | 18 +- tests/utils/login-poll-wait.test.js | 130 +++++++ 16 files changed, 1452 insertions(+), 645 deletions(-) create mode 100644 src/reporter/src/hooks/use-device-login.js create mode 100644 src/utils/login-poll-wait.js create mode 100644 tests/commands/login-integration.test.js delete mode 100644 tests/commands/login.test.js create mode 100644 tests/server/routers/auth-integration.test.js create mode 100644 tests/utils/login-poll-wait.test.js diff --git a/src/auth/client.js b/src/auth/client.js index f68dae23..362f9d9b 100644 --- a/src/auth/client.js +++ b/src/auth/client.js @@ -61,7 +61,10 @@ export function createAuthClient(options = {}) { if (!response.ok) { let body = await parseResponseBody(response); - throw parseAuthError(response.status, body, endpoint); + let error = parseAuthError(response.status, body, endpoint); + let retryAfter = Number(response.headers?.get('retry-after')); + if (retryAfter > 0) error.context.retryAfter = retryAfter; + throw error; } return response.json(); diff --git a/src/auth/core.js b/src/auth/core.js index 2b1ee9e1..f6f92134 100644 --- a/src/auth/core.js +++ b/src/auth/core.js @@ -68,23 +68,33 @@ export function parseAuthError(status, body, _endpoint) { errorText = body; } + let retryAfter = Number(body?.retryAfter); + let context = { + status, + serverCode: body?.code, + retryAfter: retryAfter > 0 ? retryAfter : undefined, + }; + if (status === 401) { return new AuthError( errorText || - 'Invalid credentials. Please check your email/username and password.' + 'Invalid credentials. Please check your email/username and password.', + context ); } if (status === 429) { return new VizzlyError( 'Too many login attempts. Please try again later.', - 'RATE_LIMIT_ERROR' + 'RATE_LIMIT_ERROR', + context ); } return new VizzlyError( `Authentication request failed: ${status}${errorText ? ` - ${errorText}` : ''}`, - 'AUTH_REQUEST_ERROR' + 'AUTH_REQUEST_ERROR', + context ); } @@ -184,3 +194,80 @@ export function validateTokens(auth, requiredField = 'accessToken') { return { valid: true, error: null }; } + +export function normalizeDeviceFlowResponse(deviceFlow) { + let verificationUri = + deviceFlow.verification_uri || deviceFlow.verificationUri; + let userCode = deviceFlow.user_code || deviceFlow.userCode; + let deviceCode = deviceFlow.device_code || deviceFlow.deviceCode; + + if (!verificationUri || !userCode || !deviceCode) { + throw new Error('Invalid device flow response from server'); + } + + return { + verificationUri, + userCode, + deviceCode, + urlWithCode: + deviceFlow.verification_uri_complete || + deviceFlow.verificationUriComplete || + `${verificationUri}?code=${encodeURIComponent(userCode)}`, + }; +} + +export function resolveAuthorizedTokenData(pollResponse) { + let accessToken = + pollResponse.tokens?.accessToken || pollResponse.tokens?.access_token; + + if (accessToken) { + return pollResponse; + } + + if (pollResponse.status === 'pending') { + throw new Error( + 'Authorization not complete yet. Please complete the authorization in your browser and try running "vizzly login" again.' + ); + } + if (pollResponse.status === 'expired') { + throw new Error('Device code expired. Please try logging in again.'); + } + if (pollResponse.status === 'invalid') { + throw new Error( + 'Device code is invalid or expired. Please try logging in again.' + ); + } + if (pollResponse.status === 'denied') { + throw new Error('Authorization denied. Please try logging in again.'); + } + + throw new Error( + 'Unexpected response from authorization server. Please try logging in again.' + ); +} + +export function buildTokens(tokenData, now = Date.now()) { + let tokensData = tokenData.tokens || tokenData; + let tokenExpiresIn = tokensData.expiresIn || tokensData.expires_in; + let tokenExpiresAt = tokenExpiresIn + ? new Date(now + tokenExpiresIn * 1000).toISOString() + : tokenData.expires_at || tokenData.expiresAt; + + return { + accessToken: tokensData.accessToken || tokensData.access_token, + refreshToken: tokensData.refreshToken || tokensData.refresh_token, + expiresAt: tokenExpiresAt, + user: tokenData.user, + organizations: tokenData.organizations, + }; +} + +/** Map protocol codes without depending on the server's human-readable text. */ +export function deviceAuthorizationStatus(error) { + return { + AUTHORIZATION_PENDING: 'pending', + ACCESS_DENIED: 'denied', + EXPIRED_TOKEN: 'expired', + INVALID_DEVICE_CODE: 'invalid', + }[error.context?.serverCode]; +} diff --git a/src/auth/operations.js b/src/auth/operations.js index 81290e86..5fbd58c4 100644 --- a/src/auth/operations.js +++ b/src/auth/operations.js @@ -8,11 +8,13 @@ * This makes them trivially testable without mocking modules. */ +import * as output from '../utils/output.js'; import { buildDevicePollPayload, buildLogoutPayload, buildRefreshPayload, buildTokenData, + deviceAuthorizationStatus, validateTokens, } from './core.js'; @@ -38,12 +40,23 @@ export async function initiateDeviceFlow(httpClient) { * @param {string} deviceCode - Device code from initiate * @returns {Promise} Token data or pending status */ -export async function pollDeviceAuthorization(httpClient, deviceCode) { - return httpClient.request('/api/auth/cli/device/poll', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(buildDevicePollPayload(deviceCode)), - }); +export async function pollDeviceAuthorization( + httpClient, + deviceCode, + options = {} +) { + try { + return await httpClient.request('/api/auth/cli/device/poll', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(buildDevicePollPayload(deviceCode)), + signal: options.signal, + }); + } catch (error) { + let status = deviceAuthorizationStatus(error); + if (status) return { status }; + throw error; + } } /** @@ -102,17 +115,18 @@ export async function logout(httpClient, tokenStore) { if (auth?.refreshToken) { try { - await httpClient.request('/api/auth/cli/logout', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(buildLogoutPayload(auth.refreshToken)), - }); + await httpClient.authenticatedRequest( + '/api/auth/cli/logout', + auth.accessToken, + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(buildLogoutPayload(auth.refreshToken)), + } + ); } catch (error) { // If server request fails, still clear local tokens - console.warn( - 'Warning: Failed to revoke tokens on server:', - error.message - ); + output.warn(`Failed to revoke tokens on server: ${error.message}`); } } diff --git a/src/commands/login.js b/src/commands/login.js index c352b5be..0f01099b 100644 --- a/src/commands/login.js +++ b/src/commands/login.js @@ -3,6 +3,13 @@ * Authenticates user via OAuth device flow */ +import { setTimeout as delay } from 'node:timers/promises'; + +import { + buildTokens, + normalizeDeviceFlowResponse, + resolveAuthorizedTokenData, +} from '../auth/core.js'; import { completeDeviceFlow, createAuthClient, @@ -10,135 +17,12 @@ import { initiateDeviceFlow, pollDeviceAuthorization, } from '../auth/index.js'; + import { openBrowser } from '../utils/browser.js'; import { getApiUrl } from '../utils/environment-config.js'; +import { waitForLoginPoll } from '../utils/login-poll-wait.js'; import * as output from '../utils/output.js'; -function createLoginDeps(deps = {}) { - return { - completeDeviceFlow: deps.completeDeviceFlow || completeDeviceFlow, - createAuthClient: deps.createAuthClient || createAuthClient, - createTokenStore: deps.createTokenStore || createTokenStore, - getApiUrl: deps.getApiUrl || getApiUrl, - initiateDeviceFlow: deps.initiateDeviceFlow || initiateDeviceFlow, - now: deps.now || (() => Date.now()), - openBrowser: deps.openBrowser || openBrowser, - output: deps.output || output, - pollDeviceAuthorization: - deps.pollDeviceAuthorization || pollDeviceAuthorization, - waitForEnter: deps.waitForEnter || waitForEnter, - exit: deps.exit || (code => process.exit(code)), - }; -} - -function configureOutput(output, globalOptions) { - output.configure({ - json: globalOptions.json, - verbose: globalOptions.verbose, - color: !globalOptions.noColor, - }); -} - -export function normalizeDeviceFlowResponse(deviceFlow) { - let verificationUri = - deviceFlow.verification_uri || deviceFlow.verificationUri; - let userCode = deviceFlow.user_code || deviceFlow.userCode; - let deviceCode = deviceFlow.device_code || deviceFlow.deviceCode; - - if (!verificationUri || !userCode || !deviceCode) { - throw new Error('Invalid device flow response from server'); - } - - return { - verificationUri, - userCode, - deviceCode, - urlWithCode: `${verificationUri}?code=${encodeURIComponent(userCode)}`, - }; -} - -export function resolveAuthorizedTokenData(pollResponse) { - let accessToken = - pollResponse.tokens?.accessToken || pollResponse.tokens?.access_token; - - if (accessToken) { - return pollResponse; - } - - if (pollResponse.status === 'pending') { - throw new Error( - 'Authorization not complete yet. Please complete the authorization in your browser and try running "vizzly login" again.' - ); - } - if (pollResponse.status === 'expired') { - throw new Error('Device code expired. Please try logging in again.'); - } - if (pollResponse.status === 'denied') { - throw new Error('Authorization denied. Please try logging in again.'); - } - - throw new Error( - 'Unexpected response from authorization server. Please try logging in again.' - ); -} - -export function buildTokens(tokenData, now = Date.now()) { - let tokensData = tokenData.tokens || tokenData; - let tokenExpiresIn = tokensData.expiresIn || tokensData.expires_in; - let tokenExpiresAt = tokenExpiresIn - ? new Date(now + tokenExpiresIn * 1000).toISOString() - : tokenData.expires_at || tokenData.expiresAt; - - return { - accessToken: tokensData.accessToken || tokensData.access_token, - refreshToken: tokensData.refreshToken || tokensData.refresh_token, - expiresAt: tokenExpiresAt, - user: tokenData.user, - organizations: tokenData.organizations, - }; -} - -export function getTokenExpiryHint(expiresAt, now = Date.now()) { - if (!expiresAt) { - return null; - } - - let expiry = new Date(expiresAt); - let msUntilExpiry = expiry.getTime() - now; - let daysUntilExpiry = Math.floor(msUntilExpiry / (1000 * 60 * 60 * 24)); - let hoursUntilExpiry = Math.floor(msUntilExpiry / (1000 * 60 * 60)); - let minutesUntilExpiry = Math.floor(msUntilExpiry / (1000 * 60)); - - if (daysUntilExpiry > 0) { - return `Token expires in ${daysUntilExpiry} day${daysUntilExpiry !== 1 ? 's' : ''}`; - } - if (hoursUntilExpiry > 0) { - return `Token expires in ${hoursUntilExpiry} hour${hoursUntilExpiry !== 1 ? 's' : ''}`; - } - if (minutesUntilExpiry > 0) { - return `Token expires in ${minutesUntilExpiry} minute${minutesUntilExpiry !== 1 ? 's' : ''}`; - } - - return null; -} - -export async function waitForEnter(stdin = process.stdin) { - await new Promise(resolve => { - let canSetRawMode = typeof stdin.setRawMode === 'function'; - if (canSetRawMode) { - stdin.setRawMode(true); - } - stdin.resume(); - stdin.once('data', () => { - if (canSetRawMode) { - stdin.setRawMode(false); - } - stdin.pause(); - resolve(); - }); - }); -} - /** * Login command implementation using OAuth device flow * @param {Object} options - Command options @@ -149,21 +33,13 @@ export async function loginCommand( globalOptions = {}, deps = {} ) { - let { - completeDeviceFlow, - createAuthClient, - createTokenStore, - getApiUrl, - initiateDeviceFlow, - now, - openBrowser, - output, - pollDeviceAuthorization, - waitForEnter, - exit, - } = createLoginDeps(deps); + let { now = Date.now, delay: sleep = delay } = deps; - configureOutput(output, globalOptions); + output.configure({ + json: globalOptions.json, + verbose: globalOptions.verbose, + color: !globalOptions.noColor, + }); let colors = output.getColors(); @@ -180,6 +56,19 @@ export async function loginCommand( output.startSpinner('Connecting to Vizzly...'); let deviceFlow = await initiateDeviceFlow(client); output.stopSpinner(); + let expiresIn = Number(deviceFlow.expires_in ?? deviceFlow.expiresIn); + let lifetime = + Number.isFinite(expiresIn) && expiresIn > 0 + ? Math.min(expiresIn, 120) + : 120; + let deadline = now() + lifetime * 1000; + let timeoutMessage = + expiresIn > 0 && expiresIn <= 120 + ? 'Device code expired. Please try logging in again.' + : 'Login timed out after 2 minutes. Please try logging in again.'; + let interval = Number(deviceFlow.interval); + let pollDelay = + Number.isFinite(interval) && interval > 0 ? interval * 1000 : 5000; // Handle both snake_case and camelCase field names let { deviceCode, urlWithCode, userCode } = @@ -210,16 +99,61 @@ export async function loginCommand( } output.blank(); - output.hint('After authorizing, press Enter to continue...'); - - // Wait for user to press Enter - await waitForEnter(); - - // Check authorization status - output.startSpinner('Checking authorization...'); - - let pollResponse = await pollDeviceAuthorization(client, deviceCode); - + output.hint( + 'Waiting for authorization in your browser (up to 2 minutes)...' + ); + let interactive = process.stdin.isTTY && !globalOptions.json; + let backoff = false; + let lastCheckAt = 0; + + let pollResponse; + while (true) { + let remaining = deadline - now(); + if (remaining <= 0) { + throw new Error(timeoutMessage); + } + await waitForLoginPoll({ + milliseconds: Math.min(pollDelay, remaining), + manualAllowed: !backoff, + earliestCheckAt: lastCheckAt + 1000, + stdin: interactive ? process.stdin : { isTTY: false }, + now, + sleep, + onTick: (milliseconds, canCheck) => { + let seconds = Math.ceil(milliseconds / 1000); + let message = backoff + ? `Rate limited. Next check in ${seconds}s` + : `Next check in ${seconds}s`; + if (canCheck) message += ' — press Enter to check now'; + if (interactive) output.updateSpinner(message); + else output.hint(message); + }, + }); + remaining = deadline - now(); + if (remaining <= 0) { + throw new Error(timeoutMessage); + } + lastCheckAt = now(); + output.updateSpinner('Checking authorization...'); + try { + pollResponse = await pollDeviceAuthorization(client, deviceCode, { + signal: AbortSignal.timeout(Math.ceil(Math.min(remaining, 30000))), + }); + } catch (error) { + if (now() >= deadline) { + throw new Error(timeoutMessage); + } + if (error.code !== 'RATE_LIMIT_ERROR') throw error; + backoff = true; + let retryAfter = Number(error.context?.retryAfter); + if (Number.isFinite(retryAfter) && retryAfter > 0) { + pollDelay = Math.max(pollDelay, retryAfter * 1000); + } + continue; + } + backoff = false; + if (pollResponse.status !== 'pending') break; + } output.stopSpinner(); let tokenData = resolveAuthorizedTokenData(pollResponse); @@ -251,19 +185,8 @@ export async function loginCommand( output.list(orgItems); } - // Show token expiry info - if (tokens.expiresAt) { - output.blank(); - let expiryHint = getTokenExpiryHint(tokens.expiresAt, now()); - if (expiryHint) { - output.hint(expiryHint); - } - } - output.blank(); - output.hint( - 'Run "vizzly project link /" to enable cloud uploads' - ); + output.hint('You can now browse cloud builds and download baselines'); output.cleanup(); } catch (error) { @@ -277,18 +200,14 @@ export async function loginCommand( output.hint( "If you don't have an account, sign up at https://vizzly.dev" ); - output.cleanup(); - exit(1); } else if (error.code === 'RATE_LIMIT_ERROR') { output.error('Too many login attempts', error); output.blank(); output.hint('Please wait a few minutes before trying again'); - output.cleanup(); - exit(1); } else { output.error('Login failed', error); - output.cleanup(); - exit(1); } + output.cleanup(); + process.exit(1); } } diff --git a/src/reporter/src/components/views/builds-view.jsx b/src/reporter/src/components/views/builds-view.jsx index a9ded7ab..5386c0e9 100644 --- a/src/reporter/src/components/views/builds-view.jsx +++ b/src/reporter/src/components/views/builds-view.jsx @@ -23,17 +23,14 @@ import { SkeletonCard, Spinner, } from '@vizzly-testing/bear-den'; -import { useCallback, useEffect, useMemo, useState } from 'react'; -import { - useAuthStatus, - useInitiateLogin, - usePollAuthorization, -} from '../../hooks/queries/use-auth-queries.js'; +import { useCallback, useMemo, useState } from 'react'; +import { useAuthStatus } from '../../hooks/queries/use-auth-queries.js'; import { useBuilds, useDownloadBaselines, useProjects, } from '../../hooks/queries/use-cloud-queries.js'; +import { useDeviceLogin } from '../../hooks/use-device-login.js'; import { queryKeys } from '../../lib/query-keys.js'; import { useToast } from '../ui/toast.jsx'; @@ -231,56 +228,27 @@ function OrganizationSection({ ); } -function DeviceFlowLogin({ onComplete }) { - let [deviceFlow, setDeviceFlow] = useState(null); - let [error, setError] = useState(null); - - let initiateLoginMutation = useInitiateLogin(); - let pollMutation = usePollAuthorization(); - let { addToast } = useToast(); - - useEffect(() => { - async function startDeviceFlow() { - try { - let flow = await initiateLoginMutation.mutateAsync(); - setDeviceFlow(flow); - } catch (err) { - setError(err.message); - addToast(`Failed to start login: ${err.message}`, 'error'); - } - } - - startDeviceFlow(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [addToast, initiateLoginMutation]); - - async function checkAuthorization() { - if (!deviceFlow?.deviceCode) return; - - setError(null); - - try { - let result = await pollMutation.mutateAsync(deviceFlow.deviceCode); - - if (result.status === 'complete') { - addToast('Login successful!', 'success'); - onComplete?.(); - } else if (result.status === 'pending') { - addToast('Still waiting for authorization...', 'info'); - } else { - setError('Unexpected response from server'); - } - } catch (err) { - setError(err.message); - addToast(`Check failed: ${err.message}`, 'error'); - } - } - - if (error) { +function DeviceFlowLogin({ login }) { + let deviceFlow = login.flow; + let terminalMessage = { + denied: 'Authorization was denied. Start a new login to try again.', + expired: 'Your login code expired. Start a new login to continue.', + invalid: + 'This login code is invalid or expired. Start a new login to continue.', + }[login.status]; + + if (login.status === 'error' || terminalMessage) { + let retryAfter = login.error?.data?.retryAfter; return ( - - {error} - +
+ + {terminalMessage || login.error.message} + {retryAfter > 0 && ` Try again after ${retryAfter} seconds.`} + + +
); } @@ -329,10 +297,17 @@ function DeviceFlowLogin({ onComplete }) { sign in.

+ {login.error && ( + + {login.error.message} + {login.error.data?.retryAfter > 0 && + ` Try again after ${login.error.data.retryAfter} seconds.`} + + )} @@ -342,28 +317,24 @@ function DeviceFlowLogin({ onComplete }) { } function LoginPrompt() { - const [showingLogin, setShowingLogin] = useState(false); const queryClient = useQueryClient(); const { addToast } = useToast(); const handleLoginComplete = useCallback(() => { - setShowingLogin(false); queryClient.invalidateQueries({ queryKey: queryKeys.auth }); queryClient.invalidateQueries({ queryKey: queryKeys.projects() }); addToast('Login successful!', 'success'); }, [queryClient, addToast]); - if (showingLogin) { + let login = useDeviceLogin({ onComplete: handleLoginComplete }); + + if (login.status !== 'idle') { return (
- - +
); } @@ -383,7 +354,7 @@ function LoginPrompt() {