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.

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

feat(v4): support v4 signed URL for GA #1072

Merged
merged 28 commits into from
Mar 5, 2020
Merged

feat(v4): support v4 signed URL for GA #1072

merged 28 commits into from
Mar 5, 2020

Conversation

jkwlui
Copy link
Member

@jkwlui jkwlui commented Feb 12, 2020

Closes #1043
Closes #1044
Closes #1045
Closes #1046

jkwlui and others added 3 commits February 10, 2020 14:07
* refactor signed url logic into its own class

* fix the test

* refactor tests to test/signer.ts

* npm run fix

* chore: generate synth.metadata

* allow bucket-operation signed url

* remove unused types

* support bucket operations

* refactor signer to not use promisify

* fix node-8 test

* add test for generation parameter

* test: add list bucket signed url system-test

* npm run fix

* fix bucket getSignedUrl docs

* chore(deps): update dependency @types/mocha to v7

* fix: unhandled promise rejection warning in samples (#1056)

* fix: unhandledPromiseRejectionWarning in samples

* fix: lint issue

* fix: lint issue

Co-authored-by: Benjamin E. Coe <[email protected]>

* docs(readme): log errors in sample

* chore: skip img.shields.io in docs test

Co-authored-by: Yoshi Automation Bot <[email protected]>
Co-authored-by: WhiteSource Renovate <[email protected]>
Co-authored-by: Lalji Kanjareeya <[email protected]>
Co-authored-by: Benjamin E. Coe <[email protected]>
Co-authored-by: Justin Beckwith <[email protected]>
…134282 (#1066)

* update conformance test json to conformance-test@9f134282

* parse updated conformance-test format

* npm run fix

* pass queryParameters to conformance test

* conformance tests ignore query parameter ordering in URL

* npm run fix

* parse sha256 option

* revert debug
* update conformance test json to conformance-test@9f134282

* parse updated conformance-test format

* npm run fix

* fix(signed-url): trailing `/` after path before query params
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to determine that you authored the commits in this PR. Maybe you used a different email address in the git commits than was used to sign the CLA? If someone else authored these commits, then please add them to this pull request and have them confirm that they're okay with them being contributed to Google. If there are co-authors, make sure they're formatted properly.

In order to pass this check, please resolve this problem and then comment@googlebot I fixed it... If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Feb 12, 2020
@jkwlui jkwlui changed the title feat(v4): GA master branch feat(v4): support v4 signed URL for GA Feb 12, 2020
@jkwlui jkwlui added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Feb 12, 2020
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to determine that you authored the commits in this PR. Maybe you used a different email address in the git commits than was used to sign the CLA? If someone else authored these commits, then please add them to this pull request and have them confirm that they're okay with them being contributed to Google. If there are co-authors, make sure they're formatted properly.

In order to pass this check, please resolve this problem and then comment@googlebot I fixed it... If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Feb 12, 2020
@jkwlui jkwlui added do not merge Indicates a pull request not ready for merge, due to either quality or timing. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Feb 12, 2020
@jkwlui jkwlui self-assigned this Feb 12, 2020
* update conformance test json to conformance-test@9f134282

* parse updated conformance-test format

* npm run fix

* pass queryParameters to conformance test

* conformance tests ignore query parameter ordering in URL

* npm run fix

* parse sha256 option

* revert debug

* fix(signed-url): include custom query parameters in signature calculation

* query param wip

* fix queryParam sorting

* test: query params are sorted

* remove unused imports

* npm run fix

* refactor

* rename QueryParams to just Query
@codecov
Copy link

codecov bot commented Feb 12, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@6720d53). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1072   +/-   ##
=========================================
  Coverage          ?   98.97%           
=========================================
  Files             ?       12           
  Lines             ?    10991           
  Branches          ?      496           
=========================================
  Hits              ?    10878           
  Misses            ?      111           
  Partials          ?        2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6720d53...d7bd23e. Read the comment docs.

* update conformance test json to conformance-test@9f134282

* parse updated conformance-test format

* npm run fix

* vhost

* unit tests

* npm run fix

* docs: document urlStyle option for Bucket

* test: add system-test to exercise virtual-hosted url

* pass queryParameters to conformance test

* conformance tests ignore query parameter ordering in URL

* npm run fix

* parse sha256 option

* revert debug

* restyle option as virtualHostedStyle as a boolean

* npm run fix

* docs: virtualHostedStyle option defaults to false

* docs: fix path style example
* feat(signed-url): support SHA256 for signed payloads

* add unit test

* remove junk import

* newline

* document contentMd5 option

* do not provide contentSha256 option, instead, detect user provided header
@jkwlui jkwlui added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Feb 14, 2020
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to determine that you authored the commits in this PR. Maybe you used a different email address in the git commits than was used to sign the CLA? If someone else authored these commits, then please add them to this pull request and have them confirm that they're okay with them being contributed to Google. If there are co-authors, make sure they're formatted properly.

In order to pass this check, please resolve this problem and then comment@googlebot I fixed it... If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Mar 2, 2020
@jkwlui jkwlui added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Mar 2, 2020
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to determine that you authored the commits in this PR. Maybe you used a different email address in the git commits than was used to sign the CLA? If someone else authored these commits, then please add them to this pull request and have them confirm that they're okay with them being contributed to Google. If there are co-authors, make sure they're formatted properly.

In order to pass this check, please resolve this problem and then comment@googlebot I fixed it... If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Mar 2, 2020
@jkwlui jkwlui added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Mar 2, 2020
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@googleapis googleapis deleted a comment from googlebot Mar 3, 2020
@googleapis googleapis deleted a comment from googlebot Mar 3, 2020
@googleapis googleapis deleted a comment from googlebot Mar 3, 2020
@googleapis googleapis deleted a comment from googlebot Mar 3, 2020
@jkwlui
Copy link
Member Author

jkwlui commented Mar 3, 2020

@stephenplusplus Thank you for your review!

I addressed your feedback, please take a look when you have a moment.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to determine that you authored the commits in this PR. Maybe you used a different email address in the git commits than was used to sign the CLA? If someone else authored these commits, then please add them to this pull request and have them confirm that they're okay with them being contributed to Google. If there are co-authors, make sure they're formatted properly.

In order to pass this check, please resolve this problem and then comment@googlebot I fixed it... If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Mar 3, 2020
@jkwlui jkwlui added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Mar 4, 2020
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@stephenplusplus
Copy link
Contributor

Thank you very much, looks great to me!

@jkwlui jkwlui merged commit 79d7b8f into master Mar 5, 2020
@jkwlui jkwlui deleted the v4-ga branch March 5, 2020 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
3 participants