[Freeipa-devel] [PATCH 0297] ULC: add user-stage command
thierry bordaz
tbordaz at redhat.com
Thu Aug 20 17:17:46 UTC 2015
On 08/20/2015 05:21 PM, Martin Basti wrote:
>
>
> On 08/20/2015 11:27 AM, Jan Cholasta wrote:
>> 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.
>>
>>
>>
> LGTM,
>
> what do you think thierry?
>
>
>
Hi,
It works like a charm and regarding the fix looks great as well.
ACK
thanks
theirry
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150820/9392664d/attachment.htm>
More information about the Freeipa-devel
mailing list