ory / hydra

The most scalable and customizable OpenID Certified™ OpenID Connect and OAuth Provider on the market. Become an OpenID Connect and OAuth2 Provider over night. Broad support for related RFCs. Written in Go, cloud native, headless, API-first. Available as a service on Ory Network and for self-hosters.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=hydra
Apache License 2.0
15.66k stars 1.5k forks source link

Hydra 2 does not send CORS headers in response to OPTIONS preflight request #3795

Open mig5 opened 4 months ago

mig5 commented 4 months ago

Preflight checklist

Ory Network Project

No response

Describe the bug

I recently upgraded a Hydra instance from v1.11.10 to v2.2.0.

After upgrading, one of my 3rd party RPs who has an SPA app and a PKCE OIDC client with our Hydra, reported that their auth flow is broken for them due to a change in the CORS behaviour.

This RP/OIDC client has a client-specific CORS URL set in their client (not in the global CORS settings)

Here is what they say:

We've dug a little deeper and I believe we've got to the root of the issue.

We noticed that despite not getting the CORS headers in the browser, we were getting them in Postman. The difference is that the browser isn't sending the 'Origin' header anymore due to a change in your response to the pre-flight OPTIONS request. When the browser doesn't receive CORS headers in the pre-flight response, it seems to assume the request isn't cross-origin and so doesn't send the headers you need for the main request.

They say this works fine on our production instance that still runs on v1.11.0 but not our public staging environment.

What do I need to do to fix it? I haven't changed anything to do with their OIDC client.

I tried adding these env vars to my docker instance but it hasn't helped:

This still doesn't return the Access-Control-Allow-Credentials, Access-Control-Allow-Headers, Access-Control-Allow-Origin, Access-Control-Allow-Methods headers in the response

curl -X OPTIONS   --user xxxxxxxx:   \
    -H "Access-Control-Request-Method: GET"   \
    -H "Access-Control-Request-Headers: Content-Type"   \
    -H "Origin: https://my-rp-site.com" \
    https://my-hydra.com/.well-known/openid-configuration -vvv

Here are all my env vars (URLs masked):

                "DSN=mysql://user:pass@tcp(dbhost:3306)/dbname?max_conns=20&max_idle_conns=4",
                "LOG_LEAK_SENSITIVE_VALUES=false",
                "OIDC_SUBJECT_IDENTIFIERS_PAIRWISE_SALT=xxxxxxxxxxxxxxxxxx",
                "OIDC_SUBJECT_IDENTIFIERS_SUPPORTED_TYPES=pairwise",
                "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
                "SECRETS_COOKIE=xxxxxxxxxxxxxxxxxx",
                "SECRETS_SYSTEM=xxxxxxxxxxxxxxxxxx",
                "SERVE_ADMIN_REQUEST_LOG_DISABLE_FOR_HEALTH=TRUE",
                "SERVE_ADMIN_TLS_ALLOW_TERMINATION_FROM=10.22.2.0/24,10.22.4.0/24",
                "SERVE_PUBLIC_CORS_ALLOWED_HEADERS=Authorization,Content-Type",
                "SERVE_PUBLIC_CORS_ALLOWED_METHODS=POST,GET,PUT,PATCH,DELETE",
                "SERVE_PUBLIC_CORS_ALLOWED_ORIGINS=https://mymainsite.com",
                "SERVE_PUBLIC_CORS_ENABLED=true",
                "SERVE_PUBLIC_CORS_EXPOSED_HEADERS=Content-Type",
                "SERVE_PUBLIC_REQUEST_LOG_DISABLE_FOR_HEALTH=TRUE",
                "SERVE_PUBLIC_TLS_ALLOW_TERMINATION_FROM=10.22.2.0/24,10.22.4.0/24,10.22.6.0/24,10.22.8.0/24",
                "SERVE_TLS_ALLOW_TERMINATION_FROM=10.22.2.0/24,10.22.4.0/24,10.22.6.0/24,10.22.8.0/24",
                "SQA_OPT_OUT=true",
                "TTL_LOGIN_CONSENT_REQUEST=1h",
                "URLS_CONSENT=https://my-login-consent-app.com/consent",
                "URLS_ERROR=https://my-login-consent-app.com/error",
                "URLS_LOGIN=https://my-login-consent-app.com/login",
                "URLS_LOGOUT=https://my-login-consent-app.com/logout",
                "URLS_SELF_ISSUER=https://my-hydra.com",

