[Pki-devel] [PATCH] 159, 160 - allow for automated generation of shared secrets for TKS/TPS connectors

[Ade Lee]

acked by Endi with a couple small changes.  Nickname no longer permitted
to be modified, and attempt to modify nickname/userid results in
UnauthorizedException being thrown.

All pushed to master.
 
On Mon, 2013-09-30 at 11:52 -0400, Ade Lee wrote:
> Based on revire comments and ack from Endi and Jack, I will push 159,
> 160 and 161.
> 
> Attaching 162 to add a modify option and add ability for a tks admin to
> remove a connector.
> 
> Please review.
> 
> Thanks, Ade
> 
> On Fri, 2013-09-27 at 14:36 -0400, Ade Lee wrote:
> > OK - I added a new patch to address the comments below and do some
> > cleanup.
> > 
> > Please apply on top of the 159 and 160.
> > 
> > Ade
> > 
> > On Thu, 2013-09-26 at 21:04 -0500, Endi Sukma Dewata wrote:
> > > On 9/26/2013 11:12 AM, Ade Lee wrote:
> > > > Patch 159:
> > > >
> > > >   Add service to generate and retrieve a shared secret
> > > >
> > > >      A new REST service has been added to the TKS to manage shared secrets.
> > > >      The shared secret is tied to the TKS-TPS connector, and is created at the
> > > >      end of the TPS configuration.  At this point, the TPS contacts the TKS and
> > > >      requests that the shared secret be generated.  The secret is returned to the
> > > >      TPS, wrapped using the subsystem certificate of the TPS.
> > > >
> > > >      The TPS should then decrypt the shared secret and store it in its certificate
> > > >      database.  This operations requires JSS changes, though, and so will be deferred
> > > >      to a later patch.  For now, though, if the TPS and TKS share the same certdb, then
> > > >      it is sufficient to generate the shared secret.
> > > >
> > > >      Clients and CLI are also provided.  The CLI in particular is used to remove the
> > > >      TPSConnector entries and the shared secret when the TPS is pkidestroyed.
> > > 
> > > In addition to the things that were discussed over IRC earlier I have 
> > > some comments:
> > > 
> > > 1. The createConnector() probably should return HTTP 201 since it's 
> > > creating a new entry. This would be consistent with other resources.
> > > 
> > > 2. The capitalization of "connector" in the help message is not consistent:
> > > 
> > >    tks-tpsconnector        TPS Connector management commands
> > >    tks-tpsconnector-add    Add TPS Connector to TKS
> > >    tks-tpsconnector-find   Find TPS connector details on TKS
> > >    tks-tpsconnector-del    Remove TPS connector from TKS
> > > 
> > > 3. The tks-tpsconnector-find output should have a header and footer like 
> > > other *-find commands:
> > > 
> > >    ----------------------
> > >    1 connector(s) matched
> > >    ----------------------
> > >      Connector ID: 0
> > >      Host: server.example.com
> > >      Port: 8443
> > >      User ID: TPS-server.example.com-8443
> > >      Nickname: TPS-server.example.com-8443 sharedSecret
> > >    ----------------------------
> > >    Number of entries returned 1
> > >    ----------------------------
> > > 
> > > 4. Currently the access to the connector is limited to the subsystem 
> > > user, which doesn't correspond to a real person. It should allow the TKS 
> > > administrator to add/delete the connectors. This can be done by checking 
> > > the principal.getRoles() in validateUser().
> > > 
> > > 5. The tks-tpsconnector-add/del parameters right now are passed as 
> > > arguments. It probably should be passed using options because it may 
> > > change in the future:
> > > 
> > >    tks-tpsconnector-add --host <host> --port <port>
> > >    tks-tpsconnector-del --connector-id <connector ID>
> > > 
> > > 6. We discussed about changing the error code for non-existent entries 
> > > to 204 (https://fedorahosted.org/pki/ticket/746). However, now I think 
> > > 404 actually makes more sense. See 
> > > http://stackoverflow.com/questions/11746894/what-is-the-proper-rest-response-code-for-a-valid-request-but-an-empty-data. 
> > > Both of the following URL's should return the same error code because 
> > > the client shouldn't need to know which path is mapped to REST method:
> > > * /rest/users/wronguser
> > > * /rest/wrongpath
> > > 
> > > So methods like deleteConnector(), deleteSharedSecret(), etc. should 
> > > throw ResourceNotFoundException instead of returning silently if the 
> > > entry doesn't exist.
> > > 
> > > I'll continue the review tomorrow.
> > > 
> > 
> > _______________________________________________
> > 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