Skip to content

[SecurityBundle] Allow to specify a RequestMatcher directly in an ACL definition #44670

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 31, 2022
Merged

[SecurityBundle] Allow to specify a RequestMatcher directly in an ACL definition #44670

merged 1 commit into from
Jan 31, 2022

Conversation

TristanPouliquen
Copy link
Contributor

@TristanPouliquen TristanPouliquen commented Dec 16, 2021 •
edited by derrabus
Loading

Q A
Branch? 6.1
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #44103
License MIT
Doc PR symfony/symfony-docs#16296

This PR allows users to directly specify a service reference in the definition of an access control rule. The given service MUST implement the RequestMatcherInterface.

The goal is to be able to pass custom request matchers, with more complex rules than the standard path, host, ips, ... options to have the same flexibility as the user has in defining his/her firewalls.

@carsonbot carsonbot added this to the 6.1 milestone Dec 16, 2021
@TristanPouliquen
Copy link
Contributor Author

I am going on holidays, as said in the Symfony Slack, I was unable to do a composer update on the 5.4 branch so was unable to run tests locally.

If someone wants to finish the work, go for it, otherwise I'll finish in January :-)

@derrabus
Copy link
Member

I was unable to do a composer update on the 5.4 branch

Note that 6.1 is our current feature branch. 5.4 is closed for features.

TristanPouliquen added a commit to TristanPouliquen/symfony-docs that referenced this pull request Dec 16, 2021
@TristanPouliquen
Add reference to new request_matcher option
f006ec9 
Linked to symfony/symfony#44670
TristanPouliquen added a commit to TristanPouliquen/symfony-docs that referenced this pull request Dec 16, 2021
@TristanPouliquen
Reference the new request_matcher option
c92d9e9 
Linked to symfony/symfony#44670
@TristanPouliquen TristanPouliquen mentioned this pull request Dec 16, 2021
Merged
TristanPouliquen added a commit to TristanPouliquen/symfony-docs that referenced this pull request Dec 16, 2021
@TristanPouliquen
Reference the new request_matcher option
de8f438 
Linked to symfony/symfony#44670
@TristanPouliquen
Copy link
Contributor Author

So no way of having this new small feature before SF6.1? That would be a shame imo

@wouterj
Copy link
Member

wouterj commented Dec 16, 2021

So no way of having this new small feature before SF6.1? That would be a shame imo

Yes. We only merge features in the unstable branch. For already released versions, stability is top priority so there should be as little changes in each patch release as possible (and definitely no new features).

@derrabus
Copy link
Member

So no way of having this new small feature before SF6.1?

No way, sorry.

wouterj
wouterj reviewed Dec 16, 2021
View reviewed changes
Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

Thank you for adding this feature! Adding support for a custom request matcher makes sense to me.

@carsonbot carsonbot changed the title Allow to specify a RequestMatcher directly in an ACL definition [SecurityBundle] Allow to specify a RequestMatcher directly in an ACL definition Jan 3, 2022
@TristanPouliquen
Copy link
Contributor Author

The final failing checks fail on issues that I don't think are related to my code. What can I do about it?

@TristanPouliquen
Copy link
Contributor Author

@derrabus Do you have time to review the final changes? Should I implement the configuration exception you were talking about?

@TristanPouliquen
Copy link
Contributor Author

@chalasr I see that your review is required too here :-)

@chalasr
Copy link
Member

chalasr commented Jan 27, 2022

Looks good to me once Alexander's comment is addressed (please ping if the given material is not enough)

@TristanPouliquen
Copy link
Contributor Author

I still have a failing check with Psalm, but the error seems to be on loading Psalm, so I don't know what I can do about it!

@chalasr
Copy link
Member

chalasr commented Jan 28, 2022

Can you update the SecurityBundle's changelog? The psalm issue is unrelated.

@derrabus
Copy link
Member

Thank you @TristanPouliquen.

@derrabus derrabus merged commit 96e7830 into symfony:6.1 Jan 31, 2022
@TristanPouliquen TristanPouliquen deleted the allow_request_matcher_in_access_control branch January 31, 2022 16:19
TristanPouliquen added a commit to TristanPouliquen/symfony-docs that referenced this pull request Feb 1, 2022
@TristanPouliquen
Reference the new request_matcher option
ba1a449 
Linked to symfony/symfony#44670
TristanPouliquen added a commit to TristanPouliquen/symfony-docs that referenced this pull request Feb 3, 2022
@TristanPouliquen
Reference the new request_matcher option
c9171ab 
Linked to symfony/symfony#44670
@fabpot fabpot mentioned this pull request Apr 15, 2022
Merged
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Jun 28, 2022
@javiereguiluz
minor #16296 [Security] Reference the new request_matcher option (Tri…
11b0377 
…stanPouliquen)

This PR was squashed before being merged into the 6.1 branch.

Discussion
----------

[Security] Reference the new request_matcher option

Linked to symfony/symfony#44670

<!--

If your pull request fixes a BUG, use the oldest maintained branch that contains
the bug (see https://symfony.com/releases for the list of maintained branches).

If your pull request documents a NEW FEATURE, use the same Symfony branch where
the feature was introduced (and `5.x` for features of unreleased versions).

-->

Commits
-------

f0432c4 [Security] Reference the new request_matcher option
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wouterj wouterj wouterj left review comments

@derrabus derrabus derrabus approved these changes

@chalasr chalasr chalasr approved these changes

Assignees
No one assigned
Labels
Feature SecurityBundle Status: Reviewed
Projects
None yet
Milestone
6.1
Development

Successfully merging this pull request may close these issues.

[Security][Access Control] Allow definition of a custom RequestMatcher in access control rules like in firewalls
5 participants
@TristanPouliquen @derrabus @wouterj @chalasr @carsonbot