[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Freeipa-devel] [PATCH 0024] Add OTP support to ipalib CLI



On 13.12.2013 21:15, Nathaniel McCallum wrote:
On Fri, 2013-12-13 at 14:50 -0500, Nathaniel McCallum wrote:
On Wed, 2013-12-11 at 13:24 +0100, Jan Cholasta wrote:
+        # Resolve the user's dn
+        owner = entry_attrs.get('ipatokenowner', None)
+        if owner is not None:
+            owner = self.api.Object.user.get_dn(owner)
+            entry_attrs['ipatokenowner'] = owner

You have a _normalize_owner function, I think the code above should go
into a _convert_owner function (use the function in
otptoken_{mod,show,find} as well).

Fixed for mod and find. Show doesn't make sense.

Please rename _normalize_owner to _convert_owner and vice versa, to match the convention used in other plugins (sorry for noticing this earlier).


This bit in otptoken_add should be replaced by a call to _normalize_owner (after the rename):

+        # Resolve the user's dn
+        owner = entry_attrs.get('ipatokenowner', None)
+        if owner is not None:
+            owner = self.api.Object.user.get_dn(owner)
+            entry_attrs['ipatokenowner'] = owner


You do the conversion from uid to DN in otptoken_find twice:

+        _convert_owner(self.api.Object.user, kwargs)
+ return super(otptoken_find, self).pre_callback(ldap, filters, *args, **kwargs)
+
+    def args_options_2_entry(self, *args, **options):
+        o = 'ipatokenowner'
+        if o in options:
+            options[o] = self.api.Object.user.get_dn(options[o])

I would suggest to do it only in args_options_2_entry like this (again, after the rename):

    def args_options_2_entry(self, *args, **options):
entry = super(otptoken_find, self).args_options_2_entry(*args, **options)
        _convert_owner(self.api.Object.user, entry)
        return entry

+        # Delete all tokens owned by this user
+        owner = self.api.Object.user.get_primary_key_from_dn(dn)
+        results =
self.api.Command.otptoken_find(ipatokenowner=owner)['result']
+        for token in results:
+            token =
self.api.Object.otptoken.get_primary_key_from_dn(token['dn'])
+            self.api.Command.otptoken_del(token)

This should probably be handled by the referint plugin.

See my reply to Martin.

I see, my mistake.


ARGH! I should try not to break stuff. New patch attached...


The patch needs a rebase (for the sake of ipapermlocation default value).

--
Jan Cholasta


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]