Re: [Pki-devel] [PATCH] 0001-KRA-changes-for-archiving-and-recovering-symmetric-keys.patch
Additional comments:
1. In SecurityDataRecoveryService, javadoc comments at top appear to be
for a generic service.
2. In SecurityDataRecoveryService.java --
a) You use RecoveryService.ATTR_TRANSPORT_PWD -- which is
transportPassword. Seems like a good idea to use the existing xml field
name.
b) change transWrappedPassPhraseStr to sessWrappedPassPhraseStr
c) check for where transWrappedPassphrase != null and
sessWrappedPassPhrase == null needs to be done at a higher level -- so
that we can throw WebApplicationException(BAD_REQUEST)
3. In SecurityDataService:
a) javadocs comments are for a generic service
b) Can options be null? You can confirm it cannot by writing some
conditional code if (options == null) { foo.., } and eclipse should
complain about dead code if it cannot be null.
c) Is there a check to confirm that the request has data in it? At top
level?
d) Put a TODO in getOwnerName()
Ade
On Wed, 2012-02-08 at 21:56 -0500, Ade Lee wrote:
Comments:
1. We should create indexes for the new attributes -- certainly for
clientID
2. In IKeyRecord, you have an accessor for clientID. Should we add
accessors for the other two attributes added?
3. In IEncryptionUnit.java, there is no javadoc comments for three of
the functions added.
4. Documentation in ITransportKeyUnit.java?
5. KeyResourceService.java still has incorrect check for
Request.Complete
6. KeysResourceService.java -- remove bad code (don't just comment out)
7. KeyDao -- rename params --> requestParams
what happens in the case where no wrapping parameters are
passed? we should check for that.
do we need to check for new() failing for KeyData?
8. In KeyRequestResourceService -> you return NOT_FOUND - should be
BAD_REQUEST instead?
9. Remove check for duplicate clientID in KeyRequestDAO in
submitRequest(ArchivalRequesData ..) Instead it should be a check for
active keys with the same client ID. Also remove the TODO in the same
function. And add a trac class to add status changes to the
interface/implemntation.
10. In KeyRequestDAO that you added queue.updateRequest(request); to
approveRequest(), please also add to cancelRequest() and rejectRequest()
11. typo in KeyRepository.java
12. Need getters for status and dataType in KeyRecord.java
More to come as I delve into the depths of KRA changes ..
On Wed, 2012-02-08 at 14:36 -0500, John Magne wrote:
> Provides the ability to archive and recover symmetric keys and pass phrases under
the restful interface.
> Java client included to test functionality.
> _______________________________________________ Pki-devel mailing list
Pki-devel(a)redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel