Skip to content
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

Fix crash when multiple tabs open and logging out in one of them #191

Closed
Powersource opened this issue Jan 8, 2021 · 8 comments · Fixed by #196
Closed

Fix crash when multiple tabs open and logging out in one of them #191

Powersource opened this issue Jan 8, 2021 · 8 comments · Fixed by #196

Comments

@Powersource
Copy link
Member

Related links

IdentityModel/oidc-client-js#49
IdentityModel/oidc-client-js#168
IdentityModel/oidc-client-js#830
#162

Doing this IdentityModel/oidc-client-js#830 (comment) (note it's supposed to be signinSilent not signInSilent) right after setting up our userManager at first seemed to have fixed the issue, but then made me run into errors including crashing the page with "Unhandled Rejection (Error): invalid_grant" (thrown by oidc-client).

@aerugo
Copy link
Collaborator

aerugo commented Jan 8, 2021

Those errors only happen when we log out manually, correct? If so, that bug is an acceptable trade-off for now.

@Powersource
Copy link
Member Author

Yep. Reproduction:

  • Log in in one tab (tab 1).
  • Open another tab (tab 2), you are automatically logged in there.
  • Log out in tab 2 (although other way around works as well).
  • Refresh tab 1, crash.

@Powersource
Copy link
Member Author

Merged now

@Powersource Powersource changed the title Don't require the user to click Login again when opening a new tab Fix crash when multiple tabs open and logging out in one of them Jan 8, 2021
@Powersource
Copy link
Member Author

Tried adding a .catch for the error and that stops the instant crashing but instead makes the user crash when they try to perform something that requires auth, which is probably worse.

@Powersource
Copy link
Member Author

Some clues in this documentation issue but even they seem confused IdentityModel/oidc-client-js#339

Just do a signinSilent before you even think about calling getUser. If it resolves, the User you get immediately after is valid; if it rejects you need to do an interactive login e.g. with signinRedirect.

The querySessionStatus method on UserManager may be able to give complete status information, but it performs a callback to silent_redirect_uri that has a very different query than signinSilent, and somehow while I was doing that I didn't think to pass that to signinSilentCallback (probably because I hadn't figured out that the different URLs MUST be distinct and was trying to detect based on the query). But it seems silly to do a querySessionStatus which does almost the same thing as a signinSilent but might have to be followed by a signinSilent anyway.

... But I've also seen cases where signinSilent seems to generate a user with invalid tokens, so I think I'm still missing something here.

@Powersource
Copy link
Member Author

@Powersource
Copy link
Member Author

Closing this through #196 now and putting my future faith in #195

@Powersource
Copy link
Member Author

oh and fwiw I realized that the "crash" in the title here was only in dev, it was an uncaught promise and that I guess crashes the dev react renderer but not in prod. Either way it's getting caught now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants