search

privacybydesign / irmago

IRMA server, client, and tooling. Documentation: http://irma.app/docs
Apache License 2.0
70 stars 48 forks source link

Cancel a IRMA session #140

Open stefanvermaas opened 3 years ago

stefanvermaas commented 3 years ago

Whenever you cancel an IRMA session (by sending DELETE /session/[:token] to the IRMA server), you get a 200 response with an empty body ("") from the server.

A common design pattern for DELETE response is to either (1) use the 204 HTTP status code with an empty body ("") or (2) a 200 HTTP status code with the deleted session status as response ({ token: [token], status: "CANCELLED" }).

The current implementation mixes both design patterns, which leads to (IMO) an unexpected response for cancelling/deleting an IRMA session.

If a DELETE method is successfully applied, the origin server SHOULD send a 202 (Accepted) status code if the action will likely succeed but has not yet been enacted, a 204 (No Content) status code if the action has been enacted and no further information is to be supplied, or a 200 (OK) status code if the action has been enacted and the response message includes a representation describing the status. - The original HTTP/1.1 spec

I was wondering; is this design on purpose, or is it open to improvement?

sietseringers commented 3 years ago

I was not aware of this pattern, and agree that the current behavior is not optimal. So this is indeed open for improvement, thanks for pointing it out. If you'd like to submit a PR fixing it I would be happy to merge it 🙂 Otherwise we will fix this ourselves later.

stefanvermaas commented 3 years ago

If you'd like to submit a PR fixing it I would be happy to merge it.

I'm a bit rusty on my go. So I'm not sure whether I'll write the best implementation, but I'll give it a go (pun intended). The main question is though; what should the response of the IRMA server be?

I assume returning a 204 would be closest to the current implementation. Some documentation might need to be adjusted, but I'm not sure whether there will be side-effects for the JavaScript packages.

stefanvermaas commented 3 years ago

I see there are no tests to check the HTTP status code, would you be open to help or write those specs? It seems to change the HTTP status code is pretty straightforward. I'll create a draft PR for this.