Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Aug 1, 2024

Conversation

rabrowne85
Copy link
Contributor

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
Copy link
Collaborator

This looks great, thank you 🙌

@rabrowne85
Copy link
Contributor Author

rabrowne85 commented Jul 30, 2024

@joedixon Thanks 👍 Does this need documenting as well?

@joedixon
Copy link
Collaborator

joedixon commented Aug 1, 2024

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

@taylorotwell taylorotwell merged commit 43ebb6f into laravel:main Aug 1, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants