Bug 619633 - attr-unique-plugin does not check for requiredObjectClass on the 'other' object
attr-unique-plugin does not check for requiredObjectClass on the 'other' object
Status: CLOSED CURRENTRELEASE
Product: 389
Classification: Community
Component: Server - Plugins (Show other bugs)
1.2.6
All All
high Severity medium
: ---
: ---
Assigned To: Nathan Kinder
Viktor Ashirov
:
Depends On:
Blocks: 639035
  Show dependency treegraph
 
Reported: 2010-07-29 21:35 EDT by Karsten Sperling
Modified: 2015-12-07 11:52 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-12-07 11:52:54 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Proposed patch (10.11 KB, patch)
2010-08-01 19:34 EDT, Karsten Sperling
no flags Details | Diff
Revised patch (8.22 KB, patch)
2010-10-25 20:02 EDT, Nathan Kinder
nhosoi: review+
Details | Diff

  None (edit)
Description Karsten Sperling 2010-07-29 21:35:07 EDT
Description of problem:

When a uniqueness constraint is restricted to a specific object class with the requiredObjectClass parameter, the plugin currently only takes this restriction into account on the object that is being changed, but not on the existing object that causes the conflict.

This is counter intuitive, as it makes the 'conflict' relationship non-symmetric, and also causes the order of operations to matter in which conflicting objects are created/modified. Example:

Enforce uniqueness of attribute 'a' on class 'C' within some scope.
Object 1: class C, a=foo
Object 2: class D, a=foo

If object 2 is created after object 1, this is allowed but if object 2 is created first the plugin considers this a conflict.

This generally does not make sense in practice, because when trying to enforce uniqueness of some attribute on certain types of objects, one does not care about the order of operations that was used to arrive at a certain state, but rather that a certain state is invalid, no matter how it was arrived at.

To avoid this problem, the requiredObjectClass (if present) should be included as a filter condition in the filter used by the uniqueness plugin to search for the conflicting object. If it is not feasible to change the behaviour outright, maybe a flag like 'symmetric=true' could be added as an additional plugin parameter to enable this behaviour.

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:
Comment 1 Karsten Sperling 2010-08-01 19:34:48 EDT
Created attachment 435928 [details]
Proposed patch

Proposed patch.

Optionally checks for the required object class on the conflict object, by including a filter for that object class in the generated search filters.

The new behaviour is triggered by using requiredObjectClassSymmetric=x instead of requiredObjectClass=x.

Please review.
Comment 2 Karsten Sperling 2010-08-01 19:43:27 EDT
Note that the patch is relative to https://bugzilla.redhat.com/show_bug.cgi?id=619623
Comment 5 Nathan Kinder 2010-10-25 16:28:36 EDT
To reproduce this, add the following attribute uniqueness configuration entry and restart ns-slapd:

dn: cn=mail uniqueness,cn=plugins,cn=config
objectClass: top
objectClass: nsSlapdPlugin
objectClass: extensibleObject
cn: mail uniqueness
nsslapd-pluginPath: libattr-unique-plugin
nsslapd-pluginInitfunc: NSUniqueAttr_Init
nsslapd-pluginType: preoperation
nsslapd-pluginEnabled: on
nsslapd-pluginarg0: attribute=mail
nsslapd-pluginarg1: markerObjectClass=organizationalUnit
nsslapd-pluginarg2: requiredObjectClass=person
nsslapd-plugin-depends-on-type: database
nsslapd-pluginId: NSUniqueAttr
nsslapd-pluginVersion: 1.2.7.a3.git13ccbd4
nsslapd-pluginVendor: 389 Project
nsslapd-pluginDescription: Enforce unique attribute values

After the config is loaded, add this group entry:

dn: cn=group1,ou=People,dc=example,dc=com
objectclass: mailGroup
cn: group1
mail: test@example.com

Attempt to add the following user entry.  This add will be rejected as a conflict, though it should be allowed since the group entry does not have the requiredObjectClass present.

