On 24 Sep 2018, at 20:25, Simo Sorce <simo(a)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? 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. 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..
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.