[Freeipa-devel] [PATCH] 917 user automember for ipa default user

[Rob Crittenden]

Martin Kosek wrote:
> On Mon, 2012-01-16 at 15:43 -0500, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> On Mon, 2011-12-12 at 23:09 -0500, Rob Crittenden wrote:
>>>> Rob Crittenden wrote:
>>>>> Rather than manually adding users to the default ipa users group
>>>>> configure automember to do it for us.
>>>>>
>>>>> This was quite simple for new installs but a bit complex on upgrades so
>>>>> I implemented it as an update plugin.
>>>>>
>>>>> I also added a unit test for the config module. The majority of config
>>>>> is ignored for now. I'm afraid we'd run into too many false positives if
>>>>> we test each element, and most of these just store data so there isn't a
>>>>> lot that can go wrong.
>>>>>
>>>>> rob
>>>>
>>>> Small revision. I wasn't shipping the update plugin.
>>>>
>>>> rob
>>>
>>> I have few minor-ish issues:
>>>
>>> 0) I was thinking if this new approach for assignment of ipa default
>>> users is safe enough. If user accidentally mess with automember and
>>> modifies/deletes the default group rule, new users may be omitted from
>>> the default group set in IPA config. Are we sure that we are OK with
>>> this?
>>
>> I made some stricter tests that don't allow users to manage the
>> conditions of the default users group nor use an existing rule with
>> conditions for the default users group.
>>
>>> 1) Several tests are provided with a hard-code basedn
>>> (dc=greyoak,dc=com). api.env.basedn would a better choice
>>
>> Ouch, fixed.
>>
>>> 2) We could optimize user.py not to retrieve config from LDAP since it
>>> is now needed only when api.env.wait_for_attr is now. I think this may
>>> speedup the command a little bit:
>>>           ...
>>>           # Automember adds our user to the default group for us.
>>>           if self.api.env.wait_for_attr:
>>>               config = ldap.get_ipa_config()[1]
>>>               def_primary_group = config.get('ipadefaultprimarygroup')
>>>               newentry = wait_for_value(ldap, dn, 'memberOf',
>>> def_primary_group)
>>>               entry_from_entry(entry_attrs, newentry)
>>>           ...
>>
>> Ok, that's a good idea. I think this path is going to go away soon
>> though once we have transactions in 389-ds.
>>
>> rob
>>
>
> Thanks, it safer now. We just have to fix ipa-server-install too:
>
> # ipa-server-install
> ...
>    [12/13]: restarting httpd
>    [13/13]: configuring httpd to start on boot
> done configuring httpd.
> Applying LDAP updates
> Unexpected error - see ipaserver-install.log for details:
>   The default users group cannot be removed or modified
>
> There is also a bug in is_default_users group - all non-group automember
> rules are rejected:
>
> # ipa hostgroup-add --desc="Web Servers" webservers
> ----------------------------
> Added hostgroup "webservers"
> ----------------------------
>    Host-group: webservers
>    Description: Web Servers
> # ipa automember-add --type=hostgroup webservers
> ----------------------------------
> Added automember rule "webservers"
> ----------------------------------
>    Automember Rule: webservers
> # ipa automember-add-condition --key=fqdn --type=hostgroup
> --inclusive-regex=^web[1-9]+\.example\.com webservers
> ipa: ERROR: The default users group cannot be removed or modified
>
> A buch of tests in test_automember_plugin.py is failing because of this
> bug too.
>
> Martin
>

Ah, I was just running the config tests :-(

The is_default_users_group() was trivial and fixed all but two tests. It 
did however show a potentially fatal problem to the patch.

If we use automember for users then the default group will NEVER get 
used because we guarantee that users are always added to one automember 
group (ipausers). This sort of defeats the purpose of being able to set 
a default group. So I'm thinking we'll need to drop this patch.

rob