Skip to content

Commit 4c35677

Browse files
committed
Refactor authentication code and add tests
1 parent 54a8fb9 commit 4c35677

File tree

3 files changed

+71
-35
lines changed

3 files changed

+71
-35
lines changed

sources/httpUtils.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,18 @@ async function fetch(input: string | URL, init?: RequestInit) {
2020
const username: string | undefined = input.username || process.env.COREPACK_NPM_USERNAME;
2121
const password: string | undefined = input.password || process.env.COREPACK_NPM_PASSWORD;
2222

23-
if (username || password) {
24-
headers = {
23+
if (username && password) {
24+
const encodedCreds = Buffer.from(`${username}:${password}`, `utf8`).toString(`base64`);
25+
headers = {
2526
...headers,
26-
authorization: `Basic ${Buffer.from(`${username}:${password}`).toString(`base64`)}`,
27+
authorization: `Basic ${encodedCreds}`,
2728
};
2829

2930
input.username = input.password = ``;
3031
}
3132

32-
if ((process.env.COREPACK_NPM_REGISTRY || DEFAULT_NPM_REGISTRY_URL).includes(input.origin) && process.env.COREPACK_NPM_TOKEN) {
33-
headers = {
33+
if (input.origin === new URL(process.env.COREPACK_NPM_REGISTRY || DEFAULT_NPM_REGISTRY_URL).origin && process.env.COREPACK_NPM_TOKEN) {
34+
headers = {
3435
...headers,
3536
authorization: `Bearer ${process.env.COREPACK_NPM_TOKEN}`,
3637
};

sources/npmRegistryUtils.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,6 @@ export async function fetchAsJson(packageName: string, version?: string) {
2121

2222
const headers = {...DEFAULT_HEADERS};
2323

24-
if (`COREPACK_NPM_TOKEN` in process.env) {
25-
headers.authorization = `Bearer ${process.env.COREPACK_NPM_TOKEN}`;
26-
} else if (`COREPACK_NPM_USERNAME` in process.env
27-
&& `COREPACK_NPM_PASSWORD` in process.env) {
28-
const encodedCreds = Buffer.from(`${process.env.COREPACK_NPM_USERNAME}:${process.env.COREPACK_NPM_PASSWORD}`, `utf8`).toString(`base64`);
29-
headers.authorization = `Basic ${encodedCreds}`;
30-
}
31-
3224
return httpUtils.fetchAsJson(`${npmRegistryUrl}/${packageName}${version ? `/${version}` : ``}`, {headers});
3325
}
3426

tests/npmRegistryUtils.test.ts

Lines changed: 65 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import {describe, beforeEach, it, expect, vi} from 'vitest';
55
import {fetchAsJson as httpFetchAsJson} from '../sources/httpUtils';
66
import {DEFAULT_HEADERS, DEFAULT_NPM_REGISTRY_URL, fetchAsJson} from '../sources/npmRegistryUtils';
77

8-
vi.mock(`../sources/httpUtils`);
8+
const fetchMock = vi.fn(() => Promise.resolve({
9+
ok: true,
10+
json: () => Promise.resolve({}),
11+
}));
12+
vi.stubGlobal(`fetch`, fetchMock);
913

1014
describe(`npm registry utils fetchAsJson`, () => {
1115
beforeEach(() => {
@@ -22,17 +26,17 @@ describe(`npm registry utils fetchAsJson`, () => {
2226
it(`loads from DEFAULT_NPM_REGISTRY_URL by default`, async () => {
2327
await fetchAsJson(`package-name`);
2428

25-
expect(httpFetchAsJson).toBeCalled();
26-
expect(httpFetchAsJson).lastCalledWith(`${DEFAULT_NPM_REGISTRY_URL}/package-name`, {headers: DEFAULT_HEADERS});
29+
expect(fetchMock).toBeCalled();
30+
expect(fetchMock).lastCalledWith(new URL(`${DEFAULT_NPM_REGISTRY_URL}/package-name`), expect.objectContaining({headers: DEFAULT_HEADERS}));
2731
});
2832

2933
it(`loads from custom COREPACK_NPM_REGISTRY if set`, async () => {
3034
// `process.env` is reset after each tests in setupTests.js.
3135
process.env.COREPACK_NPM_REGISTRY = `https://registry.example.org`;
3236
await fetchAsJson(`package-name`);
3337

34-
expect(httpFetchAsJson).toBeCalled();
35-
expect(httpFetchAsJson).lastCalledWith(`${process.env.COREPACK_NPM_REGISTRY}/package-name`, {headers: DEFAULT_HEADERS});
38+
expect(fetchMock).toBeCalled();
39+
expect(fetchMock).lastCalledWith(new URL(`${process.env.COREPACK_NPM_REGISTRY}/package-name`), expect.objectContaining({headers: DEFAULT_HEADERS}));
3640
});
3741

3842
it(`adds authorization header with bearer token if COREPACK_NPM_TOKEN is set`, async () => {
@@ -41,11 +45,13 @@ describe(`npm registry utils fetchAsJson`, () => {
4145

4246
await fetchAsJson(`package-name`);
4347

44-
expect(httpFetchAsJson).toBeCalled();
45-
expect(httpFetchAsJson).lastCalledWith(`${DEFAULT_NPM_REGISTRY_URL}/package-name`, {headers: {
46-
...DEFAULT_HEADERS,
47-
authorization: `Bearer ${process.env.COREPACK_NPM_TOKEN}`,
48-
}});
48+
expect(fetchMock).toBeCalled();
49+
expect(fetchMock).lastCalledWith(new URL(`${DEFAULT_NPM_REGISTRY_URL}/package-name`), expect.objectContaining({
50+
headers: {
51+
...DEFAULT_HEADERS,
52+
authorization: `Bearer ${process.env.COREPACK_NPM_TOKEN}`,
53+
},
54+
}));
4955
});
5056

5157
it(`only adds authorization header with bearer token if COREPACK_NPM_TOKEN and COREPACK_NPM_USERNAME are set`, async () => {
@@ -56,11 +62,13 @@ describe(`npm registry utils fetchAsJson`, () => {
5662

5763
await fetchAsJson(`package-name`);
5864

59-
expect(httpFetchAsJson).toBeCalled();
60-
expect(httpFetchAsJson).lastCalledWith(`${DEFAULT_NPM_REGISTRY_URL}/package-name`, {headers: {
61-
...DEFAULT_HEADERS,
62-
authorization: `Bearer ${process.env.COREPACK_NPM_TOKEN}`,
63-
}});
65+
expect(fetchMock).toBeCalled();
66+
expect(fetchMock).lastCalledWith(new URL(`${DEFAULT_NPM_REGISTRY_URL}/package-name`), expect.objectContaining({
67+
headers: {
68+
...DEFAULT_HEADERS,
69+
authorization: `Bearer ${process.env.COREPACK_NPM_TOKEN}`,
70+
},
71+
}));
6472
});
6573

6674

@@ -73,11 +81,13 @@ describe(`npm registry utils fetchAsJson`, () => {
7381

7482
await fetchAsJson(`package-name`);
7583

76-
expect(httpFetchAsJson).toBeCalled();
77-
expect(httpFetchAsJson).lastCalledWith(`${DEFAULT_NPM_REGISTRY_URL}/package-name`, {headers: {
78-
...DEFAULT_HEADERS,
79-
authorization: `Basic ${encodedCreds}`,
80-
}});
84+
expect(fetchMock).toBeCalled();
85+
expect(fetchMock).lastCalledWith(new URL(`${DEFAULT_NPM_REGISTRY_URL}/package-name`), expect.objectContaining({
86+
headers: {
87+
...DEFAULT_HEADERS,
88+
authorization: `Basic ${encodedCreds}`,
89+
},
90+
}));
8191
});
8292

8393
it(`does not add authorization header if COREPACK_NPM_USERNAME is set and COREPACK_NPM_PASSWORD is not.`, async () => {
@@ -86,7 +96,40 @@ describe(`npm registry utils fetchAsJson`, () => {
8696

8797
await fetchAsJson(`package-name`);
8898

89-
expect(httpFetchAsJson).toBeCalled();
90-
expect(httpFetchAsJson).lastCalledWith(`${DEFAULT_NPM_REGISTRY_URL}/package-name`, {headers: DEFAULT_HEADERS});
99+
expect(fetchMock).toBeCalled();
100+
expect(fetchMock).lastCalledWith(new URL(`${DEFAULT_NPM_REGISTRY_URL}/package-name`), expect.objectContaining({headers: DEFAULT_HEADERS}));
101+
});
102+
103+
it(`does add authorization header if registry url contains a path`, async () => {
104+
process.env.COREPACK_NPM_REGISTRY = `https://registry.example.org/some/path`;
105+
process.env.COREPACK_NPM_TOKEN = `foo`;
106+
107+
await fetchAsJson(`package-name`);
108+
109+
expect(fetchMock).toBeCalled();
110+
expect(fetchMock).lastCalledWith(new URL(`https://registry.example.org/some/path/package-name`), expect.objectContaining({
111+
headers: {
112+
...DEFAULT_HEADERS,
113+
authorization: `Bearer ${process.env.COREPACK_NPM_TOKEN}`,
114+
},
115+
}));
116+
});
117+
});
118+
119+
describe(`httpUtils fetchAsJson`, () => {
120+
beforeEach(() => {
121+
vi.resetAllMocks();
122+
});
123+
124+
it(`does not add authorization header if the origin is different from the registry origin`, async () => {
125+
process.env.COREPACK_NPM_REGISTRY = `https://registry.example.org/some/path`;
126+
process.env.COREPACK_NPM_TOKEN = `foo`;
127+
128+
await httpFetchAsJson(`https://another-registry.example.org/package-name`);
129+
130+
expect(fetchMock).toBeCalled();
131+
expect(fetchMock).lastCalledWith(new URL(`https://another-registry.example.org/package-name`), expect.objectContaining({
132+
headers: undefined,
133+
}));
91134
});
92135
});

0 commit comments

Comments
 (0)