Wrap server without cors about grpc-web HOT 5 CLOSED

PR incoming.

I think this is the correct change to make, even if it's impactful to users when upgrading, as many users may not be aware of the default behaviour which circumvents the browser's security model. For those who were relying on this change, it should be fairly clear to them what has broken (ie: CORS) and a release note / changelog can call this out explicitly.

Also given that the project is still an alpha, early adopters should expect the occasional bump in the name of security, stability and performance ;)

from grpc-web.

I resolved the issue with the Origin Option like this

// Wrap grpc server to grpc-web server
wrappedServer := grpcweb.WrapServer(s, grpcweb.WithOriginFunc(func(origin string) bool {
	// disable all origins, this is all done by the proxy before these servers
	return false
}))

is there any better solution?

from grpc-web.

@MarcusLongmuir - IMHO the approach should be restrictive by default when it comes to security features such as CORS; would you welcome a patch that defaults to denying all CORS requests (from remote origins) and adds a custom Option helper: grpcweb.WithAllowAllOrigins()

from grpc-web.

@jonnyreeves: I agree. Would definitely welcome a patch, but this is likely to be a breaking change and it will need to be communicated.

At the very least (and only just palatable), we could change the function name to surface the change as a compilation error to anyone depending on master of this repo.

I think we need to fix up the release process as per #43 with this in mind.

Thoughts, @mwitkow?

from grpc-web.

Only took a year and a half, but #158 has now landed which closes this issue :)

from grpc-web.