Client allowed_cors_origins not working

[Alt0252]

Describe the bug

Enabling CORS support on the public endpoints seems to be working as expected by enabling the settings in the configuration, however when setting the allowed_cors_origins setting in the client, the additional origin URLs are not being allowed.

Reproducing the bug

Steps to reproduce the behavior:

1. Create a hydra environment version 1.3.2 with the below configuration
2. create an authorization_code client my_client with allowed_cors_origin set to an alternate (additional) Origin URL
3. POST to the /oauth2/token endpoint with the Origin: header (ie 'Origin: https://client-app.example.com'), and client_id: my_client in the body of the request.
4. Request should return the following response headers
   access-control-allow-credentials: true
   access-control-allow-origin: https://client-app.example.com

Server logs

Server configuration

Expected behavior

expect https://client-app.example.com to be an allowed origin

Environment

• Version: v1.3.2
• Environment: Docker

Additional context

Add any other context about the problem here.

[aeneasr]

Would it be possible for you to provide a server config and client config (JSON or CLI command are ok) to make the reproduction easier?

This feature has several tests as it had some issues in the past and it will make bug hunting much easier :)

[Alt0252]

huh... I swear I had the server config in my initial post....

serve:
  public:
    cors:
      enabled: true
      allowed_origins: 
        - https://**.example.com
  tls:
    allow_termination_from:
      - 10.0.0.0/16
strategies:
  access_token: jwt
urls:
  consent: https://login.example.com/consent
  login: https://consent.example.com/login
  self:
    issuer: https://hydra.example.com/

