search

the-djmaze / snappymail

Simple, modern & fast web-based email client
https://snappymail.eu
GNU Affero General Public License v3.0
1.01k stars 121 forks source link

[Nextcloud] Impersonate user keeps old Email logged in #561

Closed relikd closed 1 year ago

relikd commented 1 year ago

Describe the bug I cannot assess whether this is a security issue or just inconvenience.

When I impersonate another user (plugin) my current auto-login account is still usable within that new user context. This was not the case for Rainloop.

To Reproduce Steps to reproduce the behavior:

  1. With Nextcloud account A, setup a new email account B and configure auto-login / remember password.
  2. Goto Users, select another user, and click "impersonate"
  3. Click on the SnappyMail Tab

Expected behavior Snappy should display the login mask as it have no E-Mail auto-login configured.

Actual behavior Instead, Snappy uses the login credentials of email account B and displays all emails. I am not sure if the login information is only restored in my browser session or whether the credentials are somehow transfered to the user. Aka. if I accidentially leak my admin login to another user simply by impersonating him/her.

Please complete the following information:

the-djmaze commented 1 year ago

There's one important thing to know: were you already logged in SnappyMail?

Probably, because Nextcloud search also tries SnappyMail and that auto-login to search.

So we need to know if we can hook to the imporsonate plugin and force a logout

relikd commented 1 year ago

Yes, as soon as I set auto-long credential, SnappyMail will always be logged-in. I would rather not hook to other plugins because you can never know if another plugin will also cause the same behavior. As long as I know that my credentials are not leaked to the impersonated user it is fine for me (and only a small inconvenience).

Another thing I noticed – maybe related to the topic – when I logged out of Rainloop it would display the login screen. With SnappyMail, if I click logout, it will immediately log back in. You said something about Search triggering the auto-login? Could this be postponed until you actually perform a search?

the-djmaze commented 1 year ago

Could this be postponed until you actually perform a search?

When i open Nextcloud a search is done immediately, it might be a bug. So i just made a workaround for that.

With SnappyMail, if I click logout, it will immediately log back in.

That's true because you've setup the login credentials.

relikd commented 1 year ago

That's true because you've setup the login credentials.

Yes, but the logout did work in Rainloop with auto-login enabled. SnappyMail just not displays the login view and instead performs the auto-login. I am actually fine with that behavior. It was just a note on the login/logout process in general. Maybe a hint that could have helped to identify the issue why the login credentials are transferred to the impersonated user.

So we need to know if we can hook to the imporsonate plugin and force a logout

As I said, I would rather not hook to other plugins. But maybe, instead of hooking to the imporsonate plugin, there is a more general hook to subscribe to changes in user-context? Ideally getting notified if the current user changes?

corvus-albus commented 1 year ago

The described behaviour can be observed more general. SnappyMail shows user B the mail account of user A, if user B logs in right after user A.

To Reproduce: We have installed SnappyMail as Nextcloud App. It is configured with "Users will login manually, ...". Log in as user A without activating "Remember login". Switch to SnappyMail and log in as User A: E-Mails of user A show up. Now log out from nextcloud (and not from SnappyMail) and log in to nextcloud as user B in the very same browser session. Switch to SnappyMail: SnappyMail loads straight away the mail-account of user A. (User B has no "Remember login" as well)

I think this shouldn't happen. User A should be logged out from SnappyMail as logging out from nextcloud. Cheers corvus-albus (nextcloud 24.0.6, SnappyMail 2.19.2)

the-djmaze commented 1 year ago

Now log out from nextcloud (and not from SnappyMail) and log in to nextcloud as user B in the very same browser session. Switch to SnappyMail: SnappyMail loads straight away the mail-account of user A.

Strange, because SnappyMail should logout due to the following Nextcloud hook: https://github.com/the-djmaze/snappymail/blob/62a14a3ab0a931b34d995ffec9108dfab010d3da/integrations/nextcloud/snappymail/lib/AppInfo/Application.php#L98-L102

So something goes wrong in Nextcloud OR SnappyMail hook OR browser cache

relikd commented 1 year ago

Just a shot in the dark: is there some local storage involved, such that the credentials are loaded from disk?

corvus-albus commented 1 year ago

I tried this on an ubuntu-desktop installation. I deleted the near history and never save passwords in the browsers password manager. I've just gone through the steps in private mode of firefox - same outcome. I'm not an expert but I strongly guess that there is nothing stored on the local disk (outside the browsers cache).

the-djmaze commented 1 year ago

Found it. The impersonate extension does not logout. It just changes the session data. https://github.com/nextcloud/impersonate/blob/38d1054de3e22e45addc646300cbe46d7b3ac5b4/lib/Controller/SettingsController.php#L157

That's why SnappyMail does not logout.

corvus-albus commented 1 year ago

