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

Weird automatic log in/log out behaviour #332

Closed
Powersource opened this issue Nov 4, 2020 · 8 comments
Closed

Weird automatic log in/log out behaviour #332

Powersource opened this issue Nov 4, 2020 · 8 comments

Comments

@Powersource
Copy link
Contributor

This might be two bugs but maybe it's just me doing something wrong. My code for reference https://github.com/Edgeryders-Participio/realities/pull/133/files

  1. If you're in your app logged in and refresh the page (even clearing cache with ctrl-f5) you are still visibly logged in. But if you open the same link in a new tab then you look logged out. Even if you can click Log in and be immediately redirected back to the app without filling in your username/password, since you in fact actually were logged in. This might be related to me setting autoSignIn={false} but that's because I don't want the user automatically redirected to the log in page on visit (it's a SPA, and some users shouldn't have to log in). So I kind of want autoSignIn to be false, but I want detection of already being signed in to be more automatic.

  2. When I call signOut this module's cache mostly seems to be cleared of the log in info. If I click Log in in the app then I'm automatically redirected back to the app since I'm technically still logged in. But I want the option of logging into another account than the one I'm currently logged into.

Any ideas how to fix either of these?

@simenandre
Copy link
Member

Hello, thank you for posting an issue, and for using oidc-react!

The underlying package, oidc-client uses session storage, but you can change that. You'll have to provide your own UserManager for that (see #327 for an example). Both of these should, if I understand correctly, fix your issues (or make it possible for you to customize it as you see fit).

Does that make sense?

@ryancole
Copy link
Contributor

Sounds like session storage. Session storage expires when you close the browser or tab.

In contrast, local storage persists across tab and browser closes.

In more contrast, I think cookies persist even much longer than that?

But you can kind of see the tiers of persistence here.

Anyway, it sounds like perhaps you're using session storage. Can you try using a different storage mechanism and let us know if that fixes your issues? @Powersource

@ryancole
Copy link
Contributor

Regarding the sign out event, you may need to implement the onSignOut event handler?

@Powersource
Copy link
Contributor Author

Thanks! Switching to localStorage fixed my first issue. See https://github.com/Edgeryders-Participio/realities/pull/140/files for code. However two things:

  • This solution is pretty verbose and feels a bit hacky, it would be nice to be able to do this with a simple boolean switch.
  • I need to import WebStorageStateStore from oidc-client. It would be nice if this was re-exported by oidc-react.

Would you accept a PR for these two changes?

And regarding my issue nr 2, that still persists and so I'm unable to properly log out of my current account and into another one. Reading the documentation a bit more closely, signOut which is what I'm using, apparently only does "Returns promise to remove from any storage the currently authenticated user." which explains the behaviour I'm seeing, and I guess I should use signOutRedirect instead. It's maybe a good idea to rename signOut to something else, since it can give the user the impression it does something other than what it does?

@ryancole
Copy link
Contributor

ryancole commented Nov 13, 2020

Hi @Powersource, what would you like to do with a boolean switch? A lot of this oidc-react library is just pass-through to the underlying oidc-client library.

Yea, re-exports make sense. Can you provide a PR for that? If not, I will do it when I find some time. I created #344 to track that.

Regarding the sign out, you're correct. If you're wanting to sign out from the oidc server, then you can provide your own sign out method in which you can provide onSignOut in that fashion. Here's how I do it below ...

  function handleSignout() {
    const destination = `${process.env.AUTHORITY_URL}/Identity/Account/Logout`;
    location.href = destination;
  }

Which just directs me to the oidc server which signs me out.

@Powersource
Copy link
Contributor Author

what would you like to do with a boolean switch?

I would like it to switch to localStorage instead of sessionStorage. My current solution is very verbose (PR 140 linked to above) and feels pretty hacky, since I've had to copy oidc-react's arguments for UserManager from

return new UserManager({

Maybe if you don't like the boolean switch, then maybe an option object that can override arguments for UserManager (certain ones instead of all of them at once)?

@ryancole
Copy link
Contributor

That doesn't seem verbose to me. It looks correct.

All of those settings that you are providing are situational to your auth provider. There's nothing left there to make generic or less verbose - those are the necessary settings for your application.

As far as the user storage mechanism, I believe the philosophy of the underlying oidc-client library is that the storage mechanism is not known by the oidc-client (or that the maintainers there feel like it's outside the scope of their library). At least, this was the response they gave me months ago when I asked why their library doesn't offer synchronous methods for accessing the current user, etc. They said they couldn't be sure that the mechanism would support synchronous access. So, with that in mind, I think it's probably best to just keep that user storage property a pass-through to their library so that oidc-react doesn't end up cutting off available functionality to certain users. The choices are not just local storage vs session storage, basically. There may be more choices. So, a boolean switch there doesn't make much sense to me.

I would be all for re-exporting WebStorageStateStore though.

@boi87
Copy link

boi87 commented Mar 1, 2024

That doesn't seem verbose to me. It looks correct.
All of those settings that you are providing are situational to your auth provider.

Hi @ryancole , just wanted to hop in on this discussion a bit late. So, I ran into a similar issue as @Powersource did a while back. My users kept getting kicked back to the login page when they opened links in new tabs.

I followed the solution mentioned above, and it did the trick for me.

What I'd like to point out is that while implementing a useLocalStorage?: boolean flag to switch between localStorage and sessionStorage storage for tokens may seem specific to @Powersource's AuthProvider implementation, the behaviour of "remaining logged in when opening links in new tabs" is a common user expectation, and therefore could benefit from having a proper config option related to it.

Although the implementation of the solution was relatively quick, possibly having a well-defined boolean flag in your library with its dedicated documentation would have spared me a few hours of googling and troubleshooting.

Please note I don't mean to be rude, I hope my input comes across constructively 😉
Thanks

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

No branches or pull requests

4 participants