Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancel a IRMA session #140

Open
stefanvermaas opened this issue Mar 16, 2021 · 3 comments · May be fixed by #142
Open

Cancel a IRMA session #140

stefanvermaas opened this issue Mar 16, 2021 · 3 comments · May be fixed by #142

Comments

@stefanvermaas
Copy link

stefanvermaas commented Mar 16, 2021

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
Copy link
Member

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
Copy link
Author

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants