servalproject / serval-dna

The Serval Project's core daemon that implements Distributed Numbering Architecture (DNA), MDP, VoMP, Rhizome, MeshMS, etc.
http://servalproject.org
Other
170 stars 81 forks source link

REST-API-Keyring Endpoints #132

Closed CryptoCopter closed 6 years ago

CryptoCopter commented 6 years ago

As already discussed in #131, all endpoints of the Keyring-API currently use GET requests, which is less than ideal.

At the very least, endpoints which manipulate remote data should use a POST request to do so, though the API could probably be improved even more.

Specifically, the relevant endpoints are:

I understand that these changes might not be easy to make because they might break downstream applications but maybe they could be part of a v2 of the API.

CryptoCopter commented 6 years ago

also, a GET /restful/keyring/SID-endpoint to quickly access the data for a specific identity would be nice

quixotique commented 6 years ago

I agree with all these points for making the API more REST-like. They apply equally well to our other APIs, such as Rhizome and MeshMS. We were definitely feeling our way forward when we started implementing the HTTP API, and it shows.

I think I prefer PUT /restful/keyring/SID/lock over POST, because PUT is idempotent but POST is not.

The new POST, DELETE and PUT requests could be implemented while retaining the existing GET requests to preserve back compatibility. We need a way for those requests to indicate that they are deprecated, and to suggest the better request. Perhaps initially we could put a comment in the returned headers that suggests the correct URL, then later on we could make the GET requests return a 301 "Moved Permanently" response, with the correct URL in the Location: header.

lakeman commented 6 years ago

An immediate 301 is probably fine. If you are pulling a new version of serval-dna, you should test it and hit this problem before you deploy it anywhere ...

On Thu, Mar 8, 2018 at 10:05 AM, Andrew Bettison notifications@github.com wrote:

I agree with all these points for making the API more REST-like. They apply equally well to our other APIs, such as Rhizome and MeshMS. We were definitely feeling our way forward when we started implementing the HTTP API, and it shows.

I think I prefer PUT /restful/keyring/SID/lock over POST, because PUT is idempotent but POST is not.

The new POST, DELETE and PUT requests could be implemented while retaining the existing GET requests to preserve back compatibility. We need a way for those requests to indicate that they are deprecated, and to suggest the better request. Perhaps initially we could put a comment in the returned headers that suggests the correct URL, then later on we could make the GET requests return a 301 "Moved Permanently" response, with the correct URL in the Location: header.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/servalproject/serval-dna/issues/132#issuecomment-371324089, or mute the thread https://github.com/notifications/unsubscribe-auth/AAkD3oBl83szxKLMhQFMWShNcAE_EoF8ks5tcG64gaJpZM4RJ_df .

quixotique commented 6 years ago

Also, PATCH /restful/keyring/SID is probably better than PUT, because PUT is defined as idempotent whereas PATCH is not. The effect of that endpoint is definitely not idempotent, since it may not completely replace the DID and Name, ie, it only partially updates the identity, so the outcome depends on prior state.