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

Unable to logout when oauth is enabled #1207

Closed
pachu4u opened this issue Aug 13, 2024 · 20 comments
Closed

Unable to logout when oauth is enabled #1207

pachu4u opened this issue Aug 13, 2024 · 20 comments
Labels
auth Pertaining to authentication. enhancement New feature or request evaluate-with-priority What's needed to address this one?

Comments

@pachu4u
Copy link

pachu4u commented Aug 13, 2024

Noticed a problem in the latest release with respect to authentication.

I am using OAuth aauthentication using GIT.

If I logout, it again logs me in automatically from the logout page. So i can not log out essentially.

@onurolgun24
Copy link

I'm also having the same problem. I'm using Azure AD and when I sign out, user automatically logs in. I believe there should be a cookie deletion or sth like that but I couldn't find a good resource about this.

I would appreciate any kind of information about this.

@dokterbob dokterbob changed the title Auto Login | Oauth Unable to logout when oauth is enabled Aug 14, 2024
@dokterbob
Copy link
Collaborator

Effectively, it seems that logout is not implemented for oauth. Not sure whether this is a bug or a feature, but I can definitely validate this.

@dokterbob dokterbob added enhancement New feature or request evaluate-with-priority What's needed to address this one? auth Pertaining to authentication. and removed needs-triage labels Aug 14, 2024
@julesterrien
Copy link

julesterrien commented Sep 7, 2024

+1 - any update on a fix?

I have auth0 / google setup and clicking 'logout' in the user menu redirects the user back to the "sign in with auth0" page which automatically redirects back to a logged in session

i tried the following but it didn't log the user out
@cl.on_logout
def logout(request, response):
logout_url = f"https://{OAUTH_AUTH0_DOMAIN}/v2/logout?client_id={OAUTH_AUTH0_CLIENT_ID}&returnTo={CHAINLIT_URL}/"
requests.get(logout_url)

@stephenrs
Copy link

stephenrs commented Sep 8, 2024

Chainlit never logs out for any auth approach because it stores the auth token in localStorage and never deletes it.

I have worked around this by deleting the token from localStorage explicitly from my own JS code.

My personal recommendation is not to rely on any of the auth features of Chainlit because storing auth tokens in localStorage doesn't suggest that security is a high priority for the project (so there may be other not-so-obvious risks) and auth is probably going to get removed see #1265.

@yumi-bk20
Copy link

Hello @stephenrs, I am actually facing the same problem where the token is saved, and user is directly re-directed to the app without entering his credentials again when logging out. Can you please show me how you did it with javascript? i tried but didnt work as i dont know coding js very well .

@stephenrs
Copy link

stephenrs commented Sep 13, 2024

@yumi-bk20 You can remove the token with just one line of code using the builtin localStorage object:

localStorage.removeItem('token');

@vivekjainmaiet
Copy link

@yumi-bk20 Could you please share how you are doing it in JavaScript.

Like how you Listen for a logout event or trigger, Remove the token from localStorage and Call the Chainlit logout endpoint .

@julesterrien
Copy link

We're using the chainlit python lib to generate the frontend so we can't natively access javascript (and localStorage) without adding a custom script to get|remove the token. This is a little hacky

Ideally the on_logout lifecycle would give us access to cl.user_session so we can access the token and send logout requests to our API using it. Ideally, on_logout would also do localStorage.remove(token) on its own

@stephenrs
Copy link

stephenrs commented Sep 14, 2024

@julesterrien You're right, it's definitely a hack, and you're right that chainlit should remove the token automatically.

However, I don't think any fixes or features will be added to the auth system in the short term, and it will probably eventually be removed. So I personally don't recommend using it or relying on it in your projects. Please see #1265

@vivekjainmaiet
Copy link

@stephenrs Than how to use Microsoft Entra with Chainlit. Have you implemented any auth for your project??

@stephenrs
Copy link

stephenrs commented Sep 15, 2024

