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 > > [email protected] > > 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. _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
