On Tue, 2012-11-06 at 14:46 +0100, Ondrej Kos wrote: > On 11/02/2012 05:32 PM, Simo Sorce wrote: > > On Fri, 2012-11-02 at 10:10 -0400, Dmitri Pal wrote: > >> On 11/02/2012 09:50 AM, Stef Walter wrote: > >>> On 11/02/2012 01:57 PM, Dmitri Pal wrote: > >>>> First let us define a general rule about how we treat the cases: > >>>> X = > >>>> Is it treated as X being undefined or X having an empty value. > >>>> It should be a general documented rule for the application. > >>>> > >>>> Current behavior is to ignore and I think it is the right one. > >>>> May be it should be clearly stated in the man for sssd.conf if not yet > >>>> stated. > >>> Understood. > >>> > >>> But to me it seems that this policy does not make sense in this > >>> situation. It's completely counter intuitive that the following, instead > >>> of allowing no users, allows any user. > >>> > >>> simple_allow_users = > >>> > >>> Sure, you can say that anyone deploying sssd, should read over every > >>> line of the documentation and be well informed. > >> > >> One does not preclude the other. The point about man pages is just to > >> make a statement explicit not implied. I do not expect everyone to read > >> it but it is better to be explicit. > >>> But there's also > >>> something to be said for readability and principle-of-least-surprise. > >> > >> Sure. Whatever we decide should be consistent and logical. > >> I seems that so far we had the rule that empty values are ignored. > >> Is it consistent and logical to change it for one attribute? > >> I doubt but your argument is valid too. > >> May be the way out is to have a different attribute > >> simple_default_allow_rule = [all] | [none] > >> By default it will be "all" meaning that if the simple_allow_users is > >> not specified or empty then all users are allowed - current behavior > >> If it is set to none then if the filter is not specified no one is allowed. > >> > >> Would that be more logical? > > > > I was thinking along these lines too. > > > > If we have a default of ALL, then 'none' is not necessary at all as long > > as a 'simple_allow_users = ' line in the conf causes the attribute to be > > completely removed from confdb > > at that point the code, not finding the special default of 'ALL' would > > cause everyone to be denied as Stef expects. > > The bonus is that an admin can explicitly allow all by setting ALL, > > which is also clearer than having to delete the configure attribute or > > disable the access check plugin. > > > > I do not like the $ suggestion as in Windows machine names have the $ > > sign in it so $none$ and none$ would be valid yet easily confused. > > > > 'ALL' is used in many tools so it is unlikely someone will have an upper > > case user or group name named that way, and if so .. well I guess they > > will have to cope. > > > > There is one other character that nobosy uses in usernames and that is > > ':' because historically that's the spearator in /etc/passwd so it > > simply can't be used in the user name. > > > > So maybe ':ALL' is also a possibility, or maybe we consider ':ALL' the > > 'escaping' version to be able to reference and actual entity called > > 'ALL' (I would prefer the escaping rule). > > > > Simo. > > > > > So should i prepare patch introducing this behaviour? > > - add configuration option > simple_default_allow_rule = [true|false] / [all|-?-] > > - if <false> > simple_allow_* list would be expected to contain some users/groups, > empty list results in no-one being allowed. > > - if <true|all> > all users are allowed by default, limitations are derived from > simple_deny_* lists. > > > is this correct?
Nope > or should we use something like: This is what we were proposing. > simple_allow_users = <ALL_keyword> | <user list> > > and not add the new option at all? in that case, what would be the > keyword and how should we handle the situation, when the configuration > file would contain the following: > > simple_allow_users = <ALL_keyword>, user1, user2, ... users here would simply be redunant as the ALL keyword alreay matches everyone. > also this method would require *simple_allow_users* rule to be present, > or would be handled as empty. when empty it will mean that nobody is allowed. (the default internally if not present at all is 'ALL' 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
