Hydra Admin: GetConsentSessions seems to always return 200 OK

[ghenry]

Describe the bug

/oauth2/auth/sessions/consent?subject=blahblah always returns 200 OK, when it should be 404 Not found

/oauth2/auth/sessions/consent?subject= correctly returns error 400 bad data.

Reproducing the bug

Steps to reproduce the behavior:

1. Run 5 mins quickstart - docker run ....quickstart.yml
2. Make API Request to with curl http://127.0.0.1:4445/oauth2/auth/sessions/consent?subject=blahblah
3. Request fails with response: {"some": "error"}

200 OK always:

Server logs

hydra_1          | time="2020-05-14T19:50:33Z" level=info msg="started handling request" method=GET remote="172.18.0.1:48944" request="/oauth2/auth/sessions/consent?subject=asdasd"
hydra_1          | time="2020-05-14T19:50:33Z" level=info msg="completed handling request" measure#hydra/admin: https://authz.surevoip.co.uk/.latency=793287 method=GET remote="172.18.0.1:48944" request="/oauth2/auth/sessions/consent?subject=asdasd" status=200 text_status=OK took="793.287µs"

Server configuration

default quickstart.yml

Expected behavior

Returns 404 as per:

https://www.ory.sh/hydra/docs/reference/api/#lists-all-consent-sessions-of-a-subject
https://github.com/ory/hydra/blob/master/consent/handler.go#L156

Environment

• Version: v1.4.8 git sha hash 18688c3
• Environment: Docker

[aeneasr]

I get an empty array in the result set - isn't that the correct response? We should however not indicate in the swagger spec that we return 404 :) Would you be open to fix that? All you need to do is remove this line: https://github.com/ory/hydra/blob/master/consent/handler.go#L145

[ghenry]

Hi @aeneasr

Sure, but it should just return 404, no body. I know you like to do things properly. On the client side, consuming this API call and writing a test for it would feel wrong.

If a subject has never consented any client access, 404 should be returned :)

Gavin.

[aeneasr]

Why should this be an error state? If you have a list, and empty it, you get a success - regardless of whether the list is actually empty or not. In most cases, you actually need to confirm that you want to delete the list contents because there are still items in the list.

In other words: If you delete consent sessions the result is always positive because you always succeeded in your action. The user does not have any consent session in the system any more. The reason for why that is does not matter.

[ghenry]

True, this isn't a delete endpoint though. This is a GET to LIST things. If no things, 404. Standard RESTful API response.

[aeneasr]

Oh I misread that - same thing applies though. I know very few collection endpoints that return 404 when they're empty. GET /clients also returns an empty array when empty, not a 404

[ghenry]

That's OK :)

I've explained this wrong. 404 should indicate no such subject. 200 OK and an empty list is fine for a valid subject.

That's what I was expecting.

[aeneasr]

Right, problem is - we can't really make that decision because Hydra does not have a concept of a user. Let's say you have a user with consent sessions - you delete those. For Hydra, the user is now unknown because purged from the Database (we don't keep records). Is that now a 404 or an empty list? Hard to say, therefore, an empty list :)

[ghenry]

Well, let's call it a subject then :)

This is easy. If you can't search for a subject UNLESS there is a subject with at least ONE client consent, then it's a 404 Not found. Easy.

A blanket 200 OK regardless of a valid or invalid subject query, does not feel right.

Thanks.

[aeneasr]

200 with an empty list is more consistent with other APIs we write, there is no up/downside for you (you need to be able to handle 404 errors, or empty list), and it would be a breaking change. The arguments for changing that don't outweigh these drawbacks, making this a wontfix.

[ghenry]

OK, as long as it's consistent across the ory projects, that makes sense. Doc bug then. Will do a PR tonight for you.

Thanks for the discussion.

[ghenry]

I do have a feeling there is a bug here as I tested with a valid subject with at least one client consent, but maybe a user error :)

Will get back to you.