laravel / reverb

Laravel Reverb provides a real-time WebSocket communication backend for Laravel applications.
https://reverb.laravel.com
MIT License
1.05k stars 76 forks source link

[1.x] Add support for wildcard allowed origins #233

Closed rabrowne85 closed 1 month ago

rabrowne85 commented 1 month ago

At present the allowed origins option does not allow for wildcard origins other than ['*']. As such, if you want to support a set of subdomains as well as the primary domain, you have to list out each one individually into the reverb.php config file, e.g.,:

...
'allowed_origins' => ['laravel.com', 'www.laravel.com', 'subdomain.laravel.com'],
...

This is all fair and well if the subdomains are well defined, but if you either have a lot of them or your application allows the users to define a subdomain e.g., in the context of a multi-tenant setup, then you may not be able to define these easily.

However, the only wildcard support available is the ['*'] option, but this opens it to all origins, which may not be desired.

This pull request would provide support for wildcard origins to be specified in the config, allowing users to set what origins are allowed, with some more flexibility e.g.:

...
'allowed_origins' => ['laravel.com', '*.laravel.com'],
...

With this config, reverb would allow an origin at the root domain level and any subdomain of the root domain.

The underlying change follows a similar pattern/implementation as the CORS middleware, i.e., Str::is()

The tests have been extended to include additional checks when wildcard origins are used. I had to extend the FakeConnection::class to allow for an origin to be defined besides localhost. I've added this as a 2nd parameter and in the extended tests used the name parameter.

N.B., the test 'accepts a connection from an valid origin' has an incorrect config set path for allowed origins, it just happens that the default (['*']) allows everything and as such the test passes.

joedixon commented 1 month ago

This looks great, thank you 🙌

rabrowne85 commented 1 month ago

@joedixon Thanks 👍 Does this need documenting as well?

joedixon commented 1 month ago

Yeah, it would be good to update the docs once this is merged and tagged.