Skip to content
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

6338 use csrfManager instead of csrfProvider #6586

Closed
wants to merge 2 commits into from
Closed

6338 use csrfManager instead of csrfProvider #6586

wants to merge 2 commits into from

Conversation

snoek09
Copy link

@snoek09 snoek09 commented May 21, 2016

Q A
Doc fix? yes
New docs? no
Applies to 2.7
Fixed tickets #6338

@wouterj
Copy link
Member

wouterj commented May 21, 2016

I don't think we should include the complete setup. Instead use // ... to indicate something is missing (maybe add // ... (see the previous CSRF Protection section for more information)).

$session = new Session();

$csrfGenerator = new UriSafeTokenGenerator();
$csrfStorage = new SessionTokenStorage($session);
Copy link
Member

@xabbuh xabbuh May 21, 2016

Choose a reason for hiding this comment

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

I wonder if it wasn't better to use the NativeSessionTokenStorage to not depend on the HttpFoundation component here.

Never mind, we already have this above.

@wouterj
Copy link
Member

wouterj commented May 21, 2016

👍 Thanks!

@xabbuh
Copy link
Member

xabbuh commented May 21, 2016

Thank you @snoek09.

xabbuh added a commit that referenced this pull request May 21, 2016
This PR was squashed before being merged into the 2.7 branch (closes #6586).

Discussion
----------

6338 use csrfManager instead of csrfProvider

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.7
| Fixed tickets | #6338

Commits
-------

500dce5 6338 use csrfManager instead of csrfProvider
@xabbuh xabbuh closed this May 21, 2016
@snoek09 snoek09 deleted the 6338-fix-csrf-provider-code-example branch May 21, 2016 12:12
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