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

Admin login fails if Google Drive permissions have been granted for the same Google user #6788

Open
robertknight opened this issue Oct 18, 2024 · 3 comments

Comments

@robertknight
Copy link
Member

robertknight commented Oct 18, 2024

Google login to the admin pages at https://lms.staging.hypothes.is/admin breaks if the same Google account used for the login has previously been used to authorize Google Drive access as part of creating an assignment.

Steps to reproduce:

  1. Create a Google Drive PDF assignment using the Hypothesis LMS staging app. Grant Google Drive access as part of this flow.
  2. Go to https://lms.staging.hypothes.is/admin and attempt to login with the same Google account

Expected: Login succeeds
Admin: Login fails with 500 error

In this scenario, visiting https://lms.staging.hypothes.is/admin redirects to a Google login page with a URL that includes openid, userinfo.email and userinfo.profile scopes. After approving access, Google redirects back to the LMS app at https://lms.staging.hypothes.is/googleauth/login/callback with an authorization code. In the redirect URL the scopes include the requested ones but also have the existing Google Drive scope added.

The presence of these existing extra permissions causes an exception on our end (Sentry).

A workaround is to go to your Google account settings and revoke Google Drive permissions for the Hypothesis LMS app under security settings.

@robertknight
Copy link
Member Author

The Warning exception is being raised by oauthlib, which in turn is called by pyramid_googleauth. See https://github.com/oauthlib/oauthlib/blob/bf75322aad61785501405308f3c4cd020983d17d/oauthlib/oauth2/rfc6749/parameters.py#L463.

@robertknight
Copy link
Member Author

robertknight commented Oct 18, 2024

oauthlib throws an exception if the OAuth callback URL includes different scopes than what were requested, even if the returned scopes are a superset of the requested scopes. The warning can be globally disabled by setting the OAUTHLIB_RELAX_TOKEN_SCOPE environment variable. See oauthlib/oauthlib#562. Another workaround documented in oauthlib/oauthlib@ca4811b is to catch the Warning exception and extract the OAuth2Token object from in, if the revised scopes are acceptable.

@robertknight
Copy link
Member Author

Catching the Warning exception thrown by oauthlib is problematic because we're not directly calling the library that is throwing it. The call chain looks like:

lms -> pyramid_googleauth -> google_auth_oauthlib -> requests_oauthlib -> oauthlib

Here requests_oauthlib.OAuth2Session has a fetch_token method that prepares a request for the authorization token, fetches it, validates the response, saves the token in the OAuth2Session and returns it. The Warning exception is thrown during validation, before the token has been saved on the session. If we catch the token at this point, it won't be saved on the session which could lead to unexpected behavior later. See also requests/requests-oauthlib#507 which is a different issue about scopes changing.

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

No branches or pull requests

1 participant