From b928869a9b2feba66ba06920e4a644ab8a986feb Mon Sep 17 00:00:00 2001 From: SiebeBaree Date: Mon, 13 Jul 2026 09:08:21 +0200 Subject: [PATCH 1/2] Add Docker-compatible device authentication --- README.md | 6 + src/api/auth.ts | 425 ++++++------------------- src/lib/keyring.ts | 117 ++++++- tests/integration/file-keyring.test.ts | 52 +++ tests/integration/login-flow.test.ts | 358 ++++++--------------- 5 files changed, 357 insertions(+), 601 deletions(-) create mode 100644 tests/integration/file-keyring.test.ts diff --git a/README.md b/README.md index d86d027..ff193ca 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,12 @@ ek configure ek run -- your-command ``` +## Docker and dev containers + +`ek login` uses a device verification URL, so the browser can run on your host while the CLI runs in a container. If the URL cannot open automatically, open the printed URL and approve the matching code. + +The CLI uses the OS keyring when available and otherwise stores credentials in `~/.enkryptify/secure-store.json` with owner-only permissions. Mount `~/.enkryptify` into the container to preserve login and configuration between rebuilds, or set `ENKRYPTIFY_STORE_PATH` to a mounted file path. + ## Documentation For usage guides, command reference, and configuration: **[docs.enkryptify.com](https://docs.enkryptify.com)** diff --git a/src/api/auth.ts b/src/api/auth.ts index 6ef9e3e..0f56874 100644 --- a/src/api/auth.ts +++ b/src/api/auth.ts @@ -1,12 +1,9 @@ -import { env } from "@/env"; +import http from "@/api/httpClient"; import { config as configManager } from "@/lib/config"; import { CLIError } from "@/lib/errors"; import { logger } from "@/lib/logger"; import { secureStore } from "@/lib/secureStore"; -import http from "@/api/httpClient"; -import { createHash, randomBytes } from "crypto"; import open from "open"; -import { URL } from "url"; export type Credentials = { [key: string]: string; @@ -22,61 +19,60 @@ type UserInfo = { name: string; }; +type DeviceCodeResponse = { + deviceCode: string; + userCode: string; + verificationUri: string; + verificationUriComplete: string; + expiresIn: number; + interval: number; +}; + type AuthResponse = { accessToken: string; tokenType: string; expiresIn: number; }; -function base64Url(buf: Buffer) { - return buf.toString("base64").replace(/\+/g, "-").replace(/\//g, "_").replace(/=+/g, ""); -} +type DeviceTokenError = { + error?: "authorization_pending" | "access_denied" | "expired_token"; +}; -export class Auth { - private readonly CLIENT_ID = "enkryptify-cli"; - private readonly REDIRECT_URL = "http://localhost:51823/callback"; - private readonly CALLBACK_PORT = 51823; - private readonly DEFAULT_SCOPES = "openid profile email secrets:read secrets:write"; +const CLIENT_ID = "enkryptify-cli"; +export class Auth { async login(options?: LoginOptions): Promise { - let envToken: string | undefined; + let accessToken: string | undefined; try { - const creds = await this.getCredentials(); - envToken = creds.accessToken; + accessToken = (await this.getCredentials()).accessToken; } catch (error: unknown) { logger.debug(error instanceof Error ? error.message : String(error)); - envToken = undefined; } - if (envToken) { + if (accessToken) { if (options?.force) { await secureStore.clearAll(); + } else if (await this.getUserInfo(accessToken).catch(() => null)) { + logger.info('Already logged in. Use "ek login --force" to re-authenticate with a different account.'); + await configManager.markAuthenticated(); + return; } else { - const isAuth = await this.getUserInfo(envToken).catch(() => false); - if (isAuth) { - logger.info( - 'Already logged in. Use "ek login --force" to re-authenticate with a different account.', - ); - - await configManager.markAuthenticated(); - return; - } else { - await secureStore.clearAll(); - } + await secureStore.clearAll(); } } - const codeVerifier = base64Url(randomBytes(32)); - const codeChallenge = base64Url(createHash("sha256").update(codeVerifier).digest()); - const state = base64Url(randomBytes(32)); + const device = await this.createDeviceCode(); + this.logAuthInstructions(device); - const authResponse = await this.runPkceFlow({ - codeVerifier, - codeChallenge, - state, + open(device.verificationUriComplete).catch((error: unknown) => { + logger.warn("Failed to open browser automatically.", { + fix: `Open this URL manually: ${device.verificationUriComplete}`, + }); + logger.debug(String(error)); }); + const authResponse = await this.pollForToken(device); const userInfo = await this.getUserInfo(authResponse.accessToken); if (!userInfo) { throw new CLIError( @@ -86,327 +82,110 @@ export class Auth { ); } - await this.markAuthenticated(authResponse.accessToken, userInfo); + await secureStore.setAuth({ + accessToken: authResponse.accessToken, + userId: userInfo.id, + email: userInfo.email, + }); + await configManager.markAuthenticated(); } - private async runPkceFlow(params: { - codeVerifier: string; - codeChallenge: string; - state: string; - }): Promise { - const { codeVerifier, codeChallenge, state } = params; - - return new Promise((resolve, reject) => { - let server: ReturnType | null = null; - let timeoutId: ReturnType | null = null; - - const cleanup = () => { - void server?.stop(); - if (timeoutId) clearTimeout(timeoutId); - server = null; - timeoutId = null; - }; - - const fail = (error: Error) => { - cleanup(); - reject(error); - }; + private async createDeviceCode(): Promise { + const response = await http.post( + "/v1/auth/device/code", + { clientId: CLIENT_ID }, + { + validateStatus: () => true, + }, + ); - const handleCallback = async (req: Request): Promise => { - try { - const url = new URL(req.url); - const error = url.searchParams.get("error"); - const errorDesc = url.searchParams.get("error_description") || error || ""; + if (response.status < 200 || response.status >= 300 || !response.data.deviceCode) { + throw new CLIError( + "Could not start device authentication.", + `The server rejected the authentication request (status ${response.status}).`, + 'Run "ek login" to try again.', + ); + } - if (error) { - setTimeout(() => { - fail(new CLIError(`Authentication failed: ${errorDesc}`)); - }, 1000); - return this.authErrorResponse(errorDesc); - } + return response.data; + } - if (url.searchParams.get("state") !== state) { - setTimeout(() => { - fail( - new CLIError( - "Authentication failed due to a security mismatch.", - "The authentication response could not be verified. This can happen if the login session expired.", - 'Run "ek login" to try again.', - ), - ); - }, 1000); - return this.authErrorResponse("Invalid state parameter"); - } + private async pollForToken(device: DeviceCodeResponse): Promise { + const expiresAt = Date.now() + device.expiresIn * 1000; + let cancelled = false; + const cancel = () => { + cancelled = true; + }; + process.once("SIGINT", cancel); - const code = url.searchParams.get("code"); - if (!code) { - setTimeout(() => { - fail( - new CLIError( - "Authentication failed. No authorization was received.", - "The browser did not return a valid authorization code. You may have denied access or the flow was interrupted.", - 'Run "ek login" to try again.', - ), - ); - }, 1000); - return this.authErrorResponse("Missing authorization code"); - } + try { + while (Date.now() < expiresAt) { + if (cancelled) { + throw new CLIError("Authentication cancelled."); + } - const authResp = await this.exchangeCodeForToken(code, codeVerifier); + await new Promise((resolve) => setTimeout(resolve, device.interval * 1000)); + const response = await http.post( + "/v1/auth/device/token", + { clientId: CLIENT_ID, deviceCode: device.deviceCode }, + { validateStatus: () => true }, + ); - setTimeout(() => { - cleanup(); - resolve(authResp); - }, 1000); + if (response.status >= 200 && response.status < 300 && "accessToken" in response.data) { + return response.data; + } - return this.authSuccessResponse(); - } catch (err: unknown) { - setTimeout(() => { - fail(err instanceof Error ? err : new Error(String(err))); - }, 1000); - return new Response("Internal error", { status: 500 }); + const error = "error" in response.data ? response.data.error : undefined; + if (error === "authorization_pending") continue; + if (error === "access_denied") { + throw new CLIError("Authentication was denied in the browser."); } - }; + if (error === "expired_token") break; - try { - server = Bun.serve({ - port: this.CALLBACK_PORT, - routes: { "/callback": handleCallback }, - fetch: () => new Response("Not Found", { status: 404 }), - }); - } catch { - reject( - new CLIError( - "Could not start the login server.", - `Port ${this.CALLBACK_PORT} is already in use by another application.`, - "Close the application using that port and try again.", - ), + throw new CLIError( + "Could not complete the login.", + `The server rejected the authentication request (status ${response.status}).`, + 'Run "ek login" to try again.', ); - return; } - - const authUrl = this.buildAuthUrl(codeChallenge, state); - this.logAuthInstructions(authUrl); - - open(authUrl).catch((openErr: unknown) => { - logger.warn("Failed to open browser automatically.", { - fix: `Open this URL manually: ${authUrl}`, - }); - logger.debug(String(openErr)); - }); - - timeoutId = setTimeout( - () => { - fail( - new CLIError( - "Authentication timed out.", - "No response was received from the browser within the time limit.", - 'Run "ek login" to try again. Make sure to complete the login in your browser.', - ), - ); - }, - 5 * 60 * 1000, - ); - }); - } - - private buildAuthUrl(codeChallenge: string, state: string): string { - const authUrl = new URL("/oauth/authorize", env.APP_BASE_URL); - authUrl.searchParams.set("client_id", this.CLIENT_ID); - authUrl.searchParams.set("response_type", "code"); - authUrl.searchParams.set("redirect_uri", this.REDIRECT_URL); - authUrl.searchParams.set("scope", this.DEFAULT_SCOPES); - authUrl.searchParams.set("state", state); - authUrl.searchParams.set("code_challenge", codeChallenge); - authUrl.searchParams.set("code_challenge_method", "S256"); - - return authUrl.toString(); - } - - private logAuthInstructions(authUrl: string): void { - logger.info("Opening browser for authentication..."); - logger.info(`Authentication URL: ${authUrl}`); - } - - private escapeHtml(text: string): string { - return text - .replace(/&/g, "&") - .replace(//g, ">") - .replace(/"/g, """) - .replace(/'/g, "'"); - } - - // The Enkryptify wordmark logo. The mark keeps the brand blue (#2B7FFF) and - // the wordmark uses the app foreground (#f5f5f5) so it matches the dashboard. - private logoSvg(): string { - return ``; - } - - // Shared page chrome for the browser-facing success/error states. Mirrors - // the app's /oauth/authorize screen: dark brutalist theme, Source Sans 3 / - // JetBrains Mono, centered logo above an animated state icon and two lines - // of copy. Inline HTML/CSS only — no React, no build step. - private renderAuthPage(params: { - state: "success" | "error"; - documentTitle: string; - title: string; - subtitle: string; - hint: string; - autoClose?: boolean; - }): string { - const { state, documentTitle, title, subtitle, hint, autoClose } = params; - - const icon = - state === "success" - ? `` - : ``; - - const autoCloseScript = autoClose ? `` : ""; - - return ` - - - - -${documentTitle} - - - - - - - -
- - ${icon} -
-

