Files
pnpm/network/web-auth/test/withOtpHandling.test.ts
Khải de3dc74439 feat(auth): keyboard event to open browser (#11148)
* feat(auth): add "Press ENTER to open in browser" during web authentication

During web-based authentication (login, publish), users can now press
ENTER to open the authentication URL in their default browser. The
background token polling continues uninterrupted, so users who prefer
to authenticate on their phone can still do so without pressing anything.

The implementation uses Node's readline module (not raw mode), so Ctrl+C
and Ctrl+Z continue to work normally. It is fully error-tolerant: if the
keyboard listener or browser opening fails, a warning is printed and the
polling continues.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix(auth): inject readline and execFile directly, not wrapper functions

Address review feedback:
- Remove defaultListenForEnter and defaultOpenBrowser wrapper functions
- Inject readline module and execFile function directly via context
- DEFAULT_CONTEXT now references modules directly (no closures)
- Use switch for platform detection, default = no browser prompt
- Rename pollWithBrowserOpen → offerToOpenBrowser (clearer name)
- Add platform-specific tests (darwin, win32, linux, freebsd)
- Use PassThrough streams for stdin mocks in tests

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix(auth): fix CI type errors in test mocks

- Type jest.fn() mocks for readline.createInterface properly
- Use PassThrough streams for stdin mocks in releasing/commands tests

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* refactor(auth): use generic Stdin parameter to eliminate PassThrough in tests

Per review feedback, add a generic Stdin type parameter to context
interfaces. This ties process.stdin and readline.createInterface together
through the same type, so tests can use simple { isTTY: true } mocks
instead of requiring PassThrough streams.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix(auth): propagate Stdin generic to releasing/commands OtpContext

The OtpContext in releasing/commands extends BaseOtpContext from
web-auth. Now that BaseOtpContext is generic, the local OtpContext
and publishWithOtpHandling must also be generic so tests can use
simple stdin mocks without PassThrough.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix: sort imports in releasing/commands otp.ts

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* refactor(auth): use .bind() for readline injection instead of generics

Per review feedback, revert the generic Stdin approach and instead use
readline.createInterface.bind(null, { input: process.stdin }) as the
injectable dependency. This avoids generics proliferation while keeping
the context clean — no arrow functions or closures in DEFAULT_CONTEXT.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* feat(publish): add "Press ENTER to open in browser" during publish OTP

Wire up createReadlineInterface and execFile in the publish
SHARED_CONTEXT so that pnpm publish also offers to open the browser
during web-based OTP authentication.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix(auth): improve browser-open prompt message

Change "Press ENTER to open in browser..." to
"Press ENTER to open the URL in your browser."

The old message implied the user should press Enter. The new wording
presents it as an available action, not an instruction — users can
also scan the QR code or copy-paste the URL.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* style: remove unnecessary arrow wrapper around createMockReadlineInterface

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* docs: explain why Enter keypress is fire-and-forget, not awaited

Add a comment explaining that only pollPromise is awaited — the Enter
listener is intentionally not part of a Promise.all. This prevents a
future refactor from reintroducing the npm bug where authentication
blocks until Enter is pressed, even when the user authenticates on
another device.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* docs: add permalink to npm's Promise.all bug in comment

Link to the specific npm-profile commit (d1a48be4259) so the comment
remains accurate even if npm fixes the bug in the future.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix: correct line numbers in npm-profile permalink (L85-L98)

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* style: apply review suggestion for npm-profile permalink format

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* style: remove duplicate line in npm-profile comment

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix: shadow global process instead of renaming to proc

Destructure as `process` (not `proc`) so the global `process` is
shadowed, preventing accidental direct access to it.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix: merge process fields in test mock contexts

Restructure createMockContext to merge process fields instead of
replacing the entire object. Tests that only need to override
platform or stdin no longer need to redundantly provide the other.

Also adds a test for undefined platform (default: case).

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix: use Omit+Partial for process overrides in test mock contexts

The process field spread `...overrides?.process` merges at runtime but
TypeScript still requires all fields in the override type. Fix by typing
the process override as Partial via Omit<..., 'process'> & { process?: Partial<...> }.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* refactor: extract a type alias

* refactor: extract MockContextOverrides type alias in remaining tests

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* refactor(auth): extract process types, use NodeJS.Platform, clean up tests

- Extract OfferToOpenBrowserProcess interface from inline process type
- Extract LoginProcess interface from inline process type in LoginContext
- Use NodeJS.Platform instead of string for platform fields (prevents typos)
- Rename simulateEnter → simulateEnterKeypress (clarify it's the key)
- Convert single-return functions to arrow expressions in tests
- Update test descriptions to say "Enter key" / "Enter keypress"

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* refactor(auth): rename offerToOpenBrowser → promptBrowserOpen

Per review feedback, "offer to open browser" was mouthful. Renamed
function, file, and all associated types (OfferToOpenBrowser* →
PromptBrowserOpen*).

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* docs: drop "IMPORTANT"

* refactor(auth): extract OtpProcess interface from inline process type

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix(auth): validate authUrl before passing to execFile

On Windows, cmd.exe re-parses execFile arguments with full shell
grammar, so metacharacters (&, |, ^, etc.) in the URL would be
interpreted as operators. Validate that authUrl is a well-formed
http(s) URL before passing it to the platform browser command.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* test(auth): add regression test for URLs with query parameters on win32

Verifies that URLs containing & and other query string characters are
passed through to execFile as-is on the win32 platform.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix(auth): escape cmd.exe metacharacters in Windows browser open URL

On Windows, cmd.exe re-parses execFile arguments and treats & | < > ^ %
as operators. Escape these with ^ so query strings in auth URLs
(e.g. ?token=abc&redirect=...) are not split by cmd.exe.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix(auth): use canonicalized URL and expand cmd.exe escape set

- Use parsedUrl.href (canonicalized by new URL()) instead of the raw
  authUrl string, ensuring percent-encoding of spaces and special chars.
- Expand cmd.exe metacharacter escaping to include () and ! in addition
  to & | < > ^ %, covering grouping operators and delayed expansion.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* docs(auth): document Windows browser-opening edge cases

Explain why cmd /c start is used instead of ShellExecuteW (not
callable from Node.js without a native addon), why alternatives
like explorer.exe, rundll32, and PowerShell are unreliable, and
note that a Rust/N-API addon could replace this in the future.

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

* fix: fix cspell errors in Windows browser-open comment

Reword to avoid unknown words "rundll" and "metacharacter".

https://claude.ai/code/session_01UtDnjrNQ2Cc3GLAPR8BrrW

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-04-03 01:36:00 +02:00

389 lines
12 KiB
TypeScript

import { jest } from '@jest/globals'
import {
type OtpContext,
OtpNonInteractiveError,
OtpSecondChallengeError,
SyntheticOtpError,
type WebAuthFetchOptions,
type WebAuthFetchResponse,
WebAuthTimeoutError,
withOtpHandling,
} from '@pnpm/network.web-auth'
function createMockResponse (init: {
ok: boolean
status: number
json?: unknown
headers?: WebAuthFetchResponse['headers']
}): WebAuthFetchResponse {
let bodyConsumed = false
return {
ok: init.ok,
status: init.status,
json: async () => {
if (bodyConsumed) throw new Error('Unexpected double consumption of response body')
bodyConsumed = true
return init.json ?? {}
},
headers: init.headers ?? {
get: name => {
throw new Error(`Unexpected call to headers.get: ${name}`)
},
},
}
}
type MockContextOverrides = Omit<Partial<OtpContext>, 'process'> & {
process?: Partial<OtpContext['process']>
}
const createOtpMockContext = (overrides?: MockContextOverrides): OtpContext => ({
Date: { now: () => 0 },
setTimeout: (cb: () => void) => cb(),
enquirer: { prompt: async () => ({ otp: '123456' }) },
fetch: async () => createMockResponse({
ok: false,
status: 404,
}),
globalInfo: msg => {
throw new Error(`Unexpected call to globalInfo: ${msg}`)
},
globalWarn: msg => {
throw new Error(`Unexpected call to globalWarn: ${msg}`)
},
...overrides,
process: {
stdin: { isTTY: true },
stdout: { isTTY: true },
...overrides?.process,
},
})
const fetchOptions: WebAuthFetchOptions = { method: 'GET' }
describe('withOtpHandling', () => {
it('returns the result when the operation succeeds without OTP', async () => {
const context = createOtpMockContext()
const result = await withOtpHandling({ context, fetchOptions, operation: async () => 'success' })
expect(result).toBe('success')
})
it('throws non-OTP errors as-is', async () => {
const error = new Error('network error')
const context = createOtpMockContext()
await expect(withOtpHandling({ context, fetchOptions, operation: async () => {
throw error
} }))
.rejects.toBe(error)
})
it('throws OtpNonInteractiveError when terminal is not interactive', async () => {
const context = createOtpMockContext({
process: { stdin: { isTTY: false } },
})
const operation = async () => {
throw Object.assign(new Error('otp'), { code: 'EOTP' })
}
await expect(withOtpHandling({ context, fetchOptions, operation }))
.rejects.toBeInstanceOf(OtpNonInteractiveError)
})
it('throws OtpNonInteractiveError when stdout is not interactive', async () => {
const context = createOtpMockContext({
process: { stdout: { isTTY: false } },
})
const operation = async () => {
throw Object.assign(new Error('otp'), { code: 'EOTP' })
}
await expect(withOtpHandling({ context, fetchOptions, operation }))
.rejects.toBeInstanceOf(OtpNonInteractiveError)
})
describe('classic OTP flow', () => {
it('prompts for OTP and retries operation', async () => {
let callCount = 0
const context = createOtpMockContext({
enquirer: { prompt: async () => ({ otp: '654321' }) },
})
const result = await withOtpHandling({
context,
fetchOptions,
operation: async otp => {
callCount++
if (callCount === 1) {
throw Object.assign(new Error('otp'), { code: 'EOTP' })
}
expect(otp).toBe('654321')
return 'ok'
},
})
expect(result).toBe('ok')
expect(callCount).toBe(2)
})
it('throws OtpSecondChallengeError if retry also requires OTP', async () => {
const context = createOtpMockContext()
const operation = async () => {
throw Object.assign(new Error('otp'), { code: 'EOTP' })
}
await expect(withOtpHandling({ context, fetchOptions, operation }))
.rejects.toBeInstanceOf(OtpSecondChallengeError)
})
it('throws non-OTP errors from the retry as-is', async () => {
let callCount = 0
const retryError = new Error('server error')
const context = createOtpMockContext()
await expect(withOtpHandling({
context,
fetchOptions,
operation: async () => {
callCount++
if (callCount === 1) {
throw Object.assign(new Error('otp'), { code: 'EOTP' })
}
throw retryError
},
})).rejects.toBe(retryError)
})
it('re-throws the original OTP error when enquirer returns no OTP', async () => {
const context = createOtpMockContext({
enquirer: { prompt: async () => ({ otp: '' }) },
})
await expect(withOtpHandling({
context,
fetchOptions,
operation: async () => {
throw Object.assign(new Error('otp'), { code: 'EOTP' })
},
})).rejects.toMatchObject({ code: 'EOTP' })
})
it('re-throws the original OTP error when enquirer returns undefined', async () => {
const context = createOtpMockContext({
enquirer: { prompt: async () => undefined },
})
await expect(withOtpHandling({
context,
fetchOptions,
operation: async () => {
throw Object.assign(new Error('otp'), { code: 'EOTP' })
},
})).rejects.toMatchObject({ code: 'EOTP' })
})
})
describe('webauth flow', () => {
it('polls doneUrl and uses returned token', async () => {
let operationCallCount = 0
let fetchCallCount = 0
const globalInfo = jest.fn()
const context = createOtpMockContext({
globalInfo,
fetch: async (): Promise<WebAuthFetchResponse> => {
fetchCallCount++
if (fetchCallCount < 3) {
return createMockResponse({
ok: true,
status: 202,
headers: { get: () => '1' },
})
}
return createMockResponse({
ok: true,
status: 200,
json: { token: 'web-token-123' },
})
},
})
const result = await withOtpHandling({
context,
fetchOptions,
operation: async otp => {
operationCallCount++
if (operationCallCount === 1) {
throw Object.assign(new Error('otp'), {
code: 'EOTP',
body: {
authUrl: 'https://registry.npmjs.org/auth/abc',
doneUrl: 'https://registry.npmjs.org/auth/abc/done',
},
})
}
expect(otp).toBe('web-token-123')
return 'published'
},
})
expect(result).toBe('published')
expect(operationCallCount).toBe(2)
expect(fetchCallCount).toBe(3)
expect(globalInfo.mock.calls).toEqual([[expect.stringContaining('https://registry.npmjs.org/auth/abc')]])
})
it('falls back to classic prompt when only authUrl is present (no doneUrl)', async () => {
let callCount = 0
const context = createOtpMockContext({
enquirer: { prompt: async () => ({ otp: 'manual-code' }) },
})
const result = await withOtpHandling({
context,
fetchOptions,
operation: async otp => {
callCount++
if (callCount === 1) {
throw Object.assign(new Error('otp'), {
code: 'EOTP',
body: { authUrl: 'https://registry.npmjs.org/auth/abc' },
})
}
expect(otp).toBe('manual-code')
return 'done'
},
})
expect(result).toBe('done')
})
it('falls back to classic prompt when only doneUrl is present (no authUrl)', async () => {
let callCount = 0
const context = createOtpMockContext({
enquirer: { prompt: async () => ({ otp: 'manual-code' }) },
})
const result = await withOtpHandling({
context,
fetchOptions,
operation: async otp => {
callCount++
if (callCount === 1) {
throw Object.assign(new Error('otp'), {
code: 'EOTP',
body: { doneUrl: 'https://registry.npmjs.org/auth/abc/done' },
})
}
expect(otp).toBe('manual-code')
return 'done'
},
})
expect(result).toBe('done')
})
it('throws WebAuthTimeoutError when webauth polling times out', async () => {
let time = 0
const globalInfo = jest.fn()
const context = createOtpMockContext({
globalInfo,
Date: { now: () => time },
setTimeout: (cb: () => void) => {
time += 6 * 60 * 1000
cb()
},
fetch: async (): Promise<WebAuthFetchResponse> => createMockResponse({
ok: true,
status: 202,
headers: { get: () => null },
}),
})
let called = false
await expect(withOtpHandling({
context,
fetchOptions,
operation: async () => {
if (!called) {
called = true
throw Object.assign(new Error('otp'), {
code: 'EOTP',
body: {
authUrl: 'https://registry.npmjs.org/auth/abc',
doneUrl: 'https://registry.npmjs.org/auth/abc/done',
},
})
}
throw new Error('Unexpected second call to operation')
},
})).rejects.toBeInstanceOf(WebAuthTimeoutError)
expect(globalInfo).toHaveBeenCalledWith(expect.stringContaining('https://registry.npmjs.org/auth/abc'))
})
})
})
describe('SyntheticOtpError', () => {
it('has EOTP code', () => {
const err = new SyntheticOtpError({ authUrl: 'https://example.com/auth', doneUrl: 'https://example.com/done' })
expect(err.code).toBe('EOTP')
})
it('stores body', () => {
const body = { authUrl: 'https://example.com/auth', doneUrl: 'https://example.com/done' }
const err = new SyntheticOtpError(body)
expect(err.body).toEqual(body)
})
})
describe('SyntheticOtpError.fromUnknownBody', () => {
const unexpectedWarn = (msg: string) => {
throw new Error(`Unexpected call to globalWarn: ${msg}`)
}
it('extracts valid string authUrl and doneUrl', () => {
const err = SyntheticOtpError.fromUnknownBody(unexpectedWarn, {
authUrl: 'https://example.com/auth',
doneUrl: 'https://example.com/done',
})
expect(err.body).toEqual({
authUrl: 'https://example.com/auth',
doneUrl: 'https://example.com/done',
})
})
it('returns undefined body when body is null', () => {
const err = SyntheticOtpError.fromUnknownBody(unexpectedWarn, null)
expect(err.body).toBeUndefined()
})
it('returns undefined body when body is not an object', () => {
const err = SyntheticOtpError.fromUnknownBody(unexpectedWarn, 'not an object')
expect(err.body).toBeUndefined()
})
it('warns when authUrl has wrong type', () => {
const globalWarn = jest.fn()
const err = SyntheticOtpError.fromUnknownBody(globalWarn, {
authUrl: 123,
doneUrl: 'https://example.com/done',
})
expect(globalWarn.mock.calls).toEqual([[expect.stringContaining('authUrl')]])
expect(err.body?.authUrl).toBeUndefined()
expect(err.body?.doneUrl).toBe('https://example.com/done')
})
it('warns when doneUrl has wrong type', () => {
const globalWarn = jest.fn()
const err = SyntheticOtpError.fromUnknownBody(globalWarn, {
authUrl: 'https://example.com/auth',
doneUrl: true,
})
expect(globalWarn.mock.calls).toEqual([[expect.stringContaining('doneUrl')]])
expect(err.body?.authUrl).toBe('https://example.com/auth')
expect(err.body?.doneUrl).toBeUndefined()
})
it('warns for both when both have wrong types', () => {
const globalWarn = jest.fn()
const err = SyntheticOtpError.fromUnknownBody(globalWarn, {
authUrl: 42,
doneUrl: false,
})
expect(globalWarn.mock.calls).toEqual([
[expect.stringContaining('authUrl')],
[expect.stringContaining('doneUrl')],
])
expect(err.body?.authUrl).toBeUndefined()
expect(err.body?.doneUrl).toBeUndefined()
})
it('returns empty body when body has no authUrl or doneUrl', () => {
const err = SyntheticOtpError.fromUnknownBody(unexpectedWarn, { something: 'else' })
expect(err.body).toEqual({})
})
})