Bug 619633 - attr-unique-plugin does not check for requiredObjectClass on the 'other' object
Summary: attr-unique-plugin does not check for requiredObjectClass on the 'other' object
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: 389
Classification: Retired
Component: Server - Plugins
Version: 1.2.6
Hardware: All
OS: All
high
medium
Target Milestone: ---
Assignee: Nathan Kinder
QA Contact: Viktor Ashirov
URL:
Whiteboard:
Depends On:
Blocks: 639035
TreeView+ depends on / blocked
 
Reported: 2010-07-30 01:35 UTC by Karsten Sperling
Modified: 2015-12-07 16:52 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-07 16:52:54 UTC
Embargoed:


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

Description Karsten Sperling 2010-07-30 01:35:07 UTC
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 23:34:48 UTC
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 23:43:27 UTC
Note that the patch is relative to https://bugzilla.redhat.com/show_bug.cgi?id=619623

Comment 5 Nathan Kinder 2010-10-25 20:28:36 UTC
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

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

Comment 6 Nathan Kinder 2010-10-25 20:32:34 UTC
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 23:28:39 UTC
(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-26 00:02:40 UTC
Created attachment 455649 [details]
Revised patch

Comment 9 Karsten Sperling 2010-10-26 04:25:40 UTC
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 15:50:22 UTC
(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 20:35:41 UTC
I've submitted my personal CLA online and the corporate one for SKY Network TV New Zealand has been emailed to legal.org.

Comment 12 Nathan Kinder 2010-10-28 16:01:37 UTC
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 13:18:20 UTC
For verification Comment#5 is ok? or any other steps needed, Please guide?

Comment 14 Nathan Kinder 2011-05-25 15:10:13 UTC
(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 07:44:25 UTC
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
> 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
> 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.