URL: https://github.com/SSSD/sssd/pull/609
Author: jhrozek
 Title: #609: SUDO: Don't save duplicates when saving qualified names
Action: opened

PR body:
"""
The sudoUser attribute which is part of the sudo rule can contain any name
that sudo can parse on the LDAP side. Internally, however, the attribute is
always qualified with the name of the SSSD domain.

This patch makes sure that if two or more sudoUser attributes contain the
same name in both qualified and an unqualified form, the rule is actually
saved. Previously, the rule would have failed to be saved and the sysdb
sudo code would have errored out with EEXIST.

Resolves: https://pagure.io/SSSD/sssd/issue/3596
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/609/head:pr609
git checkout pr609
From cb7467ffa5841a17da5720ef5de62812a97cc764 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 5 Jun 2018 10:32:06 +0200
Subject: [PATCH 1/5] sudo testcli: Use hand-crafted JSON for output so that
 the test CLI is usable in tests

The sudo testcli tool can be handy in tests, but currently its output is
hard to process from a program. This patch makes the tool print an JSON
output instead, which will make it more usable.

Related:
https://pagure.io/SSSD/sssd/issue/3596
---
 src/sss_client/sudo_testcli/sudo_testcli.c | 50 +++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/src/sss_client/sudo_testcli/sudo_testcli.c b/src/sss_client/sudo_testcli/sudo_testcli.c
index 9786c02fc..948b8b22e 100644
--- a/src/sss_client/sudo_testcli/sudo_testcli.c
+++ b/src/sss_client/sudo_testcli/sudo_testcli.c
@@ -75,15 +75,14 @@ int main(int argc, char **argv)
         goto fail;
     }
 
-    printf("User [%s:%llu] found in domain: %s\n\n",
-            username, (unsigned long long)uid,
-            domainname != NULL ? domainname : "<NULL>");
-
-    printf("=== Printing response data [default options] ===\n");
-    printf("Response code: %d\n\n", error);
+    printf("[\n");
+    printf("\t{\n");
+    printf("\t\t\"type\": \"default\",\n");
+    printf("\t\t\"retval\": %d,\n", error);
     if (error == SSS_SUDO_ERROR_OK) {
         print_sss_result(result);
     }
+    printf("\t},\n");
 
     sss_sudo_free_result(result);
     result = NULL;
@@ -96,11 +95,14 @@ int main(int argc, char **argv)
         goto fail;
     }
 
-    printf("\n=== Printing response data [rules] ===\n");
-    printf("Response code: %d\n\n", error);
+    printf("\t{\n");
+    printf("\t\t\"type\": \"rules\",\n");
+    printf("\t\t\"retval\": %d,\n", error);
     if (error == SSS_SUDO_ERROR_OK) {
         print_sss_result(result);
     }
+    printf("\t}\n");
+    printf("]\n");
 
 
     free(domainname);
@@ -121,19 +123,37 @@ void print_sss_result(struct sss_sudo_result *result)
     int j = 0;
     int k = 0;
 
-    printf("Number of rules: %d\n", result->num_rules);
-
+    printf("\t\t\"result\": {\n");
+    printf("\t\t\t\"num_rules\": %d,\n", result->num_rules);
+    printf("\t\t\t\"rules\": [\n");
     for (i = 0; i < result->num_rules; i++) {
         rule = &result->rules[i];
-        printf("=== Rule %d has %d attributes\n", i, rule->num_attrs);
+        printf("\t\t\t\t{\n");
         for (j = 0; j < rule->num_attrs; j++) {
             attr = &rule->attrs[j];
-            printf("   === Attribute named %s has %d values:\n", attr->name,
-                   attr->num_values);
+            printf("\t\t\t\t\t\"%s\": ", attr->name);
+            if (attr->num_values > 1) {
+                printf("[ ");
+                printf("\"%s\"", attr->values[0]);
+                for (k = 1; k < attr->num_values; k++) {
+                    printf(", \"%s\"", attr->values[k]);
+                }
+                printf(" ]");
+            } else {
+                printf("\"%s\"", attr->values[0]);
+            }
 
-            for (k = 0; k < attr->num_values; k++) {
-                printf("       %s\n", attr->values[k]);
+            if (j < rule->num_attrs - 1) {
+                printf(",");
             }
+            printf("\n");
+        }
+        printf("\t\t\t\t}");
+        if (i < result->num_rules - 1) {
+            printf(",");
         }
+        printf("\n");
     }
+    printf("\t\t\t]\n");
+    printf("\t\t}\n");
 }

From dbd5a4780b2d4b8810e0d9f96af48b8331ff01f1 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 5 Jun 2018 10:32:40 +0200
Subject: [PATCH 2/5] TESTS: Load the sudo schema in the default OpenLDAP test
 instance and create ou=sudoers

This will allow us to store sudo entries in our OpenLDAP test instances.

Related:
https://pagure.io/SSSD/sssd/issue/3596
---
 src/tests/intg/Makefile.am           |  1 +
 src/tests/intg/data/sudo_schema.ldif | 11 +++++++++++
 src/tests/intg/ds_openldap.py        | 13 +++++++++++++
 3 files changed, 25 insertions(+)
 create mode 100644 src/tests/intg/data/sudo_schema.ldif

diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am
index 126ca6cda..98eb31470 100644
--- a/src/tests/intg/Makefile.am
+++ b/src/tests/intg/Makefile.am
@@ -34,6 +34,7 @@ dist_noinst_DATA = \
     test_pac_responder.py \
     data/ad_data.ldif \
     data/ad_schema.ldif \
+    data/sudo_schema.ldif \
     test_pysss_nss_idmap.py \
     test_infopipe.py \
     test_ssh_pubkey.py \
diff --git a/src/tests/intg/data/sudo_schema.ldif b/src/tests/intg/data/sudo_schema.ldif
new file mode 100644
index 000000000..8c1f4e3ef
--- /dev/null
+++ b/src/tests/intg/data/sudo_schema.ldif
@@ -0,0 +1,11 @@
+dn: cn=sudo,cn=schema,cn=config
+objectClass: olcSchemaConfig
+cn: sudo
+olcAttributeTypes: ( 1.3.6.1.4.1.15953.9.1.1 NAME 'sudoUser' DESC 'User(s) who may  run sudo' EQUALITY caseExactIA5Match SUBSTR caseExactIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+olcAttributeTypes: ( 1.3.6.1.4.1.15953.9.1.2 NAME 'sudoHost' DESC 'Host(s) who may run sudo' EQUALITY caseExactIA5Match SUBSTR caseExactIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+olcAttributeTypes: ( 1.3.6.1.4.1.15953.9.1.3 NAME 'sudoCommand' DESC 'Command(s) to be executed by sudo' EQUALITY caseExactIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+olcAttributeTypes: ( 1.3.6.1.4.1.15953.9.1.4 NAME 'sudoRunAs' DESC 'User(s) impersonated by sudo (deprecated)' EQUALITY caseExactIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+olcAttributeTypes: ( 1.3.6.1.4.1.15953.9.1.5 NAME 'sudoOption' DESC 'Options(s) followed by sudo' EQUALITY caseExactIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+olcAttributeTypes: ( 1.3.6.1.4.1.15953.9.1.6 NAME 'sudoRunAsUser' DESC 'User(s) impersonated by sudo' EQUALITY caseExactIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+olcAttributeTypes: ( 1.3.6.1.4.1.15953.9.1.7 NAME 'sudoRunAsGroup' DESC 'Group(s) impersonated by sudo' EQUALITY caseExactIA5Match SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 )
+olcObjectClasses: ( 1.3.6.1.4.1.15953.9.2.1 NAME 'sudoRole' SUP top STRUCTURAL DESC 'Sudoer Entries' MUST ( cn ) MAY ( sudoUser $ sudoHost $ sudoCommand $ sudoRunAs $ sudoRunAsUser $ sudoRunAsGroup $ sudoOption $ description ) )
diff --git a/src/tests/intg/ds_openldap.py b/src/tests/intg/ds_openldap.py
index c9a4b6de8..466db052c 100644
--- a/src/tests/intg/ds_openldap.py
+++ b/src/tests/intg/ds_openldap.py
@@ -192,6 +192,12 @@ def _setup_config(self):
              "-l", "data/ssh_schema.ldif"],
         )
 
+        # Import sudo schema
+        subprocess.check_call(
+            ["slapadd", "-F", self.conf_slapd_d_dir, "-b", "cn=config",
+             "-l", "data/sudo_schema.ldif"],
+        )
+
     def _start_daemon(self):
         """Start the instance."""
         if subprocess.call(["slapd", "-F", self.conf_slapd_d_dir,
@@ -272,6 +278,13 @@ def setup(self):
             ldap_conn.add_s("ou=" + ou + "," + self.base_dn, [
                 ("objectClass", [b"top", b"organizationalUnit"]),
             ])
+        ldap_conn.add_s("ou=sudoers," + self.base_dn, [
+            ("objectClass", [b"top", b"organizationalUnit"]),
+        ])
+        ldap_conn.add_s("cn=testrule,ou=sudoers," + self.base_dn, [
+            ("objectClass", [b"top", b"sudoRole"]),
+            ("sudoUser", [b"tuser"]),
+        ])
         ldap_conn.unbind_s()
 
     def _stop_daemon(self):

From 9e044571f1fc43d42a3eeb5fa6c094b33aacf4f5 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 5 Jun 2018 10:33:38 +0200
Subject: [PATCH 3/5] TESTS: Add API to add sudo rules in tests

Actually adds an API that allows the programmer to store sudoRole
objects in LDAP.

Related:
https://pagure.io/SSSD/sssd/issue/3596
---
 src/tests/intg/ldap_ent.py | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py
index 1f23e3ab7..80e769835 100644
--- a/src/tests/intg/ldap_ent.py
+++ b/src/tests/intg/ldap_ent.py
@@ -113,6 +113,27 @@ def netgroup(base_dn, cn, triples=(), members=()):
     return ("cn=" + cn + ",ou=Netgroups," + base_dn, attr_list)
 
 
+def sudo_rule(base_dn, name, users=(), hosts=(), commands=()):
+    """
+    Generate a sudo rule for passing to ldap.add*
+    """
+    attr_list = [
+        ('objectClass', [b'top', b'sudoRole']),
+        ('cn', [name.encode('utf-8')])
+    ]
+
+    if len(users) > 0:
+        sudo_user_list = [u.encode('utf-8') for u in users]
+        attr_list.append(('sudoUser', sudo_user_list))
+    if len(hosts) > 0:
+        sudo_host_list = [h.encode('utf-8') for h in hosts]
+        attr_list.append(('sudoHost', sudo_host_list))
+    if len(commands) > 0:
+        sudo_command_list = [cmd.encode('utf-8') for cmd in commands]
+        attr_list.append(('sudoCommand', sudo_command_list))
+    return ("cn=" + name + ",ou=sudoers," + base_dn, attr_list)
+
+
 class List(list):
     """LDAP add-modlist list"""
 
@@ -159,3 +180,9 @@ def add_netgroup(self, cn, triples=(), members=(), base_dn=None):
         """Add an RFC2307bis netgroup add-modlist."""
         self.append(netgroup(base_dn or self.base_dn,
                              cn, triples, members))
+
+    def add_sudo_rule(self, name,
+                      users=(), hosts=(), commands=(),
+                      base_dn=None):
+        self.append(sudo_rule(base_dn or self.base_dn,
+                              name, users, hosts, commands))

From c6f299b177f4022486f14b78bb481048121c2f8b Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 5 Jun 2018 10:33:56 +0200
Subject: [PATCH 4/5] TESTS: Add a simple sudo LDAP test

Adds the most basic SUDO LDAP tests that makes sure a user specified in
a sudo rule can execute sudo and a user not specifed cannot.

Related:
https://pagure.io/SSSD/sssd/issue/3596
---
 src/tests/intg/Makefile.am  |   1 +
 src/tests/intg/test_sudo.py | 244 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 245 insertions(+)
 create mode 100644 src/tests/intg/test_sudo.py

diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am
index 98eb31470..75a8300a1 100644
--- a/src/tests/intg/Makefile.am
+++ b/src/tests/intg/Makefile.am
@@ -38,6 +38,7 @@ dist_noinst_DATA = \
     test_pysss_nss_idmap.py \
     test_infopipe.py \
     test_ssh_pubkey.py \
+    test_sudo.py \
     $(NULL)
 
 EXTRA_DIST = data/cwrap-dbus-system.conf.in
diff --git a/src/tests/intg/test_sudo.py b/src/tests/intg/test_sudo.py
new file mode 100644
index 000000000..36cce2eb0
--- /dev/null
+++ b/src/tests/intg/test_sudo.py
@@ -0,0 +1,244 @@
+#
+# Sudo integration test
+#
+# Copyright (c) 2018 Red Hat, Inc.
+#
+# This is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 only
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import os
+import stat
+import signal
+import subprocess
+import time
+import ldap
+import pytest
+import json
+
+import config
+import ds_openldap
+import ldap_ent
+from util import unindent, get_call_output
+
+LDAP_BASE_DN = "dc=example,dc=com"
+
+
+class SudoReplyElement:
+    def __init__(self, retval, rules):
+        self.retval = retval
+        self.rules = rules
+
+
+class SudoReply:
+    def __init__(self, json_string):
+        self.jres = json.loads(json_string)
+        for reply_elem in self.jres:
+            el = SudoReplyElement(reply_elem['retval'],
+                                  reply_elem['result']['rules'])
+            if reply_elem['type'] == 'default':
+                self.defaults = el
+            if reply_elem['type'] == 'rules':
+                self.sudo_rules = el
+
+
+@pytest.fixture(scope="module")
+def ds_inst(request):
+    """LDAP server instance fixture"""
+    ds_inst = ds_openldap.DSOpenLDAP(
+        config.PREFIX, 10389, LDAP_BASE_DN,
+        "cn=admin", "Secret123"
+    )
+
+    try:
+        ds_inst.setup()
+    except:
+        ds_inst.teardown()
+        raise
+    request.addfinalizer(ds_inst.teardown)
+    return ds_inst
+
+
+@pytest.fixture(scope="module")
+def ldap_conn(request, ds_inst):
+    """LDAP server connection fixture"""
+    ldap_conn = ds_inst.bind()
+    ldap_conn.ds_inst = ds_inst
+    request.addfinalizer(ldap_conn.unbind_s)
+    return ldap_conn
+
+
+def create_ldap_entries(ldap_conn, ent_list=None):
+    """Add LDAP entries from ent_list"""
+    if ent_list is not None:
+        for entry in ent_list:
+            ldap_conn.add_s(entry[0], entry[1])
+
+
+def cleanup_ldap_entries(ldap_conn, ent_list=None):
+    """Remove LDAP entries added by create_ldap_entries"""
+    if ent_list is None:
+        for ou in ("Users", "Groups", "Netgroups", "Services", "Policies"):
+            for entry in ldap_conn.search_s("ou=" + ou + "," +
+                                            ldap_conn.ds_inst.base_dn,
+                                            ldap.SCOPE_ONELEVEL,
+                                            attrlist=[]):
+                ldap_conn.delete_s(entry[0])
+    else:
+        for entry in ent_list:
+            ldap_conn.delete_s(entry[0])
+
+
+def create_ldap_cleanup(request, ldap_conn, ent_list=None):
+    """Add teardown for removing all user/group LDAP entries"""
+    request.addfinalizer(lambda: cleanup_ldap_entries(ldap_conn, ent_list))
+
+
+def create_ldap_fixture(request, ldap_conn, ent_list=None):
+    """Add LDAP entries and add teardown for removing them"""
+    create_ldap_entries(ldap_conn, ent_list)
+    create_ldap_cleanup(request, ldap_conn, ent_list)
+
+
+SCHEMA_RFC2307_BIS = "rfc2307bis"
+
+
+def format_basic_conf(ldap_conn, schema):
+    """Format a basic SSSD configuration"""
+    schema_conf = "ldap_schema         = " + schema + "\n"
+    schema_conf += "ldap_group_object_class = groupOfNames\n"
+    return unindent("""\
+        [sssd]
+        domains             = LDAP
+        services            = nss, sudo
+
+        [nss]
+
+        [sudo]
+        debug_level=10
+
+        [domain/LDAP]
+        {schema_conf}
+        id_provider         = ldap
+        auth_provider       = ldap
+        ldap_uri            = {ldap_conn.ds_inst.ldap_url}
+        ldap_search_base    = {ldap_conn.ds_inst.base_dn}
+        ldap_sudo_use_host_filter = false
+        debug_level=10
+    """).format(**locals())
+
+
+def create_conf_file(contents):
+    """Create sssd.conf with specified contents"""
+    conf = open(config.CONF_PATH, "w")
+    conf.write(contents)
+    conf.close()
+    os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR)
+
+
+def cleanup_conf_file():
+    """Remove sssd.conf, if it exists"""
+    if os.path.lexists(config.CONF_PATH):
+        os.unlink(config.CONF_PATH)
+
+
+def create_conf_cleanup(request):
+    """Add teardown for removing sssd.conf"""
+    request.addfinalizer(cleanup_conf_file)
+
+
+def create_conf_fixture(request, contents):
+    """
+    Create sssd.conf with specified contents and add teardown for removing it
+    """
+    create_conf_file(contents)
+    create_conf_cleanup(request)
+
+
+def create_sssd_process():
+    """Start the SSSD process"""
+    if subprocess.call(["sssd", "-D", "-f"]) != 0:
+        raise Exception("sssd start failed")
+
+
+def get_sssd_pid():
+    pid_file = open(config.PIDFILE_PATH, "r")
+    pid = int(pid_file.read())
+    return pid
+
+
+def cleanup_sssd_process():
+    """Stop the SSSD process and remove its state"""
+    try:
+        pid = get_sssd_pid()
+        os.kill(pid, signal.SIGTERM)
+        while True:
+            try:
+                os.kill(pid, signal.SIGCONT)
+            except:
+                break
+            time.sleep(1)
+    except:
+        pass
+    for path in os.listdir(config.DB_PATH):
+        os.unlink(config.DB_PATH + "/" + path)
+    for path in os.listdir(config.MCACHE_PATH):
+        os.unlink(config.MCACHE_PATH + "/" + path)
+
+
+def create_sssd_fixture(request):
+    """Start SSSD and add teardown for stopping it and removing its state"""
+    create_sssd_process()
+    create_sssd_cleanup(request)
+
+
+def create_sssd_cleanup(request):
+    """Add teardown for stopping SSSD and removing its state"""
+    request.addfinalizer(cleanup_sssd_process)
+
+
+@pytest.fixture()
+def sudocli_tool(request):
+    sudocli_path = os.path.join(config.ABS_BUILDDIR,
+                                "..", "..", "..", "sss_sudo_cli")
+    assert os.access(sudocli_path, os.X_OK)
+    return sudocli_path
+
+
+@pytest.fixture
+def add_common_rules(request, ldap_conn):
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_user("user1", 1001, 2001)
+    ent_list.add_user("user2", 1001, 2001)
+    ent_list.add_sudo_rule("user1_allow_less_shadow",
+                           users=("user1",),
+                           hosts=("ALL",),
+                           commands=("/usr/bin/less /etc/shadow", "/bin/ls"))
+    create_ldap_fixture(request, ldap_conn, ent_list)
+    conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+def test_sudo_rule_for_user(add_common_rules, sudocli_tool):
+    """
+    Test that user1 is allowed in the rule but user2 is not
+    """
+    user1_rules = get_call_output([sudocli_tool, "user1"])
+    reply = SudoReply(user1_rules)
+    assert len(reply.sudo_rules.rules) == 1
+    assert reply.sudo_rules.rules[0]['cn'] == 'user1_allow_less_shadow'
+
+    user2_rules = get_call_output([sudocli_tool, "user2"])
+    reply = SudoReply(user2_rules)
+    assert len(reply.sudo_rules.rules) == 0

From 06f17154a059daa3bb156d3aee543af84f8a215d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 5 Jun 2018 14:11:19 +0200
Subject: [PATCH 5/5] SUDO: Don't save duplicates when saving qualified names

The sudoUser attribute which is part of the sudo rule can contain any
name that sudo can parse on the LDAP side. Internally, however, the
attribute is always qualified with the name of the SSSD domain.

This patch makes sure that if two or more sudoUser attributes contain
the same name in both qualified and an unqualified form, the rule is
actually saved. Previously, the rule would have failed to be saved and
the sysdb sudo code would have errored out with EEXIST.

Resolves:
https://pagure.io/SSSD/sssd/issue/3596
---
 src/providers/ldap/sdap_async_sudo.c | 44 ++++++++++++++++++++++++++++++------
 src/tests/intg/test_sudo.py          | 36 +++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/src/providers/ldap/sdap_async_sudo.c b/src/providers/ldap/sdap_async_sudo.c
index c20132a01..5ccfad61f 100644
--- a/src/providers/ldap/sdap_async_sudo.c
+++ b/src/providers/ldap/sdap_async_sudo.c
@@ -499,6 +499,7 @@ static errno_t sdap_sudo_qualify_names(struct sss_domain_info *dom,
     char *domain;
     char *name;
     const char *orig_name;
+    struct ldb_message_element unique_el;
 
     for (size_t i = 0; i < rules_count; i++) {
         ret = sysdb_attrs_get_el_ext(rules[i], SYSDB_SUDO_CACHE_AT_USER,
@@ -507,11 +508,20 @@ static errno_t sdap_sudo_qualify_names(struct sss_domain_info *dom,
             continue;
         }
 
+        unique_el.values = talloc_zero_array(rules, struct ldb_val, el->num_values);
+        if (unique_el.values == NULL) {
+            return ENOMEM;
+        }
+        unique_el.num_values = 0;
+
         for (size_t ii = 0; ii < el->num_values; ii++) {
             orig_name = (const char *) el->values[ii].data;
 
             qualify = is_user_or_group_name(orig_name);
             if (qualify) {
+                struct ldb_val fqval;
+                struct ldb_val *dup;
+
                 ret = sss_parse_name(rules, dom->names, orig_name,
                                      &domain, &name);
                 if (ret != EOK) {
@@ -526,19 +536,39 @@ static errno_t sdap_sudo_qualify_names(struct sss_domain_info *dom,
                     }
                 }
 
-                el->values[ii].data = (uint8_t * ) sss_create_internal_fqname(
-                                                                    rules,
-                                                                    name,
-                                                                    domain);
+                fqval.data = (uint8_t * ) sss_create_internal_fqname(rules,
+                                                                     name,
+                                                                     domain);
                 talloc_zfree(domain);
                 talloc_zfree(name);
-                if (el->values[ii].data == NULL) {
+                if (fqval.data == NULL) {
                     return ENOMEM;
                 }
-                el->values[ii].length = strlen(
-                                        (const char *) el->values[ii].data);
+                fqval.length = strlen((const char *) fqval.data);
+
+                /* Prevent saving duplicates in case the sudo rule contains
+                 * e.g. foo and foo@domain
+                 */
+                dup = ldb_msg_find_val(&unique_el, &fqval);
+                if (dup != NULL) {
+                    DEBUG(SSSDBG_TRACE_FUNC,
+                          "Discarding duplicate value %s\n", (const char *) fqval.data);
+                    talloc_free(fqval.data);
+                    continue;
+                }
+                unique_el.values[unique_el.num_values].data = talloc_steal(unique_el.values, fqval.data);
+                unique_el.values[unique_el.num_values].length = fqval.length;
+                unique_el.num_values++;
+            } else {
+                unique_el.values[unique_el.num_values] = ldb_val_dup(unique_el.values,
+                                                                     &el->values[ii]);
+                unique_el.num_values++;
             }
         }
+
+        talloc_zfree(el->values);
+        el->values = unique_el.values;
+        el->num_values = unique_el.num_values;
     }
 
     return EOK;
diff --git a/src/tests/intg/test_sudo.py b/src/tests/intg/test_sudo.py
index 36cce2eb0..8f3d8be3e 100644
--- a/src/tests/intg/test_sudo.py
+++ b/src/tests/intg/test_sudo.py
@@ -242,3 +242,39 @@ def test_sudo_rule_for_user(add_common_rules, sudocli_tool):
     user2_rules = get_call_output([sudocli_tool, "user2"])
     reply = SudoReply(user2_rules)
     assert len(reply.sudo_rules.rules) == 0
+
+
+@pytest.fixture
+def add_double_qualified_rules(request, ldap_conn):
+    ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
+    ent_list.add_user("user1", 1001, 2001)
+    ent_list.add_user("user2", 1002, 2001)
+    ent_list.add_user("user3", 1003, 2001)
+    ent_list.add_user("user4", 1004, 2001)
+    ent_list.add_sudo_rule("user1_allow_less_shadow",
+                           users=("user1", "user2", "user2@LDAP", "user3"),
+                           hosts=("ALL",),
+                           commands=("/usr/bin/less /etc/shadow", "/bin/ls"))
+    create_ldap_fixture(request, ldap_conn, ent_list)
+    conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS)
+    create_conf_fixture(request, conf)
+    create_sssd_fixture(request)
+    return None
+
+
+def test_sudo_rule_duplicate_sudo_user(add_double_qualified_rules,
+                                       sudocli_tool):
+    """
+    Test that despite user1 and user1@LDAP meaning the same user,
+    the rule is still usable
+    """
+    # Try several users to make sure we don't mangle the list
+    for u in ["user1", "user2", "user3"]:
+        user_rules = get_call_output([sudocli_tool, u])
+        reply = SudoReply(user_rules)
+        assert len(reply.sudo_rules.rules) == 1
+        assert reply.sudo_rules.rules[0]['cn'] == 'user1_allow_less_shadow'
+
+    user4_rules = get_call_output([sudocli_tool, "user4"])
+    reply = SudoReply(user4_rules)
+    assert len(reply.sudo_rules.rules) == 0
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/C35X72MQBINTDJ5EBMBLLBREDGZY3IBR/

Reply via email to