authd icon indicating copy to clipboard operation
authd copied to clipboard

Support deletion of users and groups via authctl

Open adombeck opened this issue 1 year ago • 20 comments

There is currently no supported way to remove users and groups from the authd database. We want to create a command-line tool which allows doing that.

There are two issues when a user or group which still owns files on the filesystem is removed:

  1. When this user logs in again (or in the group case, a user who is a member of the group logs in), a new random UID/GID is generated, which means any existing files owned by the user/group won't be accessible to the user/group anymore.
  2. Whenever another authd user/group is added, the random UID/GID generated for that can by chance be the same as the one of the deleted user/group, allowing access to any existing files still owned by the deleted user/group.

The same is true when local users/groups are deleted via deluser/delgroup etc. There's an argument that it's worse in the authd case, because new users/groups are created without admin interaction, just by a new user logging in (unless the new device owner configuration is used, then admin interaction is actually required), so that's it's more surprising / less expected.

We want to make our users aware of that, so the tool should print a message and/or ask for confirmation when deleting a user/group. We should also support disabling a user instead of removing it, so that the user can't log in anymore but its UID is still reserved.

adombeck avatar Nov 14 '24 16:11 adombeck

Might there be an update regarding this tool? This would be very helpful. Would this tool have an option to forcefully update the user's entra groups or delete groups?

junebugin avatar Jan 13 '25 17:01 junebugin

Might there be an update regarding this tool?

We plan to implement it within the next few weeks.

Would this tool have an option to forcefully update the user's entra groups or delete groups?

The Entra groups should already be automatically updated whenever the user logs in (both via device authentication and local password). If you want to delete an Entra group, that should be done in Entra. What's your use case for wanting to do it locally?

adombeck avatar Jan 27 '25 11:01 adombeck

If you want to delete an Entra group, that should be done in Entra. What's your use case for wanting to do it locally?

Ah, actually, we don't delete the Entra groups locally when they are deleted in Entra (EDIT: They are deleted when a user is removed from a group and there are no other users in that group, but that requires all users to log in again after an Entra group was removed). So yes, that's probably also a valid use case for the command-line tool. I'll update the title and description accordingly.

adombeck avatar Jan 27 '25 12:01 adombeck

@didrocks I updated the description to mention the issues with removing users/groups from the database. Do you agree with the reasoning and the plan?

adombeck avatar Jan 27 '25 14:01 adombeck

@shiv-tyagi: Here is a short description of what I think needs to be done here.

Add new "disabled" field to user records

The user struct which is stored in the database needs a new field "disabled".

When a user tries to authenticate, the first thing we do is check if the user is disabled and return an error in that case. I think we can do that in the NewSession method.

New API methods

We need to extend the gRPC API of authd to provide methods to disable/enable a user and to delete a user/group. We might want to restructure the services/methods, but for a first iteration we can put those new methods in the NSS service. It could look something like this:

service NSS {
  rpc GetPasswdByName(GetPasswdByNameRequest) returns (PasswdEntry);
  rpc GetPasswdByUID(GetByIDRequest) returns (PasswdEntry);
  rpc GetPasswdEntries(Empty) returns (PasswdEntries);

  rpc GetGroupByName(GetGroupByNameRequest) returns (GroupEntry);
  rpc GetGroupByGID(GetByIDRequest) returns (GroupEntry);
  rpc GetGroupEntries(Empty) returns (GroupEntries);

  rpc GetShadowByName(GetShadowByNameRequest) returns (ShadowEntry);
  rpc GetShadowEntries(Empty) returns (ShadowEntries);

  rpc DisablePasswd(DisablePasswdRequest) returns (Empty);
  rpc EnablePasswd(EnablePasswdRequest) returns (Empty);

  rpc DeletePasswd(DeletePasswdRequest) returns (Empty);
  rpc DeleteGroup(DeleteGroupRequest) returns (Empty);
}

message DisablePasswdRequest{
  string name = 1;
}

message EnablePasswdRequest{
  string name = 1;
}

message DeletePasswdRequest{
  string name = 1;
}

message DeleteGroupRequest{
  string name = 1;
}

These methods need to be implemented in https://github.com/ubuntu/authd/blob/e06c6648dfbef9520c4da525320baca0d48cf505/internal/services/nss/nss.go.

We already have a DeleteUser method to delete a user from the database, but none for deleting a group. Note that we plan to migrate from bbolt to SQLite soon, so it might not be worth to spend a significant amount of time to implement bbolt specific functions for this. Maybe just implement the user-specific functions in the first iteration.

Create a command-line tool which uses the new API methods

Let's call the tool authctl for now (we didn't complete the discussion on the name but that's a promising candidate). I would use https://pkg.go.dev/github.com/spf13/cobra to implement it. It should support these commands:

authctl user disable <name>
authctl user enable <name>
authctl user delete <name>
authctl group delete <name>

Later we might also want to support other commands, like listing existing authd users/groups:

authctl user list
authctl group list

adombeck avatar Jan 27 '25 14:01 adombeck

I'm wondering if it would make sense to also to provide a script in /usr/share/examples/authd to be used to integrate with deluser via /usr/local/sbin/deluser.local´ (or even more smartly support the deluser.local` input when such tool is symlinked there...) so that it is integrated with standard tools...

Sadly we can't make this somewhat automatic since it doesn't seem to be pluggable, but it could be still installed by default from the debian package if nothing else is there (marking it as a configuration file).

3v1n0 avatar Jan 27 '25 15:01 3v1n0

@didrocks I updated the description to mention the issues with removing users/groups from the database. Do you agree with the reasoning and the plan?

