search

Chainlit / chainlit

Build Conversational AI in minutes ⚡️
https://docs.chainlit.io
Apache License 2.0
7.2k stars 945 forks source link

Fix auto login #1362

Closed ModEnter closed 1 month ago

ModEnter commented 1 month ago

Fix the automatic enter of the only oauth providers, which makes it unable to logout

ModEnter commented 1 month ago

@dokterbob I did a new PR from the good branch this time 😉

dokterbob commented 1 month ago

I could not help myself from digging a little deeper and it seems that OAuth2 has a prompt=consent option to the auth request. This way, a user is not logged out from their OAuth provider (we don't want users to log out from GH if they use GH to login to Chainlit!), but it will require users to confirm their login (and allow them to change to a different user).

To implement this, we could add prompt=consent on all outgoing auth requests -- which AFAIK (we need to test this) only happens when a user as logged out, requiring chainlit to send the request in the first place.

That seems like a much simpler and elegant solution!

dokterbob commented 1 month ago

Ref: https://developers.google.com/identity/openid-connect/openid-connect#prompt

ModEnter commented 1 month ago

Hello, this method is interesting, I'm going to dig that.

ModEnter commented 1 month ago

@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

dokterbob commented 1 month ago

here

Great! Could you please indicate to what extent you manually tested it? It's essential for this we do extensive manual testing!

ModEnter commented 1 month ago

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...

ModEnter commented 1 month ago

hello @dokterbob, for now we are just going to follow your temporary solution, prompt=login everywhere except for GH. Can you commit that or do you want me to do so ? Dan wants me to close this issue ASAP

dokterbob commented 1 month ago

hello @dokterbob, for now we are just going to follow your temporary solution, prompt=login everywhere except for GH. Can you commit that or do you want me to do so ? Dan wants me to close this issue ASAP

Won't make it today, please go ahead. I want to have this in and released (as release candidate) on Wednesday.