From 872f81ca1a42bc59edfc2aeae32bbcad34e01e68 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 3 Jun 2020 01:24:11 +0300 Subject: [PATCH] fix: don't always remove auth headers when redirecting Don't remove authorization headers when redirecting requests to the same host. close #2602 PR #2604 --- .changeset/polite-lions-sneeze.md | 5 ++++ packages/fetch-from-npm-registry/src/index.ts | 8 +++--- .../fetch-from-npm-registry/test/index.ts | 25 ++++++++++++++++++- 3 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 .changeset/polite-lions-sneeze.md diff --git a/.changeset/polite-lions-sneeze.md b/.changeset/polite-lions-sneeze.md new file mode 100644 index 0000000000..641b90f081 --- /dev/null +++ b/.changeset/polite-lions-sneeze.md @@ -0,0 +1,5 @@ +--- +"fetch-from-npm-registry": patch +--- + +Don't remove authorization headers when redirecting requests to the same host. diff --git a/packages/fetch-from-npm-registry/src/index.ts b/packages/fetch-from-npm-registry/src/index.ts index 72e96692f1..39a8b68197 100644 --- a/packages/fetch-from-npm-registry/src/index.ts +++ b/packages/fetch-from-npm-registry/src/index.ts @@ -43,8 +43,10 @@ export default function ( } let redirects = 0 + let urlObject = new URL(url) + const originalHost = urlObject.host while (true) { - const agent = npmRegistryAgent(url, { + const agent = npmRegistryAgent(urlObject.href, { ...defaultOpts, ...opts, } as any) // tslint:disable-line @@ -52,7 +54,6 @@ export default function ( // We should pass a URL object to node-fetch till this is not resolved: // https://github.com/bitinn/node-fetch/issues/245 - const urlObject = new URL(url) let response = await fetch(urlObject, { agent, // if verifying integrity, node-fetch must not decompress @@ -68,7 +69,8 @@ export default function ( // This is a workaround to remove authorization headers on redirect. // Related pnpm issue: https://github.com/pnpm/pnpm/issues/1815 redirects++ - url = response.headers.get('location')! + urlObject = new URL(response.headers.get('location')!) + if (!headers['authorization'] || originalHost === urlObject.host) continue delete headers['authorization'] } } diff --git a/packages/fetch-from-npm-registry/test/index.ts b/packages/fetch-from-npm-registry/test/index.ts index 31a4a1720d..48dac8cf18 100644 --- a/packages/fetch-from-npm-registry/test/index.ts +++ b/packages/fetch-from-npm-registry/test/index.ts @@ -21,7 +21,7 @@ test('fetchFromNpmRegistry fullMetadata', async t => { t.end() }) -test('authorization headers are removed before redirection', async (t) => { +test('authorization headers are removed before redirection if the target is on a different host', async (t) => { nock('http://registry.pnpm.js.org/', { reqheaders: { authorization: 'Bearer 123' }, }) @@ -42,6 +42,29 @@ test('authorization headers are removed before redirection', async (t) => { t.end() }) +test('authorization headers are not removed before redirection if the target is on the same host', async (t) => { + nock('http://registry.pnpm.js.org/', { + reqheaders: { authorization: 'Bearer 123' }, + }) + .get('/is-positive') + .reply(302, '', { location: 'http://registry.pnpm.js.org/is-positive-new' }) + nock('http://registry.pnpm.js.org/', { + reqheaders: { authorization: 'Bearer 123' }, + }) + .get('/is-positive-new') + .reply(200, { ok: true }) + + const fetchFromNpmRegistry = createRegClient({ fullMetadata: true }) + const res = await fetchFromNpmRegistry( + 'http://registry.pnpm.js.org/is-positive', + { authHeaderValue: 'Bearer 123' } + ) + + t.deepEqual(await res.json(), { ok: true }) + t.ok(nock.isDone()) + t.end() +}) + test('switch to the correct agent for requests on redirect from http: to https:', async (t) => { const fetchFromNpmRegistry = createRegClient({ fullMetadata: true })