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

signOut function does not call endsession endpoint #7

Closed
minkas-g-d opened this issue Jan 11, 2019 · 8 comments
Closed

signOut function does not call endsession endpoint #7

minkas-g-d opened this issue Jan 11, 2019 · 8 comments

Comments

@minkas-g-d
Copy link

What signOut does is to call userManager.removeUser() and nothing else.
I would expect to redirect and end the session of the user.
Currently I am using userManager.signoutRedirect method to sing out from Identity server.

@thchia
Copy link
Owner

thchia commented Jan 11, 2019

So the suggestion is to call signoutRedirect method on userManager internally when the signOut from UserData function is called?

That seems like a reasonable suggestion, I’m happy to make the changes in the next day or two.

@thchia
Copy link
Owner

thchia commented Jan 11, 2019

@minkas-g-d I've made some changes to accommodate this functionality. I haven't tested it fully yet, but the changes are published under the @next tag (v0.4.0-alpha.0), so feel free to try it out and provide feedback.

Note that you will need to add the "end session" route to your application and render the SignoutCallback component. As I understand it, the end session URL is defined by the Identity Provider, and calling signoutRedirect will redirect your application to this location. You can refer to the pre-release version of the README.

@minkas-g-d
Copy link
Author

Hi, @thchia .
I have tried testing the new version but I get error on signing in -
Uncaught TypeError: Cannot read property '_signinEnd' of undefined
at signinRedirectCallback (oidc-client.min.js:2381)

Tried to figure out where is the problem but with no success for the moment.

@thchia
Copy link
Owner

thchia commented Jan 14, 2019

@minkas-g-d Aside from the above issue you just mentioned, I've done more digging into the OpenID spec.

It seems that the end_session endpoint you mentioned is used to log the user out of the Provider service. That is, if I logged in to your application with my Google account, then signing out and calling signoutRedirect is actually going to log me out of Google*.

I'm not sure that this should be the default behaviour. In my experience, logging out from a client application does not log me out of the Identity Provider. We can consider adding additional functionality to support this, but it seems like the changes I made are now redundant.

Please feel free to advise if you are more familiar with the OpenID spec and I'm misunderstanding something.

* Additionally, some Identity Providers (incidentally, Google included) do not support this action.

@minkas-g-d
Copy link
Author

Hi, @thchia! Cannot say that I am better acquainted with the OpenID spec.
In my use case, logging out of the Identity provider is what I want because there is no
way to remove the cookie from the authentication server which is hosted in a place different from
the application that I am developing.
( The auth flow is the following: the user visits app.com your library redirects him to the auth server, than after login and session creation the auth server redirects back to the app. )

@thchia
Copy link
Owner

thchia commented Jan 15, 2019

@minkas-g-d I've just been looking at the spec.

I understand your use case better now, but given that not all OPs support this "single sign-out" functionality, I am inclined to leave the functionality as-is for now. I tested this yesterday using Google and it did not work. I will continue to investigate how to detect whether single sign-out is supported so that I can automatically redirect if it is.

Can I ask that you continue to implement the redirection in userland for now?

@minkas-g-d
Copy link
Author

Yes, of course. Thanks for the efforts.

@thchia
Copy link
Owner

thchia commented Jan 22, 2019

Will track this in #11

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

2 participants