On Tue, 2018-09-25 at 08:40 +0200, Jakub Hrozek wrote:
On 24 Sep 2018, at 20:25, Simo Sorce simo@redhat.com wrote:
On Mon, 2018-09-24 at 19:59 +0200, Jakub Hrozek wrote:
On Mon, Sep 24, 2018 at 10:22:35AM -0400, Simo Sorce wrote:
btw it’s a good question to ask why isn’t the check done on saving the group. I thought it was and I see code that checks for ID uniqueness and even a test..
In current code, saving would override data as if the group was renamed changed I think ?
The way the code is currently written is, if there is a duplicate:
- check if the "new" group has the same SID, uniqueID or original DN as the "old" one
- yes, same: this is a rename, allow
- no, different: this is a duplicate, error
not sure how the original DN would match if you rename the object and that changes the DN too ?
Yes, in that case, the rename would throw an error. The originalDN handling was meant to support renames in the case the RDN doesn’t contain the name and therefore doesn’t change.
This is honestly something where I don’t know what is the right thing to do. If we detect that a group with some GID already exists, then how do we distinguish between “err, there are duplicates on the LDAP side” and “look, the group was renamed” without any peristent identifier like a SID?
We can do an additional search (or always do dereference searches) for the gidNumber, and if only one entry comes back then the old entry is gone and the new one is what we should keep.
At the point the code was changed the last time we also thought about returning an error code from the sysdb save operation and then running an LDAP search by GID before deciding about rename or duplicate.
Yes.
But we also thought that a) a rename was not very common operation and b) because IPA and especially AD form the bulk of the deployments, we could go away with the SID or uniqueID helper..
Yes, we can have a functiont hat does various optimizations (entryUUID, SID, IPAUniqueID) or use ASQ/dereference controls to always get back a sub search for gidnumber.
Of course, if more people complain about group renames with a “plain LDAP” server, then we should implement the more complex logic, but my take at the time was that SSSD is complex as it is already, so I didn’t think it was worth adding a complex logic for a corner case.
The problem is that when the corner case happens you have incorrect results, so we should handle it more gracefully in any case.
Simo.