diff --git a/README.md b/README.md index 501ae53..817837e 100644 --- a/README.md +++ b/README.md @@ -68,10 +68,34 @@ 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. + +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` 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. + + +`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/credentials.ts b/src/cli/credentials.ts index c0cef08..4edf3f3 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 used for best-effort revocation on 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; diff --git a/src/cli/login.ts b/src/cli/login.ts index 0bf9030..92a8200 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 + clearAll, + readStore, + // scalar-sdk-generator:custom-code logout-imports:end refreshProfile, storeDescription, writeProfile, @@ -30,6 +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; @@ -236,18 +245,50 @@ 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 + +/** 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).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) { + let profile: StoredProfile; + try { + profile = readProfile(location, key); + } catch { + warnRevocation(); + continue; + } + for (const [clientKey, meta] of Object.entries(profile.oauth ?? {})) { + 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: 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(); + } + } + } + // Local deletion is independent of the network result; deletion errors still propagate. if (all) { clearAll(location); - return 'Signed out everywhere.'; + return 'Signed out of all saved profiles.'; } - 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') + '.'; + 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 /** * Stored credentials for one base URL, keyed by client-option name. @@ -297,7 +338,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. @@ -358,7 +403,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 }; }; @@ -682,11 +733,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.'); + // 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()); - const state = base64Url(randomBytes(16)); + // 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 @@ -708,7 +774,10 @@ const authorizationCodeFlow = async ( } throw error; }); - const redirect = awaitRedirect(server, state, REDIRECT_PATH); + // 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'); @@ -866,6 +935,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) => { @@ -890,7 +963,9 @@ const awaitRedirect = ( send(404, 'text/plain', 'Not found.\n'); return; } - if (!sameToken(params.get('state') ?? '', state)) { + // 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 // 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 @@ -906,6 +981,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 4163441..529303d 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,25 @@ 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', + kind: 'oauth', + grant: 'authorizationCode', + clientKey: 'apiKey', + clientId: 'dedalus-cli', + authorizationUrl: '', + tokenUrl: '', + refreshUrl: '', + resource: '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 new file mode 100644 index 0000000..630d2a4 --- /dev/null +++ b/tests/browser-login.test.mjs @@ -0,0 +1,208 @@ +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('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 revokes 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 revocations = []; + let rejectRevocation = false; + 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/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; + 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', 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(); }); + 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'); + 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('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'); + 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 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; + 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(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-secret' }, oauth: { apiKey: { + refreshToken: 'refresh-secret', clientId: 'client', revocationUrl: origin + '/revoke', + } } }; + 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(); + 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'); + } + 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); + 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 runLogout(config, origin, true); + 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)); +});