tech5usa / TLSential

A server for providing short-lived TLS certificates to all services within a firewall restricted network.
GNU General Public License v3.0
15 stars 2 forks source link

Debus/certificate get #35

Closed debus closed 4 years ago

debus commented 4 years ago

This PR addresses the following issues:

Refactor function that does two things into two functions that do one thing each. Improve behavior when getting an empty list of certificates and when getting a certificate that doesn't exist.

Context

both the get all certificates (/api/certificate) and get certificate (/api/certificate/{id}) endpoints are handled by the same function. They should be refactored into two separate functions. Which will also let us set different needed permissions for each endpoint.

Note that this PR also fixes an issue where /api/certificate/some_id_that_doesnt_exist would crash and generate a server error instead of returning a 404. It also changes the behavior of /api/certificate when no certificates exist from returning a JSON null to returning an empty JSON array.

Approach

Refactor the certficate.Get which handles /api/certificate and /api/certificate/{id} into two separate functions. certificate.Get which handles /api/certificate/{id} and certificate.GetAll which handles `/api/certificate.

Testing

No unit tests right now. To test that the /api/certificate/ and /api/certificate/{id} endpoints actually go to different functions I just temporarily had the handlers log which function was being called and made sure the correct functions were being called at the right times. Also made sure that the responses I got matched the responses I got from those API calls before any code was changed.

Misc.