Call revoke token endpoint using wrong client credentials returns 200 OK

[har07]

Preflight checklist

• I could not find a solution in the existing issues, docs, nor discussions.
• I agree to follow this project's Code of Conduct.
• I have read and am following this repository's Contribution Guidelines.
• This issue affects my Ory Network project.
• I have joined the Ory Community Slack.
• I am signed up to the Ory Security Patch Newsletter.

Describe the bug

Passing the wrong client id + client secret* parameter to call revoke token endpoint returns 200 OK despite the revocation process failed as indicated in the logs

*) the client id + client secret pair is valid i.e exists in database, but the token to be revoked was generated by some other client

Reproducing the bug

Steps to reproduce the behavior:

1. Generate access token using client A
2. Call revoke token endpoint /oauth2/revoke using client id + client secret of client B
3. Revoke token endpoint returns 200 OK, but the token is still valid

Relevant log output

{"audience":"application","error":{"debug":"","message":"unauthorized_client","reason":"Make sure that client id and secret are correctly specified and that the client exists.","stack_trace":"\ngithub.com/ory/x/errorsx.WithStack\n\t/go/pkg/mod/github.com/ory/[email protected]/errorsx/errors.go:41\ngithub.com/ory/fosite/handler/oauth2.(*TokenRevocationHandler).RevokeToken\n\t/go/pkg/mod/github.com/ory/[email protected]/handler/oauth2/revocation.go:54\ngithub.com/ory/fosite.(*Fosite).NewRevocationRequest\n\t/go/pkg/mod/github.com/ory/[email protected]/revoke_handler.go:55\ngithub.com/ory/hydra/oauth2.(*Handler).revokeOAuth2Token\n\t/project/oauth2/handler.go:664\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/rs/cors.(*Cors).Handler.func1\n\t/go/pkg/mod/github.com/rs/[email protected]/cors.go:231\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/ory/x/httprouterx.NoCacheHandler.func1\n\t/go/pkg/mod/github.com/ory/[email protected]/httprouterx/nocache.go:37\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/julienschmidt/httprouter.(*Router).Handler.func1\n\t/go/pkg/mod/github.com/julienschmidt/[email protected]/router.go:275\ngithub.com/julienschmidt/httprouter.(*Router).ServeHTTP\n\t/go/pkg/mod/github.com/julienschmidt/[email protected]/router.go:387\ngithub.com/urfave/negroni.Wrap.func1\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:46\ngithub.com/urfave/negroni.HandlerFunc.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:29\ngithub.com/urfave/negroni.middleware.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:38\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/ory/hydra/x.RejectInsecureRequests.func1\n\t/project/x/tls_termination.go:65\ngithub.com/urfave/negroni.HandlerFunc.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:29\ngithub.com/urfave/negroni.middleware.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:38\ngithub.com/ory/x/metricsx.(*Service).ServeHTTP\n\t/go/pkg/mod/github.com/ory/[email protected]/metricsx/middleware.go:278\ngithub.com/urfave/negroni.middleware.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:38\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerResponseSize.func1\n\t/go/pkg/mod/github.com/prometheus/[email protected]/prometheus/promhttp/instrument_server.go:284\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerCounter.func1\n\t/go/pkg/mod/github.com/prometheus/[email protected]/prometheus/promhttp/instrument_server.go:142\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerDuration.func1\n\t/go/pkg/mod/github.com/prometheus/[email protected]/prometheus/promhttp/instrument_server.go:92\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerDuration.func2\n\t/go/pkg/mod/github.com/prometheus/[email protected]/prometheus/promhttp/instrument_server.go:104\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/prometheus/client_golang/prometheus/promhttp.InstrumentHandlerRequestSize.func1\n\t/go/pkg/mod/github.com/prometheus/[email protected]/prometheus/promhttp/instrument_server.go:234\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2109\ngithub.com/ory/x/prometheusx.Metrics.instrumentHandlerStatusBucket.func1\n\t/go/pkg/mod/github.com/ory/[email protected]/prometheusx/metrics.go:111","status":"Bad Request","status_code":400},"file":"/project/x/errors.go:32","func":"github.com/ory/hydra/x.LogError","http_request":{"headers":{"accept":"*/*","accept-encoding":"gzip, deflate, br","authorization":"Value is sensitive and has been redacted. To see the value set config key \"log.leak_sensitive_values = true\" or environment variable \"LOG_LEAK_SENSITIVE_VALUES=true\".","content-length":"100","content-type":"application/x-www-form-urlencoded","cookie":"Value is sensitive and has been redacted. To see the value set config key \"log.leak_sensitive_values = true\" or environment variable \"LOG_LEAK_SENSITIVE_VALUES=true\".","postman-token":"ef816695-a487-46de-8304-971051bd1d70","user-agent":"PostmanRuntime/7.29.2","x-forwarded-for":"10.244.0.1","x-forwarded-host":"oauth.lab.nextplatform.ai","x-forwarded-port":"443","x-forwarded-proto":"https","x-forwarded-scheme":"https","x-real-ip":"10.244.0.1","x-request-id":"12d9c1c73439ddec5053fe90b776f8af","x-scheme":"https"},"host":"oauth.lab.nextplatform.ai","method":"POST","path":"/oauth2/revoke","query":null,"remote":"10.244.0.20:45826","scheme":"http"},"level":"error","msg":"An error occurred","service_name":"Ory Hydra","service_version":"v2.0.3","time":"2023-02-28T02:16:51.11585019Z"}

Relevant configuration

No response

Version

v2.0.3

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Kubernetes

Additional Context

No response

[har07]

Looks like there is a similar issue for this in fosite repo: ory/fosite#644 .

In my case the error seems to be of type ErrUnauthorizedClient, which in turn goes to the else block during WriteRevocationResponse. Should I add an if errors.Is(err, ErrUnauthorizedClient) block in fosite's WriteRevocationResponse so that this type of error return 400 instead of 200?