From 65953671c5944df1b33694f7b3790e6389b5c7ca Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Mon, 27 Nov 2023 14:50:33 +0100 Subject: [PATCH 1/7] feat: improve HTTP redirects behavior This commit modifies the Node core so that it will include "safe" headers when performing a cross-site redirect where both the original and redirected hosts are within IBM's "cloud.ibm.com" domain. Signed-off-by: Norbert Biczo --- lib/request-wrapper.ts | 36 ++++++- test/unit/redirect.test.js | 211 +++++++++++++++++++++++++++++++++++++ 2 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 test/unit/redirect.test.js diff --git a/lib/request-wrapper.ts b/lib/request-wrapper.ts index 34b1379ea..4a0cbed42 100644 --- a/lib/request-wrapper.ts +++ b/lib/request-wrapper.ts @@ -1,4 +1,4 @@ -/* eslint-disable class-methods-use-this */ +/* eslint-disable class-methods-use-this, no-restricted-syntax */ /** * (C) Copyright IBM Corp. 2014, 2023. @@ -238,6 +238,20 @@ export class RequestWrapper { data = await this.gzipRequestBody(data, headers); } + // Holds the hostname of the current request. + // It's used between redirects. + let currentHost = new URL(url).hostname; + + // Headers that are considered being safe + // and can be copied during HTTP redirects + // on the `cloud.ibm.com` namespace. + const safeHeaders = { + Authorization: headers.Authorization, + Cookie: headers.Cookie, + Cookie2: headers.Cookie2, + 'WWW-Authenticate': headers['WWW-Authenticate'], + }; + const requestParams = { url, method, @@ -247,6 +261,21 @@ export class RequestWrapper { raxConfig: this.raxConfig, responseType: options.responseType || 'json', paramsSerializer: { serialize: (params) => stringify(params) }, + maxRedirects: 10, + beforeRedirect: (nextRequest: any) => { + if (shouldCopySafeHeadersOnRedirect(currentHost, nextRequest.hostname)) { + for (const [name, value] of Object.entries(safeHeaders)) { + // Copy the header to the redirected request if it's defined + // and not already present. + if (value && nextRequest.headers[name] === undefined) { + nextRequest.headers[name] = value; + } + } + + // Update the host for the next redirect. + currentHost = nextRequest.hostname; + } + }, }; return this.axiosInstance(requestParams).then( @@ -580,3 +609,8 @@ function ensureJSONResponseBodyIsObject(response: any): any | string { return dataAsObject; } + +// Returns true iff safe headers should be copied to a redirected request. +function shouldCopySafeHeadersOnRedirect(fromHost: string, toHost: string): boolean { + return fromHost.endsWith('.cloud.ibm.com') && toHost.endsWith('.cloud.ibm.com'); +} diff --git a/test/unit/redirect.test.js b/test/unit/redirect.test.js new file mode 100644 index 000000000..1cbfdfb71 --- /dev/null +++ b/test/unit/redirect.test.js @@ -0,0 +1,211 @@ +/** + * Copyright 2023 IBM Corp. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const nock = require('nock'); +const { AxiosError } = require('axios'); +const { NoAuthAuthenticator, BaseService } = require('../../dist'); + +// Disable real network connections. +nock.disableNetConnect(); + +const safeHeaders = { + Authorization: 'foo', + Cookie: 'baz', + Cookie2: 'baz2', + 'WWW-Authenticate': 'bar', +}; + +function initService(url) { + const service = new BaseService({ + authenticator: new NoAuthAuthenticator(), + serviceUrl: url, + }); + + const parameters = { + options: { + method: 'GET', + url: '/', + headers: safeHeaders, + }, + defaultOptions: { + serviceUrl: url, + }, + }; + + return { service, parameters }; +} + +describe('Node Core redirects', () => { + afterEach(() => { + nock.cleanAll(); + }); + + it('should include safe headers within cloud.ibm.com domain', async () => { + const url1 = 'http://region1.cloud.ibm.com'; + const url2 = 'http://region2.cloud.ibm.com'; + + const { service, parameters } = initService(url1); + + const scopes = [ + nock(url1) + .matchHeader('Authorization', safeHeaders.Authorization) + .matchHeader('Cookie', safeHeaders.Cookie) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(302, 'just about to redirect', { Location: url2 }), + nock(url2) + .matchHeader('Authorization', safeHeaders.Authorization) + .matchHeader('Cookie', safeHeaders.Cookie) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(200, 'successfully redirected'), + ]; + + const result = await service.createRequest(parameters); + expect(result.result).toBe('successfully redirected'); + expect(result.status).toBe(200); + + // Ensure all mocks satisfied. + scopes.forEach((s) => s.done()); + }); + + it('should exclude safe headers from cloud.ibm.com to not cloud.ibm.com domain', async () => { + const url1 = 'http://region1.cloud.ibm.com'; + const url2 = 'http://region2.notcloud.ibm.com'; + + const { service, parameters } = initService(url1); + + const scopes = [ + nock(url1) + .matchHeader('Authorization', safeHeaders.Authorization) + .matchHeader('Cookie', safeHeaders.Cookie) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(302, 'just about to redirect', { Location: url2 }), + nock(url2) + .matchHeader('Authorization', (val) => val === undefined) + .matchHeader('Cookie', (val) => val === undefined) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(200, 'successfully redirected'), + ]; + + const result = await service.createRequest(parameters); + + expect(result.result).toBe('successfully redirected'); + expect(result.status).toBe(200); + + // Ensure all mocks satisfied. + scopes.forEach((s) => s.done()); + }); + + it('should exclude safe headers from not cloud.ibm.com to cloud.ibm.com domain', async () => { + const url1 = 'http://region2.notcloud.ibm.com'; + const url2 = 'http://region1.cloud.ibm.com'; + + const { service, parameters } = initService(url1); + + const scopes = [ + nock(url1) + .matchHeader('Authorization', safeHeaders.Authorization) + .matchHeader('Cookie', safeHeaders.Cookie) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(302, 'just about to redirect', { Location: url2 }), + nock(url2) + .matchHeader('Authorization', (val) => val === undefined) + .matchHeader('Cookie', (val) => val === undefined) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(200, 'successfully redirected'), + ]; + + const result = await service.createRequest(parameters); + + expect(result.result).toBe('successfully redirected'); + expect(result.status).toBe(200); + + // Ensure all mocks satisfied. + scopes.forEach((s) => s.done()); + }); + + it('should exclude safe headers from not cloud.ibm.com to not cloud.ibm.com domain', async () => { + const url1 = 'http://region1.notcloud.ibm.com'; + const url2 = 'http://region2.notcloud.ibm.com'; + + const { service, parameters } = initService(url1); + + const scopes = [ + nock(url1) + .matchHeader('Authorization', safeHeaders.Authorization) + .matchHeader('Cookie', safeHeaders.Cookie) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(302, 'just about to redirect', { Location: url2 }), + nock(url2) + .matchHeader('Authorization', (val) => val === undefined) + .matchHeader('Cookie', (val) => val === undefined) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(200, 'successfully redirected'), + ]; + + const result = await service.createRequest(parameters); + + expect(result.result).toBe('successfully redirected'); + expect(result.status).toBe(200); + + // Ensure all mocks satisfied. + scopes.forEach((s) => s.done()); + }); + + it('should fail due to exhaustion', async () => { + const scopes = []; + for (let i = 1; i <= 11; i++) { + scopes.push( + nock(`http://region${i}.cloud.ibm.com`) + .matchHeader('Authorization', safeHeaders.Authorization) + .matchHeader('Cookie', safeHeaders.Cookie) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(302, 'just about to redirect', { Location: `http://region${i + 1}.cloud.ibm.com` }) + ); + } + + const { service, parameters } = initService('http://region1.cloud.ibm.com'); + + let result; + let error; + try { + result = await service.createRequest(parameters); + } catch (err) { + error = err; + } + + expect(result).toBeUndefined(); + expect(error).not.toBeUndefined(); + expect(error.statusText).toBe(AxiosError.ERR_FR_TOO_MANY_REDIRECTS); + }); +}); From d8f96e5cdfa585c41a0b97ef79f5a0c203d2a565 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Mon, 27 Nov 2023 14:52:38 +0100 Subject: [PATCH 2/7] chore: update .secrets.baseline Signed-off-by: Norbert Biczo --- .secrets.baseline | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 1f229f736..d95c07b58 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "package-lock.json|^.secrets.baseline$", "lines": null }, - "generated_at": "2023-11-09T19:29:53Z", + "generated_at": "2023-11-27T13:52:22Z", "plugins_used": [ { "name": "AWSKeyDetector" @@ -96,7 +96,7 @@ "hashed_secret": "bc2f74c22f98f7b6ffbc2f67453dbfa99bce9a32", "is_secret": false, "is_verified": false, - "line_number": 97, + "line_number": 111, "type": "Secret Keyword", "verified_result": null } From aae3f89a72e59c9aff0bc3ee35906c4c9dc9e322 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Mon, 27 Nov 2023 14:55:56 +0100 Subject: [PATCH 3/7] chore: fix command in the GitHub PR template Signed-off-by: Norbert Biczo --- .github/pull_request_template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index fd6d3cf37..8b370902a 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -9,6 +9,6 @@ Bug fixes and new features should include tests whenever possible. ##### Checklist -- [ ] `npm test` passes (tip: `npm run lint-fix` can correct most style issues) +- [ ] `npm test` passes (tip: `npm run lint:fix` can correct most style issues) - [ ] tests are included - [ ] documentation is changed or added From 4773399c3c619d7cf89e96c30c8dce57ed3255b8 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Thu, 30 Nov 2023 12:59:40 +0100 Subject: [PATCH 4/7] chore: test improvements Signed-off-by: Norbert Biczo --- test/unit/redirect.test.js | 86 +++++++++++++++++++++++++++++++++----- 1 file changed, 75 insertions(+), 11 deletions(-) diff --git a/test/unit/redirect.test.js b/test/unit/redirect.test.js index 1cbfdfb71..954f37fa1 100644 --- a/test/unit/redirect.test.js +++ b/test/unit/redirect.test.js @@ -54,8 +54,8 @@ describe('Node Core redirects', () => { }); it('should include safe headers within cloud.ibm.com domain', async () => { - const url1 = 'http://region1.cloud.ibm.com'; - const url2 = 'http://region2.cloud.ibm.com'; + const url1 = 'https://region1.cloud.ibm.com'; + const url2 = 'https://region2.cloud.ibm.com'; const { service, parameters } = initService(url1); @@ -85,8 +85,8 @@ describe('Node Core redirects', () => { }); it('should exclude safe headers from cloud.ibm.com to not cloud.ibm.com domain', async () => { - const url1 = 'http://region1.cloud.ibm.com'; - const url2 = 'http://region2.notcloud.ibm.com'; + const url1 = 'https://region1.cloud.ibm.com'; + const url2 = 'https://region2.notcloud.ibm.com'; const { service, parameters } = initService(url1); @@ -117,8 +117,8 @@ describe('Node Core redirects', () => { }); it('should exclude safe headers from not cloud.ibm.com to cloud.ibm.com domain', async () => { - const url1 = 'http://region2.notcloud.ibm.com'; - const url2 = 'http://region1.cloud.ibm.com'; + const url1 = 'https://region2.notcloud.ibm.com'; + const url2 = 'https://region1.cloud.ibm.com'; const { service, parameters } = initService(url1); @@ -149,8 +149,8 @@ describe('Node Core redirects', () => { }); it('should exclude safe headers from not cloud.ibm.com to not cloud.ibm.com domain', async () => { - const url1 = 'http://region1.notcloud.ibm.com'; - const url2 = 'http://region2.notcloud.ibm.com'; + const url1 = 'https://region1.notcloud.ibm.com'; + const url2 = 'https://region2.notcloud.ibm.com'; const { service, parameters } = initService(url1); @@ -180,21 +180,85 @@ describe('Node Core redirects', () => { scopes.forEach((s) => s.done()); }); + it('should include safe headers for the same host within cloud.ibm.com', async () => { + const url1 = 'https://region1.cloud.ibm.com'; + const url2 = 'https://region1.cloud.ibm.com'; + + const { service, parameters } = initService(url1); + + const scopes = [ + nock(url1) + .matchHeader('Authorization', safeHeaders.Authorization) + .matchHeader('Cookie', safeHeaders.Cookie) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(302, 'just about to redirect', { Location: url2 }), + nock(url2) + .matchHeader('Authorization', safeHeaders.Authorization) + .matchHeader('Cookie', safeHeaders.Cookie) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(200, 'successfully redirected'), + ]; + + const result = await service.createRequest(parameters); + + expect(result.result).toBe('successfully redirected'); + expect(result.status).toBe(200); + + // Ensure all mocks satisfied. + scopes.forEach((s) => s.done()); + }); + + it('should include safe headers for the same host outside cloud.ibm.com', async () => { + const url1 = 'https://region2.notcloud.ibm.com'; + const url2 = 'https://region2.notcloud.ibm.com'; + + const { service, parameters } = initService(url1); + + const scopes = [ + nock(url1) + .matchHeader('Authorization', safeHeaders.Authorization) + .matchHeader('Cookie', safeHeaders.Cookie) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(302, 'just about to redirect', { Location: url2 }), + nock(url2) + .matchHeader('Authorization', safeHeaders.Authorization) + .matchHeader('Cookie', safeHeaders.Cookie) + .matchHeader('Cookie2', safeHeaders.Cookie2) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) + .get('/') + .reply(200, 'successfully redirected'), + ]; + + const result = await service.createRequest(parameters); + + expect(result.result).toBe('successfully redirected'); + expect(result.status).toBe(200); + + // Ensure all mocks satisfied. + scopes.forEach((s) => s.done()); + }); + it('should fail due to exhaustion', async () => { const scopes = []; for (let i = 1; i <= 11; i++) { scopes.push( - nock(`http://region${i}.cloud.ibm.com`) + nock(`https://region${i}.cloud.ibm.com`) .matchHeader('Authorization', safeHeaders.Authorization) .matchHeader('Cookie', safeHeaders.Cookie) .matchHeader('Cookie2', safeHeaders.Cookie2) .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) .get('/') - .reply(302, 'just about to redirect', { Location: `http://region${i + 1}.cloud.ibm.com` }) + .reply(302, 'just about to redirect', { Location: `https://region${i + 1}.cloud.ibm.com` }) ); } - const { service, parameters } = initService('http://region1.cloud.ibm.com'); + const { service, parameters } = initService('https://region1.cloud.ibm.com'); let result; let error; From b385b34ac43ed2b6f4d8437f27745e8928bd779d Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Wed, 6 Dec 2023 12:46:33 +0100 Subject: [PATCH 5/7] chore: revamp unit tests Signed-off-by: Norbert Biczo chore: small improvements Signed-off-by: Norbert Biczo chore: fix tests Signed-off-by: Norbert Biczo chore: fix unit tests Signed-off-by: Norbert Biczo chore: lint fix Signed-off-by: Norbert Biczo --- test/unit/redirect.test.js | 426 ++++++++++++++++++++----------------- 1 file changed, 231 insertions(+), 195 deletions(-) diff --git a/test/unit/redirect.test.js b/test/unit/redirect.test.js index 954f37fa1..a45eb3264 100644 --- a/test/unit/redirect.test.js +++ b/test/unit/redirect.test.js @@ -21,6 +21,13 @@ const { NoAuthAuthenticator, BaseService } = require('../../dist'); // Disable real network connections. nock.disableNetConnect(); +const url = { + cloud1: 'https://region1.cloud.ibm.com', + cloud2: 'https://region2.cloud.ibm.com', + notCloud1: 'https://region1.notcloud.ibm.com', + notCloud2: 'https://region2.notcloud.ibm.com', +}; + const safeHeaders = { Authorization: 'foo', Cookie: 'baz', @@ -28,221 +35,234 @@ const safeHeaders = { 'WWW-Authenticate': 'bar', }; -function initService(url) { - const service = new BaseService({ - authenticator: new NoAuthAuthenticator(), - serviceUrl: url, - }); - - const parameters = { - options: { - method: 'GET', - url: '/', - headers: safeHeaders, - }, - defaultOptions: { - serviceUrl: url, - }, - }; - - return { service, parameters }; -} - -describe('Node Core redirects', () => { - afterEach(() => { - nock.cleanAll(); - }); - - it('should include safe headers within cloud.ibm.com domain', async () => { - const url1 = 'https://region1.cloud.ibm.com'; - const url2 = 'https://region2.cloud.ibm.com'; - - const { service, parameters } = initService(url1); - - const scopes = [ - nock(url1) +const testPaths = [ + { port: '', path: '/', redirectedPort: '', redirectedPath: '/' }, + { port: ':3000', path: '/', redirectedPort: '', redirectedPath: '/' }, + { port: ':3000', path: '/', redirectedPort: ':3333', redirectedPath: '/' }, + { + port: '', + path: '/a/very/long/path/with?some=query¶ms=the_end', + redirectedPort: '', + redirectedPath: '/', + }, + { + port: ':3000', + path: '/a/very/long/path/with?some=query¶ms=the_end', + redirectedPort: '', + redirectedPath: '/', + }, + { + port: ':3000', + path: '/a/very/long/path/with?some=query¶ms=the_end', + redirectedPort: '', + redirectedPath: '/api/v1', + }, + { + port: ':3000', + path: '/a/very/long/path/with?some=query¶ms=the_end', + redirectedPort: ':3000', + redirectedPath: '/api/v1', + }, +]; + +// These test cases are created based on the logic in the `follow-redirects` package. +// See more: https://github.com/follow-redirects/follow-redirects/blob/main/index.js#L402 +const testMatrix = [ + { + status1: 301, + status2: 200, + method1: 'GET', + method2: 'GET', + methodExpected: 'GET', + bodyDropped: false, + }, + { + status1: 301, + status2: 200, + method1: 'POST', + method2: 'GET', + methodExpected: 'GET', + bodyDropped: true, + }, + { + status1: 302, + status2: 200, + method1: 'GET', + method2: 'GET', + methodExpected: 'GET', + bodyDropped: false, + }, + { + status1: 302, + status2: 200, + method1: 'POST', + method2: 'GET', + methodExpected: 'GET', + bodyDropped: true, + }, + { + status1: 303, + status2: 200, + method1: 'GET', + method2: 'GET', + methodExpected: 'GET', + bodyDropped: false, + }, + { + status1: 303, + status2: 200, + method1: 'POST', + method2: 'GET', + methodExpected: 'GET', + bodyDropped: true, + }, + { + status1: 307, + status2: 200, + method1: 'GET', + method2: 'GET', + methodExpected: 'GET', + bodyDropped: false, + }, + { + status1: 307, + status2: 200, + method1: 'POST', + method2: 'POST', + methodExpected: 'POST', + bodyDropped: false, + }, + { + status1: 308, + status2: 200, + method1: 'GET', + method2: 'GET', + methodExpected: 'GET', + bodyDropped: false, + }, + { + status1: 308, + status2: 200, + method1: 'POST', + method2: 'POST', + methodExpected: 'POST', + bodyDropped: false, + }, +]; + +async function runTest(url1, url2, safeHeadersIncluded) { + // eslint-disable-next-line no-restricted-syntax + for (const testPath of testPaths) { + // eslint-disable-next-line no-restricted-syntax + for (const testCase of testMatrix) { + const service = new BaseService({ + authenticator: new NoAuthAuthenticator(), + serviceUrl: url1 + testPath.port, + }); + + const parameters = { + options: { + method: testCase.method1, + url: testPath.path, + headers: safeHeaders, + body: 'Am I redirected?', + }, + defaultOptions: { + serviceUrl: url1 + testPath.port, + }, + }; + + // Put together the first mock. + let scope1 = nock(url1 + testPath.port) .matchHeader('Authorization', safeHeaders.Authorization) .matchHeader('Cookie', safeHeaders.Cookie) .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(302, 'just about to redirect', { Location: url2 }), - nock(url2) - .matchHeader('Authorization', safeHeaders.Authorization) - .matchHeader('Cookie', safeHeaders.Cookie) + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']); + + switch (testCase.method1) { + case 'GET': + scope1 = scope1.get(testPath.path); + break; + case 'POST': + scope1 = scope1.post(testPath.path); + break; + default: + throw new Error('unsupported HTTP method'); + } + + scope1 = scope1.reply((_, requestBody) => [ + testCase.status1, + requestBody, + { Location: url2 + testPath.redirectedPort + testPath.redirectedPath }, + ]); + + // Now create the second, redirected mock. + let scope2 = nock(url2 + testPath.redirectedPort) + .matchHeader( + 'Authorization', + (val) => val === (safeHeadersIncluded ? safeHeaders.Authorization : undefined) + ) + .matchHeader( + 'Cookie', + (val) => val === (safeHeadersIncluded ? safeHeaders.Cookie : undefined) + ) .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(200, 'successfully redirected'), - ]; + .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']); + + switch (testCase.method2) { + case 'GET': + scope2 = scope2.get(testPath.redirectedPath); + break; + case 'POST': + scope2 = scope2.post(testPath.redirectedPath); + break; + default: + throw new Error('unsupported HTTP method'); + } + + scope2 = scope2.reply((_, requestBody) => [testCase.status2, requestBody]); + + // eslint-disable-next-line no-await-in-loop + const result = await service.createRequest(parameters); + expect(result.result).toBe(testCase.bodyDropped ? '' : 'Am I redirected?'); + expect(result.status).toBe(200); + + // Ensure all mocks satisfied. + scope1.done(); + scope2.done(); + } + } - const result = await service.createRequest(parameters); - expect(result.result).toBe('successfully redirected'); - expect(result.status).toBe(200); + // Clean-up the mocks. + nock.cleanAll(); +} - // Ensure all mocks satisfied. - scopes.forEach((s) => s.done()); +describe('Node Core redirects', () => { + /* eslint-disable jest/expect-expect */ + it('should include safe headers within cloud.ibm.com domain', async () => { + await runTest(url.cloud1, url.cloud2, true); }); it('should exclude safe headers from cloud.ibm.com to not cloud.ibm.com domain', async () => { - const url1 = 'https://region1.cloud.ibm.com'; - const url2 = 'https://region2.notcloud.ibm.com'; - - const { service, parameters } = initService(url1); - - const scopes = [ - nock(url1) - .matchHeader('Authorization', safeHeaders.Authorization) - .matchHeader('Cookie', safeHeaders.Cookie) - .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(302, 'just about to redirect', { Location: url2 }), - nock(url2) - .matchHeader('Authorization', (val) => val === undefined) - .matchHeader('Cookie', (val) => val === undefined) - .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(200, 'successfully redirected'), - ]; - - const result = await service.createRequest(parameters); - - expect(result.result).toBe('successfully redirected'); - expect(result.status).toBe(200); - - // Ensure all mocks satisfied. - scopes.forEach((s) => s.done()); + await runTest(url.cloud1, url.notCloud2, false); }); it('should exclude safe headers from not cloud.ibm.com to cloud.ibm.com domain', async () => { - const url1 = 'https://region2.notcloud.ibm.com'; - const url2 = 'https://region1.cloud.ibm.com'; - - const { service, parameters } = initService(url1); - - const scopes = [ - nock(url1) - .matchHeader('Authorization', safeHeaders.Authorization) - .matchHeader('Cookie', safeHeaders.Cookie) - .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(302, 'just about to redirect', { Location: url2 }), - nock(url2) - .matchHeader('Authorization', (val) => val === undefined) - .matchHeader('Cookie', (val) => val === undefined) - .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(200, 'successfully redirected'), - ]; - - const result = await service.createRequest(parameters); - - expect(result.result).toBe('successfully redirected'); - expect(result.status).toBe(200); - - // Ensure all mocks satisfied. - scopes.forEach((s) => s.done()); + await runTest(url.notCloud2, url.cloud1, false); }); it('should exclude safe headers from not cloud.ibm.com to not cloud.ibm.com domain', async () => { - const url1 = 'https://region1.notcloud.ibm.com'; - const url2 = 'https://region2.notcloud.ibm.com'; - - const { service, parameters } = initService(url1); - - const scopes = [ - nock(url1) - .matchHeader('Authorization', safeHeaders.Authorization) - .matchHeader('Cookie', safeHeaders.Cookie) - .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(302, 'just about to redirect', { Location: url2 }), - nock(url2) - .matchHeader('Authorization', (val) => val === undefined) - .matchHeader('Cookie', (val) => val === undefined) - .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(200, 'successfully redirected'), - ]; - - const result = await service.createRequest(parameters); - - expect(result.result).toBe('successfully redirected'); - expect(result.status).toBe(200); - - // Ensure all mocks satisfied. - scopes.forEach((s) => s.done()); + await runTest(url.notCloud1, url.notCloud2, false); }); it('should include safe headers for the same host within cloud.ibm.com', async () => { - const url1 = 'https://region1.cloud.ibm.com'; - const url2 = 'https://region1.cloud.ibm.com'; - - const { service, parameters } = initService(url1); - - const scopes = [ - nock(url1) - .matchHeader('Authorization', safeHeaders.Authorization) - .matchHeader('Cookie', safeHeaders.Cookie) - .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(302, 'just about to redirect', { Location: url2 }), - nock(url2) - .matchHeader('Authorization', safeHeaders.Authorization) - .matchHeader('Cookie', safeHeaders.Cookie) - .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(200, 'successfully redirected'), - ]; - - const result = await service.createRequest(parameters); - - expect(result.result).toBe('successfully redirected'); - expect(result.status).toBe(200); - - // Ensure all mocks satisfied. - scopes.forEach((s) => s.done()); + await runTest(url.cloud1, url.cloud1, true); }); it('should include safe headers for the same host outside cloud.ibm.com', async () => { - const url1 = 'https://region2.notcloud.ibm.com'; - const url2 = 'https://region2.notcloud.ibm.com'; - - const { service, parameters } = initService(url1); - - const scopes = [ - nock(url1) - .matchHeader('Authorization', safeHeaders.Authorization) - .matchHeader('Cookie', safeHeaders.Cookie) - .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(302, 'just about to redirect', { Location: url2 }), - nock(url2) - .matchHeader('Authorization', safeHeaders.Authorization) - .matchHeader('Cookie', safeHeaders.Cookie) - .matchHeader('Cookie2', safeHeaders.Cookie2) - .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) - .get('/') - .reply(200, 'successfully redirected'), - ]; - - const result = await service.createRequest(parameters); - - expect(result.result).toBe('successfully redirected'); - expect(result.status).toBe(200); - - // Ensure all mocks satisfied. - scopes.forEach((s) => s.done()); + await runTest(url.notCloud2, url.notCloud2, true); }); + /* eslint-enable jest/expect-expect */ it('should fail due to exhaustion', async () => { const scopes = []; @@ -254,11 +274,27 @@ describe('Node Core redirects', () => { .matchHeader('Cookie2', safeHeaders.Cookie2) .matchHeader('WWW-Authenticate', safeHeaders['WWW-Authenticate']) .get('/') - .reply(302, 'just about to redirect', { Location: `https://region${i + 1}.cloud.ibm.com` }) + .reply(302, 'just about to redirect', { + Location: `https://region${i + 1}.cloud.ibm.com`, + }) ); } - const { service, parameters } = initService('https://region1.cloud.ibm.com'); + const service = new BaseService({ + authenticator: new NoAuthAuthenticator(), + serviceUrl: 'https://region1.cloud.ibm.com', + }); + + const parameters = { + options: { + method: 'GET', + url: '/', + headers: safeHeaders, + }, + defaultOptions: { + serviceUrl: 'https://region1.cloud.ibm.com', + }, + }; let result; let error; From 71eb245c69c3981cd945959e0074bf2ec7419158 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Thu, 4 Jan 2024 14:35:01 +0100 Subject: [PATCH 6/7] chore: update `nock` to latest Signed-off-by: Norbert Biczo --- package-lock.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index b5f6b7898..97082e550 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10865,9 +10865,9 @@ "dev": true }, "node_modules/nock": { - "version": "13.3.8", - "resolved": "https://registry.npmjs.org/nock/-/nock-13.3.8.tgz", - "integrity": "sha512-96yVFal0c/W1lG7mmfRe7eO+hovrhJYd2obzzOZ90f6fjpeU/XNvd9cYHZKZAQJumDfhXgoTpkpJ9pvMj+hqHw==", + "version": "13.4.0", + "resolved": "https://registry.npmjs.org/nock/-/nock-13.4.0.tgz", + "integrity": "sha512-W8NVHjO/LCTNA64yxAPHV/K47LpGYcVzgKd3Q0n6owhwvD0Dgoterc25R4rnZbckJEb6Loxz1f5QMuJpJnbSyQ==", "dev": true, "dependencies": { "debug": "^4.1.0", @@ -25880,9 +25880,9 @@ "dev": true }, "nock": { - "version": "13.3.8", - "resolved": "https://registry.npmjs.org/nock/-/nock-13.3.8.tgz", - "integrity": "sha512-96yVFal0c/W1lG7mmfRe7eO+hovrhJYd2obzzOZ90f6fjpeU/XNvd9cYHZKZAQJumDfhXgoTpkpJ9pvMj+hqHw==", + "version": "13.4.0", + "resolved": "https://registry.npmjs.org/nock/-/nock-13.4.0.tgz", + "integrity": "sha512-W8NVHjO/LCTNA64yxAPHV/K47LpGYcVzgKd3Q0n6owhwvD0Dgoterc25R4rnZbckJEb6Loxz1f5QMuJpJnbSyQ==", "dev": true, "requires": { "debug": "^4.1.0", From f85d30e446812bad91f7c2bac508778ab470ece5 Mon Sep 17 00:00:00 2001 From: Norbert Biczo Date: Thu, 4 Jan 2024 15:06:28 +0100 Subject: [PATCH 7/7] fix: use the correct conditions when copying safe headers Signed-off-by: Norbert Biczo --- lib/request-wrapper.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/request-wrapper.ts b/lib/request-wrapper.ts index 4a0cbed42..19ba962f5 100644 --- a/lib/request-wrapper.ts +++ b/lib/request-wrapper.ts @@ -612,5 +612,7 @@ function ensureJSONResponseBodyIsObject(response: any): any | string { // Returns true iff safe headers should be copied to a redirected request. function shouldCopySafeHeadersOnRedirect(fromHost: string, toHost: string): boolean { - return fromHost.endsWith('.cloud.ibm.com') && toHost.endsWith('.cloud.ibm.com'); + const sameHost = fromHost === toHost; + const safeDomain = fromHost.endsWith('.cloud.ibm.com') && toHost.endsWith('.cloud.ibm.com'); + return sameHost || safeDomain; }