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

Feature Request: User Group Admin Privileges [$5] #3362

Closed
neebski opened this issue May 15, 2013 · 27 comments
Closed

Feature Request: User Group Admin Privileges [$5] #3362

neebski opened this issue May 15, 2013 · 27 comments

Comments

@neebski
Copy link

neebski commented May 15, 2013

I would like to see the ability to fine tune what group admins can actually do. For example, if I create an Accounting or Engineering group admin I would like to disable their ability to delete users. Possibly only granting Gadmins the ability to reset passwords, add local shared folders, etc.

I searched a bit here on git but didn't see anything. Just a thought, thanks for all you guys do!

@MorrisJobke
Copy link
Contributor

cc @raghunayyar

@DeepDiver1975 DeepDiver1975 modified the milestone: backlog Mar 21, 2015
@RobinMcCorkell
Copy link
Member

Merging several duplicate issues into this one:

#1212 Group Admin: Should be able to delete from group, not from system

Today, a group admin can add new users within their group. That works well.

But, the user can delete ANY user in their group from the system completely. That should not be. There should be a configurable option set by the overall admin where the group admin can/cannot delete users from the system when deleting from the group. Also, a group admin should never be able to delete a user from the system that is a member of another group. These use cases should help explain:

Example 1: user 1, user 2 and user 3 are added by the overall admin. user 1 is the group admin of group 1. user 2 and 3 are also members of this group. user 3 is also a member of group 2. user 1 deletes user 3 from group 1, but user 3 should still be a member of group 2, and the group 1 admin should not be able to delete user 3 from the system, only from their group. This is regardless of any configurable options mentioned above.

Example 2: user 1, user 2 and user 3 are added by the overall admin, and made members of group 1. user 1 is the group admin of group 1. The admin does not grant user 1 the right to delete users from the system when deleted from the group. user 1 removes user 2 from the group with a delete. user 2 is still a member of the system, but is no longer a member of any group. The overall admin can then delete the user if necessary, or assign other groups.

Example 3: user 1, user 2 and user 3 are added by the overall admin, and members of group 1. user 1 is the admin of group 1. The overall admin GRANTS user 1 the right to delete users from the system when deleted from the group. user 1 deletes user 2 from group 1, and since he is not a member of any other group, user 2 is deleted from the system.

Example 4: user 1 is the group admin of group 1. User 1 adds a new user to the system as a member of group 1. At a later time, user 1 can delete the added user from the group and possibly from the system following the configuration switch in Examples 2 and 3.

@RobinMcCorkell
Copy link
Member

#7992 Group admin can change storage size of self created user

There should be at least an option to disable the ability for group admin to set a users quota.

@MorrisJobke
Copy link
Contributor

I migrated the high label from #7992

@encendedor
Copy link

Any news here? The topic exists now for more than two years. I disabled all group admin permissions temporarily as a work-around...

@AsavarTzeth
Copy link

I too disabled/removed all group admins. I even found I had messed up the internal security of my system by being an "admin" and putting myself in a moderated group.

The result is the group admin could reset my password and take over everything... It seems obvious to me now, but less so back when I set things up. So watch out for miss-configurations like that one.

@tessus
Copy link

tessus commented Nov 3, 2015

I'd like to know that as well. I've opened #1212, and since then I haven't used group admins, because they are just too dangerous to use. I have the feeling that sometimes new features seem more important than fixing essential issues.

@DeepDiver1975
Copy link
Member

So many people who want this feature .... but nobody is stepping up to actually do something about it ...

... looks like one has to wait longer

@tessus
Copy link

tessus commented Nov 4, 2015

IMO this is not a feature, but a matter of basic group administration and design. You can call it whatever you want, but at the moment it is not useable unless you don't care about your users' data.

@karlitschek
Copy link
Contributor

@tessus This is an open source community project. Feel free to submit a pull request for this feature.

@tessus
Copy link

