search

googleapis / google-auth-library-python

Google Auth Python Library
https://googleapis.dev/python/google-auth/latest/
Apache License 2.0
777 stars 307 forks source link

Fix: Less restrictive content-type header check for google authentication (ignores charset) #1382

Closed DanieleQuasimodo closed 1 year ago

DanieleQuasimodo commented 1 year ago

Less restrictive content-type header check for google authentication (now only the media-type is compared, ignoring charset, boundary, etc). Should fix issue #1089.

Implemented as suggested in PEP-594 (due to deprecation of cgi.parse_header. See https://peps.python.org/pep-0594/#cgi).

google-cla[bot] commented 1 year ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

arithmetic1728 commented 1 year ago

@DanieleQuasimodo thank you for your PR. Could you add some unit tests to cover your change (for example, https://github.com/googleapis/google-auth-library-python/blob/main/tests/test__helpers.py and https://github.com/googleapis/google-auth-library-python/blob/main/tests/compute_engine/test__metadata.py).

DanieleQuasimodo commented 1 year ago

@DanieleQuasimodo thank you for your PR. Could you add some unit tests to cover your change (for example, https://github.com/googleapis/google-auth-library-python/blob/main/tests/test__helpers.py and https://github.com/googleapis/google-auth-library-python/blob/main/tests/compute_engine/test__metadata.py).

Sure, no problem! Yesterday I was a bit rushed and completely forgot about it. I'll get some tests written as soon as I find a moment.

DanieleQuasimodo commented 1 year ago

These should do. The _metadata test is probably a bit redundant (_metadata inner workings haven't really changed except for invoking the new function, which is already tested in the _helpers tests), but I still added it, just in case.

The forced push to my fork was just because I'm dumb, committed with the wrong account and had to ammend the author (so Google CLA is happy) 😅

arithmetic1728 commented 1 year ago

@DanieleQuasimodo The PR looks great, thanks! Could you run python -m nox -s lint to format the files (this caused kokoro build failure)? After that I will go ahead to merge the PR.

DanieleQuasimodo commented 1 year ago

@arithmetic1728 I think now it should be ok. I ran both blacken and lint