API V2: new API with the correct response codes

josecelano commented 1 year ago

Parent issue: https://github.com/torrust/torrust-tracker/issues/141 It depends on: https://github.com/torrust/torrust-tracker/issues/143

We start building a new API using the /v2 prefix. For example: /v2/api/stats. We can use the same URLs but change the response codes.

Change the response codes to the right one: 200, 201, 404, etcetera. Right now, it only uses 200 and 500.

Potential changes

I think we could make other changes:

Refactor: create a custom Reject for all the errors. Like the one I have added: struct Unauthorized.
Refactor: move routes to their own mod routes.rs.
The authenticate function should also return a 401 response instead of token not valid when the token is present but not valid.
The GET /api/torrent/:info_hash endpoint should return a 404 when the torrent does not exist and the token is valid.
The endpoint GET /api/torrent/:info_hash should return a 404 when the torrent does not exist, and the token is invalid. See Insecure Direct Object References (IDOR)). We should not expose that the tracker has a given torrent because the tracker could be private.
The DELETE /api/whitelist/:info_hash endpoint should return: a "204 No Content" when it deletes the torrent and a "404 Not Found" when the torrent does not exist or the token is invalid.
The POST /api/whitelist/:info_hash endpoint should return a "201 Created" response with the location of the new resource GET /api/whitelist/:info_hash. In this case we would need to create that new endpoint. It could be empty. It could be used only to know if a torrent is whitelisted. Alternatively, we could consider the whitelist a unique resource and use a PUT /api/whitelist/:info_hash to add a new torrent to the list.
The POST /api/key/:seconds_valid endpoint should return a "201 Created".
The DELETE /api/key/:key should return "204 No Content".
The GET /api/whitelist/reload endpoint should be a POST, PUT or PATCH.

Notes

We can keep the old API in parallel until we release the new major version.

Long-term changes

These are changes not included in this issue, but they should be considered in future versions.

The GET /api/whitelist/reload endpoint is a little weird. It's like a remote command. I think I should have usedPOST /api/commands with the command you want to execute in the request body or POST /api/reload-whitelist.
Same for GET /api/keys/reload
We could use a header parameter for the API version instead of a URL prefix, like this.
Use a schema (most likely https://json-ld.org/).
Re-design the API and consider other endpoints like the ones suggested here by @dev1z

josecelano commented 1 year ago

@da2ce7 @WarmBeer

I've found this interesting issue https://github.com/tokio-rs/axum/issues/50 where people are asking Axum to add support for generating OpenAPI/Swagger docs.

And some mentioned that you could use utopia. It has support for all the major web frameworks.

josecelano commented 8 months ago

The rqbit - bittorrent client in Rust uses this very useful API entrypoint:

{
  "apis": {
    "GET /": "list all available APIs",
    "GET /dht/stats": "DHT stats",
    "GET /dht/table": "DHT routing table",
    "GET /torrents": "List torrents (default torrent is 0)",
    "GET /torrents/{index}": "Torrent details",
    "GET /torrents/{index}/haves": "The bitfield of have pieces",
    "GET /torrents/{index}/peer_stats": "Per peer stats",
    "GET /torrents/{index}/stats/v1": "Torrent stats",
    "GET /web/": "Web UI",
    "POST /rust_log": "Set RUST_LOG to this post launch (for debugging)",
    "POST /torrents": "Add a torrent here. magnet: or http:// or a local file.",
    "POST /torrents/{index}/delete": "Forget about the torrent, remove the files",
    "POST /torrents/{index}/forget": "Forget about the torrent, keep the files",
    "POST /torrents/{index}/pause": "Pause torrent",
    "POST /torrents/{index}/start": "Resume torrent"
  },
  "server": "rqbit",
  "version": "5.4.1"
}

cc @da2ce7

torrust / torrust-tracker