gary-kim / riotchat

Element for Nextcloud
https://apps.nextcloud.com/apps/riotchat
GNU Affero General Public License v3.0
123 stars 22 forks source link

Log Out when Nextcloud logs out #380

Open jonathanmmm opened 3 years ago

jonathanmmm commented 3 years ago

Hi,

right now I am using both nextcloud and this app via OIDC (OpenID Connect), the iframe problem is still a problem, don't know why, put the frame-src into keycloak and nginx, but didn't work.

But the more problematic thing is, that synapse doesn't automatically logs sessions out, so when I log out with the nextcloud account and somebody else goes onto the computer and into the browser and logs into with their nextcloud account they are logged in into my messages. Is it possible to send a logout message to synapse/matrix when nextcloud is logged out?

This feature request tries the complete opposite, so maybe this should be a toggle (on by default to log you out, as it is much more secure). https://github.com/gary-kim/riotchat/issues/114

gary-kim commented 3 years ago

That's strange, when a user logs out of Nextcloud, their session data should be deleted.

https://github.com/nextcloud/server/blob/71109b74259fdc5ec78e278d1674cf314c414ae1/core/src/login.js#L35-L44

gary-kim commented 3 years ago

A possible issue might be that indexedDB data is not cleared. Let's see if that can be cleared on logout

jonathanmmm commented 3 years ago

Can I help you somehow to debug this?

jonathanmmm commented 3 years ago

Maybe it is about cookies that are not deleted, I logged out and 9 cookies left on nextcloud, maybe it is because the website tries OIDC in iFrame and it fails and opens the full frame website. If I then go in to a new tab or back and log out I can't access it initially, but logging in with whatever account into nextcloud gives me access to the chats of the logged in person.

This means: If I log in on the computer of some college (if used in a company) and log out again and he logs into his nextcloud account he can see my chats and write as he was me. This is serious, as you think, logging out means logging out. And nobody will find this by their own, as they only log in with their own account.

gary-kim commented 3 years ago

What I'm thinking is we can set up an extra script loaded on logout that also deletes indexedDB data. That is where Element stores the access token.

jonathanmmm commented 3 years ago

Maybe, could you reproduce this security bug? If I can assist, let me know, I could give you a dummy account on my server.

Also does this version of Element deletes the encrytion key for E2EE chats, are these somehow saved in nextcloud? If yesy the E2EE would be broken (aa the server has the keys).

gary-kim commented 3 years ago

E2EE keys are kept in indexedDB storage on the client and are never sent to the Nextcloud server.

Feel free to reopen the issue if #383 hasn't fixed your issue.

jonathanmmm commented 3 years ago

@gary-kim Sadly it didn't fix the issue. I first thought as I am using OpenID Connect and he makes the website fullscreen (after first login doesn't work), that it is about the cookies the browser saved from matrix.my-domain.com (that got saved) instead of nextcloud.my-domain.com but after logout, closing, new window, logging in with another account I am logged in (in frame) with the first account.

After login again I have the following cookes, all for nextcloud.my-domain.com: Cookies for matrix.my-domain.com where gone.

__Host-nc_sameSiteCookielax: true
__Host-nc_sameSiteCookiestrict: true 
ocSomeString: someString
oc_session_passphrase: someString
one entry for indexed database nextcloud.my-domain.com
one entry for local storage nextcloud.my-domain.com
same for service worker and session storage (Sitzungsspeicher).

I can send you two credentials for test accounts at my server and you can try. Or what information would be useful to debug it?

jonathanmmm commented 3 years ago

I am doing this whole thing in icognito mode in the new Edge Browser with moderate tracker settings (default). I closed the icognito windows and opened and logged into both accounts (and closing after each) and was not logged into element anymore (synapse may still think the session exists, but I was greeted with the login screen of element), so it seems to be something that is saved in the browser and not on the server.

gary-kim commented 3 years ago

Okay, a few things have to be made clear, cookies are not used for Matrix anywhere outside of the SSO specification. Only the Matrix access token is used to authenticate requests and that is stored in indexedDB. Your list seems to specify that there is an indexedDB entry left for Nextcloud which should not exist.

With the latest update, I can confirm that with both Chromium and Firefox, using the Nextcloud logout button to log out of Nextcloud will log the user out of both Nextcloud and Element. Are you having a more specific issue?

Yes, the session should be left over on the Synapse side. It would require more code to delete the session properly and that does not exist at the moment.

jonathanmmm commented 3 years ago

I tried in Chrome also and I tried it there.

The following I do: Login via SSO (Keycloak) into nextcloud with user1. Go to Element Tab on the top, press login via sso, it fails and redirects me to the full page, not iframe (but still nextcloud domain). I can no press login via sso again and I am in. Then I go back to the normal nextcloud page (as in full page there is no nextcloud logout button or tope frame). I then logout and login with user2. User2 goes on top to Element and is logged in into Synapse from user1.

I am using docker nextcloud-fpm, so your application lands in custom_apps folder. I am running Nextcloud 21, mariadb, redis, can send the docker-compose file, tried with 0.8.1 and also with 0.8.2 (even if only for nextcloud 19, it seems this update is given to everybody, but then there is written uncompatible).

Is there any way I can help debug this?

jonathanmmm commented 3 years ago

I have an idea, don't know if it will work:

https://matrix.org/docs/spec/client_server/latest#post-matrix-client-r0-logout

