diff --git a/package.json b/package.json index 8e0868ba9..51e1860c2 100644 --- a/package.json +++ b/package.json @@ -132,6 +132,7 @@ "ora": "^9.3.0", "react": "^19.2.5", "react-dom": "^19.2.5", + "slugify": "^1.6.9", "smol-toml": "^1.6.1", "ws": "^8.20.0", "zod": "^3.25.76" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ac6b1b124..f8e1f07e0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -96,6 +96,9 @@ importers: react-dom: specifier: ^19.2.5 version: 19.2.5(react@19.2.5) + slugify: + specifier: ^1.6.9 + version: 1.6.9 smol-toml: specifier: ^1.6.1 version: 1.6.1 @@ -4874,6 +4877,10 @@ packages: resolution: {integrity: sha512-2wcC/oGxHis/BoHkkPwldgiPSYcpZK3JU28WoMVv55yHJgcZ8rlXvuG9iZggz+sU1d4bRgIGASwyWqjxu3FM0g==} engines: {node: '>=18'} + slugify@1.6.9: + resolution: {integrity: sha512-vZ7rfeehZui7wQs438JXBckYLkIIdfHOXsaVEUMyS5fHo1483l1bMdo0EDSWYclY0yZKFOipDy4KHuKs6ssvdg==} + engines: {node: '>=8.0.0'} + smol-toml@1.6.1: resolution: {integrity: sha512-dWUG8F5sIIARXih1DTaQAX4SsiTXhInKf1buxdY9DIg4ZYPZK5nGM1VRIYmEbDbsHt7USo99xSLFu5Q1IqTmsg==} engines: {node: '>= 18'} @@ -10684,6 +10691,8 @@ snapshots: mrmime: 2.0.1 totalist: 3.0.1 + slugify@1.6.9: {} + smol-toml@1.6.1: {} snake-case@3.0.4: diff --git a/src/base-command.ts b/src/base-command.ts index bb8d9cf16..55119cc5a 100644 --- a/src/base-command.ts +++ b/src/base-command.ts @@ -162,7 +162,9 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { constructor(argv: string[], config: CommandConfig) { super(argv, config); this.configManager = createConfigManager(); - this.interactiveHelper = new InteractiveHelper(this.configManager); + this.interactiveHelper = new InteractiveHelper(this.configManager, { + log: this.log.bind(this), + }); // Check if we're running in web CLI mode this.isWebCliMode = isWebCliMode(); } @@ -610,9 +612,9 @@ export abstract class AblyBaseCommand extends InteractiveBaseCommand { if (!appName) { try { // Get access token for control API - const currentAccount = this.configManager.getCurrentAccount(); const accessToken = - process.env.ABLY_ACCESS_TOKEN || currentAccount?.accessToken; + process.env.ABLY_ACCESS_TOKEN || + this.configManager.getAccessToken(); if (accessToken) { const controlApi = new ControlApi({ diff --git a/src/commands/accounts/current.ts b/src/commands/accounts/current.ts index 596630854..99885c843 100644 --- a/src/commands/accounts/current.ts +++ b/src/commands/accounts/current.ts @@ -2,7 +2,6 @@ import chalk from "chalk"; import { ControlBaseCommand } from "../../control-base-command.js"; import { errorMessage } from "../../utils/errors.js"; -import { ControlApi } from "../../services/control-api.js"; import { formatLabel } from "../../utils/output.js"; export default class AccountsCurrent extends ControlBaseCommand { @@ -38,14 +37,11 @@ export default class AccountsCurrent extends ControlBaseCommand { ); } - // Verify the account by making an API call to get up-to-date information + // Verify the account by making an API call to get up-to-date information. + // Route through createControlApi so OAuth accounts get the same + // TokenRefreshMiddleware used by every other control command. try { - const { accessToken } = currentAccount; - - const controlApi = new ControlApi({ - accessToken, - controlHost: flags["control-host"], - }); + const controlApi = this.createControlApi(flags); const { account, user } = await controlApi.getMe(); diff --git a/src/commands/accounts/login.ts b/src/commands/accounts/login.ts index f89c53a64..4cc7dc256 100644 --- a/src/commands/accounts/login.ts +++ b/src/commands/accounts/login.ts @@ -1,17 +1,20 @@ -import { Args, Flags } from "@oclif/core"; +import { Flags } from "@oclif/core"; import chalk from "chalk"; import * as readline from "node:readline"; -import open from "open"; +import ora from "ora"; import { ControlBaseCommand } from "../../control-base-command.js"; import { endpointFlag } from "../../flags.js"; import { ControlApi } from "../../services/control-api.js"; +import { OAuthClient, type OAuthTokens } from "../../services/oauth-client.js"; +import { BaseFlags } from "../../types/cli.js"; import { errorMessage } from "../../utils/errors.js"; import { displayLogo } from "../../utils/logo.js"; +import openUrl from "../../utils/open-url.js"; import { formatResource } from "../../utils/output.js"; import { promptForConfirmation } from "../../utils/prompt-confirmation.js"; +import { pickUniqueAlias, slugifyAccountName } from "../../utils/slugify.js"; -// Moved function definition outside the class function validateAndGetAlias( input: string, logFn: (msg: string) => void, @@ -42,18 +45,12 @@ function validateAndGetAlias( } export default class AccountsLogin extends ControlBaseCommand { - static override args = { - token: Args.string({ - description: "Access token (if not provided, will prompt for it)", - required: false, - }), - }; - static override description = "Log in to your Ably account"; static override examples = [ "<%= config.bin %> <%= command.id %>", "<%= config.bin %> <%= command.id %> --alias mycompany", + "<%= config.bin %> <%= command.id %> --no-browser", "<%= config.bin %> <%= command.id %> --json", "<%= config.bin %> <%= command.id %> --pretty-json", ]; @@ -72,113 +69,101 @@ export default class AccountsLogin extends ControlBaseCommand { }; public async run(): Promise { - const { args, flags } = await this.parse(AccountsLogin); + const { flags } = await this.parse(AccountsLogin); // Display ASCII art logo if not in JSON mode if (!this.shouldOutputJson(flags)) { displayLogo(this.log.bind(this)); } - let accessToken: string; - if (args.token) { - accessToken = args.token; - } else { - let obtainTokenPath = "https://ably.com/users/access_tokens"; - if (flags["control-host"]) { - if (!this.shouldOutputJson(flags)) { - this.log("Using control host:", flags["control-host"]); - } - - obtainTokenPath = flags["control-host"].includes("local") - ? `http://${flags["control-host"]}/users/access_tokens` - : `https://${flags["control-host"]}/users/access_tokens`; - } - - // Prompt the user to get a token - if (!flags["no-browser"]) { - this.logProgress("Opening browser to get an access token", flags); - - await this.openBrowser(obtainTokenPath); - } else if (!this.shouldOutputJson(flags)) { - this.log(`Please visit ${obtainTokenPath} to create an access token`); - } - - accessToken = await this.promptForToken(); + let oauthTokens: OAuthTokens; + try { + oauthTokens = await this.oauthLogin(flags); + } catch (error) { + this.fail(error, flags, "accountLogin"); } - // If no alias flag provided, prompt the user if they want to provide one - let { alias } = flags; - if (!alias && !this.shouldOutputJson(flags)) { - // Check if the default account already exists - const accounts = this.configManager.listAccounts(); - const hasDefaultAccount = accounts.some( - (account) => account.alias === "default", - ); + const accessToken = oauthTokens.accessToken; - if (hasDefaultAccount) { - // Explain to the user the implications of not providing an alias - this.log("\nYou have not specified an alias for this account."); - this.log( - "If you continue without an alias, your existing default account configuration will be overwritten.", - ); - this.log( - "To maintain multiple account profiles, please provide an alias.", - ); + try { + // Fetch account information + const controlApi = new ControlApi({ + accessToken, + controlHost: flags["control-host"], + }); - // Ask if they want to provide an alias - const shouldProvideAlias = await promptForConfirmation( - "Would you like to provide an alias for this account?", + const [{ account: meAccount, user }, accounts] = await Promise.all([ + controlApi.getMe(), + controlApi.getAccounts(), + ]); + + let selectedAccountInfo: { id: string; name: string }; + + if (accounts.length === 0) { + // Fallback to /me response if accounts list is empty + selectedAccountInfo = { id: meAccount.id, name: meAccount.name }; + } else if (accounts.length === 1) { + selectedAccountInfo = accounts[0]!; + } else if (accounts.length > 1 && !this.shouldOutputJson(flags)) { + const picked = + await this.interactiveHelper.selectAccountFromApi(accounts); + selectedAccountInfo = picked ?? accounts[0]!; + } else { + // Multiple accounts in JSON mode: auto-select the first but surface a + // warning so scripted callers can detect drift and choose explicitly. + selectedAccountInfo = accounts[0]!; + this.logWarning( + `Multiple accounts found (${accounts.length}); auto-selected ${selectedAccountInfo.name} (${selectedAccountInfo.id}). Run 'ably accounts login' in a terminal to choose, or use --alias to pin a specific account.`, + flags, ); + } - if (shouldProvideAlias) { - alias = await this.promptForAlias(); - } else { - alias = "default"; - this.log( - "No alias provided. The default account configuration will be overwritten.", - ); - } - } else { - // No default account exists yet, but still offer to set an alias - this.log("\nYou have not specified an alias for this account."); + if (!this.shouldOutputJson(flags)) { this.log( - "Using an alias allows you to maintain multiple account profiles that you can switch between.", + `\nUsing account ${formatResource(selectedAccountInfo.name)} (${selectedAccountInfo.id}).`, ); + } - // Ask if they want to provide an alias - const shouldProvideAlias = await promptForConfirmation( - "Would you like to provide an alias for this account?", + // Resolve alias AFTER account selection so we can default to account name + let { alias } = flags; + if (!alias && !this.shouldOutputJson(flags)) { + alias = await this.resolveAlias( + selectedAccountInfo.name, + selectedAccountInfo.id, ); - - if (shouldProvideAlias) { - alias = await this.promptForAlias(); - } else { - alias = "default"; - this.log( - "No alias provided. This will be set as your default account.", + } else if (!alias) { + // JSON / non-interactive: auto-suffix on collision with a different + // account so we never silently rebind an alias that already points + // somewhere else. + const picked = pickUniqueAlias( + slugifyAccountName(selectedAccountInfo.name), + selectedAccountInfo.id, + this.configManager.listAccounts(), + ); + alias = picked.alias; + if (picked.collidedWith) { + this.logWarning( + `Alias "${picked.collidedWith.alias}" is already used by a different account (${picked.collidedWith.accountId}); storing this login as "${alias}" instead.`, + flags, ); } } - } else if (!alias) { - alias = "default"; - } - - try { - // Fetch account information - const controlApi = new ControlApi({ - accessToken, - controlHost: flags["control-host"], - }); - - const { account, user } = await controlApi.getMe(); - // Store the account information - this.configManager.storeAccount(accessToken, alias, { - accountId: account.id, - accountName: account.name, - tokenId: "unknown", // Token ID is not returned by getMe(), would need additional API if needed - userEmail: user.email, - }); + // Store OAuth tokens (include user email from /me response). + // Pass oauthHost so the session key is scoped per authorization server + // — otherwise the same email on prod and a review deployment would + // collide. Also preserve controlHost so later commands talk to the same + // Control API deployment the user picked at login. + this.configManager.storeOAuthTokens( + alias, + { ...oauthTokens, userEmail: user.email }, + { + accountId: selectedAccountInfo.id, + accountName: selectedAccountInfo.name, + controlHost: flags["control-host"], + oauthHost: flags["oauth-host"], + }, + ); // Switch to this account this.configManager.switchAccount(alias); @@ -202,74 +187,81 @@ export default class AccountsLogin extends ControlBaseCommand { this.configManager.storeAppInfo(selectedApp.id, { appName: selectedApp.name, }); - } else if (apps.length > 1 && !this.shouldOutputJson(flags)) { - // Prompt user to select an app when multiple exist - this.log("\nSelect an app to use:"); - - selectedApp = await this.interactiveHelper.selectApp(controlApi); - - if (selectedApp) { - this.configManager.setCurrentApp(selectedApp.id); - this.configManager.storeAppInfo(selectedApp.id, { - appName: selectedApp.name, - }); - } - } else if (apps.length === 0 && !this.shouldOutputJson(flags)) { - // No apps exist - offer to create one - this.log("\nNo apps found in your account."); - - const shouldCreateApp = await promptForConfirmation( - "Would you like to create your first app now?", - ); - - if (shouldCreateApp) { - const appName = await this.promptForAppName(); + } else if (apps.length > 1) { + if (this.shouldOutputJson(flags)) { + this.logWarning( + "Multiple apps found; cannot auto-select in JSON mode. Run 'ably apps switch' in a terminal to choose one.", + flags, + ); + } else { + this.log("\nSelect an app to use:"); - try { - this.log(""); // blank line before progress - this.logProgress( - `Creating app ${formatResource(appName)}`, - flags, - ); + selectedApp = await this.interactiveHelper.selectApp(controlApi); - const app = await controlApi.createApp({ - name: appName, - tlsOnly: true, // Default to true for security + if (selectedApp) { + this.configManager.setCurrentApp(selectedApp.id); + this.configManager.storeAppInfo(selectedApp.id, { + appName: selectedApp.name, }); + } + } + } else if (apps.length === 0) { + if (this.shouldOutputJson(flags)) { + this.logWarning( + "No apps found in this account. Run 'ably apps create' to create one.", + flags, + ); + } else { + this.log("\nNo apps found in your account."); - selectedApp = app; - isAutoSelected = true; // Consider this auto-selected since it's the only one - - // Set as current app - this.configManager.setCurrentApp(app.id); - this.configManager.storeAppInfo(app.id, { appName: app.name }); + const shouldCreateApp = await promptForConfirmation( + "Would you like to create your first app now?", + ); - this.logSuccessMessage("App created successfully.", flags); - } catch (createError) { - this.warn( - `Failed to create app: ${createError instanceof Error ? createError.message : String(createError)}`, - ); - // Continue with login even if app creation fails + if (shouldCreateApp) { + const appName = await this.promptForAppName(); + + try { + this.log(""); // blank line before progress + this.logProgress( + `Creating app ${formatResource(appName)}`, + flags, + ); + + const app = await controlApi.createApp({ + name: appName, + tlsOnly: true, + }); + + selectedApp = app; + isAutoSelected = true; + + this.configManager.setCurrentApp(app.id); + this.configManager.storeAppInfo(app.id, { appName: app.name }); + + this.logSuccessMessage("App created successfully.", flags); + } catch (createError) { + this.logWarning( + `Failed to create app: ${createError instanceof Error ? createError.message : String(createError)}`, + flags, + ); + } } } } - // If apps.length === 0 and JSON mode, or user declined to create app, do nothing } catch (error) { - // Don't fail login if app fetching fails, just log for debugging - if (!this.shouldOutputJson(flags)) { - this.warn(`Could not fetch apps: ${errorMessage(error)}`); - } + this.logWarning(`Could not fetch apps: ${errorMessage(error)}`, flags); } // If we have a selected app, also handle API key selection let selectedKey = null; let isKeyAutoSelected = false; - if (selectedApp && !this.shouldOutputJson(flags)) { + if (selectedApp) { try { const keys = await controlApi.listKeys(selectedApp.id); if (keys.length === 1) { - // Auto-select the only key + // Auto-select the only key (safe in both modes) selectedKey = keys[0]!; isKeyAutoSelected = true; this.configManager.storeAppKey(selectedApp.id, selectedKey.key, { @@ -277,26 +269,35 @@ export default class AccountsLogin extends ControlBaseCommand { keyName: selectedKey.name || "Unnamed key", }); } else if (keys.length > 1) { - // Prompt user to select a key when multiple exist - this.log("\nSelect an API key to use:"); + if (this.shouldOutputJson(flags)) { + this.logWarning( + "Multiple API keys found; cannot auto-select in JSON mode. Run 'ably auth keys switch' in a terminal to choose one.", + flags, + ); + } else { + this.log("\nSelect an API key to use:"); - selectedKey = await this.interactiveHelper.selectKey( - controlApi, - selectedApp.id, - ); + selectedKey = await this.interactiveHelper.selectKey( + controlApi, + selectedApp.id, + ); - if (selectedKey) { - this.configManager.storeAppKey(selectedApp.id, selectedKey.key, { - keyId: selectedKey.id, - keyName: selectedKey.name || "Unnamed key", - }); + if (selectedKey) { + this.configManager.storeAppKey( + selectedApp.id, + selectedKey.key, + { + keyId: selectedKey.id, + keyName: selectedKey.name || "Unnamed key", + }, + ); + } } } - // If keys.length === 0, continue without key (should be rare for newly created apps) } catch (keyError) { - // Don't fail login if key fetching fails - this.warn( + this.logWarning( `Could not fetch API keys: ${keyError instanceof Error ? keyError.message : String(keyError)}`, + flags, ); } } @@ -304,6 +305,7 @@ export default class AccountsLogin extends ControlBaseCommand { if (this.shouldOutputJson(flags)) { const accountData: { alias: string; + authMethod: "oauth"; id: string; name: string; user: { email: string }; @@ -319,25 +321,24 @@ export default class AccountsLogin extends ControlBaseCommand { }; } = { alias, - id: account.id, - name: account.name, + authMethod: "oauth", + id: selectedAccountInfo.id, + name: selectedAccountInfo.name, user: { email: user.email, }, }; - if (selectedApp) { accountData.app = { + autoSelected: isAutoSelected, id: selectedApp.id, name: selectedApp.name, - autoSelected: isAutoSelected, }; - if (selectedKey) { accountData.key = { + autoSelected: isKeyAutoSelected, id: selectedKey.id, name: selectedKey.name || "Unnamed key", - autoSelected: isKeyAutoSelected, }; } } @@ -351,62 +352,154 @@ export default class AccountsLogin extends ControlBaseCommand { this.log(`Account ${formatResource(alias)} is now the current account`); } - this.logSuccessMessage( - `Successfully logged in to ${formatResource(account.name)} (account ID: ${chalk.greenBright(account.id)}).`, - flags, - ); + if (this.shouldOutputJson(flags)) { + // logJsonResult already emitted above + } else { + this.logSuccessMessage( + `Successfully logged in to ${formatResource(selectedAccountInfo.name)} (account ID: ${formatResource(selectedAccountInfo.id)}).`, + flags, + ); - if (selectedApp) { - const message = isAutoSelected - ? `Automatically selected app: ${formatResource(selectedApp.name)} (${selectedApp.id}).` - : `Selected app: ${formatResource(selectedApp.name)} (${selectedApp.id}).`; - this.logSuccessMessage(message, flags); - } + this.logToStderr("Authenticated via OAuth (token auto-refreshes)."); + + if (selectedApp) { + const message = isAutoSelected + ? `Automatically selected app: ${formatResource(selectedApp.name)} (${selectedApp.id}).` + : `Selected app: ${formatResource(selectedApp.name)} (${selectedApp.id}).`; + this.logSuccessMessage(message, flags); + } - if (selectedKey) { - const keyMessage = isKeyAutoSelected - ? `Automatically selected API key: ${formatResource(selectedKey.name || "Unnamed key")} (${selectedKey.id}).` - : `Selected API key: ${formatResource(selectedKey.name || "Unnamed key")} (${selectedKey.id}).`; - this.logSuccessMessage(keyMessage, flags); + if (selectedKey) { + const keyMessage = isKeyAutoSelected + ? `Automatically selected API key: ${formatResource(selectedKey.name || "Unnamed key")} (${selectedKey.id}).` + : `Selected API key: ${formatResource(selectedKey.name || "Unnamed key")} (${selectedKey.id}).`; + this.logSuccessMessage(keyMessage, flags); + } } } catch (error) { this.fail(error, flags, "accountLogin"); } } - private async openBrowser(url: string): Promise { + private async oauthLogin(flags: BaseFlags): Promise { + const oauthClient = new OAuthClient({ + oauthHost: flags["oauth-host"], + }); + + const deviceResponse = await oauthClient.requestDeviceCode(); + + if (this.shouldOutputJson(flags)) { + this.logJsonEvent( + { + status: "awaiting_authorization", + userCode: deviceResponse.userCode, + verificationUri: deviceResponse.verificationUri, + verificationUriComplete: deviceResponse.verificationUriComplete, + }, + flags, + ); + } else { + this.log(""); + this.log( + ` Your authorization code: ${formatResource(deviceResponse.userCode)}`, + ); + this.log(""); + this.log( + ` Visit: ${chalk.underline(deviceResponse.verificationUriComplete)}`, + ); + this.log(""); + } + + if (!flags["no-browser"]) { + await openUrl(deviceResponse.verificationUriComplete, this); + } else if (!this.shouldOutputJson(flags)) { + this.log("Open the URL above in your browser to authorize."); + } + + const spinner = this.shouldOutputJson(flags) + ? undefined + : ora("Waiting for authorization...").start(); + + // Wire up SIGINT so Ctrl+C during polling aborts the fetch and exits + // cleanly. Without this, the unsettled top-level await in bin/run.js + // triggers a confusing "Detected unsettled top-level await" warning + // from Node when the user hits Ctrl+C. + const abortController = new AbortController(); + const onSigint = () => { + abortController.abort(); + spinner?.fail("Authentication cancelled."); + // Match the conventional exit code for SIGINT (128 + signal number). + this.exit(130); + }; + process.once("SIGINT", onSigint); + try { - // Use the 'open' package for cross-platform browser opening - // This handles platform differences safely and avoids shell injection - await open(url); + const tokens = await oauthClient.pollForToken( + deviceResponse.deviceCode, + deviceResponse.interval, + deviceResponse.expiresIn, + abortController.signal, + ); + + spinner?.succeed("Authentication successful."); + return tokens; } catch (error) { - this.warn(`Failed to open browser: ${String(error)}`); - this.log(`Please visit ${url} manually to create an access token`); + spinner?.fail("Authentication failed."); + throw error; + } finally { + process.removeListener("SIGINT", onSigint); + } + } + + private async resolveAlias( + accountName: string, + accountId: string, + ): Promise { + const defaultAlias = slugifyAccountName(accountName); + const existingAccounts = this.configManager.listAccounts(); + const existing = existingAccounts.find((a) => a.alias === defaultAlias); + + // No collision, or the alias already points at the same account + // (legitimate re-login) — reuse without prompting. + if (!existing || existing.account.accountId === accountId) { + return defaultAlias; + } + + this.log( + `\nAn account with alias "${defaultAlias}" already exists (account ID: ${existing.account.accountId}) and would be overwritten.`, + ); + const shouldCustomize = await promptForConfirmation( + "Would you like to use a different alias?", + ); + if (shouldCustomize) { + return this.promptForAlias(defaultAlias); } + return defaultAlias; } - private promptForAlias(): Promise { + private promptForAlias(defaultAlias: string): Promise { const rl = readline.createInterface({ input: process.stdin, output: process.stdout, }); - // Pass this.log as the logging function to the external validator const logFn = this.log.bind(this); return new Promise((resolve) => { const askForAlias = () => { rl.question( - 'Enter an alias for this account (e.g. "dev", "production", "personal"): ', - (alias) => { - // Use the external validator function, passing the log function - const validatedAlias = validateAndGetAlias(alias, logFn); + `Enter an alias for this account [${defaultAlias}]: `, + (input) => { + // Accept default on empty input + if (!input.trim()) { + rl.close(); + resolve(defaultAlias); + return; + } - if (validatedAlias === null) { - if (!alias.trim()) { - logFn("Error: Alias cannot be empty"); // Use logFn here too - } + const validatedAlias = validateAndGetAlias(input, logFn); + if (validatedAlias === null) { askForAlias(); } else { rl.close(); @@ -444,18 +537,4 @@ export default class AccountsLogin extends ControlBaseCommand { askForAppName(); }); } - - private promptForToken(): Promise { - const rl = readline.createInterface({ - input: process.stdin, - output: process.stdout, - }); - - return new Promise((resolve) => { - rl.question("\nEnter your access token: ", (token) => { - rl.close(); - resolve(token.trim()); - }); - }); - } } diff --git a/src/commands/accounts/logout.ts b/src/commands/accounts/logout.ts index a9b03a7ad..53fb0cb8b 100644 --- a/src/commands/accounts/logout.ts +++ b/src/commands/accounts/logout.ts @@ -2,6 +2,7 @@ import { Args } from "@oclif/core"; import { ControlBaseCommand } from "../../control-base-command.js"; import { forceFlag } from "../../flags.js"; +import { OAuthClient } from "../../services/oauth-client.js"; import { promptForConfirmation } from "../../utils/prompt-confirmation.js"; export default class AccountsLogout extends ControlBaseCommand { @@ -73,6 +74,64 @@ export default class AccountsLogout extends ControlBaseCommand { } } + // Revoke OAuth tokens if this is an OAuth account and no other aliases share the session + if (this.configManager.getAuthMethod(targetAlias) === "oauth") { + const oauthTokens = this.configManager.getOAuthTokens(targetAlias); + if (oauthTokens) { + const sharingAliases = + this.configManager.getAliasesForOAuthSession(targetAlias); + const isLastAlias = sharingAliases.length <= 1; + + if (isLastAlias) { + const targetAccount = this.configManager + .listAccounts() + .find((a) => a.alias === targetAlias)?.account; + // Revoke against the host that minted the token. A --control-host + // override here would send the token to a server that never issued + // it — a no-op revocation that silently leaves the real token live. + const oauthHost = targetAccount?.oauthHost ?? flags["oauth-host"]; + const oauthClient = new OAuthClient({ + oauthHost, + }); + // Abortable best-effort revocation. The external AbortController + // cancels the in-flight fetch when the timer fires (unlike the old + // Promise.race which let the request complete in the background), + // and surfacing the reason lets us warn the user that revocation + // did not complete so they know the token may still be live until + // its natural expiry. + const revokeWithTimeout = async ( + token: string, + label: string, + timeoutMs = 5000, + ): Promise => { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + try { + await oauthClient.revokeToken(token, { + signal: controller.signal, + }); + } catch (error) { + const reason = controller.signal.aborted + ? `timed out after ${timeoutMs}ms` + : error instanceof Error + ? error.message + : String(error); + this.logWarning( + `Failed to revoke ${label} token (${reason}); it may remain valid until its natural expiry.`, + flags, + ); + } finally { + clearTimeout(timer); + } + }; + await Promise.all([ + revokeWithTimeout(oauthTokens.accessToken, "access"), + revokeWithTimeout(oauthTokens.refreshToken, "refresh"), + ]); + } + } + } + // Remove the account const success = this.configManager.removeAccount(targetAlias); diff --git a/src/commands/accounts/switch.ts b/src/commands/accounts/switch.ts index 73cff5b3d..90aac9d14 100644 --- a/src/commands/accounts/switch.ts +++ b/src/commands/accounts/switch.ts @@ -1,9 +1,12 @@ import { Args } from "@oclif/core"; +import chalk from "chalk"; +import inquirer from "inquirer"; import { ControlBaseCommand } from "../../control-base-command.js"; import { endpointFlag } from "../../flags.js"; -import { ControlApi } from "../../services/control-api.js"; +import { type AccountSummary } from "../../services/control-api.js"; import { formatResource } from "../../utils/output.js"; +import { pickUniqueAlias, slugifyAccountName } from "../../utils/slugify.js"; export default class AccountsSwitch extends ControlBaseCommand { static override args = { @@ -30,10 +33,9 @@ export default class AccountsSwitch extends ControlBaseCommand { public async run(): Promise { const { args, flags } = await this.parse(AccountsSwitch); - // Get available accounts - const accounts = this.configManager.listAccounts(); + const localAccounts = this.configManager.listAccounts(); - if (accounts.length === 0) { + if (localAccounts.length === 0) { if (this.shouldOutputJson(flags)) { this.fail( 'No accounts configured. Use "ably accounts login" to add an account.', @@ -50,18 +52,18 @@ export default class AccountsSwitch extends ControlBaseCommand { // If alias is provided, switch directly if (args.accountAlias) { - await this.switchToAccount(args.accountAlias, accounts, flags); + await this.switchToLocalAccount(args.accountAlias, localAccounts, flags); return; } - // Otherwise, show interactive selection if not in JSON mode + // JSON mode requires an explicit alias if (this.shouldOutputJson(flags)) { this.fail( "No account alias provided. Please specify an account alias to switch to.", flags, "accountSwitch", { - availableAccounts: accounts.map(({ account, alias }) => ({ + availableAccounts: localAccounts.map(({ account, alias }) => ({ alias, id: account.accountId, name: account.accountName, @@ -70,17 +72,186 @@ export default class AccountsSwitch extends ControlBaseCommand { ); } - this.log("Select an account to switch to:"); - const selectedAccount = await this.interactiveHelper.selectAccount(); + // Interactive mode: show local aliases + remote accounts + const selected = await this.interactiveSwitch(localAccounts, flags); - if (selectedAccount) { - await this.switchToAccount(selectedAccount.alias, accounts, flags); - } else { + if (!selected) { this.logWarning("Account switch cancelled.", flags); } } - private async switchToAccount( + private async interactiveSwitch( + localAccounts: Array<{ + account: { + accountId?: string; + accountName?: string; + userEmail?: string; + }; + alias: string; + }>, + flags: Record, + ): Promise { + const currentAlias = this.configManager.getCurrentAccountAlias(); + + // Try to fetch remote accounts using the current token. + let remoteAccounts: AccountSummary[] = []; + if (this.configManager.getAccessToken()) { + try { + const controlApi = this.createControlApi(flags); + remoteAccounts = await controlApi.getAccounts(); + } catch { + // Couldn't fetch remote accounts — fall back to local only + } + } + + // Build local account IDs set for deduplication + const localAccountIds = new Set( + localAccounts.map((a) => a.account.accountId).filter(Boolean), + ); + + // Remote accounts not already configured locally + const remoteOnly = remoteAccounts.filter((r) => !localAccountIds.has(r.id)); + + type Choice = { + name: string; + value: + | { type: "local"; alias: string } + | { type: "remote"; account: AccountSummary }; + }; + + const choices: Array = []; + + // Local accounts section + if (localAccounts.length > 0) { + choices.push(new inquirer.Separator("── Local accounts ──")); + for (const { account, alias } of localAccounts) { + const isCurrent = alias === currentAlias; + const name = account.accountName || account.accountId || "Unknown"; + const label = `${isCurrent ? "* " : " "}${alias} ${chalk.dim(`(${name})`)}`; + choices.push({ name: label, value: { type: "local", alias } }); + } + } + + // Remote-only accounts section + if (remoteOnly.length > 0) { + choices.push( + new inquirer.Separator("── Other accounts (no login required) ──"), + ); + for (const account of remoteOnly) { + const label = ` ${account.name} ${chalk.dim(`(${account.id})`)}`; + choices.push({ name: label, value: { type: "remote", account } }); + } + } + + if (choices.length === 0) { + this.log("No accounts available."); + return false; + } + + const { selected } = (await inquirer.prompt([ + { + choices, + message: "Select an account:", + name: "selected", + type: "list", + }, + ])) as { + selected: + | { type: "local"; alias: string } + | { type: "remote"; account: AccountSummary }; + }; + + if (selected.type === "local") { + const validAccounts = localAccounts + .filter((a) => a.account.accountId && a.account.accountName) + .map((a) => ({ + account: { + accountId: a.account.accountId!, + accountName: a.account.accountName!, + }, + alias: a.alias, + })); + await this.switchToLocalAccount(selected.alias, validAccounts, flags); + return true; + } + + // Remote account — create a local alias using the current token + this.addAndSwitchToRemoteAccount(selected.account, flags); + return true; + } + + private addAndSwitchToRemoteAccount( + remoteAccount: AccountSummary, + flags: Record, + ): void { + const currentAlias = this.configManager.getCurrentAccountAlias(); + if (!currentAlias) { + this.fail( + "No current account to copy credentials from.", + flags, + "accountSwitch", + ); + } + + const oauthTokens = this.configManager.getOAuthTokens(currentAlias); + if (!oauthTokens) { + this.fail( + "Current account does not use OAuth. Please log in with the target account directly.", + flags, + "accountSwitch", + ); + } + + const currentAccount = this.configManager.getCurrentAccount(); + // Pick a non-colliding alias — two different remote accounts whose names + // slugify identically (e.g. "Acme Prod" / "Acme-Prod") must not silently + // rebind an existing alias to a different accountId. + const picked = pickUniqueAlias( + slugifyAccountName(remoteAccount.name), + remoteAccount.id, + this.configManager.listAccounts(), + ); + const newAlias = picked.alias; + if (picked.collidedWith) { + this.logWarning( + `Alias "${picked.collidedWith.alias}" is already used by a different account (${picked.collidedWith.accountId}); storing this account as "${newAlias}" instead.`, + flags, + ); + } + + // Store the new alias with the same OAuth tokens but different account info. + // Carry over the source account's oauthHost (so the shared session key + // resolves correctly) and controlHost (so later Control API calls keep + // targeting the same deployment). + this.configManager.storeOAuthTokens( + newAlias, + { + ...oauthTokens, + userEmail: currentAccount?.userEmail, + }, + { + accountId: remoteAccount.id, + accountName: remoteAccount.name, + controlHost: currentAccount?.controlHost, + oauthHost: currentAccount?.oauthHost, + }, + ); + + this.configManager.switchAccount(newAlias); + + // Store custom endpoint if provided — parity with switchToLocalAccount so + // the flag is not silently dropped when the user picks a remote account. + if (flags.endpoint) { + this.configManager.storeEndpoint(flags.endpoint as string); + } + + this.log( + `Switched to account: ${formatResource(remoteAccount.name)} (${remoteAccount.id})`, + ); + this.log(`Saved as alias: ${formatResource(newAlias)}`); + } + + private async switchToLocalAccount( alias: string, accounts: Array<{ account: { accountId: string; accountName: string }; @@ -88,7 +259,6 @@ export default class AccountsSwitch extends ControlBaseCommand { }>, flags: Record, ): Promise { - // Check if account exists const accountExists = accounts.some((account) => account.alias === alias); if (!accountExists) { @@ -106,7 +276,6 @@ export default class AccountsSwitch extends ControlBaseCommand { ); } - // Switch to the account this.configManager.switchAccount(alias); // Store custom endpoint if provided @@ -114,21 +283,9 @@ export default class AccountsSwitch extends ControlBaseCommand { this.configManager.storeEndpoint(flags.endpoint as string); } - // Verify the account is valid by making an API call + // Verify the account is valid by making an API call. try { - const accessToken = this.configManager.getAccessToken(); - if (!accessToken) { - this.fail( - "No access token found for this account. Please log in again.", - flags, - "accountSwitch", - ); - } - - const controlApi = new ControlApi({ - accessToken, - controlHost: flags["control-host"] as string | undefined, - }); + const controlApi = this.createControlApi(flags); const { account, user } = await controlApi.getMe(); @@ -154,7 +311,7 @@ export default class AccountsSwitch extends ControlBaseCommand { this.log(`User: ${user.email}`); } } catch { - // The account switch already happened above (line 109), so this is non-fatal. + // The account switch already happened above, so this is non-fatal. // Warn the user but still report success with a warning field. const warningMessage = "Access token may have expired or is invalid. The account was switched, but token verification failed."; diff --git a/src/commands/login.ts b/src/commands/login.ts index 8a8855839..397c72096 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -1,22 +1,23 @@ -import { Command } from "@oclif/core"; - +import { ControlBaseCommand } from "../control-base-command.js"; import AccountsLogin from "./accounts/login.js"; -export default class Login extends Command { - static override args = AccountsLogin.args; - +export default class Login extends ControlBaseCommand { static override description = 'Log in to your Ably account (alias for "ably accounts login")'; static override examples = [ "<%= config.bin %> <%= command.id %>", "<%= config.bin %> <%= command.id %> --alias mycompany", + "<%= config.bin %> <%= command.id %> --json", ]; static override flags = AccountsLogin.flags; public async run(): Promise { - // Run the accounts login command with the same args and flags + // Login's own init/finally (from ControlBaseCommand) handle the lifecycle: + // web-CLI restriction check, ABLY_CURRENT_COMMAND, JSON completed signal, + // cached-client cleanup. AccountsLogin is run manually so its finally + // doesn't emit a duplicate completed signal. const accountsLogin = new AccountsLogin(this.argv, this.config); await accountsLogin.run(); } diff --git a/src/control-base-command.ts b/src/control-base-command.ts index 8b422b14f..b50497435 100644 --- a/src/control-base-command.ts +++ b/src/control-base-command.ts @@ -1,22 +1,29 @@ import { AblyBaseCommand } from "./base-command.js"; -import { controlApiFlags } from "./flags.js"; +import { controlApiFlags, oauthHostFlag } from "./flags.js"; import { ControlApi, App } from "./services/control-api.js"; +import { OAuthClient } from "./services/oauth-client.js"; +import { TokenRefreshMiddleware } from "./services/token-refresh-middleware.js"; import { BaseFlags } from "./types/cli.js"; import { errorMessage } from "./utils/errors.js"; import isWebCliMode from "./utils/web-mode.js"; export abstract class ControlBaseCommand extends AblyBaseCommand { - // Control API commands get core + hidden control API flags - static globalFlags = { ...controlApiFlags }; + // Control API commands get core + hidden control API flags + oauth-host + // (so token refresh can target the authorization server that minted the + // token even when --control-host points elsewhere). + static globalFlags = { ...controlApiFlags, ...oauthHostFlag }; /** * Create a Control API instance for making requests */ protected createControlApi(flags: BaseFlags): ControlApi { let accessToken = process.env.ABLY_ACCESS_TOKEN; + let tokenRefreshMiddleware: TokenRefreshMiddleware | undefined; + const account = accessToken + ? undefined + : this.configManager.getCurrentAccount(); if (!accessToken) { - const account = this.configManager.getCurrentAccount(); if (!account) { this.fail( `No access token provided. Please set the ABLY_ACCESS_TOKEN environment variable or configure an account with "ably accounts login".`, @@ -25,7 +32,24 @@ export abstract class ControlBaseCommand extends AblyBaseCommand { ); } - accessToken = account.accessToken; + accessToken = this.configManager.getAccessToken(); + + // Set up token refresh middleware for OAuth accounts. + // The OAuth issuer is an immutable property of the token — only the host + // that minted it can refresh it. Prefer the stored oauthHost so a + // --control-host override (intended for control-plane routing) does not + // silently direct refresh traffic at the wrong authorization server, + // which would return invalid_grant and wipe a valid session. + if (this.configManager.getAuthMethod() === "oauth") { + const oauthHost = account.oauthHost ?? flags["oauth-host"]; + const oauthClient = new OAuthClient({ + oauthHost, + }); + tokenRefreshMiddleware = new TokenRefreshMiddleware( + this.configManager, + oauthClient, + ); + } } if (!accessToken) { @@ -44,9 +68,16 @@ export abstract class ControlBaseCommand extends AblyBaseCommand { ); } + // Prefer the stored account host (what the user picked at login time) so + // later commands don't silently target the default control plane when the + // user originally logged in against a review / staging deployment. The + // flag still wins if explicitly passed, to allow one-off overrides. + const controlHost = flags["control-host"] ?? account?.controlHost; + return new ControlApi({ accessToken, - controlHost: flags["control-host"], + controlHost, + tokenRefreshMiddleware, }); } diff --git a/src/flags.ts b/src/flags.ts index 21bfed1a1..f88a681fc 100644 --- a/src/flags.ts +++ b/src/flags.ts @@ -71,6 +71,21 @@ export const clientIdFlag = { }), }; +/** + * Hidden oauth-host flag for overriding the OAuth authorization server host. + * Kept separate from --control-host because the OAuth server (ably.com) and + * the Control API (control.ably.net) are different services and may be + * overridden independently when targeting review/staging environments. + */ +export const oauthHostFlag = { + "oauth-host": Flags.string({ + description: + "Override the host for the OAuth authorization server, which defaults to ably.com", + hidden: process.env.ABLY_SHOW_DEV_FLAGS !== "true", + env: "ABLY_OAUTH_HOST", + }), +}; + /** * endpoint flag for login / accounts switch commands only. */ diff --git a/src/services/config-manager.ts b/src/services/config-manager.ts index 8d942bc28..89e147e76 100644 --- a/src/services/config-manager.ts +++ b/src/services/config-manager.ts @@ -3,6 +3,7 @@ import os from "node:os"; import path from "node:path"; import { parse, stringify } from "smol-toml"; import isTestMode from "../utils/test-mode.js"; +import { DEFAULT_OAUTH_HOST } from "./oauth-client.js"; // Updated to include key and app metadata export interface AppConfig { @@ -13,18 +14,36 @@ export interface AppConfig { } export interface AccountConfig { - accessToken: string; + // Legacy: pre-OAuth configs store the access token directly on the account. + // OAuth accounts use oauthSessionKey to reference a shared OAuthSession instead. + // Do not remove — needed for backward compatibility with existing configs. + accessToken?: string; + accessTokenExpiresAt?: number; accountId: string; accountName: string; apps?: { [appId: string]: AppConfig; }; + authMethod?: "oauth"; + controlHost?: string; currentAppId?: string; endpoint?: string; + // OAuth authorization server host (ably.com or a review-app override). + // Kept separate from controlHost so the session key and token-refresh + // traffic always targets the host that actually minted the tokens. + oauthHost?: string; + oauthSessionKey?: string; tokenId?: string; userEmail: string; } +export interface OAuthSession { + accessToken: string; + accessTokenExpiresAt: number; + oauthScope?: string; + refreshToken: string; +} + export interface AblyConfig { accounts: Record; current?: { @@ -38,11 +57,12 @@ export interface AblyConfig { }[]; }; }; + oauthSessions?: Record; } export interface ConfigManager { // Account management - getAccessToken(alias?: string): string | undefined; + getAccessToken(): string | undefined; getCurrentAccount(): AccountConfig | undefined; getCurrentAccountAlias(): string | undefined; listAccounts(): { account: AccountConfig; alias: string }[]; @@ -59,6 +79,35 @@ export interface ConfigManager { switchAccount(alias: string): boolean; removeAccount(alias: string): boolean; + // OAuth management + storeOAuthTokens( + alias: string, + tokens: { + accessToken: string; + refreshToken: string; + expiresAt: number; + scope?: string; + userEmail?: string; + }, + accountInfo?: { + accountId?: string; + accountName?: string; + controlHost?: string; + oauthHost?: string; + }, + ): void; + getOAuthTokens(alias?: string): + | { + accessToken: string; + refreshToken: string; + expiresAt: number; + } + | undefined; + isAccessTokenExpired(): boolean; + getAuthMethod(alias?: string): "oauth" | undefined; + getAliasesForOAuthSession(alias: string): string[]; + clearOAuthSession(alias?: string): void; + // App management getApiKey(appId?: string): string | undefined; getAppName(appId: string): string | undefined; @@ -101,6 +150,7 @@ export interface ConfigManager { // Config file getConfigPath(): string; saveConfig(): void; + reloadConfig(): void; } // Type declaration for test mocks available on globalThis @@ -153,14 +203,18 @@ export class TomlConfigManager implements ConfigManager { this.saveConfig(); } - // Get access token for the current account or specific alias - public getAccessToken(alias?: string): string | undefined { - if (alias) { - return this.config.accounts[alias]?.accessToken; - } + public getAccessToken(): string | undefined { + const account = this.getCurrentAccount(); + if (!account) return undefined; - const currentAccount = this.getCurrentAccount(); - return currentAccount?.accessToken; + // OAuth accounts read from the shared session + const session = account.oauthSessionKey + ? this.config.oauthSessions?.[account.oauthSessionKey] + : undefined; + if (session) return session.accessToken; + + // Fallback: pre-OAuth configs store the token directly on the account + return account.accessToken; } // Get API key for current app or specific app ID @@ -292,12 +346,27 @@ export class TomlConfigManager implements ConfigManager { // Remove an account public removeAccount(alias: string): boolean { - if (!this.config.accounts[alias]) { + const account = this.config.accounts[alias]; + if (!account) { return false; } + const sessionKey = account.oauthSessionKey; delete this.config.accounts[alias]; + // Clean up orphaned OAuth session entry + if (sessionKey && this.config.oauthSessions?.[sessionKey]) { + const stillReferenced = Object.values(this.config.accounts).some( + (a) => a.oauthSessionKey === sessionKey, + ); + if (!stillReferenced) { + delete this.config.oauthSessions[sessionKey]; + if (Object.keys(this.config.oauthSessions).length === 0) { + delete this.config.oauthSessions; + } + } + } + // If the removed account was the current one, clear the current account selection if (this.config.current?.account === alias) { delete this.config.current.account; @@ -307,6 +376,16 @@ export class TomlConfigManager implements ConfigManager { return true; } + public getAliasesForOAuthSession(alias: string): string[] { + const account = this.config.accounts[alias]; + if (!account?.oauthSessionKey) return [alias]; + + const sessionKey = account.oauthSessionKey; + return Object.entries(this.config.accounts) + .filter(([, acc]) => acc.oauthSessionKey === sessionKey) + .map(([a]) => a); + } + // Remove API key for an app public removeApiKey(appId: string): boolean { const currentAccount = this.getCurrentAccount(); @@ -335,6 +414,14 @@ export class TomlConfigManager implements ConfigManager { } } + // Re-read config from disk, discarding in-memory state. Used by the token + // refresh path to detect whether a concurrent CLI invocation has rotated + // tokens since we loaded them — otherwise we could clobber valid peer + // tokens with our stale snapshot. + public reloadConfig(): void { + this.loadConfig(); + } + // Set current app for the current account public setCurrentApp(appId: string): void { const currentAccount = this.getCurrentAccount(); @@ -476,6 +563,150 @@ export class TomlConfigManager implements ConfigManager { this.saveConfig(); } + // Store OAuth tokens, shared across aliases with the same userEmail + oauthHost. + // userEmail is supplied by callers from the /me response (fresh login) or from + // the previously-stored account metadata (token refresh, account switch) — it + // is never populated from the OAuth token response, which is RFC 6749 plain. + public storeOAuthTokens( + alias: string, + tokens: { + accessToken: string; + refreshToken: string; + expiresAt: number; + scope?: string; + userEmail?: string; + }, + accountInfo?: { + accountId?: string; + accountName?: string; + controlHost?: string; + oauthHost?: string; + }, + ): void { + const userEmail = + tokens.userEmail ?? this.config.accounts[alias]?.userEmail ?? ""; + const oauthHost = + accountInfo?.oauthHost ?? + this.config.accounts[alias]?.oauthHost ?? + DEFAULT_OAUTH_HOST; + const emailPart = userEmail.toLowerCase() || alias; + // Scope the session key by OAuth host so the same email on prod and a + // review app don't silently overwrite each other's refresh tokens — the + // issuer is the host that actually minted them. + const sessionKey = `${emailPart}::${oauthHost.toLowerCase()}`; + + // Create/update the shared OAuth session + if (!this.config.oauthSessions) { + this.config.oauthSessions = {}; + } + + // Clean up the previous session entry if this account's key is changing + // (e.g. migration from a pre-oauthHost key format). + const previousSessionKey = this.config.accounts[alias]?.oauthSessionKey; + if ( + previousSessionKey && + previousSessionKey !== sessionKey && + this.config.oauthSessions[previousSessionKey] + ) { + const stillReferenced = Object.entries(this.config.accounts).some( + ([otherAlias, acc]) => + otherAlias !== alias && acc.oauthSessionKey === previousSessionKey, + ); + if (!stillReferenced) { + delete this.config.oauthSessions[previousSessionKey]; + } + } + + this.config.oauthSessions[sessionKey] = { + accessToken: tokens.accessToken, + accessTokenExpiresAt: tokens.expiresAt, + oauthScope: tokens.scope, + refreshToken: tokens.refreshToken, + }; + + // Store account metadata and reference the OAuth session + this.config.accounts[alias] = { + ...this.config.accounts[alias], + accountId: + accountInfo?.accountId ?? this.config.accounts[alias]?.accountId ?? "", + accountName: + accountInfo?.accountName ?? + this.config.accounts[alias]?.accountName ?? + "", + apps: this.config.accounts[alias]?.apps || {}, + authMethod: "oauth", + controlHost: + accountInfo?.controlHost ?? this.config.accounts[alias]?.controlHost, + currentAppId: this.config.accounts[alias]?.currentAppId, + oauthHost: + accountInfo?.oauthHost ?? this.config.accounts[alias]?.oauthHost, + oauthSessionKey: sessionKey, + userEmail, + }; + + // Purge legacy pre-OAuth fields that the spread above may have carried + // over. They are inert for OAuth accounts but leave a stale plaintext + // token in the on-disk config. + delete this.config.accounts[alias].accessToken; + delete this.config.accounts[alias].accessTokenExpiresAt; + delete this.config.accounts[alias].tokenId; + + if (!this.config.current || !this.config.current.account) { + this.config.current = { account: alias }; + } + + this.saveConfig(); + } + + // Get OAuth tokens for the current account or specific alias + public getOAuthTokens(alias?: string): + | { + accessToken: string; + refreshToken: string; + expiresAt: number; + } + | undefined { + const account = alias + ? this.config.accounts[alias] + : this.getCurrentAccount(); + if (!account || account.authMethod !== "oauth") return undefined; + + const session = account.oauthSessionKey + ? this.config.oauthSessions?.[account.oauthSessionKey] + : undefined; + if (!session) return undefined; + + return { + accessToken: session.accessToken, + expiresAt: session.accessTokenExpiresAt, + refreshToken: session.refreshToken, + }; + } + + public isAccessTokenExpired(): boolean { + const account = this.getCurrentAccount(); + if (!account) return false; + + // OAuth accounts read expiry from the shared session; + // falls back to account-level field for pre-OAuth configs + const session = account.oauthSessionKey + ? this.config.oauthSessions?.[account.oauthSessionKey] + : undefined; + const expiresAt = + session?.accessTokenExpiresAt ?? account.accessTokenExpiresAt; + if (!expiresAt) return false; + + return Date.now() >= expiresAt - 60_000; + } + + // Get the auth method for the current account or specific alias + public getAuthMethod(alias?: string): "oauth" | undefined { + const account = alias + ? this.config.accounts[alias] + : this.getCurrentAccount(); + return account?.authMethod; + } + // Switch to a different account public switchAccount(alias: string): boolean { if (!this.config.accounts[alias]) { @@ -491,6 +722,37 @@ export class TomlConfigManager implements ConfigManager { return true; } + // Clear OAuth session(s) for an alias without removing the account itself. + // Used when a refresh token has been invalidated server-side — subsequent + // commands should surface "please re-login" immediately rather than + // re-attempting refresh against a dead token. + public clearOAuthSession(alias?: string): void { + const targetAlias = alias ?? this.config.current?.account; + if (!targetAlias) return; + const account = this.config.accounts[targetAlias]; + if (!account) return; + + const sessionKey = account.oauthSessionKey; + if (sessionKey && this.config.oauthSessions?.[sessionKey]) { + const stillReferenced = Object.entries(this.config.accounts).some( + ([otherAlias, acc]) => + otherAlias !== targetAlias && acc.oauthSessionKey === sessionKey, + ); + if (!stillReferenced) { + delete this.config.oauthSessions[sessionKey]; + if (Object.keys(this.config.oauthSessions).length === 0) { + delete this.config.oauthSessions; + } + } + } + + delete account.oauthSessionKey; + delete account.accessToken; + delete account.accessTokenExpiresAt; + + this.saveConfig(); + } + private ensureConfigDirExists(): void { if (!fs.existsSync(this.configDir)) { fs.mkdirSync(this.configDir, { mode: 0o700 }); // Secure permissions diff --git a/src/services/control-api.ts b/src/services/control-api.ts index 5565dc1d8..0da8220d7 100644 --- a/src/services/control-api.ts +++ b/src/services/control-api.ts @@ -1,11 +1,14 @@ import fetch, { type RequestInit } from "node-fetch"; import { CommandError } from "../errors/command-error.js"; import { getCliVersion } from "../utils/version.js"; +import isTestMode from "../utils/test-mode.js"; +import type { TokenRefreshMiddleware } from "./token-refresh-middleware.js"; export interface ControlApiOptions { accessToken: string; controlHost?: string; logErrors?: boolean; + tokenRefreshMiddleware?: TokenRefreshMiddleware; } export interface App { @@ -177,13 +180,31 @@ export interface MeResponse { user: { email: string }; } +export interface AccountSummary { + id: string; + name: string; +} + export class ControlApi { private accessToken: string; private controlHost: string; + private logErrors: boolean; + private tokenRefreshMiddleware?: TokenRefreshMiddleware; constructor(options: ControlApiOptions) { this.accessToken = options.accessToken; this.controlHost = options.controlHost || "control.ably.net"; + this.tokenRefreshMiddleware = options.tokenRefreshMiddleware; + // Explicit options.logErrors overrides env var; otherwise suppress in CI/test + if (options.logErrors === undefined) { + const suppressErrors = + process.env.SUPPRESS_CONTROL_API_ERRORS === "true" || + process.env.CI === "true" || + isTestMode(); + this.logErrors = !suppressErrors; + } else { + this.logErrors = options.logErrors; + } } // Ask a question to the Ably AI agent @@ -378,6 +399,20 @@ export class ControlApi { return matchingKey; } + // Get all accounts for the authenticated user + async getAccounts(): Promise { + try { + return await this.request("/me/accounts"); + } catch (error: unknown) { + // Graceful degradation: fall back to single account from /me if endpoint not available + if (error instanceof CommandError && error.statusCode === 404) { + const me = await this.getMe(); + return [{ id: me.account.id, name: me.account.name }]; + } + throw error; + } + } + // Get user and account info async getMe(): Promise { return this.request("/me"); @@ -518,9 +553,21 @@ export class ControlApi { method = "GET", body?: unknown, ): Promise { - const url = this.controlHost.includes("local") - ? `http://${this.controlHost}/api/v1${path}` - : `https://${this.controlHost}/v1${path}`; + // If we have a token refresh middleware, get a valid token before each request + if (this.tokenRefreshMiddleware) { + this.accessToken = + await this.tokenRefreshMiddleware.getValidAccessToken(); + } + + // The dedicated Control API service (control.ably.net) serves at `/v1/`. + // The website itself (ably.com and Heroku review apps) proxies the + // Control API at `/api/v1/`. Match hosts whose first label starts with + // "control" so both `control.` and `control-*.` variants route correctly, + // case insensitively so ABLY_CONTROL_HOST values aren't locale-sensitive. + const isControlService = /^control[-.]/i.test(this.controlHost); + const scheme = this.controlHost.includes("local") ? "http" : "https"; + const prefix = isControlService ? "/v1" : "/api/v1"; + const url = `${scheme}://${this.controlHost}${prefix}${path}`; const isFormData = body instanceof FormData; const options: RequestInit = { diff --git a/src/services/interactive-helper.ts b/src/services/interactive-helper.ts index daf153ee1..2b5e36f09 100644 --- a/src/services/interactive-helper.ts +++ b/src/services/interactive-helper.ts @@ -1,13 +1,15 @@ import inquirer from "inquirer"; import type { ConfigManager, AccountConfig } from "./config-manager.js"; -import type { App, ControlApi, Key } from "./control-api.js"; +import type { AccountSummary, App, ControlApi, Key } from "./control-api.js"; export interface InteractiveHelperOptions { + log?: (msg: string) => void; logErrors?: boolean; } export class InteractiveHelper { private configManager: ConfigManager; + private log: (msg: string) => void; private logErrors: boolean; constructor( @@ -15,6 +17,7 @@ export class InteractiveHelper { options: InteractiveHelperOptions = {}, ) { this.configManager = configManager; + this.log = options.log ?? console.log; this.logErrors = options.logErrors !== false; // Default to true } @@ -46,7 +49,7 @@ export class InteractiveHelper { const currentAlias = this.configManager.getCurrentAccountAlias(); if (accounts.length === 0) { - console.log( + this.log( 'No accounts configured. Use "ably accounts login" to add an account.', ); return null; @@ -74,7 +77,43 @@ export class InteractiveHelper { return selectedAccount; } catch (error) { if (this.logErrors) { - console.error("Error selecting account:", error); + this.log( + `Error selecting account: ${error instanceof Error ? error.message : String(error)}`, + ); + } + return null; + } + } + + /** + * Interactively select an account from API results (multi-account OAuth flow) + */ + async selectAccountFromApi( + accounts: AccountSummary[], + ): Promise { + try { + if (accounts.length === 0) { + return null; + } + + const { selectedAccount } = (await inquirer.prompt([ + { + choices: accounts.map((account) => ({ + name: `${account.name} (${account.id})`, + value: account, + })), + message: "Select an account:", + name: "selectedAccount", + type: "list", + }, + ])) as { selectedAccount: AccountSummary }; + + return selectedAccount; + } catch (error) { + if (this.logErrors) { + this.log( + `Error selecting account: ${error instanceof Error ? error.message : String(error)}`, + ); } return null; } @@ -88,9 +127,7 @@ export class InteractiveHelper { const apps = await controlApi.listApps(); if (apps.length === 0) { - console.log( - 'No apps found. Create an app with "ably apps create" first.', - ); + this.log('No apps found. Create an app with "ably apps create" first.'); return null; } @@ -109,7 +146,9 @@ export class InteractiveHelper { return selectedApp; } catch (error) { if (this.logErrors) { - console.error("Error fetching apps:", error); + this.log( + `Error fetching apps: ${error instanceof Error ? error.message : String(error)}`, + ); } return null; } @@ -123,7 +162,7 @@ export class InteractiveHelper { const keys = await controlApi.listKeys(appId); if (keys.length === 0) { - console.log("No keys found for this app."); + this.log("No keys found for this app."); return null; } @@ -142,7 +181,9 @@ export class InteractiveHelper { return selectedKey; } catch (error) { if (this.logErrors) { - console.error("Error fetching keys:", error); + this.log( + `Error fetching keys: ${error instanceof Error ? error.message : String(error)}`, + ); } return null; } diff --git a/src/services/oauth-client.ts b/src/services/oauth-client.ts new file mode 100644 index 000000000..19ae69fe1 --- /dev/null +++ b/src/services/oauth-client.ts @@ -0,0 +1,420 @@ +import fetch, { type Response as FetchResponse } from "node-fetch"; + +/** + * Default OAuth authorization-server host. Shared with config-manager so the + * session-key scope matches the host that actually minted the tokens. Kept + * distinct from the Control API host (control.ably.net) — they are separate + * services that happen to share the ably.com brand. + */ +export const DEFAULT_OAUTH_HOST = "ably.com"; + +/** + * Thrown by refreshAccessToken when the server rejects the refresh token + * (OAuth error "invalid_grant"). This happens when: + * - the refresh token was revoked (e.g. by logout) + * - it was rotated by a concurrent refresh (single-use refresh tokens) + * - the session has otherwise expired server-side + * Callers should treat this as "session ended, re-login required" rather + * than a transient network failure. + */ +export class OAuthRefreshExpiredError extends Error { + constructor(message: string) { + super(message); + this.name = "OAuthRefreshExpiredError"; + } +} + +export interface OAuthTokens { + accessToken: string; + expiresAt: number; + refreshToken: string; + scope?: string; + tokenType: string; +} + +export interface OAuthConfig { + clientId: string; + deviceCodeEndpoint: string; + revocationEndpoint: string; + scopes: string[]; + tokenEndpoint: string; +} + +export interface OAuthClientOptions { + oauthHost?: string; +} + +export interface DeviceCodeResponse { + deviceCode: string; + expiresIn: number; + interval: number; + userCode: string; + verificationUri: string; + verificationUriComplete: string; +} + +export class OAuthClient { + private config: OAuthConfig; + + constructor(options: OAuthClientOptions = {}) { + this.config = this.getOAuthConfig(options.oauthHost); + } + + /** + * Request a device code from the OAuth server (RFC 8628 step 1). + * A 15s abort timeout prevents a silently hung endpoint from blocking + * `ably login` indefinitely. + */ + async requestDeviceCode(): Promise { + const params = new URLSearchParams({ + client_id: this.config.clientId, + scope: this.config.scopes.join(" "), + }); + + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 15_000); + let response; + try { + response = await fetch(this.config.deviceCodeEndpoint, { + body: params.toString(), + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + method: "POST", + signal: controller.signal, + }); + } finally { + clearTimeout(timeout); + } + + if (!response.ok) { + const errorBody = await response.text(); + throw new Error( + `Device code request failed (${response.status}): ${errorBody}`, + ); + } + + const data = (await response.json()) as Record; + return { + deviceCode: data.device_code as string, + expiresIn: (data.expires_in as number) || 300, + interval: (data.interval as number) || 5, + userCode: data.user_code as string, + verificationUri: data.verification_uri as string, + verificationUriComplete: data.verification_uri_complete as string, + }; + } + + /** + * Poll for token completion (RFC 8628 step 2). + * Sleeps between requests, respects slow_down, and throws on expiry/denial. + * Accepts an optional AbortSignal for prompt cancellation. + */ + async pollForToken( + deviceCode: string, + interval: number, + expiresIn: number, + signal?: AbortSignal, + ): Promise { + const deadline = Date.now() + expiresIn * 1000; + let currentInterval = interval; + let networkRetries = 0; + const maxNetworkRetries = 3; + + while (Date.now() < deadline) { + if (signal?.aborted) { + throw new Error("Polling aborted"); + } + // Apply ±20% jitter so concurrent clients don't fall into lockstep and + // hit the authorization server in synchronized bursts, which trips the + // shared rate limit long before any individual client is misbehaving. + const jitterFactor = 0.9 + Math.random() * 0.2; + await this.sleep(currentInterval * 1000 * jitterFactor, signal); + + if (Date.now() >= deadline) { + throw new Error("Device code expired"); + } + + let response; + const controller = new AbortController(); + // Per-poll 15s timeout — without this a single hung fetch would block + // the outer `while (Date.now() < deadline)` guard and spin the spinner + // forever with no error. + const timeout = setTimeout(() => controller.abort(), 15_000); + const onExternalAbort = () => controller.abort(); + if (signal) { + if (signal.aborted) { + controller.abort(); + } else { + signal.addEventListener("abort", onExternalAbort); + } + } + try { + const params = new URLSearchParams({ + client_id: this.config.clientId, + device_code: deviceCode, + grant_type: "urn:ietf:params:oauth:grant-type:device_code", + }); + + response = await fetch(this.config.tokenEndpoint, { + body: params.toString(), + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + method: "POST", + signal: controller.signal, + }); + networkRetries = 0; + } catch { + if (signal?.aborted) { + throw new Error("Polling aborted"); + } + networkRetries++; + if (networkRetries >= maxNetworkRetries) { + throw new Error( + "Network error: failed to reach token endpoint after multiple retries", + ); + } + continue; + } finally { + clearTimeout(timeout); + signal?.removeEventListener("abort", onExternalAbort); + } + + if (response.ok) { + const data = (await response.json()) as Record; + return this.parseTokenResponse(data); + } + + const { code, description } = await parseOAuthError(response); + + switch (code) { + case "authorization_pending": { + continue; + } + case "slow_down": { + currentInterval += 5; + continue; + } + case "rate_limited": { + // Honour server-supplied Retry-After; fall back to exponential + // backoff capped at 30s if the header is missing or unparseable. + const retryAfter = Number(response.headers.get("retry-after")); + currentInterval = + Number.isFinite(retryAfter) && retryAfter > 0 + ? Math.min(retryAfter, 60) + : Math.min(currentInterval * 2, 30); + continue; + } + case "expired_token": { + throw new Error("Device code expired"); + } + case "access_denied": { + throw new Error("Authorization denied"); + } + default: { + const reason = description ?? code ?? `HTTP ${response.status}`; + throw new Error(`Token polling failed: ${reason}`); + } + } + } + + throw new Error("Device code expired"); + } + + /** + * Refresh an access token using a refresh token + */ + async refreshAccessToken(refreshToken: string): Promise { + return this.postForTokens( + { + client_id: this.config.clientId, + grant_type: "refresh_token", + refresh_token: refreshToken, + }, + "Token refresh", + refreshToken, + ); + } + + /** + * Revoke a token (access or refresh). + * + * Rejects on network failure, timeout, or non-2xx response. Callers that + * want best-effort behaviour (e.g. accounts logout) must catch the rejection + * themselves — surfacing it lets the caller distinguish a successful revoke + * from a timed-out one so it can warn the user. + * + * Pass an external `signal` to abort the in-flight fetch from the outside + * (the internal 10s timeout is still applied as a safety net). + */ + async revokeToken( + token: string, + options: { signal?: AbortSignal; timeoutMs?: number } = {}, + ): Promise { + const params = new URLSearchParams({ + client_id: this.config.clientId, + token, + }); + + const controller = new AbortController(); + const timeoutMs = options.timeoutMs ?? 10_000; + const timeout = setTimeout(() => controller.abort(), timeoutMs); + + // Link the caller-supplied signal to our internal controller so aborting + // the outer signal also cancels the fetch. + const externalSignal = options.signal; + const onExternalAbort = () => controller.abort(); + if (externalSignal) { + if (externalSignal.aborted) { + controller.abort(); + } else { + externalSignal.addEventListener("abort", onExternalAbort); + } + } + + try { + const response = await fetch(this.config.revocationEndpoint, { + body: params.toString(), + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + method: "POST", + signal: controller.signal, + }); + if (!response.ok) { + const errorBody = await response.text().catch(() => ""); + throw new Error( + `Token revocation failed (${response.status})${errorBody ? `: ${errorBody}` : ""}`, + ); + } + } finally { + // Always clear the timer, even on fetch rejection, or the pending + // setTimeout keeps the event loop alive for up to 10s after the + // command has otherwise finished. + clearTimeout(timeout); + externalSignal?.removeEventListener("abort", onExternalAbort); + } + } + + getClientId(): string { + return this.config.clientId; + } + + // --- Private helpers --- + + private getOAuthConfig(oauthHost = DEFAULT_OAUTH_HOST): OAuthConfig { + const scheme = oauthHost.includes("local") ? "http" : "https"; + return { + // Per RFC 8628 §3.1 and RFC 6749 §2.1, the device flow uses a public + // client — there is no client secret, and the client_id is not + // confidential. It is intentionally embedded in the binary; distributing + // it publicly does not weaken the security of the flow. + clientId: "gb-I8-bZRnXs-gF83jOWKQrUxPPWp_ldTfQtgGP0EFg", + deviceCodeEndpoint: `${scheme}://${oauthHost}/oauth/authorize_device`, + revocationEndpoint: `${scheme}://${oauthHost}/oauth/revoke`, + scopes: ["full_access"], + tokenEndpoint: `${scheme}://${oauthHost}/oauth/token`, + }; + } + + private async postForTokens( + params: Record, + operationName: string, + fallbackRefreshToken?: string, + ): Promise { + const body = new URLSearchParams(params); + + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 15_000); + let response; + try { + response = await fetch(this.config.tokenEndpoint, { + body: body.toString(), + headers: { "Content-Type": "application/x-www-form-urlencoded" }, + method: "POST", + signal: controller.signal, + }); + } finally { + // Always clear the timer — on network/TLS failure the exception + // propagates past clearTimeout and the setTimeout stays pending. + clearTimeout(timeout); + } + + if (!response.ok) { + const { code, description } = await parseOAuthError(response); + // Detect invalid_grant on refresh: the refresh token is gone (revoked, + // rotated by a concurrent refresh, or otherwise expired). Surface this + // as a typed error so callers can prompt the user to re-login. + if (params.grant_type === "refresh_token" && code === "invalid_grant") { + throw new OAuthRefreshExpiredError( + "OAuth refresh token is no longer valid. Please run 'ably login' again.", + ); + } + const reason = description ?? code ?? `HTTP ${response.status}`; + throw new Error(`${operationName} failed: ${reason}`); + } + + const data = (await response.json()) as Record; + return this.parseTokenResponse(data, fallbackRefreshToken); + } + + private parseTokenResponse( + data: Record, + fallbackRefreshToken?: string, + ): OAuthTokens { + const accessToken = + typeof data.access_token === "string" ? data.access_token : undefined; + const refreshToken = + typeof data.refresh_token === "string" + ? data.refresh_token + : fallbackRefreshToken; + + if (!accessToken || !refreshToken) { + throw new Error("Token response missing required fields"); + } + + const expiresIn = (data.expires_in as number) || 3600; + return { + accessToken, + expiresAt: Date.now() + expiresIn * 1000, + refreshToken, + scope: data.scope as string | undefined, + tokenType: (data.token_type as string) || "Bearer", + }; + } + + private sleep(ms: number, signal?: AbortSignal): Promise { + return new Promise((resolve, reject) => { + if (signal?.aborted) { + reject(new Error("Sleep aborted")); + return; + } + const timer = setTimeout(() => { + signal?.removeEventListener("abort", onAbort); + resolve(); + }, ms); + const onAbort = () => { + clearTimeout(timer); + reject(new Error("Sleep aborted")); + }; + signal?.addEventListener("abort", onAbort, { once: true }); + }); + } +} + +/** + * Parse an RFC 6749 §5.2 OAuth error body: `{error: "", + * error_description?: ""}`. Never throws — a non-JSON or unexpected + * body degrades to `{}` so callers fall through to their HTTP-status fallback. + */ +async function parseOAuthError( + response: FetchResponse, +): Promise<{ code?: string; description?: string }> { + try { + const data = (await response.json()) as Record; + return { + code: typeof data.error === "string" ? data.error : undefined, + description: + typeof data.error_description === "string" + ? data.error_description + : undefined, + }; + } catch { + return {}; + } +} diff --git a/src/services/token-refresh-middleware.ts b/src/services/token-refresh-middleware.ts new file mode 100644 index 000000000..dcce11178 --- /dev/null +++ b/src/services/token-refresh-middleware.ts @@ -0,0 +1,93 @@ +import type { ConfigManager } from "./config-manager.js"; +import type { OAuthClient, OAuthTokens } from "./oauth-client.js"; +import { OAuthRefreshExpiredError } from "./oauth-client.js"; + +export class TokenExpiredError extends Error { + constructor(message: string) { + super(message); + this.name = "TokenExpiredError"; + } +} + +export class TokenRefreshMiddleware { + private pendingRefresh: Promise | undefined; + + constructor( + private configManager: ConfigManager, + private oauthClient: OAuthClient, + ) {} + + async getValidAccessToken(): Promise { + const authMethod = this.configManager.getAuthMethod(); + + // Non-OAuth tokens: return as-is + if (authMethod !== "oauth") { + const token = this.configManager.getAccessToken(); + if (!token) + throw new TokenExpiredError( + "No access token found. Please run 'ably login'.", + ); + return token; + } + + // Not expired: return current token + if (!this.configManager.isAccessTokenExpired()) { + const token = this.configManager.getAccessToken(); + if (token) return token; + } + + // Expired: refresh using refresh token (deduplicate concurrent calls) + if (this.pendingRefresh) { + return this.pendingRefresh; + } + + this.pendingRefresh = this.refreshToken(); + try { + return await this.pendingRefresh; + } finally { + this.pendingRefresh = undefined; + } + } + + private async refreshToken(): Promise { + const tokens = this.configManager.getOAuthTokens(); + if (!tokens?.refreshToken) { + throw new TokenExpiredError( + "OAuth session expired. Please run 'ably login' again.", + ); + } + + const consumedRefreshToken = tokens.refreshToken; + let newTokens: OAuthTokens; + try { + newTokens = + await this.oauthClient.refreshAccessToken(consumedRefreshToken); + } catch (error) { + // invalid_grant typically means another CLI invocation already rotated + // this refresh token, or the session was revoked server-side. Before + // clearing the session, reload the on-disk config: a concurrent peer + // may have just rotated the token and persisted a fresh refresh token. + // Clobbering that by saving our stale in-memory view would destroy a + // valid session, turning a single-process failure into a fleet-wide + // "please re-login" for every invocation. + if (error instanceof OAuthRefreshExpiredError) { + this.configManager.reloadConfig(); + const onDisk = this.configManager.getOAuthTokens(); + // Only clear when disk still holds the refresh token we just tried + // (or the session is already gone). A different refresh token on + // disk means a peer rotated it successfully — their session is + // valid and must be preserved. + if (!onDisk || onDisk.refreshToken === consumedRefreshToken) { + this.configManager.clearOAuthSession(); + } + throw new TokenExpiredError( + "OAuth session expired (the refresh token is no longer valid, possibly because another CLI invocation refreshed concurrently). Please run 'ably login' again.", + ); + } + throw error; + } + const alias = this.configManager.getCurrentAccountAlias() || "default"; + this.configManager.storeOAuthTokens(alias, newTokens); + return newTokens.accessToken; + } +} diff --git a/src/types/cli.ts b/src/types/cli.ts index 82238f82c..03fec39a3 100644 --- a/src/types/cli.ts +++ b/src/types/cli.ts @@ -8,6 +8,7 @@ export interface BaseFlags { "client-id"?: string; "control-host"?: string; "dashboard-host"?: string; + "oauth-host"?: string; endpoint?: string; port?: number; tls?: string; diff --git a/src/utils/slugify.ts b/src/utils/slugify.ts new file mode 100644 index 000000000..b79401cb8 --- /dev/null +++ b/src/utils/slugify.ts @@ -0,0 +1,53 @@ +import slugify from "slugify"; + +/** + * Convert an account name to a valid alias slug. + * Rules: lowercase, alphanumeric + dashes, must start with a letter. + */ +export function slugifyAccountName(name: string): string { + const slug = slugify(name, { lower: true, strict: true, trim: true }) + .replaceAll(/-+/g, "-") + .replaceAll(/^-|-$/g, ""); + if (!slug) { + return "default"; + } + if (!/^[a-z]/.test(slug)) { + return `account-${slug}`; + } + return slug; +} + +/** + * Resolve a non-colliding alias for an OAuth account. + * + * Two distinct accounts whose names slugify to the same string would + * otherwise overwrite each other silently (e.g. "Acme Prod" and "Acme-Prod" + * both slugify to "acme-prod"). Re-logging into the *same* account (same + * accountId) is treated as a legitimate refresh and reuses the base alias. + * + * Returns the base alias when safe, or an auto-suffixed variant + * (`base-2`, `base-3`, ...) when a different account already owns the base. + */ +export function pickUniqueAlias( + desiredBase: string, + targetAccountId: string, + existingAccounts: { account: { accountId?: string }; alias: string }[], +): { alias: string; collidedWith?: { accountId?: string; alias: string } } { + const existing = existingAccounts.find((a) => a.alias === desiredBase); + if (!existing || existing.account.accountId === targetAccountId) { + return { alias: desiredBase }; + } + + const aliases = new Set(existingAccounts.map((a) => a.alias)); + let suffix = 2; + while (aliases.has(`${desiredBase}-${suffix}`)) { + suffix++; + } + return { + alias: `${desiredBase}-${suffix}`, + collidedWith: { + accountId: existing.account.accountId, + alias: desiredBase, + }, + }; +} diff --git a/test/helpers/mock-config-manager.ts b/test/helpers/mock-config-manager.ts index a7a4aee9b..d89129d77 100644 --- a/test/helpers/mock-config-manager.ts +++ b/test/helpers/mock-config-manager.ts @@ -26,6 +26,13 @@ import type { ConfigManager, } from "../../src/services/config-manager.js"; +// Kept in sync with DEFAULT_OAUTH_HOST in src/services/oauth-client.ts. +// Duplicated here because importing from oauth-client pulls node-fetch into +// the vitest setup graph (via test/unit/setup.ts → mock-config-manager), +// which installs node-fetch's global agent *before* nock is set up per test +// and breaks interception for commands that make HTTPS requests. +const DEFAULT_OAUTH_HOST = "ably.com"; + /** * Type for test configuration values. */ @@ -204,6 +211,7 @@ export class MockConfigManager implements ConfigManager { */ public clearAccounts(): void { this.config.accounts = {}; + delete this.config.oauthSessions; if (this.config.current) { delete this.config.current.account; } @@ -215,12 +223,18 @@ export class MockConfigManager implements ConfigManager { delete this.config.helpContext; } - public getAccessToken(alias?: string): string | undefined { - if (alias) { - return this.config.accounts[alias]?.accessToken; - } - const currentAccount = this.getCurrentAccount(); - return currentAccount?.accessToken; + public getAccessToken(): string | undefined { + const account = this.getCurrentAccount(); + if (!account) return undefined; + + // OAuth accounts read from the shared session + const session = account.oauthSessionKey + ? this.config.oauthSessions?.[account.oauthSessionKey] + : undefined; + if (session) return session.accessToken; + + // Fallback: pre-OAuth configs store the token directly on the account + return account.accessToken; } public getApiKey(appId?: string): string | undefined { @@ -331,12 +345,27 @@ export class MockConfigManager implements ConfigManager { } public removeAccount(alias: string): boolean { - if (!this.config.accounts[alias]) { + const account = this.config.accounts[alias]; + if (!account) { return false; } + const sessionKey = account.oauthSessionKey; delete this.config.accounts[alias]; + // Clean up orphaned OAuth session entry + if (sessionKey && this.config.oauthSessions?.[sessionKey]) { + const stillReferenced = Object.values(this.config.accounts).some( + (a) => a.oauthSessionKey === sessionKey, + ); + if (!stillReferenced) { + delete this.config.oauthSessions[sessionKey]; + if (Object.keys(this.config.oauthSessions).length === 0) { + delete this.config.oauthSessions; + } + } + } + if (this.config.current?.account === alias) { delete this.config.current.account; } @@ -360,6 +389,10 @@ export class MockConfigManager implements ConfigManager { // No-op for in-memory implementation } + public reloadConfig(): void { + // No-op: in-memory mock has no on-disk state to reload. + } + public setCurrentApp(appId: string): void { const currentAccount = this.getCurrentAccount(); const currentAlias = this.getCurrentAccountAlias(); @@ -377,6 +410,8 @@ export class MockConfigManager implements ConfigManager { accountInfo: { accountId: string; accountName: string; + controlHost?: string; + oauthHost?: string; tokenId?: string; userEmail: string; } = { @@ -390,6 +425,8 @@ export class MockConfigManager implements ConfigManager { accessToken, accountId: accountInfo.accountId, accountName: accountInfo.accountName, + controlHost: accountInfo.controlHost, + oauthHost: accountInfo.oauthHost, userEmail: accountInfo.userEmail, tokenId: accountInfo.tokenId, apps: existing?.apps || {}, @@ -482,6 +519,173 @@ export class MockConfigManager implements ConfigManager { ); } + public storeOAuthTokens( + alias: string, + tokens: { + accessToken: string; + refreshToken: string; + expiresAt: number; + scope?: string; + userEmail?: string; + }, + accountInfo?: { + accountId?: string; + accountName?: string; + controlHost?: string; + oauthHost?: string; + }, + ): void { + const userEmail = + tokens.userEmail ?? this.config.accounts[alias]?.userEmail ?? ""; + const oauthHost = + accountInfo?.oauthHost ?? + this.config.accounts[alias]?.oauthHost ?? + DEFAULT_OAUTH_HOST; + const emailPart = userEmail.toLowerCase() || alias; + const sessionKey = `${emailPart}::${oauthHost.toLowerCase()}`; + + // Create/update the shared OAuth session + if (!this.config.oauthSessions) { + this.config.oauthSessions = {}; + } + + // Clean up the previous session entry if this account's key is changing + const previousSessionKey = this.config.accounts[alias]?.oauthSessionKey; + if ( + previousSessionKey && + previousSessionKey !== sessionKey && + this.config.oauthSessions[previousSessionKey] + ) { + const stillReferenced = Object.entries(this.config.accounts).some( + ([otherAlias, acc]) => + otherAlias !== alias && acc.oauthSessionKey === previousSessionKey, + ); + if (!stillReferenced) { + delete this.config.oauthSessions[previousSessionKey]; + } + } + + this.config.oauthSessions[sessionKey] = { + accessToken: tokens.accessToken, + accessTokenExpiresAt: tokens.expiresAt, + oauthScope: tokens.scope, + refreshToken: tokens.refreshToken, + }; + + // Store account metadata and reference the OAuth session + this.config.accounts[alias] = { + ...this.config.accounts[alias], + accountId: + accountInfo?.accountId ?? this.config.accounts[alias]?.accountId ?? "", + accountName: + accountInfo?.accountName ?? + this.config.accounts[alias]?.accountName ?? + "", + apps: this.config.accounts[alias]?.apps || {}, + authMethod: "oauth", + controlHost: + accountInfo?.controlHost ?? this.config.accounts[alias]?.controlHost, + currentAppId: this.config.accounts[alias]?.currentAppId, + oauthHost: + accountInfo?.oauthHost ?? this.config.accounts[alias]?.oauthHost, + oauthSessionKey: sessionKey, + userEmail, + }; + + // Purge legacy pre-OAuth fields that the spread above may have carried + // over. They are inert for OAuth accounts but leave a stale plaintext + // token in the on-disk config. + delete this.config.accounts[alias].accessToken; + delete this.config.accounts[alias].accessTokenExpiresAt; + delete this.config.accounts[alias].tokenId; + + if (!this.config.current || !this.config.current.account) { + this.config.current = { account: alias }; + } + } + + public getOAuthTokens(alias?: string): + | { + accessToken: string; + refreshToken: string; + expiresAt: number; + } + | undefined { + const account = alias + ? this.config.accounts[alias] + : this.getCurrentAccount(); + if (!account || account.authMethod !== "oauth") return undefined; + + const session = account.oauthSessionKey + ? this.config.oauthSessions?.[account.oauthSessionKey] + : undefined; + if (!session) return undefined; + + return { + accessToken: session.accessToken, + expiresAt: session.accessTokenExpiresAt, + refreshToken: session.refreshToken, + }; + } + + public isAccessTokenExpired(): boolean { + const account = this.getCurrentAccount(); + if (!account) return false; + + // OAuth accounts read expiry from the shared session; + // falls back to account-level field for pre-OAuth configs + const session = account.oauthSessionKey + ? this.config.oauthSessions?.[account.oauthSessionKey] + : undefined; + const expiresAt = + session?.accessTokenExpiresAt ?? account.accessTokenExpiresAt; + if (!expiresAt) return false; + + return Date.now() >= expiresAt - 60_000; + } + + public getAuthMethod(alias?: string): "oauth" | undefined { + const account = alias + ? this.config.accounts[alias] + : this.getCurrentAccount(); + return account?.authMethod; + } + + public getAliasesForOAuthSession(alias: string): string[] { + const account = this.config.accounts[alias]; + if (!account?.oauthSessionKey) return [alias]; + + const sessionKey = account.oauthSessionKey; + return Object.entries(this.config.accounts) + .filter(([, acc]) => acc.oauthSessionKey === sessionKey) + .map(([a]) => a); + } + + public clearOAuthSession(alias?: string): void { + const targetAlias = alias ?? this.config.current?.account; + if (!targetAlias) return; + const account = this.config.accounts[targetAlias]; + if (!account) return; + + const sessionKey = account.oauthSessionKey; + if (sessionKey && this.config.oauthSessions?.[sessionKey]) { + const stillReferenced = Object.entries(this.config.accounts).some( + ([otherAlias, acc]) => + otherAlias !== targetAlias && acc.oauthSessionKey === sessionKey, + ); + if (!stillReferenced) { + delete this.config.oauthSessions[sessionKey]; + if (Object.keys(this.config.oauthSessions).length === 0) { + delete this.config.oauthSessions; + } + } + } + + delete account.oauthSessionKey; + delete account.accessToken; + delete account.accessTokenExpiresAt; + } + public switchAccount(alias: string): boolean { if (!this.config.accounts[alias]) { return false; diff --git a/test/unit/commands/accounts/login.test.ts b/test/unit/commands/accounts/login.test.ts index 096c87c94..1c2444f38 100644 --- a/test/unit/commands/accounts/login.test.ts +++ b/test/unit/commands/accounts/login.test.ts @@ -13,8 +13,38 @@ import { } from "../../../helpers/standard-tests.js"; import { parseNdjsonLines } from "../../../helpers/ndjson.js"; +const mockOAuthAccessToken = "oauth_access_token_12345"; +const mockRefreshToken = "oauth_refresh_token_67890"; + +/** + * Mock the OAuth device flow endpoints (device code + token polling). + * The device code endpoint returns immediately, and the token endpoint + * returns a successful token on the first poll. + */ +function mockOAuthDeviceFlow(host = "ably.com") { + const scheme = host.includes("local") ? "http" : "https"; + + nock(`${scheme}://${host}`) + .post("/oauth/authorize_device") + .reply(200, { + device_code: "dc_test", + expires_in: 300, + interval: 0.01, + user_code: "TEST-CODE", + verification_uri: `${scheme}://${host}/device`, + verification_uri_complete: `${scheme}://${host}/device?user_code=TEST-CODE`, + }); + + nock(`${scheme}://${host}`).post("/oauth/token").reply(200, { + access_token: mockOAuthAccessToken, + expires_in: 3600, + refresh_token: mockRefreshToken, + scope: "full_access", + token_type: "Bearer", + }); +} + describe("accounts:login command", () => { - const mockAccessToken = "test_access_token_12345"; const mockAccountId = "test-account-id"; beforeEach(() => { @@ -33,6 +63,8 @@ describe("accounts:login command", () => { describe("functionality", () => { it("should output JSON format when --json flag is used", async () => { + mockOAuthDeviceFlow(); + // Mock the /me endpoint nockControl() .get("/v1/me") @@ -41,11 +73,16 @@ describe("accounts:login command", () => { user: { email: "test@example.com" }, }); + // Mock the /me/accounts endpoint + nock("https://control.ably.net") + .get("/v1/me/accounts") + .reply(200, [{ id: mockAccountId, name: "Test Account" }]); + // Mock the apps list endpoint nockControl().get(`/v1/accounts/${mockAccountId}/apps`).reply(200, []); const { stdout } = await runCommand( - ["accounts:login", mockAccessToken, "--json"], + ["accounts:login", "--json"], import.meta.url, ); @@ -57,22 +94,31 @@ describe("accounts:login command", () => { const account = result.account as Record; expect(account).toHaveProperty("id", mockAccountId); expect(account).toHaveProperty("name", "Test Account"); + expect(account).toHaveProperty("authMethod", "oauth"); const user = account.user as Record; expect(user).toHaveProperty("email", "test@example.com"); // Verify config was updated correctly via mock const mock = getMockConfigManager(); const config = mock.getConfig(); - expect(config.current?.account).toBe("default"); - expect(config.accounts["default"]).toBeDefined(); - expect(config.accounts["default"].accessToken).toBe(mockAccessToken); - expect(config.accounts["default"].accountId).toBe(mockAccountId); - expect(config.accounts["default"].accountName).toBe("Test Account"); - expect(config.accounts["default"].userEmail).toBe("test@example.com"); + expect(config.current?.account).toBe("test-account"); + expect(config.accounts["test-account"]).toBeDefined(); + expect(config.accounts["test-account"].accountId).toBe(mockAccountId); + expect(config.accounts["test-account"].accountName).toBe("Test Account"); + expect(config.accounts["test-account"].authMethod).toBe("oauth"); + expect(config.accounts["test-account"].oauthSessionKey).toBeDefined(); + const session = + config.oauthSessions?.[ + config.accounts["test-account"].oauthSessionKey! + ]; + expect(session?.accessToken).toBe(mockOAuthAccessToken); + expect(session?.refreshToken).toBe(mockRefreshToken); + expect(session?.accessTokenExpiresAt).toBeDefined(); }); it("should include alias in JSON response when --alias flag is provided", async () => { const customAlias = "mycompany"; + mockOAuthDeviceFlow(); // Mock the /me endpoint nockControl() @@ -82,11 +128,16 @@ describe("accounts:login command", () => { user: { email: "test@example.com" }, }); + // Mock the /me/accounts endpoint + nock("https://control.ably.net") + .get("/v1/me/accounts") + .reply(200, [{ id: mockAccountId, name: "Test Account" }]); + // Mock the apps list endpoint nockControl().get(`/v1/accounts/${mockAccountId}/apps`).reply(200, []); const { stdout } = await runCommand( - ["accounts:login", mockAccessToken, "--alias", customAlias, "--json"], + ["accounts:login", "--alias", customAlias, "--json"], import.meta.url, ); @@ -102,15 +153,18 @@ describe("accounts:login command", () => { const config = mock.getConfig(); expect(config.current?.account).toBe(customAlias); expect(config.accounts[customAlias]).toBeDefined(); - expect(config.accounts[customAlias].accessToken).toBe(mockAccessToken); + expect(config.accounts[customAlias].oauthSessionKey).toBeDefined(); + const session = + config.oauthSessions?.[config.accounts[customAlias].oauthSessionKey!]; + expect(session?.accessToken).toBe(mockOAuthAccessToken); expect(config.accounts[customAlias].accountId).toBe(mockAccountId); expect(config.accounts[customAlias].accountName).toBe("Test Account"); - expect(config.accounts[customAlias].userEmail).toBe("test@example.com"); }); it("should include app info when single app is auto-selected", async () => { const mockAppId = "app-123"; const mockAppName = "My Only App"; + mockOAuthDeviceFlow(); // Mock the /me endpoint twice - once for initial login, once for listApps nockControl() @@ -121,6 +175,11 @@ describe("accounts:login command", () => { user: { email: "test@example.com" }, }); + // Mock the /me/accounts endpoint + nock("https://control.ably.net") + .get("/v1/me/accounts") + .reply(200, [{ id: mockAccountId, name: "Test Account" }]); + // Mock the apps list endpoint with single app nockControl() .get(`/v1/accounts/${mockAccountId}/apps`) @@ -129,7 +188,7 @@ describe("accounts:login command", () => { ]); const { stdout } = await runCommand( - ["accounts:login", mockAccessToken, "--json"], + ["accounts:login", "--json"], import.meta.url, ); @@ -145,18 +204,25 @@ describe("accounts:login command", () => { // Verify config was written with app info via mock const mock = getMockConfigManager(); const config = mock.getConfig(); - expect(config.current?.account).toBe("default"); - expect(config.accounts["default"]).toBeDefined(); - expect(config.accounts["default"].accessToken).toBe(mockAccessToken); - expect(config.accounts["default"].accountId).toBe(mockAccountId); - expect(config.accounts["default"].currentAppId).toBe(mockAppId); - expect(config.accounts["default"].apps?.[mockAppId]).toBeDefined(); - expect(config.accounts["default"].apps?.[mockAppId]?.appName).toBe( + expect(config.current?.account).toBe("test-account"); + expect(config.accounts["test-account"]).toBeDefined(); + expect(config.accounts["test-account"].oauthSessionKey).toBeDefined(); + const session = + config.oauthSessions?.[ + config.accounts["test-account"].oauthSessionKey! + ]; + expect(session?.accessToken).toBe(mockOAuthAccessToken); + expect(config.accounts["test-account"].accountId).toBe(mockAccountId); + expect(config.accounts["test-account"].currentAppId).toBe(mockAppId); + expect(config.accounts["test-account"].apps?.[mockAppId]).toBeDefined(); + expect(config.accounts["test-account"].apps?.[mockAppId]?.appName).toBe( mockAppName, ); }); it("should not include app info when multiple apps exist (no interactive selection in JSON mode)", async () => { + mockOAuthDeviceFlow(); + // Mock the /me endpoint twice - once for initial login, once for listApps nockControl() .get("/v1/me") @@ -166,6 +232,11 @@ describe("accounts:login command", () => { user: { email: "test@example.com" }, }); + // Mock the /me/accounts endpoint + nock("https://control.ably.net") + .get("/v1/me/accounts") + .reply(200, [{ id: mockAccountId, name: "Test Account" }]); + // Mock the apps list endpoint with multiple apps nockControl() .get(`/v1/accounts/${mockAccountId}/apps`) @@ -175,7 +246,7 @@ describe("accounts:login command", () => { ]); const { stdout } = await runCommand( - ["accounts:login", mockAccessToken, "--json"], + ["accounts:login", "--json"], import.meta.url, ); @@ -188,22 +259,29 @@ describe("accounts:login command", () => { // Verify config was written without app selection via mock const mock = getMockConfigManager(); const config = mock.getConfig(); - expect(config.current?.account).toBe("default"); - expect(config.accounts["default"]).toBeDefined(); - expect(config.accounts["default"].accessToken).toBe(mockAccessToken); - expect(config.accounts["default"].accountId).toBe(mockAccountId); + expect(config.current?.account).toBe("test-account"); + expect(config.accounts["test-account"]).toBeDefined(); + expect(config.accounts["test-account"].oauthSessionKey).toBeDefined(); + const session = + config.oauthSessions?.[ + config.accounts["test-account"].oauthSessionKey! + ]; + expect(session?.accessToken).toBe(mockOAuthAccessToken); + expect(config.accounts["test-account"].accountId).toBe(mockAccountId); // Should NOT have currentAppId when multiple apps exist - expect(config.accounts["default"].currentAppId).toBeUndefined(); + expect(config.accounts["test-account"].currentAppId).toBeUndefined(); }); }); describe("error handling", () => { it("should output error in JSON format when authentication fails", async () => { + mockOAuthDeviceFlow(); + // Mock authentication failure nockControl().get("/v1/me").reply(401, { error: "Unauthorized" }); const { stdout } = await runCommand( - ["accounts:login", "invalid_token", "--json"], + ["accounts:login", "--json"], import.meta.url, ); @@ -215,11 +293,16 @@ describe("accounts:login command", () => { }); it("should output error in JSON format when network fails", async () => { - // Mock network error + mockOAuthDeviceFlow(); + + // Mock network error for both endpoints called in parallel nockControl().get("/v1/me").replyWithError("Network error"); + nock("https://control.ably.net") + .get("/v1/me/accounts") + .replyWithError("Network error"); const { stdout } = await runCommand( - ["accounts:login", mockAccessToken, "--json"], + ["accounts:login", "--json"], import.meta.url, ); @@ -232,13 +315,15 @@ describe("accounts:login command", () => { }); it("should handle 500 server error", async () => { + mockOAuthDeviceFlow(); + // Mock server error nockControl() .get("/v1/me") .reply(500, { error: "Internal Server Error" }); const { stdout } = await runCommand( - ["accounts:login", mockAccessToken, "--json"], + ["accounts:login", "--json"], import.meta.url, ); @@ -248,33 +333,91 @@ describe("accounts:login command", () => { expect(result).toHaveProperty("success", false); expect(result).toHaveProperty("error"); }); + + it("should output error when OAuth device flow fails", async () => { + // Mock device code request failure + nock("https://ably.com") + .post("/oauth/authorize_device") + .reply(400, "invalid_client"); + + const { stdout } = await runCommand( + ["accounts:login", "--json"], + import.meta.url, + ); + + const result = parseNdjsonLines(stdout).find((r) => r.type === "error")!; + expect(result).toHaveProperty("type", "error"); + expect(result).toHaveProperty("command", "accounts:login"); + expect(result).toHaveProperty("success", false); + expect(result).toHaveProperty("error"); + }); + + it("should output error when authorization is denied", async () => { + // Device code succeeds + nock("https://ably.com").post("/oauth/authorize_device").reply(200, { + device_code: "dc_denied", + expires_in: 300, + interval: 0.01, + user_code: "DENY-CODE", + verification_uri: "https://ably.com/device", + verification_uri_complete: + "https://ably.com/device?user_code=DENY-CODE", + }); + + // Token polling returns access_denied + nock("https://ably.com") + .post("/oauth/token") + .reply(400, { error: "access_denied" }); + + const { stdout } = await runCommand( + ["accounts:login", "--json"], + import.meta.url, + ); + + const result = parseNdjsonLines(stdout).find((r) => r.type === "error")!; + expect(result).toHaveProperty("type", "error"); + expect(result).toHaveProperty("success", false); + expect(result.error.message).toContain("Authorization denied"); + }); }); standardFlagTests("accounts:login", import.meta.url, ["--json"]); - describe("custom control host", () => { - it("should use custom control host when --control-host flag is provided", async () => { - const customHost = "custom.ably.net"; + describe("custom hosts", () => { + it("routes OAuth to --oauth-host and Control API to --control-host independently", async () => { + // OAuth and Control API are distinct services; the CLI must target + // them separately. Using a `control.` prefix on the control host + // forces the /v1/ path; everything else uses /api/v1/. + const oauthHost = "oauth.custom.ably.net"; + const controlHost = "control.custom.ably.net"; - // Mock the /me endpoint on custom host - nock(`https://${customHost}`) + mockOAuthDeviceFlow(oauthHost); + + // Mock the /me endpoint on the control host + nock(`https://${controlHost}`) .get("/v1/me") .reply(200, { account: { id: mockAccountId, name: "Test Account" }, user: { email: "test@example.com" }, }); - // Mock the apps list endpoint on custom host - nock(`https://${customHost}`) + // Mock the /me/accounts endpoint on the control host + nock(`https://${controlHost}`) + .get("/v1/me/accounts") + .reply(200, [{ id: mockAccountId, name: "Test Account" }]); + + // Mock the apps list endpoint on the control host + nock(`https://${controlHost}`) .get(`/v1/accounts/${mockAccountId}/apps`) .reply(200, []); const { stdout } = await runCommand( [ "accounts:login", - mockAccessToken, + "--oauth-host", + oauthHost, "--control-host", - customHost, + controlHost, "--json", ], import.meta.url, @@ -290,12 +433,93 @@ describe("accounts:login command", () => { // Verify config was written correctly via mock const mock = getMockConfigManager(); const config = mock.getConfig(); - expect(config.current?.account).toBe("default"); - expect(config.accounts["default"]).toBeDefined(); - expect(config.accounts["default"].accessToken).toBe(mockAccessToken); - expect(config.accounts["default"].accountId).toBe(mockAccountId); - expect(config.accounts["default"].accountName).toBe("Test Account"); - expect(config.accounts["default"].userEmail).toBe("test@example.com"); + expect(config.current?.account).toBe("test-account"); + expect(config.accounts["test-account"]).toBeDefined(); + expect(config.accounts["test-account"].oauthSessionKey).toBeDefined(); + const session = + config.oauthSessions?.[ + config.accounts["test-account"].oauthSessionKey! + ]; + expect(session?.accessToken).toBe(mockOAuthAccessToken); + expect(config.accounts["test-account"].accountId).toBe(mockAccountId); + expect(config.accounts["test-account"].accountName).toBe("Test Account"); + }); + }); + + describe("OAuth device flow", () => { + it("should show --no-browser flag in help output", async () => { + const { stdout } = await runCommand( + ["accounts:login", "--help"], + import.meta.url, + ); + + expect(stdout).toContain("--no-browser"); + expect(stdout).toContain("Do not open a browser"); + }); + + it("should store OAuth tokens with authMethod, refreshToken, and expiresAt", async () => { + mockOAuthDeviceFlow(); + + nock("https://control.ably.net") + .get("/v1/me") + .reply(200, { + account: { id: mockAccountId, name: "Test Account" }, + user: { email: "test@example.com" }, + }); + + nock("https://control.ably.net") + .get("/v1/me/accounts") + .reply(200, [{ id: mockAccountId, name: "Test Account" }]); + + nock("https://control.ably.net") + .get(`/v1/accounts/${mockAccountId}/apps`) + .reply(200, []); + + await runCommand(["accounts:login", "--json"], import.meta.url); + + const mock = getMockConfigManager(); + const config = mock.getConfig(); + expect(config.accounts["test-account"].authMethod).toBe("oauth"); + expect(config.accounts["test-account"].oauthSessionKey).toBeDefined(); + const session = + config.oauthSessions?.[ + config.accounts["test-account"].oauthSessionKey! + ]; + expect(session?.refreshToken).toBe(mockRefreshToken); + expect(session?.accessTokenExpiresAt).toBeDefined(); + expect(session?.accessTokenExpiresAt).toBeGreaterThan(Date.now()); + }); + + it("should emit awaiting_authorization event in JSON mode", async () => { + mockOAuthDeviceFlow(); + + nock("https://control.ably.net") + .get("/v1/me") + .reply(200, { + account: { id: mockAccountId, name: "Test Account" }, + user: { email: "test@example.com" }, + }); + + nock("https://control.ably.net") + .get("/v1/me/accounts") + .reply(200, [{ id: mockAccountId, name: "Test Account" }]); + + nock("https://control.ably.net") + .get(`/v1/accounts/${mockAccountId}/apps`) + .reply(200, []); + + const { stdout } = await runCommand( + ["accounts:login", "--json"], + import.meta.url, + ); + + const events = parseNdjsonLines(stdout); + const authEvent = events.find( + (e) => e.status === "awaiting_authorization", + ); + expect(authEvent).toBeDefined(); + expect(authEvent).toHaveProperty("userCode", "TEST-CODE"); + expect(authEvent).toHaveProperty("verificationUri"); }); }); }); diff --git a/test/unit/commands/accounts/logout.test.ts b/test/unit/commands/accounts/logout.test.ts index 2f728665f..84785e7cf 100644 --- a/test/unit/commands/accounts/logout.test.ts +++ b/test/unit/commands/accounts/logout.test.ts @@ -1,5 +1,6 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { runCommand } from "@oclif/test"; +import nock from "nock"; import { getMockConfigManager } from "../../../helpers/mock-config-manager.js"; import { standardHelpTests, @@ -194,5 +195,175 @@ describe("accounts:logout command", () => { }); }); + describe("OAuth token revocation", () => { + afterEach(() => { + nock.cleanAll(); + }); + + it("should call revocation endpoint when logging out an OAuth account", async () => { + const mock = getMockConfigManager(); + mock.setConfig({ + current: { account: "testaccount" }, + accounts: { + testaccount: { + accessToken: "oauth_access_token", + accountId: "acc-123", + accountName: "Test Account", + userEmail: "test@example.com", + authMethod: "oauth", + oauthSessionKey: "test@example.com::ably.com", + accessTokenExpiresAt: Date.now() + 3600000, + }, + }, + oauthSessions: { + "test@example.com::ably.com": { + accessToken: "oauth_access_token", + refreshToken: "oauth_refresh_token", + accessTokenExpiresAt: Date.now() + 3600000, + }, + }, + }); + + // Expect two revocation calls (one for access token, one for refresh token) + const revokeScope = nock("https://ably.com") + .post("/oauth/revoke") + .twice() + .reply(200); + + const { stdout } = await runCommand( + ["accounts:logout", "--force", "--json"], + import.meta.url, + ); + + const result = parseNdjsonLines(stdout).find( + (r) => r.type === "result" || r.type === "error", + )!; + expect(result).toHaveProperty("success", true); + expect(revokeScope.isDone()).toBe(true); + }); + + it("should succeed even if revocation endpoint fails", async () => { + const mock = getMockConfigManager(); + mock.setConfig({ + current: { account: "testaccount" }, + accounts: { + testaccount: { + accessToken: "oauth_access_token", + accountId: "acc-123", + accountName: "Test Account", + userEmail: "test@example.com", + authMethod: "oauth", + oauthSessionKey: "test@example.com::ably.com", + accessTokenExpiresAt: Date.now() + 3600000, + }, + }, + oauthSessions: { + "test@example.com::ably.com": { + accessToken: "oauth_access_token", + refreshToken: "oauth_refresh_token", + accessTokenExpiresAt: Date.now() + 3600000, + }, + }, + }); + + // Revocation endpoint returns errors + nock("https://ably.com") + .post("/oauth/revoke") + .twice() + .replyWithError("Connection refused"); + + const { stdout } = await runCommand( + ["accounts:logout", "--force", "--json"], + import.meta.url, + ); + + const result = parseNdjsonLines(stdout).find( + (r) => r.type === "result" || r.type === "error", + )!; + expect(result).toHaveProperty("success", true); + + // Verify the account was still removed + const config = mock.getConfig(); + expect(config.accounts["testaccount"]).toBeUndefined(); + }); + + it("should not call revocation endpoint for non-OAuth account logout", async () => { + const mock = getMockConfigManager(); + mock.setConfig({ + current: { account: "testaccount" }, + accounts: { + testaccount: { + accessToken: "regular_token", + accountId: "acc-123", + accountName: "Test Account", + userEmail: "test@example.com", + }, + }, + }); + + // Set up nock interceptor that should NOT be called + const revokeScope = nock("https://ably.com") + .post("/oauth/revoke") + .reply(200); + + const { stdout } = await runCommand( + ["accounts:logout", "--force", "--json"], + import.meta.url, + ); + + const result = parseNdjsonLines(stdout).find( + (r) => r.type === "result" || r.type === "error", + )!; + expect(result).toHaveProperty("success", true); + + // The revocation endpoint should not have been called + expect(revokeScope.isDone()).toBe(false); + }); + + it("should not revoke tokens when other aliases share the same OAuth session", async () => { + const mock = getMockConfigManager(); + // Store two aliases sharing the same session via storeOAuthTokens + mock.storeOAuthTokens( + "alias-a", + { + accessToken: "shared_access_token", + refreshToken: "shared_refresh_token", + expiresAt: Date.now() + 3600000, + userEmail: "shared@example.com", + }, + { accountId: "acc-a", accountName: "Account A" }, + ); + mock.storeOAuthTokens( + "alias-b", + { + accessToken: "shared_access_token", + refreshToken: "shared_refresh_token", + expiresAt: Date.now() + 3600000, + userEmail: "shared@example.com", + }, + { accountId: "acc-b", accountName: "Account B" }, + ); + mock.setCurrentAccountAlias("alias-a"); + + const revokeScope = nock("https://ably.com") + .post("/oauth/revoke") + .twice() + .reply(200); + + await runCommand( + ["accounts:logout", "--force", "--json"], + import.meta.url, + ); + + // Revocation should NOT have been called — alias-b still uses the session + expect(revokeScope.isDone()).toBe(false); + // alias-b should still have valid tokens + expect(mock.getOAuthTokens("alias-b")).toBeDefined(); + expect(mock.getOAuthTokens("alias-b")?.refreshToken).toBe( + "shared_refresh_token", + ); + }); + }); + standardFlagTests("accounts:logout", import.meta.url, ["--json"]); }); diff --git a/test/unit/commands/accounts/switch.test.ts b/test/unit/commands/accounts/switch.test.ts index 89ac20cbf..dd3d0960b 100644 --- a/test/unit/commands/accounts/switch.test.ts +++ b/test/unit/commands/accounts/switch.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { runCommand } from "@oclif/test"; +import nock from "nock"; import { nockControl, controlApiCleanup, @@ -137,6 +138,115 @@ describe("accounts:switch command", () => { }); }); + describe("OAuth account switching", () => { + it("should switch between local OAuth accounts", async () => { + const mock = getMockConfigManager(); + + // Store an OAuth account + mock.storeOAuthTokens( + "oauth-acct", + { + accessToken: "oauth_token_123", + refreshToken: "refresh_token_123", + expiresAt: Date.now() + 3600000, + userEmail: "oauth@example.com", + }, + { + accountId: "oauth-account-id", + accountName: "OAuth Account", + }, + ); + + nockControl() + .get("/v1/me") + .reply(200, { + account: { id: "oauth-account-id", name: "OAuth Account" }, + user: { email: "oauth@example.com" }, + }); + + const { stderr } = await runCommand( + ["accounts:switch", "oauth-acct"], + import.meta.url, + ); + + expect(stderr).toContain("Switched to account"); + expect(mock.getCurrentAccountAlias()).toBe("oauth-acct"); + expect(mock.getAuthMethod("oauth-acct")).toBe("oauth"); + }); + + it("hits the stored controlHost when the account was logged in against a custom host", async () => { + const mock = getMockConfigManager(); + const customHost = "review-abc.herokuapp.com"; + + mock.storeOAuthTokens( + "custom-host-acct", + { + accessToken: "oauth_token_custom", + refreshToken: "refresh_token_custom", + expiresAt: Date.now() + 3_600_000, + userEmail: "custom@example.com", + }, + { + accountId: "custom-account-id", + accountName: "Custom Host Account", + controlHost: customHost, + oauthHost: customHost, + }, + ); + + const scope = nock(`https://${customHost}`) + .get("/api/v1/me") + .reply(200, { + account: { id: "custom-account-id", name: "Custom Host Account" }, + user: { email: "custom@example.com" }, + }); + + const { stderr } = await runCommand( + ["accounts:switch", "custom-host-acct"], + import.meta.url, + ); + + expect(stderr).toContain("Switched to account"); + expect(stderr).not.toContain("Access token may have expired"); + expect(scope.isDone()).toBe(true); + }); + + it("should return JSON with account info when switching OAuth account with --json", async () => { + const mock = getMockConfigManager(); + + mock.storeOAuthTokens( + "oauth-json", + { + accessToken: "oauth_token_json", + refreshToken: "refresh_token_json", + expiresAt: Date.now() + 3600000, + userEmail: "json@example.com", + }, + { + accountId: "json-account-id", + accountName: "JSON Account", + }, + ); + + nockControl() + .get("/v1/me") + .reply(200, { + account: { id: "json-account-id", name: "JSON Account" }, + user: { email: "json@example.com" }, + }); + + const { stdout } = await runCommand( + ["accounts:switch", "oauth-json", "--json"], + import.meta.url, + ); + + const result = parseJsonOutput(stdout); + expect(result).toHaveProperty("success", true); + expect(result.account.alias).toBe("oauth-json"); + expect(result.account.id).toBe("json-account-id"); + }); + }); + standardHelpTests("accounts:switch", import.meta.url); standardArgValidationTests("accounts:switch", import.meta.url); standardFlagTests("accounts:switch", import.meta.url, ["--json"]); diff --git a/test/unit/services/control-api.test.ts b/test/unit/services/control-api.test.ts index d4985543e..e1037d033 100644 --- a/test/unit/services/control-api.test.ts +++ b/test/unit/services/control-api.test.ts @@ -29,12 +29,12 @@ describe("ControlApi", function () { it("should use provided control host", function () { const customApi = new ControlApi({ accessToken, - controlHost: "custom.control.host", + controlHost: "control.custom.host", logErrors: false, }); // Set up nock to intercept request to custom host - const scope = nock("https://custom.control.host") + const scope = nock("https://control.custom.host") .get("/v1/me") .reply(200, { account: { id: "test-account-id" } }); diff --git a/test/unit/services/oauth-client.test.ts b/test/unit/services/oauth-client.test.ts new file mode 100644 index 000000000..a76a69d9c --- /dev/null +++ b/test/unit/services/oauth-client.test.ts @@ -0,0 +1,511 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import nock from "nock"; +import { OAuthClient } from "../../../src/services/oauth-client.js"; + +describe("OAuthClient", () => { + beforeEach(() => { + nock.cleanAll(); + }); + + afterEach(() => { + nock.cleanAll(); + }); + + describe("OAuth config derivation", () => { + it("default host uses https scheme", async () => { + const client = new OAuthClient(); + + const scope = nock("https://ably.com") + .post("/oauth/authorize_device") + .reply(200, { + device_code: "dc_test", + expires_in: 300, + interval: 5, + user_code: "ABCD-1234", + verification_uri: "https://ably.com/device", + verification_uri_complete: + "https://ably.com/device?user_code=ABCD-1234", + }); + + await client.requestDeviceCode(); + + expect(scope.isDone()).toBe(true); + }); + + it("host containing 'local' uses http scheme", async () => { + const client = new OAuthClient({ oauthHost: "localhost:3000" }); + + const scope = nock("http://localhost:3000") + .post("/oauth/authorize_device") + .reply(200, { + device_code: "dc_test", + expires_in: 300, + interval: 5, + user_code: "ABCD-1234", + verification_uri: "http://localhost:3000/device", + verification_uri_complete: + "http://localhost:3000/device?user_code=ABCD-1234", + }); + + await client.requestDeviceCode(); + + expect(scope.isDone()).toBe(true); + }); + + it("client ID matches the expected value", () => { + const client = new OAuthClient(); + expect(client.getClientId()).toBe( + "gb-I8-bZRnXs-gF83jOWKQrUxPPWp_ldTfQtgGP0EFg", + ); + }); + }); + + describe("requestDeviceCode", () => { + it("returns all fields from server response", async () => { + const client = new OAuthClient(); + + nock("https://ably.com").post("/oauth/authorize_device").reply(200, { + device_code: "dc_abc123", + expires_in: 600, + interval: 10, + user_code: "WXYZ-5678", + verification_uri: "https://ably.com/device", + verification_uri_complete: + "https://ably.com/device?user_code=WXYZ-5678", + }); + + const result = await client.requestDeviceCode(); + + expect(result.deviceCode).toBe("dc_abc123"); + expect(result.expiresIn).toBe(600); + expect(result.interval).toBe(10); + expect(result.userCode).toBe("WXYZ-5678"); + expect(result.verificationUri).toBe("https://ably.com/device"); + expect(result.verificationUriComplete).toBe( + "https://ably.com/device?user_code=WXYZ-5678", + ); + }); + + it("sends correct client_id and scope in body", async () => { + const client = new OAuthClient(); + + const scope = nock("https://ably.com") + .post( + "/oauth/authorize_device", + (body: Record) => + body.client_id === "gb-I8-bZRnXs-gF83jOWKQrUxPPWp_ldTfQtgGP0EFg" && + body.scope === "full_access", + ) + .reply(200, { + device_code: "dc_test", + expires_in: 300, + interval: 5, + user_code: "TEST-CODE", + verification_uri: "https://ably.com/device", + verification_uri_complete: + "https://ably.com/device?user_code=TEST-CODE", + }); + + await client.requestDeviceCode(); + + expect(scope.isDone()).toBe(true); + }); + + it("throws on non-200 response", async () => { + const client = new OAuthClient(); + + nock("https://ably.com") + .post("/oauth/authorize_device") + .reply(400, "invalid_client"); + + await expect(client.requestDeviceCode()).rejects.toThrow( + "Device code request failed (400): invalid_client", + ); + }); + }); + + describe("pollForToken", () => { + it("returns tokens after authorization_pending then success", async () => { + const client = new OAuthClient(); + + // First call: authorization_pending + nock("https://ably.com") + .post("/oauth/token") + .reply(400, { error: "authorization_pending" }); + + // Second call: success + nock("https://ably.com").post("/oauth/token").reply(200, { + access_token: "at_device_flow", + expires_in: 3600, + refresh_token: "rt_device_flow", + scope: "full_access", + token_type: "Bearer", + }); + + const tokens = await client.pollForToken("dc_test", 0.01, 10); + + expect(tokens.accessToken).toBe("at_device_flow"); + expect(tokens.refreshToken).toBe("rt_device_flow"); + expect(tokens.tokenType).toBe("Bearer"); + }); + + it("increases interval on slow_down", async () => { + const client = new OAuthClient(); + + // First call: slow_down (interval increases by 5s per RFC 8628) + nock("https://ably.com") + .post("/oauth/token") + .reply(400, { error: "slow_down" }); + + // Second call: success + nock("https://ably.com").post("/oauth/token").reply(200, { + access_token: "at_slow", + expires_in: 3600, + refresh_token: "rt_slow", + token_type: "Bearer", + }); + + const tokens = await client.pollForToken("dc_test", 0.01, 30); + + expect(tokens.accessToken).toBe("at_slow"); + }, 15_000); + + it("throws on expired_token", async () => { + const client = new OAuthClient(); + + nock("https://ably.com") + .post("/oauth/token") + .reply(400, { error: "expired_token" }); + + await expect(client.pollForToken("dc_test", 0.01, 10)).rejects.toThrow( + "Device code expired", + ); + }); + + it("throws on access_denied", async () => { + const client = new OAuthClient(); + + nock("https://ably.com") + .post("/oauth/token") + .reply(400, { error: "access_denied" }); + + await expect(client.pollForToken("dc_test", 0.01, 10)).rejects.toThrow( + "Authorization denied", + ); + }); + + it("sends correct grant_type and device_code", async () => { + const client = new OAuthClient(); + + const scope = nock("https://ably.com") + .post("/oauth/token", (body: Record) => { + return ( + body.grant_type === + "urn:ietf:params:oauth:grant-type:device_code" && + body.device_code === "dc_verify" && + body.client_id === "gb-I8-bZRnXs-gF83jOWKQrUxPPWp_ldTfQtgGP0EFg" + ); + }) + .reply(200, { + access_token: "at_verify", + expires_in: 3600, + refresh_token: "rt_verify", + token_type: "Bearer", + }); + + await client.pollForToken("dc_verify", 0.01, 10); + + expect(scope.isDone()).toBe(true); + }); + + it("throws after deadline exceeded", async () => { + const client = new OAuthClient(); + + // Keep returning authorization_pending; the deadline will expire + nock("https://ably.com") + .post("/oauth/token") + .times(10) + .reply(400, { error: "authorization_pending" }); + + // Use a very short expiresIn so the deadline passes quickly + await expect( + client.pollForToken("dc_expired", 0.01, 0.05), + ).rejects.toThrow("Device code expired"); + }); + }); + + describe("refreshAccessToken", () => { + it("returns OAuthTokens on successful refresh", async () => { + const client = new OAuthClient(); + + nock("https://ably.com").post("/oauth/token").reply(200, { + access_token: "refreshed_access_token", + expires_in: 7200, + refresh_token: "refreshed_refresh_token", + scope: "full_access", + token_type: "Bearer", + }); + + const tokens = await client.refreshAccessToken("old_refresh_token"); + + expect(tokens.accessToken).toBe("refreshed_access_token"); + expect(tokens.refreshToken).toBe("refreshed_refresh_token"); + expect(tokens.tokenType).toBe("Bearer"); + expect(tokens.scope).toBe("full_access"); + expect(tokens.expiresAt).toBeGreaterThan(Date.now()); + }); + + it("throws OAuthRefreshExpiredError on invalid_grant", async () => { + const client = new OAuthClient(); + + nock("https://ably.com").post("/oauth/token").reply(400, { + error: "invalid_grant", + error_description: "The refresh token is invalid.", + }); + + await expect( + client.refreshAccessToken("bad_refresh_token"), + ).rejects.toThrow(/OAuth refresh token is no longer valid/); + }); + + it("surfaces error_description on other OAuth errors", async () => { + const client = new OAuthClient(); + + nock("https://ably.com").post("/oauth/token").reply(400, { + error: "invalid_request", + error_description: "The refresh_token parameter is missing.", + }); + + await expect( + client.refreshAccessToken("bad_refresh_token"), + ).rejects.toThrow( + /Token refresh failed: The refresh_token parameter is missing\./, + ); + }); + + it("falls back to HTTP status when body is unparseable", async () => { + const client = new OAuthClient(); + + nock("https://ably.com").post("/oauth/token").reply(500, "upstream down"); + + await expect( + client.refreshAccessToken("bad_refresh_token"), + ).rejects.toThrow(/Token refresh failed: HTTP 500/); + }); + + it("sends correct form-encoded body", async () => { + const client = new OAuthClient(); + const refreshToken = "my_refresh_token"; + + const scope = nock("https://ably.com") + .post("/oauth/token", (body: Record) => { + return ( + body.grant_type === "refresh_token" && + body.client_id === "gb-I8-bZRnXs-gF83jOWKQrUxPPWp_ldTfQtgGP0EFg" && + body.refresh_token === refreshToken + ); + }) + .reply(200, { + access_token: "new_access", + expires_in: 3600, + refresh_token: "new_refresh", + token_type: "Bearer", + }); + + await client.refreshAccessToken(refreshToken); + + expect(scope.isDone()).toBe(true); + }); + + it("preserves existing refresh token when response omits refresh_token", async () => { + const client = new OAuthClient(); + + // Response omits refresh_token (allowed by RFC 6749 §5.1) + nock("https://ably.com").post("/oauth/token").reply(200, { + access_token: "new_access_token", + expires_in: 3600, + token_type: "Bearer", + }); + + const tokens = await client.refreshAccessToken("existing_refresh_token"); + + expect(tokens.accessToken).toBe("new_access_token"); + expect(tokens.refreshToken).toBe("existing_refresh_token"); + }); + + it("throws when response missing both access_token and refresh_token", async () => { + const client = new OAuthClient(); + + nock("https://ably.com").post("/oauth/token").reply(200, { + token_type: "Bearer", + expires_in: 3600, + }); + + await expect( + client.refreshAccessToken("some_refresh_token"), + ).rejects.toThrow("Token response missing required fields"); + }); + }); + + describe("revokeToken", () => { + it("sends POST to revocation endpoint", async () => { + const client = new OAuthClient(); + + const scope = nock("https://ably.com").post("/oauth/revoke").reply(200); + + await client.revokeToken("token_to_revoke"); + + expect(scope.isDone()).toBe(true); + }); + + it("throws on network error so the caller can report it", async () => { + const client = new OAuthClient(); + + nock("https://ably.com") + .post("/oauth/revoke") + .replyWithError("Connection refused"); + + await expect(client.revokeToken("token_to_revoke")).rejects.toThrow(); + }); + + it("throws on non-2xx response", async () => { + const client = new OAuthClient(); + + nock("https://ably.com") + .post("/oauth/revoke") + .reply(500, "internal_error"); + + await expect(client.revokeToken("token_to_revoke")).rejects.toThrow( + /Token revocation failed \(500\)/, + ); + }); + + it("rejects when external AbortSignal is aborted", async () => { + const client = new OAuthClient(); + + // Never responds — ensures the abort is what ends the request. + nock("https://ably.com").post("/oauth/revoke").delay(10_000).reply(200); + + const controller = new AbortController(); + const promise = client.revokeToken("token_to_revoke", { + signal: controller.signal, + }); + // Abort after the fetch is in flight. + setTimeout(() => controller.abort(), 20); + + await expect(promise).rejects.toThrow(); + }); + }); + + describe("pollForToken error handling", () => { + it("backs off on rate_limited (429) and eventually succeeds", async () => { + const client = new OAuthClient(); + + // First call: 429 with RFC 6749 §5.2 flat body + Retry-After header, + // matching what the website now emits (see website PR #7962). + nock("https://ably.com").post("/oauth/token").reply( + 429, + { + error: "rate_limited", + error_description: "Rate limit exceeded. Retry after 1 second.", + }, + { "Retry-After": "1" }, + ); + + nock("https://ably.com").post("/oauth/token").reply(200, { + access_token: "at_after_429", + expires_in: 3600, + refresh_token: "rt_after_429", + token_type: "Bearer", + }); + + const tokens = await client.pollForToken("dc_429", 0.01, 30); + expect(tokens.accessToken).toBe("at_after_429"); + }, 15_000); + + it("surfaces error_description on unknown error codes", async () => { + const client = new OAuthClient(); + + nock("https://ably.com").post("/oauth/token").reply(400, { + error: "invalid_request", + error_description: "The device_code parameter is malformed.", + }); + + await expect(client.pollForToken("dc_obj", 0.01, 10)).rejects.toThrow( + /Token polling failed: The device_code parameter is malformed\./, + ); + }); + + it("falls back to the error code when no description is provided", async () => { + const client = new OAuthClient(); + + nock("https://ably.com") + .post("/oauth/token") + .reply(400, { error: "invalid_request" }); + + await expect(client.pollForToken("dc_obj", 0.01, 10)).rejects.toThrow( + /Token polling failed: invalid_request/, + ); + }); + + it("falls back to HTTP status when the body is not parseable", async () => { + const client = new OAuthClient(); + + nock("https://ably.com") + .post("/oauth/token") + .reply(500, "upstream error"); + + await expect(client.pollForToken("dc_obj", 0.01, 10)).rejects.toThrow( + /Token polling failed: HTTP 500/, + ); + }); + }); + + describe("pollForToken abort signal", () => { + it("rejects promptly when the signal is aborted mid-sleep", async () => { + const client = new OAuthClient(); + const controller = new AbortController(); + + // A long interval (10s) guarantees we're in the sleep, not the fetch, + // when the signal fires. + const promise = client.pollForToken( + "dc_abort_sleep", + 10, + 60, + controller.signal, + ); + setTimeout(() => controller.abort(), 20); + + await expect(promise).rejects.toThrow(/aborted/i); + }, 2_000); + + it("rejects promptly when the signal is aborted during a fetch", async () => { + const client = new OAuthClient(); + const controller = new AbortController(); + + // The endpoint never responds, so without the abort the request would + // hit pollForToken's internal 15s fetch timeout instead. + nock("https://ably.com").post("/oauth/token").delay(10_000).reply(200); + + const promise = client.pollForToken( + "dc_abort_fetch", + 0.01, + 60, + controller.signal, + ); + setTimeout(() => controller.abort(), 100); + + await expect(promise).rejects.toThrow(/aborted/i); + }, 5_000); + + it("rejects immediately when the signal is already aborted", async () => { + const client = new OAuthClient(); + const controller = new AbortController(); + controller.abort(); + + await expect( + client.pollForToken("dc_pre_aborted", 1, 60, controller.signal), + ).rejects.toThrow(/aborted/i); + }); + }); +}); diff --git a/test/unit/services/shared-oauth-session.test.ts b/test/unit/services/shared-oauth-session.test.ts new file mode 100644 index 000000000..d4761ae64 --- /dev/null +++ b/test/unit/services/shared-oauth-session.test.ts @@ -0,0 +1,219 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { getMockConfigManager } from "../../helpers/mock-config-manager.js"; + +describe("Shared OAuth session", () => { + let mock: ReturnType; + + beforeEach(() => { + mock = getMockConfigManager(); + mock.clearAccounts(); + }); + + it("two aliases with the same userEmail share one session", () => { + mock.storeOAuthTokens( + "alias-a", + { + accessToken: "at_1", + refreshToken: "rt_1", + expiresAt: Date.now() + 3600000, + userEmail: "user@example.com", + }, + { accountId: "acc-a", accountName: "Account A" }, + ); + mock.storeOAuthTokens( + "alias-b", + { + accessToken: "at_1", + refreshToken: "rt_1", + expiresAt: Date.now() + 3600000, + userEmail: "user@example.com", + }, + { accountId: "acc-b", accountName: "Account B" }, + ); + + const config = mock.getConfig(); + expect(config.accounts["alias-a"].oauthSessionKey).toBe( + "user@example.com::ably.com", + ); + expect(config.accounts["alias-b"].oauthSessionKey).toBe( + "user@example.com::ably.com", + ); + // Only one session entry + expect(Object.keys(config.oauthSessions!)).toHaveLength(1); + }); + + it("refreshing one alias propagates tokens to all sharing aliases", () => { + // Initial store for both aliases + mock.storeOAuthTokens( + "alias-a", + { + accessToken: "at_old", + refreshToken: "rt_old", + expiresAt: Date.now() + 3600000, + userEmail: "user@example.com", + }, + { accountId: "acc-a", accountName: "Account A" }, + ); + mock.storeOAuthTokens( + "alias-b", + { + accessToken: "at_old", + refreshToken: "rt_old", + expiresAt: Date.now() + 3600000, + userEmail: "user@example.com", + }, + { accountId: "acc-b", accountName: "Account B" }, + ); + + // Simulate token refresh while on alias-a + mock.switchAccount("alias-a"); + mock.storeOAuthTokens("alias-a", { + accessToken: "at_new", + refreshToken: "rt_new", + expiresAt: Date.now() + 7200000, + userEmail: "user@example.com", + }); + + // Switch to alias-b — should see the refreshed tokens + mock.switchAccount("alias-b"); + expect(mock.getAccessToken()).toBe("at_new"); + expect(mock.getOAuthTokens()?.refreshToken).toBe("rt_new"); + }); + + it("different userEmails get separate sessions", () => { + mock.storeOAuthTokens( + "alias-a", + { + accessToken: "at_a", + refreshToken: "rt_a", + expiresAt: Date.now() + 3600000, + userEmail: "user1@example.com", + }, + { accountId: "acc-a", accountName: "Account A" }, + ); + mock.storeOAuthTokens( + "alias-b", + { + accessToken: "at_b", + refreshToken: "rt_b", + expiresAt: Date.now() + 3600000, + userEmail: "user2@example.com", + }, + { accountId: "acc-b", accountName: "Account B" }, + ); + + const config = mock.getConfig(); + expect(Object.keys(config.oauthSessions!)).toHaveLength(2); + expect(config.accounts["alias-a"].oauthSessionKey).toBe( + "user1@example.com::ably.com", + ); + expect(config.accounts["alias-b"].oauthSessionKey).toBe( + "user2@example.com::ably.com", + ); + + // Refreshing alias-a should NOT affect alias-b + mock.switchAccount("alias-a"); + mock.storeOAuthTokens("alias-a", { + accessToken: "at_a_new", + refreshToken: "rt_a_new", + expiresAt: Date.now() + 7200000, + userEmail: "user1@example.com", + }); + + mock.switchAccount("alias-b"); + expect(mock.getOAuthTokens()?.accessToken).toBe("at_b"); + expect(mock.getOAuthTokens()?.refreshToken).toBe("rt_b"); + }); + + it("getAliasesForOAuthSession returns all sharing aliases", () => { + mock.storeOAuthTokens( + "work", + { + accessToken: "at", + refreshToken: "rt", + expiresAt: Date.now() + 3600000, + userEmail: "user@example.com", + }, + { accountId: "acc-1", accountName: "Work" }, + ); + mock.storeOAuthTokens( + "personal", + { + accessToken: "at", + refreshToken: "rt", + expiresAt: Date.now() + 3600000, + userEmail: "user@example.com", + }, + { accountId: "acc-2", accountName: "Personal" }, + ); + + const aliases = mock.getAliasesForOAuthSession("work"); + expect(aliases).toContain("work"); + expect(aliases).toContain("personal"); + expect(aliases).toHaveLength(2); + }); + + it("removing last alias for a session cleans up the session entry", () => { + mock.storeOAuthTokens( + "only-alias", + { + accessToken: "at", + refreshToken: "rt", + expiresAt: Date.now() + 3600000, + userEmail: "user@example.com", + }, + { accountId: "acc-1", accountName: "Only" }, + ); + + expect(mock.getConfig().oauthSessions).toBeDefined(); + mock.removeAccount("only-alias"); + expect(mock.getConfig().oauthSessions).toBeUndefined(); + }); + + it("removing one alias keeps session when other aliases still reference it", () => { + mock.storeOAuthTokens( + "alias-a", + { + accessToken: "at", + refreshToken: "rt", + expiresAt: Date.now() + 3600000, + userEmail: "user@example.com", + }, + { accountId: "acc-a", accountName: "A" }, + ); + mock.storeOAuthTokens( + "alias-b", + { + accessToken: "at", + refreshToken: "rt", + expiresAt: Date.now() + 3600000, + userEmail: "user@example.com", + }, + { accountId: "acc-b", accountName: "B" }, + ); + + mock.removeAccount("alias-a"); + + // Session should still exist for alias-b + const config = mock.getConfig(); + expect(config.oauthSessions?.["user@example.com::ably.com"]).toBeDefined(); + expect(mock.getOAuthTokens("alias-b")?.refreshToken).toBe("rt"); + }); + + it("refreshToken is stored only in the session, not the account", () => { + mock.storeOAuthTokens( + "test", + { + accessToken: "at", + refreshToken: "rt", + expiresAt: Date.now() + 3600000, + userEmail: "user@example.com", + }, + { accountId: "acc-1", accountName: "Test" }, + ); + + mock.switchAccount("test"); + expect(mock.getOAuthTokens()?.refreshToken).toBe("rt"); + expect(mock.getAccessToken()).toBe("at"); + }); +}); diff --git a/test/unit/services/token-refresh-middleware.test.ts b/test/unit/services/token-refresh-middleware.test.ts new file mode 100644 index 000000000..ecff439e5 --- /dev/null +++ b/test/unit/services/token-refresh-middleware.test.ts @@ -0,0 +1,271 @@ +import { describe, it, expect, vi } from "vitest"; +import { OAuthRefreshExpiredError } from "../../../src/services/oauth-client.js"; +import { + TokenRefreshMiddleware, + TokenExpiredError, +} from "../../../src/services/token-refresh-middleware.js"; + +function createMockConfigManager(overrides: Record = {}) { + return { + clearOAuthSession: vi.fn(), + getAccessToken: vi.fn().mockReturnValue("current_access_token"), + getAuthMethod: vi.fn(), + getCurrentAccountAlias: vi.fn().mockReturnValue("default"), + getOAuthTokens: vi.fn(), + isAccessTokenExpired: vi.fn().mockReturnValue(false), + reloadConfig: vi.fn(), + storeOAuthTokens: vi.fn(), + ...overrides, + }; +} + +function createMockOAuthClient(overrides: Record = {}) { + return { + refreshAccessToken: vi.fn(), + ...overrides, + }; +} + +describe("TokenRefreshMiddleware", () => { + describe("non-OAuth passthrough", () => { + it("returns access token as-is when authMethod is not oauth", async () => { + const configManager = createMockConfigManager({ + getAuthMethod: vi.fn().mockReturnValue("token"), + getAccessToken: vi.fn().mockReturnValue("legacy_token_value"), + }); + const oauthClient = createMockOAuthClient(); + + const middleware = new TokenRefreshMiddleware( + configManager as never, + oauthClient as never, + ); + + const token = await middleware.getValidAccessToken(); + + expect(token).toBe("legacy_token_value"); + expect(oauthClient.refreshAccessToken).not.toHaveBeenCalled(); + }); + + it("throws TokenExpiredError when no access token is found", async () => { + const configManager = createMockConfigManager({ + getAuthMethod: vi.fn(), + getAccessToken: vi.fn(), + }); + const oauthClient = createMockOAuthClient(); + + const middleware = new TokenRefreshMiddleware( + configManager as never, + oauthClient as never, + ); + + await expect(middleware.getValidAccessToken()).rejects.toThrow( + TokenExpiredError, + ); + await expect(middleware.getValidAccessToken()).rejects.toThrow( + "No access token found", + ); + }); + }); + + describe("non-expired OAuth token", () => { + it("returns current token without calling refresh when not expired", async () => { + const configManager = createMockConfigManager({ + getAuthMethod: vi.fn().mockReturnValue("oauth"), + getAccessToken: vi.fn().mockReturnValue("valid_oauth_token"), + isAccessTokenExpired: vi.fn().mockReturnValue(false), + }); + const oauthClient = createMockOAuthClient(); + + const middleware = new TokenRefreshMiddleware( + configManager as never, + oauthClient as never, + ); + + const token = await middleware.getValidAccessToken(); + + expect(token).toBe("valid_oauth_token"); + expect(oauthClient.refreshAccessToken).not.toHaveBeenCalled(); + expect(configManager.storeOAuthTokens).not.toHaveBeenCalled(); + }); + }); + + describe("expired OAuth token", () => { + it("refreshes token, stores new tokens, and returns new access token", async () => { + const newTokens = { + accessToken: "refreshed_access_token", + expiresAt: Date.now() + 7200000, + refreshToken: "refreshed_refresh_token", + scope: "full_access", + tokenType: "Bearer", + }; + + const configManager = createMockConfigManager({ + getAuthMethod: vi.fn().mockReturnValue("oauth"), + getAccessToken: vi.fn().mockReturnValue("expired_oauth_token"), + isAccessTokenExpired: vi.fn().mockReturnValue(true), + getOAuthTokens: vi.fn().mockReturnValue({ + accessToken: "expired_oauth_token", + refreshToken: "valid_refresh_token", + expiresAt: Date.now() - 1000, + }), + getCurrentAccountAlias: vi.fn().mockReturnValue("default"), + }); + const oauthClient = createMockOAuthClient({ + refreshAccessToken: vi.fn().mockResolvedValue(newTokens), + }); + + const middleware = new TokenRefreshMiddleware( + configManager as never, + oauthClient as never, + ); + + const token = await middleware.getValidAccessToken(); + + expect(token).toBe("refreshed_access_token"); + expect(oauthClient.refreshAccessToken).toHaveBeenCalledWith( + "valid_refresh_token", + ); + expect(configManager.storeOAuthTokens).toHaveBeenCalledWith( + "default", + newTokens, + ); + }); + }); + + describe("missing refresh token", () => { + it("throws TokenExpiredError when token is expired but no refresh token available", async () => { + const configManager = createMockConfigManager({ + getAuthMethod: vi.fn().mockReturnValue("oauth"), + getAccessToken: vi.fn().mockReturnValue("expired_token"), + isAccessTokenExpired: vi.fn().mockReturnValue(true), + getOAuthTokens: vi.fn(), + }); + const oauthClient = createMockOAuthClient(); + + const middleware = new TokenRefreshMiddleware( + configManager as never, + oauthClient as never, + ); + + await expect(middleware.getValidAccessToken()).rejects.toThrow( + TokenExpiredError, + ); + await expect(middleware.getValidAccessToken()).rejects.toThrow( + "run 'ably login' again", + ); + }); + }); + + describe("refresh failure", () => { + it("propagates error when refreshAccessToken throws", async () => { + const configManager = createMockConfigManager({ + getAuthMethod: vi.fn().mockReturnValue("oauth"), + getAccessToken: vi.fn().mockReturnValue("expired_token"), + isAccessTokenExpired: vi.fn().mockReturnValue(true), + getOAuthTokens: vi.fn().mockReturnValue({ + accessToken: "expired_token", + refreshToken: "bad_refresh_token", + expiresAt: Date.now() - 1000, + }), + getCurrentAccountAlias: vi.fn().mockReturnValue("default"), + }); + const oauthClient = createMockOAuthClient({ + refreshAccessToken: vi + .fn() + .mockRejectedValue( + new Error("Token refresh failed (401): invalid_grant"), + ), + }); + + const middleware = new TokenRefreshMiddleware( + configManager as never, + oauthClient as never, + ); + + await expect(middleware.getValidAccessToken()).rejects.toThrow( + "Token refresh failed (401): invalid_grant", + ); + }); + }); + + describe("invalid_grant race with concurrent refresh", () => { + it("clears session when on-disk refresh token still matches the consumed one", async () => { + const consumed = "consumed_refresh_token"; + const getOAuthTokens = vi.fn().mockReturnValue({ + accessToken: "expired_access", + refreshToken: consumed, + expiresAt: Date.now() - 1000, + }); + const configManager = createMockConfigManager({ + getAuthMethod: vi.fn().mockReturnValue("oauth"), + getAccessToken: vi.fn().mockReturnValue("expired_access"), + isAccessTokenExpired: vi.fn().mockReturnValue(true), + getOAuthTokens, + getCurrentAccountAlias: vi.fn().mockReturnValue("default"), + }); + const oauthClient = createMockOAuthClient({ + refreshAccessToken: vi + .fn() + .mockRejectedValue( + new OAuthRefreshExpiredError("refresh token invalid"), + ), + }); + + const middleware = new TokenRefreshMiddleware( + configManager as never, + oauthClient as never, + ); + + await expect(middleware.getValidAccessToken()).rejects.toThrow( + TokenExpiredError, + ); + expect(configManager.reloadConfig).toHaveBeenCalled(); + expect(configManager.clearOAuthSession).toHaveBeenCalled(); + }); + + it("does NOT clear session when a peer rotated the refresh token on disk", async () => { + const consumed = "stale_refresh_token"; + const peerRotated = "fresh_refresh_token_from_peer"; + // First call (before refresh): returns the consumed token. + // Second call (after reload): returns the peer's fresh token. + const getOAuthTokens = vi + .fn() + .mockReturnValueOnce({ + accessToken: "expired_access", + refreshToken: consumed, + expiresAt: Date.now() - 1000, + }) + .mockReturnValueOnce({ + accessToken: "peer_fresh_access", + refreshToken: peerRotated, + expiresAt: Date.now() + 3600_000, + }); + const configManager = createMockConfigManager({ + getAuthMethod: vi.fn().mockReturnValue("oauth"), + getAccessToken: vi.fn().mockReturnValue("expired_access"), + isAccessTokenExpired: vi.fn().mockReturnValue(true), + getOAuthTokens, + getCurrentAccountAlias: vi.fn().mockReturnValue("default"), + }); + const oauthClient = createMockOAuthClient({ + refreshAccessToken: vi + .fn() + .mockRejectedValue( + new OAuthRefreshExpiredError("refresh token invalid"), + ), + }); + + const middleware = new TokenRefreshMiddleware( + configManager as never, + oauthClient as never, + ); + + await expect(middleware.getValidAccessToken()).rejects.toThrow( + TokenExpiredError, + ); + expect(configManager.reloadConfig).toHaveBeenCalled(); + // Critical: must NOT clear — peer's session is valid. + expect(configManager.clearOAuthSession).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/test/unit/utils/slugify.test.ts b/test/unit/utils/slugify.test.ts new file mode 100644 index 000000000..13908b4f5 --- /dev/null +++ b/test/unit/utils/slugify.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect } from "vitest"; +import { slugifyAccountName } from "../../../src/utils/slugify.js"; + +describe("slugifyAccountName", () => { + it("lowercases and replaces spaces with dashes", () => { + expect(slugifyAccountName("My Account")).toBe("my-account"); + }); + + it("strips leading and trailing dashes", () => { + expect(slugifyAccountName(" My Account ")).toBe("my-account"); + }); + + it("replaces non-alphanumeric characters with dashes", () => { + expect(slugifyAccountName("Ably (Production)")).toBe("ably-production"); + }); + + it("collapses consecutive non-alphanumeric to single dash", () => { + expect(slugifyAccountName("foo---bar")).toBe("foo-bar"); + }); + + it("returns 'default' for empty string", () => { + expect(slugifyAccountName("")).toBe("default"); + }); + + it("returns 'default' for whitespace-only string", () => { + expect(slugifyAccountName(" ")).toBe("default"); + }); + + it("prefixes with 'account-' when name starts with a number", () => { + expect(slugifyAccountName("123 Corp")).toBe("account-123-corp"); + }); + + it("prefixes with 'account-' when name starts with a dash after cleanup", () => { + expect(slugifyAccountName("--test")).toBe("test"); + }); + + it("handles single letter name", () => { + expect(slugifyAccountName("a")).toBe("a"); + }); + + it("handles purely numeric name", () => { + expect(slugifyAccountName("12345")).toBe("account-12345"); + }); + + it("maps unicode characters via the slugify charmap", () => { + expect(slugifyAccountName("André's Co")).toBe("andres-co"); + }); +});