(PXP-7429): fix/workspaceLogout: added backup if api call fails to refresh user, … #804
Conversation
…added listeners to iframe when on workspace page
| @@ -83,6 +83,9 @@ export class SessionMonitor { | |||
| if (response.type === 'UPDATE_POPUP') { | |||
| this.popupShown = true; | |||
| } | |||
| }).catch(() => { | |||
| // if API failed check if user is still logged in | |||
| this.notifyUserIfTheyAreNotLoggedIn(); | |||
There was a problem hiding this comment.
why logging out user at here? the logic was if it can reach refreshSession() then it means the user is still logged in and actively using the portal
There was a problem hiding this comment.
the bug had to do with the user being logged out, but not alerted that they had been logged out, leading to other errors. this code checks if they are still logged in and if not alerts them, it does not log them out. I am still not 100% on how all of this code works if you do not think this check is needed i can remove
There was a problem hiding this comment.
I think the bug was actually buried deeper in the logic flow: the notifyUserIfTheyAreNotLoggedIn function will make a call to the submission api and when it returns a 401 the portal assumes the user is logged out. But this will result in as the current behavior that for some times the popup showed up early while in other occassions some are shown up later.
This is mainly due to the fact that the access token in cookie is not updated until the pervious one has expired. After this fix uc-cdis/fence#874 is merged, both session token and access token will be updated.
For that case, I probably will test and change this logic flow back to let portal actively logout users. So this piece of code will probably be removed in my work. I'm okay will merging as is because I think it won't break things, just a FYI
The changes to the workspace code LGTM
There was a problem hiding this comment.
sounds good, thanks for looking over, im glad someone else was also looking into this
| iframeContent.addEventListener('keypress', () => sessionMonitor.updateUserActivity(), false); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
See also this for what I think should have been used: https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage . And this PR which proposes a solution using "message" events: #1002
There was a problem hiding this comment.
But maybe the context in which your code works is somehow different...not sure.
Jira Ticket: PXP-7429
Bug Fixes