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

Create a command-line tool which allows removing authd users and groups #640

Open
adombeck opened this issue Nov 14, 2024 · 18 comments
Open
Labels

Comments

@adombeck
Copy link
Contributor

adombeck commented Nov 14, 2024

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.

@junebugin
Copy link

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?

@adombeck
Copy link
Contributor Author

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 adombeck changed the title Create a command-line tool which allows removing users from the database Create a command-line tool which allows disabling authd users Jan 27, 2025
@adombeck
Copy link
Contributor Author

adombeck commented Jan 27, 2025

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 adombeck changed the title Create a command-line tool which allows disabling authd users Create a command-line tool which allows disabling authd users and groups Jan 27, 2025
@adombeck adombeck changed the title Create a command-line tool which allows disabling authd users and groups Create a command-line tool which allows removing authd users and groups Jan 27, 2025
@adombeck
Copy link
Contributor Author

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

@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

@3v1n0
Copy link
Collaborator

3v1n0 commented Jan 27, 2025

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).

@didrocks
Copy link
Member

@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?

@adombeck
Copy link
Contributor Author

adombeck commented Jan 27, 2025

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

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

And there's no delgroup.local :/

@didrocks
Copy link
Member

We shouldn't automatically delete groups (as discussed in #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.

@adombeck
Copy link
Contributor Author

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.

@3v1n0
Copy link
Collaborator

3v1n0 commented Jan 28, 2025

And there's no delgroup.local :/

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

@adombeck
Copy link
Contributor Author

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.

@3v1n0
Copy link
Collaborator

3v1n0 commented Jan 28, 2025

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)

@shiv-tyagi
Copy link
Contributor

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?

@adombeck
Copy link
Contributor Author

Add new "disabled" field to user records

How is this related to this?

That's explained in the description of this issue.

@shiv-tyagi
Copy link
Contributor

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.

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

No branches or pull requests

5 participants