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

fix: try refreshing cookie on multiple cases #683

Merged
merged 3 commits into from
Aug 18, 2021

Conversation

StephaneRob
Copy link
Contributor

Currently we try to refresh cookie only when the access token is found and is expired.
If the refresh cookie is present we should try refreshing session with it even if invalid or not found.

@StephaneRob StephaneRob requested a review from a team as a code owner August 17, 2021 14:02
@yordis
Copy link
Member

yordis commented Aug 17, 2021

Would you mind adding an entry in the changelog https://github.com/ueberauth/guardian/blob/master/CHANGELOG.md

Copy link
Member

@yordis yordis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some entry in the changelog so we keep the PR ready to be release

@StephaneRob
Copy link
Contributor Author

Hi @yordis, I added some test cases and updated the CHANGELOG.
The changelog doesn't mention the neither refresh_from_cookie option nor VerifyCookie plug deprecation. Do you want I add it under 2.2.0 version?

@yordis
Copy link
Member

yordis commented Aug 18, 2021

@StephaneRob please if you don't mind I would appreciate updating the CHANGELOG for sure

@StephaneRob
Copy link
Contributor Author

@yordis done.

@yordis
Copy link
Member

yordis commented Aug 18, 2021

You are the best! Thank you so much!

@yordis yordis merged commit 9baef5b into ueberauth:master Aug 18, 2021
@StephaneRob StephaneRob deleted the fix-refresh-cookie branch August 18, 2021 06:53
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.

2 participants