Set default timeout for Google OAuth to prevent 503s
What does this MR do?
Fixes #198056 (closed)
The Google OAuth callback /users/auth/google_oauth2/callback sometimes exceeds 60s worker timeout.
This change sets the timeout for Google OAuth2 calls to remaining_seconds which takes into account how much time has already passed relative to the configured max_request_duration_seconds per @reprazent's suggestion in https://gitlab.com/reprazent.
By setting timeout for the OAuth::Client we are affecting any and all timeouts - open timeout, read timeout, etc. They can be set individually, but if timeout is set and the more specific timeouts are not, timeout is used.
Since we're setting the generic timeout it is still conceivable that both the open and read portion of the request are slow, but below the timeout and collectively above the Rails (Puma/Unicorn) timeout. This could still result in a 503. Do we want to somehow ensure that the collective timeouts are below the worker timeout?
We also have an internal use of OAuth2::Client for GKE which should also use the same timeout, since we use the google_oauth2 configuration for that client, too. For that I rescue the errors and display a flash alert.
NOTE: This does include a monkey patch for OmniAuth::OAuth2
Until/unless https://github.com/omniauth/omniauth-oauth2/pull/129 is merged and released. I am hoping the project is still active enough that this will be merged but it may be months until a release based on history. It would be ideal if we didn't have to wait, and if we avoid forking the gem just yet. I'm open to discussion on this, though.
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec -
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team