From ab321cf1e538a78d3e05235dcd90a483af990a93 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Nov 07 2018 20:07:20 +0000 Subject: Ticket 50004 - lib389 - improve X-ORIGIN schema parsing Bug Description: Schema parsing assumed X-ORIGIN was always in this format "X-ORIGIN '", but it can also be in other formats like: "X-ORIGIN (". So when it did not contain the original format we got list index errors. Fix Description: Loosen the format to " X-ORIGIN " which all the formats. Also: improve from UI schema error messages updated create_test for python3 https://pagure.io/389-ds-base/issue/50004 Reviewed by: firstyear & spichugi(Thanks!!) --- diff --git a/dirsrvtests/create_test.py b/dirsrvtests/create_test.py index bf4abbd..fd6df7b 100755 --- a/dirsrvtests/create_test.py +++ b/dirsrvtests/create_test.py @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/python3 # # --- BEGIN COPYRIGHT BLOCK --- # Copyright (C) 2016 Red Hat, Inc. @@ -122,10 +122,11 @@ def check_id_uniqueness(id_value): for root, dirs, files in os.walk(tests_dir): for name in files: - with open(os.path.join(root, name), "r") as file: - for line in file: - if re.search(str(id_value), line): - return False + if name.endswith('.py'): + with open(os.path.join(root, name), "r") as cifile: + for line in cifile: + if re.search(str(id_value), line): + return False return True diff --git a/src/cockpit/389-console/src/schema.js b/src/cockpit/389-console/src/schema.js index c815a04..c1c3b82 100644 --- a/src/cockpit/389-console/src/schema.js +++ b/src/cockpit/389-console/src/schema.js @@ -127,7 +127,7 @@ function load_schema_objects_to_select(object, select_id) { })); }); }).fail(function(select_data) { - console.log("failed: " + select_data.message); + console.log("Get schema failed: " + select_data.message); check_inst_alive(1); }); } @@ -219,7 +219,7 @@ function get_and_set_schema_tables() { }] }); }).fail(function(oc_data) { - console.log("failed: " + oc_data.message); + console.log("Get objectclasses failed: " + oc_data.message); check_inst_alive(1); }); @@ -342,12 +342,12 @@ function get_and_set_schema_tables() { }] }); }).fail(function(at_data) { - console.log("failed: " + at_data.message); + console.log("Get attributes failed: " + at_data.message); check_inst_alive(1); }); }).fail(function(syntax_data) { - console.log("failed: " + syntax_data.message); + console.log("Get syntaxes failed: " + syntax_data.message); check_inst_alive(1); }); @@ -379,7 +379,7 @@ function get_and_set_schema_tables() { console.log("Finished loading schema."); }).fail(function(mr_data) { - console.log("failed: " + mr_data.cmd); + console.log("Get matching rules failed: " + mr_data.cmd); check_inst_alive(1); }); } @@ -498,7 +498,7 @@ $(document).ready( function() { }). fail(function(oc_data) { popup_err("err", oc_data.message); - console.log("failed: " + oc_data.message); + console.log("Search objectclasses failed: " + oc_data.message); check_inst_alive(1); }); // Replace the option in 'Edit objectClass' window @@ -693,7 +693,7 @@ $(document).ready( function() { }). fail(function(at_data) { popup_err("err", at_data.message); - console.log("failed: " + at_data.message); + console.log("Query attributes failed: " + at_data.message); check_inst_alive(1); }); if (!edit) { diff --git a/src/lib389/lib389/schema.py b/src/lib389/lib389/schema.py index 4090977..8ad42e2 100755 --- a/src/lib389/lib389/schema.py +++ b/src/lib389/lib389/schema.py @@ -13,6 +13,7 @@ import glob import ldap import ldif +import re from itertools import count from json import dumps as dump_json from operator import itemgetter @@ -41,7 +42,7 @@ ATTR_SYNTAXES = {"1.3.6.1.4.1.1466.115.121.1.5": "Binary", "1.3.6.1.4.1.1466.115.121.1.7": "Boolean", "1.3.6.1.4.1.1466.115.121.1.11": "Country String", "1.3.6.1.4.1.1466.115.121.1.12": "DN", - "1.3.6.1.4.1.1466.115.121.1.14": "Delivery Method", + "1.3.6.1.4.1.1466.115.121.1.14": "Delivery Method", "1.3.6.1.4.1.1466.115.121.1.15": "Directory String", "1.3.6.1.4.1.1466.115.121.1.21": "Enhanced Guide", "1.3.6.1.4.1.1466.115.121.1.22": "Facsimile", @@ -64,6 +65,9 @@ ATTR_SYNTAXES = {"1.3.6.1.4.1.1466.115.121.1.5": "Binary", "1.3.6.1.4.1.1466.115.121.1.52": "Telex Number"} +X_ORIGIN_REGEX = r'\'(.*?)\'' + + class Schema(DSLdapObject): """An object that represents the schema entry @@ -113,35 +117,46 @@ class Schema(DSLdapObject): return result def _get_schema_objects(self, object_model, json=False): + """Get all the schema objects for a specific model: Attribute, Objectclass, + or Matchingreule. + """ attr_name = self._get_attr_name_by_model(object_model) - results = self.get_attr_vals_utf8(attr_name) if json: - object_insts = [vars(object_model(obj_i)) for obj_i in results] - - for obj_i in object_insts: - # Add normalized name for sorting. Some matching rules don't have a name + object_insts = [] + for obj in results: + obj_i = vars(object_model(obj)) if len(obj_i["names"]) == 1: - obj_i['name'] = obj_i['names'][0] + obj_i['name'] = obj_i['names'][0].lower() obj_i['aliases'] = "" elif len(obj_i["names"]) > 1: - obj_i['name'] = obj_i['names'][0] + obj_i['name'] = obj_i['names'][0].lower() obj_i['aliases'] = obj_i['names'][1:] else: obj_i['name'] = "" + # Temporary workaround for X-ORIGIN in ObjectClass objects. # It should be removed after https://github.com/python-ldap/python-ldap/pull/247 is merged - if "x_origin" not in obj_i and object_model != MatchingRule: - for object_str in results: - if obj_i['names'] == vars(object_model(object_str))['names'] and "X-ORIGIN" in object_str: - obj_i['x_origin'] = object_str.split("X-ORIGIN '")[1].split("'")[0] - object_insts = sorted(object_insts, key=itemgetter('name')) - result = {'type': 'list', 'items': object_insts} + if " X-ORIGIN " in obj and obj_i['names'] == vars(object_model(obj))['names']: + remainder = obj.split(" X-ORIGIN ")[1] + if remainder[:1] == "(": + # Have multiple values + end = remainder.find(')') + vals = remainder[1:end] + vals = re.findall(X_ORIGIN_REGEX, vals) + # For now use the first value, but this should be a set (another bug in python-ldap) + obj_i['x_origin'] = vals[0] + else: + # Single X-ORIGIN value + obj_i['x_origin'] = obj.split(" X-ORIGIN ")[1].split("'")[1] + object_insts.append(obj_i) - return result + object_insts = sorted(object_insts, key=itemgetter('name')) + return {'type': 'list', 'items': object_insts} else: - return [object_model(obj_i) for obj_i in results] + object_insts = [object_model(obj_i) for obj_i in results] + return sorted(object_insts, key=lambda x: x.names, reverse=False) def _get_schema_object(self, name, object_model, json=False): objects = self._get_schema_objects(object_model, json=json) diff --git a/src/lib389/lib389/tests/cli/conf_schema_test.py b/src/lib389/lib389/tests/cli/conf_schema_test.py index e69de29..40a96af 100644 --- a/src/lib389/lib389/tests/cli/conf_schema_test.py +++ b/src/lib389/lib389/tests/cli/conf_schema_test.py @@ -0,0 +1 @@ +# -*- coding: utf-8 -*- diff --git a/src/lib389/lib389/tests/schema_test.py b/src/lib389/lib389/tests/schema_test.py index 28daefe..8f4e86c 100644 --- a/src/lib389/lib389/tests/schema_test.py +++ b/src/lib389/lib389/tests/schema_test.py @@ -1,72 +1,121 @@ # --- BEGIN COPYRIGHT BLOCK --- -# Copyright (C) 2015 Red Hat, Inc. +# Copyright (C) 2018 Red Hat, Inc. # All rights reserved. # # License: GPL (version 3 or any later version). # See LICENSE for details. # --- END COPYRIGHT BLOCK --- # -import pytest - -from lib389 import DirSrv -from lib389._constants import SER_HOST, SER_PORT, SER_SERVERID_PROP, LOCALHOST -from lib389.schema import Schema - -INSTANCE_PORT = 54321 -INSTANCE_SERVERID = 'standalone' - - -class TopologyInstance(object): - def __init__(self, instance): - instance.open() - self.instance = instance -@pytest.fixture(scope="module") -def topology(request): - instance = DirSrv(verbose=False) - instance.log.debug("Instance allocated") - args = {SER_HOST: LOCALHOST, - SER_PORT: INSTANCE_PORT, - SER_SERVERID_PROP: INSTANCE_SERVERID} - instance.allocate(args) - if instance.exists(): - instance.delete() - instance.create() - instance.open() +import logging +import pytest +import os +from lib389._constants import * +from lib389.schema import Schema +from lib389.topologies import topology_st as topo - def fin(): - instance.delete() - request.addfinalizer(fin) +DEBUGGING = os.getenv("DEBUGGING", default=False) +if DEBUGGING: + logging.getLogger(__name__).setLevel(logging.DEBUG) +else: + logging.getLogger(__name__).setLevel(logging.INFO) +log = logging.getLogger(__name__) - return TopologyInstance(instance) +def test_schema(topo): + """Basic schema querying test -def test_schema(topology): - must_expect = ['uidObject', 'account', 'posixAccount', 'shadowAccount'] + :id: 995acc60-243b-45b0-9c1c-12bbe6a2882f + :setup: Standalone Instance + :steps: + 1. Get attribute info for 'uid' + 2. Check the attribute is found in the expected objectclasses (must, may) + 3. Check that the 'account' objectclass is found + :expectedresults: + 1. Success + 2. Success + 3. Success + """ + must_expect = ['uidObject', 'account', 'posixAccount', 'shadowAccount', 'ipaUser', + 'sambaSamAccount'] may_expect = ['cosDefinition', 'inetOrgPerson', 'inetUser', - 'mailRecipient'] - attrtype, must, may = topology.instance.schema.query_attributetype('uid') + 'mailRecipient', 'nsOrgPerson', 'ipaUserOverride'] + attrtype, must, may = topo.standalone.schema.query_attributetype('uid') assert attrtype.names == ('uid', 'userid') for oc in must: assert oc.names[0] in must_expect for oc in may: assert oc.names[0] in may_expect - assert topology.instance.schema.query_objectclass('account').names == \ + assert topo.standalone.schema.query_objectclass('account').names == \ ('account', ) -def test_schema_reload(topology): - """Test that the appropriate task entry is created when reloading schema.""" - schema = Schema(topology.instance) + +def test_schema_reload(topo): + """Run a schema reload task + + :id: 995acc60-243b-45b0-9c1c-12bbe6a2882e + :setup: Standalone Instance + :steps: + 1. Add schema reload task + 2. Schema reload task succeeds + :expectedresults: + 1. Success + 2. Success + """ + + schema = Schema(topo.standalone) task = schema.reload() assert task.exists() task.wait() assert task.get_exit_code() == 0 +def test_x_origin(topo): + """ Test that the various forms of X-ORIGIN are handled correctly + + :id: 995acc60-243b-45b0-9c1c-12bbe6a2882d + :setup: Standalone Instance + :steps: + 1. Create schema file with specific/unique formats for X-ORIGIN + 2. Reload the schema file (schema reload task) + 3. List all attributes without error + 4. Confirm the expected results + :expectedresults: + 1. Success + 2. Success + 3. Success + 4. Success + """ + + # Create a custom schema file so we can tests specific formats + schema_file_name = topo.standalone.schemadir + '/98test.ldif' + schema_file = open(schema_file_name, "w") + testschema = ("dn: cn=schema\n" + + "attributetypes: ( 8.9.10.11.12.13.16 NAME 'testattr' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'USER_DEFINED' )\n" + + "attributetypes: ( 8.9.10.11.12.13.17 NAME 'testattrtwo' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN ( 'USER_DEFINED' 'should be not ignored!!' ) )\n") + schema_file.write(testschema) + schema_file.close() + + # Reload the schema + myschema = Schema(topo.standalone) + task = myschema.reload() + assert task.exists() + task.wait() + assert task.get_exit_code() == 0 + + # Now search attrs and make sure there are no errors + myschema.get_attributetypes() + myschema.get_objectclasses() + + # Check we have X-ORIGIN as expected + assert " 'USER_DEFINED' " in str(myschema.query_attributetype("testattr")) + assert " 'USER_DEFINED' " in str(myschema.query_attributetype("testattrtwo")) + + if __name__ == '__main__': # Run isolated # -s for DEBUG mode - import os CURRENT_FILE = os.path.realpath(__file__) - pytest.main("-s %s" % CURRENT_FILE) + pytest.main(["-s", CURRENT_FILE]) +