From 01c3c79b4573329c2d55322354117d8d2672a0f7 Mon Sep 17 00:00:00 2001 From: Shengming Liang <231182611+AgentWings@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:59:41 -0700 Subject: [PATCH 1/3] feat(auth): enable generated browser login --- README.md | 20 +++++- package.json | 2 +- src/cli/login.ts | 16 ++++- src/commands/index.ts | 17 ++++- tests/browser-login.test.mjs | 129 +++++++++++++++++++++++++++++++++++ 5 files changed, 177 insertions(+), 7 deletions(-) create mode 100644 tests/browser-login.test.mjs diff --git a/README.md b/README.md index 501ae53..2ee823b 100644 --- a/README.md +++ b/README.md @@ -68,10 +68,26 @@ See the [API reference](./api.md) for every available operation. ## Signing In -`dedalus login` signs you in and saves the credential for later commands, so it does not have to be passed every time. It goes into your operating system's credential store — the system keyring on Linux, Credential Manager on Windows — and falls back to a file in your state directory, readable only by you, when no such store is available. On macOS it is always that file, because the system's own tool accepts a password only on its command line, where other processes could read it. Either way it is filed under the base URL it was captured for, so a credential saved for one host is never sent to another. `dedalus logout` forgets it. A credential passed with a flag, or set in the environment, still takes precedence over a saved one. Sign-in methods: api-key, x-api-key. Pass `--flow ` to pick one without being asked. +Run `dedalus login --flow browser` to sign in through your browser. The CLI uses +PKCE, verifies the callback state and issuer, and saves the access and refresh +tokens for subsequent commands. It refreshes expiring access tokens automatically. +Browser sign-in is bound to `https://dcs.dedaluslabs.ai`; an alternate API base URL +cannot receive this session. The interactive `dedalus login` menu also offers +browser sign-in and manual credentials. + +Browser sign-in requires the public `/oauth2/auth` and `/oauth2/token` endpoints +and a registered `dedalus-cli` public client supporting loopback callbacks. +Until that server support is deployed, use an API key. Local protocol tests do +not establish that hosted account sign-in is available. + +`dedalus logout` removes local credentials; it does not revoke tokens on the +server or sign out your browser. It does not read or remove sessions from the +older `dedalus auth login` implementation. + +`dedalus login` signs you in and saves the credential for later commands, so it does not have to be passed every time. It goes into your operating system's credential store — the system keyring on Linux, Credential Manager on Windows — and falls back to a file in your state directory, readable only by you, when no such store is available. On macOS it is always that file, because the system's own tool accepts a password only on its command line, where other processes could read it. Either way it is filed under the base URL it was captured for, so a credential saved for one host is never sent to another. `dedalus logout` forgets it. A credential passed with a flag, or set in the environment, still takes precedence over a saved one. Sign-in methods: browser, api-key, x-api-key. Pass `--flow ` to pick one without being asked. ```sh -dedalus login +dedalus login --flow browser dedalus login --flow api-key dedalus logout dedalus logout --all diff --git a/package.json b/package.json index 2d54ffa..9d568e0 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,7 @@ "scripts": { "build": "node -e \"require('node:fs').rmSync('dist', { recursive: true, force: true })\" && tsc -p tsconfig.json && tsc -p tsconfig.cjs.json && node scripts/finalize-build.mjs", "typecheck": "tsc -p tsconfig.json --noEmit && tsc -p tsconfig.cjs.json --noEmit", - "test": "node --test tests/feedback-*.test.mjs", + "test": "node --test tests/feedback-*.test.mjs tests/browser-login.test.mjs", "ci:generate": "hollywood generate 'ci/**/*.ts' --source-root ci --output .", "test:package": "node scripts/verify-staging-package.mjs" }, diff --git a/src/cli/login.ts b/src/cli/login.ts index 0bf9030..9e8319d 100644 --- a/src/cli/login.ts +++ b/src/cli/login.ts @@ -30,6 +30,8 @@ export type CliAuthMethodDefinition = { | { readonly kind: 'oauth'; readonly grant: 'authorizationCode'; + readonly resource?: string; + readonly issuer?: string; readonly clientKey: string; readonly tokenUrl: string; readonly refreshUrl: string; @@ -684,9 +686,15 @@ const authorizationCodeFlow = async ( const clientId = method.clientId; if (!clientId || !method.authorizationUrl) throw new Error('This flow needs a configured OAuth client id.'); const authorizeUrl = requireSecureUrl(method.authorizationUrl, baseUrl, 'authorization endpoint'); + if (method.resource) { + if (profileKey(baseUrl) !== profileKey(method.resource)) { + throw new UsageError('Browser sign-in is unavailable for this API base URL.'); + } + authorizeUrl.searchParams.set('resource', method.resource); + } const verifier = base64Url(randomBytes(32)); const challenge = base64Url(createHash('sha256').update(verifier).digest()); - const state = base64Url(randomBytes(16)); + const state = base64Url(randomBytes(32)); const server = createServer(); // Bound first, and only then given its request handler. Registering the handler earlier meant a @@ -708,7 +716,7 @@ const authorizationCodeFlow = async ( } throw error; }); - const redirect = awaitRedirect(server, state, REDIRECT_PATH); + const redirect = awaitRedirect(server, state, REDIRECT_PATH, method.issuer); const redirectUri = 'http://127.0.0.1:' + String(port) + REDIRECT_PATH; try { authorizeUrl.searchParams.set('response_type', 'code'); @@ -866,6 +874,7 @@ const awaitRedirect = ( server: ReturnType, state: string, path: string, + issuer?: string, ): Promise => new Promise((resolve, reject) => { server.on('request', (request, response) => { @@ -890,7 +899,8 @@ const awaitRedirect = ( send(404, 'text/plain', 'Not found.\n'); return; } - if (!sameToken(params.get('state') ?? '', state)) { + if (params.getAll('state').length !== 1 || !sameToken(params.get('state') ?? '', state) || + (issuer !== undefined && (params.getAll('iss').length !== 1 || params.get('iss') !== issuer))) { // Ends the sign-in rather than waiting for a better redirect. A request that reaches here is // on the registered path and carries a `code` or an `error`, which makes it the provider's // redirect rather than a stray probe — the path check above is what turns those away — so a diff --git a/src/commands/index.ts b/src/commands/index.ts index 4163441..462c2eb 100644 --- a/src/commands/index.ts +++ b/src/commands/index.ts @@ -841,7 +841,7 @@ const commands = [ const groups = [] as const satisfies readonly CliCommandGroup[]; -const auth = { +export const auth = { loginPath: ['login'], logoutPath: ['logout'], loginCommand: 'dedalus login', @@ -856,6 +856,21 @@ const auth = { xAPIKey: 'DEDALUS_X_API_KEY', }, methods: [ + { + name: 'browser', + label: 'Sign in with your browser', + kind: 'oauth', + grant: 'authorizationCode', + clientKey: 'apiKey', + clientId: 'dedalus-cli', + authorizationUrl: 'https://dcs.dedaluslabs.ai/oauth2/auth', + tokenUrl: 'https://dcs.dedaluslabs.ai/oauth2/token', + refreshUrl: 'https://dcs.dedaluslabs.ai/oauth2/token', + resource: 'https://dcs.dedaluslabs.ai', + issuer: 'https://dcs.dedaluslabs.ai', + scopes: ['dedalus:cli', 'offline_access'], + redirectPort: 0, + }, { name: 'api-key', label: 'Enter your access token', diff --git a/tests/browser-login.test.mjs b/tests/browser-login.test.mjs new file mode 100644 index 0000000..fbef463 --- /dev/null +++ b/tests/browser-login.test.mjs @@ -0,0 +1,129 @@ +import assert from 'node:assert/strict'; +import { spawn } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import { mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; +import { createServer } from 'node:http'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { test } from 'node:test'; +import { auth } from '../dist/esm/commands/index.js'; +import { runLogin } from '../dist/esm/cli/login.js'; + +test('browser login refuses to store a public session for another API', async () => { + await assert.rejects(runLogin(auth, 'https://other.invalid', 'browser'), /unavailable for this API/); +}); + +test('browser configuration uses the public native client contract', () => { + const method = auth.methods.find((entry) => entry.name === 'browser'); + assert.equal(method.clientId, 'dedalus-cli'); + assert.equal(method.resource, auth.defaultBaseUrl); + assert.equal(method.issuer, auth.defaultBaseUrl); + assert.equal(method.authorizationUrl, auth.defaultBaseUrl + '/oauth2/auth'); + assert.equal(method.tokenUrl, auth.defaultBaseUrl + '/oauth2/token'); + assert.deepEqual(method.scopes, ['dedalus:cli', 'offline_access']); +}); + +for (const invalid of [false, true]) { + test(`browser login ${invalid ? 'rejects another issuer before exchange' : 'persists, refreshes and forgets credentials'}`, { + skip: process.platform === 'win32', timeout: 15000, + }, async (t) => { + const directory = mkdtempSync(join(tmpdir(), 'dedalus-browser-test-')); + t.after(() => rmSync(directory, { recursive: true, force: true })); + // Capture the printed URL instead of opening a real account in the test. + for (const opener of ['open', 'xdg-open']) { + writeFileSync(join(directory, opener), '#!/bin/sh\nexit 0\n', { mode: 0o700 }); + } + let authorization; + const exchanges = []; + const requests = []; + const server = createServer(async (req, res) => { + if (req.url.startsWith('/oauth2/auth?')) { + authorization = new URL(req.url, origin).searchParams; + const callback = new URL(authorization.get('redirect_uri')); + callback.searchParams.set('code', 'test-code'); + callback.searchParams.set('state', authorization.get('state')); + callback.searchParams.set('iss', invalid ? 'https://other.invalid' : origin); + res.writeHead(302, { location: callback.href }).end(); + } else if (req.url === '/oauth2/token') { + let body = ''; + for await (const chunk of req) body += chunk; + const form = new URLSearchParams(body); + exchanges.push(form); + res.writeHead(200, { 'content-type': 'application/json' }).end(JSON.stringify({ + access_token: exchanges.length === 1 ? 'test-access' : 'test-refreshed', + refresh_token: exchanges.length === 1 ? 'test-refresh' : 'test-rotated', + token_type: 'Bearer', expires_in: 3600, + })); + } else { + requests.push(req.headers.authorization); + res.writeHead(200, { 'content-type': 'application/json' }) + .end(JSON.stringify({ items: [], next_cursor: null })); + } + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + t.after(() => { server.closeAllConnections(); server.close(); }); + const origin = 'http://127.0.0.1:' + server.address().port; + const store = join(directory, 'credentials.json'); + const env = { ...process.env, PATH: directory + ':' + process.env.PATH, + DEDALUS_CREDENTIALS_FILE: store, TEST_API_ORIGIN: origin }; + for (const key of ['DEDALUS_API_KEY', 'DEDALUS_X_API_KEY', 'DEDALUS_BASE_URL', 'DEDALUS_CUSTOM_HEADERS']) delete env[key]; + const run = (args, browser = false) => new Promise((resolve, reject) => { + const child = spawn(process.execPath, ['--input-type=module', '-e', ` + import { auth, getProgram } from './dist/esm/commands/index.js'; + const origin = process.env.TEST_API_ORIGIN; + auth.backend = 'file'; + auth.defaultBaseUrl = origin; + Object.assign(auth.methods[0], { resource: origin, issuer: origin, + authorizationUrl: origin + '/oauth2/auth', tokenUrl: origin + '/oauth2/token', + refreshUrl: origin + '/oauth2/token' }); + await getProgram().parseAsync(JSON.parse(process.env.TEST_ARGS), { from: 'user' }); + `], { env: { ...env, TEST_ARGS: JSON.stringify([...args, '--base-url', origin]) }, stdio: ['ignore', 'pipe', 'pipe'] }); + t.after(() => { if (child.exitCode === null) child.kill(); }); + let stdout = '', stderr = '', opened = false; + child.stdout.on('data', (chunk) => { stdout += chunk; }); + child.stderr.on('data', (chunk) => { + stderr += chunk; + const match = stderr.match(/ (http:\/\/127\.0\.0\.1:[^\s]+)/); + if (browser && match && !opened) { + opened = true; + fetch(match[1]).then((response) => response.text()).catch(reject); + } + }); + child.on('error', reject); + child.on('exit', (code) => resolve({ code, stdout, stderr })); + }); + const login = await run(['login', '--flow', 'browser'], true); + if (invalid) { + assert.notEqual(login.code, 0); + assert.equal(exchanges.length, 0); + assert.throws(() => readFileSync(store), { code: 'ENOENT' }); + return; + } + assert.equal(login.code, 0, login.stderr); + assert.equal(authorization.size, 8); + assert.equal(authorization.get('resource'), origin); + assert.equal(authorization.get('client_id'), 'dedalus-cli'); + assert.equal(authorization.get('scope'), 'dedalus:cli offline_access'); + assert.equal(Buffer.from(authorization.get('state'), 'base64url').length, 32); + assert.equal(authorization.get('code_challenge_method'), 'S256'); + assert.equal(createHash('sha256').update(exchanges[0].get('code_verifier')).digest('base64url'), + authorization.get('code_challenge')); + assert.equal(exchanges[0].get('redirect_uri'), authorization.get('redirect_uri')); + assert.equal(statSync(store).mode & 0o777, 0o600); + assert.equal((await run(['machines', 'list'])).code, 0); + assert.equal(requests.at(-1), 'Bearer test-access'); + const saved = JSON.parse(readFileSync(store, 'utf8')); + saved.profiles[origin].oauth.apiKey.expiresAt = 0; + writeFileSync(store, JSON.stringify(saved)); + assert.equal((await run(['machines', 'list'])).code, 0); + assert.equal(exchanges[1].get('grant_type'), 'refresh_token'); + assert.equal(exchanges[1].get('refresh_token'), 'test-refresh'); + assert.equal(requests.at(-1), 'Bearer test-refreshed'); + assert.equal(JSON.parse(readFileSync(store, 'utf8')).profiles[origin].oauth.apiKey.refreshToken, 'test-rotated'); + assert.equal((await run(['machines', 'list', '--api-key', 'explicit'])).code, 0); + assert.equal(requests.at(-1), 'Bearer explicit'); + assert.equal((await run(['logout'])).code, 0); + assert.equal(JSON.parse(readFileSync(store, 'utf8')).profiles[origin], undefined); + assert.equal(login.stdout.includes('test-access'), false); + }); +} From c4507a059b832ce300c1b2dfae1b42aada491ae8 Mon Sep 17 00:00:00 2001 From: Shengming Liang Date: Tue, 29 Sep 2026 12:47:19 -0700 Subject: [PATCH 2/3] fix(auth): revoke saved browser sessions on logout Signed-off-by: Shengming Liang --- README.md | 35 ++++++++------ src/cli/credentials.ts | 39 +++++++++++++-- src/cli/login.ts | 93 ++++++++++++++++++++++++++++-------- src/commands/index.ts | 12 +++-- tests/browser-login.test.mjs | 89 ++++++++++++++++++++++++++++++---- 5 files changed, 215 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index 2ee823b..79aaffd 100644 --- a/README.md +++ b/README.md @@ -68,21 +68,26 @@ See the [API reference](./api.md) for every available operation. ## Signing In -Run `dedalus login --flow browser` to sign in through your browser. The CLI uses -PKCE, verifies the callback state and issuer, and saves the access and refresh -tokens for subsequent commands. It refreshes expiring access tokens automatically. -Browser sign-in is bound to `https://dcs.dedaluslabs.ai`; an alternate API base URL -cannot receive this session. The interactive `dedalus login` menu also offers -browser sign-in and manual credentials. - -Browser sign-in requires the public `/oauth2/auth` and `/oauth2/token` endpoints -and a registered `dedalus-cli` public client supporting loopback callbacks. -Until that server support is deployed, use an API key. Local protocol tests do -not establish that hosted account sign-in is available. - -`dedalus logout` removes local credentials; it does not revoke tokens on the -server or sign out your browser. It does not read or remove sessions from the -older `dedalus auth login` implementation. + +Browser sign-in is not enabled in the public package until its provider and +client registration are verified. `dedalus login --flow browser` fails before +opening a browser when that configuration is absent. Use an API key meanwhile. +A configured provider must supply its exact callback issuer, authorization, +token, refresh, and revocation endpoints, client ID, scopes, and API resource. +The API resource must match the command's base URL. + +The configured browser flow uses PKCE, verifies callback state and issuer, and +saves access and refresh tokens. Token refresh retains the revocation endpoint. +`dedalus logout` revokes the refresh token and access token before deleting the +local profile. A failed revocation retains credentials and exits unsuccessfully +so logout can be retried. `--all` applies this to each saved profile. Manually +entered credentials are removed locally, without attempting OAuth revocation. +Sessions saved before revocation metadata was added require provider-side +revocation. Logout refuses to guess their provider endpoint. + +Logout does not sign out the browser or read sessions from the older +`dedalus auth login` implementation. Local tests do not prove hosted readiness. + `dedalus login` signs you in and saves the credential for later commands, so it does not have to be passed every time. It goes into your operating system's credential store — the system keyring on Linux, Credential Manager on Windows — and falls back to a file in your state directory, readable only by you, when no such store is available. On macOS it is always that file, because the system's own tool accepts a password only on its command line, where other processes could read it. Either way it is filed under the base URL it was captured for, so a credential saved for one host is never sent to another. `dedalus logout` forgets it. A credential passed with a flag, or set in the environment, still takes precedence over a saved one. Sign-in methods: browser, api-key, x-api-key. Pass `--flow ` to pick one without being asked. diff --git a/src/cli/credentials.ts b/src/cli/credentials.ts index c0cef08..9c67b79 100644 --- a/src/cli/credentials.ts +++ b/src/cli/credentials.ts @@ -32,6 +32,10 @@ export type CredentialStoreLocation = { /** Refresh metadata kept beside a stored OAuth access token. */ export type StoredOAuth = { + // scalar-sdk-generator:custom-code revocation-metadata:start + // provider endpoint retained for logout + readonly revocationUrl?: string; + // scalar-sdk-generator:custom-code revocation-metadata:end readonly refreshToken?: string; /** Unix epoch milliseconds the access token stops being usable, when the server said so. */ readonly expiresAt?: number; @@ -103,23 +107,27 @@ const stateDirectory = (): string => { * the CLI: the worst outcome of ignoring it is one `login` the user has to run again, whereas * throwing would take out commands that were never going to need a stored credential at all. */ -export const readStore = (location: CredentialStoreLocation): CredentialStoreFile => { +// scalar-sdk-generator:custom-code strict-credential-store:start +export const readStore = (location: CredentialStoreLocation, strict = false): CredentialStoreFile => { let text: string; try { text = readFileSync(storePath(location), 'utf8'); - } catch { + } catch (error) { + if (strict && (error as NodeJS.ErrnoException).code !== 'ENOENT') throw new Error('Could not read the credential store.'); return EMPTY_STORE; } try { const parsed: unknown = JSON.parse(text); - if (!parsed || typeof parsed !== 'object') return EMPTY_STORE; + if (!parsed || typeof parsed !== 'object') throw new Error('Invalid credential store.'); const profiles = (parsed as { profiles?: unknown }).profiles; - if (!profiles || typeof profiles !== 'object') return EMPTY_STORE; + if (!profiles || typeof profiles !== 'object' || Array.isArray(profiles)) throw new Error('Invalid credential store.'); return { version: STORE_VERSION, profiles: profiles as Record }; } catch { + if (strict) throw new Error('Could not parse the credential store. Credentials retained.'); return EMPTY_STORE; } }; +// scalar-sdk-generator:custom-code strict-credential-store:end /** * Replaces the store file. @@ -354,6 +362,20 @@ const osStoreName = (): string => { export const readProfile = (location: CredentialStoreLocation, key: string): StoredProfile => readProfileResult(location, key).profile; +// scalar-sdk-generator:custom-code logout-profile-read:start + +// an unreadable secret cannot be acknowledged as logged out +export const readProfileForLogout = (location: CredentialStoreLocation, key: string): StoredProfile => { + const entry = readStore(location, true).profiles[key]; + if (entry && (entry.backend === 'keychain' ? !mayUseKeychain(location) : location.backend === 'keychain')) { + throw new Error('Use the credential backend that stored this session before logging out.'); + } + const result = readProfileResult(location, key); + if (result.unreadable || (entry?.backend === 'keychain' && !result.profile.credentials)) throw new Error('Could not read credentials for revocation. Retry logout when the credential store is available.'); + return result.profile; +}; +// scalar-sdk-generator:custom-code logout-profile-read:end + /** * A profile read, and whether the store actually answered. * @@ -561,8 +583,13 @@ const withoutMarker = (profile: StoredProfile): StoredProfile => ({ * entry while the secret stays behind would leave a credential that still authenticates every * command — and that `logout --all`, which enumerates the index, could no longer even find. */ -export const deleteProfile = (location: CredentialStoreLocation, key: string): 'removed' | 'absent' => +// scalar-sdk-generator:custom-code conditional-credential-deletion:start +// refuse deletion if login or refresh replaced the revoked profile +export const deleteProfile = (location: CredentialStoreLocation, key: string, expected?: StoredProfile): 'removed' | 'absent' => withStoreLock(location, () => { + if (expected && JSON.stringify(readProfileForLogout(location, key)) !== JSON.stringify(expected)) { + throw new Error('Credentials changed during logout. Retry to revoke the current session.'); + } const entry = indexEntry(location, key); if (entry.backend === 'keychain') { const outcome = removeSecret(location, key); @@ -573,6 +600,7 @@ export const deleteProfile = (location: CredentialStoreLocation, key: string): ' throw new Error('Could not remove the credential from your ' + osStoreName() + '.'); } if (outcome === 'unconfirmed') { + if (expected) throw new Error('Could not confirm credential deletion. Retry logout.'); warn('Your ' + osStoreName() + ' may still hold a credential for this host; remove it there if so.'); } } @@ -583,6 +611,7 @@ export const deleteProfile = (location: CredentialStoreLocation, key: string): ' writeStore(location, { version: STORE_VERSION, profiles }); return 'removed'; }); +// scalar-sdk-generator:custom-code conditional-credential-deletion:end /** * Forgets everything, in both places. diff --git a/src/cli/login.ts b/src/cli/login.ts index 9e8319d..62ce459 100644 --- a/src/cli/login.ts +++ b/src/cli/login.ts @@ -10,10 +10,13 @@ import { type CredentialStoreLocation, type StoredOAuth, type StoredProfile, - clearAll, deleteProfile, profileKey, readProfile, + // scalar-sdk-generator:custom-code logout-imports:start + readProfileForLogout, + readStore, + // scalar-sdk-generator:custom-code logout-imports:end refreshProfile, storeDescription, writeProfile, @@ -30,8 +33,12 @@ export type CliAuthMethodDefinition = { | { readonly kind: 'oauth'; readonly grant: 'authorizationCode'; + // scalar-sdk-generator:custom-code oauth-provider-contract:start + // resource-bound provider configuration readonly resource?: string; readonly issuer?: string; + readonly revocationUrl?: string; + // scalar-sdk-generator:custom-code oauth-provider-contract:end readonly clientKey: string; readonly tokenUrl: string; readonly refreshUrl: string; @@ -238,18 +245,39 @@ export const runLogin = async ( return outstanding ? line + '\n' + outstanding : line; }; -/** Forgets stored credentials, returning the line to print. */ -export const runLogout = (auth: CliAuthDefinition, baseUrl: string, all: boolean): string => { +// scalar-sdk-generator:custom-code oauth-logout:start + +// revoke OAuth grants before deleting credentials +/** Revokes stored OAuth tokens before removing their local profile. */ +export const runLogout = async (auth: CliAuthDefinition, baseUrl: string, all: boolean): Promise => { const location = storeLocation(auth); - if (all) { - clearAll(location); - return 'Signed out everywhere.'; + const keys = all ? Object.keys(readStore(location, true).profiles) : [profileKey(baseUrl)]; + let removed = false; + for (const key of keys) { + const profile = readProfileForLogout(location, key); + for (const [clientKey, meta] of Object.entries(profile.oauth ?? {})) { + if (!meta?.revocationUrl) { + throw new Error('Cannot revoke this saved OAuth session: revocation configuration is missing. Credentials retained.'); + } + const url = requireSecureUrl(meta.revocationUrl, '', 'revocation endpoint'); + for (const [hint, token] of [ + ['refresh_token', meta.refreshToken], ['access_token', profile.credentials?.[clientKey]], + ] as const) { + if (!token) continue; + const { response } = await postForToken(url, { + token, token_type_hint: hint, + }).catch(() => { throw new Error('Could not revoke the OAuth session. Credentials retained; retry logout.'); }); + if (response.status !== 200) { + throw new Error('Could not revoke the OAuth session (HTTP ' + response.status + '). Credentials retained; retry logout.'); + } + } + } + removed = deleteProfile(location, key, profile) === 'removed' || removed; } - const key = profileKey(baseUrl); - const outcome = deleteProfile(location, key); - if (outcome === 'removed') return 'Signed out of ' + safeText(baseUrl || 'the API') + '.'; - return 'No stored credentials for ' + safeText(baseUrl || 'the API') + '.'; + if (all) return 'Signed out of all saved profiles.'; + return (removed ? 'Signed out of ' : 'No stored credentials for ') + safeText(baseUrl || 'the API') + '.'; }; +// scalar-sdk-generator:custom-code oauth-logout:end /** * Stored credentials for one base URL, keyed by client-option name. @@ -299,7 +327,11 @@ export const storedCredentials = async ( // must not be undone by the token it produced. See `refreshProfile`. refreshProfile(location, key, { credentials: { [clientKey]: token.accessToken }, - oauth: { [clientKey]: oauthMetadata(token, refreshUrl, meta.clientId, meta.refreshToken) }, + // scalar-sdk-generator:custom-code refresh-revocation-metadata:start + // refresh retains revocation configuration + oauth: { [clientKey]: { ...oauthMetadata(token, refreshUrl, meta.clientId, meta.refreshToken), + ...(meta.revocationUrl ? { revocationUrl: meta.revocationUrl } : {}) } }, + // scalar-sdk-generator:custom-code refresh-revocation-metadata:end }); } catch { // Deliberately silent; see this function's doc comment. @@ -360,7 +392,13 @@ const captureCredentials = async ( const { token, clientId, refreshUrl } = await runOauthFlow(method, baseUrl); return { credentials: { [method.clientKey]: token.accessToken }, - oauth: { [method.clientKey]: oauthMetadata(token, refreshUrl, clientId, undefined) }, + // scalar-sdk-generator:custom-code login-revocation-metadata:start + // retain the provider revocation endpoint with the grant + oauth: { [method.clientKey]: { + ...oauthMetadata(token, refreshUrl, clientId, undefined), + ...(method.grant === 'authorizationCode' && method.revocationUrl ? { revocationUrl: method.revocationUrl } : {}), + } }, + // scalar-sdk-generator:custom-code login-revocation-metadata:end }; }; @@ -684,17 +722,26 @@ const authorizationCodeFlow = async ( baseUrl: string, ): Promise<{ readonly token: TokenResponse; readonly clientId: string | undefined }> => { const clientId = method.clientId; - if (!clientId || !method.authorizationUrl) throw new Error('This flow needs a configured OAuth client id.'); - const authorizeUrl = requireSecureUrl(method.authorizationUrl, baseUrl, 'authorization endpoint'); - if (method.resource) { - if (profileKey(baseUrl) !== profileKey(method.resource)) { - throw new UsageError('Browser sign-in is unavailable for this API base URL.'); - } - authorizeUrl.searchParams.set('resource', method.resource); + // scalar-sdk-generator:custom-code configured-browser-login:start + // public provider settings must be verified before enabling login + if (method.resource && profileKey(baseUrl) !== profileKey(method.resource)) { + throw new UsageError('Browser sign-in is unavailable for this API base URL.'); } + if (!clientId || !method.authorizationUrl || !method.tokenUrl || (method.resource && (!method.issuer || !method.revocationUrl))) { + throw new UsageError('Browser sign-in is not configured for this release. Use an API key until the provider rollout is complete.'); + } + // scalar-sdk-generator:custom-code configured-browser-login:end + const authorizeUrl = requireSecureUrl(method.authorizationUrl, baseUrl, 'authorization endpoint'); + // scalar-sdk-generator:custom-code oauth-resource:start + // RFC 8707 resource binding + if (method.resource) authorizeUrl.searchParams.set('resource', method.resource); + // scalar-sdk-generator:custom-code oauth-resource:end const verifier = base64Url(randomBytes(32)); const challenge = base64Url(createHash('sha256').update(verifier).digest()); + // scalar-sdk-generator:custom-code oauth-state:start + // 256-bit callback state const state = base64Url(randomBytes(32)); + // scalar-sdk-generator:custom-code oauth-state:end const server = createServer(); // Bound first, and only then given its request handler. Registering the handler earlier meant a @@ -716,7 +763,10 @@ const authorizationCodeFlow = async ( } throw error; }); + // scalar-sdk-generator:custom-code callback-issuer:start + // RFC 9207 issuer binding const redirect = awaitRedirect(server, state, REDIRECT_PATH, method.issuer); + // scalar-sdk-generator:custom-code callback-issuer:end const redirectUri = 'http://127.0.0.1:' + String(port) + REDIRECT_PATH; try { authorizeUrl.searchParams.set('response_type', 'code'); @@ -874,7 +924,10 @@ const awaitRedirect = ( server: ReturnType, state: string, path: string, + // scalar-sdk-generator:custom-code callback-issuer-parameter:start + // RFC 9207 issuer binding issuer?: string, + // scalar-sdk-generator:custom-code callback-issuer-parameter:end ): Promise => new Promise((resolve, reject) => { server.on('request', (request, response) => { @@ -899,6 +952,7 @@ const awaitRedirect = ( send(404, 'text/plain', 'Not found.\n'); return; } + // scalar-sdk-generator:custom-code callback-verification:start if (params.getAll('state').length !== 1 || !sameToken(params.get('state') ?? '', state) || (issuer !== undefined && (params.getAll('iss').length !== 1 || params.get('iss') !== issuer))) { // Ends the sign-in rather than waiting for a better redirect. A request that reaches here is @@ -916,6 +970,7 @@ const awaitRedirect = ( reject(new Error('The browser redirect did not match this sign-in attempt.')); return; } + // scalar-sdk-generator:custom-code callback-verification:end const failure = params.get('error'); if (failure) { finish( diff --git a/src/commands/index.ts b/src/commands/index.ts index 462c2eb..529303d 100644 --- a/src/commands/index.ts +++ b/src/commands/index.ts @@ -856,6 +856,8 @@ export const auth = { xAPIKey: 'DEDALUS_X_API_KEY', }, methods: [ + // scalar-sdk-generator:custom-code configured-browser-method:start + // configured browser login pending the public provider rollout { name: 'browser', label: 'Sign in with your browser', @@ -863,14 +865,16 @@ export const auth = { grant: 'authorizationCode', clientKey: 'apiKey', clientId: 'dedalus-cli', - authorizationUrl: 'https://dcs.dedaluslabs.ai/oauth2/auth', - tokenUrl: 'https://dcs.dedaluslabs.ai/oauth2/token', - refreshUrl: 'https://dcs.dedaluslabs.ai/oauth2/token', + authorizationUrl: '', + tokenUrl: '', + refreshUrl: '', resource: 'https://dcs.dedaluslabs.ai', - issuer: 'https://dcs.dedaluslabs.ai', + issuer: '', + revocationUrl: '', scopes: ['dedalus:cli', 'offline_access'], redirectPort: 0, }, + // scalar-sdk-generator:custom-code configured-browser-method:end { name: 'api-key', label: 'Enter your access token', diff --git a/tests/browser-login.test.mjs b/tests/browser-login.test.mjs index fbef463..99acd9d 100644 --- a/tests/browser-login.test.mjs +++ b/tests/browser-login.test.mjs @@ -13,18 +13,13 @@ test('browser login refuses to store a public session for another API', async () await assert.rejects(runLogin(auth, 'https://other.invalid', 'browser'), /unavailable for this API/); }); -test('browser configuration uses the public native client contract', () => { - const method = auth.methods.find((entry) => entry.name === 'browser'); - assert.equal(method.clientId, 'dedalus-cli'); - assert.equal(method.resource, auth.defaultBaseUrl); - assert.equal(method.issuer, auth.defaultBaseUrl); - assert.equal(method.authorizationUrl, auth.defaultBaseUrl + '/oauth2/auth'); - assert.equal(method.tokenUrl, auth.defaultBaseUrl + '/oauth2/token'); - assert.deepEqual(method.scopes, ['dedalus:cli', 'offline_access']); +test('invariant_unconfigured_public_browser_login_fails_before_opening_a_browser', async () => { + assert.equal(auth.methods[0].issuer, ''); + await assert.rejects(runLogin(auth, auth.defaultBaseUrl, 'browser'), /not configured/); }); for (const invalid of [false, true]) { - test(`browser login ${invalid ? 'rejects another issuer before exchange' : 'persists, refreshes and forgets credentials'}`, { + test(`browser login ${invalid ? 'rejects another issuer before exchange' : 'persists, refreshes and revokes credentials'}`, { skip: process.platform === 'win32', timeout: 15000, }, async (t) => { const directory = mkdtempSync(join(tmpdir(), 'dedalus-browser-test-')); @@ -36,6 +31,8 @@ for (const invalid of [false, true]) { let authorization; const exchanges = []; const requests = []; + const revocations = []; + let rejectRevocation = true; const server = createServer(async (req, res) => { if (req.url.startsWith('/oauth2/auth?')) { authorization = new URL(req.url, origin).searchParams; @@ -44,6 +41,12 @@ for (const invalid of [false, true]) { callback.searchParams.set('state', authorization.get('state')); callback.searchParams.set('iss', invalid ? 'https://other.invalid' : origin); res.writeHead(302, { location: callback.href }).end(); + } else if (req.url === '/oauth2/revoke') { + let body = ''; + for await (const chunk of req) body += chunk; + const form = new URLSearchParams(body); + revocations.push(Object.fromEntries(form)); + res.writeHead(rejectRevocation ? 503 : 200).end(); } else if (req.url === '/oauth2/token') { let body = ''; for await (const chunk of req) body += chunk; @@ -75,7 +78,7 @@ for (const invalid of [false, true]) { auth.defaultBaseUrl = origin; Object.assign(auth.methods[0], { resource: origin, issuer: origin, authorizationUrl: origin + '/oauth2/auth', tokenUrl: origin + '/oauth2/token', - refreshUrl: origin + '/oauth2/token' }); + refreshUrl: origin + '/oauth2/token', revocationUrl: origin + '/oauth2/revoke' }); await getProgram().parseAsync(JSON.parse(process.env.TEST_ARGS), { from: 'user' }); `], { env: { ...env, TEST_ARGS: JSON.stringify([...args, '--base-url', origin]) }, stdio: ['ignore', 'pipe', 'pipe'] }); t.after(() => { if (child.exitCode === null) child.kill(); }); @@ -122,8 +125,74 @@ for (const invalid of [false, true]) { assert.equal(JSON.parse(readFileSync(store, 'utf8')).profiles[origin].oauth.apiKey.refreshToken, 'test-rotated'); assert.equal((await run(['machines', 'list', '--api-key', 'explicit'])).code, 0); assert.equal(requests.at(-1), 'Bearer explicit'); + const rejected = await run(['logout']); + assert.notEqual(rejected.code, 0); + assert.match(rejected.stderr, /revok/i); + assert.equal(JSON.parse(readFileSync(store, 'utf8')).profiles[origin].oauth.apiKey.refreshToken, 'test-rotated'); + rejectRevocation = false; + revocations.length = 0; assert.equal((await run(['logout'])).code, 0); + assert.deepEqual(revocations.map(({ token, token_type_hint }) => [token, token_type_hint]), [ + ['test-rotated', 'refresh_token'], ['test-refreshed', 'access_token'], + ]); assert.equal(JSON.parse(readFileSync(store, 'utf8')).profiles[origin], undefined); assert.equal(login.stdout.includes('test-access'), false); }); } + +test('invariant_logout_all_preserves_unrevoked_or_replaced_credentials', async (t) => { + const { runLogout } = await import('../dist/esm/cli/login.js'); + const directory = mkdtempSync(join(tmpdir(), 'dedalus-logout-test-')); + const store = join(directory, 'credentials.json'); + const variable = 'DEDALUS_LOGOUT_TEST_STORE'; + process.env[variable] = store; + t.after(() => { delete process.env[variable]; rmSync(directory, { recursive: true, force: true }); }); + const config = { ...auth, backend: 'file', storeEnv: variable }; + let failure, replace = false; + const requests = []; + const server = createServer(async (req, res) => { + let body = ''; + for await (const chunk of req) body += chunk; + const form = new URLSearchParams(body); + requests.push(form.get('token')); + if (replace) { + const data = JSON.parse(readFileSync(store, 'utf8')); + data.profiles[origin].credentials.apiKey = 'new-access'; + writeFileSync(store, JSON.stringify(data)); + } + res.writeHead(form.get('token_type_hint') === failure ? 503 : 200).end(); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + t.after(() => { server.closeAllConnections(); server.close(); }); + const origin = 'http://127.0.0.1:' + server.address().port; + const oauth = { credentials: { apiKey: 'access' }, oauth: { apiKey: { + refreshToken: 'refresh', clientId: 'client', revocationUrl: origin + '/revoke', + } } }; + const save = (profile = oauth) => writeFileSync(store, JSON.stringify({ version: 1, profiles: { + [origin]: profile, 'https://manual.example': { credentials: { apiKey: 'manual' } }, + } })); + for (failure of ['refresh_token', 'access_token']) { + save(); + await assert.rejects(runLogout(config, origin, true), /Credentials retained/); + assert.deepEqual(JSON.parse(readFileSync(store, 'utf8')).profiles[origin], oauth); + } + failure = undefined; + save(); + replace = true; + await assert.rejects(runLogout(config, origin, false), /Credentials changed/); + assert.equal(JSON.parse(readFileSync(store, 'utf8')).profiles[origin].credentials.apiKey, 'new-access'); + replace = false; + const legacy = structuredClone(oauth); + delete legacy.oauth.apiKey.revocationUrl; + save(legacy); + requests.length = 0; + await assert.rejects(runLogout(config, origin, true), /configuration is missing/); + assert.equal(requests.length, 0); + writeFileSync(store, '{corrupt'); + await assert.rejects(runLogout(config, origin, true), /parse the credential store/); + assert.equal(readFileSync(store, 'utf8'), '{corrupt'); + save(); + await runLogout(config, origin, true); + assert.deepEqual(requests, ['refresh', 'access']); + assert.deepEqual(JSON.parse(readFileSync(store, 'utf8')).profiles, {}); +}); From 6ad1f2748968d4ff3efc07c7217cec9e50e8a073 Mon Sep 17 00:00:00 2001 From: Shengming Liang Date: Tue, 29 Sep 2026 13:13:40 -0700 Subject: [PATCH 3/3] fix(auth): allow local logout when revocation fails Signed-off-by: Shengming Liang --- README.md | 15 +++--- src/cli/credentials.ts | 37 +++------------ src/cli/login.ts | 51 +++++++++++++-------- tests/browser-login.test.mjs | 88 ++++++++++++++++++++---------------- 4 files changed, 95 insertions(+), 96 deletions(-) diff --git a/README.md b/README.md index 79aaffd..817837e 100644 --- a/README.md +++ b/README.md @@ -78,12 +78,15 @@ The API resource must match the command's base URL. The configured browser flow uses PKCE, verifies callback state and issuer, and saves access and refresh tokens. Token refresh retains the revocation endpoint. -`dedalus logout` revokes the refresh token and access token before deleting the -local profile. A failed revocation retains credentials and exits unsuccessfully -so logout can be retried. `--all` applies this to each saved profile. Manually -entered credentials are removed locally, without attempting OAuth revocation. -Sessions saved before revocation metadata was added require provider-side -revocation. Logout refuses to guess their provider endpoint. +`dedalus logout` attempts to revoke the refresh token, or the access token when +no refresh token exists, with a 10-second timeout per request. It then removes +local credentials even if revocation fails or the device is offline. A warning +means the server session may still be active; local deletion does not confirm +server-side revocation. Local credential deletion failures still fail logout. +`--all` applies this to all saved profiles. Manually entered credentials are +removed locally without OAuth revocation. Older sessions without a saved +revocation endpoint are cleared locally with a warning; the CLI does not guess +their provider endpoint. Logout does not sign out the browser or read sessions from the older `dedalus auth login` implementation. Local tests do not prove hosted readiness. diff --git a/src/cli/credentials.ts b/src/cli/credentials.ts index 9c67b79..4edf3f3 100644 --- a/src/cli/credentials.ts +++ b/src/cli/credentials.ts @@ -33,7 +33,7 @@ export type CredentialStoreLocation = { /** Refresh metadata kept beside a stored OAuth access token. */ export type StoredOAuth = { // scalar-sdk-generator:custom-code revocation-metadata:start - // provider endpoint retained for logout + /** Provider endpoint used for best-effort revocation on logout. */ readonly revocationUrl?: string; // scalar-sdk-generator:custom-code revocation-metadata:end readonly refreshToken?: string; @@ -107,27 +107,23 @@ const stateDirectory = (): string => { * the CLI: the worst outcome of ignoring it is one `login` the user has to run again, whereas * throwing would take out commands that were never going to need a stored credential at all. */ -// scalar-sdk-generator:custom-code strict-credential-store:start -export const readStore = (location: CredentialStoreLocation, strict = false): CredentialStoreFile => { +export const readStore = (location: CredentialStoreLocation): CredentialStoreFile => { let text: string; try { text = readFileSync(storePath(location), 'utf8'); - } catch (error) { - if (strict && (error as NodeJS.ErrnoException).code !== 'ENOENT') throw new Error('Could not read the credential store.'); + } catch { return EMPTY_STORE; } try { const parsed: unknown = JSON.parse(text); - if (!parsed || typeof parsed !== 'object') throw new Error('Invalid credential store.'); + if (!parsed || typeof parsed !== 'object') return EMPTY_STORE; const profiles = (parsed as { profiles?: unknown }).profiles; - if (!profiles || typeof profiles !== 'object' || Array.isArray(profiles)) throw new Error('Invalid credential store.'); + if (!profiles || typeof profiles !== 'object') return EMPTY_STORE; return { version: STORE_VERSION, profiles: profiles as Record }; } catch { - if (strict) throw new Error('Could not parse the credential store. Credentials retained.'); return EMPTY_STORE; } }; -// scalar-sdk-generator:custom-code strict-credential-store:end /** * Replaces the store file. @@ -362,20 +358,6 @@ const osStoreName = (): string => { export const readProfile = (location: CredentialStoreLocation, key: string): StoredProfile => readProfileResult(location, key).profile; -// scalar-sdk-generator:custom-code logout-profile-read:start - -// an unreadable secret cannot be acknowledged as logged out -export const readProfileForLogout = (location: CredentialStoreLocation, key: string): StoredProfile => { - const entry = readStore(location, true).profiles[key]; - if (entry && (entry.backend === 'keychain' ? !mayUseKeychain(location) : location.backend === 'keychain')) { - throw new Error('Use the credential backend that stored this session before logging out.'); - } - const result = readProfileResult(location, key); - if (result.unreadable || (entry?.backend === 'keychain' && !result.profile.credentials)) throw new Error('Could not read credentials for revocation. Retry logout when the credential store is available.'); - return result.profile; -}; -// scalar-sdk-generator:custom-code logout-profile-read:end - /** * A profile read, and whether the store actually answered. * @@ -583,13 +565,8 @@ const withoutMarker = (profile: StoredProfile): StoredProfile => ({ * entry while the secret stays behind would leave a credential that still authenticates every * command — and that `logout --all`, which enumerates the index, could no longer even find. */ -// scalar-sdk-generator:custom-code conditional-credential-deletion:start -// refuse deletion if login or refresh replaced the revoked profile -export const deleteProfile = (location: CredentialStoreLocation, key: string, expected?: StoredProfile): 'removed' | 'absent' => +export const deleteProfile = (location: CredentialStoreLocation, key: string): 'removed' | 'absent' => withStoreLock(location, () => { - if (expected && JSON.stringify(readProfileForLogout(location, key)) !== JSON.stringify(expected)) { - throw new Error('Credentials changed during logout. Retry to revoke the current session.'); - } const entry = indexEntry(location, key); if (entry.backend === 'keychain') { const outcome = removeSecret(location, key); @@ -600,7 +577,6 @@ export const deleteProfile = (location: CredentialStoreLocation, key: string, ex throw new Error('Could not remove the credential from your ' + osStoreName() + '.'); } if (outcome === 'unconfirmed') { - if (expected) throw new Error('Could not confirm credential deletion. Retry logout.'); warn('Your ' + osStoreName() + ' may still hold a credential for this host; remove it there if so.'); } } @@ -611,7 +587,6 @@ export const deleteProfile = (location: CredentialStoreLocation, key: string, ex writeStore(location, { version: STORE_VERSION, profiles }); return 'removed'; }); -// scalar-sdk-generator:custom-code conditional-credential-deletion:end /** * Forgets everything, in both places. diff --git a/src/cli/login.ts b/src/cli/login.ts index 62ce459..92a8200 100644 --- a/src/cli/login.ts +++ b/src/cli/login.ts @@ -14,7 +14,7 @@ import { profileKey, readProfile, // scalar-sdk-generator:custom-code logout-imports:start - readProfileForLogout, + clearAll, readStore, // scalar-sdk-generator:custom-code logout-imports:end refreshProfile, @@ -247,34 +247,45 @@ export const runLogin = async ( // scalar-sdk-generator:custom-code oauth-logout:start -// revoke OAuth grants before deleting credentials -/** Revokes stored OAuth tokens before removing their local profile. */ +/** Attempts provider revocation, then clears local credentials even when offline. */ export const runLogout = async (auth: CliAuthDefinition, baseUrl: string, all: boolean): Promise => { const location = storeLocation(auth); - const keys = all ? Object.keys(readStore(location, true).profiles) : [profileKey(baseUrl)]; - let removed = false; + const keys = all ? Object.keys(readStore(location).profiles) : [profileKey(baseUrl)]; + const warnRevocation = () => processStderr.write( + 'Warning: OAuth revocation could not be confirmed. Clearing local credentials; the server session may remain active.\n', + ); for (const key of keys) { - const profile = readProfileForLogout(location, key); + let profile: StoredProfile; + try { + profile = readProfile(location, key); + } catch { + warnRevocation(); + continue; + } for (const [clientKey, meta] of Object.entries(profile.oauth ?? {})) { - if (!meta?.revocationUrl) { - throw new Error('Cannot revoke this saved OAuth session: revocation configuration is missing. Credentials retained.'); - } - const url = requireSecureUrl(meta.revocationUrl, '', 'revocation endpoint'); - for (const [hint, token] of [ - ['refresh_token', meta.refreshToken], ['access_token', profile.credentials?.[clientKey]], - ] as const) { + try { + // Match Codex: prefer refresh revocation; use access only when no refresh token exists. + const token = meta?.refreshToken || profile.credentials?.[clientKey]; if (!token) continue; + if (!meta?.revocationUrl) throw new Error('Missing revocation endpoint.'); + const url = requireSecureUrl(meta.revocationUrl, '', 'revocation endpoint'); const { response } = await postForToken(url, { - token, token_type_hint: hint, - }).catch(() => { throw new Error('Could not revoke the OAuth session. Credentials retained; retry logout.'); }); - if (response.status !== 200) { - throw new Error('Could not revoke the OAuth session (HTTP ' + response.status + '). Credentials retained; retry logout.'); - } + token, + token_type_hint: meta.refreshToken ? 'refresh_token' : 'access_token', + }, 10_000); + if (!response.ok) throw new Error('Revocation failed.'); + } catch { + // Never print provider bodies or request errors: they can contain credentials. + warnRevocation(); } } - removed = deleteProfile(location, key, profile) === 'removed' || removed; } - if (all) return 'Signed out of all saved profiles.'; + // Local deletion is independent of the network result; deletion errors still propagate. + if (all) { + clearAll(location); + return 'Signed out of all saved profiles.'; + } + const removed = deleteProfile(location, profileKey(baseUrl)) === 'removed'; return (removed ? 'Signed out of ' : 'No stored credentials for ') + safeText(baseUrl || 'the API') + '.'; }; // scalar-sdk-generator:custom-code oauth-logout:end diff --git a/tests/browser-login.test.mjs b/tests/browser-login.test.mjs index 99acd9d..630d2a4 100644 --- a/tests/browser-login.test.mjs +++ b/tests/browser-login.test.mjs @@ -32,7 +32,7 @@ for (const invalid of [false, true]) { const exchanges = []; const requests = []; const revocations = []; - let rejectRevocation = true; + let rejectRevocation = false; const server = createServer(async (req, res) => { if (req.url.startsWith('/oauth2/auth?')) { authorization = new URL(req.url, origin).searchParams; @@ -125,22 +125,15 @@ for (const invalid of [false, true]) { assert.equal(JSON.parse(readFileSync(store, 'utf8')).profiles[origin].oauth.apiKey.refreshToken, 'test-rotated'); assert.equal((await run(['machines', 'list', '--api-key', 'explicit'])).code, 0); assert.equal(requests.at(-1), 'Bearer explicit'); - const rejected = await run(['logout']); - assert.notEqual(rejected.code, 0); - assert.match(rejected.stderr, /revok/i); - assert.equal(JSON.parse(readFileSync(store, 'utf8')).profiles[origin].oauth.apiKey.refreshToken, 'test-rotated'); - rejectRevocation = false; - revocations.length = 0; - assert.equal((await run(['logout'])).code, 0); - assert.deepEqual(revocations.map(({ token, token_type_hint }) => [token, token_type_hint]), [ - ['test-rotated', 'refresh_token'], ['test-refreshed', 'access_token'], - ]); + const logout = await run(['logout']); + assert.equal(logout.code, 0, logout.stderr); + assert.deepEqual(revocations, [{ token: 'test-rotated', token_type_hint: 'refresh_token' }]); assert.equal(JSON.parse(readFileSync(store, 'utf8')).profiles[origin], undefined); assert.equal(login.stdout.includes('test-access'), false); }); } -test('invariant_logout_all_preserves_unrevoked_or_replaced_credentials', async (t) => { +test('design_logout_clears_local_credentials_despite_revocation_failure', { timeout: 20000 }, async (t) => { const { runLogout } = await import('../dist/esm/cli/login.js'); const directory = mkdtempSync(join(tmpdir(), 'dedalus-logout-test-')); const store = join(directory, 'credentials.json'); @@ -148,51 +141,68 @@ test('invariant_logout_all_preserves_unrevoked_or_replaced_credentials', async ( process.env[variable] = store; t.after(() => { delete process.env[variable]; rmSync(directory, { recursive: true, force: true }); }); const config = { ...auth, backend: 'file', storeEnv: variable }; - let failure, replace = false; + let behavior = 'success'; + let warnings = ''; + t.mock.method(process.stderr, 'write', (chunk) => { warnings += chunk; return true; }); const requests = []; const server = createServer(async (req, res) => { let body = ''; for await (const chunk of req) body += chunk; - const form = new URLSearchParams(body); - requests.push(form.get('token')); - if (replace) { + requests.push(Object.fromEntries(new URLSearchParams(body))); + if (behavior === 'offline') { req.socket.destroy(); return; } + if (behavior === 'timeout') return; + if (behavior === 'replace') { const data = JSON.parse(readFileSync(store, 'utf8')); data.profiles[origin].credentials.apiKey = 'new-access'; writeFileSync(store, JSON.stringify(data)); } - res.writeHead(form.get('token_type_hint') === failure ? 503 : 200).end(); + res.writeHead(behavior === 'failure' ? 503 : 204).end(); }); await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); t.after(() => { server.closeAllConnections(); server.close(); }); const origin = 'http://127.0.0.1:' + server.address().port; - const oauth = { credentials: { apiKey: 'access' }, oauth: { apiKey: { - refreshToken: 'refresh', clientId: 'client', revocationUrl: origin + '/revoke', + const oauth = { credentials: { apiKey: 'access-secret' }, oauth: { apiKey: { + refreshToken: 'refresh-secret', clientId: 'client', revocationUrl: origin + '/revoke', } } }; - const save = (profile = oauth) => writeFileSync(store, JSON.stringify({ version: 1, profiles: { - [origin]: profile, 'https://manual.example': { credentials: { apiKey: 'manual' } }, - } })); - for (failure of ['refresh_token', 'access_token']) { + const save = (profile = oauth) => { + warnings = ''; + requests.length = 0; + writeFileSync(store, JSON.stringify({ version: 1, profiles: { + [origin]: profile, 'https://manual.example': { credentials: { apiKey: 'manual' } }, + } })); + }; + const profiles = () => JSON.parse(readFileSync(store, 'utf8')).profiles; + for (behavior of ['success', 'failure', 'offline', 'timeout', 'replace']) { save(); - await assert.rejects(runLogout(config, origin, true), /Credentials retained/); - assert.deepEqual(JSON.parse(readFileSync(store, 'utf8')).profiles[origin], oauth); + const started = Date.now(); + await runLogout(config, origin, true); + assert.throws(() => readFileSync(store), { code: 'ENOENT' }); + assert.deepEqual(requests, [{ token: 'refresh-secret', token_type_hint: 'refresh_token' }]); + if (['failure', 'offline', 'timeout'].includes(behavior)) assert.match(warnings, /revocation could not be confirmed/); + else assert.equal(warnings, ''); + assert.doesNotMatch(warnings, /access-secret|refresh-secret/); + if (behavior === 'timeout') assert.ok(Date.now() - started < 14000, 'revocation must be bounded to 10 seconds'); } - failure = undefined; - save(); - replace = true; - await assert.rejects(runLogout(config, origin, false), /Credentials changed/); - assert.equal(JSON.parse(readFileSync(store, 'utf8')).profiles[origin].credentials.apiKey, 'new-access'); - replace = false; + behavior = 'success'; + const accessOnly = structuredClone(oauth); + delete accessOnly.oauth.apiKey.refreshToken; + save(accessOnly); + await runLogout(config, origin, false); + assert.deepEqual(requests, [{ token: 'access-secret', token_type_hint: 'access_token' }]); + assert.equal(profiles()[origin], undefined); + assert.ok(profiles()['https://manual.example']); const legacy = structuredClone(oauth); delete legacy.oauth.apiKey.revocationUrl; save(legacy); - requests.length = 0; - await assert.rejects(runLogout(config, origin, true), /configuration is missing/); - assert.equal(requests.length, 0); + await runLogout(config, origin, true); + assert.deepEqual(requests, []); + assert.match(warnings, /revocation could not be confirmed/); + assert.throws(() => readFileSync(store), { code: 'ENOENT' }); writeFileSync(store, '{corrupt'); - await assert.rejects(runLogout(config, origin, true), /parse the credential store/); - assert.equal(readFileSync(store, 'utf8'), '{corrupt'); - save(); await runLogout(config, origin, true); - assert.deepEqual(requests, ['refresh', 'access']); - assert.deepEqual(JSON.parse(readFileSync(store, 'utf8')).profiles, {}); + assert.throws(() => readFileSync(store), { code: 'ENOENT' }); + // Local removal failures must still fail logout, even when no revocation is possible. + const { mkdirSync } = await import('node:fs'); + mkdirSync(store); + await assert.rejects(runLogout(config, origin, true)); });