CORS allowed_origins not working as expected

[davideferre]

After many retries I found that Origin header is requested to get correct CORS headers.
In one of this retry in my configuration file I've specified allowed_origins as http://* and I've created a client with allowed_cors_origins: ["http://*"]. When I call (from postman) ./well-known/jwks.json url specifying Origin: http://127.0.0.1:4200, CORS headers are missing, but when I specified Origin: http://* it worked.

Reproducing the bug
I've tried with 1.0.2 version and 1.0.8 version but the bug is still present. See attached configuration.

Server configuration

cors:
enabled: true
allowed_origins:
- http://*
allowed_methods:
- POST
- GET
- PUT
- PATCH
- DELETE
allowed_headers:
- Authorization
- Content-Type

Environment

• Version: v1.0.8
• Environment: Ubuntu, Docker

Additional context

For get it working I've to specify allowed_origins: http://localhost:4200 in server configuration and "allowed_cors_origins": ["http://127.0.0.1:4200"] into the client.

[aeneasr]

Yes, that's how CORS works: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS

Without Origin in the HTTP header, the whole CORS idea is baseless. If you use Postman to emulate a browser, you need to behave like a browser - which, when using CORS, consists of (among others) setting the right Origin header.

Unless I misunderstood what your issue is, I'm closing this. If I indeed misunderstood you, feel free to comment and I'll reopen promptly.

[davideferre]

I'm not sure if hydra is working properly because if I specify allowed_origins as http://* I would expect that with Origin:http://127.0.0.1:4200 headers the CORS are working fine but this didn't happen.

[aeneasr]

Okay, I think I misundestood what you said! So it appears that http://* is parsed as a literal string as opposed to a wildcard when using allowed_cors_origins in the client?

[davideferre]

Correct!

[aeneasr]

Got it, thanks! Triaging as bug/client.

What I don't really understand is, we actually have a test case covering your bug report:

hydra/driver/cors_test.go

Lines 92 to 100 in 95a51de

	{
	d: "should accept when basic auth client exists and origin (with partial wildcard) is allowed per client",
	prep: func() {
	r.ClientManager().CreateClient(context.Background(), &client.Client{ClientID: "foo-4", Secret: "bar", AllowedCORSOrigins: []string{"http://*.foobar.com"}})
	},
	code: http.StatusNotImplemented,
	header: http.Header{"Origin": {"http://foo.foobar.com"}, "Authorization": {"Basic Zm9vLTQ6YmFy"}},
	expectHeader: http.Header{"Vary": {"Origin"}, "Access-Control-Allow-Origin": {"http://foo.foobar.com"}},
	},

hydra/driver/cors_test.go

Lines 92 to 100 in 95a51de

	{
	d: "should accept when basic auth client exists and origin (with partial wildcard) is allowed per client",
	prep: func() {
	r.ClientManager().CreateClient(context.Background(), &client.Client{ClientID: "foo-4", Secret: "bar", AllowedCORSOrigins: []string{"http://*.foobar.com"}})
	},
	code: http.StatusNotImplemented,
	header: http.Header{"Origin": {"http://foo.foobar.com"}, "Authorization": {"Basic Zm9vLTQ6YmFy"}},
	expectHeader: http.Header{"Vary": {"Origin"}, "Access-Control-Allow-Origin": {"http://foo.foobar.com"}},
	},

The only test case I couldn't find and taht would co-incide with your report would be where both the global config and the client config are using a wildcard.

Alternatively, it could be cause you're using http://* instead of http://*.something which might break the following lines and is also not covered by tests:

hydra/driver/cors.go

Lines 119 to 125 in 95a51de

	for _, o := range cl.AllowedCORSOrigins {
	g, err := glob.Compile(strings.ToLower(o), '.')
	if err != nil {
	return false
	}
	clientPatterns = append(patterns, g)
	}

Would you be open to checking this out and submitting a PR or info with your findings?

[Aterocana]

Hello, I'm proposing this solution: Aterocana@5d7cf58 with this pull request
Basically I think the issue here is the protocol is specified, but no domain is: for example https://*.
Internally glob.Glob is compiled (with glob.Compile) with the provided string and '.' rune separator. Since g:=glob.Compile("https://*", '.') doesn't match g.Match("http://foobar.com") due to . separator, we could use g:=glob.Compile("https://**", '.') to ignore .` separator in this specific case.
In order to do so I simply tried to match if "://" substring is into an allowed-origin string.

[aeneasr]

Ohh, yes, that makes a ton of sense!