diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 46f7c82c50e..b630ef3b844 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -44,7 +44,10 @@ jobs: for node in $node_versions; do OS_NODE+=("ubuntu-latest:$node") done - if [[ "$HEAD_REF" == release-please--* ]]; then + # XXX(serhalp): also run the macOS + Windows matrix on this branch so we can verify the new keychain code + # paths before merge. + # Revert before merging. + if [[ "$HEAD_REF" == release-please--* || "$HEAD_REF" == "serhalp/feat/native-token-keychain-storage" ]]; then for os in $extra_oses; do OS_NODE+=("$os:$primary") done diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4aeac96f0ae..26f252cc306 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -137,7 +137,8 @@ Every `track()` event includes these properties, which callers can't override: - `ci_is_pr`: whether the CI run is for a pull request, on platforms that expose it Properties with no value are omitted. The `cli:command` event sent after each command also includes `command`, `opts` -(with sensitive values masked), `args`, `frameworks`, `buildSystem`, `packageManager`, and `monorepo`. +(with sensitive values masked), `args`, `frameworks`, `buildSystem`, `packageManager`, `monorepo`, and +`authTokenSource` (`flag`, `env`, `config`, `keychain`, or `not found`). The test suites set `NETLIFY_TELEMETRY_DISABLED` in their Vitest config, so tests never send real telemetry. Tests that spawn the CLI with `extendEnv: false` must pass it explicitly. diff --git a/docs/commands/login.md b/docs/commands/login.md index 6304e5dceb4..c7b406c0c2a 100644 --- a/docs/commands/login.md +++ b/docs/commands/login.md @@ -9,7 +9,8 @@ description: Login to your Netlify account Login to your Netlify account -Opens a web browser to acquire an OAuth token. +Opens a web browser to acquire an OAuth token, which is stored in your OS keychain when available. +To store it in a plaintext config file instead, set NETLIFY_USE_LEGACY_AUTH_STORAGE=1. This is not recommended. **Usage** diff --git a/docs/index.md b/docs/index.md index 528cda2c788..209ebe8b41b 100644 --- a/docs/index.md +++ b/docs/index.md @@ -235,6 +235,6 @@ A program that runs `netlify login` on a user’s behalf can set `NETLIFY_LOGIN_ ## Telemetry -The CLI sends usage telemetry and error reports to `cli.netlify.com`. Usage telemetry covers which commands run, their flags with sensitive values masked, the CLI and Node.js versions, the detected framework and package manager, the AI agent running the CLI, and the CI platform. +The CLI sends usage telemetry and error reports to `cli.netlify.com`. Usage telemetry covers which commands run, their flags with sensitive values masked, the CLI and Node.js versions, the detected framework and package manager, the AI agent running the CLI, the CI platform, and where the auth token is stored (never the token itself). To disable telemetry, run `netlify --telemetry-disable`, or set `NETLIFY_TELEMETRY_DISABLED=1` or `DO_NOT_TRACK=1`. The environment variables also disable error reporting, and are the most reliable option in CI, where the setting saved by `--telemetry-disable` may not persist between runs. diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 5d76d095ed1..1568bd170a7 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -45,7 +45,7 @@ "count": 7 }, "@typescript-eslint/no-unsafe-assignment": { - "count": 14 + "count": 13 }, "@typescript-eslint/no-unsafe-call": { "count": 2 diff --git a/package-lock.json b/package-lock.json index 8f739a04cea..c3282da044c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,7 +19,7 @@ "@netlify/build-info": "^11.3.1", "@netlify/config": "^25.5.1", "@netlify/dev": "^5.1.12", - "@netlify/dev-utils": "^6.0.4", + "@netlify/dev-utils": "https://pkg.pr.new/@netlify/dev-utils@2b6145e", "@netlify/edge-bundler": "^16.2.0", "@netlify/edge-functions": "^4.0.3", "@netlify/edge-functions-bootstrap": "^3.2.0", @@ -1386,9 +1386,9 @@ } }, "node_modules/@fastify/busboy": { - "version": "3.2.3", - "resolved": "https://registry.npmjs.org/@fastify/busboy/-/busboy-3.2.3.tgz", - "integrity": "sha512-fmo/IKgkenPzz3qTKG+sIUXdF/Kv83AkvvOXJHPTHJeKJbat8SsL1ihUaFaDotqoEl4UZ1Lf8Eb/3vK6eaWX2A==", + "version": "3.2.4", + "resolved": "https://registry.npmjs.org/@fastify/busboy/-/busboy-3.2.4.tgz", + "integrity": "sha512-lQS8xcXT54rdJb+wlG/Q7SMSdtkZEpLPHooSDUZyyZm9LJD7X1kONPhVoPaHq+dsPtWUGGLIz9gBNhs9qKbylw==", "license": "MIT" }, "node_modules/@fastify/error": { @@ -3119,6 +3119,240 @@ "node": ">=18" } }, + "node_modules/@napi-rs/keyring": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring/-/keyring-2.1.2.tgz", + "integrity": "sha512-9i1SR6tYeQ3o1beLeVMxhN6Fq513m9TZlTs+46+4JgvSNkaFoW9S4WrpV5MjzE+0CzVoC+dZ4RfZDIgW6+afnQ==", + "license": "MIT", + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + }, + "optionalDependencies": { + "@napi-rs/keyring-darwin-arm64": "2.1.2", + "@napi-rs/keyring-darwin-x64": "2.1.2", + "@napi-rs/keyring-freebsd-x64": "2.1.2", + "@napi-rs/keyring-linux-arm-gnueabihf": "2.1.2", + "@napi-rs/keyring-linux-arm64-gnu": "2.1.2", + "@napi-rs/keyring-linux-arm64-musl": "2.1.2", + "@napi-rs/keyring-linux-riscv64-gnu": "2.1.2", + "@napi-rs/keyring-linux-x64-gnu": "2.1.2", + "@napi-rs/keyring-linux-x64-musl": "2.1.2", + "@napi-rs/keyring-win32-arm64-msvc": "2.1.2", + "@napi-rs/keyring-win32-ia32-msvc": "2.1.2", + "@napi-rs/keyring-win32-x64-msvc": "2.1.2" + } + }, + "node_modules/@napi-rs/keyring-darwin-arm64": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-darwin-arm64/-/keyring-darwin-arm64-2.1.2.tgz", + "integrity": "sha512-vsInUWUzWS6/WzR/TFNwmdtmwKWjnnR8TOpvZ/pjjxDAkJB/waz37PWzp2b8AFcw1aCcq2aQ7k2vvbLQy8pTrw==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-darwin-x64": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-darwin-x64/-/keyring-darwin-x64-2.1.2.tgz", + "integrity": "sha512-wQihYroaQ3r++R6u5c9Gj0TCw1l1F53LCb5aLOXfVvT4DsnDnpe7p7VGYo4/33bLe0iqVevtAj+ET3yKCtOlJQ==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-freebsd-x64": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-freebsd-x64/-/keyring-freebsd-x64-2.1.2.tgz", + "integrity": "sha512-Ve5ha7MbJsq+4TYyXYiTy4IeZOnYxkBB05uH2+nIrEKBkHmyfHttF814EfWAfoJa63KqBhYRXdYJo0o1WVL2sg==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm-gnueabihf": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm-gnueabihf/-/keyring-linux-arm-gnueabihf-2.1.2.tgz", + "integrity": "sha512-zxijTnVnTApEyJMjgnLdVQL1g1DAudku/5brrWNDRWj4J0q0nFhUZEtzUA+nY2FvgDjl4s+2YCZYXSTH4TFMUg==", + "cpu": [ + "arm" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm64-gnu": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm64-gnu/-/keyring-linux-arm64-gnu-2.1.2.tgz", + "integrity": "sha512-PM+SAcl/yGzgGbeeCsKrnlG4T5DBIDjmDqtKm5an1bJz0o0gwSzR3BMiXf04JenkcqascZpdcIReoEvLj0cvCw==", + "cpu": [ + "arm64" + ], + "libc": [ + "glibc" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm64-musl": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm64-musl/-/keyring-linux-arm64-musl-2.1.2.tgz", + "integrity": "sha512-+ERS9oyInPCALN957cViBDbYJc724/Q+rZ8ZAz4rOHj4B4v0VR4OAWThdYZgZqD54sZVV0MdyX24qva97hYoAQ==", + "cpu": [ + "arm64" + ], + "libc": [ + "musl" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-riscv64-gnu": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-riscv64-gnu/-/keyring-linux-riscv64-gnu-2.1.2.tgz", + "integrity": "sha512-pGADtqeT3U1SaFZGjkVsmZVjeNOQx0fr77S+NWszXjZboyaUOLlGKgqG6ssAWzcwxv8uPhKc9Ij17ujNZhSqLw==", + "cpu": [ + "riscv64" + ], + "libc": [ + "glibc" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-x64-gnu": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-x64-gnu/-/keyring-linux-x64-gnu-2.1.2.tgz", + "integrity": "sha512-Fj5TmTnl2oriuCT5DDEYlhgxYes20uGfgOg10LK9PyPVsI8/PQ1m8I4rq/4A5hkrMtY7wvFb70YmX/yPHDYWvQ==", + "cpu": [ + "x64" + ], + "libc": [ + "glibc" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-x64-musl": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-x64-musl/-/keyring-linux-x64-musl-2.1.2.tgz", + "integrity": "sha512-C+KhAeU6x2A43FV3dOmMvUraJb8zsC9VZfsunjZ1cgtuAb208TALaUuAY0jThOphXqhyhBJOs2xTCQhOrRkJIw==", + "cpu": [ + "x64" + ], + "libc": [ + "musl" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-arm64-msvc": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-arm64-msvc/-/keyring-win32-arm64-msvc-2.1.2.tgz", + "integrity": "sha512-kBq80ZZfjXoa5wHG9kgAVhYYjaPOLgHh5ngt7r1ftwFXPXDPXKH7oM2r2j2Q64cs0pVtmodVtn5Q9e6xCwzQUQ==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-ia32-msvc": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-ia32-msvc/-/keyring-win32-ia32-msvc-2.1.2.tgz", + "integrity": "sha512-bhgKvVbWlZ6iR/tmvb2ht6/o0OzarUX9W7hjg+G+eVdGjR1prwtQ9lYo5ZESdn/+zyF1MIFvHkOCGja9YqDcww==", + "cpu": [ + "ia32" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-x64-msvc": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-x64-msvc/-/keyring-win32-x64-msvc-2.1.2.tgz", + "integrity": "sha512-yQtFdfuwqM0ezKlYO6r2n44in2LGzd4MC679Ddhvojip3t1hn+q/4I+gJU+cIJQVd5FAZSKNrK4tGfew3UTpAQ==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, "node_modules/@netlify/ai": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/@netlify/ai/-/ai-1.0.2.tgz", @@ -3855,10 +4089,11 @@ }, "node_modules/@netlify/dev-utils": { "version": "6.0.4", - "resolved": "https://registry.npmjs.org/@netlify/dev-utils/-/dev-utils-6.0.4.tgz", - "integrity": "sha512-VZgq2gqRLQual8P2v6B8/W1WgUNYJvVvgqCIbviPR/BcI6hKvxVBdAMjz+84eU3qpfMau1Bn2J70JDSQSNkNgg==", + "resolved": "https://pkg.pr.new/@netlify/dev-utils@2b6145e", + "integrity": "sha512-4+mudx9u5qa68lceMVke8XAtjn0BO5Wh1M8UCV8kw8ZqnPd/8L+7sNPMH7aoApS0vIxFYYpbg+1rxe13fWL5Pg==", "license": "MIT", "dependencies": { + "@napi-rs/keyring": "^2.1.2", "@whatwg-node/server": "^0.11.0", "ansis": "^4.1.0", "atomically": "^2.0.3", diff --git a/package.json b/package.json index 009c8ab87cc..ce28e4e9a11 100644 --- a/package.json +++ b/package.json @@ -61,7 +61,7 @@ "@netlify/build-info": "^11.3.1", "@netlify/config": "^25.5.1", "@netlify/dev": "^5.1.12", - "@netlify/dev-utils": "^6.0.4", + "@netlify/dev-utils": "https://pkg.pr.new/@netlify/dev-utils@2b6145e", "@netlify/edge-bundler": "^16.2.0", "@netlify/edge-functions": "^4.0.3", "@netlify/edge-functions-bootstrap": "^3.2.0", @@ -212,6 +212,9 @@ "verdaccio": "^6.3.2", "vitest": "^5.0.0" }, + "overrides": { + "@netlify/dev-utils": "$@netlify/dev-utils" + }, "engines": { "node": ">=22.13.0" }, diff --git a/src/commands/base-command.ts b/src/commands/base-command.ts index a4c467d7fd4..7f4a5338246 100644 --- a/src/commands/base-command.ts +++ b/src/commands/base-command.ts @@ -16,6 +16,7 @@ import { findUp } from 'find-up' import { deepMerge, pick } from '../utils/object-utilities.js' import { getAgent } from '../lib/http-agent.js' +import { skipKeychainMigrationPrompt, writeAuthTokenForStorage } from '../lib/secure-storage.js' import { NETLIFY_CYAN, USER_AGENT, @@ -181,11 +182,14 @@ export type BaseOptionValues = { verbose?: boolean } -export function storeToken( +export async function storeToken( globalConfig: Awaited>, { userId, name, email, accessToken }: { userId: string; name?: string; email?: string; accessToken: string }, -) { - const userData = deepMerge(globalConfig.get(`users.${userId}`), { +): Promise { + globalConfig.set('userId', userId) + const mode = await writeAuthTokenForStorage(userId, accessToken) + + const userData = deepMerge(globalConfig.get(`users.${userId}`) as { auth?: { token?: string } } | undefined, { id: userId, name, email, @@ -197,7 +201,7 @@ export function storeToken( }, }, }) - globalConfig.set('userId', userId) + if (mode === 'keychain') delete userData.auth?.token globalConfig.set(`users.${userId}`, userData) } @@ -564,13 +568,12 @@ export default class BaseCommand extends Command { return logAndThrowError('Could not retrieve user ID from Netlify API') } - storeToken(this.netlify.globalConfig, { userId, name, email, accessToken }) - await identify({ name, email, userId, }) + await storeToken(this.netlify.globalConfig, { userId, name, email, accessToken }) await track('user_login', { email, }) @@ -667,7 +670,11 @@ export default class BaseCommand extends Command { // Retrieve Site id and build state from the state.json // ================================================== const state = new LocalState(this.workingDir) - const [token] = await getToken(flags.auth) + const isDiscardingStoredToken = + actionCommand.name() === 'logout' || (actionCommand.name() === 'login' && Boolean(flags.new)) + if (isDiscardingStoredToken) skipKeychainMigrationPrompt() + const [token, tokenLocation] = await getToken(flags.auth) + this.setAnalyticsPayload({ authTokenSource: tokenLocation }) const apiUrlOpts: { userAgent: string diff --git a/src/commands/login/index.ts b/src/commands/login/index.ts index e97caff7d26..d842fb4bfe5 100644 --- a/src/commands/login/index.ts +++ b/src/commands/login/index.ts @@ -8,7 +8,8 @@ export const createLoginCommand = (program: BaseCommand) => .command('login') .description( `Login to your Netlify account -Opens a web browser to acquire an OAuth token.`, +Opens a web browser to acquire an OAuth token, which is stored in your OS keychain when available. +To store it in a plaintext config file instead, set NETLIFY_USE_LEGACY_AUTH_STORAGE=1. This is not recommended.`, ) .option('--new', 'Login to new Netlify account') .option('--request ', 'Create a login ticket for agent/human-in-the-loop auth') diff --git a/src/commands/login/login-check.ts b/src/commands/login/login-check.ts index bf74cb2be8a..69f885d6db9 100644 --- a/src/commands/login/login-check.ts +++ b/src/commands/login/login-check.ts @@ -45,7 +45,7 @@ export const loginCheck = async ( return logAndThrowError('Could not retrieve user ID from Netlify API') } - storeToken(globalConfig, { + await storeToken(globalConfig, { userId: user.id, name: user.full_name, email: user.email, diff --git a/src/commands/login/login.ts b/src/commands/login/login.ts index ab3d4a6c34c..5791432a4e2 100644 --- a/src/commands/login/login.ts +++ b/src/commands/login/login.ts @@ -12,6 +12,8 @@ const msg = function (location: TokenLocation) { return 'via CLI --auth flag' case 'config': return 'via netlify config on your machine' + case 'keychain': + return 'via your OS keychain (secure storage)' default: return '' } diff --git a/src/commands/logout/logout.ts b/src/commands/logout/logout.ts index 7a6c63a998d..ce946e943b8 100644 --- a/src/commands/logout/logout.ts +++ b/src/commands/logout/logout.ts @@ -16,7 +16,7 @@ export const logout = async (_options: OptionValues, command: BaseCommand) => { await track('user_logout') - // unset userID without deleting key + // Keep stored tokens so `netlify switch` can return to this account without logging in again command.netlify.globalConfig.set('userId', null) if (location === 'env') { diff --git a/src/commands/status/status.ts b/src/commands/status/status.ts index c136171e207..34e252aafc9 100644 --- a/src/commands/status/status.ts +++ b/src/commands/status/status.ts @@ -1,4 +1,5 @@ import clean from 'clean-deep' +import { getLegacyConfigFilePath } from '@netlify/dev-utils' import type { OptionValues } from 'commander' import prettyjson from 'prettyjson' @@ -13,6 +14,7 @@ import { type APIError, } from '../../utils/command-helpers.js' import { isInteractive } from '../../utils/scripted-commands.js' +import type { TokenLocation } from '../../utils/types.js' import type BaseCommand from '../base-command.js' // TODO: centralize these in a shared error-code dictionary alongside the exit codes. @@ -21,10 +23,25 @@ export const STATUS_ERROR_CODES = { NOT_LINKED: 'NOT_LINKED', } as const +const formatTokenStorage = (location: TokenLocation): string => { + switch (location) { + case 'keychain': + return 'native secure storage' + case 'config': + return `plaintext config file (${getLegacyConfigFilePath()})` + case 'env': + return 'NETLIFY_AUTH_TOKEN environment variable' + case 'flag': + return '--auth flag' + default: + return 'not stored' + } +} + export const status = async (options: OptionValues, command: BaseCommand) => { const { accounts, api, globalConfig, site, siteInfo } = command.netlify const currentUserId = globalConfig.get('userId') as string | undefined - const [accessToken] = await getToken() + const [accessToken, tokenLocation] = await getToken(options.auth as string | undefined) if (!accessToken) { if (options.json) { @@ -98,7 +115,7 @@ export const status = async (options: OptionValues, command: BaseCommand) => { // another lib. (clean as unknown as >(obj: T) => Partial)(accountData) - log(prettyjson.render(cleanAccountData)) + log(prettyjson.render({ ...cleanAccountData, 'Auth token storage': formatTokenStorage(tokenLocation) })) if (!siteId) { if (options.json) { @@ -121,6 +138,10 @@ export const status = async (options: OptionValues, command: BaseCommand) => { if (options.json) { logJson({ account: cleanAccountData, + authTokenStorage: { + source: tokenLocation, + ...(tokenLocation === 'config' ? { configPath: getLegacyConfigFilePath() } : {}), + }, siteData: { 'site-name': siteInfo.name, 'config-path': site.configPath, diff --git a/src/lib/secure-storage.ts b/src/lib/secure-storage.ts new file mode 100644 index 00000000000..4c61a35dbca --- /dev/null +++ b/src/lib/secure-storage.ts @@ -0,0 +1,143 @@ +import { confirm } from '@inquirer/prompts' +import { + getGlobalConfigStore, + getLegacyConfigFilePath, + isLegacyAuthStorageForced, + LEGACY_STORAGE_ENV_VAR, + readStoredAPIToken, + storeTokenInKeychain, +} from '@netlify/dev-utils' + +import { chalk, isLogSuppressed, log, warn } from '../utils/command-helpers.js' +import { isInteractive } from '../utils/scripted-commands.js' +import { reportError, track } from '../utils/telemetry/index.js' + +export type TokenStorageMode = 'keychain' | 'legacy' + +let skipMigrationThisSession = false +let keychainReadFailureWarned = false + +const skipMigrationKey = (userId: string) => `users.${userId}.auth.skipKeychainMigration` + +const trackStored = (mode: TokenStorageMode, migrated: boolean, keychainFailed = false): void => { + void track('user_authTokenStored', { mode, migrated, ...(keychainFailed ? { keychainFailed: true } : {}) }) +} + +const warnKeychainReadFailed = (error: Error): void => { + if (keychainReadFailureWarned) return + keychainReadFailureWarned = true + warn( + `Could not read your Netlify auth token from the OS keychain: ${error.message} +If your keychain is locked or access was denied, unlock it or allow access and try again, or run ${chalk.cyanBright( + 'netlify login --new', + )}. In scripts, CI or agents, set ${chalk.cyanBright('NETLIFY_AUTH_TOKEN')} instead.`, + ) +} + +export const skipKeychainMigrationPrompt = (): void => { + skipMigrationThisSession = true +} + +const canUseKeychainInteractively = (env: NodeJS.ProcessEnv): boolean => + isInteractive() || env.TESTING_PROMPTS === 'true' + +const isPromptExit = (error: unknown): boolean => error instanceof Error && error.name === 'ExitPromptError' + +const attemptMigration = async (token: string, env: NodeJS.ProcessEnv): Promise => { + if ( + isLegacyAuthStorageForced(env) || + !canUseKeychainInteractively(env) || + skipMigrationThisSession || + // --json and --silent hide the prompt's context lines but not the question itself + isLogSuppressed() + ) + return false + const globalConfig = await getGlobalConfigStore() + const userId = globalConfig.get('userId') as string | undefined + if (userId == null || globalConfig.get(skipMigrationKey(userId)) === true) return false + + skipMigrationThisSession = true + + log() + log(`Your Netlify auth token is stored in plaintext at ${chalk.cyanBright(getLegacyConfigFilePath())}.`) + log(`The CLI can move it to your OS keychain, which is more secure. Your OS may ask you to allow access.`) + log() + let confirmed: boolean + try { + confirmed = await confirm({ message: 'Move the token to the keychain now?', default: true }) + } catch (error) { + if (isPromptExit(error)) throw error + void reportError(error, { severity: 'error' }) + return false + } + + if (!confirmed) { + globalConfig.set(skipMigrationKey(userId), true) + log( + chalk.dim( + `The CLI won't ask again. To move it later, run ${chalk.cyanBright('netlify login --new')} in a terminal.`, + ), + ) + void track('user_authTokenMigrationDeclined', {}) + return false + } + + const write = await storeTokenInKeychain(userId, token) + if (write.status === 'error') { + globalConfig.set(skipMigrationKey(userId), true) + warn( + `Could not save your token to the OS keychain: ${write.error.message} +It stays in plaintext and the CLI won't ask again. To retry, unlock your keychain and run ${chalk.cyanBright( + 'netlify login --new', + )}.`, + ) + trackStored('legacy', false, true) + return false + } + globalConfig.set(`users.${userId}.auth.token`, undefined) + log(chalk.green('Auth token moved to the OS keychain.')) + trackStored('keychain', true) + return true +} + +export const getStoredAPIToken = async ( + env: NodeJS.ProcessEnv = process.env, +): Promise<{ token: string | undefined; fromKeychain: boolean }> => { + const stored = await readStoredAPIToken(env) + switch (stored.source) { + case 'keychain': + return { token: stored.token, fromKeychain: true } + case 'config': + return { token: stored.token, fromKeychain: await attemptMigration(stored.token, env) } + case 'none': + if (stored.keychainError) warnKeychainReadFailed(stored.keychainError) + return { token: undefined, fromKeychain: false } + } +} + +// Non-interactive logins never touch the keychain, so they can't trigger an OS dialog nobody answers +export const writeAuthTokenForStorage = async ( + userId: string, + accessToken: string, + env: NodeJS.ProcessEnv = process.env, +): Promise => { + if (isLegacyAuthStorageForced(env) || !canUseKeychainInteractively(env)) { + trackStored('legacy', false) + return 'legacy' + } + + const write = await storeTokenInKeychain(userId, accessToken) + if (write.status === 'stored') { + trackStored('keychain', false) + return 'keychain' + } + + warn( + `Could not save your auth token to the OS keychain: ${write.error.message} +It was saved in plaintext at ${getLegacyConfigFilePath()} instead. To retry, unlock your keychain and run ${chalk.cyanBright( + 'netlify login --new', + )}. To always use plaintext storage and hide this warning, set ${chalk.cyanBright(`${LEGACY_STORAGE_ENV_VAR}=1`)}.`, + ) + trackStored('legacy', false, true) + return 'legacy' +} diff --git a/src/utils/command-helpers.ts b/src/utils/command-helpers.ts index 2b3faf08674..250cc2512eb 100644 --- a/src/utils/command-helpers.ts +++ b/src/utils/command-helpers.ts @@ -4,12 +4,12 @@ import process from 'process' import { format, inspect } from 'util' import type { NetlifyAPI } from '@netlify/api' -import { getAPIToken } from '@netlify/dev-utils' import { Chalk, type ChalkInstance as ChalkInstancePrimitiveType } from 'chalk' import type { Option } from 'commander' import WSL from 'is-wsl' import terminalLink from 'terminal-link' +import { getStoredAPIToken } from '../lib/secure-storage.js' import { startSpinner } from '../lib/spinner.js' import { getDrivingAgent } from './agent-detection.js' @@ -132,7 +132,6 @@ export const pollForToken = async ({ /** * Get a netlify token * @param {string} [tokenFromOptions] optional token from the provided --auth options - * @returns {Promise<[null|string, 'flag' | 'env' |'config' |'not found']>} */ export type TokenTuple = [string | null, TokenLocation] @@ -147,10 +146,9 @@ export const getToken = async (tokenFromOptions?: string): Promise = if (NETLIFY_AUTH_TOKEN && NETLIFY_AUTH_TOKEN !== 'null') { return [NETLIFY_AUTH_TOKEN, 'env'] } - // 3. If no env var use global user setting - const tokenFromConfig = await getAPIToken() - if (tokenFromConfig) { - return [tokenFromConfig, 'config'] + const { token, fromKeychain } = await getStoredAPIToken() + if (token) { + return [token, fromKeychain ? 'keychain' : 'config'] } return [null, 'not found'] } @@ -168,9 +166,10 @@ export const logJson = (message: unknown = '') => { } } +export const isLogSuppressed = (): boolean => argv.includes('--json') || argv.includes('--silent') || isDefaultJson() + export const log = (message = '', ...args: string[]) => { - // If --silent or --json flag passed disable logger - if (argv.includes('--json') || argv.includes('--silent') || isDefaultJson()) { + if (isLogSuppressed()) { return } message = typeof message === 'string' ? message : inspect(message) diff --git a/src/utils/types.ts b/src/utils/types.ts index 2995ce31ad0..4db7918caec 100644 --- a/src/utils/types.ts +++ b/src/utils/types.ts @@ -186,7 +186,7 @@ export interface SiteInfo { user_id: string } -export type TokenLocation = 'env' | 'flag' | 'config' | 'not found' +export type TokenLocation = 'env' | 'flag' | 'config' | 'keychain' | 'not found' export type EnvVar = { key: string diff --git a/tests/integration/auth-token-storage.test.ts b/tests/integration/auth-token-storage.test.ts new file mode 100644 index 00000000000..2f5a3281afb --- /dev/null +++ b/tests/integration/auth-token-storage.test.ts @@ -0,0 +1,377 @@ +import { execFile } from 'node:child_process' +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { promisify } from 'node:util' + +import execa from 'execa' +import { getLegacyConfigFilePath } from '@netlify/dev-utils' +import { afterEach, beforeEach, describe, expect, test } from 'vitest' + +import { callCli } from './utils/call-cli.js' +import { cliPath } from './utils/cli-path.js' +import { CONFIRM, NO, handleQuestions } from './utils/handle-questions.js' +import { withMockApi } from './utils/mock-api.js' +import type { MinimalAccount } from '../../src/utils/types.js' + +interface StoredConfig { + telemetryDisabled?: boolean + cliId?: string + userId?: string + users?: Record +} + +interface StatusJSON { + authTokenStorage: { source: string; configPath?: string } + account: Record + siteData: Record +} + +const readConfig = async (configDir: string): Promise => + JSON.parse(await readFile(path.join(configDir, 'config.json'), 'utf8')) as StoredConfig + +const siteInfo = { + account_slug: 'test-account', + id: 'site_id', + name: 'site-name', + admin_url: 'https://app.netlify.com/projects/test-site/overview', + url: 'https://test-site.netlify.app/', +} +const user = { full_name: 'Test User', email: 'test@netlify.com', id: 'user-id' } +const accounts: MinimalAccount[] = [ + { + id: 'user-id', + name: user.full_name, + slug: siteInfo.account_slug, + default: true, + team_logo_url: null, + on_pro_trial: false, + organization_id: null, + type_name: 'placeholder', + type_slug: 'placeholder', + members_count: 1, + }, +] +const routes = [ + { path: 'sites/site_id', response: siteInfo }, + { path: 'sites/site_id/service-instances', response: [] }, + { path: 'accounts', response: accounts }, + { path: 'user', response: user }, +] + +// macOS resolves the default keychain via $HOME, so each test gets a throwaway login keychain +// under its temp home. Other platforms can't be redirected; entries written there are deleted afterwards. +const usesIsolatedKeychain = process.platform === 'darwin' +const execFileAsync = promisify(execFile) + +const createIsolatedKeychain = async (home: string): Promise => { + await mkdir(path.join(home, 'Library', 'Keychains'), { recursive: true }) + // HOME is overridden for `security` too, or it would add the keychain to the real search list + await execFileAsync( + 'security', + ['create-keychain', '-p', '', path.join(home, 'Library', 'Keychains', 'login.keychain-db')], + { + env: { ...process.env, HOME: home }, + }, + ) +} + +const DEV_UTILS_URL = import.meta.resolve('@netlify/dev-utils') + +// Constant source: inputs arrive via argv so no value is ever spliced into code +const KEYCHAIN_SCRIPT = ` +const [moduleUrl, operation, account, value] = process.argv.slice(1) +const k = await import(moduleUrl) +if (operation === 'store') { + const result = await k.storeTokenInKeychain(account, value) + if (result.status === 'error') throw result.error +} else if (operation === 'read') { + process.stdout.write(JSON.stringify(await k.getTokenFromKeychain(account))) +} else { + await k.deleteTokenFromKeychain(account) +} +` + +// Keychain ops run in a child process so they hit the same keychain as the CLI under test. Items +// are created by `node` (not `security`) so the CLI can read them without an access dialog. +const runKeychain = async (home: string, args: string[]): Promise => { + const { stdout } = await execFileAsync( + process.execPath, + ['--input-type=module', '-e', KEYCHAIN_SCRIPT, DEV_UTILS_URL, ...args], + { env: usesIsolatedKeychain ? { ...process.env, HOME: home } : process.env }, + ) + return stdout +} + +const writeKeychainEntry = async (home: string, account: string, value: string): Promise => { + await runKeychain(home, ['store', account, value]) +} + +const readKeychainEntry = async (home: string, account: string): Promise => + JSON.parse(await runKeychain(home, ['read', account])) as string | null + +const deleteKeychainEntry = async (home: string, account: string): Promise => { + await runKeychain(home, ['delete', account]) +} + +const detectKeychainAvailability = async (): Promise => { + const home = await mkdtemp(path.join(tmpdir(), 'nf-cli-auth-token-storage-probe-')) + const account = `netlify-cli-test-probe-${String(Date.now())}` + try { + if (usesIsolatedKeychain) await createIsolatedKeychain(home) + await writeKeychainEntry(home, account, 'ok') + const value = await readKeychainEntry(home, account) + await deleteKeychainEntry(home, account) + return value === 'ok' + } catch { + return false + } finally { + await rm(home, { recursive: true, force: true }) + } +} + +// Mirrors env-paths('netlify', { suffix: '' }) for the platform the subprocess will run on +const getConfigDirForHome = (home: string): string => { + if (process.platform === 'darwin') return path.join(home, 'Library', 'Preferences', 'netlify') + if (process.platform === 'win32') return path.join(home, 'AppData', 'Roaming', 'netlify', 'Config') + return path.join(home, '.config', 'netlify') +} + +const writeLegacyTokenToConfig = async (configDir: string, userId: string, token: string) => { + await mkdir(configDir, { recursive: true }) + const config: StoredConfig = { + telemetryDisabled: true, + cliId: 'test-cli-id', + userId, + users: { [userId]: { id: userId, auth: { token } } }, + } + await writeFile(path.join(configDir, 'config.json'), JSON.stringify(config, null, '\t')) +} + +let tmpHome: string +let configDir: string +const accountsToCleanup: string[] = [] + +const isolatedEnv = (apiUrl: string, extra: Record = {}) => ({ + HOME: tmpHome, + USERPROFILE: tmpHome, + XDG_CONFIG_HOME: path.join(tmpHome, '.config'), + APPDATA: path.join(tmpHome, 'AppData', 'Roaming'), + PATH: process.env.PATH ?? '', + // Linux reaches Secret Service over the session bus; share it so the CLI sees the same keychain + DBUS_SESSION_BUS_ADDRESS: process.env.DBUS_SESSION_BUS_ADDRESS, + XDG_RUNTIME_DIR: process.env.XDG_RUNTIME_DIR, + NETLIFY_SITE_ID: 'site_id', + NETLIFY_API_URL: apiUrl, + ...extra, +}) + +beforeEach(async () => { + tmpHome = await mkdtemp(path.join(tmpdir(), 'nf-cli-auth-token-storage-')) + configDir = getConfigDirForHome(tmpHome) + if (usesIsolatedKeychain) await createIsolatedKeychain(tmpHome) +}) + +afterEach(async () => { + await Promise.all(accountsToCleanup.splice(0).map((account) => deleteKeychainEntry(tmpHome, account))) + await rm(tmpHome, { recursive: true, force: true }) +}) + +const keychainReady = await detectKeychainAvailability() + +describe('netlify status auth token storage', () => { + test('reports env source in JSON when NETLIFY_AUTH_TOKEN is set', async () => { + await withMockApi(routes, async ({ apiUrl }) => { + const json = (await callCli( + ['status', '--json'], + { env: isolatedEnv(apiUrl, { NETLIFY_AUTH_TOKEN: 'env-tok' }), extendEnv: false }, + true, + )) as StatusJSON + + expect(json.authTokenStorage).toEqual({ source: 'env' }) + expect(json.account.Email).toBe('test@netlify.com') + expect(json.account['Auth token storage']).toBeUndefined() + }) + }) + + test('reports env source in human output with a friendly label', async () => { + await withMockApi(routes, async ({ apiUrl }) => { + const stdout = (await callCli(['status'], { + env: isolatedEnv(apiUrl, { NETLIFY_AUTH_TOKEN: 'env-tok' }), + extendEnv: false, + })) as string + + expect(stdout).toContain('Auth token storage') + expect(stdout).toContain('NETLIFY_AUTH_TOKEN') + }) + }) + + test('reports config source with the config file path when reading from the legacy plaintext config', async () => { + await writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const json = (await callCli( + ['status', '--json'], + { env: isolatedEnv(apiUrl, { NETLIFY_USE_LEGACY_AUTH_STORAGE: '1' }), extendEnv: false }, + true, + )) as StatusJSON + + expect(json.authTokenStorage.source).toBe('config') + expect(json.authTokenStorage.configPath).toBe(path.join(configDir, 'config.json')) + }) + }) + + test('reports flag source when --auth is used', async () => { + await withMockApi(routes, async ({ apiUrl }) => { + const json = (await callCli( + ['status', '--json', '--auth=flag-tok'], + { env: isolatedEnv(apiUrl), extendEnv: false }, + true, + )) as StatusJSON + + expect(json.authTokenStorage).toEqual({ source: 'flag' }) + }) + }) +}) + +describe('NETLIFY_USE_LEGACY_AUTH_STORAGE env var', () => { + test('forces the CLI to read from the legacy config and skip the keychain entirely', async () => { + await writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const json = (await callCli( + ['status', '--json'], + { env: isolatedEnv(apiUrl, { NETLIFY_USE_LEGACY_AUTH_STORAGE: '1' }), extendEnv: false }, + true, + )) as StatusJSON + + expect(json.authTokenStorage.source).toBe('config') + const config = await readConfig(configDir) + expect(config.users?.[user.id]?.auth?.token).toBe('plain-tok') + }) + }) +}) + +test('never prompts when run non-interactively', async () => { + await writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const stdout = (await callCli(['status'], { env: isolatedEnv(apiUrl), extendEnv: false })) as string + + expect(stdout).not.toContain('Move the token to the keychain') + const config = await readConfig(configDir) + expect(config.users?.[user.id]?.auth?.token).toBe('plain-tok') + expect(config.users?.[user.id]?.auth?.skipKeychainMigration).not.toBe(true) + }) +}) + +describe('migration prompt interactive flow', () => { + test('declining the prompt persists the choice and leaves the legacy token in place', async () => { + await writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const childProcess = execa(cliPath, ['status'], { + env: isolatedEnv(apiUrl, { TESTING_PROMPTS: 'true' }), + extendEnv: false, + timeout: 60_000, + }) + + handleQuestions(childProcess, [{ question: 'Move the token to the keychain', answer: NO + CONFIRM }]) + + await childProcess + + const config = await readConfig(configDir) + expect(config.users?.[user.id]?.auth?.token).toBe('plain-tok') + expect(config.users?.[user.id]?.auth?.skipKeychainMigration).toBe(true) + }) + + await withMockApi(routes, async ({ apiUrl }) => { + const stdout = (await callCli(['status'], { + env: isolatedEnv(apiUrl, { TESTING_PROMPTS: 'true' }), + extendEnv: false, + })) as string + + expect(stdout).not.toContain('Move the token to the keychain') + }) + }) + + test('does not offer to migrate a token that `logout` is discarding', async () => { + await writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const { stdout } = await execa(cliPath, ['logout'], { + env: isolatedEnv(apiUrl, { TESTING_PROMPTS: 'true' }), + extendEnv: false, + timeout: 60_000, + input: '', + }) + + expect(stdout).not.toContain('Move the token to the keychain') + expect(stdout).toContain('Logging you out') + expect((await readConfig(configDir)).userId).toBeNull() + }) + }) + + test.runIf(keychainReady)( + 'confirming the prompt moves the token to the keychain and removes it from the plaintext config', + async () => { + accountsToCleanup.push(user.id) + await writeLegacyTokenToConfig(configDir, user.id, 'plain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const childProcess = execa(cliPath, ['status'], { + env: isolatedEnv(apiUrl, { TESTING_PROMPTS: 'true' }), + extendEnv: false, + timeout: 60_000, + }) + + handleQuestions(childProcess, [{ question: 'Move the token to the keychain', answer: CONFIRM }]) + + await childProcess + + expect(await readKeychainEntry(tmpHome, user.id)).toBe('plain-tok') + const config = await readConfig(configDir) + expect(config.users?.[user.id]?.auth?.token).toBeUndefined() + expect(config.users?.[user.id]?.auth?.skipKeychainMigration).not.toBe(true) + }) + }, + ) + + test.runIf(keychainReady)( + 'reports keychain source in status output after the token has been moved to the keychain', + async () => { + accountsToCleanup.push(user.id) + await mkdir(configDir, { recursive: true }) + const config: StoredConfig = { + telemetryDisabled: true, + cliId: 'test-cli-id', + userId: user.id, + users: { [user.id]: { id: user.id, auth: {} } }, + } + await writeFile(path.join(configDir, 'config.json'), JSON.stringify(config)) + + await writeKeychainEntry(tmpHome, user.id, 'keychain-tok') + + await withMockApi(routes, async ({ apiUrl }) => { + const json = (await callCli( + ['status', '--json'], + { env: isolatedEnv(apiUrl), extendEnv: false }, + true, + )) as StatusJSON + + expect(json.authTokenStorage).toEqual({ source: 'keychain' }) + + const stdout = (await callCli(['status'], { + env: isolatedEnv(apiUrl), + extendEnv: false, + })) as string + expect(stdout).toContain('native secure storage') + }) + }, + ) +}) + +test('integration tests never touch the real netlify config', async () => { + const raw = await readFile(getLegacyConfigFilePath(), 'utf8').catch(() => '') + expect(raw).not.toContain('test-cli-id') +}) diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index 93f39fa5e65..9ea50e24364 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -110,6 +110,7 @@ test('should send invoked command on success', async () => { expect(request!.body).toHaveProperty('event', 'cli:command') expect(request!.body).toHaveProperty('status', 'success') expect(request!.body).toHaveProperty('properties', { + authTokenSource: 'env', buildSystem: [], cliVersion: pkg.version, command: 'api', @@ -136,6 +137,7 @@ test('should send invoked command on failure', async () => { expect(request!.body).toHaveProperty('event', 'cli:command') expect(request!.body).toHaveProperty('status', 'error') expect(request!.body).toHaveProperty('properties', { + authTokenSource: 'env', buildSystem: [], cliVersion: pkg.version, command: 'dev:exec', diff --git a/tests/unit/commands/base-command-store-token.test.ts b/tests/unit/commands/base-command-store-token.test.ts new file mode 100644 index 00000000000..b13af37d529 --- /dev/null +++ b/tests/unit/commands/base-command-store-token.test.ts @@ -0,0 +1,65 @@ +import type { GlobalConfigStore } from '@netlify/dev-utils' +import { getProperty, setProperty } from 'dot-prop' +import { beforeEach, describe, expect, test, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ writeAuthTokenForStorage: vi.fn() })) + +vi.mock('../../../src/lib/secure-storage.js', async () => ({ + ...(await vi.importActual('../../../src/lib/secure-storage.js')), + writeAuthTokenForStorage: mocks.writeAuthTokenForStorage, +})) + +import { storeToken } from '../../../src/commands/base-command.js' + +const createGlobalConfig = (initial: Record) => { + const data: Record = structuredClone(initial) + return { + get: (key: string): unknown => getProperty(data, key), + set: (key: string, value: unknown) => { + setProperty(data, key, value) + }, + } as unknown as GlobalConfigStore +} + +const userWithPlaintextToken = { + userId: 'user-1', + users: { 'user-1': { id: 'user-1', auth: { token: 'old-plaintext', github: { user: 'gh-user', token: 'gh-tok' } } } }, +} + +describe('storeToken', () => { + beforeEach(() => { + mocks.writeAuthTokenForStorage.mockReset() + }) + + test('clears a pre-existing plaintext token when the new token goes to the keychain', async () => { + mocks.writeAuthTokenForStorage.mockResolvedValue('keychain') + const globalConfig = createGlobalConfig(userWithPlaintextToken) + + await storeToken(globalConfig, { userId: 'user-1', accessToken: 'new-token' }) + + expect(globalConfig.get('users.user-1.auth.token')).toBeUndefined() + expect(globalConfig.get('users.user-1.auth.github')).toEqual({ user: 'gh-user', token: 'gh-tok' }) + }) + + test('switches the current user before storing, so storage telemetry is attributed to them', async () => { + let userIdDuringWrite: unknown + mocks.writeAuthTokenForStorage.mockImplementation(() => { + userIdDuringWrite = globalConfig.get('userId') + return Promise.resolve('keychain') + }) + const globalConfig = createGlobalConfig({ ...userWithPlaintextToken, userId: 'previous-user' }) + + await storeToken(globalConfig, { userId: 'user-1', accessToken: 'new-token' }) + + expect(userIdDuringWrite).toBe('user-1') + }) + + test('writes the token to the config file when falling back to legacy storage', async () => { + mocks.writeAuthTokenForStorage.mockResolvedValue('legacy') + const globalConfig = createGlobalConfig(userWithPlaintextToken) + + await storeToken(globalConfig, { userId: 'user-1', accessToken: 'new-token' }) + + expect(globalConfig.get('users.user-1.auth.token')).toBe('new-token') + }) +}) diff --git a/tests/unit/commands/status/status.test.ts b/tests/unit/commands/status/status.test.ts index 9f9b9d0fe2b..41d2d9e32fb 100644 --- a/tests/unit/commands/status/status.test.ts +++ b/tests/unit/commands/status/status.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test, vi, beforeEach } from 'vitest' +import { getLegacyConfigFilePath } from '@netlify/dev-utils' const { mockGetToken, mockGetCurrentUser, logMessages, jsonMessages, exitCalls } = vi.hoisted(() => { const mockGetToken = vi.fn() @@ -141,6 +142,10 @@ describe('status', () => { Email: 'test@example.com', Teams: ['My Team'], }, + authTokenStorage: { + source: 'config', + configPath: getLegacyConfigFilePath(), + }, siteData: { 'site-name': 'my-site', 'config-path': '/project/netlify.toml', diff --git a/tests/unit/lib/secure-storage.test.ts b/tests/unit/lib/secure-storage.test.ts new file mode 100644 index 00000000000..14ff8098a7b --- /dev/null +++ b/tests/unit/lib/secure-storage.test.ts @@ -0,0 +1,207 @@ +import type { StoredAPIToken } from '@netlify/dev-utils' +import { beforeEach, describe, expect, test, vi } from 'vitest' + +interface MockState { + config: Record + stored: StoredAPIToken + interactive: boolean + logSuppressed: boolean +} + +const mocks = vi.hoisted(() => { + const state: MockState = { + config: {}, + stored: { source: 'none' }, + interactive: false, + logSuppressed: false, + } + return Object.assign(state, { + confirm: vi.fn(), + storeTokenInKeychain: vi.fn(), + warn: vi.fn(), + }) +}) + +vi.mock('@netlify/dev-utils', async () => ({ + ...(await vi.importActual('@netlify/dev-utils')), + getGlobalConfigStore: () => + Promise.resolve({ + get: (key: string) => mocks.config[key], + set: (key: string, value: unknown) => { + mocks.config[key] = value + }, + }), + readStoredAPIToken: () => Promise.resolve(mocks.stored), + storeTokenInKeychain: mocks.storeTokenInKeychain, +})) + +vi.mock('@inquirer/prompts', () => ({ confirm: mocks.confirm })) + +vi.mock('../../../src/utils/scripted-commands.js', () => ({ isInteractive: () => mocks.interactive })) + +vi.mock('../../../src/utils/command-helpers.js', async () => ({ + ...(await vi.importActual('../../../src/utils/command-helpers.js')), + isLogSuppressed: () => mocks.logSuppressed, + log: vi.fn(), + warn: mocks.warn, +})) + +vi.mock('../../../src/utils/telemetry/index.js', () => ({ track: vi.fn(), reportError: vi.fn() })) + +// Prompt-once and warn-once flags are module state, so each test gets a fresh module +const loadSecureStorage = async () => { + vi.resetModules() + return import('../../../src/lib/secure-storage.js') +} + +beforeEach(() => { + mocks.config = { userId: 'user-1', 'users.user-1.auth.token': 'plain-tok' } + mocks.stored = { source: 'none' } + mocks.interactive = false + mocks.logSuppressed = false + mocks.confirm.mockReset() + mocks.storeTokenInKeychain.mockReset().mockResolvedValue({ status: 'stored' }) + mocks.warn.mockReset() +}) + +describe('getStoredAPIToken', () => { + test('returns a keychain token', async () => { + mocks.stored = { source: 'keychain', token: 'kc-tok' } + const { getStoredAPIToken } = await loadSecureStorage() + + await expect(getStoredAPIToken({})).resolves.toEqual({ token: 'kc-tok', fromKeychain: true }) + }) + + test('warns once, with next steps, when the keychain cannot be read', async () => { + mocks.stored = { source: 'none', keychainError: new Error('User interaction is not allowed.') } + const { getStoredAPIToken } = await loadSecureStorage() + + await expect(getStoredAPIToken({})).resolves.toEqual({ token: undefined, fromKeychain: false }) + await getStoredAPIToken({}) + + expect(mocks.warn).toHaveBeenCalledOnce() + const [message] = mocks.warn.mock.calls[0] as [string] + expect(message).toContain('User interaction is not allowed.') + expect(message).toContain('netlify login --new') + expect(message).toContain('NETLIFY_AUTH_TOKEN') + }) + + test('does not warn when the user is simply not logged in', async () => { + const { getStoredAPIToken } = await loadSecureStorage() + + await expect(getStoredAPIToken({})).resolves.toEqual({ token: undefined, fromKeychain: false }) + expect(mocks.warn).not.toHaveBeenCalled() + }) +}) + +describe('plaintext token migration', () => { + beforeEach(() => { + mocks.stored = { source: 'config', token: 'plain-tok' } + }) + + test('never prompts in a non-interactive session', async () => { + const { getStoredAPIToken } = await loadSecureStorage() + + await expect(getStoredAPIToken({})).resolves.toEqual({ token: 'plain-tok', fromKeychain: false }) + expect(mocks.confirm).not.toHaveBeenCalled() + }) + + test('never prompts when legacy storage is forced', async () => { + mocks.interactive = true + const { getStoredAPIToken } = await loadSecureStorage() + + await getStoredAPIToken({ NETLIFY_USE_LEGACY_AUTH_STORAGE: '1' }) + expect(mocks.confirm).not.toHaveBeenCalled() + }) + + test('moves the token to the keychain and removes the plaintext copy when confirmed', async () => { + mocks.interactive = true + mocks.confirm.mockResolvedValue(true) + const { getStoredAPIToken } = await loadSecureStorage() + + await expect(getStoredAPIToken({})).resolves.toEqual({ token: 'plain-tok', fromKeychain: true }) + expect(mocks.storeTokenInKeychain).toHaveBeenCalledWith('user-1', 'plain-tok') + expect(mocks.config['users.user-1.auth.token']).toBeUndefined() + }) + + test('stops asking after a failed keychain write and explains how to retry', async () => { + mocks.interactive = true + mocks.confirm.mockResolvedValue(true) + mocks.storeTokenInKeychain.mockResolvedValue({ status: 'error', error: new Error('Keychain is locked') }) + const { getStoredAPIToken } = await loadSecureStorage() + + await expect(getStoredAPIToken({})).resolves.toEqual({ token: 'plain-tok', fromKeychain: false }) + expect(mocks.config['users.user-1.auth.token']).toBe('plain-tok') + expect(mocks.config['users.user-1.auth.skipKeychainMigration']).toBe(true) + const [message] = mocks.warn.mock.calls[0] as [string] + expect(message).toContain('Keychain is locked') + expect(message).toContain('netlify login --new') + }) + + test('stops asking once declined', async () => { + mocks.interactive = true + mocks.confirm.mockResolvedValue(false) + const { getStoredAPIToken } = await loadSecureStorage() + + await getStoredAPIToken({}) + expect(mocks.config['users.user-1.auth.skipKeychainMigration']).toBe(true) + expect(mocks.storeTokenInKeychain).not.toHaveBeenCalled() + }) + + test('never prompts when output is suppressed by --json or --silent', async () => { + mocks.interactive = true + mocks.logSuppressed = true + const { getStoredAPIToken } = await loadSecureStorage() + + await getStoredAPIToken({}) + expect(mocks.confirm).not.toHaveBeenCalled() + }) + + test('never prompts once skipped for this session', async () => { + mocks.interactive = true + const { getStoredAPIToken, skipKeychainMigrationPrompt } = await loadSecureStorage() + + skipKeychainMigrationPrompt() + await expect(getStoredAPIToken({})).resolves.toEqual({ token: 'plain-tok', fromKeychain: false }) + expect(mocks.confirm).not.toHaveBeenCalled() + }) + + test('lets Ctrl+C at the prompt abort the command', async () => { + mocks.interactive = true + mocks.confirm.mockRejectedValue( + Object.assign(new Error('User force closed the prompt'), { name: 'ExitPromptError' }), + ) + const { getStoredAPIToken } = await loadSecureStorage() + + await expect(getStoredAPIToken({})).rejects.toMatchObject({ name: 'ExitPromptError' }) + }) +}) + +describe('writeAuthTokenForStorage', () => { + test('uses plaintext without touching the keychain in a non-interactive session', async () => { + const { writeAuthTokenForStorage } = await loadSecureStorage() + + await expect(writeAuthTokenForStorage('user-1', 'tok', {})).resolves.toBe('legacy') + expect(mocks.storeTokenInKeychain).not.toHaveBeenCalled() + }) + + test('uses the keychain in an interactive session', async () => { + mocks.interactive = true + const { writeAuthTokenForStorage } = await loadSecureStorage() + + await expect(writeAuthTokenForStorage('user-1', 'tok', {})).resolves.toBe('keychain') + expect(mocks.warn).not.toHaveBeenCalled() + }) + + test('falls back to plaintext with an actionable warning when the keychain write fails', async () => { + mocks.interactive = true + mocks.storeTokenInKeychain.mockResolvedValue({ status: 'error', error: new Error('Keychain is locked') }) + const { writeAuthTokenForStorage } = await loadSecureStorage() + + await expect(writeAuthTokenForStorage('user-1', 'tok', {})).resolves.toBe('legacy') + const [message] = mocks.warn.mock.calls[0] as [string] + expect(message).toContain('Keychain is locked') + expect(message).toContain('netlify login --new') + expect(message).toContain('NETLIFY_USE_LEGACY_AUTH_STORAGE=1') + }) +})