tessus commented Nov 4, 2015

@karlitschek I'd love to, but I'm a bit stressed for time. also, this group admin code is core code, which I don't know. I'd need help to learn and understand it, otherwise it would take me even more time.
So for now I cannot commit to coding any change.

Another problem is that the issues referenced here are not really the same problem. e.g. #1212 is different than the fine tuning issue described in the first comment of this issue.

As I have mentioned earlier, the fact that a group admin can delete a user (and the data) from the system, even, if that user belongs to another group and/or is a group admin as well, is highly unusual and dangerous. That's why I don't use group admins and neither does anyone I know who's running their own ownCloud installation.

ownCloud is a great project and I believe that some core devs understand the consequences of the current group admin implementation and will change it eventually.

@PVince81
Copy link
Contributor

If I understand well this is mainly about preventing subadmins to delete users from the system, at least as a first step.

Let's do a small step to move this forward design-wise.

The tasks would be as follows:

  • add a checkbox "subadmin_can_delete_user" in the admin page to configure subadmin's delete permission that defaults to true
  • modify the UsersController to disallow deletion of users if the user is not an admin (GroupManager->isAdmin($userId)) and the flag "subadmin_can_delete_user" is false
  • add a check in the JS code with the same checks (as was proposed in Quick fix to prevent group admin to delete an user #22793 but was missing the flag check)

@tessus
Copy link

tessus commented May 13, 2016

@PVince81 I think the situation is a bit more complicated. Deleting a user is fine, if certain pre-requisites have been met.
If I am a group admin and create a user, I should be able to delete the user as well. But only, iff from the time the user has been created, the user has not been added to another group or promoted to an admin.

Here is a list of checks to determine, if user A can delete user B:

  • A is a group admin for C
  • A created B and added B to group C
  • B does not belong to any other group than C

If above checks are all true, A can delete B.

@PVince81
Copy link
Contributor

Hmm, this would require tracking which user was created by who, which makes the data structure and logic much more complex. Also this makes it more difficult to understand for subadmins.

What do others think about this proposal ?
Are there other user management software that behave this way ?

@AsavarTzeth
Copy link

In my opinion "A created B and added B to group C" is irrelevant and also the cause of any complexity. For example the following scenario would unnecessarily break the system.

  • A second admin user D, (super admin or admin for another group) creates user B for group E (or no group at all).
  • The new user B is then later moved to group C.
  • Then user A cannot delete the user, even if it belongs only to group C, because "A created B and added B to group C" is false.

This makes no sense at all. Remove the second check is my simple solution.

The only thing that matters is what group, or groups the user currently belongs to. How to handle what a group admin can do in case the user belongs to several groups is a topic of by itself.

I don't remember, can group admins change which group you belong to? Obviously this would break any privilege system if you can simply own any user on the system.

@tessus
Copy link

tessus commented May 17, 2016

Ok, @AsavarTzeth raised a valid point, although (when looking at my example) when user B is moved to group D, so what? It still should not be possible that user B is deleted by the group admin (or any of the group admins) of D. But this is a philosophical discussion. I agree that tracking who created whom might be a pain in the neck.
I just want to make sure that a group admin cannot delete a user (and the data) from the system, even, if that user belongs to another group and/or is a group admin as well.

@interos99
Copy link

interos99 commented May 17, 2016

So, what if de group admin is always allowed to kick any user out of his group and, if desired to start a request to delete an user. so all the "delete requested" user will appear in a group oder something similar, which will make it easyer for admins to delete this users?
hope the understanding is possible... :-)

@AsavarTzeth
Copy link

AsavarTzeth commented May 20, 2016

@interos99 To simplify. Do you mean something like a user trashbin managed by the ownCloud admins?

@tessus I think using letters to describe both users and groups makes things a bit confusing. You now used D to describe a group. I assume this was intentional? It is slightly confusing since in my additions D was a user (admin of another group) and the group itself was E. Not important really, just wanted to point it out.

