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

4668 document isCsrfTokenValid #5325

Closed
wants to merge 4 commits into from
Closed

4668 document isCsrfTokenValid #5325

wants to merge 4 commits into from

Conversation

snoek09
Copy link

@snoek09 snoek09 commented May 28, 2015

Q A
Doc fix? yes
New docs? yes
Applies to 2.6
Fixed tickets #4668


if ($this->isCsrfTokenValid('token_id', 'TOKEN')) {
// ... do something, like deleting an object
}
Copy link
Member

Choose a reason for hiding this comment

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

For consistency with other parts of the doc, there should be a comment showing the alternative not using the shortcut

}

.. versionadded:: 2.6
The ``isCsrfTokenValid()`` shortcut method was added in Symfony 2.6.
Copy link
Member

Choose a reason for hiding this comment

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

We always use "was introduced in".

@xabbuh
Copy link
Member

xabbuh commented May 30, 2015

@snoek09 Thank you for your contribution. Do you also like to open a pull request for the 2.3 branch to document the old way there?

@snoek09
Copy link
Author

snoek09 commented May 30, 2015

@xabbuh PR for 2.3 documenting the old way: #5340

Sometimes you want to use CSRF protection in an action where you don't want to use a
Symfony form.

If, for example, you're doing a DELETE action, you can use the :method:`Symfony\\Bundle\\FrameworkBundle\\Controller\\Controller::isCsrfTokenValid`
Copy link
Member

Choose a reason for hiding this comment

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

Can you please move the method role to its own line?

@snoek09
Copy link
Author

snoek09 commented Jun 2, 2015

@xabbuh done, please let me know if this needs any other changes.

}

.. versionadded:: 2.6
The ``isCsrfTokenValid()`` shortcut method was introduced in Symfony 2.6.
Copy link
Member

Choose a reason for hiding this comment

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

I prefer something like this:

.. versionadded:: 2.6
    The ``isCsrfTokenValid()`` shortcut method was introduced in Symfony 2.6.
    It is equivalent to executing the following code::

        use Symfony\Component\Security\Csrf\CsrfToken;

        $this->get('security.csrf.token_manager')->isTokenValid(new CsrfToken('token_id', 'TOKEN'));

@wouterj
Copy link
Member

wouterj commented Jul 28, 2015

👍 (apart from my 2 comments, but these can be fixed by the merger quite easily)

@snoek09
Copy link
Author

snoek09 commented Jul 28, 2015

@wouterj I created a new PR for this: #5572

@snoek09 snoek09 closed this Jul 28, 2015
wouterj added a commit that referenced this pull request Jul 29, 2015
This PR was squashed before being merged into the 2.6 branch (closes #5572).

Discussion
----------

4668 document isCsrfTokenValid

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | yes
| Applies to    | 2.6
| Fixed tickets | #4668

See original PR #5325 for comments.

Commits
-------

11383f8 4668 document isCsrfTokenValid
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.

4 participants