Client configuration

    "client_id": "op-events-app",
    "client_name": "",
    "redirect_uris": [
        "https://myapp.example.biz/loggedIn",
    ],
    "grant_types": [
        "authorization_code",
        "refresh_token"
    ],
    "response_types": [
        "token",
        "code",
        "id_token"
    ],
    "scope": "openid offline offline_access",
    "audience": [
        "https://resource.server.example.com"
    ],
    "owner": "",
    "policy_uri": "",
    "allowed_cors_origins": [
        "https://myapp.example.biz",
    ],
    "tos_uri": "",
    "client_uri": "",
    "logo_uri": "",
    "contacts": [],
    "client_secret_expires_at": 0,
    "subject_type": "public",
    "token_endpoint_auth_method": "none",
    "userinfo_signed_response_alg": "none",
    "created_at": "2020-02-14T19:26:23Z",
    "updated_at": "2020-03-04T23:52:10Z",
    "metadata": null
}```

[aeneasr]

Nice find, it actually appears that the server-based origin only works if the server config is either

serve:
  public:
    cors:
      enabled: true
      allowed_origins: 
        - *
        # - you may include additional origins here but since there's already a wildcard it doesn't make sense

or

serve:
  public:
    cors:
      enabled: true

but not if a concrete value is set. That's definitely a bug!

Instead of doing a check here

https://github.com/ory/hydra/blob/master/driver/cors.go#L60-L70

we should merge these origins with values coming the client and then run the glob matching on top of that.

We can still keep the alwaysAllow check to reduce request latency in cases where all origins are allowed.

[aeneasr]

I think this issue was incorrectly being kept open and has been resolved since. #1959 added a test case to cover this so hopefully the issue is properly resolved :)

[Alt0252]

Unfortunately, this still isn't working as I would expect. (tested with version 1.5.2) Configuring allowed_cors_origins on the oauth client effectively does nothing, whatever I configure in serve.public.cors.allowed_origins is the only thing that's honored.

If I set serve.public.cors.enabled=false or just don't set it, then I get no access-control headers returned at all regardless of what I set in the client for allowed_cors_origins.

If I set serve.public.cors.allowed_origins to "" that works, but only because it's returning the Access-Control-Allow-Origin header of "" which effectively makes client configured cors obsolete.

[aeneasr]

Ok, sorry about that - looks like I need to spend some more time debugging this

[aeneasr]

Nope, I can't reproduce this. Please upgrade to 1.6+ just in case. Using env vars

    environment:
      - SERVE_PUBLIC_CORS_ENABLED=true
      - SERVE_PUBLIC_CORS_ALLOWED_METHODS=POST,GET,PUT,DELETE
      - SERVE_ADMIN_CORS_ENABLED=true
      - SERVE_ADMIN_CORS_ALLOWED_METHODS=POST,GET,PUT,DELETE
      - SERVE_PUBLIC_CORS_ALLOWED_ORIGINS=https://baz.bar.com

and an OAuth2 Client

$ hydra clients create --allowed-cors-origins https://foo.bar.com --id foobar --secret bazbar --endpoint http://127.0.0.1:4445

I get the correct Origin headers for baz.bar.com (server config)

 % curl -v --user foobar:bazbar -H "Origin: https://baz.bar.com" -X POST http://127.0.0.1:4444/oauth2/token --form "grant_type=code"

> POST /oauth2/token HTTP/1.1
> Host: 127.0.0.1:4444
> Authorization: Basic Zm9vYmFyOmJhemJhcg==
> User-Agent: curl/7.64.1
> Origin: https://baz.bar.com/

< Access-Control-Allow-Origin: https://baz.bar.com/
< Access-Control-Expose-Headers: Content-Type
< Content-Type: application/json;charset=UTF-8
< Vary: Origin

and foo.bar.com (client config)

% curl -v --user foobar:bazbar -H "Origin: https://foo.bar.com" -X POST http://127.0.0.1:4444/oauth2/token --form "grant_type=code"
> POST /oauth2/token HTTP/1.1
> Host: 127.0.0.1:4444
> Authorization: Basic Zm9vYmFyOmJhemJhcg==
> Origin: https://foo.bar.com

< HTTP/1.1 400 Bad Request
< Access-Control-Allow-Credentials: true
< Access-Control-Allow-Origin: https://foo.bar.com
< Access-Control-Expose-Headers: Content-Type

using any other non-whitelisted origin:

% curl -v --user foobar:bazbar -H "Origin: https://not-whitelisted.com" -X POST http://127.0.0.1:4444/oauth2/token --form "grant_type=code"
> POST /oauth2/token HTTP/1.1
> Host: 127.0.0.1:4444
> Authorization: Basic Zm9vYmFyOmJhemJhcg==
> Origin: https://not-whitelisted.com/

< HTTP/1.1 400 Bad Request
< Content-Type: application/json;charset=UTF-8
< Vary: Origin
< Date: Mon, 10 Aug 2020 10:02:17 GMT
< Content-Length: 429
< 
* Connection #0 to host 127.0.0.1 left intact

So as you can see, and as the tests already confirmed, this all works as expected.

[lorenzoranucci]

Nope, I can't reproduce this. Please upgrade to 1.6+ just in case. Using env vars
...
and an OAuth2 Client

$ hydra clients create --allowed-cors-origins https://foo.bar.com --id foobar --secret bazbar --endpoint http://127.0.0.1:4445

...
So as you can see, and as the tests already confirmed, this all works as expected.

@aeneasr I'm experiencing the same issue with hydra v1.7.4. I think that your replication attempt failed because @Alt0252 client has "token_endpoint_auth_method": "none" while your client has --secret bazbar.

It seems like that client-specific allowed_cors_domains are not considered when client request at /oauth2/token is not authenticated (public clients). I can understand the point of this behavior, but I cannot understand if it's a bug or a feature.

[aeneasr]

Right, looks like that could be an issue! Would you be open to provide a fix for this or help investigate the issue further?

[gpor0]

Is it possible to address this in 1.9.0?

[aeneasr]

Thank you, I think this is the very likely root cause of this issue, and also why we have not caught it in the integration tests (where we don't follow strict browser CORS rules but just test different headers).

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?

If so, this pesky issue could be resolved with a very brief:

if r.Method == "OPTIONS" {
  return true
}

in

hydra/x/oauth2cors/cors.go

Lines 82 to 92 in 3f96276

	AllowOriginRequestFunc: func(r *http.Request, origin string) bool {
	if alwaysAllow {
	return true
	}
	origin = strings.ToLower(origin)
	for _, p := range patterns {
	if p.Match(origin) {
	return true
	}
	}

[aeneasr]

#3159

[IronGeek]

@aeneasr, thank you for the quick fix.

I'm aware that it's already freezed, but is there a chance this update will also land on v1.x branch?

[aeneasr]

Since it's such a small change please feel free to back port it to v1.x in a PR and we'll get it into the 1.x branch :)

[har07]

Hi @aeneasr do you have plan to create v1.x release that contains this fix? thanks

[lortabac]

I'm using hydra v1.11.10 and I'm still having this issue.
If I use basic authentication the Access-Control-Allow-Origin header is set correctly. If I use the PKCE flow (no client_secret) the header is missing.

[IronGeek]

@lortabac I remember having similiar problem...

This may or may not solve your problem, but if I remember correctly...

Hydra needed the client_id for determining client specific cors settings.
The problem was, back then Hydra only look for client_id in the Authorization headers via Basic Authentication (I believe this has been fixed by #3457).

So if you're using old Hydra, even with PKCE, you still need to manually send at least the client_id information via Basic Authentication

[lortabac]

@IronGeek Thanks for your suggestion.

Unfortunately I can't include an Authorization header since the client is configured with token_endpoint_auth_method = none.
I tried but Hydra responded with "The OAuth 2.0 Client supports client authentication method 'none', but method 'client_secret_basic' was requested".

[lortabac]

As far as I can see the bug has been fixed for authentication method client_secret_basic and client_secret_post, but not yet for none (I was able to reproduce the bug on Hydra 2.2.0 too).

[IronGeek]

@IronGeek Thanks for your suggestion.

Unfortunately I can't include an Authorization header since the client is configured with token_endpoint_auth_method = none. I tried but Hydra responded with "The OAuth 2.0 Client supports client authentication method 'none', but method 'client_secret_basic' was requested".

Oh, right... I forgot about that part.
We're using token_endpoint_auth_method = none too, but since we have full control of the client, we ended up modifying it to also send Basic Authentication header (just for the client_id) for token_endpoint_auth_method = none (PKCE)...

[lortabac]

@IronGeek Sorry I don't understand.

Did you have to edit Hydra's source code? Or is it something that can be done just by configuring the client appropriately?

If I set token_endpoint_auth_method = none Hydra doesn't let me use basic-auth. If I set it to client_secret_basic public clients may leak the secret. I don't see any solutions. Maybe I am missing something.

[IronGeek]

Did you have to edit Hydra's source code? Or is it something that can be done just by configuring the client appropriately?

No, other than having this PR (#3163) merged, I didn't need to edit Hydra's source code...

I just need to make sure the client still send the client_id using basic authentication header eventhough it is using token_endpoint_auth_method = none.

To emphasize again, ONLY THE client_id is sent by the clients, the client_secret is not included, therefore it is never leaked...

With the recent PR (#3457), I think you could also sent the client_id as part of the request body

[lortabac]

Ok at last I found the solution!

In order to pass the client_id in basic-auth when token_endpoint_auth_method = none, you need to leave the colon after the client_id.

For example:

  xhr.setRequestHeader(
    "Authorization",
    "Basic " + btoa(clientId + ":")
  );

If you include a dummy password or omit the colon, it will not work.

Thank you very much @IronGeek !