If I understand it the right way, than you found a solution for the originally mentioned problem. As my observation has nothing to do with the impersonate extension: Shall I open a new issue? (By the way: I reproduced my observations with a clean VM-Installation of Ubuntu.)

the-djmaze commented 1 year ago

No solution yet, just found out why SnappyMail stays logged in. It's because the impersonate plugin does not logout.

corvus-albus commented 1 year ago

Sorry for being back again ;-). I tried with another nextcloud instance. To observe the described behaviour at the second instance it was essential to tick "Remember login". On of the main differences between the installations: The first works with ldap, the users email adress follows the pattern @server.de. The second has only handmade logins, email adresses are not derived from the login name. On both installation I deinstalled impersonate plugin without any effect. Cheers corvus-albus

the-djmaze commented 1 year ago

WITHOUT impersonate plugin AND NO credentials stored in Nextcloud -> Settings -> Additional:

  1. Login Nextcloud
  2. Login SnappyMail without "Remember login"
  3. Logout Nextcloud
  4. Login Nextcloud
  5. Click SnappyMail icon
  6. Are you logged in? If so, with who?

WITHOUT impersonate plugin AND NO credentials stored in Nextcloud -> Settings -> Additional:

  1. Login Nextcloud
  2. Login SnappyMail with "Remember login"
  3. Logout Nextcloud
  4. Login Nextcloud
  5. Click SnappyMail icon
  6. Are you logged in? If so, with who?

WITHOUT impersonate plugin but WITH credentials stored in Nextcloud -> Settings -> Additional:

  1. Login Nextcloud
  2. Logout Nextcloud
  3. Login Nextcloud
  4. Click SnappyMail icon
  5. Are you logged in? If so, with who?

And then same with impersonate, what happens?

relikd commented 1 year ago

WITH impersonate plugin AND NO credentials stored AND remember=False:

WITH impersonate plugin AND NO credentials stored AND remember=True:

WITH impersonate plugin AND credentials stored:

EDIT: and exact same behavior without impersonate plugin

the-djmaze commented 1 year ago

@relikd Go to Nextcloud -> Settings -> Administration -> Additional settings

Set it to Users will login manually, or define credentials in their personal settings for automatic logins.

I've also asked without the impersonate plugin because that is the base to see how it works. If everything works as expected then try the impersonate plugin.

relikd commented 1 year ago

