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

Allow for restricted domains #50

Merged
merged 5 commits into from
Mar 2, 2021
Merged

Allow for restricted domains #50

merged 5 commits into from
Mar 2, 2021

Conversation

jeroenvanrensen
Copy link
Contributor

Hi,

First of all, I have to say I absolutely love Filament! I really like the syntax and the ease of use.

I would like to use Filament only on a specific domain, so I created this PR.

This PR allows anyone to select a domain, and only from this domain Filament will be usable. All other domains will throw a 404 errorr.

If a user leaves the option empty, all domains will be allowed.

Thank you!
Jeroen

*/

'path' => 'admin',

'domain' => '',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a comment above the config option, like the path one above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think it might make sense to allow using an env() here too, that way you could change the domain based on staging, production, local, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryangjchandler yes I agree, I updated my PR

@ryangjchandler
Copy link
Member

@jeroenvanrensen Should the default value be null instead of an empty string?

@danharrin danharrin changed the base branch from main to develop March 2, 2021 18:02
@danharrin danharrin added the enhancement New feature or request label Mar 2, 2021
@danharrin danharrin merged commit 6430d8b into filamentphp:develop Mar 2, 2021
@danharrin
Copy link
Member

Thank you!

@jeroenvanrensen
Copy link
Contributor Author

@ryangjchandler I tested it with an empty string and it worked, so I don't think so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants