Fix auto login #1362
Fix auto login #1362
Conversation
…single one when only one is defined
|
@dokterbob I did a new PR from the good branch this time 😉 |
There was a problem hiding this comment.
I just reviewed the functionality (in our own app, with a single oauth provider).
What I hoped this would do, was to remove the intermittent login step before the redirect, which our users have found confusing (as there's no user interaction required).
What instead happened, is that now there is a single button for the user to click; there is a choice to be made but nothing to choose. Remember those websites back in the 90's with the 'Click here to ender the website'-button?
Of course, users need to be able to log out and I see the pain there. Our own users also have this issue.
What's happening AFAIK, is that when a user logs out from chainlit, the oauth authentication is immediately initiated again. If a user was not logged out from the oauth provider, this means that they are automatically logged in again. What the user's expectation is instead, is that when they're logging out, they are presented a prompt to log in again. If there's a single OAuth provider, they should end up seeing that provider's oauth login screen again (not a page with a single button).
So IMHO, what's necessary is that logging a user out in the frontend also logs them out on the oauth backend. The proper way might be to add an optional async def logout() method to OAuthProvider which for each OAuth provider uses the available credentials to have the provider revoke their credentials.
Alternatively, if 'properly' implementing OAuth (logout) (not a trivial matter!), perhaps we could make this behaviour configurable, setting prior behaviour as the default (e.g. single_oauth_auto_redirect = True, allowing users to specifically disable it?
|
I could not help myself from digging a little deeper and it seems that OAuth2 has a To implement this, we could add That seems like a much simpler and elegant solution! |
|
Hello, this method is interesting, I'm going to dig that. |
dosubot
bot
added
the
size:S
|
@dokterbob I restored the previous behaviour and added prompt=consent to the authorization params for each one of the 9 chainlit external providers, as refered here |
Great! Could you please indicate to what extent you manually tested it? It's essential for this we do extensive manual testing! |
|
Sure, I only tested it on github oauth, but since it is defined in the OpenID connect core 1.0 spec, it should be sure that it work with the other one. If you want to test them, it would be great, I personnaly do not have the possibility to do so... |
There was a problem hiding this comment.
@ModEnter I've just tested your branch with our own Descope integration and have unable to verify the solution. The prompt=consent parameter is correctly passed, yet it does not seem to be respected by Descope's oauth2's authorize endpoint.
Descope's docs (for my own reference): https://docs.descope.com/getting-started/oidc-endpoints#guide-to-using-oidc-endpoints. For obvious reasons, I'm not going to paste a full breakdown of URL parameters, but it does seem the consent prompt request is passed along the login flow, so it might be a bug on their side. It's interesting to note that the login request is entirely absent from their login page.
After changing it to prompt=login, I got a login page.
Similarly, I've found that prompt=select_account does not have any effect.
Hence, for now, I'd suggest that at least for Descope, we use prompt=login. Alternatively, as you've verified GH to function, we could use prompt=login everywhere except for GH.
|
hello @dokterbob, for now we are just going to follow your temporary solution, |
Won't make it today, please go ahead. I want to have this in and released (as release candidate) on Wednesday. |
…Descope, should work for Google and Okta (at least)
Merge main into fix_auto_login
Fix the automatic enter of the only oauth providers, which makes it unable to logout