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

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-respo.... > 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(a)redhat.com https://www.redhat.com/mailman/listinfo/pki-devel