[Freeipa-devel] [PATCH 0297] ULC: add user-stage command

Jan Cholasta jcholast at redhat.com
Thu Aug 20 09:27:55 UTC 2015


On 19.8.2015 10:57, Jan Cholasta wrote:
> On 19.8.2015 10:47, thierry bordaz wrote:
>> On 08/19/2015 10:34 AM, Jan Cholasta wrote:
>>> On 19.8.2015 09:39, thierry bordaz wrote:
>>>> Hi,
>>>>
>>>> It worked like a charm.
>>>> I had a problem to commit it because of the VERSION stuff that changed.
>>>>
>>>> Except that (changing VERSION), the fix looks good to me
>>>>
>>>> thanks
>>>> thierry
>>>> On 08/18/2015 07:21 PM, Martin Basti wrote:
>>>>> Thank you for the patch, I checked it, I just changed permission name
>>>>> to have all first letters in uppercase as others.
>>>>> Updated merged patch attached.
>>>>>
>>>>> On 08/18/2015 05:34 PM, thierry bordaz wrote:
>>>>>> On 08/18/2015 04:13 PM, thierry bordaz wrote:
>>>>>>> On 08/18/2015 04:04 PM, Martin Basti wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 08/18/2015 03:49 PM, thierry bordaz wrote:
>>>>>>>>> On 08/18/2015 03:06 PM, Martin Basti wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 08/18/2015 11:32 AM, thierry bordaz wrote:
>>>>>>>>>>> On 08/18/2015 10:02 AM, Martin Basti wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 08/18/2015 09:59 AM, thierry bordaz wrote:
>>>>>>>>>>>>> On 08/18/2015 09:55 AM, Martin Basti wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 08/18/2015 09:50 AM, thierry bordaz wrote:
>>>>>>>>>>>>>>> On 08/17/2015 08:33 PM, Martin Basti wrote:
>>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> the 'user-stage' command replaces 'stageuser-add
>>>>>>>>>>>>>>>> --from-delete' command.
>>>>>>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5041
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thierry can you check If I don't break everything, it works
>>>>>>>>>>>>>>>> for me, but the one never knows.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Honza can you please check the framework side? I use
>>>>>>>>>>>>>>>> self.api.Object.stageuser.add.* in user command, I'm not
>>>>>>>>>>>>>>>> sure if this is right way, but it works.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Patch attached. I created it in hurry, I'm expecting
>>>>>>>>>>>>>>>> NACK :D
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Just question at the end: should I implement way Active
>>>>>>>>>>>>>>>> user -> stageuser? IMHO it would be implemented internally
>>>>>>>>>>>>>>>> by calling 'user-del --preserve' inside 'user-stage'.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Martin,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> There is a small failure with VERSION (edewata pushed his
>>>>>>>>>>>>>>> patch first ;-) )
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>     git apply -v
>>>>>>>>>>>>>>> /tmp/freeipa-mbasti-0297-Add-user-stage-command.patch
>>>>>>>>>>>>>>>     Checking patch API.txt...
>>>>>>>>>>>>>>>     Checking patch VERSION...
>>>>>>>>>>>>>>>     error: while searching for:
>>>>>>>>>>>>>>>     # #
>>>>>>>>>>>>>>> ########################################################
>>>>>>>>>>>>>>>     IPA_API_VERSION_MAJOR=2
>>>>>>>>>>>>>>>     IPA_API_VERSION_MINOR=148
>>>>>>>>>>>>>>>     # Last change: ftweedal - add --out option to user-show
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>     error: patch failed: VERSION:90
>>>>>>>>>>>>>>>     error: VERSION: patch does not apply
>>>>>>>>>>>>>>>     Checking patch ipalib/plugins/stageuser.py...
>>>>>>>>>>>>>>>     Checking patch ipalib/plugins/user.py...
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> There is many pending patches that may change VERSION number,
>>>>>>>>>>>>>> I will change it to right one before push.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Does code looks good for you?
>>>>>>>>>>>>> Hi Martin,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Just a question, there is no additional permission. Did you
>>>>>>>>>>>>> test being 'admin' ?
>>>>>>>>>>>>>
>>>>>>>>>>>>> thanks
>>>>>>>>>>>>> theirry
>>>>>>>>>>>> No I didn't,.
>>>>>>>>>>>>
>>>>>>>>>>>> I preserver all permission, the original permissions should
>>>>>>>>>>>> work.
>>>>>>>>>>>>
>>>>>>>>>>>> Martin
>>>>>>>>>>> Hi Martin,
>>>>>>>>>>>
>>>>>>>>>>> Running a test script, I have an issue with
>>>>>>>>>>>
>>>>>>>>>>>     ipa stageuser-add --first=t --last=b tb1
>>>>>>>>>>>     ipa: ERROR: an internal error has occurred
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>     [Tue Aug 18 11:16:56.440658 2015] [wsgi:error] [pid 10486]
>>>>>>>>>>>     ipa: INFO: [jsonserver_kerb]
>>>>>>>>>>>     stageadm at ABC.IDM.LAB.ENG.BRQ.REDHAT.COM:
>>>>>>>>>>>     stageuser_add(u'tb1', givenname=u't', sn=u'b', cn=u't b',
>>>>>>>>>>>     displayname=u't b', initials=u'tb', gecos=u't b',
>>>>>>>>>>> krbprincipalname=u'tb1 at ABC.IDM.LAB.ENG.BRQ.REDHAT.COM',
>>>>>>>>>>>     random=False, all=False, raw=False, version=u'2.149',
>>>>>>>>>>>     no_members=False): AttributeError
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198021 2015] [wsgi:error] [pid 10485]
>>>>>>>>>>>     ipa: ERROR: non-public: AttributeError: 'DN' object has no
>>>>>>>>>>>     attribute 'setdefault'
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198053 2015] [wsgi:error] [pid 10485]
>>>>>>>>>>>     Traceback (most recent call last):
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198058 2015] [wsgi:error] [pid 10485]
>>>>>>>>>>>     File
>>>>>>>>>>> "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py",
>>>>>>>>>>>     line 347, in wsgi_execute
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198062 2015] [wsgi:error] [pid
>>>>>>>>>>>     10485]     result = self.Command[name](*args, **options)
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198066 2015] [wsgi:error] [pid 10485]
>>>>>>>>>>>     File "/usr/lib/python2.7/site-packages/ipalib/frontend.py",
>>>>>>>>>>>     line 443, in __call__
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198070 2015] [wsgi:error] [pid
>>>>>>>>>>>     10485]     ret = self.run(*args, **options)
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198081 2015] [wsgi:error] [pid 10485]
>>>>>>>>>>>     File "/usr/lib/python2.7/site-packages/ipalib/frontend.py",
>>>>>>>>>>>     line 760, in run
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198133 2015] [wsgi:error] [pid
>>>>>>>>>>>     10485]     return self.execute(*args, **options)
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198139 2015] [wsgi:error] [pid 10485]
>>>>>>>>>>>     File
>>>>>>>>>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py",
>>>>>>>>>>>     line 1227, in execute
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198144 2015] [wsgi:error] [pid
>>>>>>>>>>>     10485]     *keys, **options)
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198147 2015] [wsgi:error] [pid 10485]
>>>>>>>>>>>     File
>>>>>>>>>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/stageuser.py",
>>>>>>>>>>>     line 373, in pre_callback
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198151 2015] [wsgi:error] [pid
>>>>>>>>>>>     10485]     attrs_list, *keys, **options)
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198155 2015] [wsgi:error] [pid 10485]
>>>>>>>>>>>     File
>>>>>>>>>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/stageuser.py",
>>>>>>>>>>>     line 277, in set_default_values_pre_callback
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198159 2015] [wsgi:error] [pid 10485]
>>>>>>>>>>>     entry_attrs.setdefault('description', [])
>>>>>>>>>>>     [Tue Aug 18 11:21:25.198163 2015] [wsgi:error] [pid 10485]
>>>>>>>>>>>     AttributeError: 'DN' object has no attribute 'setdefault'
>>>>>>>>>>>     [Tue Aug 18 11:21:25.199276 2015] [wsgi:error] [pid 10485]
>>>>>>>>>>>     ipa: INFO: [jsonserver_session]
>>>>>>>>>>>     stageadm at ABC.IDM.LAB.ENG.BRQ.REDHAT.COM:
>>>>>>>>>>>     stageuser_add(u'tb1', givenname=u't', sn=u'b', cn=u't b',
>>>>>>>>>>>     displayname=u't b', initials=u'tb', gecos=u't b',
>>>>>>>>>>> krbprincipalname=u'tb1 at ABC.IDM.LAB.ENG.BRQ.REDHAT.COM',
>>>>>>>>>>>     random=False, all=False, raw=False, version=u'2.149',
>>>>>>>>>>>     no_members=False): AttributeError
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> The new set_default_values_pre_callback, can not use the
>>>>>>>>>>> set_default function. It is not clear why. entry_attrs is one of
>>>>>>>>>>> pre_callback parameter.
>>>>>>>>>>> Should set_default_values_pre_callback be a subfonction of
>>>>>>>>>>> pre_callback ?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> thanks
>>>>>>>>>>> thierry
>>>>>>>>>>
>>>>>>>>>> Thank you,
>>>>>>>>>>
>>>>>>>>>> updated patch attached.
>>>>>>>>>
>>>>>>>>> So far, tests are ok.
>>>>>>>>> Just one comment, the 'user-stage' command description is wrong,
>>>>>>>>> as it moves an active user into the staged area
>>>>>>>>>
>>>>>>>>> user-stage                             Move deleted user into
>>>>>>>>> staged area
>>>>>>>> No, it's not doing that.
>>>>>>>>
>>>>>>>> user-stage is replacement of stageuser-add --from-delete, it
>>>>>>>> doesn't work for active users.
>>>>>>>> The support to move active user to staged area is RFE, I did not
>>>>>>>> implemented it yet, and I dont know if this will fit IPA 4.2
>>>>>>>> timeframe
>>>>>>> Ok. thanks.
>>>>>>> Sure user-stage (active->stage) will not fit into IPA 4.2 timeframe.
>>>>>>>
>>>>>>> Running the tests being admin, there is no problem.
>>>>>>> I have a permission issue, when running as 'Stage administrator'.
>>>>>>> The 'delete' entry being moved to 'stage' container, we need the a
>>>>>>> special permission for it.
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> I tested this new permission to  grant 'Stage user administrator' to
>>>>>> do a 'user-stage'.
>>>>>> Is it ok to add it to your patch ?
>>>>>>
>>>>>> thanks
>>>>>> thierry
>>>>>>>
>>>>>>> [root at vm-141 ~]# ipa user-del ttest1 --preserve
>>>>>>> ---------------------
>>>>>>> Deleted user "ttest1"
>>>>>>> ---------------------
>>>>>>>
>>>>>>> [root at vm-141 ~]# ipa user-stage ttest1
>>>>>>> ipa: ERROR: Insufficient access: Insufficient 'moddn' privilege to
>>>>>>> move an entry to 'cn=staged
>>>>>>> users,cn=accounts,cn=provisioning,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com'.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> [root at vm-141 ~]# klist
>>>>>>> Ticket cache: KEYRING:persistent:0:krb_ccache_hw3P667
>>>>>>> Default principal: stageadm at ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
>>>>>>>
>>>>>>> Valid starting       Expires              Service principal
>>>>>>> 08/18/2015 15:45:43  08/19/2015 15:45:42
>>>>>>> ldap/vm-141.abc.idm.lab.eng.brq.redhat.com at ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
>>>>>>>
>>>>>>>
>>>>>>> 08/18/2015 15:45:42  08/19/2015 15:45:42
>>>>>>> krbtgt/ABC.IDM.LAB.ENG.BRQ.REDHAT.COM at ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
>>>>>>>
>>>>>>> [root at vm-141 ~]# kinit admin
>>>>>>> Password for admin at ABC.IDM.LAB.ENG.BRQ.REDHAT.COM:
>>>>>>> [root at vm-141 ~]# ipa user-stage ttest1
>>>>>>> ----------------------------
>>>>>>> Staged user account "ttest1"
>>>>>>> ----------------------------
>>>>>>> [root at vm-141 ~]# ipa stageuser-find ttest1
>>>>>>> --------------
>>>>>>> 1 user matched
>>>>>>> --------------
>>>>>>>   User login: ttest1
>>>>>>>   First name: t
>>>>>>>   Last name: test1
>>>>>>>   Home directory: /home/ttest1
>>>>>>>   Login shell: /bin/sh
>>>>>>>   Email address: ttest1 at abc.idm.lab.eng.brq.redhat.com
>>>>>>>   UID: 1814000011
>>>>>>>   GID: 1814000011
>>>>>>>   Password: False
>>>>>>>   Kerberos keys available: False
>>>>>>> ----------------------------
>>>>>>> Number of entries returned 1
>>>>>>> ----------------------------
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>> NACK.
>>>
>>> 1) Use ADD+DEL instead of MODRDN as we agreed before:
>>> <https://www.redhat.com/archives/freeipa-devel/2015-August/msg00148.html>.
>>>
>>>
>>
>> Hi,
>>
>> I have a slight preference doing MODRDN than ADD+DEL but I think it is
>> for corner case.
>> Before preserving a user, the user was active and could be updated. If
>> the user gets updated on a replica (e.g. change its phonenumer) but for
>> some reason the update is not immediately replicated, then a later
>> 'user-del --preserve' + 'user-stage' will stage the user without the
>> updated phonenumber.
>>
>> In addition, doing 2 ops rather than one costs more and is not atomic
>> (more complex to handle failure).
>
> The same problem exists for stageuser_activate, and unless you want to
> change it to use MODRDN as well, user_stage must use ADD+DEL.
>
> This was already discussed quite thoroughly and we reached the decision
> to use ADD+DEL, because it is consistent with the rest of the user code.
> I don't see a point in discussing this further and rehashing what was
> already said.
>
>>
>> thank
>> thierry
>>>
>>> 2) You can't use the entry preparation code from stageuser-add in
>>> user-stage - it is supposed to normalize user input, not already
>>> normalized data from LDAP, and could lead to subtle and hard to track
>>> errors.
>>>
>>> Honza
>>>
>>
>

I have updated Martin's patch with fixes for the above. See attachment.

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0297.4-Add-user-stage-command.patch
Type: text/x-patch
Size: 8898 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150820/b83aaecb/attachment.bin>


More information about the Freeipa-devel mailing list