hydra
hydra copied to clipboard
CORS not available on /health/ready
Preflight checklist
- [X] I could not find a solution in the existing issues, docs, nor discussions.
- [X] I agree to follow this project's Code of Conduct.
- [X] I have read and am following this repository's Contribution Guidelines.
- [ ] This issue affects my Ory Cloud project.
- [ ] I have joined the Ory Community Slack.
- [ ] I am signed up to the Ory Security Patch Newsletter.
Describe the bug
When CORS is enabled for the public endpoints, the HTTP response headers are set correctly for /.well-known/openid-configuration but not for /health/ready.
This is the logical endpoint to use for confirming service health and we would like to fetch it from our generic HTML status page.
Reproducing the bug
Returns expected result...
curl 'https://HYDRA.SERVER/.well-known/openid-configuration' -H 'User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:82.0) Gecko/20100101 Firefox/82.0' -H 'Accept: application/json' -H 'Accept-Language: en-US,en;q=0.5' --compressed -H 'Origin: http://localhost:9595' -H 'Connection: keep-alive' -H 'Referer: http://localhost:9595/' -H 'Cache-Control: max-age=0' --verbose
Returns unexpected result...
curl 'https://HYDRA.SERVER/health/ready' -H 'User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:82.0) Gecko/20100101 Firefox/82.0' -H 'Accept: application/json' -H 'Accept-Language: en-US,en;q=0.5' --compressed -H 'Origin: http://localhost:9595' -H 'Connection: keep-alive' -H 'Referer: http://localhost:9595/' -H 'Cache-Control: max-age=0' --verbose
Relevant log output
No response
Relevant configuration
server:
cors:
enabled: true
Version
1.11.0
On which operating system are you observing this issue?
Linux
In which environment are you deploying?
Docker
Additional Context
No response
server:
cors:
enabled: true
is not a valid Ory Hydra configuration. Could you please provide the configuration you are using so that we can better understand your problem? Thanks!
I don't use a configuration file, but I set SERVE_PUBLIC_CORS_ENABLED to be true in my Docker container. The rest of the configuration is completely standard. Any Hydra server with CORS enabled should exhibit this behaviour I think. Mine looks something like this...
docker run -d \
-p 4444:4444 \
-p 4445:4445 \
-e DSN=$DSN \
-e SECRETS_SYSTEM=$SYSTEM_SECRET \
-e URLS_SELF_ISSUER=https://oauth.xxx.com/ \
-e URLS_LOGIN=https://auth.xxx.com/idp/login \
-e URLS_CONSENT=https://auth.xxx.com/idp/consent \
-e URLS_ERROR=https://auth.xxx.com/idp/error \
-e SERVE_PUBLIC_CORS_ENABLED=true \
-e LOG_LEVEL=debug \
oryd/hydra:v1.11.0 serve all --dangerous-force-http
@rawnsley I've been trying to reproduce the error and here's the setup I have right now, I'd like you to add any other details you think I'm missing that are in your setup.
The script running docker
#!/bin/bash
docker run -d \
-p 4444:4444 \
-p 4445:4445 \
-e DSN=memory \
-e SECRETS_SYSTEM=¹f��JjSFC�9zx2���BJ�G�C�b�Ow�4�G��2G�B9�52y�7MUSҳ�P�S \
-e URLS_SELF_ISSUER=https://oauth.com/ \
-e URLS_LOGIN=https://auth.com/idp/login \
-e URLS_CONSENT=https://auth.com/idp/consent \
-e URLS_ERROR=https://auth.com/idp/error \
-e SERVE_PUBLIC_CORS_ENABLED=true \
-e LOG_LEVEL=debug \
oryd/hydra:v1.11.0 serve all --dangerous-force-http
Image of the same curl command
Also, If you can send through the logs from the docker container when you are making this request, that'd help too
Note: I'm using an in-memory db for this for now, so if that works and the db connection doesn't then we definitely need the logs to help you out
@barelyhuman The problem there is that the CORS headers (like Access-Control-Allow-Origin) are missing. Compare that request to any other request, e.g. /.well-known/openid-configuration which has those headers.
@zepatrik I see, I missed the following line while going through the issue
the HTTP response headers are set correctly
Fixing it
@zepatrik @Benehiko
Before I make the changes, I'd like to know your preference in this.
- assign the
corsMiddlewarehelper on the routers manually using the routes exported fromhealthx - modify
healthxitself to allow adding middleware to the handlers
I prefer the 1st one but might be a little more code than the 2nd one
I think here we just have a problem with the ordering. In Kratos the CORS middleware also works for health endpoints, so you should be able to fix it by first registering the health routes, and then later wrap the CORS middleware around that.
See the relevant code in Kratos (r.RegisterPublicRoutes also registers the health handler):
https://github.com/ory/kratos/blob/93bf1e2cd53f3a4de3ff414017c17813d36b56da/cmd/daemon/serve.go#L114-L121
I might be wrong but this is what I understand from the code and the ordering might not be the reason.
enableCORS is a passed parameter and is what decides if the below CORS() execution adds the headers or not, correct?
https://github.com/ory/hydra/blob/e586fd72a8b052c64b74acf2e1b6745556ef8348/cmd/server/handler.go#L66
and the the same is being sent an hard coded value false to the public router in both RunServePublic and RunServeAll , the env variable SERVE_PUBLIC_CORS_ENABLED isn't being used to decide if it should be enabled for the public router and I just turned it true to check if the curl is actually hitting the public router or the admin router. And it does hit the public router and the cors headers were added
as for the .well-known/* routes, which are a part of the OAuth2 handler, the m.OAuth2AwareMiddleware() (internally oauth2cors.Middleware) adds the needed headers even if the top level one's are disabled.
If these assumptions are correct, shouldn't the flag be considered ?
Yeah so this is disabled on the public routes specifically because you do not need CORS with OAuth2 requests since the browser is redirected to the page instead of doing fetch requests. Only on certain requests need CORS which is why only certain handlers are wrapped with the CORS middleware.
You can use this library https://github.com/rs/cors to wrap the health handlers
Given that health checks should be used as part of the general load balancing, and aren't really public facing endpoints, I'M closing this.