hydra icon indicating copy to clipboard operation
hydra copied to clipboard

CORS not available on /health/ready

Open rawnsley opened this issue 3 years ago • 9 comments
trafficstars

Preflight checklist

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

rawnsley avatar Feb 14 '22 10:02 rawnsley

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!

aeneasr avatar Feb 20 '22 19:02 aeneasr

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 avatar Feb 21 '22 09:02 rawnsley

@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��JjSFC�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

Screenshot 2022-05-14 at 10 18 32 AM

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 avatar May 14 '22 04:05 barelyhuman

@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 avatar May 17 '22 08:05 zepatrik

@zepatrik I see, I missed the following line while going through the issue

the HTTP response headers are set correctly

Fixing it

barelyhuman avatar May 17 '22 08:05 barelyhuman

@zepatrik @Benehiko

Before I make the changes, I'd like to know your preference in this.

  1. assign the corsMiddleware helper on the routers manually using the routes exported from healthx
  2. modify healthx itself to allow adding middleware to the handlers

I prefer the 1st one but might be a little more code than the 2nd one

barelyhuman avatar May 17 '22 08:05 barelyhuman

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

zepatrik avatar May 17 '22 09:05 zepatrik

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 ?

barelyhuman avatar May 17 '22 12:05 barelyhuman

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

Benehiko avatar May 17 '22 14:05 Benehiko

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.

aeneasr avatar Mar 16 '23 11:03 aeneasr