On 12/11/2013 01:24 PM, Jan Cholasta wrote:
On 14.11.2013 20:23, Nathaniel McCallum wrote:
On Wed, 2013-10-30 at 08:57 +0100, Jan Cholasta wrote:
On 8.10.2013 16:35, Nathaniel McCallum wrote:
On Tue, 2013-10-08 at 09:19 +0200, Jan Cholasta wrote:
+class Base32DecodeError(ExecutionError):
Is this really necessary? Are we going to add <encoding>DecodeError for
every kind of new encoding in IPA? Can't we just have generic
DecodeError? (This is not an issue in your patch per se, I'm just
wondering if we can do it better in the framework.)
I added the new error due to the existence of a Base64DecodeError. I
figured changing the existing error to be more generic would break api.
Nathaniel
I think you can use ConversionError instead. I don't see any reason why
base32/64 decoding errors should be special cased like this and would
like to see Base64DecodeError gone.
Fixed.
...
+ # Get the issuer for the URI
+ issuer = api.env.realm
+ if owner is not None:
+ try:
+ issuer = ldap.get_entry(owner,
['krbprincipalname'])['krbprincipalname'][0]
+ except:
+ pass
Please use "except PublicError" here, we don't want internal errors to be
ignored.
Right. I think what Nathaniel wanted to do is:
except errors.NotFound:
pass
Bare except-pass is wrong on several levels anyway. For example it also catches
syntax or interrupt exceptions.
+ # 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.
Honza
I think referint plugin would just remove the attribute containing DN to user,
not the whole entry. I.e. I think Nathaniel need to do it this way anyway.
Though I think it would be more efficient if we do just one 'otptoken_del' call
with the list of tokens as primary key to it. All standard LDAPDelete based
plugins can do that. You may also want to pass continue=True in the list of
it's options in this case.
Martin
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
