-
Notifications
You must be signed in to change notification settings - Fork 95
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
Redirect to login page in case token is expired instead of showing an error page #395
Redirect to login page in case token is expired instead of showing an error page #395
Conversation
Instead of showing an error page
There are a lot of test failures. I will check them |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #395 +/- ##
============================================
+ Coverage 71.88% 72.72% +0.84%
- Complexity 239 245 +6
============================================
Files 12 12
Lines 1010 1023 +13
Branches 145 148 +3
============================================
+ Hits 726 744 +18
+ Misses 206 199 -7
- Partials 78 80 +2 ☔ View full report in Codecov by Sentry. |
is this behaving differently to Okta? I tried to find some docs from Amazon on using Cognito as an OP but failed. |
Dunno if your statement is for me. AFAIK, Cognito's Refresh Token will expire after a configurable amount of time. Then, the ID Token can't be refreshed anymore. This is great. This PR is just about redirecting the user to the login page in case that the user need to re-login instead of showing a plain error page. |
@jtnord @michael-doubez Anything left I should do so that this PR could get merged? |
I was being selfish and wanted to land #409 before adapting other changes (rather than the other way around). If you are ok waiting then I will rebase this after #409 lands, otherwise please tell me so |
Yes, understandable. I would say: it depends on the waiting timespan 😄 - I you land in less than 1 week, I would say: sure. If it takes (unpredictable) longer, I would appreciate it if you could release it first. If you don't mind being honest 😅 |
No worries, I am going on holiday, so this will be merged this week whichever way it goes. |
Instead of showing an error page, the user should be redirected to the login page.
fixes: #396
Testing done
I have tested this with 4.340.ve70636c6590e and by setting the following
(Cognito) Refresh Configuration:
Refresh token expiration: 60 minutes
Access token expiration: 10 minutes
ID token expiration: 10 minutes
After 60 minutes, I got redirected to the login page instead of getting presented an error page.
Submitter checklist