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

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

Closed
relikd opened this issue Oct 19, 2022 · 39 comments
Closed

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

relikd opened this issue Oct 19, 2022 · 39 comments
Labels
external Issue is outside our code Nextcloud

Comments

@relikd
Copy link

relikd commented Oct 19, 2022

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:

  • SnappyMail Version: 2.19.1
@the-djmaze
Copy link
Owner

the-djmaze commented Oct 19, 2022

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
Copy link
Author

relikd commented Oct 19, 2022

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
Copy link
Owner

the-djmaze commented Oct 19, 2022

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
Copy link
Author

relikd commented Oct 19, 2022

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
Copy link

corvus-albus commented Oct 21, 2022

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
Copy link
Owner

the-djmaze commented Oct 21, 2022

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:

$userSession->listen('\OC\User', 'logout', function($user) {
$oSession = \OC::$server->getSession()['snappymail-password'] = '';
SnappyMailHelper::loadApp();
\RainLoop\Api::LogoutCurrentLogginedUser();
});

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

@relikd
Copy link
Author

relikd commented Oct 21, 2022

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

@corvus-albus
Copy link

corvus-albus commented Oct 21, 2022

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
Copy link
Owner

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
Copy link

corvus-albus commented Oct 22, 2022

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
Copy link
Owner

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

@corvus-albus
Copy link

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
Copy link
Owner

the-djmaze commented Oct 24, 2022

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
Copy link
Author

relikd commented Oct 24, 2022

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

  • after nextcloud login the email account is also logged in.

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

  • no need to test, as it was already logged in even without "remember me"

WITH impersonate plugin AND credentials stored:

  • also logged in with email (following the login, logout, login procedure)

EDIT: and exact same behavior without impersonate plugin

@the-djmaze
Copy link
Owner

the-djmaze commented Oct 24, 2022

@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
Copy link
Author

relikd commented Oct 24, 2022

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
Copy link

corvus-albus commented Oct 25, 2022

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
Copy link
Author

relikd commented Oct 25, 2022

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
Copy link

corvus-albus commented Oct 25, 2022

Testreport in one post:

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

  • Login SnappyMail without "Remember login"
  • Are you logged in? If so, with who?
    • ldap: logged in as user A.
    • ldap: deleting cookies in firefox - not logged in (deleting smaccount is sufficient)
    • non-ldap: Not logged in (value of smaccount seems to be changed as SnappyMail starts for user B, no clue why.)

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

  • Login SnappyMail with "Remember login"
  • Are you logged in? If so, with who?
    • ldap: logged in as user A.
    • ldap: deleting the entries smaccount AND smremember: not logged in.
    • non-ldap: same as ldap

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

  • No SnappyMail on first login
  • Are you logged in? If so, with who?
    • ldap: SnappyMail with user B logged in
    • non-ldap: couldn't test because of troubles with installations.

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

@relikd
Copy link
Author

relikd commented Oct 25, 2022

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
Copy link
Owner

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

@corvus-albus
Copy link

corvus-albus commented Oct 31, 2022

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

@corvus-albus
Copy link

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
Copy link
Owner

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

@relikd
Copy link
Author

relikd commented Nov 1, 2022

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
Copy link
Author

relikd commented Nov 1, 2022

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
Copy link
Author

relikd commented Nov 2, 2022

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

@the-djmaze
Copy link
Owner

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

I will make a solution for it.

@the-djmaze
Copy link
Owner

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 😠

So now, i just write it to a cookie myself

@relikd
Copy link
Author

relikd commented Nov 2, 2022

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
Copy link
Owner

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
Copy link
Author

relikd commented Nov 3, 2022

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
Copy link
Owner

Thanks for the help but the approach is the same.

I've reported the issue to Nextcloud
nextcloud/server#34935

the-djmaze pushed a commit that referenced this issue Nov 3, 2022
@the-djmaze
Copy link
Owner

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

@the-djmaze the-djmaze reopened this Nov 3, 2022
@the-djmaze the-djmaze added the external Issue is outside our code label Nov 3, 2022
@the-djmaze
Copy link
Owner

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
Copy link
Author

relikd commented Nov 3, 2022

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
Copy link
Owner

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

@the-djmaze
Copy link
Owner

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 the-djmaze changed the title [Nextcloud] Switching user keeps old Email logged in [Nextcloud] Impersonate user keeps old Email logged in Nov 9, 2022
@the-djmaze
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external Issue is outside our code Nextcloud
Projects
None yet
Development

No branches or pull requests

3 participants