Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Add Cloudfront package to sign URLs and cookies #3461

Merged
merged 53 commits into from
May 11, 2022
Merged

feat: Add Cloudfront package to sign URLs and cookies #3461

merged 53 commits into from
May 11, 2022

Conversation

keithalpichi
Copy link
Contributor

@keithalpichi keithalpichi commented Mar 22, 2022 •
edited
Loading

Issue

#1822 & #1862

Description

This PR adds a new package called @aws-sdk/cloudfront-sign that exposes two functions (signUrl and signCookies) related to signing Cloudfront URL & cookies. It's implemented as a package, like s3-request-presigner, as it doesn't use any code related to the client-cloudfront client.

Testing

With Jest unit tests. Signatures are created and verified using NodeJS crypto library.

Additional context

There hasn't been much discussion in #1822 for the details of this implementation so I went ahead and took a stab on an initial implementation. I am sure there will be many comments surrounding design and requests for change. Please feel free to let me know what those comments are. I'll get them changed as soon as I can.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@keithalpichi keithalpichi requested a review from a team as a code owner March 22, 2022 03:05
@keithalpichi keithalpichi changed the title Add cloudfront sign pkg Feat: Add Cloudfront sign pkg Mar 22, 2022
@keithalpichi keithalpichi changed the title Feat: Add Cloudfront sign pkg Feat: Add Cloudfront package to sign URLs and cookies Mar 22, 2022
@keithalpichi keithalpichi changed the title Feat: Add Cloudfront package to sign URLs and cookies feat: Add Cloudfront package to sign URLs and cookies Mar 22, 2022
@keithalpichi
Copy link
Contributor Author

I realized the commit message conventions are incorrect. I'll fix these.

@ShivamJoker
Copy link

Great work. I'm hoping someone from AWS will review this soon

@keithalpichi keithalpichi mentioned this pull request Mar 22, 2022
Closed
@kuhe kuhe self-requested a review April 15, 2022 17:27
@kuhe kuhe self-assigned this Apr 15, 2022
@kuhe kuhe added the pr/needs-review This PR needs a review from a Member. label Apr 15, 2022
@kuhe
Copy link
Contributor

kuhe commented Apr 15, 2022 •
edited
Loading

hi @keithalpichi ,

Thanks for submitting this PR. There are some changes I'd like to make before merging the code. Would you like me to add my commits on top of yours, or would you like to receive PR review comments? I will take the former route if you're otherwise occupied.

@keithalpichi
Copy link
Contributor Author

PR review comments will work for me.

kuhe
kuhe reviewed Apr 15, 2022
View reviewed changes
packages/cloudfront-sign/package.json Outdated Show resolved Hide resolved
packages/cloudfront-sign/package.json Outdated Show resolved Hide resolved
packages/cloudfront-sign/package.json Outdated Show resolved Hide resolved
packages/cloudfront-sign/package.json Outdated Show resolved Hide resolved
packages/cloudfront-sign/src/sign.spec.ts Outdated Show resolved Hide resolved
packages/cloudfront-sign/src/sign.ts Outdated Show resolved Hide resolved
packages/cloudfront-sign/src/sign.ts Outdated Show resolved Hide resolved
packages/cloudfront-sign/src/sign.ts Outdated Show resolved Hide resolved
packages/cloudfront-sign/package.json Outdated Show resolved Hide resolved
packages/cloudfront-sign/src/sign.ts Outdated Show resolved Hide resolved
@keithalpichi
Copy link
Contributor Author

Woops. Messed up there. I'll remove these unnecessary commits unrelated to the PR.

@keithalpichi
feat(cloudfront-sign): add package to sign Cloudfront URLs
019fd7a
@keithalpichi
test(cloudfront-sign): add test to validate signed CloudFront URL wit…
6cd19a1 
…h a custom policy
@keithalpichi
test(cloudfront-sign): rename test
7fbda67
@keithalpichi
feat(cloudfront-sign): add function to sign Cloudfront cookies
82bf4cd
@keithalpichi
test(cloudfront-sign): modify tests to use JSON.stringify instead of …
5fea11c 
…hard-coded string
@keithalpichi
test(cloudront-sign): remove duplicated constants
458ac0d
@keithalpichi
test(cloudfront-sign): rename test file
30615c9
@keithalpichi
chore(cloudfront-sign): refactor to reduce code duplication
75a2f7e
@keithalpichi
chore(cloudfront-sign): rename source file
3850f18
@keithalpichi
chore(cloudfront-sign): improve interfaces
ba2f906
@keithalpichi
chore(cloudfront-sign): add url-parser & util-format-url dependencies
3c639e2
@keithalpichi
chore(cloudfront-sign): maintain existing query params when signing url
bf74f26
@keithalpichi
test(cloudfront-sign): add test to validate url path is included in p…
f869645 
…olicy of signed URL
@keithalpichi
test(cloudfront-sign): fix signature verification
96d6c40
@keithalpichi
test(cloudfront-sign): add test to validate signCookies maintains wil…
4b9bbb9 
…dcards in URLs
@keithalpichi keithalpichi requested a review from kuhe April 23, 2022 06:32
@kuhe kuhe linked an issue May 4, 2022 that may be closed by this pull request
Create CloudFront signed URL #1822
Closed
@kuhe
Copy link
Contributor

kuhe commented May 11, 2022 •
edited
Loading

Thanks for the PR updates. I plan to merge this soon. One last thing you could change, or leave to me, is renaming the package again (sorry).

After some discussion about the various signing packages, we realized this package should be called simply cloudfront-signer, which is very close to what you had originally named it.

@keithalpichi
Copy link
Contributor Author

You're welcome. Feel free to go ahead and change the package name. I'm a bit busy the rest of the week.

@kuhe kuhe merged commit f109ed5 into aws:main May 11, 2022
@kuhe kuhe mentioned this pull request May 11, 2022
Merged
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kuhe kuhe kuhe approved these changes

Assignees

@kuhe kuhe

Labels
pr/needs-review This PR needs a review from a Member.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create CloudFront signed URL
3 participants
@keithalpichi @ShivamJoker @kuhe