#50054 Ticket 50053 - Subtree password policy overrides a user-defined password policy
Closed 3 years ago by spichugi. Opened 5 years ago by tbordaz.
tbordaz/389-ds-base ticket_50053  into  master

@@ -0,0 +1,193 @@ 

+ # --- BEGIN COPYRIGHT BLOCK ---

+ # Copyright (C) 2016 Red Hat, Inc.

+ # All rights reserved.

+ #

+ # License: GPL (version 3 or any later version).

+ # See LICENSE for details.

+ # --- END COPYRIGHT BLOCK ---

+ #

+ '''

+ Created on Nov 27th, 2018

+ 

+ @author: tbordaz

+ '''

+ import logging

+ import subprocess

+ import pytest

+ from lib389 import Entry

+ from lib389.utils import *

+ from lib389.plugins import *

+ from lib389._constants import *

+ from lib389.topologies import topology_st as topo

+ 

+ def add_user(server, uid, testbase, locality=None, tel=None, title=None):

+     dn = 'uid=%s,%s' % (uid, testbase)

+     log.fatal('Adding user (%s): ' % dn)

+     server.add_s(Entry((dn, {'objectclass': ['top', 'person', 'organizationalPerson', 'inetOrgPerson'],

+                              'cn': 'user_%s' % uid,

+                              'sn': 'user_%s' % uid,

+                              'uid': uid,

+                              'l': locality,

+                              'title': title,

+                              'telephoneNumber': tel})))

+ 

+ @pytest.mark.ds50053

+ def test_cos_operational_default(topo):

+     """operational-default cosAttribute should not overwrite an existing value

+ 

+     :id: 12fadff9-e14a-4c64-a3ee-51152cb8fcfb

+     :setup: Standalone Instance

+     :steps:

+         1. Create a user entry with attribute 'l' and 'telephonenumber' (real attribute with real value)

+         2. Create cos that defines 'l' as operational-default (virt. attr. with value != real value)

+         3. Create cos that defines 'telephone' as default (virt. attr. with value != real value)

+         4. Check that telephone is retrieved with real value

+         5. Check that 'l' is retrieved with real value

+     :expectedresults:

+         1. should succeed

+         2. should succeed

+         3. should succeed

+     """

+ 

+     REAL = 'real'

+     VIRTUAL = 'virtual'

+     TEL_REAL = '1234 is %s' % REAL

+     TEL_VIRT = '4321 is %s' % VIRTUAL

+ 

+     LOC_REAL = 'here is %s' % REAL

+     LOC_VIRT = 'there is %s' % VIRTUAL

+ 

+     TITLE_REAL = 'title is %s' % REAL

+ 

+     inst = topo[0]

+ 

+     PEOPLE = 'ou=people,%s' % SUFFIX

+     add_user(inst, 'user_0', PEOPLE, locality=LOC_REAL, tel=TEL_REAL, title=TITLE_REAL)

+ 

+     # locality cos operational-default

+     LOC_COS_TEMPLATE = "cn=locality_template,%s" % PEOPLE

+     LOC_COS_DEFINITION = "cn=locality_definition,%s" % PEOPLE

+     inst.add_s(Entry((LOC_COS_TEMPLATE, {

+             'objectclass': ['top', 'extensibleObject', 'costemplate',

+                             'ldapsubentry'],

+             'l': LOC_VIRT})))

+ 

+     inst.add_s(Entry((LOC_COS_DEFINITION, {

+             'objectclass': ['top', 'LdapSubEntry', 'cosSuperDefinition',

+                             'cosPointerDefinition'],

+             'cosTemplateDn': LOC_COS_TEMPLATE,

+             'cosAttribute': 'l operational-default'})))

+ 

+     # telephone cos default

+     TEL_COS_TEMPLATE = "cn=telephone_template,%s" % PEOPLE

+     TEL_COS_DEFINITION = "cn=telephone_definition,%s" % PEOPLE

