[Freeipa-devel] [PATCH 0167] ipa-client-install: Configure sudo to use SSSD as data source

[Jakub Hrozek]

On Wed, May 07, 2014 at 05:29:37PM +0200, Tomas Babej wrote:
> 
> On 04/30/2014 02:44 PM, Jakub Hrozek wrote:
> > On Wed, Apr 30, 2014 at 11:05:52AM +0200, Tomas Babej wrote:
> >> On 03/24/2014 03:27 PM, Jan Pazdziora wrote:
> >>> On Mon, Mar 24, 2014 at 02:57:30PM +0100, Martin Kosek wrote:
> >>>> On 03/24/2014 02:47 PM, Jan Pazdziora wrote:
> >>>>> On Mon, Mar 03, 2014 at 08:24:41PM +0100, Tomas Babej wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> Makes ipa-client-install configure SSSD as the data provider
> >>>>>> for the sudo service by default. This behaviour can be disabled
> >>>>>> by using --no-sudo flag.
> >>>>>>
> >>>>>> https://fedorahosted.org/freeipa/ticket/3358
> >>>>> Ack.
> >>>>>
> >>>>> Applied against ipa-client-3.0.0-37.el6.x86_64, tried without
> >>>>> --no-sudo and sudo was added to sssd.conf's services list and sudoeers
> >>>>> added to /etc/nsswitch.conf.
> >>>>>
> >>>>> Rerun with --uninstall and run again with the --no-sudo parameter,
> >>>>> those settings were not longer there.
> >>>>>
> >>>> Did you also do the functional test?
> >>> No. I do not want to get dragged into the discussion of having the
> >>> correct sssd and sudo and glibc versions and SELinux and stuff. The
> >>> ticket explicitly talk about setting configuration in config files,
> >>> which the patch does.
> >>>
> >>>> To ack and push this ticket, following
> >>>> scenario needs to work:
> >>> Consumption of those configuration changes is really different story,
> >>> isn't it?
> >>>
> >>>> 1) IPA clients enroll against IPA server without --no-sudo
> >>>> 2) IPA client user logs in, types "sudo -l", gets all allowed commands
> >>>> (prerequisite is of course to have sudo commands defined on the IPA server)
> >>>> 3) IPA client reboots, IPA client user logs in, types "sudo -l", gets all
> >>>> allowed commands
> >>>>
> >>>> For 2) to work, NIS domain name must be set, nsswitch and SSSD changes must be done
> >>>>
> >>>> For 3) to work, related systemd service preserving NIS domain name setting
> >>>> needs to be enabled
> >>> With the commit message only talking about configuring sssd, I assume
> >>> the NIS domain name mentioned in the ticket will be done by some other
> >>> patch.
> >>>
> >>> To me, the patch does what is advertised in the commit message, and is
> >>> in line with what the ticket asks to be done.
> >>>
> >> Attached are rebased versions of the patches 113 and 167 (which was
> >> marked as 157 in the thread previously by mistake).
> >>
> >> There is a slight behaviour change in 167, if there is no sudoers line
> >> in nsswitch.conf, we add both files and sss as sudoers sources.
> >>
> >> I also developed CI test that covers the functionality of the IPA - sudo
> >> integration feature, which is attached.
> >>
> >> Please note that the last three tests are expected to fail until:
> >>
> >> https://fedorahosted.org/freeipa/ticket/4324
> >>
> >> is fixed.
> >>
> >> -- 
> >> Tomas Babej
> >> Associate Software Engineer | Red Hat | Identity Management
> >> RHCE | Brno Site | IRC: tbabej | freeipa.org 
> >>
> > Hi,
> >
> > I haven't done a thorough review, but the patch looks good to me in
> > general -- in other words, seems to cover what I've been doing manually
> > for my test setups.
> >
> > My only suggestion (maybe for future) would be to split changing the
> > nsswitch.conf into its own separate helper class or a function, because
> > you might want to do the same change for automount or other services in
> > nsswitch.conf.
> >
> > But I think this version is OK at the moment.
> >
> > _______________________________________________
> > Freeipa-devel mailing list
> > Freeipa-devel at redhat.com
> > https://www.redhat.com/mailman/listinfo/freeipa-devel
> 
> I created a rather general function for editing the nsswitch.conf as
> requesting.
> 
> Updated patch attached.

Thanks, looks good to me, although I haven't done a thorough review.