-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[SecurityBundle] Comma separated ips for security.access_control #38149
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
Conversation
bf95e64
to
eb570f3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Looks good to me, I don't think there is any downside to this.
src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php
Outdated
Show resolved
Hide resolved
$ips = null !== $ips ? (array) $ips : []; | ||
|
||
$this->ips = array_reduce($ips, static function (array $ips, string $ip) { | ||
return array_merge($ips, preg_split('/\s*,\s*/', $ip)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for next reviewers, this is the core of the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With minor comment
a3e3e40
to
0412e91
Compare
Thank you @a-menshchikov. |
… comma-separated string (edefimov) This PR was merged into the 5.2 branch. Discussion ---------- [Security] Allow ips parameter in access_control to accept comma-separated string | Q | A | ------------- | --- | Branch? | 5.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #40881, #40864, #40865 | License | MIT PR #38149 introduced a new feature to accept a comma-separated string in ip adresses setting in `access_control` configuration section of security bundle. However the feature works in inconsistent manner: comma-separated string can be successfully passed via environment variable, but can not be passed as plain string. This PR changes this inconsistent behavior by allowing validation pass if comma-separated list of ip addresses is given in plain string. More detailed explanation about the inconsistent behavior can be found [here](#40881 (comment)) Commits ------- 8947482 [SecurityBundle] Allow ips parameter in access_control accept comma-separated string
There is currently no way to use env vars to configure
security.access_control
ips with multiple values. Ability to use comma separated ips make it able.