+     inst.add_s(Entry((TEL_COS_TEMPLATE, {

+             'objectclass': ['top', 'extensibleObject', 'costemplate',

+                             'ldapsubentry'],

+             'telephonenumber': TEL_VIRT})))

+ 

+     inst.add_s(Entry((TEL_COS_DEFINITION, {

+             'objectclass': ['top', 'LdapSubEntry', 'cosSuperDefinition',

+                             'cosPointerDefinition'],

+             'cosTemplateDn': TEL_COS_TEMPLATE,

+             'cosAttribute': 'telephonenumber default'})))

+ 

+     # seeAlso cos operational

+     SEEALSO_VIRT = "dc=%s,dc=example,dc=com" % VIRTUAL

+     SEEALSO_COS_TEMPLATE = "cn=seealso_template,%s" % PEOPLE

+     SEEALSO_COS_DEFINITION = "cn=seealso_definition,%s" % PEOPLE

+     inst.add_s(Entry((SEEALSO_COS_TEMPLATE, {

+             'objectclass': ['top', 'extensibleObject', 'costemplate',

+                             'ldapsubentry'],

+             'seealso': SEEALSO_VIRT})))

+ 

+     inst.add_s(Entry((SEEALSO_COS_DEFINITION, {

+             'objectclass': ['top', 'LdapSubEntry', 'cosSuperDefinition',

+                             'cosPointerDefinition'],

+             'cosTemplateDn': SEEALSO_COS_TEMPLATE,

+             'cosAttribute': 'seealso operational'})))

+ 

+     # description cos override

+     DESC_VIRT = "desc is %s" % VIRTUAL

+     DESC_COS_TEMPLATE = "cn=desc_template,%s" % PEOPLE

+     DESC_COS_DEFINITION = "cn=desc_definition,%s" % PEOPLE

+     inst.add_s(Entry((DESC_COS_TEMPLATE, {

+             'objectclass': ['top', 'extensibleObject', 'costemplate',

+                             'ldapsubentry'],

+             'description': DESC_VIRT})))

+ 

+     inst.add_s(Entry((DESC_COS_DEFINITION, {

+             'objectclass': ['top', 'LdapSubEntry', 'cosSuperDefinition',

+                             'cosPointerDefinition'],

+             'cosTemplateDn': DESC_COS_TEMPLATE,

+             'cosAttribute': 'description override'})))

+ 

+     # title cos override

+     TITLE_VIRT = "title is %s" % VIRTUAL

+     TITLE_COS_TEMPLATE = "cn=title_template,%s" % PEOPLE

+     TITLE_COS_DEFINITION = "cn=title_definition,%s" % PEOPLE

+     inst.add_s(Entry((TITLE_COS_TEMPLATE, {

+             'objectclass': ['top', 'extensibleObject', 'costemplate',

+                             'ldapsubentry'],

+             'title': TITLE_VIRT})))

+ 

+     inst.add_s(Entry((TITLE_COS_DEFINITION, {

+             'objectclass': ['top', 'LdapSubEntry', 'cosSuperDefinition',

+                             'cosPointerDefinition'],

+             'cosTemplateDn': TITLE_COS_TEMPLATE,

+             'cosAttribute': 'title merge-schemes'})))

+ 

+     # note that the search requests both attributes (it is required for operational*)

+     ents = inst.search_s(SUFFIX, ldap.SCOPE_SUBTREE, "uid=user_0", ["telephonenumber", "l"])

+     assert len(ents) == 1

+     ent = ents[0]

+ 

+     # Check telephonenumber (specifier default)

+     assert ent.hasAttr('telephonenumber')

+     value = ent.getValue('telephonenumber')

+     log.info('Returned telephonenumber: %s' % value)

+     log.info('Returned telephonenumber: %d' % value.find(REAL.encode()))

+     assert value.find(REAL.encode()) != -1

+ 

