diff --git a/.changeset/fix-dist-tag-web-otp.md b/.changeset/fix-dist-tag-web-otp.md new file mode 100644 index 0000000000..627054ef54 --- /dev/null +++ b/.changeset/fix-dist-tag-web-otp.md @@ -0,0 +1,8 @@ +--- +"@pnpm/registry-access.client": minor +"@pnpm/registry-access.commands": patch +"@pnpm/network.fetch": patch +"pnpm": patch +--- + +Fix `pnpm dist-tag add` and `pnpm dist-tag rm` against npmjs.org failing without `--otp` with `[ERR_PNPM_UNAUTHORIZED] You must be logged in to set dist-tag … "You must provide a one-time pass. Upgrade your client to npm@latest in order to use 2FA."`. pnpm now sends `npm-auth-type: web` on dist-tag writes and surfaces the resulting OTP challenge through the existing browser-based 2FA flow (the same `withOtpHandling` helper used by `pnpm publish`), so the browser opens, the user authenticates, and the dist-tag is set on retry. `--otp=` continues to work via the classic flow. diff --git a/network/fetch/src/fetchFromRegistry.ts b/network/fetch/src/fetchFromRegistry.ts index f7dec438f3..591af884bc 100644 --- a/network/fetch/src/fetchFromRegistry.ts +++ b/network/fetch/src/fetchFromRegistry.ts @@ -68,6 +68,7 @@ export function createFetchFromRegistry (defaultOpts: CreateFetchFromRegistryOpt ...getHeaders({ auth: opts?.authHeaderValue, fullMetadata: opts?.fullMetadata, + method: opts?.method, userAgent: defaultOpts.userAgent, }), } @@ -125,7 +126,7 @@ export function createFetchFromRegistry (defaultOpts: CreateFetchFromRegistryOpt } interface Headers { - accept: string + accept?: string authorization?: string 'user-agent'?: string } @@ -134,11 +135,16 @@ function getHeaders ( opts: { auth?: string fullMetadata?: boolean + method?: string userAgent?: string } ): Headers { - const headers: { accept: string, authorization?: string, 'user-agent'?: string } = { - accept: opts.fullMetadata === true ? ACCEPT_FULL_DOC : ACCEPT_ABBREVIATED_DOC, + const headers: Headers = {} + // The abbreviated/full-metadata Accept header is meaningful only on package + // metadata reads. Setting it on writes (PUT/POST/DELETE) breaks npmjs.org's + // dist-tag endpoint, which rejects the request with a generic 400. + if (!opts.method || opts.method === 'GET' || opts.method === 'HEAD') { + headers.accept = opts.fullMetadata === true ? ACCEPT_FULL_DOC : ACCEPT_ABBREVIATED_DOC } if (opts.auth) { headers['authorization'] = opts.auth diff --git a/network/fetch/test/fetchFromRegistry.test.ts b/network/fetch/test/fetchFromRegistry.test.ts index 474eec53c8..e5ff34f6ce 100644 --- a/network/fetch/test/fetchFromRegistry.test.ts +++ b/network/fetch/test/fetchFromRegistry.test.ts @@ -311,6 +311,52 @@ test('createDispatchedFetch returns a fetch bound to the given dispatcher option } }) +test('abbreviated metadata Accept header is not sent on write requests', async () => { + const receivedHeaders = await new Promise((resolve, reject) => { + const server = http.createServer((req, res) => { + resolve(req.headers) + res.writeHead(200, { 'content-type': 'application/json' }) + res.end('{"ok":true}') + }) + server.listen(0, () => { + const { port } = server.address() as { port: number } + const fetchFromRegistry = createFetchFromRegistry({}) + fetchFromRegistry(`http://127.0.0.1:${port}/-/package/pnpm/dist-tags/latest-10`, { + method: 'PUT', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify('10.34.0'), + }).then( + (res) => res.text().then(() => server.close()), + (err) => { + server.close(); reject(err) + } + ) + }) + }) + expect(receivedHeaders.accept).not.toContain('application/vnd.npm.install-v1+json') +}) + +test('abbreviated metadata Accept header is sent on GET requests', async () => { + const receivedHeaders = await new Promise((resolve, reject) => { + const server = http.createServer((req, res) => { + resolve(req.headers) + res.writeHead(200, { 'content-type': 'application/json' }) + res.end('{"ok":true}') + }) + server.listen(0, () => { + const { port } = server.address() as { port: number } + const fetchFromRegistry = createFetchFromRegistry({}) + fetchFromRegistry(`http://127.0.0.1:${port}/test`).then( + (res) => res.text().then(() => server.close()), + (err) => { + server.close(); reject(err) + } + ) + }) + }) + expect(receivedHeaders.accept).toBe('application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*') +}) + test('sec-fetch-* headers are stripped from requests', async () => { const receivedHeaders = await new Promise((resolve, reject) => { const server = http.createServer((req, res) => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ac75341ce0..cf2e534b4d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -8171,6 +8171,9 @@ importers: '@pnpm/network.fetch': specifier: workspace:* version: link:../../network/fetch + '@pnpm/network.web-auth': + specifier: workspace:* + version: link:../../network/web-auth '@pnpm/npm-package-arg': specifier: 'catalog:' version: 2.0.0 @@ -8199,6 +8202,9 @@ importers: '@pnpm/network.fetch': specifier: workspace:* version: link:../../network/fetch + '@pnpm/network.web-auth': + specifier: workspace:* + version: link:../../network/web-auth '@pnpm/npm-package-arg': specifier: 'catalog:' version: 2.0.0 @@ -8214,6 +8220,9 @@ importers: chalk: specifier: 'catalog:' version: 5.6.2 + enquirer: + specifier: 'catalog:' + version: 2.4.1 ramda: specifier: 'catalog:' version: '@pnpm/ramda@0.28.1' @@ -8227,6 +8236,9 @@ importers: '@jest/globals': specifier: 'catalog:' version: 30.3.0 + '@pnpm/logger': + specifier: workspace:* + version: link:../../core/logger '@pnpm/prepare': specifier: workspace:* version: link:../../__utils__/prepare diff --git a/registry-access/client/package.json b/registry-access/client/package.json index bfae679625..4b184f6462 100644 --- a/registry-access/client/package.json +++ b/registry-access/client/package.json @@ -33,6 +33,7 @@ "dependencies": { "@pnpm/error": "workspace:*", "@pnpm/network.fetch": "workspace:*", + "@pnpm/network.web-auth": "workspace:*", "@pnpm/npm-package-arg": "catalog:" }, "devDependencies": { diff --git a/registry-access/client/src/setDistTag.ts b/registry-access/client/src/setDistTag.ts index 86aac9d0b3..b145fe5d65 100644 --- a/registry-access/client/src/setDistTag.ts +++ b/registry-access/client/src/setDistTag.ts @@ -1,7 +1,10 @@ import { PnpmError } from '@pnpm/error' import type { FetchFromRegistry } from '@pnpm/network.fetch' +import { SyntheticOtpError } from '@pnpm/network.web-auth' import npa from '@pnpm/npm-package-arg' +export type AuthType = 'web' | 'legacy' + export interface SetDistTagOptions { packageName: string version: string @@ -9,6 +12,12 @@ export interface SetDistTagOptions { registryUrl: string fetchFromRegistry: FetchFromRegistry authHeader?: string + /** Mirrors npm CLI's `auth-type` flat option: `'web'` (default) opts into the + * web-OTP challenge, `'legacy'` is set automatically when the user passes + * `--otp` so a classic 6-digit code can be sent. */ + authType?: AuthType + /** OTP token to send as `npm-otp`. May be a classic 6-digit code (legacy) or + * the 64-character token returned by the web flow. */ otp?: string } @@ -20,18 +29,41 @@ export async function setDistTag (opts: SetDistTagOptions): Promise { method: 'PUT', headers: { 'content-type': 'application/json', + 'npm-auth-type': opts.authType ?? 'web', ...(opts.otp ? { 'npm-otp': opts.otp } : {}), }, body: JSON.stringify(opts.version), }) if (response.ok) return const body = await response.text() - const action = `set dist-tag "${opts.distTag}" on` if (response.status === 401) { - throw new PnpmError('UNAUTHORIZED', `You must be logged in to ${action} packages. ${body}`) + throw parseAuthError(body, opts.distTag) } + const action = `set dist-tag "${opts.distTag}" on` if (response.status === 403) { throw new PnpmError('FORBIDDEN', `You do not have permission to ${action} this package. ${body}`) } throw new PnpmError('REGISTRY_ERROR', `Failed to ${action} package: ${response.status} ${response.statusText}. ${body}`) } + +function parseAuthError (body: string, distTag: string): Error { + const parsed = tryParseJson(body) + if (parsed != null && typeof parsed === 'object' && 'authUrl' in parsed && 'doneUrl' in parsed) { + return new SyntheticOtpError({ + authUrl: typeof parsed.authUrl === 'string' ? parsed.authUrl : undefined, + doneUrl: typeof parsed.doneUrl === 'string' ? parsed.doneUrl : undefined, + }) + } + if (/one-time pass/i.test(body)) { + return new SyntheticOtpError(undefined) + } + return new PnpmError('UNAUTHORIZED', `You must be logged in to set dist-tag "${distTag}" on packages. ${body}`) +} + +function tryParseJson (body: string): unknown { + try { + return JSON.parse(body) + } catch { + return undefined + } +} diff --git a/registry-access/client/tsconfig.json b/registry-access/client/tsconfig.json index 78c3dcbd2d..3942d639d3 100644 --- a/registry-access/client/tsconfig.json +++ b/registry-access/client/tsconfig.json @@ -14,6 +14,9 @@ }, { "path": "../../network/fetch" + }, + { + "path": "../../network/web-auth" } ] } diff --git a/registry-access/commands/package.json b/registry-access/commands/package.json index 386f890058..2926aee586 100644 --- a/registry-access/commands/package.json +++ b/registry-access/commands/package.json @@ -38,17 +38,23 @@ "@pnpm/error": "workspace:*", "@pnpm/network.auth-header": "workspace:*", "@pnpm/network.fetch": "workspace:*", + "@pnpm/network.web-auth": "workspace:*", "@pnpm/npm-package-arg": "catalog:", "@pnpm/registry-access.client": "workspace:*", "@pnpm/resolving.registry.types": "workspace:*", "@pnpm/types": "workspace:*", "chalk": "catalog:", + "enquirer": "catalog:", "ramda": "catalog:", "render-help": "catalog:", "semver": "catalog:" }, + "peerDependencies": { + "@pnpm/logger": "catalog:" + }, "devDependencies": { "@jest/globals": "catalog:", + "@pnpm/logger": "workspace:*", "@pnpm/prepare": "workspace:*", "@pnpm/registry-access.commands": "workspace:*", "@pnpm/registry-mock": "catalog:", diff --git a/registry-access/commands/src/distTag.ts b/registry-access/commands/src/distTag.ts index c5a02fb147..a8bbb05f2b 100644 --- a/registry-access/commands/src/distTag.ts +++ b/registry-access/commands/src/distTag.ts @@ -1,11 +1,21 @@ +import readline from 'node:readline' + import { docsUrl } from '@pnpm/cli.utils' import { pickRegistryForPackage } from '@pnpm/config.pick-registry-for-package' import { PnpmError } from '@pnpm/error' +import { globalInfo, globalWarn } from '@pnpm/logger' import { createGetAuthHeaderByURI } from '@pnpm/network.auth-header' import { createFetchFromRegistry, type CreateFetchFromRegistryOptions, type FetchFromRegistry } from '@pnpm/network.fetch' +import { + type OtpContext, + SyntheticOtpError, + type WebAuthFetchOptions, + withOtpHandling, +} from '@pnpm/network.web-auth' import npa from '@pnpm/npm-package-arg' import { setDistTag } from '@pnpm/registry-access.client' import type { Registries, RegistryConfig } from '@pnpm/types' +import enquirer from 'enquirer' import { renderHelp } from 'render-help' import semver from 'semver' @@ -140,16 +150,22 @@ async function distTagAdd ( const registryUrl = pickRegistryForPackage(opts.registries ?? { default: 'https://registry.npmjs.org/' }, packageName) const authHeader = getAuthHeaderForRegistry(opts.configByUri, registryUrl) const fetchFromRegistry = createFetchFromRegistry(opts) - const otp = opts.cliOptions?.otp + const cliOtp = opts.cliOptions?.otp + const authType = cliOtp ? 'legacy' : 'web' - await setDistTag({ - packageName, - version, - distTag: tag, - registryUrl, - authHeader, - fetchFromRegistry, - otp, + await withOtpHandling({ + context: createOtpContext(opts), + fetchOptions: WEB_AUTH_FETCH_OPTIONS, + operation: (otp) => setDistTag({ + packageName, + version, + distTag: tag, + registryUrl, + authHeader, + authType, + fetchFromRegistry, + otp: otp ?? cliOtp, + }), }) return `+${tag}: ${packageName}@${version}` @@ -173,7 +189,7 @@ async function distTagRm ( const registryUrl = pickRegistryForPackage(opts.registries ?? { default: 'https://registry.npmjs.org/' }, packageName) const authHeader = getAuthHeaderForRegistry(opts.configByUri, registryUrl) const fetchFromRegistry = createFetchFromRegistry(opts) - const otp = opts.cliOptions?.otp + const cliOtp = opts.cliOptions?.otp // First check the tag exists const distTags = await fetchDistTags(packageName, registryUrl, fetchFromRegistry, authHeader) @@ -182,19 +198,79 @@ async function distTagRm ( } const distTagUrl = getDistTagUrl(packageName, registryUrl, tag) + const authType: 'web' | 'legacy' = cliOtp ? 'legacy' : 'web' + + await withOtpHandling({ + context: createOtpContext(opts), + fetchOptions: WEB_AUTH_FETCH_OPTIONS, + operation: (otp) => deleteDistTag({ + authHeader, + authType, + distTagUrl, + fetchFromRegistry, + otp: otp ?? cliOtp, + tag, + }), + }) + + return `-${tag}: ${packageName}@${distTags[tag]}` +} + +interface DeleteDistTagParams { + authHeader: string | undefined + authType: 'web' | 'legacy' + distTagUrl: string + fetchFromRegistry: FetchFromRegistry + otp: string | undefined + tag: string +} + +async function deleteDistTag ({ + authHeader, + authType, + distTagUrl, + fetchFromRegistry, + otp, + tag, +}: DeleteDistTagParams): Promise { const response = await fetchFromRegistry(distTagUrl, { authHeaderValue: authHeader, method: 'DELETE', headers: { + 'npm-auth-type': authType, ...(otp ? { 'npm-otp': otp } : {}), }, }) - if (!response.ok) { - await throwRegistryError(response, `remove dist-tag "${tag}" from`) + if (response.ok) return + const body = await response.text() + const action = `remove dist-tag "${tag}" from` + if (response.status === 401) { + throw parseAuthError(body, action) } + if (response.status === 403) { + throw new PnpmError('FORBIDDEN', `You do not have permission to ${action} this package. ${body}`) + } + throw new PnpmError('REGISTRY_ERROR', `Failed to ${action} package: ${response.status} ${response.statusText}. ${body}`) +} - return `-${tag}: ${packageName}@${distTags[tag]}` +function parseAuthError (body: string, action: string): Error { + let parsed: unknown + try { + parsed = JSON.parse(body) + } catch { + parsed = undefined + } + if (parsed != null && typeof parsed === 'object' && 'authUrl' in parsed && 'doneUrl' in parsed) { + return new SyntheticOtpError({ + authUrl: typeof parsed.authUrl === 'string' ? parsed.authUrl : undefined, + doneUrl: typeof parsed.doneUrl === 'string' ? parsed.doneUrl : undefined, + }) + } + if (/one-time pass/i.test(body)) { + return new SyntheticOtpError(undefined) + } + return new PnpmError('UNAUTHORIZED', `You must be logged in to ${action} packages. ${body}`) } function getAuthHeaderForRegistry ( @@ -232,13 +308,22 @@ async function fetchDistTags ( return await response.json() as Record } -async function throwRegistryError (response: Response, action: string): Promise { - const errorBody = await response.text() - if (response.status === 401) { - throw new PnpmError('UNAUTHORIZED', `You must be logged in to ${action} packages. ${errorBody}`) - } - if (response.status === 403) { - throw new PnpmError('FORBIDDEN', `You do not have permission to ${action} this package. ${errorBody}`) - } - throw new PnpmError('REGISTRY_ERROR', `Failed to ${action} package: ${response.status} ${response.statusText}. ${errorBody}`) +const WEB_AUTH_FETCH_OPTIONS: WebAuthFetchOptions = { + method: 'GET', +} + +// `withOtpHandling` polls `doneUrl` through `context.fetch`, so it must inherit +// the command's proxy/TLS/`configByUri` config — otherwise the write succeeds +// but the web-auth retry fails in custom-network environments. +function createOtpContext (opts: CreateFetchFromRegistryOptions): OtpContext { + return { + Date, + createReadlineInterface: readline.createInterface.bind(null, { input: process.stdin }), + enquirer, + fetch: createFetchFromRegistry(opts), + globalInfo, + globalWarn, + process, + setTimeout, + } } diff --git a/registry-access/commands/test/setDistTag.ts b/registry-access/commands/test/setDistTag.ts new file mode 100644 index 0000000000..8c803ad934 --- /dev/null +++ b/registry-access/commands/test/setDistTag.ts @@ -0,0 +1,150 @@ +import { afterEach, beforeEach, describe, expect, it } from '@jest/globals' +import { createFetchFromRegistry } from '@pnpm/network.fetch' +import { SyntheticOtpError } from '@pnpm/network.web-auth' +import { setDistTag } from '@pnpm/registry-access.client' +import { getMockAgent, setupMockAgent, teardownMockAgent } from '@pnpm/testing.mock-agent' + +const REGISTRY_URL = 'https://registry.npmjs.org/' +const PUT_PATH = /^\/-\/package\/pnpm\/dist-tags\/latest-10$/ + +describe('setDistTag', () => { + beforeEach(async () => { + await setupMockAgent() + }) + + afterEach(async () => { + await teardownMockAgent() + }) + + it('sends npm-auth-type: web when authType=web and no OTP yet', async () => { + let capturedHeaders: Record = {} + getMockAgent().get('https://registry.npmjs.org').intercept({ + method: 'PUT', + path: PUT_PATH, + }).reply(({ headers }) => { + capturedHeaders = headers as typeof capturedHeaders + return { statusCode: 200, data: {} } + }) + + await setDistTag({ + packageName: 'pnpm', + version: '10.34.0', + distTag: 'latest-10', + registryUrl: REGISTRY_URL, + fetchFromRegistry: createFetchFromRegistry({}), + authType: 'web', + }) + + expect(capturedHeaders['npm-auth-type']).toBe('web') + expect(capturedHeaders['npm-otp']).toBeUndefined() + }) + + it('keeps npm-auth-type: web alongside npm-otp on the web-flow retry', async () => { + let capturedHeaders: Record = {} + getMockAgent().get('https://registry.npmjs.org').intercept({ + method: 'PUT', + path: PUT_PATH, + }).reply(({ headers }) => { + capturedHeaders = headers as typeof capturedHeaders + return { statusCode: 200, data: {} } + }) + + await setDistTag({ + packageName: 'pnpm', + version: '10.34.0', + distTag: 'latest-10', + registryUrl: REGISTRY_URL, + fetchFromRegistry: createFetchFromRegistry({}), + authType: 'web', + otp: 'a'.repeat(64), + }) + + expect(capturedHeaders['npm-auth-type']).toBe('web') + expect(capturedHeaders['npm-otp']).toBe('a'.repeat(64)) + }) + + it('sends authType=legacy and npm-otp when the user passed --otp', async () => { + let capturedHeaders: Record = {} + getMockAgent().get('https://registry.npmjs.org').intercept({ + method: 'PUT', + path: PUT_PATH, + }).reply(({ headers }) => { + capturedHeaders = headers as typeof capturedHeaders + return { statusCode: 200, data: {} } + }) + + await setDistTag({ + packageName: 'pnpm', + version: '10.34.0', + distTag: 'latest-10', + registryUrl: REGISTRY_URL, + fetchFromRegistry: createFetchFromRegistry({}), + authType: 'legacy', + otp: '123456', + }) + + expect(capturedHeaders['npm-auth-type']).toBe('legacy') + expect(capturedHeaders['npm-otp']).toBe('123456') + }) + + it('throws SyntheticOtpError carrying authUrl/doneUrl when the 401 body has them', async () => { + getMockAgent().get('https://registry.npmjs.org').intercept({ + method: 'PUT', + path: PUT_PATH, + }).reply(401, { + authUrl: 'https://www.npmjs.com/login?next=/-/v1/done?sessionId=abc', + doneUrl: 'https://registry.npmjs.org/-/v1/done?sessionId=abc', + }) + + let caught: unknown + try { + await setDistTag({ + packageName: 'pnpm', + version: '10.34.0', + distTag: 'latest-10', + registryUrl: REGISTRY_URL, + fetchFromRegistry: createFetchFromRegistry({}), + }) + } catch (err) { + caught = err + } + expect(caught).toBeInstanceOf(SyntheticOtpError) + expect((caught as SyntheticOtpError).body).toEqual({ + authUrl: 'https://www.npmjs.com/login?next=/-/v1/done?sessionId=abc', + doneUrl: 'https://registry.npmjs.org/-/v1/done?sessionId=abc', + }) + }) + + it('throws SyntheticOtpError without body when the 401 mentions "one-time pass"', async () => { + getMockAgent().get('https://registry.npmjs.org').intercept({ + method: 'PUT', + path: PUT_PATH, + }).reply( + 401, + '"You must provide a one-time pass. Upgrade your client to npm@latest in order to use 2FA."' + ) + + await expect(setDistTag({ + packageName: 'pnpm', + version: '10.34.0', + distTag: 'latest-10', + registryUrl: REGISTRY_URL, + fetchFromRegistry: createFetchFromRegistry({}), + })).rejects.toBeInstanceOf(SyntheticOtpError) + }) + + it('throws UNAUTHORIZED PnpmError on a 401 that is not an OTP challenge', async () => { + getMockAgent().get('https://registry.npmjs.org').intercept({ + method: 'PUT', + path: PUT_PATH, + }).reply(401, 'Bad token') + + await expect(setDistTag({ + packageName: 'pnpm', + version: '10.34.0', + distTag: 'latest-10', + registryUrl: REGISTRY_URL, + fetchFromRegistry: createFetchFromRegistry({}), + })).rejects.toMatchObject({ code: 'ERR_PNPM_UNAUTHORIZED' }) + }) +}) diff --git a/registry-access/commands/tsconfig.json b/registry-access/commands/tsconfig.json index 89f18db4d8..a2d3837007 100644 --- a/registry-access/commands/tsconfig.json +++ b/registry-access/commands/tsconfig.json @@ -24,6 +24,9 @@ { "path": "../../core/error" }, + { + "path": "../../core/logger" + }, { "path": "../../core/types" }, @@ -33,6 +36,9 @@ { "path": "../../network/fetch" }, + { + "path": "../../network/web-auth" + }, { "path": "../../releasing/commands" },