5.5.3 describes a logout. If nextclouds logout is triggered and we could send to the matrix server a logout signal this should solve this problem (if all databases are deleted in nextcloud, like you merged) and even deletes this device from the list of devices for this user in matrix.

@gary-kim what do you think about this?

gary-kim commented 3 years ago

The how to log out has never been the issue. The issue is that Element sometimes stores the access token encrypted and the access token is needed to log out. Now, the pickle key is stored next to the access token and Element is open source so we can just use their decryption code but Webpack tree shaking doesn't do a good job of reducing the file size of that so it ends up being something like a 30MB script to just decrypt the access token. We could also reimplement the decryption but that would break again as soon as Element changes anything about it.

Basically, for now, the priority is to ensure that the account cannot be accessed after logging out from Nextcloud. Making sure it is logged out on the Matrix Homeserver side would be nice as well but I personally have neither a plan or the time to implement that at the moment. Of course, always open for PRs.

jonathanmmm commented 3 years ago

@gary-kim I didn't know this was that complicated, I thought we just send a command like

POST /_matrix/client/r0/logout HTTP/1.1 with the appropiate auth token in the header or so.

I found out, that there is in cookies a folder called 'Local Storage' (Lokaler Speicher), in this folder is an icon for a database with the name displayed from the url of nextcloud (with https://nextcloud.my-domain.com). If I delete this manually I get logged out, so it seems somehow it doesn't delete this database at logout at my browser (trying right now with Chrome).

I tried with an extension diabling Content Security Policy (don't know form where right now the frame-src errors are coming from, I actually allowed the SSO and synapse urls to be frame-src and frame-ancestors and also nextcloud, still errors. Maybe it is about this framing or it is about nextcloud in docker. I will try it without docker, maybe it works that way. Is there an easy way to disable this 'detect sso and use the full frame version' so I can check, if non iframe is the problem?

jonathanmmm commented 3 years ago

Ok, so if I log in via a normal account or via an account through LDAP seems to work. Of course I have to disable the SSO session manually as it is not deleted if I logout of nextcloud. So maybe it is a problem with https://github.com/pulsejet/nextcloud-oidc-login

 'oidc_login_provider_url' => 'https://keycloak.my-domain.com/auth/realms/my-domain',
  'oidc_login_client_id' => 'nextcloud-openid-connect',
  'oidc_login_client_secret' => 'somestring',
  'oidc_login_logout_url' => 'https://keycloak.my-domain.com/auth/realms/my-domain/protocol/openid-connect/logout?redirect_uri=https%3A%2F%2Fnextcloud.my-domain.com',
  'oidc_login_hide_password_form' => true,
  'oidc_login_proxy_ldap' => true,
  'oidc_login_auto_redirect' => false,
  'oidc_login_button_text' => 'OpenID Connect Login',
  'oidc_login_attributes' =>
  array (
    'id' => 'sub',
    'name' => '${given_name} ${family_name}',
    'mail' => 'email',
    'ldap_uid' => 'preferred_username',
  ),
  'oidc_login_scope' => 'openid profile',
  'oidc_login_disable_registration' => true,

Maybe this plugin somehow is in conflict with this. This plugin only uses SSO to authenticate, data is retrieved via LDAP. I tried without retriebing data from LDAP. I will link this issue, as it seems to only happen when logging in via this app.

jonathanmmm commented 3 years ago

@gary-kim The logout.js is never used, when logging out via oidc. I put console.log("Database deleted successfully") in this file and only when logging out via LDAP I get this message in the console, so it seems to be an openid connect issue or something or this script has to somewere mentioned?

Also, it seems to be an issue with: 'oidc_login_logout_url', if I comment it out, it gets deleted, but I the SSO session is not revoked, so still not working perfectly.

gary-kim commented 3 years ago

Did you run npm run build? The way the script runs is pretty much the same as the standard cookie and local storage delete that Nextcloud already has.

jonathanmmm commented 3 years ago

No, I just nano into the file and put after all the deletion this console.log command. And when logging out via LDAP I can see this command, so it seems to be that while the Nextcloud user is logged in via oidc that this script isn't called.

Somehow oidc and riotchat does not seem to work with each other. Seems to be more an oidc problem and not yours, but I don't know exactly.

I know that the problem seems to be with the redirect url, somehow it doesn't trigger your script.

You can see more in the issue I linked.

jonathanmmm commented 2 years ago

https://github.com/pulsejet/nextcloud-oidc-login/pull/164 fixed this, this was a problem in this oidc plugin, nothing to do with riotchat. But still riotchat is still logged in (meaning, synapse thinks the device is logged in, can it see under logged in devices, this could probably fill up the list of logged in devices after a while).

jonathanmmm commented 2 years ago

@gary-kim should we open for that another issue / feature request? Would be probably better, right? The issue about security is fixed, now just clogging up the list of "logged in devices".

WaaromZoMoeilijk commented 3 months ago

I can confirm that loggin out on Nextcloud and loggin back yields the previous account on the app still being logged in. If there's anything I can help with to get some momentum going on this, please let me know.

gary-kim commented 3 months ago

On Wed Jun 5, 2024 at 11:52 AM EDT, WaaromZoMoeilijk wrote:

I can confirm that loggin out on Nextcloud and loggin back yields the previous account on the app still being logged in. If there's anything I can help with to get some momentum going on this, please let me know.

Anyone is welcome to make a pull request or send a patch (even with git-send-email, if you prefer) fixing any issues.

I, unfortunately, cannot reproduce this issue (I just tried on my install) without more information.