I have been thinking. To me this makes the most sense:

  1. A group admin wants to delete a user. This is only allowed uninhibited if that user is managed by that admin only (only in that one group).
  2. If the user is managed by several group admins (in more than one group) a super admin (ownCloud admin) has to handle it. Additionally you could implement a request for deletion system like @interos99 suggested (I think?), which would make this more manageable at scale.
  3. If the ownCloud admin(s) is uncomfortable with group admins deleting users entirely, you could make a server wide setting that will remove the delete user privilege entirely (or maybe per group?). In that case you could force the use of the request system mentioned in 2.

I tried to answer everyone's needs. What do you think? I encourage you to break it, of course.

@interos99
Copy link

@AsavarTzeth Yes, this is what I ment. In my cloud, used in an NGO with about 900 members, this would be really helpful. What do others think about this? And how much to work would this mean to realise?

@tessus
Copy link

tessus commented May 21, 2016

@AsavarTzeth I just extended my example and the last letter I used was C, so D was up. I should have used Gn and Un for groups and users, but I didn't know how to use subscript for the numbers n.

As I mentioned before, I'm happy with any solution that ensures a group admin cannot delete a user (and the data) from the system, if that user belongs to another group and/or is a group admin as well.

@PVince81
Copy link
Contributor

@interos99 mind telling us a little bit more about how you guys at the NGO are using subadmins ?

Somehow I start having the feeling that subadmin are being "misused" because ownCloud doesn't have any better way of achieving what you want.

Let's consider a new role "group admin" or "group membership admin". If we consider that groups are used like workgroups, you'd want the super-admin to create the said groups and specify someone who is able to add/remove people from their groups whenever they get added/removed to projects.
This means that the role is mostly about managing memberships and not emails/quotas/etc. So instead of having a "delete" feature it would only be "remove from my group".
Would that be more suitable ? 😄

In this case subadmins would be a completely different use case than group admins. (need to find better names for the roles)

@PVince81
Copy link
Contributor

Seems like there were other ideas related to "group management" but in an automatic fashion. Not directly related but still mentioning in case it can provide more inspiration to get closer to the actual use case you guys are thinking about: #8201

@PVince81
Copy link
Contributor

Someone suggested a new role "supervisor role", see #26723

As for "managing workgroups" where it's only about membership but not creating users, there's yet another variation here: #8201 (allowing users to create their own groups and add people there)

@DeepDiver1975 DeepDiver1975 changed the title Feature Request: User Group Admin Privileges Feature Request: User Group Admin Privileges [$5] Feb 7, 2017
@ghost ghost mentioned this issue Jul 20, 2017
@vasyugan
Copy link

I am really irritated that this very obvious issue hasn't been fixed yet.

This is a real risk of data loss. It happened to us recently, where a group admin did not understand that deleting a user really and completely deletes him. Fortunately we have backups, but this is a glaring issue. It really shoud be given priority!

@Lockorocko
Copy link

Still no fix or solution for this issue? :(
A group admin just wanted to remove a user out of a testgroup but deleted 30GB of data by mistake.
Most data was synchronized so we had a lot of luck .. Nevertheless it is a very dangerous privilege/issue and actually i unfortunately cant use the group admin function :(

@stale
Copy link

stale bot commented Sep 21, 2021

This issue has been automatically closed.

@stale stale bot closed this as completed Sep 21, 2021
@jasbroKBI
Copy link

jasbroKBI commented Jan 12, 2023

If anyone stumbles on this and wants a quick fix, just modify /var/www/owncloud/settings/templates/users/part.userlist.php to wrap the "remove" table cell with an isAdmin. I just needed to keep my group admins from seeing the delete button on their users.

                        <?php if ($_['isAdmin']): ?>
                          <td class="remove"></td>
                        <?php endif; ?>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests