Skip to content

Conversation

tritone
Copy link
Contributor

@tritone tritone commented Feb 17, 2021

V2 signature was passing a string to the sign_bytes function
instead of bytes. This works fine for most credentials (since
their sign_bytes implementations accept strings) but not for
impersonated credentials. V4 signature encodes the string before
calling sign_bytes, so I do the same here.

We should also look into clarifying the contract for the
sign_bytes interface in the auth library.

Fixes #373

V2 signature was passing a string to the sign_bytes function
instead of bytes. This works fine for most credentials (since
their sign_bytes implementations accept strings) but not for
impersonated credentials. V4 signature encodes the string before
calling sign_bytes, so I do the same here.

We should also look into clarifying the contract for the
sign_bytes interface in the auth library.

Fixes googleapis#373
@tritone tritone requested a review from a team February 17, 2021 03:30
@tritone tritone requested a review from a team as a code owner February 17, 2021 03:30
@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/python-storage API. label Feb 17, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 17, 2021
@@ -77,7 +77,7 @@ def get_signed_query_params_v2(credentials, expiration, string_to_sign):
signed payload.
"""
ensure_signed_credentials(credentials)
signature_bytes = credentials.sign_bytes(string_to_sign)
signature_bytes = credentials.sign_bytes(string_to_sign.encode("ascii"))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@frankyn frankyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change LGTM.

Opnion: Issue here is that we have multiple sign versions and we focused on v4 more recently.

@tritone tritone merged commit f44212b into googleapis:master Feb 17, 2021
@tritone tritone deleted the signed-url-fix branch February 17, 2021 19:01
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
* fix: correctly encode bytes for V2 signature

V2 signature was passing a string to the sign_bytes function
instead of bytes. This works fine for most credentials (since
their sign_bytes implementations accept strings) but not for
impersonated credentials. V4 signature encodes the string before
calling sign_bytes, so I do the same here.

We should also look into clarifying the contract for the
sign_bytes interface in the auth library.

Fixes googleapis#373

* fix py2 failure
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
* fix: correctly encode bytes for V2 signature

V2 signature was passing a string to the sign_bytes function
instead of bytes. This works fine for most credentials (since
their sign_bytes implementations accept strings) but not for
impersonated credentials. V4 signature encodes the string before
calling sign_bytes, so I do the same here.

We should also look into clarifying the contract for the
sign_bytes interface in the auth library.

Fixes googleapis#373

* fix py2 failure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/python-storage API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It's impossible to generate a signed url (v2) with impersonated Credentials
2 participants