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

google.auth.transport.Request is called with type(body) == str #318

Closed
Nikratio opened this issue Jan 15, 2019 · 2 comments · Fixed by #421
Closed

google.auth.transport.Request is called with type(body) == str #318

Nikratio opened this issue Jan 15, 2019 · 2 comments · Fixed by #421
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@Nikratio
Copy link

According to eg https://google-auth.readthedocs.io/en/latest/reference/google.auth.html#google.auth.default, classes implementing google.auth.transport.Request will be called with a body parameter of type bytes. This makes sense, because the request agent is in no position to decide how a str body should be encoded for transport.

Unfortunately, in some cases the googl.auth module does pass str value. One such call is made in

body = urllib.parse.urlencode(body)
, but there may be more.

@JustinBeckwith JustinBeckwith added 🚨 This issue needs some love. triage me I really want to be triaged. labels Jan 22, 2019
@busunkim96 busunkim96 added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Mar 27, 2019
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Jul 14, 2019
@SurferJeffAtGoogle
Copy link
Contributor

@Nikratio Thank you for reporting this issue. I apologize for the late reply. I clicked on the link https://google-auth.readthedocs.io/en/latest/reference/google.auth.html#google.auth.default, and I don't see a discussion of a body parameter or bytes.

Is this issue still reproducible today? Is there another doc that describes the conflict between the docs and the code?

@busunkim96
Copy link
Contributor

busunkim96 commented Oct 22, 2019

The link may have changed since the issue was posted:

google.auth.transport.Request documentation says that the body should have type bytes

https://google-auth.readthedocs.io/en/latest/reference/google.auth.transport.html#google.auth.transport.Request
image


Investigation (in progress):
_token_endpoint_request is a private method that makes a request using parameter body

def _token_endpoint_request(request, token_uri, body):
"""Makes a request to the OAuth 2.0 authorization server's token endpoint.
Args:
request (google.auth.transport.Request): A callable used to make
HTTP requests.
token_uri (str): The OAuth 2.0 authorizations server's token endpoint
URI.
body (Mapping[str, str]): The parameters to send in the request body.

All the public methods that call it specify parameter type str for the values that are used to make the body.

def jwt_grant(request, token_uri, assertion):
"""Implements the JWT Profile for OAuth 2.0 Authorization Grants.
For more details, see `rfc7523 section 4`_.
Args:
request (google.auth.transport.Request): A callable used to make
HTTP requests.
token_uri (str): The OAuth 2.0 authorizations server's token endpoint
URI.
assertion (str): The OAuth 2.0 assertion.
Returns:
Tuple[str, Optional[datetime], Mapping[str, str]]: The access token,
expiration, and additional data returned by the token endpoint.
Raises:
google.auth.exceptions.RefreshError: If the token endpoint returned
an error.
.. _rfc7523 section 4: https://tools.ietf.org/html/rfc7523#section-4

google.auth.transport.Request calls requests.Sesion.request.

def __call__(self, url, method='GET', body=None, headers=None,
timeout=None, **kwargs):
"""Make an HTTP request using requests.
Args:
url (str): The URI to be requested.
method (str): The HTTP method to use for the request. Defaults
to 'GET'.
body (bytes): The payload / body in HTTP request.
headers (Mapping[str, str]): Request headers.
timeout (Optional[int]): The number of seconds to wait for a
response from the server. If not specified or if None, the
requests default timeout will be used.
kwargs: Additional arguments passed through to the underlying
requests :meth:`~requests.Session.request` method.
Returns:
google.auth.transport.Response: The HTTP response.
Raises:
google.auth.exceptions.TransportError: If any exception occurred.
"""
try:
_LOGGER.debug('Making request: %s %s', method, url)
response = self.session.request(
method, url, data=body, headers=headers, timeout=timeout,
**kwargs)
return _Response(response)
except requests.exceptions.RequestException as caught_exc:
new_exc = exceptions.TransportError(caught_exc)
six.raise_from(new_exc, caught_exc)

data – (optional) Dictionary, list of tuples, bytes, or file-like object to send in the body of the Request.

busunkim96 added a commit that referenced this issue Jan 10, 2020
#421)

[`google.auth.transport.Request`](https://google-auth.readthedocs.io/en/latest/reference/google.auth.transport.html#google.auth.transport.Request) says that the body should be of type bytes. Some of our code was passing in strings as reported in #318.

In practice this was not causing issues, as the two http transports [requests](https://google-auth.readthedocs.io/en/latest/reference/google.auth.transport.requests.html) and [urllib3](https://google-auth.readthedocs.io/en/latest/reference/google.auth.transport.urllib3.html) are able to handle bodies passed as strings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
5 participants