Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Security] Add support for dynamic CSRF id with Expression in#[IsCsrfTokenValid]#54443

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@yguedidi
Copy link
Contributor

@yguedidiyguedidi commentedMar 30, 2024
edited
Loading

QA
Branch?7.1
Bug fix?no
New feature?yes
Deprecations?no
Issuescontinuation of#52961 from Hackday
LicenseMIT

Use case is for example on a list page with delete action per item, and you want a CSRF token per item, so in the template you have something like the following:

{# in a loop over multiple posts#}<formaction="{{ path('post_delete', {post:post.id}) }}"method="POST">    <inputtype="hidden"name="_token"value="{{ csrf_token('delete-post-'~post.id) }}">    ...</form>

The new feature will allow:

#[IsCsrfTokenValid(newExpression('"delete-post-" ~ args["post"].id'))]publicfunctiondelete(Request$request,Post$post):Response{// ... delete the post}

Maybe this need more tests but need help identify which test cases are useful.
Hope this can pass before the feature freeze

webda2l reacted with thumbs up emoji
@94noni
Copy link
Contributor

will this works with for example an object in the controller action ?

Expression (delete item « post.slug »)
__invoke(Post $post)

@yguedidiyguedidiforce-pushed theadd-support-for-dynamic-csrf-id-in-iscsrftokenvalid branch frome40a7e1 toa9643cbCompareMarch 30, 2024 20:35
@yguedidi
Copy link
ContributorAuthor

will this works with for example an object in the controller action ?

@94noni yes it should as it used the ExpressionLanguage component, like inIsGranted you can setsubject="post.author"

94noni reacted with rocket emoji

@yguedidiyguedidi requested a review fromsmnandreMarch 30, 2024 20:38
@smnandre
Copy link
Member

I'm curious what is the problem this scenario help to solve ? You still have to write manually this id on the template right ?

@yguedidi
Copy link
ContributorAuthor

I'm curious what is the problem this scenario help to solve ? You still have to write manually this id on the template right ?

@smnandre it's for example on a list page with delete action per item, and you want a CSRF token per item, so in the template you have something like the following:

{# in a loop over multiple posts#}<formaction="{{ path('post_delete', {post:post.id}) }}"method="POST">    <inputtype="hidden"name="_token"value="{{ csrf_token('delete-post-'~post.id) }}">    ...</form>
smnandre reacted with thumbs up emoji

@yguedidiyguedidiforce-pushed theadd-support-for-dynamic-csrf-id-in-iscsrftokenvalid branch froma9643cb to6858285CompareMarch 30, 2024 22:08
@yguedidiyguedidi requested a review fromsmnandreMarch 30, 2024 22:09
Copy link
Member

@stofstof left a comment

Choose a reason for hiding this comment

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

The code snippet in the description is also wrong. Accessing a key in an array will beargs["post"], notargs.post. In ExpressionLanguage, the. operator is a strict equivalent of the-> operator in PHP. It does not have the Twig magic.

@yguedidi
Copy link
ContributorAuthor

The code snippet in the description is also wrong. Accessing a key in an array will be args["post"], not args.post. In ExpressionLanguage, the . operator is a strict equivalent of the -> operator in PHP. It does not have the Twig magic.

good catch! was written from memory without checking, thank you, description updated

@yguedidiyguedidi requested a review fromstofApril 2, 2024 21:57
@carsonbotcarsonbot changed the titleAdd support for dynamic CSRF id with Expression in#[IsCsrfTokenValid][Security] Add support for dynamic CSRF id with Expression in#[IsCsrfTokenValid]Apr 3, 2024
@yguedidiyguedidiforce-pushed theadd-support-for-dynamic-csrf-id-in-iscsrftokenvalid branch 2 times, most recently from4dc452b to7db4866CompareApril 4, 2024 03:18
@nicolas-grekasnicolas-grekas added the ❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze" labelApr 4, 2024
@yguedidiyguedidiforce-pushed theadd-support-for-dynamic-csrf-id-in-iscsrftokenvalid branch from7db4866 toc09c734CompareApril 4, 2024 17:19
@nicolas-grekasnicolas-grekasforce-pushed theadd-support-for-dynamic-csrf-id-in-iscsrftokenvalid branch fromc09c734 to8f99ca5CompareApril 5, 2024 09:34
@nicolas-grekas
Copy link
Member

Thank you@yguedidi.

yguedidi reacted with thumbs up emoji

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

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@welcoMatticwelcoMatticwelcoMattic approved these changes

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

@smnandresmnandreAwaiting requested review from smnandre

@stofstofAwaiting requested review from stof

Assignees

No one assigned

Labels

FeatureSecurity❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze"Status: Reviewed

Projects

None yet

Milestone

7.1

Development

Successfully merging this pull request may close these issues.

7 participants

@yguedidi@94noni@smnandre@nicolas-grekas@stof@welcoMattic@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp