Application passwords implementation

[scabrero]

This pull request replaces #2457 and follows the design document #2427.

It is just the server side for now.

Related to #41

Checklist

• This pr contains no AI generated code
• cargo fmt has been run
• cargo clippy has been run
• cargo test has been run and passes
• book chapter included (if relevant)
• design document included (if relevant)

[scabrero]

@Firstyear, the last test for:

Since application passwords are related to applications, on delete of an application all entries
that have a bound application password should be removed from user accounts.

fails. Any hint about how to enforce reference integrity on a "complex" attribute?

[Firstyear]

Regarding referential integrity have a look at https://github.com/kanidm/kanidm/blob/master/server/lib/src/valueset/oauth.rs#L369

It has handling for Refer as a special case, as well as Iutf8 for matching labels. You'll also notice https://github.com/kanidm/kanidm/blob/master/server/lib/src/valueset/oauth.rs#L607 which indexes the uuids of the groups related.

That way if you search an application id with PartialValue::Uuid yuo get the application itself, but PartialValue::Refer gets everything that refers to it.

That's what enables https://github.com/kanidm/kanidm/blob/master/server/lib/src/schema.rs#L771 which indicates that a reference relationship exists, and that automatically triggers https://github.com/kanidm/kanidm/blob/master/server/lib/src/plugins/refint.rs

[scabrero]

Hi @Firstyear, PR is not updated yet because I am rebasing on top of master commit by commit while addressing your comments.

[Firstyear]

Hi @Firstyear, PR is not updated yet because I am rebasing on top of master commit by commit while addressing your comments.

Ahhh okay. Sorry about that :)

Want me to wait til you are ready for me to look again then?

[scabrero]

Hi @Firstyear, PR is not updated yet because I am rebasing on top of master commit by commit while addressing your comments.

Ahhh okay. Sorry about that :)

Want me to wait til you are ready for me to look again then?

Except the refint for the ApplicationPassword struct to make the last test pass, it is now ready for a second review :)

[scabrero]

Ready for review.

[Firstyear]

Haven't finished the full review but here is the first pass.

[Firstyear]

Previous comment was "how do you envisage these access controls working with human roles and tasks", and I think you need to answer that because these access controls otherwise seem confused.

[scabrero]

Ok, I can think about two possibilities:

Use case 1, the IDM administrator creates the application, links the group, and sets up the members.
Use case 2, there is a person managing service X, e.g. mail. The IDM administrator creates the app, links a group,
and delegates group permission for the group to the X service manager who sets up the members.

[Firstyear]

Okay, so I actually think both possibilities are valid. We have the ability to create new applications (which we will give to an idm_application_admins group, and by default, idm_admins is a member of that, see https://github.com/kanidm/kanidm/blob/master/server/lib/src/constants/groups.rs#L203 and https://github.com/kanidm/kanidm/blob/master/server/lib/src/constants/acp.rs#L594 ) and then for group 2, we already have a delegation feature thanks to our "Entry Managed By" feature. You can see an example of this https://github.com/kanidm/kanidm/blob/master/server/lib/src/constants/acp.rs#L1047

So we'll need to support Attribute::EntryManagedBy on the LDAP application class, as well as make EntryManagedBy writable by the application_admins group, then there is a second ACP for ApplicationEntryManager which has the receiver as EntryManager and similar or constrained application rights. Does that seem reasonable?

[scabrero]

Ok, it is clearer now but I still have questions:

For case 1:
I have merged IDM_ACP_APPLICATION_{CREATE,MANAGE,DELETE}_DL7 into IDM_ACP_APPLICATION_MANAGE_DL7. The receiver is the idm_application_admins builtin group and has "full control" over the application entry.

For case 2:
I don't see the need for Attribute::EntryManagedBy in the application entry nor the need for a second ACP having EntryManager as receiver. IIUC a idm_application_admins_group member will create the application and will link a group, and then will delegate the linked group management, not the application entry. And as you said we already have IDM_ACP_GROUP_ENTRY_MANAGER_V1 to allow the delegated person to modify the linked group members. Unless you think the delegated person should be able to modify the linked group in the application entry.

[Firstyear]

But linked group is about the users who can access the application, not the group that owns and manages the application integration itself. So that's why I think there needs to be a separation.

[scabrero]

Ok, understood. I have added it.

[Firstyear]

What's the logic of using a string to index into this Map instead of UUID?

[scabrero]

Mainly because LdapApplicationAuthEvent carries the application name extracted from the bind DN, so we can do https://github.com/scabrero/kanidm/blob/app-passwords/server/lib/src/idm/application.rs#L178

[Firstyear]

This is looking much better, responded re the acp parts. I think next I need to just review that the tests cover all the situations we can think of, then I think we're almost there.

[Firstyear]

Okay, so I actually think both possibilities are valid. We have the ability to create new applications (which we will give to an idm_application_admins group, and by default, idm_admins is a member of that, see https://github.com/kanidm/kanidm/blob/master/server/lib/src/constants/groups.rs#L203 and https://github.com/kanidm/kanidm/blob/master/server/lib/src/constants/acp.rs#L594 ) and then for group 2, we already have a delegation feature thanks to our "Entry Managed By" feature. You can see an example of this https://github.com/kanidm/kanidm/blob/master/server/lib/src/constants/acp.rs#L1047

So we'll need to support Attribute::EntryManagedBy on the LDAP application class, as well as make EntryManagedBy writable by the application_admins group, then there is a second ACP for ApplicationEntryManager which has the receiver as EntryManager and similar or constrained application rights. Does that seem reasonable?

[Firstyear]

I'm glad I left you with one more bit in the change flags 😅