[Freeipa-devel] [PATCH 0297] ULC: add user-stage command
Martin Basti
mbasti at redhat.com
Thu Aug 20 15:21:42 UTC 2015
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?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150820/a9860ea4/attachment.htm>
More information about the Freeipa-devel
mailing list