User should be redirect to 'loginRedirect' if revisiting page when refresh token is valid and access is expired
reft opened this issue · 13 comments
Access token exp - 1 hour
Refresh token exp - 8 hour
If a user login and leaves the page and then come back after two hours, the refresh token will be valid and retrieve a new access and refresh token but never redirect from log-in page to 'logInRedirect' page.
Currently the user stays on the log-in page after a successfull refresh of access and refresh token.
Feel free to make a pull request if you feel up for it :)
I think this is in line with another issue you created. Once the token is valid, I don't think this plugin should be responsible for redirecting you anywhere, unless it's a specific action.
It can be added, as an entirely opt-in feature though, if you're up for it.
@RWOverdijk
But how would you implement this?
How would you know if a user redirects back to your page with an expired access token/valid refresh token?
I somehow need to know when a user does that and then refresh/redirect the user to the authenticated startpage.
Even if I implement my own authentication step for auth:false pages, the user would still always be authenticated if the access token is expired but the refresh token is not.
Revising the page with expired access but valid refresh token will not be detected as a not authenticated user..
There is an expired check in the library. I'd probably use that. When expired, refresh, when that's also expired, redirect.
I've added another step in my route config:
Step:
import { inject } from aurelia-framework;
import { AuthService } from aurelia-authentication;
import { RedirectToRoute, Redirect } from aurelia-router
@inject(AuthService)
export class Anonymous {
constructor(authService) {
this.authService = authService;
}
run(routingContext, next) {
if (routingContext.config.anonymous != null && routingContext.config.anonymous && this.authService.isAuthenticated()) {
return next.cancel(new RedirectToRoute('home'));
}
routingContext.getAllInstructions();
return next();
}
}]
App.js:
config.addPipelineStep(authorize, Anonymous);
Added step for specific route:
{ route: ['log-in', 'log-in'], name: 'log-in', moduleId: 'views/authentication/log-in/log-in', title: ${this.i18n.tr('app:Authentication')} - ${this.domain}, anonymous: true },
It works great when already on the site and trying to reach anonymous pages when authenticated, (redirects).
However, if I leave the page and come back when the accesstoken is expired / refreshtoken not expired I get:
10 navigation instructions have been attempted without success. Restoring last known good location.
As I understand it, it tries to redirect before refresh token is called, (route:home is then unreachable).
I've tried using 'postRender' instead of 'authorize' when adding pipeline step, but the refreshtoken is still being called after the anonymous step.
Do you have any tips for me regarding this problem:)? Does au auth have an event for when refreshtoken is done?
That error means that the page it's being sent to threw an error, and restoring the previous route was also not possible (so it keeps bouncing back and forth, ten times).
I'd have to dig into your problem more to answer these questions; but it looks like the plugin needs some changes. I think it's simply a bug in the refresh logic. @alfkonee you're the super star that built this feature, could you confirm or deny that this logic isn't working properly?
@RWOverdijk @alfkonee Any update on this? I can't seem to find a solution for this
@RWOverdijk @alfkonee I've made a temporary solution for this in my app.js file.
activate() {
this.fetchConfig.configure();
//If access token is expired but refresh token is still valid
if (this.authService.isTokenExpired()) {
this.redirect = true;
}
}
attached() {
this.subscribeUpdatePage = this.eventAggregator.subscribe(authentication-change, response => {
if (this.authService.isAuthenticated() && this.redirect) {
//Redirect to auth startpage after refresh token is used
this.redirect = false;
this.router.navigateToRoute(this.baseConfig.loginRedirect);
}
});
}
@reft That looks like a decent solution. Was there a reason we didn't PR this into the codebase?
@RWOverdijk I was using an old version when I came up with this so I'm not sure that this code works anymore.
I didn't like the dirty checking for the isAuthenticated() function. First I tried using only the 'authentication-change' event instead of the dirty checking but it didn't work 100% because of the: coming back with an invalid access/valid refresh so I then scrapped that aswell.
What I'm doing now is everytime the 'authentication-change' event is executed, wether it's logging in/out or coming back with an invalid access/valid refresh I always hard refresh the page. That way I'm sure that my header/footer and my main view always shows the right content, (hide buttons in nav if not auth etc..), and the correct getMe data is loaded.
app.js
activate() {
this.fetchConfig.configure();
if (this.authService.isAuthenticated() && this.authService.isTokenExpired()) {
//Get new access token if refresh token is valid
this.authService.updateToken().then(result => {
return this.getData();
}
);
}
else {
return this.getData();
}
}
getData() {
this.authenticated = this.authService.isAuthenticated() && !this.authService.isTokenExpired();
let deferreds = [];
if (this.authenticated) {
deferreds.push(
this.authService.getMe().then(data => {
this.claims = data;
})
);
}
deferreds.push(
this.appService.getAppInformation().then(data => {
this.appInformation = data
})
);
return Promise.all(deferreds);
}
attached() {
//Redirect all open tabs if authentication changes
this.subscribeAuthenticationChange = this.eventAggregator.subscribe('authentication-change', authenticated => {
window.location.href = '';
});
}
Sign in
return this.authService.login(this.logInModel, null, baseUrl, null)
I pass a redirect url param so the page is reloaded, I don't remember why but it worked. If I only sent the logInModel I had problems where the authentication-change event fired BEFORE the login was done(or something like that), because of promise..
Sign out
let baseUrl = window.location.origin;
this.authService.logout(baseUrl)
Same as Sign in, we need to use that promise..
This is absolutly not a proper solution.. I'm not sure what to do but there is definitly something wrong with the refreshing of the access token when coming back to a page with invalid access/valid refresh. Check my first post in this issue to see what I mean :P
I also do a hard refresh, but mainly to trigger the browser's "remember password" feature. I think that's why I didn't really notice it.
I'll have to make the dashboard I'm building public soon, so this will have to be fixed by then. I'll pin it.
@RWOverdijk How is your hard refresh implemented? You've seen mine, I bet yours is cleaner. I've tried using storageChangedReload: 1 in config but it doesn't hard refresh, (never f5s)
@reft Oh, I mean, hard page reload. Login, when done reload. Logout, when done reload. Exception from recovery (expired token) hard reload.
But that's not clean, that's avoiding the writing of a fix. If you could see my Trello board you'd understand why haha.
I know that currently when the token expires, nothing happens. It just sits there.
Actually re-reads issue
Oh... I think I finally get it now... I could use some help with managing all this open source stuff.
When the token is valid, the user gets sent to `loginRedirect, when it's expired and successfully refreshed, the user doesn't get redirected. That just means that the place responsible for the redirect needs expanding.
This is the code you're talking about, right? The isAuthenticated check is async, so maybe this should be made to wait for it, including the refresh, before deciding whether or not the user is authenticated. If you use this method it will include the refresh I think. Could you give that a go? Just make your own auth step, or edit the source for the plugin to see if it works.
I hope this finally answers the actual issue. Sorry about the lag.