Skip to content

[SecurityBundle] Allow ips parameter in access_control to accept comma-separated string #40902

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

Merged
merged 1 commit into from
May 7, 2021

Conversation

edefimov
Copy link
Contributor

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

@carsonbot carsonbot added this to the 5.2 milestone Apr 22, 2021
@edefimov edefimov force-pushed the fix-40881 branch 5 times, most recently from 9b5173f to cc6fe6f Compare April 22, 2021 21:33
@carsonbot
Copy link

Hey!

I think @scheb has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@edefimov edefimov force-pushed the fix-40881 branch 2 times, most recently from 93efd05 to 5eb1801 Compare May 1, 2021 10:18
@nicolas-grekas nicolas-grekas changed the title [Security] Allow ips parameter in access_control to accept comma-separated string [SecurityBundle] Allow ips parameter in access_control to accept comma-separated string May 7, 2021
@nicolas-grekas
Copy link
Member

Thank you @edefimov.

@nicolas-grekas nicolas-grekas merged commit 897f287 into symfony:5.2 May 7, 2021
This was referenced May 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants