Storage: add support for V4 signed URLs #7460

tseaver · 2019-02-26T22:50:34Z

Partial support for the new feature: adds a version argument to Blob.generate_signed_url, and uses it to switch between the two signing algorithms.

Remaining work:

Improve assertions for how variations in calling are handled (different methods, headers, etc.) See the data-driven tests in Data-driven V4 URL signing tests, and implementation changes google-cloud-dotnet#2882 for examples (even better, work out how to share them like the Firestore conformance tests).
Add system tests for V4 blob signed URLs.
Add Bucket.generate_signed_url, to permit listing bucket contents.
Add system test for resumable upload using V4 blob signed URL.

Deferred work:

Add system tests for V4 bucket signed URLs (in place, but currently skipped due to back-end case-flattening issue for the X-Goog-Signed header) (Storage: un-skip system tests for 'Bucket.generate_signed_url' #7625).
Add support for signing URLs on GCE following Frank's Ruby PoC. (Storage: add support for signing URLs using IAM / tokens. #7627)
Add system tests for CSEK upload / download using V4 blob-signed URLs (blocked by a similar back-end bug as above). (Storage: add systests for V4 signed URLs with CSEK headers #7626).

No longer in scope:

~~Add Client.generate_signed_url, to permit listing buckets.~~

tseaver · 2019-03-04T22:24:28Z

The docs build failure on CI is unrelated to this PR: it was fixed in #7458.

frankyn

Doing a first pass.

frankyn · 2019-03-05T19:20:00Z

storage/google/cloud/storage/_signing.py

+    .. note::
+
+        If you are on Google Compute Engine, you can't generate a signed URL.
+        Follow `Issue 922`_ for updates on this. If you'd like to be able to


@tseaver this is supported in the Python auth library. Is this a typo?

Ref: https://github.com/googleapis/google-auth-library-python/blob/edfe24602051969e32917e82bcedd2bace43e260/google/auth/iam.py#L36

@frankyn googleapis/google-auth-library-python#50 is still open: AFAIK, the only way to get signing done on GCE is to use the IAM-based workaround. I plan to add that here (following your Ruby PoC) once we're OK with the surface.

storage/google/cloud/storage/_signing.py

tseaver · 2019-03-11T19:08:17Z

@frankyn A question: the current Python V2 version allows for passing method="RESUMABLE", and converts that to "POST" and mutates the resource per https://cloud.google.com/storage/docs/access-control/signed-urls#signing-resumable. Should V4 follow suit?

frankyn · 2019-03-12T01:08:11Z

At a blob level, yes. The resumable header is also supported when using V4 signed URLs.

One request I have is to state in the documentation that the HeaderName:HeaderValue is expected in a follow-up request. At the moment only the name is documented.

tseaver · 2019-03-12T17:52:51Z

Flaky spanner systest reported in #7504.

tseaver · 2019-03-12T19:00:05Z

@crwilcox Can you check what is hanging the Kokoro builds here?

frankyn · 2019-03-12T19:56:41Z

@tseaver please do not support signed URLs at a client level.

storage/google/cloud/storage/_signing.py

crwilcox · 2019-03-28T22:34:29Z

@tseaver do you need anything from me to move this along?

frankyn

Python doesn't have the timestamp offset bug mentioned in email.

tseaver · 2019-04-01T02:07:16Z

@frankyn As of 0d54faa, this PR is passing all the cross-language conformance tests from the dotnet repository. I have one system test passing as well (simple Blob GET). 734a9b3 adds V4 systests paralleling the existing V2 ones.

tseaver · 2019-04-01T02:46:42Z

@frankyn 68742be adds Bucket.generate_signed_url (with support for both V2 and V4). 6c5cfc7 adds systests for that feature.

tseaver · 2019-04-01T02:50:47Z

@frankyn I believe the only remaining work is to add support for IAM / access token signing: note that the feature does not exist yet for V2 in Python.

frankyn · 2019-04-01T03:25:04Z

storage/google/cloud/storage/blob.py

@@ -387,16 +432,25 @@ def generate_signed_url(
            client = self._require_client(client)
            credentials = client._credentials

-        return generate_signed_url(
+        if version == "v2":


@tseaver could you add a warning as log output that V2 will change to V4 in the future:

You have generated a signed URL using the default v2 signing implementation. In the future, this will default to v4. You may experience breaking changes if you use longer than 7 day expiration times with v4. To opt-in to the behavior specify version="v2".

tseaver · 2019-04-01T07:01:30Z

@frankyn 2a956e5 adds the systest for signed resumable upload URLs (both V2 and V4), including round-tripping.

Exclusive with 'expiration', to permit clearer semantics for users.

Headers have to be sorted *after* lower-casing the keys.

In addition to stripping leading / trailing spaces, collapse multiple interior spaces into one.

tseaver · 2019-04-17T20:39:04Z

@frankyn PTAL.

frankyn

Two nits and LGTM.
Thanks @tseaver!

storage/google/cloud/storage/blob.py

storage/google/cloud/storage/bucket.py

storage/tests/system.py

frankyn

LGTM, pending tests. Thanks @tseaver!

tseaver added the api: storage Issues related to the Cloud Storage API. label Feb 26, 2019

tseaver requested a review from frankyn February 26, 2019 22:50

tseaver requested a review from crwilcox as a code owner February 26, 2019 22:50

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 26, 2019

tseaver added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Feb 26, 2019

frankyn suggested changes Mar 5, 2019

View reviewed changes

yoshi-automation added the 🚨 This issue needs some love. label Mar 5, 2019

frankyn added needs work This is a pull request that needs a little love. and removed 🚨 This issue needs some love. labels Mar 12, 2019

frankyn reviewed Mar 18, 2019

View reviewed changes

storage/google/cloud/storage/_signing.py Show resolved Hide resolved

frankyn reviewed Mar 29, 2019

View reviewed changes

tseaver removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. needs work This is a pull request that needs a little love. labels Apr 1, 2019

tseaver changed the title ~~[WIP] Storage: add support for V4 signed URLs~~ Storage: add support for V4 signed URLs Apr 1, 2019

yoshi-automation added the 🚨 This issue needs some love. label Apr 1, 2019

frankyn reviewed Apr 1, 2019

View reviewed changes

This was referenced Apr 1, 2019

Storage: un-skip system tests for 'Bucket.generate_signed_url' #7625

Closed

Storage: add systests for V4 signed URLs with CSEK headers #7626

Closed

Storage: add support for signing URLs using IAM / tokens. #7627

Closed

tseaver added 19 commits April 17, 2019 15:31

Prepare signed URL systests for testing both V2 and V4.

943e273

Add 'max_age' argument to 'Blob.generate_signed_url'.

0c9c812

Exclusive with 'expiration', to permit clearer semantics for users.

Add 'Host' header to signed headers, if not passed.

a78605d

Add support for system tests using 'expiration' and 'max_age'.

5a6d44c

Add partial V4 conformance test suite and bash to fit.

9833d68

Add support for bucket-level conformance tests.

c860d51

Add 'Simple headers' conformance test and fix.

87aa6b6

Headers have to be sorted *after* lower-casing the keys.

Add conformance tests for collapsed header values and fix.

e08ad30

In addition to stripping leading / trailing spaces, collapse multiple interior spaces into one.

Lint.

db05933

Add V4 systests paralleling existing V2 tests.

dada493

Add 'Bucket.generate_signed_url' method.

de892c6

Add systests for listing bucket blobs (V2/V4).

eae12c5

Add warning for users signing URLs without passing an explicit version.

dc7bd12

Clean up V2 signing using 'canonicalize' better.

c52e2db

Tidy up V4 signing using 'get_canonical_headers'.

2c5759e

Handle 'RESUMABLE' method alias for V4 signing.

7617b4f

Add round-trip systest for signed resumable uploads.

d33c68e

Remove 'max_age' argument for signing.

c37e77a

Remove warning when no version is passed to 'generate_signed_url'.

2d04844

frankyn suggested changes Apr 17, 2019

View reviewed changes

storage/google/cloud/storage/blob.py Outdated Show resolved Hide resolved

storage/google/cloud/storage/bucket.py Outdated Show resolved Hide resolved

Remove warning message template.

ccccaed

frankyn reviewed Apr 17, 2019

View reviewed changes

storage/tests/system.py Outdated Show resolved Hide resolved

tseaver added 2 commits April 17, 2019 18:16

Remove no-version warning for 'Bucket.generate_signed_url', too.

3a583a4

Un-skip systests blocked on back-end case-flattening bug (now fixed).

8f2e13f

frankyn approved these changes Apr 17, 2019

View reviewed changes

tseaver merged commit 7527784 into googleapis:master Apr 17, 2019

tseaver deleted the storage-v4_signing-wip branch April 17, 2019 22:34

martey mentioned this pull request Apr 20, 2019

Allow custom endpoints with Google Cloud Storage. jschneier/django-storages#648

Merged

tseaver mentioned this pull request Oct 22, 2019

Cloud storage signing urls #5300

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Storage: add support for V4 signed URLs #7460

Storage: add support for V4 signed URLs #7460

tseaver commented Feb 26, 2019 •

edited

Loading

tseaver commented Mar 4, 2019

frankyn left a comment

frankyn Mar 5, 2019

tseaver Mar 11, 2019

tseaver commented Mar 11, 2019

frankyn commented Mar 12, 2019 •

edited

Loading

tseaver commented Mar 12, 2019

tseaver commented Mar 12, 2019

frankyn commented Mar 12, 2019

crwilcox commented Mar 28, 2019

frankyn left a comment

tseaver commented Apr 1, 2019

tseaver commented Apr 1, 2019

tseaver commented Apr 1, 2019

frankyn Apr 1, 2019

tseaver Apr 1, 2019

tseaver commented Apr 1, 2019

tseaver commented Apr 17, 2019

frankyn left a comment

frankyn left a comment

Storage: add support for V4 signed URLs #7460

Storage: add support for V4 signed URLs #7460

Conversation

tseaver commented Feb 26, 2019 • edited Loading

tseaver commented Mar 4, 2019

frankyn left a comment

Choose a reason for hiding this comment

frankyn Mar 5, 2019

Choose a reason for hiding this comment

tseaver Mar 11, 2019

Choose a reason for hiding this comment

tseaver commented Mar 11, 2019

frankyn commented Mar 12, 2019 • edited Loading

tseaver commented Mar 12, 2019

tseaver commented Mar 12, 2019

frankyn commented Mar 12, 2019

crwilcox commented Mar 28, 2019

frankyn left a comment

Choose a reason for hiding this comment

tseaver commented Apr 1, 2019

tseaver commented Apr 1, 2019

tseaver commented Apr 1, 2019

frankyn Apr 1, 2019

Choose a reason for hiding this comment

tseaver Apr 1, 2019

Choose a reason for hiding this comment

tseaver commented Apr 1, 2019

tseaver commented Apr 17, 2019

frankyn left a comment

Choose a reason for hiding this comment

frankyn left a comment

Choose a reason for hiding this comment

tseaver commented Feb 26, 2019 •

edited

Loading

frankyn commented Mar 12, 2019 •

edited

Loading