[Freeipa-devel] [PATCH] 0003 Honor the default home directory in user_add

[Martin Kosek]

On Mon, 2012-02-13 at 11:01 -0500, Simo Sorce wrote:
> On Mon, 2012-02-13 at 10:42 -0500, Rob Crittenden wrote:
> > Martin Kosek wrote:
> > > On Wed, 2012-02-08 at 08:22 -0500, Rob Crittenden wrote:
> > >> Martin Kosek wrote:
> > >>> On Tue, 2012-02-07 at 16:31 -0500, Rob Crittenden wrote:
> > >>>> Petr Viktorin wrote:
> > >>>>> On 02/07/2012 01:52 PM, Petr Viktorin wrote:
> > >>>>>> Honor the default home directory base when creating a new user. Test
> > >>>>>> included. I also cleaned up the way home directory was created.
> > >>>>>>
> > >>>>>> This patch removes the default from the --homedirectory option, letting
> > >>>>>> the server fill it in pre_callback. If I'm reading this correctly,
> > >>>>>> default_from and create_default run on the client-side, so they can't
> > >>>>>> get to the config without round-tripping to the server.
> > >>>>>>
> > >>>>>> https://fedorahosted.org/freeipa/ticket/2332
> > >>>>>>
> > >>>>>> Also, I've cleaned up the home directory generation to use
> > >>>>>> posixpath.join instead of '%s/%s' and ad-hoc cleanup. This should be
> > >>>>>> more robust. (It will also behave differently if the username starts
> > >>>>>> with '/' or maybe similar cases of the user asking for trouble.)
> > >>>>>>
> > >>>>>> A question: Do we want to use posixpath here, or os.path? Put another
> > >>>>>> way, should the home directories separated by '\' if the server runs on
> > >>>>>> Windows?
> > >>>>>>
> > >>>>>>
> > >>>>>> _______________________________________________
> > >>>>>> Freeipa-devel mailing list
> > >>>>>> Freeipa-devel at redhat.com
> > >>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
> > >>>>>
> > >>>>> Martin told me I need to make two changes: remove autofill along with
> > >>>>> default_from, and since I have touched the API, update API.txt.
> > >>>>>
> > >>>>> Attaching the updated patch.
> > >>>>
> > >>>> This works well. I noticed that the default shell has the same problem.
> > >>>>
> > >>>> I wonder if we should roll that similar change in or open a separate ticket.
> > >>>>
> > >>>> rob
> > >>>
> > >>> Hm, default shell works for me:
> > >>>
> > >>> # ipa config-mod --defaultshell=/bin/bash
> > >>> # ipa user-add --first=Foo --last=Bar fbar2
> > >>> ------------------
> > >>> Added user "fbar2"
> > >>> ------------------
> > >>>     User login: fbar2
> > >>>     First name: Foo
> > >>>     Last name: Bar
> > >>>     Full name: Foo Bar
> > >>>     Display name: Foo Bar
> > >>>     Initials: FB
> > >>>     Home directory: /home/fbar2
> > >>>     GECOS field: Foo Bar
> > >>>     Login shell: /bin/bash<<<<   config is honored
> > >>>     Kerberos principal: fbar2 at IDM.LAB.BOS.REDHAT.COM
> > >>>     UID: 480800097
> > >>>     GID: 480800097
> > >>>     Password: False
> > >>>     Member of groups: ipausers
> > >>>     Kerberos keys available: False
> > >>
> > >> Odd, I did exactly the same thing and got the wrong shell.
> > >>
> > >>> Oh, one more thing that came up to my mind when testing config plugin.
> > >>> Rob, why do we have config params as optional? We don't expect that the
> > >>> config attribute is missing in LDAP and IPA crashes in such cases (as in
> > >>> ticket 2159). IMO they should all be required.
> > >>
> > >> So that on a mod you don't have to provide all values. I think we need a
> > >> non-empty option.
> > >>
> > >> rob
> > >
> > > mod operation does not require all required options to be passed. You
> > > can simply update just one (required) attribute, it just must not be set
> > > to None - which is exactly what we want:
> > >
> > > # ipa config-mod --searchrecordslimit=
> > > ipa: ERROR: 'ipasearchrecordslimit' is required
> > > # ipa config-mod --searchrecordslimit=150
> > >    Maximum username length: 32
> > >    Home directory base: /home
> > >    Default shell: /bin/bash
> > >    Default users group: ipausers
> > >    Default e-mail domain: idm.lab.bos.redhat.com
> > >    Search time limit: 2
> > >    Search size limit: 150
> > >    User search fields: uid,givenname,sn,telephonenumber,ou,title
> > >    Group search fields: cn,description
> > >    Enable migration mode: FALSE
> > >    Certificate Subject base: O=IDM.LAB.BOS.REDHAT.COM
> > >    Password Expiration Notification (days): 4
> > >    SELinux user map order: guest_u:s0$xguest_u:s0$user_u:s0-s0:c0.c1023
> > > $staff_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023
> > >    Default SELinux user: guest_u:s0
> > >
> > > You can verify it with the attached testing patch. If you agree, I will
> > > create a new ticket to do this change and send a proper official patch
> > > for that.
> > >
> > > Martin
> > 
> > ACK. At one time this would cause all options to be prompted when 
> > executed interactively.
> 
> Wasn't this patch already pushed last week ?
> 
> Simo.
> 

That was just a patch fixing default shell and home directory. What Rob
acked was my proposal to fix params in config object to be required so
that user cannot remove the LDAP attribute. Most often we don't expect
that and crash badly.

Martin