+     # Check 'locality' (specifier operational-default)

+     assert ent.hasAttr('l')

+     value = ent.getValue('l')

+     log.info('Returned l: %s' % value)

+     log.info('Returned l: %d' % value.find(REAL.encode()))

+     assert value.find(REAL.encode()) != -1

+     

+     # Check 'seealso' (specifier operational)

+     assert not ent.hasAttr('seealso')

+     ents = inst.search_s(SUFFIX, ldap.SCOPE_SUBTREE, "uid=user_0", ["seealso"])

+     assert len(ents) == 1

+     ent = ents[0]

+     value = ent.getValue('seealso')

+     log.info('Returned seealso: %s' % value)

+     log.info('Returned seealso: %d' % value.find(VIRTUAL.encode()))

+     assert value.find(VIRTUAL.encode()) != -1

+     

+     # Check 'description' (specifier override)

+     assert not ent.hasAttr('description')

+     ents = inst.search_s(SUFFIX, ldap.SCOPE_SUBTREE, "uid=user_0")

+     assert len(ents) == 1

+     ent = ents[0]

+     value = ent.getValue('description')

+     log.info('Returned description: %s' % value)

+     log.info('Returned description: %d' % value.find(VIRTUAL.encode()))

+     assert value.find(VIRTUAL.encode()) != -1

+ 

+     # Check 'title' (specifier merge-schemes)

+     # commented because it does not work need to open a new ticket

+ #    ents = inst.search_s(SUFFIX, ldap.SCOPE_SUBTREE, "uid=user_0")

+ #    assert len(ents) == 1

+ #    ent = ents[0]

+ #    found_real = False

+ #    found_virtual = False

+ #    for value in ent.getValues('title'):

+ #        log.info('Returned title: %s' % value)

+ #        if value.find(VIRTUAL.encode()) != -1:

+ #            found_virtual = True

+ #        if value.find(REAL.encode()) != -1:

+ #            found_real = True

+ #    assert found_virtual

+ #    assert found_real 

\ No newline at end of file

@@ -2279,7 +2279,7 @@ 

          /* now for the tests */

  

          /* would we be allowed to supply this attribute if we had one? */

-         if (entry_has_value && !pAttr->attr_override && !pAttr->attr_operational && !pAttr->attr_operational_default) {

+         if (entry_has_value && !pAttr->attr_override && !pAttr->attr_operational) {

              /* answer: no, move on to the next attribute */

              attr_index++;

              continue;

Bug Description:
When an entry contains an attribute that is also defined by a cos definition
a specifier defines which values win: the real values that are in the entry or the
virtual values that are cos defined.
The specifier 'default' means that the real values are the winners (returned).
'operational-default' has the same behavior but just specify that the attribute
is operational.
The bug is that when real values exists, the 'operational-default' specifier
drops the real values in favor of the virtual ones.

Fix Description:
Change the test, so that real values are not kept for 'operation-default'
Note: the full routine cos_cache_query_attr looks quite messy and error prone
It would be nice to rewrite it when we have time

https://pagure.io/389-ds-base/issue/50053

Reviewed by: ?

Platforms tested: F27

Flag Day: no

Doc impact: no

Can you add a change to the test to make sure COS still works for the virtual attribute values when they are not present in the entry? Looks like this test just makes sure that present/existing values are still returned, but i want to make sure there are no regressions. Thanks!

rebased onto cde992ba2787d05147e1b07a0579110a88631bf9

5 years ago

@mreynolds I improved the test coverage.. and of course hit another issue ;)
I commented out the test that was failing. I am still unsure if it is a bug or test issue but I reading code/doc I tend to think it is a bug.

I would prefer to keep this ticket to the original customer issue (operational-default) and open a different one (merge-schemes) if necessary

That works, you have my ack

rebased onto 5d7b95c

5 years ago

Pull-Request has been merged by tbordaz

5 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3113

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

3 years ago