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

session cookies are still valid after logout #24713

Open
kamalcodez opened this issue Jul 17, 2023 · 15 comments
Open

session cookies are still valid after logout #24713

kamalcodez opened this issue Jul 17, 2023 · 15 comments
Assignees
Labels
validation:required A committer should validate the issue

Comments

@kamalcodez
Copy link

kamalcodez commented Jul 17, 2023

After logout from superset, session cookies continues to be valid.
User can still login using those cookies if he has session cookie saved.
They should be invalidated after logout.

How to reproduce the bug

  1. Login into superset.
  2. Copy the session cookies (with the help of browser extension).
  3. Logout from superset.
  4. Import session cookies into second browser.
  5. Hit superset login url in second browser.
  6. You will be logged in automatically.

Same can be done in single browser.

Expected results

User should not be able to login, cookies should be invalidated after logout.

Actual results

User is able to login, cookies continues to be valid even after logout.

Environment

(please complete the following information):

  • browser type and version : chrome 114.0.5735.199 , Firefox : 115.0.2
  • superset version: 2.0.1
  • python version: 3.9.16
  • browser extension : Cookie-Editor

How do I invalidate cookies after logout ??

@kamalcodez kamalcodez changed the title cookie are still valid after logout session cookies are still valid after logout Jul 17, 2023
@aibunny
Copy link

aibunny commented Jan 26, 2024

I am also facing this issue though I am using keycloak as my OIDC.

@rusackas
Copy link
Member

Is this still an issue in 3.1/4.0? I would assume so, but this risks being closed as stale/obsolete if we can't determine that. CC @craig-rueda in case he has any ideas how to directly answer/resolve this.

@rusackas rusackas added the validation:required A committer should validate the issue label May 13, 2024
@iali9906
Copy link

i have the same error with AUTH_OID any news?

@leezj-cbm
Copy link

Hi All, i am facing this same issue. I have tried these and it still doesn't work. The app is unable to modify or delete the cookie.

Manually expiring the cookie or deleting the cookie in browser prevents this.

I have trying to code the app to delete or modify the cookie.

Anybody have any advice for me?

...
oidc.logout()
super(AuthOIDCView, self).logout()
...
response = make_response(redirect(oidc.client_secrets.get('issuer') + 
                                          '/protocol/openid-connect/logout?post_logout_redirect_uri='+
                                          quote(redirect_url)+f"&client_id={client_id}&id_token_hint={id_token}"))
response.set_cookie('oidc_id_token',value='',max_age=0,expires=0, path='/',domain=request.host,secure=False, httponly=False, samesite=None)
response.delete_cookie('oidc_id_token',path='/',domain=request.host)
return response

@jkogut
Copy link

jkogut commented Jul 25, 2024

@rusackas Yes, same issues on 3.1.3 version

@dpgaspar
Copy link
Member

To fully invalidate sessions on logout use this: https://superset.apache.org/docs/security/#switching-to-server-side-sessions

@iali9906
Copy link

To fully invalidate sessions on logout use this: https://superset.apache.org/docs/security/#switching-to-server-side-sessions

Hi @dpgaspar I set the configuration as follows:
but nothing changes:

AUTH_TYPE = AUTH_OID
curr  =  os.path.abspath(os.getcwd())
OIDC_CLIENT_SECRETS =  curr + '/docker/pythonpath_dev/client_secret.json'
OIDC_ID_TOKEN_COOKIE_SECURE = True
OIDC_OPENID_REALM: 'stest'
AUTH_USER_REGISTRATION = True
AUTH_USER_REGISTRATION_ROLE = 'Public'
OIDC_INTROSPECTION_AUTH_METHOD: 'client_secret_post'
CUSTOM_SECURITY_MANAGER = OIDCSecurityManager
OIDC_TOKEN_TYPE_HINT = 'access_token'
OIDC_SCOPES = ["openid","userinfo"]

