nextcloud / impersonate

👻 Allow administrators to become a different user
https://apps.nextcloud.com/apps/impersonate
GNU Affero General Public License v3.0
56 stars 15 forks source link

Inform impersonated user about impersonation #80

Open wiswedel opened 5 years ago

wiswedel commented 5 years ago

Description

The impersonated user should get informed about any performed impersonation.

Details

Location

Motivation

Even though the impersonation is logged in the Nextcloud log, the impersonated user has no idea someone acted in their name. Too much harm could be done by a bad admin without anyone noticing it for quite some time.

wiswedel commented 5 years ago

also related: https://github.com/nextcloud/impersonate/issues/50

beccon4 commented 5 years ago

also related to #17

As it is now, Impersonate looks like a big hole in the wall.

cagdasbas commented 4 years ago

Hi everyone. I created a working version of notification and email #84 . No activity stream for now. It is working but not very clean. I can update it with your feedback

virtualdxs commented 4 years ago

IMO this is something that should be configurable, if not from the gui at least in config.php or something. There's plenty of legitimate reasons why an admin would want to impersonate without notifying.

szaimen commented 4 years ago

I don't think that this should be configurable because then it wouldn't make any sense to have this notification feature at all.

@virtualdxs please tell me just one good reason why you should be able to disable these notifications.

virtualdxs commented 4 years ago

Better close source the whole project then so admins can't build their own copy and disable the feature.

One good reason: Gathering evidence from an employee who's about to be fired without giving them a chance to delete it or heads up that they're being investigated.

Fundamentally, an admin should have full control of their systems and be able to do what they want with them.

virtualdxs commented 4 years ago

Note that this doesn't make the feature completely useless - Only the highest level admins, not necessarily all with impersonate privileges, could disable it.

cagdasbas commented 4 years ago

How about adding a switch to config.php instead to settings. This way, the admin can't easily switch on and off and think twice before restarting the app for a config change?

virtualdxs commented 4 years ago

That sounds reasonable to me. Ideally, I'd like the switch to be whether to allow this to be configured, not whether to notify. That way, high level admins could impersonate without notification while lower levels do still trigger notifications (as an example).

TheManchineel commented 4 years ago

If I say my server must not do x, then it must NOT do x. Users will be informed of what I'm able to do with the data on my private server and they can choose whether to use it or not.
Forcing this "feature" turns Nextcloud into a piece of software that restricts an admin's control over their own property. It's also completely useless as anybody can modify the source and remove it on their server. It additionally gives users a "false sense of security" in the sense that they take for granted that their data can't be accessed without them being notified. If you own the server you can already do everything. Adding this bit of inconvenience to the person who sets up and hosts the application is ridiculous.

wiswedel commented 4 years ago

Thank you for your concerns, @TheManchineel (even if your language could have been slightly more diplomatic for my taste).

One good thing about Nextcloud is its scalability - in terms of user count but also in terms of target audience. The almighty admin picture might fit for the private small-server use case. However, in a GDPR-bound corporate use case you may need some more transparency when it comes to user data access, since data security is one of the core principles why people choose using Nextcloud in the first place. Please feel invited to contribute constructive ideas.

TheManchineel commented 4 years ago

@wiswedel Sure, having this as an option one can enable (especially if you're using Nextcloud and fall under a case where GDPR applies such as using the server in an enterprise environment, or basically anything other than a small private install, as you mentioned) would be great to have. I do think we should be transparent towards users, in fact, I believe such a feature would greatly discourage abuse of impersonation features, especially when it comes to Nextcloud Chat, Email and Calendar applications, more than file access - considering files are already there on the FS, while access to these applications allow you to act on a user's behalf. This goes in tandem with a need for a redesigned group admin feature that allows limiting the group admin's ability to management of a user's membership to a group and deletion of the group.

However, with this said, I do believe it should be up to the server's physical administrator (as in, someone who can write to config.php) to choose whether to enable notifications for the impersonate plugin or not. It would make sense to have this enabled by default for GDPR compliance, and to warn an administrator, before they click on "Impersonate", that the user will be warned of the action. Even better, config.php could have the option to disable this feature for a specific user or admin group, as has been suggested by @virtualdxs.

0-wHiTeHand-0 commented 3 weeks ago

Hi, this is just only a workaround (I do agree that "impersonate" needs this feature by itself), but the Passwords application already does it if the last option in the 'notifications' area is enabled: Image

nursoda commented 3 weeks ago

Just in case other people also want to check this out: @0-wHiTeHand-0 is talking about the installed-and-active additional app "Passwords", and its settings WITHIN the app, like /apps/passwords/#/settings – not the Settings section, like /settings/user/notifications or /settings/admin/notifications. Personally, I find it rather weird that the passwords app does inform about Impersonation, but this would be a workaround if it worked for non-admin users, too, To my findings, it doesn't. Those users only have these options there: Image

0-wHiTeHand-0 commented 2 weeks ago

Thanks for your clarifications @nursoda; in my NC instance non-admin users receive a push notification when I impersonate them, which is triggered by the Passwords app. I just wanted to share it here because it's a workaround (not the best one, i know) for this issue, at least in my case. And also, I think I was wrong regarding the Notifications option; it's not the Admin Issues checkbox. It could be the "Security issues" one but haven't checked.

nursoda commented 2 weeks ago

I checked and that option already was checked. However, it does not work for me on NC30. Any idea why your users do get the notification email and I don't?

0-wHiTeHand-0 commented 2 weeks ago

It's a push notification, not an email. And it seems it's triggered only when the admin opens the user's Passwords app:

https://git.mdns.eu/nextcloud/passwords/-/wikis/Users/F.A.Q#the-notification-administrative-access-to-your-account

My users see it a lot because I have a script performing some admin stuff in everyone's Passwords app using Impersonate, but definitively it isn't a workaround for this issue.