${title}

-

${subtitle}

-
-

${hint}

-
-${autoCloseScript} - -`; - } - - private authErrorResponse(message: string): Response { - const html = this.renderAuthPage({ - state: "error", - documentTitle: "Sign-in failed — Enkryptify", - title: "We couldn't sign you in", - subtitle: this.escapeHtml(message), - hint: "Close this tab and try again from your terminal", - }); - return new Response(html, { status: 400, headers: { "Content-Type": "text/html" } }); - } - - private authSuccessResponse(): Response { - const html = this.renderAuthPage({ - state: "success", - documentTitle: "Signed in — Enkryptify", - title: "You're all set", - subtitle: "Signed in to Enkryptify. Head back to your terminal.", - hint: "This tab will close automatically", - autoClose: true, - }); - return new Response(html, { status: 200, headers: { "Content-Type": "text/html" } }); - } - private async exchangeCodeForToken(code: string, codeVerifier: string): Promise { - const payload = { - grant_type: "authorization_code", - client_id: this.CLIENT_ID, - code, - redirect_uri: this.REDIRECT_URL, - code_verifier: codeVerifier, - }; - - const res = await http.post("/v1/auth/token", payload, { - validateStatus: () => true, - }); - - if (res.status < 200 || res.status >= 300) { - throw new CLIError( - "Could not complete the login.", - `The server rejected the authentication request (status ${res.status}).`, - 'Run "ek login" to try again. If this persists, check your network connection.', - ); - } - - const data = res.data as AuthResponse; - if (!data.accessToken) { - throw new CLIError( - "Could not complete the login.", - "The server response was incomplete. No access token was provided.", - 'Run "ek login" to try again.', - ); + } finally { + process.removeListener("SIGINT", cancel); } - return data; + throw new CLIError( + "Authentication timed out.", + "The device verification request expired before it was approved.", + 'Run "ek login" to try again.', + ); } - private async markAuthenticated(accessToken: string, user: UserInfo): Promise { - await secureStore.setAuth({ - accessToken, - userId: user.id, - email: user.email, - }); - - await configManager.markAuthenticated(); + private logAuthInstructions(device: DeviceCodeResponse): void { + logger.info("Opening browser for authentication..."); + logger.info(`Verification code: ${device.userCode}`); + logger.info(`Authentication URL: ${device.verificationUriComplete}`); } async getUserInfo(token: string): Promise { - const res = await http.get("/v1/me", { - headers: { - Authorization: `Bearer ${token}`, - }, + const response = await http.get("/v1/me", { + headers: { Authorization: `Bearer ${token}` }, validateStatus: () => true, }); - if (res.status === 401 || res.status === 403) { - return null; - } - - if (res.status < 200 || res.status >= 300) { + if (response.status === 401 || response.status === 403) return null; + if (response.status < 200 || response.status >= 300) { throw new CLIError( "Could not retrieve your account information.", - `The server returned an unexpected response (status ${res.status}).`, + `The server returned an unexpected response (status ${response.status}).`, 'Run "ek login" to try again.', ); } - return res.data as UserInfo; + return response.data as UserInfo; } async getCredentials(): Promise { const authData = await secureStore.getAuth(); - if (!authData?.accessToken) { - throw CLIError.from("AUTH_NOT_LOGGED_IN"); - } - + if (!authData?.accessToken) throw CLIError.from("AUTH_NOT_LOGGED_IN"); return { accessToken: authData.accessToken }; } } diff --git a/src/lib/keyring.ts b/src/lib/keyring.ts index 79917ca..0b807cb 100644 --- a/src/lib/keyring.ts +++ b/src/lib/keyring.ts @@ -1,7 +1,11 @@ -import * as keytar from "keytar"; import { logger } from "@/lib/logger"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import type * as KeytarModule from "keytar"; const SERVICE_NAME = "enkryptify-cli"; +const STORE_PATH = process.env.ENKRYPTIFY_STORE_PATH ?? path.join(os.homedir(), ".enkryptify", "secure-store.json"); export interface Keyring { set(key: string, value: string): Promise; @@ -11,37 +15,116 @@ export interface Keyring { } class OSKeyring implements Keyring { + private keytar?: Promise; + + private load(): Promise { + this.keytar ??= import("keytar"); + return this.keytar; + } + async set(key: string, value: string): Promise { - try { - await keytar.setPassword(SERVICE_NAME, key, value); - } catch (error: unknown) { - logger.debug(`Keyring set failed: ${error instanceof Error ? error.message : String(error)}`); - throw error; - } + await (await this.load()).setPassword(SERVICE_NAME, key, value); } async get(key: string): Promise { + return (await this.load()).getPassword(SERVICE_NAME, key); + } + + async delete(key: string): Promise { + await (await this.load()).deletePassword(SERVICE_NAME, key); + } + + async has(key: string): Promise { + return (await this.get(key)) !== null; + } +} + +class FileKeyring implements Keyring { + private async read(): Promise> { try { - return await keytar.getPassword(SERVICE_NAME, key); + const raw = await fs.readFile(STORE_PATH, "utf8"); + const parsed: unknown = JSON.parse(raw); + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return {}; + + return Object.fromEntries( + Object.entries(parsed).filter((entry): entry is [string, string] => typeof entry[1] === "string"), + ); } catch (error: unknown) { - logger.debug(`Keyring get failed: ${error instanceof Error ? error.message : String(error)}`); - throw error; + if (error instanceof Error && "code" in error && error.code === "ENOENT") return {}; + logger.debug( + `File credential store read failed: ${error instanceof Error ? error.message : String(error)}`, + ); + return {}; } } + private async write(data: Record): Promise { + const directory = path.dirname(STORE_PATH); + const temporaryPath = `${STORE_PATH}.${process.pid}.tmp`; + await fs.mkdir(directory, { recursive: true, mode: 0o700 }); + await fs.chmod(directory, 0o700); + await fs.writeFile(temporaryPath, JSON.stringify(data), { mode: 0o600 }); + await fs.rename(temporaryPath, STORE_PATH); + await fs.chmod(STORE_PATH, 0o600); + } + + async set(key: string, value: string): Promise { + await this.write({ ...(await this.read()), [key]: value }); + } + + async get(key: string): Promise { + return (await this.read())[key] ?? null; + } + async delete(key: string): Promise { + const data = await this.read(); + delete data[key]; + if (Object.keys(data).length === 0) { + await fs.rm(STORE_PATH, { force: true }); + return; + } + await this.write(data); + } + + async has(key: string): Promise { + return (await this.get(key)) !== null; + } +} + +class FallbackKeyring implements Keyring { + private readonly os = new OSKeyring(); + private readonly file = new FileKeyring(); + private active: Keyring | undefined = process.env.ENKRYPTIFY_STORE_PATH ? this.file : undefined; + + private async run(operation: (store: Keyring) => Promise): Promise { + if (this.active) return operation(this.active); try { - await keytar.deletePassword(SERVICE_NAME, key); + const result = await operation(this.os); + this.active = this.os; + return result; } catch (error: unknown) { - logger.debug(`Keyring delete failed: ${error instanceof Error ? error.message : String(error)}`); - throw error; + logger.debug(`OS keyring unavailable: ${error instanceof Error ? error.message : String(error)}`); + logger.warn("OS keyring unavailable; using the protected file credential store."); + this.active = this.file; + return operation(this.file); } } - async has(key: string): Promise { - const value = await this.get(key); - return value !== null; + set(key: string, value: string): Promise { + return this.run((store) => store.set(key, value)); + } + + get(key: string): Promise { + return this.run((store) => store.get(key)); + } + + delete(key: string): Promise { + return this.run((store) => store.delete(key)); + } + + has(key: string): Promise { + return this.run((store) => store.has(key)); } } -export const keyring = new OSKeyring(); +export const keyring = new FallbackKeyring(); diff --git a/tests/integration/file-keyring.test.ts b/tests/integration/file-keyring.test.ts new file mode 100644 index 0000000..fa0a82b --- /dev/null +++ b/tests/integration/file-keyring.test.ts @@ -0,0 +1,52 @@ +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/logger", () => ({ + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), success: vi.fn() }, +})); + +describe("file credential store", () => { + let temporaryDirectory: string; + let storePath: string; + + beforeEach(async () => { + temporaryDirectory = await fs.mkdtemp(path.join(os.tmpdir(), "enkryptify-store-")); + storePath = path.join(temporaryDirectory, "credentials", "secure-store.json"); + process.env.ENKRYPTIFY_STORE_PATH = storePath; + vi.resetModules(); + }); + + afterEach(async () => { + delete process.env.ENKRYPTIFY_STORE_PATH; + await fs.rm(temporaryDirectory, { recursive: true, force: true }); + }); + + it("stores credentials with owner-only permissions", async () => { + const { keyring } = await import("@/lib/keyring"); + + await keyring.set("enkryptify", "secret-value"); + + await expect(keyring.get("enkryptify")).resolves.toBe("secret-value"); + expect((await fs.stat(path.dirname(storePath))).mode & 0o777).toBe(0o700); + expect((await fs.stat(storePath)).mode & 0o777).toBe(0o600); + }); + + it("removes the fallback file when its final value is deleted", async () => { + const { keyring } = await import("@/lib/keyring"); + await keyring.set("enkryptify", "secret-value"); + + await keyring.delete("enkryptify"); + + await expect(fs.stat(storePath)).rejects.toMatchObject({ code: "ENOENT" }); + }); + + it("treats a corrupted fallback file as empty", async () => { + await fs.mkdir(path.dirname(storePath), { recursive: true }); + await fs.writeFile(storePath, JSON.stringify({ enkryptify: 123 })); + const { keyring } = await import("@/lib/keyring"); + + await expect(keyring.get("enkryptify")).resolves.toBeNull(); + }); +}); diff --git a/tests/integration/login-flow.test.ts b/tests/integration/login-flow.test.ts index 2b95bbc..4c6c46a 100644 --- a/tests/integration/login-flow.test.ts +++ b/tests/integration/login-flow.test.ts @@ -26,320 +26,156 @@ vi.mock("@/lib/logger", () => ({ })); vi.mock("@/lib/config", () => ({ - config: { - markAuthenticated: vi.fn().mockResolvedValue(undefined), - }, + config: { markAuthenticated: vi.fn().mockResolvedValue(undefined) }, })); vi.mock("@/api/httpClient", () => ({ - default: { - get: vi.fn(), - post: vi.fn(), - put: vi.fn(), - delete: vi.fn(), - }, + default: { get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn() }, })); -vi.mock("open", () => ({ - default: vi.fn().mockResolvedValue(undefined), -})); +vi.mock("open", () => ({ default: vi.fn().mockResolvedValue(undefined) })); import http from "@/api/httpClient"; import { config as configManager } from "@/lib/config"; +import { logger } from "@/lib/logger"; import open from "open"; import { Auth } from "@/api/auth"; -describe("Auth.login() flow (integration)", () => { - let auth: Auth; - let capturedRoutes: Record; - let mockServer: { stop: ReturnType; port: number }; +const DEVICE_RESPONSE = { + deviceCode: "device-code-with-at-least-thirty-two-characters", + userCode: "ABCDE-12345", + verificationUri: "https://app.enkryptify.com/oauth/device", + verificationUriComplete: "https://app.enkryptify.com/oauth/device?user_code=ABCDE-12345", + expiresIn: 300, + interval: 0, +}; +const TOKEN_RESPONSE = { accessToken: "new-access-token-xyz", tokenType: "Bearer", expiresIn: 28800 }; + +describe("Auth.login() device flow", () => { beforeEach(() => { vi.clearAllMocks(); mockKeyring.reset(); - capturedRoutes = {}; - mockServer = { stop: vi.fn(), port: 51823 }; - - vi.stubGlobal("Bun", { - serve: vi.fn((serverConfig: Record) => { - const routes = serverConfig.routes as Record | undefined; - if (routes) { - for (const [path, handler] of Object.entries(routes)) { - capturedRoutes[path] = handler; - } - } - return mockServer; - }), + vi.mocked(http.get).mockImplementation((_url: string, config?: AxiosRequestConfig) => { + if (_url !== "/v1/me") return Promise.reject(new Error(`Unexpected URL: ${_url}`)); + const authorization = (config?.headers as Record | undefined)?.Authorization; + if (!authorization) return Promise.resolve({ status: 401, data: null }); + return Promise.resolve({ + status: 200, + data: { id: "user-test-123", email: "test@enkryptify.com", name: "Test User" }, + }); }); - // Default: getUserInfo returns valid user for token validation - vi.mocked(http.get).mockImplementation((_url: string, reqConfig?: AxiosRequestConfig) => { - if (_url === "/v1/me") { - const authHeader = (reqConfig?.headers as Record | undefined)?.Authorization; - if (authHeader) { - return Promise.resolve({ - status: 200, - data: { id: "user-test-123", email: "test@enkryptify.com", name: "Test User" }, - }); - } - return Promise.resolve({ status: 401, data: null }); - } - return Promise.reject(new Error(`Unexpected URL: ${_url}`)); - }); - - // Default: token exchange succeeds + let polls = 0; vi.mocked(http.post).mockImplementation((_url: string) => { - if (_url === "/v1/auth/token") { - return Promise.resolve({ - status: 200, - data: { accessToken: "new-access-token-xyz", tokenType: "Bearer", expiresIn: 3600 }, - }); + if (_url === "/v1/auth/device/code") return Promise.resolve({ status: 200, data: DEVICE_RESPONSE }); + if (_url === "/v1/auth/device/token") { + polls += 1; + return polls === 1 + ? Promise.resolve({ status: 400, data: { error: "authorization_pending" } }) + : Promise.resolve({ status: 200, data: TOKEN_RESPONSE }); } return Promise.reject(new Error(`Unexpected URL: ${_url}`)); }); - - auth = new Auth(); }); afterEach(() => { - vi.unstubAllGlobals(); vi.restoreAllMocks(); }); - // --- Helpers --- - - /** Give the async login flow time to reach Bun.serve + open */ - async function waitForPkceSetup() { - await new Promise((r) => setTimeout(r, 100)); - } - - /** Simulate the OAuth callback hitting the local server */ - async function simulateCallback(params: Record): Promise { - const handler = capturedRoutes["/callback"]; - expect(handler).toBeDefined(); - - const url = new URL("http://localhost:51823/callback"); - for (const [k, v] of Object.entries(params)) { - url.searchParams.set(k, v); - } - return (handler as (req: Request) => Promise)(new Request(url.toString())); - } - - /** Extract the state parameter from the auth URL that `open` was called with */ - function extractState(): string { - const openCall = vi.mocked(open).mock.calls[0]; - expect(openCall).toBeDefined(); - const authUrl = new URL(openCall![0] as string); - const state = authUrl.searchParams.get("state"); - expect(state).toBeTruthy(); - return state!; - } - - // --- Skip login when already authenticated --- - - it("skips login when valid token exists and --force is not set", async () => { + it("skips device authentication when a valid token exists", async () => { await mockKeyring.set("enkryptify", JSON.stringify(FAKE_AUTH_DATA)); - await auth.login(); + await new Auth().login(); - // Should NOT have started Bun.serve (no PKCE flow needed) - expect(Bun.serve).not.toHaveBeenCalled(); - // Should have called markAuthenticated - expect(configManager.markAuthenticated).toHaveBeenCalled(); + expect(http.post).not.toHaveBeenCalled(); + expect(configManager.markAuthenticated).toHaveBeenCalledOnce(); }); - it("deletes old token and re-authenticates when --force is set", async () => { - await mockKeyring.set("enkryptify", JSON.stringify(FAKE_AUTH_DATA)); - - const loginPromise = auth.login({ force: true }); - await waitForPkceSetup(); - - // Should have started Bun.serve for PKCE flow - expect(Bun.serve).toHaveBeenCalled(); - - // Complete the flow - const state = extractState(); - await simulateCallback({ code: "test-auth-code", state }); - await loginPromise; - - // New token should be stored (old one replaced) - const stored = await mockKeyring.get("enkryptify"); - const parsed = JSON.parse(stored!); - expect(parsed.auth.accessToken).toBe("new-access-token-xyz"); - }, 5000); - - // --- PKCE flow --- - - it("starts server and opens browser with correct auth URL params", async () => { - const loginPromise = auth.login(); - await waitForPkceSetup(); - - expect(Bun.serve).toHaveBeenCalledOnce(); - - const openCall = vi.mocked(open).mock.calls[0]; - expect(openCall).toBeDefined(); - const authUrl = new URL(openCall![0] as string); - - expect(authUrl.searchParams.get("client_id")).toBe("enkryptify-cli"); - expect(authUrl.searchParams.get("response_type")).toBe("code"); - expect(authUrl.searchParams.get("redirect_uri")).toBe("http://localhost:51823/callback"); - expect(authUrl.searchParams.get("code_challenge_method")).toBe("S256"); - expect(authUrl.searchParams.get("state")).toBeTruthy(); - expect(authUrl.searchParams.get("code_challenge")).toBeTruthy(); - - // Complete the flow so the promise resolves - const state = extractState(); - await simulateCallback({ code: "test-code", state }); - await loginPromise; - }, 5000); - - it("completes flow when valid callback received (code + matching state)", async () => { - const loginPromise = auth.login(); - await waitForPkceSetup(); - - const state = extractState(); - await simulateCallback({ code: "valid-code", state }); + it("opens the verification URL, polls, and stores the approved token", async () => { + await new Auth().login(); - await expect(loginPromise).resolves.toBeUndefined(); - - // Token exchange should have been called + expect(open).toHaveBeenCalledWith(DEVICE_RESPONSE.verificationUriComplete); + expect(logger.info).toHaveBeenCalledWith(`Verification code: ${DEVICE_RESPONSE.userCode}`); expect(http.post).toHaveBeenCalledWith( - "/v1/auth/token", - expect.objectContaining({ - grant_type: "authorization_code", - code: "valid-code", - }), + "/v1/auth/device/token", + { clientId: "enkryptify-cli", deviceCode: DEVICE_RESPONSE.deviceCode }, expect.any(Object), ); - }, 5000); - - it("rejects when state parameter doesn't match (CSRF protection)", async () => { - const loginPromise = auth.login(); - await waitForPkceSetup(); - - // Send callback with wrong state - await simulateCallback({ code: "valid-code", state: "wrong-state-value" }); - - try { - await loginPromise; - expect.unreachable("Should have thrown"); - } catch (error: unknown) { - expect(error).toBeInstanceOf(CLIError); - expect((error as CLIError).message).toContain("security mismatch"); - } - }, 5000); - - it("rejects when no code in callback", async () => { - const loginPromise = auth.login(); - await waitForPkceSetup(); - - const state = extractState(); - // Send callback with state but no code - await simulateCallback({ state }); - - try { - await loginPromise; - expect.unreachable("Should have thrown"); - } catch (error: unknown) { - expect(error).toBeInstanceOf(CLIError); - expect((error as CLIError).message).toContain("No authorization was received"); - } - }, 5000); - - // --- Token storage --- - - it("stores access token in keyring after successful exchange", async () => { - const loginPromise = auth.login(); - await waitForPkceSetup(); - - const state = extractState(); - await simulateCallback({ code: "test-code", state }); - await loginPromise; - - const stored = await mockKeyring.get("enkryptify"); - expect(stored).toBeTruthy(); - const parsed = JSON.parse(stored!); - expect(parsed).toEqual({ - version: 1, - auth: { - accessToken: "new-access-token-xyz", - userId: "user-test-123", - email: "test@enkryptify.com", - }, + const stored = JSON.parse((await mockKeyring.get("enkryptify"))!); + expect(stored.auth).toEqual({ + accessToken: TOKEN_RESPONSE.accessToken, + userId: "user-test-123", + email: "test@enkryptify.com", }); - }, 5000); + expect(configManager.markAuthenticated).toHaveBeenCalledOnce(); + }); - it("calls config.markAuthenticated() after storing token", async () => { - const loginPromise = auth.login(); - await waitForPkceSetup(); + it("continues when the browser cannot be opened", async () => { + vi.mocked(open).mockRejectedValueOnce(new Error("no browser")); - const state = extractState(); - await simulateCallback({ code: "test-code", state }); - await loginPromise; + await expect(new Auth().login()).resolves.toBeUndefined(); + await Promise.resolve(); - expect(configManager.markAuthenticated).toHaveBeenCalled(); - }, 5000); + expect(logger.warn).toHaveBeenCalledWith( + "Failed to open browser automatically.", + expect.objectContaining({ fix: expect.stringContaining(DEVICE_RESPONSE.verificationUriComplete) }), + ); + }); - it("throws CLIError on token exchange failure (non-2xx response)", async () => { + it("fails when device authorization is denied", async () => { vi.mocked(http.post).mockImplementation((_url: string) => { - if (_url === "/v1/auth/token") { - return Promise.resolve({ - status: 400, - data: { error: "invalid_grant" }, - }); - } - return Promise.reject(new Error(`Unexpected URL: ${_url}`)); + if (_url === "/v1/auth/device/code") return Promise.resolve({ status: 200, data: DEVICE_RESPONSE }); + return Promise.resolve({ status: 400, data: { error: "access_denied" } }); }); - const loginPromise = auth.login(); - await waitForPkceSetup(); - - const state = extractState(); - await simulateCallback({ code: "bad-code", state }); + await expect(new Auth().login()).rejects.toThrow("denied"); + }); - try { - await loginPromise; - expect.unreachable("Should have thrown"); - } catch (error: unknown) { - expect(error).toBeInstanceOf(CLIError); - expect((error as CLIError).message).toContain("Could not complete the login"); - } - }, 5000); + it("fails when the device code expires", async () => { + vi.mocked(http.post).mockImplementation((_url: string) => { + if (_url === "/v1/auth/device/code") return Promise.resolve({ status: 200, data: DEVICE_RESPONSE }); + return Promise.resolve({ status: 400, data: { error: "expired_token" } }); + }); - // --- Expired token re-authentication --- + await expect(new Auth().login()).rejects.toThrow("timed out"); + }); - it("re-authenticates when stored token is invalid (401 from /v1/me)", async () => { + it("forces a new device flow when requested", async () => { await mockKeyring.set("enkryptify", JSON.stringify(FAKE_AUTH_DATA)); - // getUserInfo returns 401 for the old token, 200 for the new one - vi.mocked(http.get).mockImplementation((_url: string, reqConfig?: AxiosRequestConfig) => { - if (_url === "/v1/me") { - const authHeader = (reqConfig?.headers as Record | undefined)?.Authorization; - if (authHeader === `Bearer ${FAKE_TOKEN}`) { - return Promise.resolve({ status: 401, data: null }); - } - return Promise.resolve({ - status: 200, - data: { id: "user-test-123", email: "test@enkryptify.com", name: "Test User" }, - }); - } - return Promise.reject(new Error(`Unexpected URL: ${_url}`)); + await new Auth().login({ force: true }); + + expect(http.post).toHaveBeenCalledWith( + "/v1/auth/device/code", + { clientId: "enkryptify-cli" }, + expect.any(Object), + ); + const stored = JSON.parse((await mockKeyring.get("enkryptify"))!); + expect(stored.auth.accessToken).toBe(TOKEN_RESPONSE.accessToken); + }); + + it("re-authenticates when the stored token is invalid", async () => { + await mockKeyring.set("enkryptify", JSON.stringify(FAKE_AUTH_DATA)); + vi.mocked(http.get).mockImplementation((_url: string, config?: AxiosRequestConfig) => { + const authorization = (config?.headers as Record | undefined)?.Authorization; + if (authorization === `Bearer ${FAKE_TOKEN}`) return Promise.resolve({ status: 401, data: null }); + return Promise.resolve({ + status: 200, + data: { id: "user-test-123", email: "test@enkryptify.com", name: "Test User" }, + }); }); - const loginPromise = auth.login(); - await waitForPkceSetup(); + await new Auth().login(); - // Should have started PKCE flow because old token was invalid - expect(Bun.serve).toHaveBeenCalled(); + const stored = JSON.parse((await mockKeyring.get("enkryptify"))!); + expect(stored.auth.accessToken).toBe(TOKEN_RESPONSE.accessToken); + }); - const state = extractState(); - await simulateCallback({ code: "re-auth-code", state }); - await loginPromise; + it("reports a device-code creation failure", async () => { + vi.mocked(http.post).mockResolvedValueOnce({ status: 503, data: {} }); - // New token should be stored - const stored = await mockKeyring.get("enkryptify"); - const parsed = JSON.parse(stored!); - expect(parsed.auth.accessToken).toBe("new-access-token-xyz"); - }, 5000); + await expect(new Auth().login()).rejects.toBeInstanceOf(CLIError); + }); }); From 8625049dbe36249a564378f9b18380af8c484561 Mon Sep 17 00:00:00 2001 From: SiebeBaree Date: Mon, 13 Jul 2026 13:36:18 +0200 Subject: [PATCH 2/2] Harden credential error handling --- src/api/auth.ts | 12 ++++++++---- src/lib/keyring.ts | 15 ++++++++++++--- tests/integration/file-keyring.test.ts | 15 ++++++++++++++- tests/integration/login-flow.test.ts | 11 +++++++++++ 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/api/auth.ts b/src/api/auth.ts index 0f56874..c9a7875 100644 --- a/src/api/auth.ts +++ b/src/api/auth.ts @@ -53,11 +53,15 @@ export class Auth { if (accessToken) { if (options?.force) { await secureStore.clearAll(); - } else if (await this.getUserInfo(accessToken).catch(() => null)) { - logger.info('Already logged in. Use "ek login --force" to re-authenticate with a different account.'); - await configManager.markAuthenticated(); - return; } else { + const userInfo = await this.getUserInfo(accessToken); + if (userInfo) { + logger.info( + 'Already logged in. Use "ek login --force" to re-authenticate with a different account.', + ); + await configManager.markAuthenticated(); + return; + } await secureStore.clearAll(); } } diff --git a/src/lib/keyring.ts b/src/lib/keyring.ts index 0b807cb..f7872f0 100644 --- a/src/lib/keyring.ts +++ b/src/lib/keyring.ts @@ -41,8 +41,18 @@ class OSKeyring implements Keyring { class FileKeyring implements Keyring { private async read(): Promise> { + let raw: string; + try { + raw = await fs.readFile(STORE_PATH, "utf8"); + } catch (error: unknown) { + if (error instanceof Error && "code" in error && error.code === "ENOENT") return {}; + logger.debug( + `File credential store read failed: ${error instanceof Error ? error.message : String(error)}`, + ); + throw error; + } + try { - const raw = await fs.readFile(STORE_PATH, "utf8"); const parsed: unknown = JSON.parse(raw); if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return {}; @@ -50,9 +60,8 @@ class FileKeyring implements Keyring { Object.entries(parsed).filter((entry): entry is [string, string] => typeof entry[1] === "string"), ); } catch (error: unknown) { - if (error instanceof Error && "code" in error && error.code === "ENOENT") return {}; logger.debug( - `File credential store read failed: ${error instanceof Error ? error.message : String(error)}`, + `File credential store is corrupted: ${error instanceof Error ? error.message : String(error)}`, ); return {}; } diff --git a/tests/integration/file-keyring.test.ts b/tests/integration/file-keyring.test.ts index fa0a82b..e69f24e 100644 --- a/tests/integration/file-keyring.test.ts +++ b/tests/integration/file-keyring.test.ts @@ -44,9 +44,22 @@ describe("file credential store", () => { it("treats a corrupted fallback file as empty", async () => { await fs.mkdir(path.dirname(storePath), { recursive: true }); - await fs.writeFile(storePath, JSON.stringify({ enkryptify: 123 })); + await fs.writeFile(storePath, "not-json"); const { keyring } = await import("@/lib/keyring"); await expect(keyring.get("enkryptify")).resolves.toBeNull(); }); + + it("does not overwrite the fallback file when reading it fails", async () => { + const original = JSON.stringify({ enkryptify: "secret-value" }); + await fs.mkdir(path.dirname(storePath), { recursive: true }); + await fs.writeFile(storePath, original, { mode: 0o600 }); + await fs.chmod(storePath, 0o000); + const { keyring } = await import("@/lib/keyring"); + + await expect(keyring.set("enkryptify", "replacement-value")).rejects.toMatchObject({ code: "EACCES" }); + + await fs.chmod(storePath, 0o600); + await expect(fs.readFile(storePath, "utf8")).resolves.toBe(original); + }); }); diff --git a/tests/integration/login-flow.test.ts b/tests/integration/login-flow.test.ts index 4c6c46a..5db05f4 100644 --- a/tests/integration/login-flow.test.ts +++ b/tests/integration/login-flow.test.ts @@ -173,6 +173,17 @@ describe("Auth.login() device flow", () => { expect(stored.auth.accessToken).toBe(TOKEN_RESPONSE.accessToken); }); + it("preserves the stored token when validation fails transiently", async () => { + await mockKeyring.set("enkryptify", JSON.stringify(FAKE_AUTH_DATA)); + vi.mocked(http.get).mockRejectedValueOnce(new Error("network unavailable")); + + await expect(new Auth().login()).rejects.toThrow("network unavailable"); + + const stored = JSON.parse((await mockKeyring.get("enkryptify"))!); + expect(stored.auth.accessToken).toBe(FAKE_TOKEN); + expect(http.post).not.toHaveBeenCalled(); + }); + it("reports a device-code creation failure", async () => { vi.mocked(http.post).mockResolvedValueOnce({ status: 503, data: {} });