With v2.19.2? That is what I tried. I also tried with the impersonate plugin disabled (I shouldn't have edited the message and instead posted a new reply, my bad)

corvus-albus commented 1 year ago

As I mentioned previously I tested on two different nextcloud installations. The non-ldap installation shows the same problematic behaviour but under more restrictiv conditions. The emailaccounts for the non-ldap installation are not connected to the usernames, for the ldap-installation the account have the pattern: \<username>@nc-server.de.

I'm not ready with testing all the 6 situation but found a hint while analyzing the first situation. WITHOUT impersonate plugin AND NO credentials stored in Nextcloud -> Settings -> Additional: Login SnappyMail without "Remember login" Are you logged in? If so, with who? no-ldap installation: Not logged in ldap installation: logged in as user A. ldap installation: deleting cookies in firefox - not logged in

In fact deleting the cookie did prevent SnappyMail from being logged in false for user B. I had a closer look at the cookies: the value for smaccount did not change. Deleting only the entry smaccount was enough to prevent SnappyMail from being logged in.

I#m going to test the other situations I will report.

relikd commented 1 year ago

In fact deleting the cookie did prevent SnappyMail from being logged in

As I said, probably a local storage issue.

But I think the actual issue is to know when to trigger the logout aka. delete the cookie. It seems that listening to the logout hook is not enough. The credentials also change if the user session changes (as the-djmaze commented above). I wonder whether Nextcloud has a hook for the session-change event...

Also, it seems strange that on my machine, without impersonate, no credentials, no remember login, and no ldap, I am still being logged in again. Even the logout hook seems unreliable.

corvus-albus commented 1 year ago

Testreport in one post:

WITHOUT impersonate plugin AND NO credentials stored in Nextcloud -> Settings -> Additional:

WITHOUT impersonate plugin AND NO credentials stored in Nextcloud -> Settings -> Additional:

WITHOUT impersonate plugin but WITH credentials stored in Nextcloud -> Settings -> Additional:

WITH impersonate It's all the same, impersonate extension seems to have no effect like relikd reported above.

relikd commented 1 year ago

It's all the same, impersonate extension seems to have no effect like relikd reported above.

The only difference being that using impersonate you can change an account without logout. The underlying issue is more fundamental ...

the-djmaze commented 1 year ago

I made a check to force logout. Try with v2.19.6

corvus-albus commented 1 year ago

Cannot update within Nextcloud: "App with id snappymail has invalid signature". It's working now. Perhaps I was too fast ...

corvus-albus commented 1 year ago

Thank you für adressing the bug. The problem is solved, after switching user I'm no longer logged in as the previous user in snappymail. But ... with v2.19.6 I can see no emails at all after I logged in snappymail! As I want to send one, a window pops up and I have to choose wether I will stay on the page or leave it. The email won't be send with "authentification failed". The solution goes a little too far I suppose ...

the-djmaze commented 1 year ago

Great we have progress! I now know what to do about this issue because it now keeps logging you out when impersonate

relikd commented 1 year ago

Can confirm. Does work for me too. Many thanks!

But ... with v2.19.6 I can see no emails at all after I logged in snappymail! As I want to send one, a window pops up and I have to choose wether I will stay on the page or leave it. The email won't be send with "authentification failed".

This, I do not experience. I can see mails, and I can send them. I do not know why it is not working for you. Maybe a caching issue?

relikd commented 1 year ago

But I believe this broke the admin panel. I cannot open it anymore, it redirects to the "normal" email overview. ... missing /run/ in url?

relikd commented 1 year ago

v2.19.7 undid the auto-logout changes. Now it keeps the old account active even after logout and impersonate

the-djmaze commented 1 year ago

Good to know, now i know exactly what happens in Nextcloud.

I will make a solution for it.

the-djmaze commented 1 year ago

The code did ocSession->set('snappymail-uid', 'Nextcloud user id') And it checks the value if the value changed. But.... ocSession keeps returning NULL as if the value was never set :angry:

So now, i just write it to a cookie myself

relikd commented 1 year ago

uff, storing cookies manually sounds like a security nightmare. Isn't a set operation supposed to return null? I mean, what else should it return? Does it return the correct value if you get() the new value? .... why do you even need to write it to a cookie, wasnt the problem to clear a previous authentication? Dont take my advise too seriously, I am probably missing things to evaluate the situation properly, just my two cents.

the-djmaze commented 1 year ago

The cookie only contains your uid, but indeed it's not a great solution. I have no idea why Nextcloud session doesn't work properly.

If someone with Nextcloud expertise maybe can solve this.

relikd commented 1 year ago

I found there is also a getUserSession in contrast to getSession, maybe that is helpful?

$uid = \OC::$server->getUserSession()->getUser()->getUID();

Looking at the code of @pierre-alain-b ... but I cannot assess how helpful that is for you.

https://github.com/pierre-alain-b/rainloop-nextcloud/blob/e424dbd8f854024663a6fdcbb70f34b1c2a5b32c/lib/Util/RainLoopHelper.php#L29-L31

$this->userSession->listen('\OC\User', 'logout', function() {
    $this->logout($this->appManager, $this->config, $this->session);
});

https://github.com/pierre-alain-b/rainloop-nextcloud/blob/4de6909ad2d3c1b994de55607a7d30d1e53dc381/app/rainloop/v/1.16.0/app/libraries/RainLoop/Api.php#L259-L263

public static function LogoutCurrentLogginedUser() {
    \RainLoop\Utils::ClearCookie('rlsession');
    return true;
}

https://github.com/pierre-alain-b/rainloop-nextcloud/blob/4de6909ad2d3c1b994de55607a7d30d1e53dc381/app/rainloop/v/1.16.0/app/libraries/RainLoop/Actions.php#L614-L618

public function SetAuthLogoutToken() {
    @\header('X-RainLoop-Action: Logout');
    \RainLoop\Utils::SetCookie(self::AUTH_SPEC_LOGOUT_TOKEN_KEY, \md5(APP_START_TIME), 0);
}
the-djmaze commented 1 year ago

Thanks for the help but the approach is the same.

I've reported the issue to Nextcloud https://github.com/nextcloud/server/issues/34935

the-djmaze commented 1 year ago

I've disabled all checks and this issue will be back until Nextcloud and/or Impersonate app solve the issues.

the-djmaze commented 1 year ago

Released v2.20.0 and the impersonate checks are disabled.

When you use impersonate plugin:

  1. Disable the autologin with Nextcloud username/email
  2. When impersonating:
    • Open SnappyMail account menu
    • Click logout yourself
    • Now login
relikd commented 1 year ago

I've read over the NC thread, does this also affect the logout/login behavior or can we have a version where at least the logout is triggered on NC logout? The problem with the Impersonate plugin has lower priority as I can just take precautions to not leak the login information (always open Snappy and click logout with the impersonated user)

the-djmaze commented 1 year ago

@relikd i've reported to Impersonate app with an optional solution

the-djmaze commented 1 year ago

I've read over the NC thread, does this also affect the logout/login behavior or can we have a version where at least the logout is triggered on NC logout?

While creating a PR for the Impersonate app, i found out about the NC18+ change. So i've modified the old RainLoop code to use the new IEventDispatcher. Maybe that will improve the logout handling

the-djmaze commented 1 year ago

https://github.com/nextcloud/impersonate/pull/180