On Thu, Feb 26, 2015 at 11:26:13AM +0100, Lukas Slebodnik wrote:
> On (26/02/15 11:17), Jakub Hrozek wrote:
> > On Wed, Feb 25, 2015 at 11:53:00PM +0100, Lukas Slebodnik wrote:
> > > On (25/02/15 23:34), Jakub Hrozek wrote:
> > > > On Wed, Feb 25, 2015 at 11:29:20PM +0100, Lukas Slebodnik
> > > > wrote:
> > > > > On (25/02/15 20:57), Jakub Hrozek wrote:
> > > > > > On Fri, Jan 30, 2015 at 04:42:13PM +0100, Michal Židek
> > > > > > wrote:
> > > > > > >
https://fedorahosted.org/sssd/ticket/2569
> > > > > > >
> > > > > > > Simple patch is attached.
> > > > > > > I do not think that man page update is necessary.
> > > > > > >
> > > > > > > Michal
> > > > > >
> > > > > > I'm sorry the review took so long. Seems to work fine:
> > > > > > [jhrozek@client] sssd $ [(review)] getent passwd
> > > > > > administrator
administrator@ad.example.com:*:198600500:198600500:Administrator:/home/AD.EXAMPLE.COM/administrator:
> > > > > >
> > > > > > [jhrozek@client] sssd $ [(review)] getent passwd admin
> > > > > > [jhrozek@client] sssd $ [(review)] getent passwd
> > > > > > admin(a)ipa.example.com
admin@ipa.example.com:*:1546600000:1546600000:Administrator:/home/admin:/bin/bash
> > > > > >
> > > > > > [jhrozek@client] sssd $ [(review)] su -
> > > > > > admin(a)ipa.example.com
> > > > > > Password:
> > > > > > [admin@ipa.example.com(a)client ~]$ logout
> > > > > > [jhrozek@client] sssd $ [(review)] getent passwd admin
> > > > > > [jhrozek@client] sssd $ [(review)] su - administrator
> > > > > > Password:
> > > > > > -sh-4.3$ logout
> > > > > >
> > > > > > The code looks almost good to me, I will just put a
> > > > > > space between "if" and "(".
> > > > > >
> > > > > > CI link:
> > > > > >
http://sssd-ci.duckdns.org/logs/job/8/31/summary.html
> > > > > I would prefer to add debug message(SSSDBG_CONF_SETTINGS)
> > > > > that we use different
> > > > > default value for use_fully_qualified name.
> > > >
> > > > It's just internal issue, see below.
> > > >
> > > I do not see any reason why it cannot be logged.
> >
> > Fine, OK.
> >
> > >
> > > > >
> > > > > And it should be also documented in manual pages.
> > > >
> > > > default_domain_suffix documentation already says:
> > > > "Please note that if this option is set all users from the
> > > > primary domain have to use their fully qualified name, e.g.
> > > > user(a)domain.name, to log in"
> > > >
> > > > The fact that we say fqdn=true internally is just our
> > > > internal issue.
> > > It's not just an internal change.
> > >
> > > The result of this patch is that configuration line
> > > "use_fully_qualified_names = false" will be ignored if the
> > > option
> > > default_domain_suffix is configured
> > >
> > > And it is not documented.
> >
> > Yes, but that's an invalid configuration with
> > default_domain_suffix enabled. I don't feel too strongly,
> > though, I just wonder if documenting every corner case in man
> > pages can turn into noise...if you think it's important for
> > users to have this case documented, let's add it..
> If you don't want to document all corner cases then we should not
> create hacks (ignore/override user specified options).
>
> IMHO, We should just give an advice (message to syslog?) that user
> should
> enable the option use_fully_qualified_names if the option
> default_domain_suffix
> is configured.
>
> It does not mean I'm against current patch with updating
> documentation.
OK, let's update the documentation, then.
I'd also recommend that if default_domain_suffix is specified, it
should be a configuration error to set use_fully_qualified_names=False
and that SSSD should fail to start.