@adombeck: I am unsure I fully understand the description update on top for groups. I think the reasoning that was discussed in another place is to keep the groups created (local or remote), but just empty the membership, correct? So basically, we never delete any of them and don’t need to print a warning? (or maybe, we can print a warning that a group has no member now).

On the GRPC API, do you think this should be part of the NSS service? I liked the fact that the NSS service really mirrored parts of the NSS getent functionalities. Shouldn’t we create another service then for those users/groups updates? Would that makes sense?

didrocks avatar Jan 27 '25 16:01 didrocks

@adombeck: I am unsure I fully understand the description update on top for groups. I think the reasoning that was discussed in another place is to keep the groups created (local or remote), but just empty the membership, correct? So basically, we never delete any of them and don’t need to print a warning? (or maybe, we can print a warning that a group has no member now).

We shouldn't automatically delete groups (as discussed in https://github.com/ubuntu/authd/issues/757), but my argument is that admins should still be able to manually delete groups (and users) when they are not used anymore, similar to how local groups can be deleted with delgroup. I propose that we print a warning and/or ask for confirmation in that case, to make the admin aware that other users/groups might be assigned the UID/GID, which would allow access to existing files still owned by the user/group.

On the GRPC API, do you think this should be part of the NSS service? I liked the fact that the NSS service really mirrored parts of the NSS getent functionalities. Shouldn’t we create another service then for those users/groups updates? Would that makes sense?

No, I don't think it should be part of the NSS service. That's why I wrote

We might want to restructure the services/methods

I'm still undecided how exactly it should be restructured (a new service which just provides the additional methods used by the command-line tool? what if the command-line tool also supports listing users/groups, which is already provided by the NSS service? maybe we should create Passwd, Group, and Shadow services instead, and remove the NSS service?), and I didn't want to block @shiv-tyagi on this. We should decide and restructure before merging anything to main though.

adombeck avatar Jan 27 '25 16:01 adombeck

I'm wondering if it would make sense to also to provide a script in /usr/share/examples/authd to be used to integrate with deluser via /usr/local/sbin/deluser.local´ (or even more smartly support the deluser.local` input when such tool is symlinked there...) so that it is integrated with standard tools...

Good idea, definitely something to consider. It's a shame that there's no deluser.local.d where we could just drop in a script.

adombeck avatar Jan 27 '25 16:01 adombeck

And there's no delgroup.local :/

adombeck avatar Jan 27 '25 16:01 adombeck

We shouldn't automatically delete groups (as discussed in https://github.com/ubuntu/authd/issues/757), but my argument is that admins should still be able to manually delete groups (and users) when they are not used anymore, similar to how local groups can be deleted with delgroup. I propose that we print a warning and/or ask for confirmation in that case, to make the admin aware that other users/groups might be assigned the UID/GID, which would allow access to existing files still owned by the user/group.

Oh, I didn’t get that was in that context only (when admin process via the command line), ofc, +1 on the message with confirmation.

No, I don't think it should be part of the NSS service.

Ok, we agreed thus!

I would still keep everything NSS query related to the NSS services as we don’t have that many calls and they are already namespaced. Those are all the same functionliaties.

But yeah, definitively another service at least for the user/groups managements. Now, we need to come with a name…

Good idea, definitely something to consider. It's a shame that there's no deluser.local.d where we could just drop in a script.

For reference, for zsys, we created distro patches for deluser and userdel (which are different!). If we feel the need, we can do that even if it’s better if we avoid this. Maybe let’s start with the script suggestions and see if there is a demand.

didrocks avatar Jan 27 '25 16:01 didrocks

For reference, for zsys, we created distro patches for deluser and userdel (which are different!). If we feel the need, we can do that even if it’s better if we avoid this. Maybe let’s start with the script suggestions and see if there is a demand.

Agreed, that's an option, but let's first see if there's demand for that feature.

adombeck avatar Jan 27 '25 17:01 adombeck

And there's no delgroup.local :/

It's not there by default, but it's triggered on deluser if there (check the man)

3v1n0 avatar Jan 28 '25 05:01 3v1n0

It's not there by default, but it's triggered on deluser if there (check the man)

Which man page do you mean? I read https://manpages.ubuntu.com/manpages/oracular/en/man8/deluser.local.8.html and there's no mention of delgroup in there.

adombeck avatar Jan 28 '25 09:01 adombeck

Which man page do you mean? I read https://manpages.ubuntu.com/manpages/oracular/en/man8/deluser.local.8.html and there's no mention of delgroup in there.

Sorry, I misread... I meant that man deluser mentions the local one, but indeed there's not a delgroup.local, only deluser.git is called with the gid parameter, so not sure if that is also involved when using delgroup (since it's mentioned in the FILES section of the delgroup man)

3v1n0 avatar Jan 28 '25 16:01 3v1n0

Add new "disabled" field to user records

How is this related to this? Where does this disabled field get the information from? Entra API?

I see there is a discussion going on about the deluser/delgroup scripts, so should those scripts be implemented first or should I start with the cli tool?

shiv-tyagi avatar Jan 30 '25 04:01 shiv-tyagi

Add new "disabled" field to user records

How is this related to this?

That's explained in the description of this issue.

adombeck avatar Jan 30 '25 10:01 adombeck

That's explained in the description of this issue.

Ah I missed that. Thanks. I'll start with adding the functionality to disable the user then.

shiv-tyagi avatar Jan 30 '25 14:01 shiv-tyagi

Any update on the timeline for the command-line tool?

ignirtoq avatar Aug 02 '25 01:08 ignirtoq

https://github.com/ubuntu/authd/pull/782 was now merged, which adds a command-line tool, authctl, which allows locking/unlocking authd users. We will add commands to remove authd users and groups in a follow-up.

authctl will be part of the next stable authd release.

adombeck avatar Sep 23 '25 14:09 adombeck