SESSION_SERVER_SIDE = True
SESSION_TYPE = 'redis'
SESSION_REDIS = redis.from_url('redis://superset-redis-prod-01:6379')
SESSION_USE_SIGNER = True
SESSION_COOKIE_SECURE = True
SESSION_COOKIE_HTTPONLY = True
SESSION_COOKIE_SAMESITE = "Lax"
SECRET_KEY = os.getenv('SUPERSET_SECRET_KEY', 'Tv********************************************************Rr')
def encode_data(data):
    return data.encode('utf-8') if isinstance(data, str) else data
def decode_data(data):
    return data.decode('utf-8') if isinstance(data, bytes) else data

and in my keycloack_security_manager.py file I modified the logout section like this:

from:

@expose('/logout/', methods=['GET', 'POST'])
    def logout(self):
        oidc = self.appbuilder.sm.oid

        oidc.logout()
        super(AuthOIDCView, self).logout()
        redirect_url = request.url_root.strip('/') + self.appbuilder.get_url_for_login

        return redirect(
            oidc.client_secrets.get('issuer') + '/protocol/openid-connect/logout?redirect_uri=' + quote(redirect_url))

to:

    @expose('/logout/')
    def logout(self):
        oidc = self.appbuilder.sm.oid

        # Invalidare la sessione di Superset
        oidc.logout()

        # Invalidate the session in the current application
        super(AuthOIDCView, self).logout()

        # Create a response object
        redirect_url = 'https://bi.company.it/login'
        full_redirect_url = oidc.client_secrets.get('issuer') + '/protocol/openid-connect/logout?redirect_uri=' + quote(redirect_url)
        resp = make_response(redirect(full_redirect_url))
        resp.set_cookie('session', '', expires=0, secure=True, httponly=True, samesite='Lax')

        return resp

But nothing has changed.

By leaving the original logout function, I was being redirected back to the initial dashboard.

putting it like this instead, I get redirected to the keycloack that does the logout and shows me the screen to log in again, but if in the address bar I remove bi.company.co.uk/login and put bi.company.co.uk I still get redirected to the dashboard without login.

so it logs out the session on keycloack but it doesn't superset.

@kamalcodez
Copy link
Author

closed by mistake.

@kamalcodez kamalcodez reopened this Aug 12, 2024
@Nboaram
Copy link

Nboaram commented Aug 16, 2024

I've found that once moving to a server side session the session still isn't being fully deleted, but when I do session.clear() afterwards it cleans up properly and therefore logs out properly, but I wonder if there's a better way of doing that.

@abhishekcms
Copy link

Any solution? I am struggling to fix this issue.

@iali9906
Copy link

I've found that once moving to a server side session the session still isn't being fully deleted, but when I do session.clear() afterwards it cleans up properly and therefore logs out properly, but I wonder if there's a better way of doing that.

Can you point us to where and how you integrated that part?
Thank you

@Nboaram
Copy link

Nboaram commented Aug 16, 2024

If your talking about the session.clear() part it's just after the logout call for me.

oidc.logout()
super(AuthOIDCView, self).logout()
session.clear()

https://superset.apache.org/docs/configuration/configuring-superset/#keycloak-specific-configuration-using-flask-oidc

@kamalcodez
Copy link
Author

@Nboaram How do I achieve this with ldap settings ? we are using AD/ldap for user authentication.

@kamalcodez
Copy link
Author

@rusackas @dpgaspar @jkogut @Nboaram
can you please tell me which files/methods to modify in order to invalidate cookies after logout.
file location for /logout api.

@rusackas
Copy link
Member

rusackas commented Sep 4, 2024

I'm no expert here, but I think @Nboaram is right... I assume you're using a custom security manager, like in the docs:
https://superset.apache.org/docs/configuration/configuring-superset/#keycloak-specific-configuration-using-flask-oidc

If so, indeed you can just call session.clear().

I'm not sure, but if this is standard practice, it might be nice to add it to the docs. I don't deal with custom security managers since I happen to work for Preset, and this is all a solved problem. Let us know if you see that part of the logic flow in your fork, and do our best to help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validation:required A committer should validate the issue
Projects
None yet
Development

No branches or pull requests

9 participants