On Thu, 2012-11-01 at 10:53 -0400, Dmitri Pal wrote: > On 11/01/2012 09:11 AM, Simo Sorce wrote: > > On Thu, 2012-11-01 at 12:03 +0100, Michal Židek wrote: > >> On 10/30/2012 04:53 PM, Ondrej Kos wrote: > >>> 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. > >>> > >> It is still not working correctly. With your patch *empty* > >> simple_allow_users list allows access to everyone (like it should be > >> with *nonexistent* list). > >> > >> The behaviour should be like this: > >> simple_allow_users = user1, user2 // allow access to user1 and user2 > >> simple_allow_users = // do not allow access to anyone > >> /* nothing */ // allow access to everyone > >> > >> > >> I think that the problem is that our .conf file parser ignores empty > >> values. Do we have ticket to track this issue? > > Nope, and it is not that easy to change IIRC. > > > > I think for now it would be easier to use a user name of 'none' to > > indicate none has access. The only issue would be if a user 'none' > > exists. > > > > Simo. > > > Is this hard because config DB pre-populates all the default values in > the DB and then overwrites it from INI file? > Is this the case?
It's because the DB cannot hold an empty value. With the current interface empty value = No value. > It seems that it should be easy to differentiate whether something is > defined and empty value. We might be able to do it, maybe we should have a ticket. > I do not think it is an INI problem. Seems like something is not done > correctly in the config DB layer. > IMO we should have a ticket to improve that if this is really a problem. It is a confdb limitation, but it has never been a problem before, we never had to make a difference, and in general I do not like much to use these semantics, I feel confused about this syntax if I put on my admin hat. Simo. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
