diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 78bc2019..b5d25ecb 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -6,6 +6,8 @@ All notable CLI behavior changes are documented in this file. ### Added +- Add OAuth Device Flow to `skillhub login` when no API token is supplied, including best-effort + browser launch, a `--no-open` headless mode, bounded polling, and non-secret JSON progress output. - Add the `pi` agent profile, displayed as Pi, with `--agent pi`, project-level `/.pi/skills/`, and user-level `~/.pi/agent/skills/` support. - Add the user-level `astudio` agent profile, displayed as AStudio, with automatic detection of diff --git a/cli/README.md b/cli/README.md index 49d04e75..15839e09 100644 --- a/cli/README.md +++ b/cli/README.md @@ -18,8 +18,8 @@ bun add -g @astron-team/skillhub ## 🚀 Quick Start ```bash -# Login -skillhub login --token sk_xxx +# Log in interactively with OAuth Device Flow +skillhub login # Search skills skillhub search pdf @@ -68,7 +68,8 @@ set SKILLHUB_REGISTRY=https://skillhub.example.com ## 🔐 Authentication -Token resolution priority: +`skillhub login` uses OAuth Device Flow when no API token is supplied. Token resolution priority for +explicit token-based login and all other authenticated commands is: 1. `--token ` command-line argument 2. `SKILLHUB_TOKEN` environment variable @@ -77,14 +78,23 @@ Token resolution priority: ### Login ```bash -# Login with API token -skillhub login --token sk_xxx +# Interactive login: opens the registry's verification page and displays a user code +skillhub login -# Login to specific registry +# Interactive login on a remote/headless terminal +skillhub login --no-open --registry https://skillhub.example.com + +# Non-interactive login with an API token skillhub login --token sk_xxx --registry https://skillhub.example.com ``` -`login` validates the token, stores it in `~/.skillhub/credentials.json`, and writes the registry to `~/.skillhub/config.json`. +During interactive login, complete authentication in the browser and enter the displayed user code. +The CLI polls only until the server-provided expiry. It then validates the issued token, stores it in +`~/.skillhub/credentials.json`, and writes the registry to `~/.skillhub/config.json`. `--no-open` +suppresses automatic browser launch while retaining the verification URL and code in the terminal. + +Token-based login remains available for CI and other non-interactive automation. In both modes, +credentials are persisted only after `whoami` succeeds. Both files are updated non-destructively: SkillHub CLI changes only its own `tokens` and `registry` fields and preserves unknown fields written by other compatible tools. This allows tools that share @@ -461,7 +471,7 @@ Update mechanism: |---------|-------------| | `skillhub help [command]` | Display help information | | `skillhub version [--json]`, `skillhub --version`, `skillhub -v` | Display CLI version | -| `skillhub login --token [--registry ] [--json]` | Save token and registry configuration | +| `skillhub login [--no-open] [--token ] [--registry ] [--json]` | Log in with OAuth Device Flow or an API token | | `skillhub logout [--registry ] [--json]` | Remove token for specified registry | | `skillhub whoami [--registry ] [--token ] [--json]` | Validate current token and display user information | | `skillhub search [--registry ] [--token ] [--limit ] [--json]` | Search published skills | @@ -491,7 +501,10 @@ Update mechanism: # Verify token validity skillhub whoami -# Re-login +# Re-login interactively +skillhub login + +# Or use a token for non-interactive automation skillhub login --token sk_xxx ``` diff --git a/cli/src/clients/skillhub-client.ts b/cli/src/clients/skillhub-client.ts index a64d5ca3..4ecd81b2 100644 --- a/cli/src/clients/skillhub-client.ts +++ b/cli/src/clients/skillhub-client.ts @@ -7,6 +7,20 @@ export interface WhoAmIResponse { email?: string } +export interface DeviceCodeResponse { + deviceCode: string + userCode: string + verificationUri: string + expiresIn: number + interval: number +} + +export interface DeviceTokenResponse { + accessToken?: string | null + tokenType?: string | null + error?: string | null +} + export interface SearchItem { namespace: string slug: string @@ -146,6 +160,14 @@ export class SkillHubClient { return this.getJson('/auth/whoami') } + async requestDeviceCode(): Promise { + return this.postPublicJson('/api/v1/auth/device/code') + } + + async pollDeviceToken(deviceCode: string): Promise { + return this.postPublicJson('/api/v1/auth/device/token', { deviceCode }) + } + async serverMetadata(): Promise { let response: Response try { @@ -347,6 +369,20 @@ export class SkillHubClient { return this.handleJsonResponse(response) } + private async postPublicJson(path: string, body?: Record): Promise { + let response: Response + try { + response = await this.fetchImpl(`${this.registry}${path}`, { + method: 'POST', + headers: body ? { 'Content-Type': 'application/json' } : {}, + ...(body ? { body: JSON.stringify(body) } : {}) + }) + } catch { + throw new CliError('registry unreachable', EXIT.network, { registry: this.registry, next: 'check network or pass --registry' }) + } + return this.handleJsonResponse(response) + } + private async handleJsonResponse(response: Response): Promise { if (!response.ok) { throw await this.createResponseError(response, 'json') diff --git a/cli/src/commands/help.ts b/cli/src/commands/help.ts index 7d70cfb7..c9448c68 100644 --- a/cli/src/commands/help.ts +++ b/cli/src/commands/help.ts @@ -14,9 +14,13 @@ export const commands = { examples: ['skillhub version', 'skillhub version --json', 'skillhub --version', 'skillhub -v'] }, login: { - summary: 'Save registry and token', - usage: 'skillhub login [--token ] [--registry ] [--json]', - examples: ['skillhub login --token sk_xxx', 'skillhub login --registry https://skillhub.example.com'] + summary: 'Log in with OAuth Device Flow or an API token', + usage: 'skillhub login [--token ] [--no-open] [--registry ] [--json]', + examples: [ + 'skillhub login --registry https://skillhub.example.com', + 'skillhub login --registry https://skillhub.example.com --no-open', + 'skillhub login --token sk_xxx' + ] }, logout: { summary: 'Remove local token', diff --git a/cli/src/commands/login.ts b/cli/src/commands/login.ts index 850c23c4..141dcb27 100644 --- a/cli/src/commands/login.ts +++ b/cli/src/commands/login.ts @@ -2,11 +2,13 @@ import { ConfigStore } from '../stores/config-store' import { CredentialsStore } from '../stores/credentials-store' import { AuthService } from '../services/auth-service' import { resolveRegistry, resolveToken } from '../services/registry-service' +import { openExternalUrl } from '../platform/browser' export interface LoginCommandOptions { registry?: string token?: string json?: boolean + noOpen?: boolean } export async function loginCommand(options: LoginCommandOptions): Promise { @@ -14,7 +16,21 @@ export async function loginCommand(options: LoginCommandOptions): Promise { + const opened = options.noOpen ? false : openExternalUrl(details.verificationUri) + const message = options.json + ? JSON.stringify({ event: 'device_authorization', ...details, browserOpened: opened }) + : [ + `Authorize this device at: ${details.verificationUri}`, + `Code: ${details.userCode}`, + opened ? 'Browser launch requested. If no browser opened, use the URL above.' : 'Open the URL in a browser to continue.', + `Waiting for authorization (expires in ${details.expiresIn}s)...` + ].join('\n') + process.stderr.write(`${message}\n`) + }) return options.json ? JSON.stringify({ ok: true, registry, handle: result.handle }) : `Logged in to ${registry} as ${result.handle}` diff --git a/cli/src/index.ts b/cli/src/index.ts index cb21cdac..d8db4fd0 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -232,12 +232,16 @@ cli }) cli - .command('login', 'Save registry and token') + .command('login', 'Log in with OAuth Device Flow or an API token') .option('--registry ', 'Registry URL') .option('--token ', 'API token') + .option('--no-open', 'Do not open the verification URL in a browser') .option('--json', 'Output JSON') - .action((options: { registry?: string; token?: string; json?: boolean }) => { - return runCommand(() => loginCommand(options), Boolean(options.json)) + .action((options: { registry?: string; token?: string; open?: boolean; json?: boolean }) => { + return runCommand( + () => loginCommand({ ...options, noOpen: options.open === false }), + Boolean(options.json) + ) }) cli diff --git a/cli/src/platform/browser.ts b/cli/src/platform/browser.ts new file mode 100644 index 00000000..24d91a08 --- /dev/null +++ b/cli/src/platform/browser.ts @@ -0,0 +1,56 @@ +import { spawn } from 'node:child_process' + +type SupportedPlatform = 'darwin' | 'linux' | 'win32' +type Launch = (command: string, args: string[]) => boolean + +interface OpenExternalUrlOptions { + platform?: NodeJS.Platform + launch?: Launch + env?: NodeJS.ProcessEnv +} + +export function canOpenBrowser( + platform: NodeJS.Platform = process.platform, + env: NodeJS.ProcessEnv = process.env +): boolean { + if (env.CI || env.SSH_CONNECTION || env.SSH_TTY) return false + if (platform === 'linux') return Boolean(env.DISPLAY || env.WAYLAND_DISPLAY) + return platform === 'darwin' || platform === 'win32' +} + +function launchDetached(command: string, args: string[]): boolean { + try { + const child = spawn(command, args, { detached: true, stdio: 'ignore' }) + child.on('error', () => {}) + child.unref() + return true + } catch { + return false + } +} + +function launcherFor(platform: SupportedPlatform, url: string): [string, string[]] { + if (platform === 'darwin') return ['open', [url]] + if (platform === 'win32') return ['rundll32', ['url.dll,FileProtocolHandler', url]] + return ['xdg-open', [url]] +} + +export function openExternalUrl(url: string, options: OpenExternalUrlOptions = {}): boolean { + let parsed: URL + try { + parsed = new URL(url) + } catch { + return false + } + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') return false + + const platform = options.platform ?? process.platform + if (platform !== 'darwin' && platform !== 'linux' && platform !== 'win32') return false + if (!canOpenBrowser(platform, options.env ?? process.env)) return false + const [command, args] = launcherFor(platform, parsed.toString()) + try { + return (options.launch ?? launchDetached)(command, args) + } catch { + return false + } +} diff --git a/cli/src/services/auth-service.ts b/cli/src/services/auth-service.ts index 02235b09..11b5e025 100644 --- a/cli/src/services/auth-service.ts +++ b/cli/src/services/auth-service.ts @@ -5,21 +5,82 @@ import { CliError } from '../shared/errors' import { EXIT } from '../shared/constants' export class AuthService { + private readonly clientFactory: (registry: string, token?: string) => SkillHubClient + private readonly sleep: (milliseconds: number) => Promise + private readonly now: () => number + constructor( private readonly configStore: ConfigStore, - private readonly credentialsStore: CredentialsStore - ) {} + private readonly credentialsStore: CredentialsStore, + options: { + clientFactory?: (registry: string, token?: string) => SkillHubClient + sleep?: (milliseconds: number) => Promise + now?: () => number + } = {} + ) { + this.clientFactory = options.clientFactory ?? ((registry, token) => new SkillHubClient(registry, token)) + this.sleep = options.sleep ?? (milliseconds => new Promise(resolve => setTimeout(resolve, milliseconds))) + this.now = options.now ?? Date.now + } - async login(registry: string, token?: string): Promise<{ handle: string }> { - if (!token) { - throw new CliError('token is required', EXIT.usage, { next: 'pass --token, set SKILLHUB_TOKEN, or use interactive login' }) - } - const user = await new SkillHubClient(registry, token).whoami() + async login(registry: string, token: string): Promise<{ handle: string }> { + const user = await this.clientFactory(registry, token).whoami() await this.configStore.setRegistry(registry) await this.credentialsStore.setToken(registry, token) return { handle: user.handle } } + async loginWithDeviceFlow( + registry: string, + onVerification: (details: { userCode: string; verificationUri: string; expiresIn: number }) => Promise + ): Promise<{ handle: string }> { + const client = this.clientFactory(registry) + const device = await client.requestDeviceCode() + if ( + !device.deviceCode || + !device.userCode || + !device.verificationUri || + !Number.isFinite(device.expiresIn) || + !Number.isFinite(device.interval) || + device.expiresIn <= 0 || + device.interval <= 0 + ) { + throw new CliError('registry returned invalid device authorization data', EXIT.auth, { registry }) + } + + let verificationUri: string + try { + const parsedVerificationUri = new URL(device.verificationUri, `${registry}/`) + if (parsedVerificationUri.protocol !== 'http:' && parsedVerificationUri.protocol !== 'https:') { + throw new Error('unsupported protocol') + } + verificationUri = parsedVerificationUri.toString() + } catch { + throw new CliError('registry returned invalid device verification URL', EXIT.auth, { registry }) + } + await onVerification({ userCode: device.userCode, verificationUri, expiresIn: device.expiresIn }) + + const expiresAt = this.now() + device.expiresIn * 1_000 + const intervalMilliseconds = device.interval * 1_000 + while (this.now() < expiresAt) { + await this.sleep(Math.min(intervalMilliseconds, Math.max(0, expiresAt - this.now()))) + if (this.now() >= expiresAt) break + + const response = await client.pollDeviceToken(device.deviceCode) + if (response.accessToken) { + if (response.tokenType && response.tokenType.toLowerCase() !== 'bearer') { + throw new CliError('registry returned unsupported device token type', EXIT.auth, { registry }) + } + return this.login(registry, response.accessToken) + } + if (response.error && response.error !== 'authorization_pending') { + throw new CliError(`device authorization failed: ${response.error}`, EXIT.auth, { registry }) + } + } + + throw new CliError('device authorization expired', EXIT.auth, { registry, next: 'run `skillhub login` again' }) + } + async logout(registry: string): Promise { await this.credentialsStore.deleteToken(registry) } diff --git a/cli/test/helpers/fake-registry.ts b/cli/test/helpers/fake-registry.ts index fa97c79f..1ebdbf7a 100644 --- a/cli/test/helpers/fake-registry.ts +++ b/cli/test/helpers/fake-registry.ts @@ -168,6 +168,13 @@ interface FakeRegistryOptions { dryRunResponse?: { valid: boolean; errors: string[]; warnings: string[]; resolvedSlug: string | null; resolvedVersion: string | null } publishStatus?: string namespacePageSize?: number + deviceFlow?: { + accessToken: string + pendingPolls?: number + userCode?: string + expiresIn?: number + interval?: number + } /** * Per-endpoint failure injection. When set for an endpoint, that endpoint * ignores all other logic and returns the specified failure (or throws for @@ -228,6 +235,7 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { downloads: number reviews: number namespaceRequests: number + devicePolls: number } = { publish: null, resolve: null, @@ -237,7 +245,8 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { resolves: 0, downloads: 0, reviews: 0, - namespaceRequests: 0 + namespaceRequests: 0, + devicePolls: 0 } // If any endpoint is configured with 'network' failure mode, we need a real @@ -283,6 +292,33 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { const path = url.pathname const baseUrl = `${url.protocol}//${url.host}` + // ------------------------------------------------------------------ // + // POST /api/v1/auth/device/code and /api/v1/auth/device/token + // ------------------------------------------------------------------ // + if (path === '/api/v1/auth/device/code' && req.method === 'POST' && options.deviceFlow) { + return Response.json({ + code: 0, + data: { + deviceCode: 'device-secret', + userCode: options.deviceFlow.userCode ?? 'ABCD-2345', + verificationUri: `${baseUrl}/device`, + expiresIn: options.deviceFlow.expiresIn ?? 60, + interval: options.deviceFlow.interval ?? 0.001 + } + }) + } + + if (path === '/api/v1/auth/device/token' && req.method === 'POST' && options.deviceFlow) { + state.devicePolls += 1 + if (state.devicePolls <= (options.deviceFlow.pendingPolls ?? 0)) { + return Response.json({ code: 0, data: { accessToken: null, tokenType: null, error: 'authorization_pending' } }) + } + return Response.json({ + code: 0, + data: { accessToken: options.deviceFlow.accessToken, tokenType: 'Bearer', error: null } + }) + } + // ------------------------------------------------------------------ // // GET /api/cli/v1/auth/whoami // ------------------------------------------------------------------ // diff --git a/cli/test/helpers/run-cli.ts b/cli/test/helpers/run-cli.ts index 00a9562d..be9b4db9 100644 --- a/cli/test/helpers/run-cli.ts +++ b/cli/test/helpers/run-cli.ts @@ -41,7 +41,9 @@ export async function runCli( const proc = Bun.spawn({ cmd: [bunPath, entry, ...args], cwd: options.cwd ?? cliRoot, - env: { ...sanitizeProcessEnv(), ...env }, + // Integration tests must never launch real desktop applications. Tests + // that exercise browser-launch behavior inject a fake launcher directly. + env: { ...sanitizeProcessEnv(), CI: 'true', ...env }, stdout: 'pipe', stderr: 'pipe' }) diff --git a/cli/test/integration/auth-commands.test.ts b/cli/test/integration/auth-commands.test.ts index d862224e..9e659eb2 100644 --- a/cli/test/integration/auth-commands.test.ts +++ b/cli/test/integration/auth-commands.test.ts @@ -3,7 +3,7 @@ import { createTempHome } from '../helpers/temp-env' import { startFakeRegistry } from '../helpers/fake-registry' import { runCli } from '../helpers/run-cli' -let registry: { url: string; stop: () => void } | undefined +let registry: Awaited> | undefined afterEach(() => { registry?.stop() @@ -79,18 +79,52 @@ describe('auth commands', () => { expect(result.stderr).toContain('authentication failed') }) - // [P0] missing token → EXIT.usage, stderr contains "token is required" - test('login without --token exits with usage error', async () => { + test('login without a token uses device flow and never prints the access token', async () => { const env = await createTempHome() - registry = await startFakeRegistry({ token: 'sk_ok' }) + registry = await startFakeRegistry({ + token: 'oauth-secret', + user: { handle: 'oauth-user', displayName: 'OAuth User' }, + deviceFlow: { accessToken: 'oauth-secret', pendingPolls: 1 } + }) - const result = await runCli(['login', '--registry', registry.url], { + const result = await runCli(['login', '--registry', registry.url, '--no-open'], { HOME: env.home, USERPROFILE: env.home }) - expect(result.exitCode).toBe(5) // EXIT.usage - expect(result.stderr).toContain('token is required') + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('Logged in') + expect(result.stderr).toContain('ABCD-2345') + expect(result.stderr).toContain('/device') + expect(`${result.stdout}\n${result.stderr}`).not.toContain('oauth-secret') + expect(await Bun.file(`${env.home}/.skillhub/credentials.json`).json()) + .toMatchObject({ tokens: { [registry.url]: 'oauth-secret' } }) + expect(registry.received.devicePolls).toBe(2) + }) + + test('device login --json emits a non-secret authorization event and final result', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'oauth-secret', + user: { handle: 'oauth-user', displayName: 'OAuth User' }, + deviceFlow: { accessToken: 'oauth-secret' } + }) + + const result = await runCli(['login', '--registry', registry.url, '--no-open', '--json'], { + HOME: env.home, + USERPROFILE: env.home + }) + + expect(result.exitCode).toBe(0) + expect(JSON.parse(result.stdout)).toEqual({ ok: true, registry: registry.url, handle: 'oauth-user' }) + expect(JSON.parse(result.stderr)).toMatchObject({ + event: 'device_authorization', + userCode: 'ABCD-2345', + verificationUri: `${registry.url}/device`, + browserOpened: false + }) + expect(`${result.stdout}\n${result.stderr}`).not.toContain('oauth-secret') + expect(`${result.stdout}\n${result.stderr}`).not.toContain('device-secret') }) // [P0] whoami failure must NOT write credentials diff --git a/cli/test/integration/help-command.test.ts b/cli/test/integration/help-command.test.ts index 0a50779f..88a3f74c 100644 --- a/cli/test/integration/help-command.test.ts +++ b/cli/test/integration/help-command.test.ts @@ -2,6 +2,14 @@ import { describe, expect, test } from 'bun:test' import { runCli } from '../helpers/run-cli' describe('help command', () => { + test('documents interactive device login and the headless fallback', async () => { + const result = await runCli(['help', 'login']) + + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('OAuth Device Flow') + expect(result.stdout).toContain('--no-open') + expect(result.stdout).toContain('--token') + }) test('prints detailed help for install', async () => { const result = await runCli(['help', 'install']) expect(result.exitCode).toBe(0) diff --git a/cli/test/unit/clients/skillhub-client.test.ts b/cli/test/unit/clients/skillhub-client.test.ts index 6d0249d2..5aab4e43 100644 --- a/cli/test/unit/clients/skillhub-client.test.ts +++ b/cli/test/unit/clients/skillhub-client.test.ts @@ -133,6 +133,39 @@ describe('SkillHubClient', () => { await err.toHaveProperty('exitCode', EXIT.auth) }) + test('device authorization uses the public endpoints without bearer credentials', async () => { + const requests: Array<{ path: string; authorization: string | null; body?: unknown }> = [] + const fetchImpl = (async (input: URL | RequestInfo, init?: RequestInit) => { + const url = new URL(String(input)) + requests.push({ + path: url.pathname, + authorization: new Headers(init?.headers).get('authorization'), + ...(init?.body ? { body: JSON.parse(String(init.body)) } : {}) + }) + if (url.pathname.endsWith('/code')) { + return Response.json({ + data: { + deviceCode: 'device-secret', + userCode: 'ABCD-2345', + verificationUri: '/device', + expiresIn: 60, + interval: 5 + } + }) + } + return Response.json({ data: { accessToken: null, tokenType: null, error: 'authorization_pending' } }) + }) as unknown as typeof fetch + const client = new SkillHubClient('https://skillhub.example.com', 'stored-token', fetchImpl) + + await client.requestDeviceCode() + await client.pollDeviceToken('device-secret') + + expect(requests).toEqual([ + { path: '/api/v1/auth/device/code', authorization: null }, + { path: '/api/v1/auth/device/token', authorization: null, body: { deviceCode: 'device-secret' } } + ]) + }) + // --- search() (P1) --- test('search() returns items', async () => { diff --git a/cli/test/unit/platform/browser.test.ts b/cli/test/unit/platform/browser.test.ts new file mode 100644 index 00000000..0fa6dea7 --- /dev/null +++ b/cli/test/unit/platform/browser.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, mock, test } from 'bun:test' +import { canOpenBrowser, openExternalUrl } from '../../../src/platform/browser' + +describe('openExternalUrl', () => { + test.each([ + ['darwin', 'open', ['https://skillhub.example.com/device']], + ['linux', 'xdg-open', ['https://skillhub.example.com/device']], + ['win32', 'rundll32', ['url.dll,FileProtocolHandler', 'https://skillhub.example.com/device']] + ] as const)('uses the platform launcher on %s', (platform, command, args) => { + const launch = mock(() => true) + + const env = platform === 'linux' ? { DISPLAY: ':0' } : {} + expect(openExternalUrl('https://skillhub.example.com/device', { platform, launch, env })).toBe(true) + expect(launch).toHaveBeenCalledWith(command, [...args]) + }) + + test('rejects non-http URLs without launching a process', () => { + const launch = mock(() => true) + + expect(openExternalUrl('javascript:alert(1)', { platform: 'linux', launch, env: { DISPLAY: ':0' } })).toBe(false) + expect(launch).not.toHaveBeenCalled() + }) + + test('falls back cleanly when the browser launcher is unavailable', () => { + const launch = mock(() => false) + + expect(openExternalUrl('https://skillhub.example.com/device', { platform: 'linux', launch, env: { DISPLAY: ':0' } })).toBe(false) + }) + + test.each([ + ['CI', { CI: 'true', DISPLAY: ':0' }], + ['SSH', { SSH_CONNECTION: 'client server', DISPLAY: ':0' }], + ['SSH TTY', { SSH_TTY: '/dev/pts/0', DISPLAY: ':0' }], + ['Linux without a display', {}] + ])('does not launch in %s environments', (_name, env) => { + const launch = mock(() => true) + + expect(openExternalUrl('https://skillhub.example.com/device', { platform: 'linux', launch, env })).toBe(false) + expect(launch).not.toHaveBeenCalled() + }) + + test('allows Linux desktops with X11 or Wayland', () => { + expect(canOpenBrowser('linux', { DISPLAY: ':0' })).toBe(true) + expect(canOpenBrowser('linux', { WAYLAND_DISPLAY: 'wayland-0' })).toBe(true) + }) + + test('rejects unsupported platforms without launching a process', () => { + const launch = mock(() => true) + + expect(openExternalUrl('https://skillhub.example.com/device', { + platform: 'freebsd', + launch, + env: {} + })).toBe(false) + expect(launch).not.toHaveBeenCalled() + }) + + test('contains a synchronous launcher failure', () => { + const launch = mock(() => { throw new Error('launcher unavailable') }) + + expect(() => openExternalUrl('https://skillhub.example.com/device', { + platform: 'darwin', + launch, + env: {} + })).not.toThrow() + expect(openExternalUrl('https://skillhub.example.com/device', { + platform: 'darwin', + launch, + env: {} + })).toBe(false) + }) +}) diff --git a/cli/test/unit/services/auth-service-device-flow.test.ts b/cli/test/unit/services/auth-service-device-flow.test.ts new file mode 100644 index 00000000..95ef7c83 --- /dev/null +++ b/cli/test/unit/services/auth-service-device-flow.test.ts @@ -0,0 +1,249 @@ +import { describe, expect, mock, test } from 'bun:test' +import { AuthService } from '../../../src/services/auth-service' +import { CliError } from '../../../src/shared/errors' +import { EXIT } from '../../../src/shared/constants' + +describe('AuthService device flow', () => { + test('polls pending authorization, validates the token, then persists credentials', async () => { + const setRegistry = mock(async () => {}) + const setToken = mock(async () => {}) + const sleep = mock(async () => {}) + const pollDeviceToken = mock() + .mockResolvedValueOnce({ error: 'authorization_pending' }) + .mockResolvedValueOnce({ accessToken: 'oauth-secret', tokenType: 'Bearer' }) + const clientFactory = mock((_registry: string, token?: string) => token + ? { whoami: async () => ({ handle: 'oauth-user', displayName: 'OAuth User' }) } + : { + requestDeviceCode: async () => ({ + deviceCode: 'device-secret', + userCode: 'ABCD-2345', + verificationUri: '/device', + expiresIn: 60, + interval: 2 + }), + pollDeviceToken + }) + + const service = new AuthService( + { setRegistry } as never, + { setToken } as never, + { clientFactory: clientFactory as never, sleep, now: () => 0 } + ) + const onVerification = mock(async () => {}) + + const result = await service.loginWithDeviceFlow('https://skillhub.example.com', onVerification) + + expect(result).toEqual({ handle: 'oauth-user' }) + expect(onVerification).toHaveBeenCalledWith({ + userCode: 'ABCD-2345', + verificationUri: 'https://skillhub.example.com/device', + expiresIn: 60 + }) + expect(sleep).toHaveBeenCalledTimes(2) + expect(sleep).toHaveBeenCalledWith(2_000) + expect(setRegistry).toHaveBeenCalledWith('https://skillhub.example.com') + expect(setToken).toHaveBeenCalledWith('https://skillhub.example.com', 'oauth-secret') + }) + + test('stops at expiry without persisting credentials', async () => { + const setRegistry = mock(async () => {}) + const setToken = mock(async () => {}) + let now = 0 + const service = new AuthService( + { setRegistry } as never, + { setToken } as never, + { + clientFactory: (() => ({ + requestDeviceCode: async () => ({ + deviceCode: 'device-secret', + userCode: 'ABCD-2345', + verificationUri: '/device', + expiresIn: 1, + interval: 1 + }), + pollDeviceToken: async () => ({ error: 'authorization_pending' }) + })) as never, + sleep: async (milliseconds: number) => { now += milliseconds }, + now: () => now + } + ) + + await expect(service.loginWithDeviceFlow('https://skillhub.example.com', async () => {})) + .rejects.toMatchObject({ message: 'device authorization expired', exitCode: 2 }) + expect(setRegistry).not.toHaveBeenCalled() + expect(setToken).not.toHaveBeenCalled() + }) + + test('stops when the authorization server denies the request', async () => { + const setToken = mock(async () => {}) + const service = new AuthService( + { setRegistry: mock(async () => {}) } as never, + { setToken } as never, + { + clientFactory: (() => ({ + requestDeviceCode: async () => ({ + deviceCode: 'device-secret', + userCode: 'ABCD-2345', + verificationUri: '/device', + expiresIn: 60, + interval: 1 + }), + pollDeviceToken: async () => ({ error: 'access_denied' }) + })) as never, + sleep: async () => {}, + now: () => 0 + } + ) + + await expect(service.loginWithDeviceFlow('https://skillhub.example.com', async () => {})) + .rejects.toMatchObject({ message: 'device authorization failed: access_denied', exitCode: 2 }) + expect(setToken).not.toHaveBeenCalled() + }) + + test('rejects a non-positive polling interval from the registry', async () => { + const service = new AuthService( + { setRegistry: mock(async () => {}) } as never, + { setToken: mock(async () => {}) } as never, + { + clientFactory: (() => ({ + requestDeviceCode: async () => ({ + deviceCode: 'device-secret', + userCode: 'ABCD-2345', + verificationUri: '/device', + expiresIn: 60, + interval: 0 + }) + })) as never + } + ) + + await expect(service.loginWithDeviceFlow('https://skillhub.example.com', async () => {})) + .rejects.toMatchObject({ message: 'registry returned invalid device authorization data', exitCode: 2 }) + }) + + test('rejects a non-HTTP verification URL from the registry', async () => { + const service = new AuthService( + { setRegistry: mock(async () => {}) } as never, + { setToken: mock(async () => {}) } as never, + { + clientFactory: (() => ({ + requestDeviceCode: async () => ({ + deviceCode: 'device-secret', + userCode: 'ABCD-2345', + verificationUri: 'javascript:alert(1)', + expiresIn: 60, + interval: 1 + }) + })) as never + } + ) + + await expect(service.loginWithDeviceFlow('https://skillhub.example.com', async () => {})) + .rejects.toMatchObject({ message: 'registry returned invalid device verification URL', exitCode: 2 }) + }) + + test.each([ + ['missing device code', { deviceCode: '', userCode: 'ABCD-2345', verificationUri: '/device', expiresIn: 60, interval: 1 }], + ['missing user code', { deviceCode: 'device-secret', userCode: '', verificationUri: '/device', expiresIn: 60, interval: 1 }], + ['missing verification URI', { deviceCode: 'device-secret', userCode: 'ABCD-2345', verificationUri: '', expiresIn: 60, interval: 1 }], + ['zero expiry', { deviceCode: 'device-secret', userCode: 'ABCD-2345', verificationUri: '/device', expiresIn: 0, interval: 1 }], + ['negative expiry', { deviceCode: 'device-secret', userCode: 'ABCD-2345', verificationUri: '/device', expiresIn: -1, interval: 1 }], + ['non-finite expiry', { deviceCode: 'device-secret', userCode: 'ABCD-2345', verificationUri: '/device', expiresIn: Number.NaN, interval: 1 }] + ])('rejects malformed device authorization data: %s', async (_name, device) => { + const service = new AuthService( + { setRegistry: mock(async () => {}) } as never, + { setToken: mock(async () => {}) } as never, + { clientFactory: (() => ({ requestDeviceCode: async () => device })) as never } + ) + + await expect(service.loginWithDeviceFlow('https://skillhub.example.com', async () => {})) + .rejects.toMatchObject({ message: 'registry returned invalid device authorization data', exitCode: EXIT.auth }) + }) + + test('propagates a device-code request network failure without persisting credentials', async () => { + const setToken = mock(async () => {}) + const service = new AuthService( + { setRegistry: mock(async () => {}) } as never, + { setToken } as never, + { + clientFactory: (() => ({ + requestDeviceCode: async () => { throw new CliError('registry unreachable', EXIT.network) } + })) as never + } + ) + + await expect(service.loginWithDeviceFlow('https://skillhub.example.com', async () => {})) + .rejects.toMatchObject({ message: 'registry unreachable', exitCode: EXIT.network }) + expect(setToken).not.toHaveBeenCalled() + }) + + test('propagates a polling network failure without persisting credentials', async () => { + const setToken = mock(async () => {}) + const service = new AuthService( + { setRegistry: mock(async () => {}) } as never, + { setToken } as never, + { + clientFactory: (() => ({ + requestDeviceCode: async () => ({ + deviceCode: 'device-secret', userCode: 'ABCD-2345', verificationUri: '/device', expiresIn: 60, interval: 1 + }), + pollDeviceToken: async () => { throw new CliError('registry unreachable', EXIT.network) } + })) as never, + sleep: async () => {}, + now: () => 0 + } + ) + + await expect(service.loginWithDeviceFlow('https://skillhub.example.com', async () => {})) + .rejects.toMatchObject({ message: 'registry unreachable', exitCode: EXIT.network }) + expect(setToken).not.toHaveBeenCalled() + }) + + test('rejects a non-Bearer device token without persisting credentials', async () => { + const setToken = mock(async () => {}) + const service = new AuthService( + { setRegistry: mock(async () => {}) } as never, + { setToken } as never, + { + clientFactory: (() => ({ + requestDeviceCode: async () => ({ + deviceCode: 'device-secret', userCode: 'ABCD-2345', verificationUri: '/device', expiresIn: 60, interval: 1 + }), + pollDeviceToken: async () => ({ accessToken: 'oauth-secret', tokenType: 'MAC' }) + })) as never, + sleep: async () => {}, + now: () => 0 + } + ) + + await expect(service.loginWithDeviceFlow('https://skillhub.example.com', async () => {})) + .rejects.toMatchObject({ message: 'registry returned unsupported device token type', exitCode: EXIT.auth }) + expect(setToken).not.toHaveBeenCalled() + }) + + test('does not persist the issued token when whoami validation fails', async () => { + const setRegistry = mock(async () => {}) + const setToken = mock(async () => {}) + const service = new AuthService( + { setRegistry } as never, + { setToken } as never, + { + clientFactory: ((_registry: string, token?: string) => token + ? { whoami: async () => { throw new CliError('authentication failed', EXIT.auth) } } + : { + requestDeviceCode: async () => ({ + deviceCode: 'device-secret', userCode: 'ABCD-2345', verificationUri: '/device', expiresIn: 60, interval: 1 + }), + pollDeviceToken: async () => ({ accessToken: 'oauth-secret', tokenType: 'Bearer' }) + }) as never, + sleep: async () => {}, + now: () => 0 + } + ) + + await expect(service.loginWithDeviceFlow('https://skillhub.example.com', async () => {})) + .rejects.toMatchObject({ message: 'authentication failed', exitCode: EXIT.auth }) + expect(setRegistry).not.toHaveBeenCalled() + expect(setToken).not.toHaveBeenCalled() + }) +}) diff --git a/compose.release.yml b/compose.release.yml index 317226b5..7b84eb56 100644 --- a/compose.release.yml +++ b/compose.release.yml @@ -75,7 +75,7 @@ services: SKILLHUB_SUITE_REVIEW_WRITES_ENABLED: ${SKILLHUB_SUITE_REVIEW_WRITES_ENABLED:-true} SESSION_COOKIE_SECURE: ${SESSION_COOKIE_SECURE:-false} SKILLHUB_PUBLIC_BASE_URL: ${SKILLHUB_PUBLIC_BASE_URL:-} - DEVICE_AUTH_VERIFICATION_URI: ${DEVICE_AUTH_VERIFICATION_URI:-} + DEVICE_AUTH_VERIFICATION_URI: ${DEVICE_AUTH_VERIFICATION_URI:-/device} SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET: ${SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET:?required} SKILLHUB_STORAGE_PROVIDER: ${SKILLHUB_STORAGE_PROVIDER:-s3} STORAGE_BASE_PATH: /var/lib/skillhub/storage diff --git a/docs/ai-changes-cli-oauth-device-flow.md b/docs/ai-changes-cli-oauth-device-flow.md new file mode 100644 index 00000000..5417f420 --- /dev/null +++ b/docs/ai-changes-cli-oauth-device-flow.md @@ -0,0 +1,30 @@ +# AI 变更记录：CLI OAuth Device Flow 登录 + +## 需求背景 + +SkillHub 服务端已经提供 Device Code 申请、授权轮询和浏览器确认能力，但 CLI `0.1.12` +在没有 API Token 时仍直接返回 `token is required`，无法使用既有的 OAuth 登录链路。 +对应社区需求记录在 Issue #856。 + +## 实现内容 + +- `skillhub login` 未显式提供 Token 时，调用现有 `/api/v1/auth/device/code` 发起授权。 +- 展示服务端返回的验证地址和用户码，并在受支持的平台上尝试打开默认浏览器。 +- 增加 `--no-open`，支持远程服务器和无图形界面的终端。 +- 按服务端返回的 `interval` 和 `expiresIn` 轮询 `/api/v1/auth/device/token`，处理等待、拒绝、过期和网络失败。 +- 获取 Bearer Token 后先调用 `whoami` 验证，成功后才写入现有配置与凭据存储。 +- 保留 `--token`、`SKILLHUB_TOKEN` 和已存 Token 的兼容行为，供 CI 与自动化使用。 +- JSON 模式只输出验证地址、用户码、有效期和浏览器打开状态，不输出 device code 或 access token。 + +## 安全边界 + +- 只允许打开 HTTP/HTTPS 验证地址，不通过 shell 拼接 URL。 +- access token 不进入标准输出或错误输出。 +- Device Flow 失败、拒绝或超时时不更新本地凭据。 +- 浏览器打开失败不会中断流程，用户仍可复制终端中的验证地址继续授权。 + +## 测试策略 + +- 单元测试覆盖轮询成功、等待、拒绝、过期、非法服务端参数和跨平台浏览器启动。 +- 客户端测试验证 Device Flow 使用公开端点且不携带已有 Bearer Token。 +- 集成测试覆盖普通输出、JSON 输出、`--no-open`、Token 不泄漏及原有 Token 登录兼容性。 diff --git a/server/skillhub-app/src/main/resources/application.yml b/server/skillhub-app/src/main/resources/application.yml index ca11660a..c5507532 100644 --- a/server/skillhub-app/src/main/resources/application.yml +++ b/server/skillhub-app/src/main/resources/application.yml @@ -208,7 +208,7 @@ skillhub: editable: false requires-review: false device-auth: - verification-uri: ${DEVICE_AUTH_VERIFICATION_URI:${skillhub.public.base-url:}/cli/auth} + verification-uri: ${DEVICE_AUTH_VERIFICATION_URI:${skillhub.public.base-url:}/device} security: scanner: enabled: ${SKILLHUB_SECURITY_SCANNER_ENABLED:true} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java index 5061f854..d0eead62 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/device/DeviceAuthService.java @@ -41,7 +41,7 @@ public class DeviceAuthService { public DeviceAuthService(RedisTemplate redisTemplate, ApiTokenService apiTokenService, ObjectMapper objectMapper, - @Value("${skillhub.device-auth.verification-uri:/cli/auth}") String verificationUri) { + @Value("${skillhub.device-auth.verification-uri:/device}") String verificationUri) { this.redisTemplate = redisTemplate; this.apiTokenService = apiTokenService; this.objectMapper = objectMapper; diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java index fca992b2..f9517e43 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/device/DeviceAuthServiceTest.java @@ -44,7 +44,7 @@ class DeviceAuthServiceTest { @BeforeEach void setUp() { lenient().when(redisTemplate.opsForValue()).thenReturn(valueOperations); - service = new DeviceAuthService(redisTemplate, apiTokenService, new ObjectMapper(), "/cli/auth"); + service = new DeviceAuthService(redisTemplate, apiTokenService, new ObjectMapper(), "/device"); } /** diff --git a/web/src/app/router.test.ts b/web/src/app/router.test.ts index 79beeb6e..f8a95847 100644 --- a/web/src/app/router.test.ts +++ b/web/src/app/router.test.ts @@ -45,4 +45,10 @@ describe('router', () => { const childPaths = children.map((route) => route.fullPath ?? route.path) expect(childPaths).toContain('/space/$namespace/$slug/compare') }) + + it('registers the device authorization route', () => { + const children = (router.routeTree.children ?? []) as Array<{ fullPath?: string; path?: string }> + const childPaths = children.map((route) => route.fullPath ?? route.path) + expect(childPaths).toContain('/device') + }) }) diff --git a/web/src/app/router.tsx b/web/src/app/router.tsx index 65ee019e..d20ea269 100644 --- a/web/src/app/router.tsx +++ b/web/src/app/router.tsx @@ -86,6 +86,7 @@ function createRoleProtectedRouteComponent import('@/pages/landing'), 'LandingPage') const HomePage = createLazyRouteComponent(() => import('@/pages/home'), 'HomePage') const LoginPage = createLazyRouteComponent(() => import('@/pages/login'), 'LoginPage') +const DeviceAuthPage = createLazyRouteComponent(() => import('@/pages/device'), 'DeviceAuthPage') const RegisterPage = createLazyRouteComponent(() => import('@/pages/register'), 'RegisterPage') const ResetPasswordPage = createLazyRouteComponent(() => import('@/pages/reset-password'), 'ResetPasswordPage') const PrivacyPolicyPage = createLazyRouteComponent(() => import('@/pages/privacy'), 'PrivacyPolicyPage') @@ -587,6 +588,12 @@ const cliAuthRoute = createRoute({ }, }) +const deviceAuthRoute = createRoute({ + getParentRoute: () => rootRoute, + path: 'device', + component: DeviceAuthPage, +}) + const settingsSecurityRoute = createRoute({ getParentRoute: () => rootRoute, path: 'settings/security', @@ -683,6 +690,7 @@ const routeTree = rootRoute.addChildren([ dashboardNotificationsRoute, dashboardTokensRoute, cliAuthRoute, + deviceAuthRoute, settingsSecurityRoute, settingsProfileRoute, settingsNotificationsRoute,