[Pki-devel] [pki-devel][Patch] 0014-Initial-enrollment-progress.patch

[John Magne]

Based on cfu ACK, pushed to master.

Created brand new instance locally with this code and format and the current
progress of enrollment worked as expected.



----- Original Message -----
> From: "Christina Fu" <cfu at redhat.com>
> To: pki-devel at redhat.com
> Sent: Thursday, June 5, 2014 4:07:20 PM
> Subject: Re: [Pki-devel] [pki-devel][Patch]	0014-Initial-enrollment-progress.patch
> 
> ACK if format is tested to work.  I'm not drilling down on the
> enrollment part since we are still not quite there yet.
> 
> Christina
> 
> On 06/04/2014 01:54 PM, John Magne wrote:
> > CFU:
> >
> > Review comments addressed.
> >
> > Plus a little bit more progress on enrollment,
> > up until the point we need to parse the public key blob returned
> > by the token. Also, rebased and merged to to the latest trunk as of an hour
> > ago.
> >
> >
> >
> >
> >
> > ----- Original Message -----
> >> From: "Christina Fu" <cfu at redhat.com>
> >> To: pki-devel at redhat.com
> >> Sent: Tuesday, June 3, 2014 9:28:46 AM
> >> Subject: Re: [Pki-devel] [pki-devel][Patch]
> >> 	0013-Initial-enrollment-progress.patch
> >>
> >> Jack,
> >>
> >> Overall, thanks for the refactoring. It does make things easier to read. I
> >> understand there are still code to be filled. Here are my comments.
> >>
> >> 1. getConfiguredKeyType()
> >> - The original code goes to loser if the keytype is undefined. You
> >> assigned
> >> "signing" as default. I think the original code is probably better
> >> - The original code verifies if the keytype gotten from config is one of
> >> the
> >> TokenKeyType defined. You probably should add such check as well.
> >>
> >> 2. I noticed that the format() method does not take the skipAuth boolean
> >> like
> >> before when enroll() has already done authentication. Wouldn't that mean
> >> format() could result authentication being done twice if format is forced
> >> (required) during enrollment?
> >>
> >> 3. a few typos
> >> - "TPSProcessor.enroll:" in debug should be "TPSEnrollProcessor.enroll:"
> >> - "TPSEnrollProcessor>enroll:" in debug should be
> >> "TPSEnrollProcessor.enroll:"
> >> - "selelect" should be "select"
> >>
> >> 4. generateCertificates()
> >> - I see that EnrolledCertsInfo serves both as input and output, so passing
> >> it
> >> in as a parameter is reasonable. But why do you need to return it? And you
> >> didn't' really return it. Or you can just do void instead. It already
> >> throws
> >> TPSException in case of errors.
> >>
> >> 5. I think there are a few missed "TODO". If you happen to see them, you
> >> might want to add to comment so we don't miss them.
> >>
> >> 6. I'm sorry I checked in my patch before you, so there will be conflicts
> >> to
> >> be resolved. Let's take one more look after your merge.
> >>
> >> Christina
> >>
> >>
> >> On 05/28/2014 07:47 PM, John Magne wrote:
> >>
> >>
> >>
> >> Initial enrollment operation progress.
> >>      
> >>      1. Changed the names of some message classes for convenience.
> >>      2. Did some minor refactoring of methods needed by both the enroll
> >>      and
> >>      tps processor.
> >>      3. Created classes to handle the parsing and archival of PKCS#11
> >>      token
> >>      data.
> >>      4. Created prep code for enrollment that reads in a bunch of config
> >>      params and creates
> >>      convenience objects to carry the data instead of the lengthy
> >>      parameter
> >>      lists we have had before.
> >>
> >>
> >> _______________________________________________
> >> 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
> 
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
>