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

fix: correctly encode bytes for V2 signature #382

Merged
merged 3 commits into from Feb 17, 2021

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 Feb 17, 2021
@tritone tritone requested a review from as a code owner Feb 17, 2021
@google-cla google-cla bot added the cla: yes 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

@tritone tritone Feb 17, 2021

Copy link
Member

@frankyn frankyn left a comment

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
4 checks passed
@tritone tritone deleted the signed-url-fix branch Feb 17, 2021
cojenco added a commit to cojenco/python-storage that referenced this issue 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 added a commit to cojenco/python-storage that referenced this issue 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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants