SpoonX / aurelia-authentication

Authentication plugin for aurelia.
http://aurelia-authentication.spoonx.org
MIT License
90 stars 60 forks source link

Manually setting expired tokens ruins routing #400

Closed MaximBalaganskiy closed 3 years ago

MaximBalaganskiy commented 6 years ago

I was testing a scenario when a user does not return to his session long enough so that both his tokens are expired.

So I would login normally and then set the token in localStorage manually with localStorage.setItem("token","{token: '[expired_token]', refreshToken:'[expired_token]'}"). Straight after this nothing happens with the page and all navigation is broken - changing urls or tapping anchors does not do anything. Interestingly enough, calling router.navigateToRoute works still but only if I don't try changing the url first.

Hard reloading the page with the expired tokens still in the storage works correctly in 99% of the cases. But sometimes I observe the same issue - navigation stops working right after reload.

MaximBalaganskiy commented 6 years ago

Probably something local... closing for now

doktordirk commented 6 years ago

there was a quite similar issue. must be something with the interceptor i guess

MaximBalaganskiy commented 6 years ago

investigating at the moment... I explicitly call isAuthenticated in the activate method to force a token refresh. If the refresh token is expired itself then isAuthenticated throws and that stops activate completely

MaximBalaganskiy commented 6 years ago

this is something to do with updateTokenCallstack. When refresh token fails for any reason the callstack gets rejected and this breaks activate flow, even though all the methods are in the try/catch

MaximBalaganskiy commented 6 years ago

ok, my problem was in the isAuthenticatedAsync which is my wrapper around isAuthenticated. Something is not right when this function is promisified - rejections still break execution even when try/catch us used. I will submit a PR for a true async function later, which I successfully tested here...

MaximBalaganskiy commented 6 years ago

The true reason is that isAuthenticated does not call a callback in catch, thus a promisified function is awaited forever and activate never completes

doktordirk commented 6 years ago

a case for 'never log, always throw'. hmm, would callback(new Error(..)) work? or callback(()=> throw new Error())?

MaximBalaganskiy commented 6 years ago

Returning false would do I recon, it will enable promisification :) or, a true async function, which rethrows

On Wed., 8 Aug. 2018, 7:04 pm doktordirk, notifications@github.com wrote:

a case for 'never log, always throw'. hmm, would callback(new Error(..)) work? or callback(()=> throw new Error())?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/SpoonX/aurelia-authentication/issues/400#issuecomment-411338405, or mute the thread https://github.com/notifications/unsubscribe-auth/ADimvOqxkshsUtnIGmE2RsEv8R8cobiOks5uOqmEgaJpZM4VzSNf .

doktordirk commented 6 years ago

as such that'll be better. would mean a 4.x and still maintaining the 3.x branch though. but then, i kinda would like a bigger refactoring anyways, it got untidy at places