Skip to content

Commit

Permalink
fix(s3-request-presigner): identify correct authscheme for mrap (#5742)
Browse files Browse the repository at this point in the history
* fix(s3-request-presigner): adjust signing region based on authScheme for sigv4a

* fix(s3-request-presigner): identify correct authscheme for mrap

* fix(s3-request-presigner): small refactor

* chore: remove unused import

---------

Co-authored-by: Ran Vaknin <rvaknin@dev-dsk-rvaknin2-2a-e69e90c1.us-west-2.amazon.com>
Co-authored-by: RanVaknin <RanVaknin@users.noreply.github.com>
Co-authored-by: George Fu <kuhe@users.noreply.github.com>
  • Loading branch information
4 people committed Jan 31, 2024
1 parent d054fe1 commit 04df491
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 3 deletions.
31 changes: 31 additions & 0 deletions packages/s3-request-presigner/src/getSignedUrl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,20 @@ import { RequestPresigningArguments } from "@smithy/types";

import { getSignedUrl } from "./getSignedUrl";

jest.mock("@smithy/middleware-endpoint", () => {
const originalModule = jest.requireActual("@smithy/middleware-endpoint");
return {
...originalModule,
getEndpointFromInstructions: jest.fn(() =>
Promise.resolve({
properties: {
authSchemes: [{ name: "sigv4a", signingRegionSet: ["*"] }],
},
})
),
};
});

describe("getSignedUrl", () => {
const clientParams = {
region: "us-foo-1",
Expand Down Expand Up @@ -141,6 +155,23 @@ describe("getSignedUrl", () => {
expect(mockPresign.mock.calls[0][0].headers[header]).toBeUndefined();
}
);
it("should set region to * when sigv4a is the auth scheme", async () => {
const mockPresigned = "a presigned url";
mockPresign.mockReturnValue(mockPresigned);

const client = new S3Client(clientParams);
const command = new GetObjectCommand({
Bucket: "Bucket",
Key: "Key",
});

await getSignedUrl(client, command);
const presignerArgs = mockPresigner.mock.calls[0][0];
const region = await presignerArgs.region();

expect(region).toBe("*");
expect(mockPresign).toBeCalled();
});

// TODO(endpointsv2) fix this test
it.skip("should presign request with MRAP ARN", async () => {
Expand Down
11 changes: 8 additions & 3 deletions packages/s3-request-presigner/src/getSignedUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,23 @@ export const getSignedUrl = async <
): Promise<string> => {
let s3Presigner: S3RequestPresigner;

let region: string | undefined;
if (typeof client.config.endpointProvider === "function") {
const endpointV2: EndpointV2 = await getEndpointFromInstructions(
command.input as Record<string, unknown>,
command.constructor as EndpointParameterInstructionsSupplier,
client.config
);
const authScheme = endpointV2.properties?.authSchemes?.[0];

if (authScheme?.name === "sigv4a") {
region = authScheme?.signingRegionSet?.join(",");
} else {
region = authScheme?.signingRegion;
}
s3Presigner = new S3RequestPresigner({
...client.config,
signingName: authScheme?.signingName,
region: async () => authScheme?.signingRegion,
region: async () => region,
});
} else {
s3Presigner = new S3RequestPresigner(client.config);
Expand All @@ -58,7 +63,7 @@ export const getSignedUrl = async <
let presigned: IHttpRequest;
const presignerOptions = {
...options,
signingRegion: options.signingRegion ?? context["signing_region"],
signingRegion: options.signingRegion ?? context["signing_region"] ?? region,
signingService: options.signingService ?? context["signing_service"],
};

Expand Down

0 comments on commit 04df491

Please sign in to comment.