Is UNAUTHORISED event firing correctly?

[rishabhpoddar]

An instance of when we fire them is if after getting the refresh lock, and if the session doesn't exist, then we fire it. This may not be correct (see fetch.ts, line 357)

[porcellus]

I'll look for a better place to fire this event, but a quick and easy solution seems to be to add a property to the event that would show if the current call removed the token. This way, both use cases would get the information necessary:

• showing a session expired modal (and other local state changes): all tabs get the information
• for analytics purposes: the event is only received once, even if multiple tabs are active in the browser.

[rishabhpoddar]

I don't quite understand the hacky solution. Perhaps elaborate a bit more:

• Would a session expired modal be shown only if the event indicates that the token was removed?
• "the event is only received once" -> how?
• In general, we want to avoid complicating the user facing API. Adding an extra param in the event makes it more complex since it's not obvious why one would fire this event without the token being removed, and even if one understands it, what are the not supposed to do in this case vs when the token was removed?

[porcellus]

It's not really hacky; it's more of a workaround.

• The session expired modal would be shown for all events firing, so if another tab removed the session token, the modal could still show up
• It's a poor choice of words, but I meant that there would be a way of telling if the analytics event should be fired or not. Even if we fire the event only once per tab (which is necessary to show the modal), we would need something like this to be useful for analytics because then they would log the user logging out once per tab.
• We could add a new event instead of a property, and we could fire both if this is the tab removing the token. I can look for a better solution. This just came to mind after a first look while working on something else.

[rishabhpoddar]

Let's discuss this over a call. I am still unclear about what you are trying to say.

[porcellus]

I propose calling this property sessionExpiredOrRevoked, but we should make it explicit that this only happens in one tab (once per session). removingSession would be more accurate, but it doesn't tell the user how to react to this event.

The cases we the "UNAUTHORISED" event is:

1. We never had a session but tried calling an endpoint that requires one.
2. We thought we had a session, but it was removed before we could try to refresh it (likely another process tried and failed)
3. After a 401, the frontend thought it had a session but could not refresh it. This indicates that the session expired or was revoked on the server and this is the first the frontend notices.

Cases 1&2 can occur many times, but 3 can only happen once per session, and as far as I can tell, this is the one we want to separate. We could indicate something along the lines of hadSession or triedRefreshing, but that would not be too useful.

[rishabhpoddar]

The code changes look good. Please do docs modification for the same.

[rishabhpoddar]

@porcellus we also need to update the onHandleEvent type in the auth-react SDK as that doesn't have the extra boolean along with the UNAUTHORISED action.