The uniqueness plugin should return the right text for the error (or be silent). Otherwise the error can be extremely confusing. For example see freeipa ticket 558: https://fedorahosted.org/freeipa/ticket/558
An interesting find in testing is that a modify operation on a non-existent entry will only return the constraint violation if the modify would indeed result in a uniqueness conflict (if the target existed). For example: ======================================================================== [nkinder@localhost ~]$ ldapmodify -x -D "cn=directory manager" -w - dn: cn=test user2,dc=example,dc=com changetype: add objectclass: posixAccount uid: tuser2 homeDirectory: /home/tuser uidnumber: 500 gidnumber: 500 adding new entry "cn=test user2,dc=example,dc=com" [nkinder@localhost ~]$ ldapmodify -x -D "cn=directory manager" -w - dn: cn=test user3,dc=example,dc=com changetype: modify replace: uid uid: tuser2 modifying entry "cn=test user3,dc=example,dc=com" ldap_modify: Constraint violation (19) additional info: Another entry with the same attribute value already exists (attribute: "uid") [nkinder@localhost ~]$ ldapmodify -x -D "cn=directory manager" -w - dn: cn=test user3,dc=example,dc=com changetype: modify replace: uid uid: tuser3 modifying entry "cn=test user3,dc=example,dc=com" ldap_modify: No such object (32) matched DN: dc=example,dc=com ======================================================================== Now for a MODRDN operation, a constraint violation is returned when you attempt to rename a non-existent entry even if the operation would not result in a uniqueness conflict: ======================================================================== [nkinder@localhost ~]$ ldapmodify -x -D "cn=directory manager" -w - dn: cn=test user3,dc=example,dc=com changetype: modrdn newrdn: uid=tuser2 deleteoldrdn: 1 modifying rdn of entry "cn=test user3,dc=example,dc=com" ldap_rename: Operations error (1) additional info: Another entry with the same attribute value already exists (attribute: "uid") [nkinder@localhost ~]$ ldapmodify -x -D "cn=directory manager" -w - dn: cn=test user3,dc=example,dc=com changetype: modrdn newrdn: uid=tuser3 deleteoldrdn: 1 modifying rdn of entry "cn=test user3,dc=example,dc=com" ldap_rename: Operations error (1) additional info: Another entry with the same attribute value already exists (attribute: "uid") ======================================================================== I feel that the MODRDN should return an error 32 if the change would not result in a conflict. I'm not so sure about changing the result code to 32 if the changes would result in a conflict. It's true that the target does not exist at the time of checking for uniqueness (preop), but there is a slight window where the target entry could be created between the check for uniqueness and the actual processing of the operation by the server. This would result in a duplicate value for an attribute that is supposed to be unique.
If you look at the error in my previous examples closely, the result code differs when there is really a uniqueness collision vs. some other error (LDAP_CONSTRAINT_VIOLATION vs. LDAP_OPERATIONS_ERROR). I think the right thing to do is to only return the "Another entry with the same attribute value already exists" error text when there is a constraint violation and to return something more generic for other errors, such as "Error checking for attribute uniqueness." In addition, the attribute uniqueness plug-in should return LDAP_NO_SUCH_OBJECT when a target does not exist instead of LDAP_OPERATIONS_ERROR.
I agree. Note that the way the attribute uniqueness plugin is designed, there is a window for race conditions for any operation.
Created attachment 476902 [details] Patch
Comment on attachment 476902 [details] Patch + result = uid_op_error(35); + /* We want to return a no such object error if the target doesn't exist. */ + result = LDAP_NO_SUCH_OBJECT; try (void)uid_op_error(35); instead of assigning to result. Also, if err != LDAP_SUCCESS, does that always mean NO_SUCH_OBJECT? Or is it possible that there could be some other LDAP error?
Created attachment 476905 [details] Revised Patch This addresses the issues pointed out in Rich's review.
Created attachment 476911 [details] Re-revised patch
Pushed to master. Thanks to Rich and Noriko for their reviews! Counting objects: 13, done. Delta compression using up to 2 threads. Compressing objects: 100% (7/7), done. Writing objects: 100% (7/7), 1.07 KiB, done. Total 7 (delta 5), reused 0 (delta 0) To ssh://git.fedorahosted.org/git/389/ds.git be92452..da53383 master -> master
[nkinder@localhost ~]$ ldapmodify -x -D "cn=directory manager" -w - dn: cn=test user2,dc=example,dc=com changetype: add objectclass: posixAccount uid: tuser2 homeDirectory: /home/tuser uidnumber: 500 gidnumber: 500 adding new entry "cn=test user2,dc=example,dc=com" [nkinder@localhost ~]$ ldapmodify -x -D "cn=directory manager" -w - dn: cn=test user3,dc=example,dc=com changetype: modify replace: uid uid: tuser2 <<modifying entry "cn=test user3,dc=example,dc=com" ldap_modify: No such object (32) matched DN: dc=example,dc=com >> [nkinder@localhost ~]$ ldapmodify -x -D "cn=directory manager" -w - dn: cn=test user3,dc=example,dc=com changetype: modify replace: uid uid: tuser3 <<modifying entry "cn=test user3,dc=example,dc=com" ldap_modify: No such object (32) matched DN: dc=example,dc=com >> [nkinder@localhost ~]$ ldapmodify -x -D "cn=directory manager" -w - dn: cn=test user3,dc=example,dc=com changetype: modrdn newrdn: uid=tuser2 deleteoldrdn: 1 <<modifying rdn of entry "cn=test user3,dc=example,dc=com" ldap_rename: No such object (32) matched DN: dc=example,dc=com >> [nkinder@localhost ~]$ ldapmodify -x -D "cn=directory manager" -w - dn: cn=test user3,dc=example,dc=com changetype: modrdn newrdn: uid=tuser3 deleteoldrdn: 1 << modifying rdn of entry "cn=test user3,dc=example,dc=com" ldap_rename: No such object (32) matched DN: dc=example,dc=com >> Till here all is well.. but then .. I guess the below, should return some error like "ldap_modify: Constraint violation (19) additional info: Another entry with the same attribute value already exists (attribute: "uid")" But it just passed : [root@testvm /]# ldapmodify -x -h localhost -p 389 -D "cn=Directory Manager" -w Secret123 << EOF > dn: cn=test11,dc=example,dc=com > changetype: add > objectclass: posixAccount > uid: rosy > homeDirectory: /home/tuser > uidnumber: 500 > gidnumber: 500 > EOF adding new entry "cn=test11,dc=example,dc=com" [root@testvm /]# ldapmodify -x -h localhost -p 389 -D "cn=Directory Manager" -w Secret123 << EOF > dn: cn=test12,dc=example,dc=com > changetype: add > objectclass: posixAccount > uid: rosy1 > homeDirectory: /home/tuser > uidnumber: 500 > gidnumber: 500 > EOF adding new entry "cn=test12,dc=example,dc=com" [root@testvm /]# ldapmodify -x -h localhost -p 389 -D "cn=Directory Manager" -w Secret123 << EOF > dn: cn=test12,dc=example,dc=com > changetype: modify > replace: uid > uid: rosy > EOF modifying entry "cn=test12,dc=example,dc=com"
Amita - What does your attribute uniqueness config entry look like? It's the "cn=attribute uniqueness,cn=plugins,cn=config" entry.
It has the uid included... dn: cn=attribute uniqueness,cn=plugins,cn=config objectClass: top objectClass: nsSlapdPlugin objectClass: extensibleObject cn: attribute uniqueness nsslapd-pluginPath: libattr-unique-plugin nsslapd-pluginInitfunc: NSUniqueAttr_Init nsslapd-pluginType: preoperation nsslapd-pluginEnabled: off nsslapd-pluginarg0: uid nsslapd-pluginarg1: dc=example,dc=com nsslapd-plugin-depends-on-type: database nsslapd-pluginId: NSUniqueAttr nsslapd-pluginVersion: 1.2.8.3 nsslapd-pluginVendor: 389 Project nsslapd-pluginDescription: Enforce unique attribute values
(In reply to comment #11) > It has the uid included... > > dn: cn=attribute uniqueness,cn=plugins,cn=config > objectClass: top > objectClass: nsSlapdPlugin > objectClass: extensibleObject > cn: attribute uniqueness > nsslapd-pluginPath: libattr-unique-plugin > nsslapd-pluginInitfunc: NSUniqueAttr_Init > nsslapd-pluginType: preoperation > nsslapd-pluginEnabled: off > nsslapd-pluginarg0: uid > nsslapd-pluginarg1: dc=example,dc=com > nsslapd-plugin-depends-on-type: database > nsslapd-pluginId: NSUniqueAttr > nsslapd-pluginVersion: 1.2.8.3 > nsslapd-pluginVendor: 389 Project > nsslapd-pluginDescription: Enforce unique attribute values It does have uid included, but the plug-in is disabled. You need to set nsslapd-pluginEnabled to "on" and re-run all of your verification tests.
oops, I just missed that .. > dn: cn=attribute uniqueness,cn=plugins,cn=config > changetype: modify > replace: nsslapd-pluginEnabled > nsslapd-pluginEnabled: on > EOF modifying entry "cn=attribute uniqueness,cn=plugins,cn=config" Now---- dn: cn=attribute uniqueness,cn=plugins,cn=config objectClass: top objectClass: nsSlapdPlugin objectClass: extensibleObject cn: attribute uniqueness nsslapd-pluginPath: libattr-unique-plugin nsslapd-pluginInitfunc: NSUniqueAttr_Init nsslapd-pluginType: preoperation nsslapd-pluginEnabled: on nsslapd-pluginarg0: uid nsslapd-pluginarg1: dc=example,dc=com nsslapd-plugin-depends-on-type: database nsslapd-pluginId: NSUniqueAttr nsslapd-pluginVersion: 1.2.8.3 nsslapd-pluginVendor: 389 Project nsslapd-pluginDescription: Enforce unique attribute values modifiersName: cn=directory manager modifyTimestamp: 20110519070110Z testing --- [root@testvm ~]# ldapmodify -x -h localhost -p 389 -D "cn=Directory Manager" -w xxx << EOF > dn: cn=test111,dc=example,dc=com > changetype: add > objectclass: posixAccount > uid: ripsi > homeDirectory: /home/tuser > uidnumber: 500 > gidnumber: 500 > EOF adding new entry "cn=test111,dc=example,dc=com" [root@testvm ~]# ldapmodify -x -h localhost -p 389 -D "cn=Directory Manager" -w xxx << EOF > dn: cn=test112,dc=example,dc=com > changetype: add > objectclass: posixAccount > uid: ripsi > homeDirectory: /home/tuser > uidnumber: 500 > gidnumber: 500 > EOF adding new entry "cn=test112,dc=example,dc=com" ldap_add: Constraint violation (19) additional info: Another entry with the same attribute value already exists (attribute: "uid") [root@testvm ~]# ldapmodify -x -h localhost -p 389 -D "cn=Directory Manager" -w Secret123 << EOF > dn: cn=test112,dc=example,dc=com > changetype: add > objectclass: posixAccount > uid: ripsi1 > homeDirectory: /home/tuser > uidnumber: 500 > gidnumber: 500 > EOF adding new entry "cn=test112,dc=example,dc=com" [root@testvm ~]# ldapmodify -x -h localhost -p 389 -D "cn=Directory Manager" -w Secret123 << EOF > dn: cn=test112,dc=example,dc=com > changetype: modify > replace: uid > uid: ripsi > EOF modifying entry "cn=test112,dc=example,dc=com" ldap_modify: Constraint violation (19) additional info: Another entry with the same attribute value already exists (attribute: "uid") [root@testvm ~]# ldapmodify -x -h localhost -p 389 -D "cn=Directory Manager" -w Secret123 << EOF > dn: cn=test111,dc=example,dc=com > changetype: modify > replace: uid > uid: ripsi11 > EOF modifying entry "cn=test111,dc=example,dc=com" [root@testvm ~]# ldapmodify -x -h localhost -p 389 -D "cn=Directory Manager" -w Secret123 << EOF dn: cn=test112,dc=example,dc=com changetype: modify replace: uid uid: ripsi EOF modifying entry "cn=test112,dc=example,dc=com" now, we are good to mark it VERIFIED.