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

handle token expire #4

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

handle token expire #4

wants to merge 2 commits into from

Conversation

BlitzJB
Copy link

@BlitzJB BlitzJB commented Aug 3, 2022

No description provided.

@BlitzJB
Copy link
Author

BlitzJB commented Aug 3, 2022

I have done two things here:

  • Update google scopes
  • Handle for token expiration error

Only these scopes seem to work for me, while some have said the old scopes do work for them, this could just be google preserving backward compatibility. Also, setting OAUTHLIB_RELAX_TOKEN_SCOPE = True doesn't fix this

Token expiration left me confused for quite a while when I initially faced it, as it was something I expected flask-dance to handle out of the box, Adding this into the example might save a lot of people from puzzles

Kindly do let me know if I could make this PR more helpful. This is literally my first PR into an open-source project ❤️

@BlitzJB
Copy link
Author

BlitzJB commented Aug 4, 2022

@singingwolfboy Could you take a look at this?

@singingwolfboy
Copy link
Owner

Kindly do let me know if I could make this PR more helpful. This is literally my first PR into an open-source project ❤️

First of all, thank you for the pull request, and welcome to the world of open source! Sorry it took me a few days to get to it; I do have other things going on in my life as well. 🙂

I have done two things here:

  • Update google scopes
  • Handle for token expiration error

Usually, it's a good idea to have one pull request per logical change. By "logical change", I mean an idea that should be discussed and reviewed separately. Smaller pull requests are easier to understand and review, and by splitting PRs like this, they can usually get reviewed and merged more quickly.

Only these scopes seem to work for me, while some have said the old scopes do work for them, this could just be google preserving backward compatibility. Also, setting OAUTHLIB_RELAX_TOKEN_SCOPE = True doesn't fix this

Interesting. What error message are you getting with the existing scopes? I haven't manually tested this code in awhile, and it's entirely possible that Google has changed their mind about what scopes they accept. If that's the case, then we should definitely update these scopes!

Token expiration left me confused for quite a while when I initially faced it, as it was something I expected flask-dance to handle out of the box, Adding this into the example might save a lot of people from puzzles

Flask-Dance has the capability to handle token expiration out of the box, and in fact it should do that for the Google configuration -- if you set offline=True. Google's documentation of this is really bad, but you can check the access_type value in this table. Essentially, the default access type is "online", meaning that the user needs to be present for refreshing OAuth tokens. If we use the "offline" access type, then Google passes a refresh token as well, so the access token can be refreshed without the user.

For this example repo, I really want the code to be as short and simple as possible: truly a minimalist example of how to make Flask-Dance work with Google. That means I don't want to add the try/except code, because the code works without it. However, I would gladly accept a change to the README or some other kind of documentation that explains this offline=True parameter, and when and why you would want to use it for token expiration.

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

Successfully merging this pull request may close these issues.

None yet

2 participants