dn: uid=tuser1,ou=People,dc=example,dc=com
uid: tuser1
objectclass: inetorgperson
objectclass: person
cn: Test User
sn: User
mail: test@example.com
Comment 6 Nathan Kinder 2010-10-25 16:32:34 EDT
I do not think that it is required to keep the existing behavior, as it makes little sense and seems that not using the requiredObjectClass to search for existing entries was an oversight.  I will modify the contributed patch to simply change the behavior of the requiredObjectClass parameter instead of adding a new configuration attribute.
Comment 7 Nathan Kinder 2010-10-25 19:28:39 EDT
(In reply to comment #1)
> Created attachment 435928 [details]
> Proposed patch
> 
> Proposed patch.
> 
> Optionally checks for the required object class on the conflict object, by
> including a filter for that object class in the generated search filters.
> 
> The new behaviour is triggered by using requiredObjectClassSymmetric=x instead
> of requiredObjectClass=x.
> 
> Please review.

Thanks for your patch Karsten.  Have you signed the Fedora Contributor License Agreement yet?  If not, please see this page for details on signing it:

  http://directory.fedoraproject.org/wiki/Contributing

I have a slightly modified version of your patch that I will attach here shortly.
Comment 8 Nathan Kinder 2010-10-25 20:02:40 EDT
Created attachment 455649 [details]
Revised patch
Comment 9 Karsten Sperling 2010-10-26 00:25:40 EDT
Hi Nathan,

I've set the wheels in motion for getting the corporate CLA signed, shouldn't take too long hopefully.

Are you going to merge the related fix for https://bugzilla.redhat.com/show_bug.cgi?id=619623 at the same time?

Cheers, Karsten
Comment 10 Nathan Kinder 2010-10-26 11:50:22 EDT
(In reply to comment #9)

> Are you going to merge the related fix for
> https://bugzilla.redhat.com/show_bug.cgi?id=619623 at the same time?

Thank you for bringing this bug to my attention.  I will apply the fix for that issue as a separate patch, but will do it at the same time that I apply this fix.
Comment 11 Karsten Sperling 2010-10-26 16:35:41 EDT
I've submitted my personal CLA online and the corporate one for SKY Network TV New Zealand has been emailed to legal@lists.fedoraproject.org.
Comment 12 Nathan Kinder 2010-10-28 12:01:37 EDT
Pushed to master.  Thanks to Karsten for the initial patch contribution, and to Noriko for her review!

Counting objects: 26, done.
Delta compression using 2 threads.
Compressing objects: 100% (17/17), done.
Writing objects: 100% (17/17), 4.75 KiB, done.
Total 17 (delta 13), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
   13b9aff..20833de  master -> master
Comment 13 Amita Sharma 2011-05-25 09:18:20 EDT
For verification Comment#5 is ok? or any other steps needed, Please guide?
Comment 14 Nathan Kinder 2011-05-25 11:10:13 EDT
(In reply to comment #13)
> For verification Comment#5 is ok? or any other steps needed, Please guide?

Yes, the steps in comment #5 should be fine for verification.
Comment 15 Amita Sharma 2011-05-27 03:44:25 EDT
ldapmodify -x -h localhost -p 389 -D "cn=directory manager" -w xxx -a << EOF
> dn: cn=mail uniqueness,cn=plugins,cn=config
> objectClass: top
> objectClass: nsSlapdPlugin
> objectClass: extensibleObject
> cn: mail uniqueness
> nsslapd-pluginPath: libattr-unique-plugin
> nsslapd-pluginInitfunc: NSUniqueAttr_Init
> nsslapd-pluginType: preoperation
> nsslapd-pluginEnabled: on
> nsslapd-pluginarg0: attribute=mail
> nsslapd-pluginarg1: markerObjectClass=organizationalUnit
> nsslapd-pluginarg2: requiredObjectClass=person
> nsslapd-plugin-depends-on-type: database
> nsslapd-pluginId: NSUniqueAttr
> nsslapd-pluginVersion: 1.2.7.a3.git13ccbd4
> nsslapd-pluginVendor: 389 Project
> nsslapd-pluginDescription: Enforce unique attribute values
> EOF
adding new entry "cn=mail uniqueness,cn=plugins,cn=config"

ldapmodify -x -h localhost -p 389 -D "cn=directory manager" -w xxx -a << EOF
> dn: cn=group1,ou=people,dc=redhat,dc=com
> objectclass: mailGroup
> cn: group1
> mail: test@example.com
> EOF
adding new entry "cn=group1,ou=people,dc=redhat,dc=com"

ldapmodify -x -h localhost -p 389 -D "cn=directory manager" -w xxx -a << EOF
> dn: uid=tuser1,ou=people,dc=redhat,dc=com
> uid: tuser1
> objectclass: inetorgperson
> objectclass: person
> cn: Test User
> sn: User
> mail: test@example.com
> EOF
adding new entry "uid=tuser1,ou=people,dc=redhat,dc=com"

Note You need to log in before you can comment on or make changes to this bug.