Reproducing the bug

Configure an RP with client-specific CORS URL with clientID xxxxxxxx

Try and make a pre-flight CORS request

curl -X OPTIONS   --user xxxxxxxx:   \
    -H "Access-Control-Request-Method: GET"   \
    -H "Access-Control-Request-Headers: Content-Type"   \
    -H "Origin: https://my-rp-site.com" \
    https://my-hydra.com/.well-known/openid-configuration -vvv

Relevant log output

No response

Relevant configuration

No response

Version

2.2.0

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Docker

Additional Context

No response

mig5 commented 4 months ago

OK

With the SERVE_PUBLIC_CORS_DEBUG=1, I see this in the log:

[cors] 2024/07/11 21:57:33 ServeHTTP: Preflight request
[cors] 2024/07/11 21:57:33   Preflight aborted: origin 'https://my-rp-site.com' not allowed

This is despite the fact that that origin is set for the OIDC client in question, which I am sending in the Authorization header (without a client secret, just as the 'username', which worked on Hydra 1.11.0).

If I set https://my-rp-site.com in the global Hydra CORS variable SERVE_PUBLIC_CORS_ALLOWED_ORIGINS, everything works.

But this is not ideal, it means Hydra 2.2.0 no longer responds for client-specific CORS header for a preflight request.

mig5 commented 4 months ago

Given that it no longer works when sending an Authorization header with the client_id as the username, it begs the question: what exactly is the point of the allowed_cors_origins parameter for an OIDC client, now?

It seems that it is ignored, so only the global setting works? With the global setting, I don't need to send an Authorization header at all, which makes perfect sense. But I am struggling to see a scenario where Hydra 2.2.0 would return the headers for a client that has had specific CORS headers set?

mig5 commented 4 months ago

Re-reading https://github.com/ory/hydra/issues/1754

I'm wondering what the solution here is? Is it ok to return, on OPTIONS, always true and only restrict the CORS behaviour when the actual GET/POST/... request comes through? Is that always the case and secure?

Is what @aeneasr saying here, effectively: "To make an OPTIONS pre-flight request return the allowed origin URL, you must also (in addition to configuring allowed_cors_origins for the client) configure the URL in the 'global' settings - that means, the 'client-specific' allowed_cors_origins setting is only enforced for an actual GET/POST request after the pre-flight request?

That is, a subsequent GET/POST request for that client, but which sends an Origin that is not tied to the client but is in the global settings, would correctly not be allowed (for such authenticated requests, only the URL defined in allowed_cors_origins for that client would be permitted)?

mig5 commented 4 months ago

OK, sorry for the noise. I re-read https://www.ory.sh/docs/hydra/guides/cors

Some endpoints (/oauth2/token, /userinfo, /oauth2/revoke) also include URLs listed in field allowed_cors_origins of the OAuth 2.0 Client that is making the request. For example, OAuth 2.0 Client

{ "client_id": "foo", "allowed_cors_origins": ["https://foo-bar.com/"] }

is allowed to make CORS request to /oauth2/token from origin https://foo-bar.com/ even if that origin isn't listed in public.cors.allowed_origins.

What I think the doc should also say, as a final sentence, is:

However, note that if your application needs to make the client send an unauthenticated, CORS pre-flight OPTIONS request, that origin URL needs to also be configured in the global settings, because there is no way to look up the allowed_cors_origins setting of the Client ID (there is no Client ID sent in such a request).

On the other hand, if this also means that:

^ if that works (all global CORS urls are respected, instead of enforcing only those set in allowed_cors_origins for the client as a more specific override), then I consider that a bug. I'm not sure if that's true or not though.

So that's the question: does 'allowed_cors_origins' enforce limiting the allowed URLs to only those URLs, for authenticated requests (regardless of what the global URLs are), or is it in addition to the global URLs?

lortabac commented 3 months ago

Any news on this?

lortabac commented 3 months ago

The culprit seems to be here: https://github.com/ory/hydra/blob/master/cmd/server/handler.go#L60

As far as I can see this handler overrides the one that is set in oauth2cors, which takes client-specific configurations into account.

If you delete the lines 60-67, the old behavior is restored (accept all OPTIONS requests and only restrict CORS when the actual request comes).