Skip to content

Commit e9e857d

Browse files
authored
fix(credential-provider-http): fix token and URI path priority resolution (#8159)
1 parent ee9dc95 commit e9e857d

3 files changed

Lines changed: 122 additions & 14 deletions

File tree

packages-internal/credential-provider-http/src/fromHttp/fromHttp.integ.spec.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
import fs from "node:fs";
12
import http from "node:http";
23
import type { AddressInfo, Socket } from "node:net";
4+
import os from "node:os";
5+
import path from "node:path";
36
import { afterAll, beforeAll, describe, expect, it } from "vitest";
47

58
import { fromHttp } from "./fromHttp";
@@ -9,6 +12,10 @@ describe("fromHttp socket cleanup", () => {
912
let port: number;
1013
let activeConnections: Set<Socket>;
1114

15+
let tokenServer: http.Server;
16+
let tmpDir: string;
17+
18+
1219
beforeAll(async () => {
1320
activeConnections = new Set();
1421

@@ -38,6 +45,13 @@ describe("fromHttp socket cleanup", () => {
3845
socket.destroy();
3946
}
4047
await new Promise<void>((resolve) => server.close(() => resolve()));
48+
49+
if (tokenServer) {
50+
await new Promise<void>((resolve) => tokenServer.close(() => resolve()));
51+
}
52+
if (tmpDir) {
53+
fs.rmSync(tmpDir, { recursive: true });
54+
}
4155
});
4256

4357
it("destroys sockets after each credential fetch", async () => {
@@ -56,6 +70,42 @@ describe("fromHttp socket cleanup", () => {
5670
expect(activeConnections.size).toBe(0);
5771
});
5872

73+
it("re-reads token file on each request", async () => {
74+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "fromHttp-integ-"));
75+
const tokenFile = path.join(tmpDir, "token");
76+
fs.writeFileSync(tokenFile, "token-v1");
77+
78+
const receivedAuths: string[] = [];
79+
tokenServer = http.createServer((req, res) => {
80+
receivedAuths.push(req.headers.authorization ?? "");
81+
res.writeHead(200, { "content-type": "application/json" });
82+
res.end(
83+
JSON.stringify({
84+
AccessKeyId: "AKID",
85+
SecretAccessKey: "SECRET",
86+
Token: "TOKEN",
87+
Expiration: new Date(Date.now() + 3600_000).toISOString(),
88+
})
89+
);
90+
});
91+
92+
await new Promise<void>((resolve) => tokenServer.listen(0, "127.0.0.1", resolve));
93+
const tokenPort = (tokenServer.address() as AddressInfo).port;
94+
95+
const provider = fromHttp({
96+
awsContainerCredentialsFullUri: `http://127.0.0.1:${tokenPort}/creds`,
97+
awsContainerCredentialsRelativeUri: "",
98+
awsContainerAuthorizationTokenFile: tokenFile,
99+
});
100+
101+
await provider();
102+
fs.writeFileSync(tokenFile, "token-v2");
103+
await provider();
104+
105+
expect(receivedAuths[0]).toBe("token-v1");
106+
expect(receivedAuths[1]).toBe("token-v2");
107+
});
108+
59109
it("destroys sockets even when requests fail", async () => {
60110
// Use a server that always returns 500 to trigger retries and failure.
61111
const failServer = http.createServer((_, res) => {

packages-internal/credential-provider-http/src/fromHttp/fromHttp.spec.ts

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { HttpResponse } from "@smithy/core/protocols";
22
import { Readable } from "node:stream";
3-
import { afterAll, describe, expect, test as it, vi } from "vitest";
3+
import { afterAll, beforeEach, describe, expect, test as it, vi } from "vitest";
44

55
import { fromHttp } from "./fromHttp";
66
import * as helpers from "./requestHelpers";
@@ -51,6 +51,11 @@ vi.mock("fs/promises", () => ({
5151
}));
5252

5353
describe(fromHttp.name, () => {
54+
beforeEach(() => {
55+
mockHandle.mockClear();
56+
mockDestroy.mockClear();
57+
});
58+
5459
afterAll(() => {
5560
vi.resetAllMocks();
5661
});
@@ -81,9 +86,23 @@ describe(fromHttp.name, () => {
8186
});
8287
});
8388

89+
it("relative uri should take precedence over full uri", async () => {
90+
const provider = fromHttp({
91+
awsContainerCredentialsFullUri: "https://u1.aws",
92+
awsContainerCredentialsRelativeUri: "/credential-path",
93+
});
94+
95+
await provider();
96+
97+
expect(mockHandle).toHaveBeenCalledWith(helpers.createGetRequest(new URL("http://169.254.170.2/credential-path")), {
98+
requestTimeout: 1000,
99+
});
100+
});
101+
84102
it("can use the token", async () => {
85103
const provider = fromHttp({
86104
awsContainerCredentialsFullUri: "https://t1.aws",
105+
awsContainerCredentialsRelativeUri: "",
87106
awsContainerAuthorizationToken: mockToken,
88107
});
89108

@@ -98,20 +117,48 @@ describe(fromHttp.name, () => {
98117
it("can use the token file", async () => {
99118
const provider = fromHttp({
100119
awsContainerCredentialsFullUri: "https://t2.aws",
120+
awsContainerCredentialsRelativeUri: "",
101121
awsContainerAuthorizationTokenFile: "some-file",
102122
});
103123

104-
const request = helpers.createGetRequest(new URL("https://t1.aws"));
124+
const request = helpers.createGetRequest(new URL("https://t2.aws"));
125+
request.headers.Authorization = mockToken;
126+
127+
await provider();
128+
129+
expect(mockHandle).toHaveBeenCalledWith(request, { requestTimeout: 1000 });
130+
});
131+
132+
it("token file takes precedence over token", async () => {
133+
const provider = fromHttp({
134+
awsContainerCredentialsFullUri: "https://t3.aws",
135+
awsContainerCredentialsRelativeUri: "",
136+
awsContainerAuthorizationToken: "static-token",
137+
awsContainerAuthorizationTokenFile: "some-file",
138+
});
139+
140+
const request = helpers.createGetRequest(new URL("https://t3.aws"));
105141
request.headers.Authorization = mockToken;
106142

107143
await provider();
108144

109145
expect(mockHandle).toHaveBeenCalledWith(request, { requestTimeout: 1000 });
110146
});
111147

148+
it("rejects token containing \\r\\n", async () => {
149+
const provider = fromHttp({
150+
awsContainerCredentialsFullUri: "https://t4.aws",
151+
awsContainerCredentialsRelativeUri: "",
152+
awsContainerAuthorizationToken: "Bearer token\r\nX-Injected: header",
153+
});
154+
155+
await expect(provider()).rejects.toThrow("Authorization token contains invalid \\r\\n sequence.");
156+
});
157+
112158
it("passes custom timeout as requestTimeout to handle()", async () => {
113159
const provider = fromHttp({
114160
awsContainerCredentialsFullUri: "https://u1.aws",
161+
awsContainerCredentialsRelativeUri: "",
115162
timeout: 5000,
116163
});
117164

@@ -121,10 +168,9 @@ describe(fromHttp.name, () => {
121168
});
122169

123170
it("destroys the request handler after the provider resolves", async () => {
124-
mockDestroy.mockClear();
125-
126171
const provider = fromHttp({
127172
awsContainerCredentialsFullUri: "https://u1.aws",
173+
awsContainerCredentialsRelativeUri: "",
128174
});
129175

130176
await provider();
@@ -133,14 +179,14 @@ describe(fromHttp.name, () => {
133179
});
134180

135181
it("destroys the request handler even when the request fails", async () => {
136-
mockDestroy.mockClear();
137182
mockHandle.mockRejectedValueOnce(new Error("network error"));
138183
mockHandle.mockRejectedValueOnce(new Error("network error"));
139184
mockHandle.mockRejectedValueOnce(new Error("network error"));
140185
mockHandle.mockRejectedValueOnce(new Error("network error"));
141186

142187
const provider = fromHttp({
143188
awsContainerCredentialsFullUri: "https://u1.aws",
189+
awsContainerCredentialsRelativeUri: "",
144190
});
145191

146192
await expect(provider()).rejects.toThrow();

packages-internal/credential-provider-http/src/fromHttp/fromHttp.ts

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,21 @@ export const fromHttp = (options: FromHttpOptions = {}): AwsCredentialIdentityPr
3737
"@aws-sdk/credential-provider-http: " +
3838
"you have set both awsContainerCredentialsRelativeUri and awsContainerCredentialsFullUri."
3939
);
40-
warn("awsContainerCredentialsFullUri will take precedence.");
40+
warn("awsContainerCredentialsRelativeUri will take precedence.");
4141
}
4242

4343
if (token && tokenFile) {
4444
warn(
4545
"@aws-sdk/credential-provider-http: " +
4646
"you have set both awsContainerAuthorizationToken and awsContainerAuthorizationTokenFile."
4747
);
48-
warn("awsContainerAuthorizationToken will take precedence.");
48+
warn("awsContainerAuthorizationTokenFile will take precedence.");
4949
}
5050

51-
if (full) {
52-
host = full;
53-
} else if (relative) {
51+
if (relative) {
5452
host = `${DEFAULT_LINK_LOCAL_HOST}${relative}`;
53+
} else if (full) {
54+
host = full;
5555
} else {
5656
throw new CredentialsProviderError(
5757
`No HTTP credential provider host provided.
@@ -73,12 +73,12 @@ Set AWS_CONTAINER_CREDENTIALS_FULL_URI or AWS_CONTAINER_CREDENTIALS_RELATIVE_URI
7373
async (): Promise<AwsCredentialIdentity> => {
7474
const request = createGetRequest(url);
7575

76-
if (token) {
77-
request.headers.Authorization = token;
78-
} else if (tokenFile) {
76+
if (tokenFile) {
7977
// Note: specification requires a file read on each request
8078
// to allow for updates to the file contents.
81-
request.headers.Authorization = (await fs.readFile(tokenFile)).toString();
79+
request.headers.Authorization = validateToken((await fs.readFile(tokenFile)).toString());
80+
} else if (token) {
81+
request.headers.Authorization = validateToken(token);
8282
}
8383
try {
8484
const result = await requestHandler.handle(request, { requestTimeout });
@@ -99,3 +99,15 @@ Set AWS_CONTAINER_CREDENTIALS_FULL_URI or AWS_CONTAINER_CREDENTIALS_RELATIVE_URI
9999
}
100100
};
101101
};
102+
103+
/**
104+
* Validates that the token does not contain the \r\n sequence
105+
* that would otherwise break the HTTP request.
106+
* @internal
107+
*/
108+
const validateToken = (token: string): string => {
109+
if (token.includes("\r\n")) {
110+
throw new CredentialsProviderError("Authorization token contains invalid \\r\\n sequence.");
111+
}
112+
return token;
113+
};

0 commit comments

Comments
 (0)