Skip to content

Commit

Permalink
fix(s3-request-presigner): skip hoisting SSE headers
Browse files Browse the repository at this point in the history
  • Loading branch information
AllanZhengYP committed Nov 19, 2020
1 parent f430d94 commit cf0590f
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 3 deletions.
17 changes: 15 additions & 2 deletions packages/s3-request-presigner/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ You can get signed URL for other S3 operations too, like `PutObjectCommand`.
`expiresIn` config from the examples above is optional. If not set, it's default
at `900`.

If your request contains server-side encryption(`SSE*`) configurations, because
of S3 limitation, you need to send corresponding headers along with the
presigned url. For more information, please go to [S3 SSE reference](https://docs.aws.amazon.com/AmazonS3/latest/dev/KMSUsingRESTAPI.html)

If you already have a request, you can pre-sign the request following the
section bellow.

Expand All @@ -50,7 +54,7 @@ const signer = new S3Presigner({
credentials: credentialsProvider,
sha256: nodeSha256, //if the signer is used in browser, use `browserSha256` then
});
const url = await signer.presign(request);
const presigned = await signer.presign(request);
```

ES6 Example:
Expand All @@ -64,7 +68,7 @@ const signer = new S3RequestPresigner({
credentials: credentialsProvider,
sha256: nodeSha256, //if the signer is used in browser, use `browserSha256` then
});
const url = await signer.presign(request);
const presigned = await signer.presign(request);
```

To avoid redundant construction parameters when instantiating the s3 presigner,
Expand All @@ -77,3 +81,12 @@ const signer = new S3RequestPresigner({
...s3.config,
});
```

If your request contains server-side encryption(`x-amz-server-side-encryption*`)
headers, because of S3 limitation, you need to send these headers along
with the presigned url. That is to say, the url only from calling `formatUrl()`
to `presigned` is not sufficient to make a request. You need to send the
server-side encryption headers along with url. These headers remain in the
`presigned.headers`

For more information, please go to [S3 SSE reference](https://docs.aws.amazon.com/AmazonS3/latest/dev/KMSUsingRESTAPI.html)
15 changes: 15 additions & 0 deletions packages/s3-request-presigner/src/presigner.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,19 @@ describe("s3 presigner", () => {
[EXPIRES_QUERY_PARAM]: "900",
});
});

it("should disable hoisting server-side-encryption headers to query", async () => {
const signer = new S3RequestPresigner(s3ResolvedConfig);
const signed = await signer.presign({
...minimalRequest,
headers: {
...minimalRequest.headers,
"x-amz-server-side-encryption": "kms",
},
});
expect(signed.headers).toMatchObject({
"x-amz-server-side-encryption": "kms",
});
expect(signed.query?.["X-Amz-SignedHeaders"]).toEqual(expect.stringContaining("x-amz-server-side-encryption"));
});
});
11 changes: 10 additions & 1 deletion packages/s3-request-presigner/src/presigner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,22 @@ export class S3RequestPresigner implements RequestPresigner {

public async presign(
requestToSign: IHttpRequest,
{ unsignableHeaders = new Set(), ...options }: RequestPresigningArguments = {}
{ unsignableHeaders = new Set(), unhoistableHeaders = new Set(), ...options }: RequestPresigningArguments = {}
): Promise<IHttpRequest> {
unsignableHeaders.add("content-type");
// S3 requires SSE headers to be signed in headers instead of query
// See: https://github.com/aws/aws-sdk-js-v3/issues/1576
Object.keys(requestToSign.headers)
.map((header) => header.toLowerCase())
.filter((header) => header.startsWith("x-amz-server-side-encryption"))
.forEach((header) => {
unhoistableHeaders.add(header);
});
requestToSign.headers[SHA256_HEADER] = UNSIGNED_PAYLOAD;
return this.signer.presign(requestToSign, {
expiresIn: 900,
unsignableHeaders,
unhoistableHeaders,
...options,
});
}
Expand Down

0 comments on commit cf0590f

Please sign in to comment.