[Pki-devel] [PATCH] 0001-KRA-changes-for-archiving-and-recovering-symmetric-keys.patch

[Ade Lee]

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 at redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
> 
> 
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel