Group racl #4865
Group racl #4865
Conversation
Yeah, I've updated the data file that LDAP.pm uses as well :) |
|
I've re-requested review! This now includes the two extra bits; a "raclmodseq" command that only admins can use to touch a user's raclmodseq (needed when group membership changes); and a mode to ptexpire which takes a username or group and nukes the cached lookup immediately. plus of course fixes to LDAP and to tests. |
cassandane/data/directory.ldif
Outdated
| hasmember: cassandane | ||
| hasmember: otheruser |
There was a problem hiding this comment.
Usually these would be dns, not just uids, because the directory wants to be able to enforce foreign key constraints and so on. Compare for example the memberof attribute within the user objects, whose values are the dns of the groups. Also, I'd expect this attribute to typically be called just "member" too, or perhaps be named to denote a specific kind of member (real world examples from memory: "uniquemember", "rfc822mailmember", etc), but not "hasmember".
We can make directory.ldif say whatever the hell we want, but it's important to remember that if it isn't representative of a plausible real world directory layout, then testing against it doesn't prove anything about whether Cyrus's LDAP support works, and a test that doesn't prove anything is a waste of time
|
Hi again @ksmurchison and @elliefm ! I've updated the ldap module to work the way I think it should from my reading of the internets - with "memberof" for the group list in a user, and "member" for the user entry in a group. I've also fixed the HTTPPTS tests with correct group names and memberships, and added the other groups referenced in the tests to the auth daemon, since it's now mandatory for a group to exists (return 200 to a lookup) in order to be valid (and we check it when setting ACLs) |
|
OK, I've re-requested review @elliefm - added fixes for the two nits you have proposed. Still happily passing all the LDAP tests. |
|
Should the documentation be updated? |
There was a problem hiding this comment.
One last thing: can you add a changes/next/* file please -- copy 0000-TEMPLATE and fill in the fields. I think we're thinking of this as experimental for the time being, so please describe it as such. I'm happy to approve it once that's done.
@dilyanpalauzov asked about documentation. If we're thinking of this as experimental then I don't think that's strictly necessary yet, as long as we're willing to help out anyone else who wants to experiment with it.
It can't return failure, it zeros everything, you only need space for groups if you're actually adding groups
This matches the behaviour of ptclient/ldap for group: (though, ptclient/ldap doesn't currently support fetching group membership)
If a user has multiple uids, include them all in the groups
|
Thanks @elliefm - I've added changes/next file and rebased. This is still an old "in upstream" branch :) |
This is a little skanky, but "PTS" is basically a key to list lookup protocol, so I'm using it in reverse to map a group to its members.
(This pairs with an MR in Fastmail hm to add the same thing to our lookup)