On 10/17/2012 03:28 PM, Michal Židek wrote:
On 10/16/2012 03:45 PM, Stef Walter wrote:
On 10/16/2012 02:04 PM, Jakub Hrozek wrote:
I was wondering for a while whether to change the behaviour directly in
confdb_get_string_as_list() but I think the attached patch takes a
better
approach because the other consumers of confdb_get_string_as_list() do
not see any difference between empty and missing parameter.

Yeah figures.

The patch works as advertized, there is just one compilation warning:

src/providers/simple/simple_access.c: In function
'get_conf_list_or_empty':
src/providers/simple/simple_access.c:284:9: warning: unused variable 'r'
[-Wunused-variable]

New patch attached.

Thanks for the review.

Stef


Hi Stef,

your patch solves the problem with *empty* 'simple_allow_users = ', but
it introduces new problem with *nonexistent* simple_allow_users. With
your patch, if no simple_allow_users list was specified, it behaves as
if empty simple_allow_users was provided and denies access to all users.

I think this new behaviour could brake existing configurations and it
also differs from the behaviour described in man pages.

Michal

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

I fixed the new problem and prepared patch that applies on top of this one.
attached

O.

--
Ondrej Kos
Associate Software Engineer
Identity Management
Red Hat Czech

phone: +420-532-294-558
cell:  +420-736-417-909
ext:   82-62558
loc:   1/5C Brno 1 office
irc:   okos @ #brno
From a81bfeed069e085258d1c84ef20fec68681fd59b Mon Sep 17 00:00:00 2001
From: Ondrej Kos <o...@redhat.com>
Date: Tue, 30 Oct 2012 16:48:14 +0100
Subject: [PATCH] Fix of stefw's patch for simple provider

The patch applies on top of "Recognize empty string lists in the
'simple' access provider" and corrects the behaviour when access
provider is set as 'simple' and allow/deny lists are not configured.
---
 src/providers/simple/simple_access.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/providers/simple/simple_access.c b/src/providers/simple/simple_access.c
index bd333cde06b07eb7f1d99879e8c0e7d57291a9f4..dd7e7f4c578e5b3dad648a9c2d69d1a4235d48ad 100644
--- a/src/providers/simple/simple_access.c
+++ b/src/providers/simple/simple_access.c
@@ -281,17 +281,21 @@ int get_conf_list_or_empty(struct confdb_ctx *cdb, TALLOC_CTX *ctx,
                            char ***result)
 {
     int ret;
+    char ***simple_rule_name = NULL;
 
-    ret = confdb_get_string_as_list(cdb, ctx, section, attribute, result);
+    simple_rule_name = talloc_zero(ctx, char **);
+    if (!simple_rule_name)
+        return ENOMEM;
 
-    if (ret == ENOENT) {
-        /*
-         * If config has an empty string value, then we want to return
-         * an empty string list, not ENOENT.
-         */
-        ret = confdb_get_param (cdb, ctx, section, attribute, result);
-        if (*result == NULL)
-            ret = ENOENT;
+    ret = confdb_get_param(cdb, ctx, section, attribute, simple_rule_name);
+    /*
+     * If config has an empty string value, then we want to return
+     * an empty string list, not ENOENT.
+     */
+    if (*simple_rule_name != NULL) {
+        ret = confdb_get_string_as_list(cdb, ctx, section, attribute, result);
+    } else {
+        ret = ENOENT;
     }
 
     return ret;
-- 
1.7.11.7

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to