Skip to content

[Security] Reference the new request_matcher option #16296

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
Jun 28, 2022
Merged

[Security] Reference the new request_matcher option #16296

merged 1 commit into from
Jun 28, 2022

Conversation

TristanPouliquen
Copy link
Contributor

@carsonbot carsonbot added this to the 5.4 milestone Dec 16, 2021
@TristanPouliquen TristanPouliquen mentioned this pull request Dec 16, 2021
Merged
@derrabus
Copy link
Member

Please target 6.1 here as well.

@xabbuh xabbuh modified the milestones: 5.4, 6.1, next Dec 26, 2021
@TristanPouliquen TristanPouliquen changed the base branch from 5.4 to 6.1 January 3, 2022 16:51
@wouterj wouterj added the Waiting Code Merge Docs for features pending to be merged label Jan 14, 2022
derrabus added a commit to symfony/symfony that referenced this pull request Jan 31, 2022
@derrabus
feature #44670 [SecurityBundle] Allow to specify a RequestMatcher dir…
96e7830 
…ectly in an ACL definition (TristanPouliquen)

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

Discussion
----------

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

| 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.

Commits
-------

1b5516e [SecurityBundle] Allow to specify a RequestMatcher directly in an ACL definition
@derrabus
Copy link
Member

I think, the documentation should show an actual code example of a configured request matcher instead of just mentioning that it is somehow possible to provide your own matcher.

@TristanPouliquen
Copy link
Contributor Author

I tried adding one copying from other similar configuration code blocks, is it ok/enough?

@derrabus
Copy link
Member

We usually don't write whole documentation blocks inside a versionadded block. Just document the feature as if it has always existed and add a small versionadded block blow that new piece of documentation where you tell the reader that this feature has actually been added in 6.1. Once we work on the documentation for Symfony 7, a maintainer can simply drop that versionadded block without touching the rest of your documentation.

@TristanPouliquen TristanPouliquen force-pushed the TristanPouliquen-patch-request-matcher branch from 99bdf47 to 036cb66 Compare February 1, 2022 08:58
derrabus
derrabus reviewed Feb 1, 2022
View reviewed changes
@TristanPouliquen TristanPouliquen force-pushed the TristanPouliquen-patch-request-matcher branch from b06c057 to 51181f5 Compare February 3, 2022 08:55
@TristanPouliquen
Copy link
Contributor Author

@derrabus Is it ok for you to merge now? :)

derrabus
derrabus approved these changes Feb 16, 2022
View reviewed changes
Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

LGTM, but I'm powerless on this repository. Someone from the docs team will need to review and merge your PR. 🙃

@TristanPouliquen
Copy link
Contributor Author

LGTM, but I'm powerless on this repository. Someone from the docs team will need to review and merge your PR. upside_down_face

Ok, sorry for the ping then, who should I notify to see it merged? :)

nicolas-grekas
nicolas-grekas reviewed Feb 23, 2022
View reviewed changes
security.rst
.. versionadded:: 6.1

Since Symfony 6.1, an access control rule can also be directly configured by passing a service
implementing `RequestMatcherInterface` through the `request_matcher` option.
Copy link
Member

Choose a reason for hiding this comment

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

backticks should be doubled

@javiereguiluz javiereguiluz added Security and removed Waiting Code Merge Docs for features pending to be merged labels Jun 28, 2022
@carsonbot carsonbot changed the title Reference the new request_matcher option [Security] Reference the new request_matcher option Jun 28, 2022
@javiereguiluz javiereguiluz force-pushed the TristanPouliquen-patch-request-matcher branch from 51181f5 to f0432c4 Compare June 28, 2022 13:48
@javiereguiluz javiereguiluz merged commit 11b0377 into symfony:6.1 Jun 28, 2022
@javiereguiluz
Copy link
Member

Tristan, sorry it took us so long to merge your contribution 🙏 ... and congrats on your first Symfony Docs contribution 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@derrabus derrabus derrabus approved these changes

Assignees
No one assigned
Labels
Security Status: Reviewed
Projects
None yet
Milestone
next
Development

Successfully merging this pull request may close these issues.
7 participants
@TristanPouliquen @derrabus @javiereguiluz @nicolas-grekas @wouterj @xabbuh @carsonbot