On Thu, May 08, 2014 at 10:10:34AM -0400, Abhishek Koneru wrote:
Please review the attached patch with fixes for Ade's comments on
IRC.
- Fix all the errors/warnings and some typos shown by PyCharm.
- Add Link class which stores information of the Link object.
- Add an internal method _submit_revoke_request and for the common code
in revoke_cert and revoke_ca_cert.
- Mention the need of agent authentication for the methods where it is
required.
- Default revocation_reason should be REASON_UNSPECIFIED
- Add the decorator pki.handle_exceptions() to handle the PKIExceptions
sent by the server.
--Abhishek
On Mon, 2014-05-05 at 06:46 -0400, Abhishek Koneru wrote:
> Please review the attached patch which implements the Cert resource
> methods in CertClient on the python side.
>
> --Abhishek
> _______________________________________________
> Pki-devel mailing list
> Pki-devel(a)redhat.com
>
https://www.redhat.com/mailman/listinfo/pki-devel
This change (along with patch 93) works fine for the examples in the
``main`` method. I'm going to test it some more, though.
Some stylistic comments inline below.
Cheers,
Fraser
8< SNIP >8
class CertSearchRequest(object):
+ """
+ An object of this class is used to store the search parameters
+ and send them to server.
+ """
- def __init__(self):
- self.serialNumberRangeInUse = False
- self.serialTo = None
- self.serialFrom = None
- self.subjectInUse = False
- self.eMail = None
- self.commonName = None
- self.userID = None
- self.orgUnit = None
-
self.org = None
- self.locality = None
- self.state = None
- self.country = None
- self.matchExactly = None
- self.status = None
- self.revokedBy = None
- self.revokedOnFrom = None
- self.revokedOnTo = None
- self.revocationReason = None
- self.issuedBy = None
- self.issuedOnFrom = None
- self.issuedOnTo = None
- self.validNotBeforeFrom = None
- self.validNotBeforeTo = None
- self.validNotAfterFrom = None
- self.validNotAfterTo = None
- self.validityOperation = None
- self.validityCount = None
- self.validityUnit = None
- self.certTypeSubEmailCA = None
- self.certTypeSubSSLCA = None
- self.certTypeSecureEmail = None
- self.certTypeSSLClient = None
- self.certTypeSSLServer = None
- self.revokedByInUse = False
- self.revokedOnInUse = False
- self.revocationReasonInUse = None
- self.issuedByInUse = False
- self.issuedOnInUse = False
- self.validNotBeforeInUse = False
- self.validNotAfterInUse = False
- self.validityLengthInUse = False
- self.certTypeInUse = False
+ search_params = {'serial_to': 'serialTo', 'serial_from':
'serialFrom',
+ 'email': 'eMail', 'common_name':
'commonName', 'user_id': 'userID',
+ 'org_unit': 'orgUnit', 'org':
'org', 'locality': 'locality',
+ 'state': 'state', 'country':
'country', 'match_exactly': 'matchExactly',
+ 'status': 'status', 'revoked_by':
'revokedBy', 'revoked_on_from': 'revokedOnFrom',
+ 'revoked_on_to': 'revokedOnTo',
'revocation_reason': 'revocationReason',
+ 'issued_by': 'issuedBy', 'issued_on_from':
'issuedOnFrom', 'issued_on_to': 'issuedOnTo',
+ 'valid_not_before_from': 'validNotBeforeFrom',
'valid_not_before_to': 'validNotBeforeTo',
+ 'valid_not_after_from': 'validNotAfterFrom',
'valid_not_after_to': 'validNotAfterTo',
+ 'validity_operation': 'validityOperation',
'validity_count': 'validityCount',
+ 'validity_unit': 'validityUnit',
'cert_type_sub_email_ca': 'certTypeSubEmailCA',
+ 'cert_type_sub_ssl_ca': 'certTypeSubSSLCA',
'cert_type_secure_email': 'certTypeSecureEmail',
+ 'cert_type_ssl_client': 'certTypeSSLClient',
'cert_type_ssl_server': 'certTypeSSLServer'}
+
+ def __init__(self, **cert_search_params):
+ """ Constructor """
+
+ if len(cert_search_params) == 0:
+ setattr(self, 'serialNumberRangeInUse', True)
+
+ for param in cert_search_params:
Here you could iterate the (key, value) pairs to save keystrokes and
a bit of CPU doing ``cert_search_params[param]`` in all those places
below. e.g.:
for param, value in cert_search_params.viewitems():
...
``dict.viewitems()`` returns a *dictview* iterator-like object that
avoids creating an intermediate list as ``dict.items()`` would.
+ if not param in CertSearchRequest.search_params:
+ raise ValueError('Invalid search parameter: ' + param)
+
+ if param == 'serial_to' or param == 'serial_from':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+ setattr(self, 'serialNumberRangeInUse', True)
+
+ if param == 'email' or param == 'common_name' or param ==
'user_id' or param == 'org_unit' \
+ or param == 'org' or param == 'locality' or param ==
'state' or param == 'country' \
+ or param == 'match_exactly':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+ setattr(self, 'subjectInUse', True)
Here and in a few other places below it might improve readability to
test for set membership, e.g.:
if param in {
'email', 'common_name', 'user'_id',
'org_unit',
'org', ...
}:
setattr...
+
+ if param == 'status':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+
+ if param == 'revoked_by':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+ setattr(self, 'revokedByInUse', True)
+
+ if param == 'revoked_on_from' or param == 'revoked_on_to':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+ setattr(self, 'revokedOnInUse', True)
+
+ if param == 'revocation_reason':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+ setattr(self, 'revocationReasonInUse', True)
+
+ if param == 'issued_by':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+ setattr(self, 'issuedByInUse', True)
+
+ if param == 'issued_on_from' or param == 'issued_on_to':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+ setattr(self, 'issuedOnInUse', True)
+
+ if param == 'valid_not_before_from' or param ==
'valid_not_before_to':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+ setattr(self, 'validNotBeforeInUse', True)
+
+ if param == 'valid_not_after_from' or param ==
'valid_not_after_to':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+ setattr(self, 'validNotAfterInUse', True)
+
+ if param == 'validity_operation' or param ==
'validity_count' or param == 'validity_unit':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+ setattr(self, 'validityLengthInUse', True)
+
+ if param == 'cert_type_sub_email_ca' or param ==
'cert_type_sub_ssl_ca' \
+ or param == 'cert_type_secure_email' or param ==
'cert_type_ssl_client' \
+ or param == 'cert_type_ssl_server':
+ setattr(self, CertSearchRequest.search_params[param],
cert_search_params[param])
+ setattr(self, 'certTypeInUse', True)
8< SNIP >8