@vivekjainmaiet I'm not familiar with Microsoft Entra, but in my case, I'm integrating chainlit into an existing Flask-based (python) app that already handles authentication (using Auth0).

As far as I'm aware, most/all authentication providers make it fairly easy to secure apps, so I would tend to expect that MS Entra is no different.

So even if you don't have an existing app, it should be reasonably straightforward to set one up (using Flask or FastAPI, for example) as a type of wrapper around chainlit. Then, you will have full control of the security of your project, access to the wealth of flexibility and support that auth/identity services provide, and you won't have to be concerned about the additional complexity or potential for security gaps in chainlit, or getting stuck with an unsupported configuration if/when they decide to remove auth.

Also, the auth system is pretty tightly coupled with the data layer, and the chainlit team appears to also be planning to remove the data layer, which is also discussed on #1265.

@yumi-bk20
Copy link

@stephenrs but are you like pointing to the Logout button to trigger the action ? or some other function, coz this is how am doing it, and its not working for me:
document.getElementById('LogoutIcon').addEventListener('click', function() {
localStorage.removeItem('token'); }
I tried also: localStorage.clear(); didnt work either
PS: I added the .js file in 'public' directory and I added the path also in chainlit configuration file config.toml

anyone that knows something plz help!

@stephenrs
Copy link

@yumi-bk20 Since I'm not using the chainlit auth features, I'm not relying on any of the logout functionality of chainlit, including the on_logout callback or the logout menu. login/logout takes place at the level of my app and I delete the token from my app's frontend code, and I've removed the logout menu option from the chainlit UI.

Have you confirmed that your event listener is getting triggered when you think it is?

Also, have you used your browser's dev console to make sure the token is not getting deleted but then re-saved later in your execution flow? For example, I'm not exactly sure when chainlit saves the token to localStorage or if it does it more than once.

@ModEnter
Copy link
Contributor

Hello, I have made a PR to remove the "autologin" behaviour.
#1357

@lolapirespintopolarys
Copy link

@stephenrs but are you like pointing to the Logout button to trigger the action ? or some other function, coz this is how am doing it, and its not working for me: document.getElementById('LogoutIcon').addEventListener('click', function() { localStorage.removeItem('token'); } I tried also: localStorage.clear(); didnt work either PS: I added the .js file in 'public' directory and I added the path also in chainlit configuration file config.toml

anyone that knows something plz help!

hello @yumi-bk20 ! I got the same issue... have you found something that works? :)

@karthikvarmak1
Copy link

@julesterrien @stephenrs @yumi-bk20 can you share the custom script snippet to delete the token? I have tried below code and it did not work for me.

document.addEventListener('DOMContentLoaded', function() {
    function handleLogout() {
        console.log('Logging out');
        localStorage.removeItem("token");
        alert('You have been logged out');
        // You might want to redirect the user or update the UI here
    }

    // For the SVG icon
    const logoutIcon = document.querySelector('[data-testid="LogoutIcon"]');
    console.log(logoutIcon);
    if (logoutIcon) {
        logoutIcon.addEventListener("click", handleLogout);
    }

    // For the text span
    const logoutText = document.querySelector('span');
    console.log(logoutText);
    if (logoutText && logoutText.textContent === 'Logout') {
        logoutText.addEventListener("click", handleLogout);
    }
});

@dokterbob
Copy link
Collaborator

dokterbob commented Sep 30, 2024

Hey everyone, here's a little update on this saga which admittedly, is far from an ideal situation.

Let me start by stating clearly that getting all auth stuff out of this library, e.g. not rolling our own, is quite high on our wish list. That having been said, we cannot afford simply break all existing use cases, there needs to be a clear transition path, in addition to a clear destination. Mapping that takes time and attention.

Concrete suggestions, including proposals for how to code it as well as other contributions of a positive nature are very welcome.

Now, feel free to join me in this rabbit hole, which I opened whilst reviewing #1362 tries to address this issue.

This seems to be all components related to logging out. Thus far, I'm still unsure what exactly is going on.

Towards a proper solution

We'd need to add something like logout() to OAuthProvider, adding logout URL's to all providers, then ensuring that (when using OAuth), it is automatically called on POST to logout endpoints.

Note that, particularly, the UX flow here is non trivial. When clicking 'log out', the last thing a user expects is to be logged out of Google/GH etc.

Refs:

Current issues (e.g. unbreaking the UX)

The above having been said, simply logging out the user on the client side and prompting the user for login consent as in #1362, should solve the UX. Yet, somehow, it doesn't. It seems there might be a combination of two things going on:

  1. removeToken() isn't being called, or isn't doing it's job in logout(), as is suggested by some of the workarounds above. I have not been able to replicate this behaviour locally (e.g. it seems to be called, but not yet 100% sure).
  2. prompt=consent isn't properly passed to or respected by (all) OAuth providers.

I'd love to hear your thoughts on the above. Explicitly not interested in meta-issues, like your opinion on our level of freely provided community support.

Breadcrumbs (research)

React library

In the React Client library, logout seems to happen here:

There's also a logout hook here, with behaviour which seems (roughly) sensible:

const logout = async (reload = false) => {

Frontend

This is where logout() is called in the frontend:

It's instantiated here:
https://github.com/Chainlit/chainlit/blob/1c5098c0cb25c40a0c18721bb1389a0d0319032e/frontend/src/components/atoms/buttons/userButton/menu.tsx#L32C3-L32C38

And imported from here:

import { useAuth, useConfig } from '@chainlit/react-client';

Backend

Meanwhile, the backend API on /logout does, exactly, nothing (unless hooks are defined):

async def logout(request: Request, response: Response):

@dokterbob
Copy link
Collaborator

TLDR; Initial testing with #1362 suggests that:

  • GH respects prompt=consent
  • Descope only respects prompt=login

I'd love to have feedback on (and don't have scope to test) other OAuth providers.

In addition, I think we should indeed accelerate decoupling (= getting rid of our own) auth and/or implement logout() on OAuthProvider.

@dokterbob
Copy link
Collaborator

Quick update; we'll do a RC with #1362 on Wednesday, solving the UX part of this issue.

@stephenrs
Copy link

stephenrs commented Oct 1, 2024

My (perhaps heavy-handed, big hammer) suggestion, as a possible bridge solution is to:

  1. Announce loudly that auth will be removed completely in a near-term release, so people should set up a little FastAPI (or other) app and easily handle their own auth without all the bugs, limitations, and extra complexity of chainlit. They can do this in preparation for the release, and the maintainers/community should be ready to help with questions as users migrate. Ideally, simple instructions for how to do this should be provided.
  2. Since the CL code thinks it needs auth to enable persistence-related features, but all it really needs is a User object, use or rename/refactor the header auth code flow as a way to allow devs to create a User for CL. I'm doing this now by passing an auth token as an httponly cookie, for example.
  3. Clicking the logout menu from within CL (for people who want to use it) can call a custom webhook or a custom JS function (similar to the copilot callback functionality), so devs have control and freedom regarding handling logout. Ideally, removing the CL logout menu option completely should be configurable in config.toml for users who don't need it.
  4. CL should stop saving an auth token to localStorage and any code that relies on this should be refactored.

This seems like a lightweight way to drop a major burden from the project quickly and in a controlled way, while not hurting/blocking existing user projects, although I could be missing something.

I just don't see any point in investing more time in trying to fix an auth system that shouldn't exist at all, and it's just too easy (and more reliable/flexible/secure) to get auth without CL providing it.

NOTE: My understanding is that CL currently doesn't play well with any of the most popular python frameworks, so a prerequisite to this proposed path to auth freedom might be that CL might have to be made to at least work with a parent FastAPI app without breaking routing (which has been documented on a separate issue).

It would also be great to see a more comprehensive proposed roadmap included in the announcement about dropping auth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth Pertaining to authentication. enhancement New feature or request evaluate-with-priority What's needed to address this one?
Projects
None yet
Development

No branches or pull requests

10 participants