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(a)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.
May be it should be "$none$" to express none to not ever collide with
allowed names.
Then an empty value would be treated as not defined i.e. everyone allowed.
However in this specific case does "none" really makes sense? Can you
imagine the real world use case that would lead to setting this option
to "none"? Nobody would be able to log into the system is this a use
case we need to worry about?
--
Thank you,
Dmitri Pal
Sr. Engineering Manager for IdM portfolio
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?