Skip to content

Auth call timeout option #535

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

Closed
gzamb opened this issue Mar 10, 2021 · 13 comments · Fixed by #538
Closed

Auth call timeout option #535

gzamb opened this issue Mar 10, 2021 · 13 comments · Fixed by #538

Comments

@gzamb
Copy link

gzamb commented Mar 10, 2021

Using firebase-admin sdk v4.0.0 for python3.8.
We have recently noticed that one of our time sensitive services is struggling due to long calls on auth.verify_id_token function. We'd like the option to have a timeout for the actual request call that happens here, since it does appear that the google-auth library allows you to set a timeout to the request. We do use the httpTimeout on initialization of the app already but it appears this config option is not used at all when making the request.

@google-oss-bot
Copy link

I found a few problems with this issue:

  • I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.
  • This issue does not seem to follow the issue template. Make sure you provide all the required information.

@hiranya911
Copy link
Contributor

We don't make any direct HTTP requests in verify_id_token(). We simply call google.oauth2.id_token.verify_token(). It is that library that makes the request to fetch certificates.

We do pass in a request object (which wraps a Session instance) when making the above call, but unfortunately the requests.Session API doesn't support injecting a timeout as far as I can tell. We will probably have to implement a workaround.

@daniellehanks
Copy link
Contributor

It looks like google.oauth2.id_token.verify_token() just takes a generic google.auth.transport.Request interface. Swapping out requests for urllib3, which does support setting a global timeout on its connection pools could be another option?

@daniellehanks
Copy link
Contributor

Additionally, it looks like this call (made when check_revoked=True) does not pass a timeout either?

@hiranya911
Copy link
Contributor

Swapping out requests for urllib3, which does support setting a global timeout on its connection pools could be another option

I'd rather implement one of the workarounds (e.g. subclassing the requests.Session) than taking a direct dependency on urllib3.

Additionally, it looks like this call (made when check_revoked=True) does not pass a timeout either?

That call goes through our own _http_client.HttpClient which sets a default timeout on all requests.

DEFAULT_TIMEOUT_SECONDS = 120

def request(self, method, url, **kwargs):
"""Makes an HTTP call using the Python requests library.
This is the sole entry point to the requests library. All other helper methods in this
class call this method to send HTTP requests out. Refer to
http://docs.python-requests.org/en/master/api/ for more information on supported options
and features.
Args:
method: HTTP method name as a string (e.g. get, post).
url: URL of the remote endpoint.
kwargs: An additional set of keyword arguments to be passed into the requests API
(e.g. json, params, timeout).
Returns:
Response: An HTTP response object.
Raises:
RequestException: Any requests exceptions encountered while making the HTTP call.
"""
if 'timeout' not in kwargs:
kwargs['timeout'] = self.timeout
resp = self._session.request(method, self.base_url + url, **kwargs)
resp.raise_for_status()
return resp

But the entire auth module doesn't honor the httpTimeout app option as of today, if that's what you mean. But that's easy enough to fix.

@daniellehanks
Copy link
Contributor

But the entire auth module doesn't honor the httpTimeout app option as of today, if that's what you mean. But that's easy enough to fix.

Yes, that's exactly what I meant. It is that call that keeps hanging on the service we are having problems with. We have had to temporarily disable check_revoked.

@hiranya911
Copy link
Contributor

Ok, thanks for clarifying. I can hopefully find some time to fix that next week. You can also help speed things along by sending a PR.

@daniellehanks
Copy link
Contributor

@hiranya911 I think I've got a fix for the simpler part (respecting app options httpTimeout in UserManager), but I'm having trouble pushing. Last I committed to a google-owned repo (python spanner), the workflow was to fork the repo. There aren't specific instructions covering this in the contributor guide. This is the error I'm getting:

(py3.8clean)  (535/auth-timeout) firebase-admin-python> git push --set-upstream origin 535/auth-timeout
remote: Permission to firebase/firebase-admin-python.git denied to daniellehanks.
fatal: unable to access 'https://github.com/firebase/firebase-admin-python.git/': The requested URL returned error: 403

If you could help me get this resolved, I will happily submit a PR.

@hiranya911
Copy link
Contributor

@daniellehanks you need to fork our repo and submit a PR through that.

@daniellehanks
Copy link
Contributor

@daniellehanks you need to fork our repo and submit a PR through that.

It might be good to put that in the contributor guide.

@hiranya911
Copy link
Contributor

This fix was included in the last release.

@daniellehanks
Copy link
Contributor

Thanks for the swift release. We've just updated 3 of our services in production. My PR though only added a timeout on one of the two outgoing network calls. There still needs to be timeout instrumentation on the transport given to google.oauth2.id_token.verify_token(). Can we reopen this ticket to reflect that work @hiranya911 ?

@daniellehanks
Copy link
Contributor

Thanks for getting the other half patched @hiranya911!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants