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

[FrameworkBundle] Automatically enable the CSRF protection if CSRF manager exists#25151

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

Merged

Conversation

@sroze
Copy link
Contributor

QA
Branch?3.4
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsø
LicenseMIT

This will automatically enable the CSRF protection ifCsrfTokenManagerInterface exists.

@sroze
Copy link
ContributorAuthor

Travis failure is not related :)

@xabbuhxabbuh added this to the3.4 milestoneNov 24, 2017
@fabpot
Copy link
Member

Thank you@sroze.

@fabpotfabpot merged commitfd43406 intosymfony:3.4Nov 24, 2017
fabpot added a commit that referenced this pull requestNov 24, 2017
… if CSRF manager exists (sroze)This PR was merged into the 3.4 branch.Discussion----------[FrameworkBundle] Automatically enable the CSRF protection if CSRF manager exists| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | ø| License       | MITThis will automatically enable the CSRF protection if `CsrfTokenManagerInterface` exists.Commits-------fd43406 Automatically enable the CSRF protection if CSRF manager exists
This was referencedNov 30, 2017
fabpot added a commit that referenced this pull requestDec 14, 2017
This PR was merged into the 3.4 branch.Discussion----------Fixing wrong class_exists on interface| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | none| License       | MIT| Doc PR        |symfony/symfony-docs#8873 already does not mention changing anything in the configThis was a bug introduced in#25151 on the 3.4 branch. It's... pretty self-explanatory I hope :).Cheers!Commits-------be75bd9 Fixing wrong class_exists on interface
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedDec 14, 2017•
edited
Loading

Is this really a good idea?
All tests are broken now, unless we patch them to explicitly disable CSRF.
seehttps://travis-ci.org/symfony/symfony/jobs/316664330
Not a real issue on the tests side (except that someone needs to fix them actually),
but is this really the DX we want?
ping@weaverryan

@Tobion
Copy link
Contributor

Auto-enabling CSRF without it working because session are not enabled, sounds like a bad DX

LogicException: CSRF protection needs sessions to be enabled.

How about only enabling it when session is enabled as well? Or even better, we implement#13464

@fabpot
Copy link
Member

I'm going to revert this change as it was broken anyway before the fix today, so it never worked. That will give us some time to implement it properly.

fabpot added a commit that referenced this pull requestDec 15, 2017
…otection if CSRF manager exists (sroze)"This reverts commitd5f0428, reversingchanges made toe52825e.
fabpot added a commit that referenced this pull requestDec 15, 2017
* 3.4:  Revert "bug#25151 [FrameworkBundle] Automatically enable the CSRF protection if CSRF manager exists (sroze)"  Revert "bug#25502 Fixing wrong class_exists on interface (weaverryan)"
fabpot added a commit that referenced this pull requestDec 15, 2017
* 4.0:  Revert "bug#25151 [FrameworkBundle] Automatically enable the CSRF protection if CSRF manager exists (sroze)"  Revert "bug#25502 Fixing wrong class_exists on interface (weaverryan)"
@srozesroze deleted the enable-csrf-if-token-manager-exists branchDecember 15, 2017 10:14
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@chalasrchalasrchalasr approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.4

Development

Successfully merging this pull request may close these issues.

7 participants

@sroze@fabpot@nicolas-grekas@Tobion@chalasr@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp