-
Notifications
You must be signed in to change notification settings - Fork 6
fix: prevent workos auth login from hanging indefinitely #139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+73
−159
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,66 +11,7 @@ import type { CliConfig } from '../lib/config-store.js'; | |
| import { formatWorkOSCommand } from '../utils/command-invocation.js'; | ||
| import { autoInstallSkills } from './install-skill.js'; | ||
| import { isJsonMode } from '../utils/output.js'; | ||
|
|
||
| /** | ||
| * Parse JWT payload | ||
| */ | ||
| function parseJwt(token: string): Record<string, unknown> | null { | ||
| try { | ||
| const parts = token.split('.'); | ||
| if (parts.length !== 3) return null; | ||
| return JSON.parse(Buffer.from(parts[1], 'base64url').toString('utf-8')); | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Extract expiry time from JWT token | ||
| */ | ||
| function getJwtExpiry(token: string): number | null { | ||
| const payload = parseJwt(token); | ||
| if (!payload || typeof payload.exp !== 'number') return null; | ||
| return payload.exp * 1000; | ||
| } | ||
|
|
||
| const POLL_TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes | ||
|
|
||
| /** | ||
| * Get Connect OAuth endpoints from AuthKit domain | ||
| */ | ||
| function getConnectEndpoints() { | ||
| const domain = getAuthkitDomain(); | ||
| return { | ||
| deviceAuthorization: `${domain}/oauth2/device_authorization`, | ||
| token: `${domain}/oauth2/token`, | ||
| }; | ||
| } | ||
|
|
||
| interface DeviceAuthResponse { | ||
| device_code: string; | ||
| user_code: string; | ||
| verification_uri: string; | ||
| verification_uri_complete: string; | ||
| expires_in: number; | ||
| interval: number; | ||
| } | ||
|
|
||
| interface ConnectTokenResponse { | ||
| access_token: string; | ||
| id_token: string; | ||
| token_type: string; | ||
| expires_in: number; | ||
| refresh_token?: string; | ||
| } | ||
|
|
||
| interface AuthErrorResponse { | ||
| error: string; | ||
| } | ||
|
|
||
| function sleep(ms: number): Promise<void> { | ||
| return new Promise((resolve) => setTimeout(resolve, ms)); | ||
| } | ||
| import { requestDeviceCode, pollForToken, DeviceAuthTimeoutError } from '../lib/device-auth.js'; | ||
|
|
||
| /** | ||
| * Best-effort skill install after a successful auth-login. | ||
|
|
@@ -169,29 +110,19 @@ export async function runLogin(): Promise<void> { | |
| } | ||
| } | ||
|
|
||
| clack.log.step('Starting authentication...'); | ||
|
|
||
| const endpoints = getConnectEndpoints(); | ||
| const authkitDomain = getAuthkitDomain(); | ||
|
|
||
| const authResponse = await fetch(endpoints.deviceAuthorization, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/x-www-form-urlencoded', | ||
| }, | ||
| body: new URLSearchParams({ | ||
| client_id: clientId, | ||
| scope: 'openid email staging-environment:credentials:read offline_access', | ||
| }), | ||
| }); | ||
| clack.log.step('Starting authentication...'); | ||
|
|
||
| if (!authResponse.ok) { | ||
| clack.log.error(`Failed to start authentication: ${authResponse.status}`); | ||
| let deviceAuth; | ||
| try { | ||
| deviceAuth = await requestDeviceCode({ clientId, authkitDomain }); | ||
| } catch (error) { | ||
| const msg = error instanceof Error ? error.message : String(error); | ||
| clack.log.error(`Failed to start authentication: ${msg}`); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const deviceAuth = (await authResponse.json()) as DeviceAuthResponse; | ||
| const pollIntervalMs = (deviceAuth.interval || 5) * 1000; | ||
|
|
||
| clack.log.info(`\nOpen this URL in your browser:\n`); | ||
| console.log(` ${deviceAuth.verification_uri}`); | ||
| console.log(`\nEnter code: ${deviceAuth.user_code}\n`); | ||
|
|
@@ -206,84 +137,44 @@ export async function runLogin(): Promise<void> { | |
| const spinner = clack.spinner(); | ||
| spinner.start('Waiting for authentication...'); | ||
|
|
||
| const startTime = Date.now(); | ||
| let currentInterval = pollIntervalMs; | ||
|
|
||
| while (Date.now() - startTime < POLL_TIMEOUT_MS) { | ||
| await sleep(currentInterval); | ||
|
|
||
| try { | ||
| const tokenResponse = await fetch(endpoints.token, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/x-www-form-urlencoded', | ||
| }, | ||
| body: new URLSearchParams({ | ||
| grant_type: 'urn:ietf:params:oauth:grant-type:device_code', | ||
| device_code: deviceAuth.device_code, | ||
| client_id: clientId, | ||
| }), | ||
| }); | ||
|
|
||
| const data = await tokenResponse.json(); | ||
|
|
||
| if (tokenResponse.ok) { | ||
| const result = data as ConnectTokenResponse; | ||
|
|
||
| // Parse user info from id_token JWT | ||
| const idTokenPayload = parseJwt(result.id_token); | ||
| const userId = (idTokenPayload?.sub as string) || 'unknown'; | ||
| const email = (idTokenPayload?.email as string) || undefined; | ||
|
|
||
| // Extract actual expiry from access token JWT, fallback to response or 15 min | ||
| const jwtExpiry = getJwtExpiry(result.access_token); | ||
| const expiresAt = | ||
| jwtExpiry ?? (result.expires_in ? Date.now() + result.expires_in * 1000 : Date.now() + 15 * 60 * 1000); | ||
|
|
||
| const expiresInSec = Math.round((expiresAt - Date.now()) / 1000); | ||
|
|
||
| saveCredentials({ | ||
| accessToken: result.access_token, | ||
| expiresAt, | ||
| userId, | ||
| email, | ||
| refreshToken: result.refresh_token, | ||
| }); | ||
| try { | ||
| const result = await pollForToken(deviceAuth.device_code, { | ||
| clientId, | ||
| authkitDomain, | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. test |
||
| interval: deviceAuth.interval, | ||
| }); | ||
|
|
||
| spinner.stop('Authentication successful!'); | ||
| clack.log.success(`Logged in as ${email || userId}`); | ||
| clack.log.info(`Token expires in ${expiresInSec} seconds`); | ||
| const expiresInSec = Math.round((result.expiresAt - Date.now()) / 1000); | ||
|
|
||
| // Auto-provision staging environment | ||
| const provisioned = await provisionStagingEnvironment(result.access_token); | ||
| if (provisioned) { | ||
| clack.log.success('Staging environment configured automatically'); | ||
| } else { | ||
| clack.log.info(chalk.dim('Run `workos env add` to configure an environment manually')); | ||
| } | ||
| saveCredentials({ | ||
| accessToken: result.accessToken, | ||
| expiresAt: result.expiresAt, | ||
| userId: result.userId, | ||
| email: result.email, | ||
| refreshToken: result.refreshToken, | ||
| }); | ||
|
|
||
| // Best-effort skill install. Wrapped helper guarantees login never | ||
| // fails on skill errors. | ||
| await installSkillsAfterLogin(); | ||
| return; | ||
| } | ||
| spinner.stop('Authentication successful!'); | ||
| clack.log.success(`Logged in as ${result.email || result.userId}`); | ||
| clack.log.info(`Token expires in ${expiresInSec} seconds`); | ||
|
|
||
| const errorData = data as AuthErrorResponse; | ||
| if (errorData.error === 'authorization_pending') continue; | ||
| if (errorData.error === 'slow_down') { | ||
| currentInterval += 5000; | ||
| continue; | ||
| } | ||
| const provisioned = await provisionStagingEnvironment(result.accessToken); | ||
| if (provisioned) { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. test |
||
| clack.log.success('Staging environment configured automatically'); | ||
| } else { | ||
| clack.log.info(chalk.dim('Run `workos env add` to configure an environment manually')); | ||
| } | ||
|
|
||
| await installSkillsAfterLogin(); | ||
| } catch (error) { | ||
| if (error instanceof DeviceAuthTimeoutError) { | ||
| spinner.stop('Authentication timed out'); | ||
| clack.log.error('Authentication timed out. Please try again.'); | ||
| } else { | ||
| spinner.stop('Authentication failed'); | ||
| clack.log.error(`Authentication error: ${errorData.error}`); | ||
| process.exit(1); | ||
| } catch { | ||
| continue; | ||
| const msg = error instanceof Error ? error.message : String(error); | ||
| clack.log.error(`Authentication error: ${msg}`); | ||
| } | ||
| process.exit(1); | ||
| } | ||
|
|
||
| spinner.stop('Authentication timed out'); | ||
| clack.log.error('Authentication timed out. Please try again.'); | ||
| process.exit(1); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add JSON-mode handling for the new login status/error output paths.
Line 115, Line 122, and Lines 157-177 introduce/modify human-only
clackoutput without a JSON branch. In--jsonmode, this can emit non-machine-readable text on the command path you changed.Suggested direction
As per coding guidelines:
src/commands/**/*.ts: Implement both human and JSON output modes in commands; checkOutputModeusage insrc/bin.ts.Also applies to: 157-177
🤖 Prompt for AI Agents