URL: https://github.com/SSSD/sssd/pull/115
Author: sumit-bose
 Title: #115: libwbclient-sssd: wbcLookupSid() allow NULL arguments
Action: opened

PR body:
"""
Some caller might not be interested in some of the values wbcLookupSid()
returns and just pass NULL. Currently 'net ads user info' does this
because it is not interested in the domain. wbcLookupSid() should handle
this gracefully.

Resolves https://fedorahosted.org/sssd/ticket/3273

A unit-test for wbcLookupSid() is added as well. I had to wrap
sss_nss_getnamebysid() because the test shouldn't connect to SSSD but just
return some result. Interestingly the wrapping didn't work when I just added
libwbclient.la to _LDADD but I had to explicitly add the needed sources. Does
anyone know why?
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/115/head:pr115
git checkout pr115
From 135d9a073f5621abc904e7a3e326e7096dc42f96 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Thu, 22 Dec 2016 11:15:17 +0100
Subject: [PATCH] libwbclient-sssd: wbcLookupSid() allow NULL arguments

Some caller might not be interested in some of the values wbcLookupSid()
returns and just pass NULL. Currently 'net ads user info' does this
because it is not interested in the domain. wbcLookupSid() should handle
this gracefully.

Resolves https://fedorahosted.org/sssd/ticket/3273
---
 Makefile.am                               |  23 ++++++
 src/sss_client/libwbclient/wbc_sid_sssd.c |  38 ++++++----
 src/tests/cmocka/test_wbc_calls.c         | 122 ++++++++++++++++++++++++++++++
 3 files changed, 167 insertions(+), 16 deletions(-)
 create mode 100644 src/tests/cmocka/test_wbc_calls.c

diff --git a/Makefile.am b/Makefile.am
index 06dc628..07369fb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -243,6 +243,7 @@ if HAVE_CMOCKA
         test_sysdb_subdomains \
         test_sysdb_sudo \
         test_sysdb_utils \
+        test_wbc_calls \
         test_be_ptask \
         test_copy_ccache \
         test_copy_keytab \
@@ -2656,6 +2657,28 @@ test_sysdb_utils_LDADD = \
     libsss_test_common.la \
     $(NULL)
 
+test_wbc_calls_SOURCES = \
+    src/tests/cmocka/test_wbc_calls.c \
+    src/sss_client/idmap/sss_nss_idmap.c \
+    src/sss_client/libwbclient/wbc_sid_sssd.c \
+    src/sss_client/libwbclient/wbclient_common.c \
+    src/sss_client/libwbclient/wbc_sid_common.c \
+    src/sss_client/common.c \
+    $(NULL)
+test_wbc_calls_CFLAGS = \
+    $(AM_CFLAGS) \
+    $(NULL)
+test_wbc_calls_LDFLAGS = \
+    -Wl,-wrap,sss_nss_getnamebysid \
+    $(NULL)
+test_wbc_calls_LDADD = \
+    $(CMOCKA_LIBS) \
+    $(POPT_LIBS) \
+    $(TALLOC_LIBS) \
+    $(SSSD_INTERNAL_LTLIBS) \
+    libsss_test_common.la \
+    $(NULL)
+
 test_be_ptask_SOURCES = \
     src/tests/cmocka/common_mock_be.c \
     src/tests/cmocka/test_be_ptask.c \
diff --git a/src/sss_client/libwbclient/wbc_sid_sssd.c b/src/sss_client/libwbclient/wbc_sid_sssd.c
index cde65fa..3736d41 100644
--- a/src/sss_client/libwbclient/wbc_sid_sssd.c
+++ b/src/sss_client/libwbclient/wbc_sid_sssd.c
@@ -99,9 +99,9 @@ wbcErr wbcLookupName(const char *domain,
 
 /* Convert a SID to a domain and name */
 wbcErr wbcLookupSid(const struct wbcDomainSid *sid,
-            char **pdomain,
-            char **pname,
-            enum wbcSidType *pname_type)
+                    char **pdomain,
+                    char **pname,
+                    enum wbcSidType *pname_type)
 {
     char *str_sid;
     char *fq_name = NULL;
@@ -121,10 +121,12 @@ wbcErr wbcLookupSid(const struct wbcDomainSid *sid,
         return WBC_ERR_UNKNOWN_FAILURE;
     }
 
-    ret = sss_id_type_to_wbcSidType(type, pname_type);
-    if (ret != 0) {
-        wbc_status = WBC_ERR_UNKNOWN_FAILURE;
-        goto done;
+    if (pname_type != NULL) {
+        ret = sss_id_type_to_wbcSidType(type, pname_type);
+        if (ret != 0) {
+            wbc_status = WBC_ERR_UNKNOWN_FAILURE;
+            goto done;
+        }
     }
 
     /* TODO: it would be nice to have a sss_nss_getnamebysid() call which
@@ -136,17 +138,21 @@ wbcErr wbcLookupSid(const struct wbcDomainSid *sid,
     }
 
     *p = '\0';
-    *pname = wbcStrDup(fq_name);
-    if (*pname == NULL) {
-        wbc_status = WBC_ERR_NO_MEMORY;
-        goto done;
+    if (pname != NULL) {
+        *pname = wbcStrDup(fq_name);
+        if (*pname == NULL) {
+            wbc_status = WBC_ERR_NO_MEMORY;
+            goto done;
+        }
     }
 
-    *pdomain = wbcStrDup(p + 1);
-    if (*pdomain == NULL) {
-        wbcFreeMemory(*pname);
-        wbc_status = WBC_ERR_NO_MEMORY;
-        goto done;
+    if (pdomain != NULL) {
+        *pdomain = wbcStrDup(p + 1);
+        if (*pdomain == NULL) {
+            wbcFreeMemory(*pname);
+            wbc_status = WBC_ERR_NO_MEMORY;
+            goto done;
+        }
     }
 
     wbc_status = WBC_ERR_SUCCESS;
diff --git a/src/tests/cmocka/test_wbc_calls.c b/src/tests/cmocka/test_wbc_calls.c
new file mode 100644
index 0000000..7257770
--- /dev/null
+++ b/src/tests/cmocka/test_wbc_calls.c
@@ -0,0 +1,122 @@
+/*
+    SSSD
+
+    wbc-calls - Tests for selected libwbclient calls
+
+    Authors:
+        Sumit Bose <sb...@redhat.com>
+
+    Copyright (C) 2015 Red Hat
+
+    This program 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; either version 3 of the License, or
+    (at your option) any later version.
+
+    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/>.
+*/
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+#include <popt.h>
+
+#include "tests/cmocka/common_mock.h"
+
+#include "sss_client/libwbclient/wbclient_sssd.h"
+#include "sss_client/idmap/sss_nss_idmap.h"
+
+struct wbcDomainSid test_sid = {1, 5, {0, 0, 0, 0, 0, 5},
+                                {21, 2127521184, 1604012920, 1887927527, 72713,
+                                0, 0, 0, 0, 0, 0, 0, 0, 0, 0}};
+
+int __wrap_sss_nss_getnamebysid(const char *sid, char **fq_name,
+                                enum sss_id_type *type)
+{
+    *fq_name = strdup("name@domain");
+    assert_non_null(*fq_name);
+    *type = SSS_ID_TYPE_UID;
+
+    return EOK;
+}
+
+void test_wbcLookupSid(void **state)
+{
+    wbcErr wbc_status;
+    char *pdomain;
+    char *pname;
+    enum wbcSidType pname_type;
+
+    wbc_status = wbcLookupSid(NULL, NULL, NULL, NULL);
+    assert_int_equal(wbc_status, WBC_ERR_INVALID_SID);
+
+    wbc_status = wbcLookupSid(&test_sid, NULL, NULL, NULL);
+    assert_int_equal(wbc_status, WBC_ERR_SUCCESS);
+
+    wbc_status = wbcLookupSid(&test_sid, &pdomain, NULL, NULL);
+    assert_int_equal(wbc_status, WBC_ERR_SUCCESS);
+    assert_string_equal(pdomain, "domain");
+    wbcFreeMemory(pdomain);
+
+    wbc_status = wbcLookupSid(&test_sid, NULL, &pname,  NULL);
+    assert_int_equal(wbc_status, WBC_ERR_SUCCESS);
+    assert_string_equal(pname, "name");
+    wbcFreeMemory(pname);
+
+    wbc_status = wbcLookupSid(&test_sid, NULL, NULL, &pname_type);
+    assert_int_equal(wbc_status, WBC_ERR_SUCCESS);
+    assert_int_equal(pname_type, WBC_SID_NAME_USER);
+
+    wbc_status = wbcLookupSid(&test_sid, &pdomain, &pname, &pname_type);
+    assert_int_equal(wbc_status, WBC_ERR_SUCCESS);
+    assert_string_equal(pdomain, "domain");
+    assert_string_equal(pname, "name");
+    assert_int_equal(pname_type, WBC_SID_NAME_USER);
+    wbcFreeMemory(pdomain);
+    wbcFreeMemory(pname);
+}
+
+int main(int argc, const char *argv[])
+{
+    int rv;
+    poptContext pc;
+    int opt;
+    struct poptOption long_options[] = {
+        POPT_AUTOHELP
+        SSSD_DEBUG_OPTS
+        POPT_TABLEEND
+    };
+
+    const struct CMUnitTest tests[] = {
+        cmocka_unit_test(test_wbcLookupSid),
+    };
+
+    /* Set debug level to invalid value so we can deside if -d 0 was used. */
+    debug_level = SSSDBG_INVALID;
+
+    pc = poptGetContext(argv[0], argc, argv, long_options, 0);
+    while((opt = poptGetNextOpt(pc)) != -1) {
+        switch(opt) {
+        default:
+            fprintf(stderr, "\nInvalid option %s: %s\n\n",
+                    poptBadOption(pc, 0), poptStrerror(opt));
+            poptPrintUsage(pc, stderr, 0);
+            return 1;
+        }
+    }
+    poptFreeContext(pc);
+
+    DEBUG_CLI_INIT(debug_level);
+
+    tests_set_cwd();
+    rv = cmocka_run_group_tests(tests, NULL, NULL);
+
+    return rv;
+}
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to