Skip to content

Fix : the "secret" config parameter is not used anymore for CSRF tokens #7891

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

Merged
merged 1 commit into from
May 17, 2017
Merged

Fix : the "secret" config parameter is not used anymore for CSRF tokens #7891

merged 1 commit into from
May 17, 2017

Conversation

cnotin
Copy link
Contributor

@cnotin cnotin commented May 10, 2017

I previously commented on this in an old closed issue (that I suppose will remain ignored): symfony/symfony#14026 (comment)

The secret config parameter is not used anymore for CSRF tokens but this statement is still written in the current documentation https://symfony.com/doc/current/reference/configuration/framework.html#secret
This is confusing and I would recommend to clarify this by removing the statement or describing that it was valid until a certain version.
My understanding is that the CSRF token provider that used this parameter has been deprecated since v2.4 and removed in v3.0 according to https://github.com/symfony/symfony/blob/2.4/src/Symfony/Component/Form/Extension/Csrf/CsrfProvider/DefaultCsrfProvider.php#L22 or http://api.symfony.com/2.3/Symfony/Component/Form/Extension/Csrf/CsrfProvider/SessionCsrfProvider.html#method___construct

I previously commented on this in an old closed issue (that I suppose will remain ignored): symfony/symfony#14026 (comment)

The _secret_ config parameter is not used anymore for CSRF tokens but this statement is still written in the current documentation https://symfony.com/doc/current/reference/configuration/framework.html#secret
This is confusing and I would recommend to clarify this by removing the statement or describing that it was valid until a certain version.
My understanding is that the CSRF token provider that used this parameter has been deprecated since v2.4 and removed in v3.0 according to https://github.com/symfony/symfony/blob/2.4/src/Symfony/Component/Form/Extension/Csrf/CsrfProvider/DefaultCsrfProvider.php#L22 or http://api.symfony.com/2.3/Symfony/Component/Form/Extension/Csrf/CsrfProvider/SessionCsrfProvider.html#method___construct
in the :doc:`remember me functionality </security/remember_me>` and for
creating signed URIs when using :ref:`ESI (Edge Side Includes) <edge-side-includes>`.
Up to version 2.4, Symfony used this value for generating the
:doc:`CSRF tokens </form/csrf_protection>`.
Copy link
Member

Choose a reason for hiding this comment

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

I would just drop this sentence entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As you wish, you know better than me the best practices of Symfony's documentation

@xabbuh xabbuh added this to the 3.2 milestone May 12, 2017
@xabbuh
Copy link
Member

xabbuh commented May 17, 2017

Thank you @ClementNotin.

@xabbuh xabbuh merged commit 61e0bc7 into symfony:3.2 May 17, 2017
xabbuh added a commit that referenced this pull request May 17, 2017
…or CSRF tokens (ClementNotin)

This PR was merged into the 3.2 branch.

Discussion
----------

Fix : the "secret" config parameter is not used anymore for CSRF tokens

I previously commented on this in an old closed issue (that I suppose will remain ignored): symfony/symfony#14026 (comment)

The _secret_ config parameter is not used anymore for CSRF tokens but this statement is still written in the current documentation https://symfony.com/doc/current/reference/configuration/framework.html#secret
This is confusing and I would recommend to clarify this by removing the statement or describing that it was valid until a certain version.
My understanding is that the CSRF token provider that used this parameter has been deprecated since v2.4 and removed in v3.0 according to https://github.com/symfony/symfony/blob/2.4/src/Symfony/Component/Form/Extension/Csrf/CsrfProvider/DefaultCsrfProvider.php#L22 or http://api.symfony.com/2.3/Symfony/Component/Form/Extension/Csrf/CsrfProvider/SessionCsrfProvider.html#method___construct

Commits
-------

61e0bc7 The "secret" config parameter is not used anymore for CSRF tokens
xabbuh added a commit that referenced this pull request May 17, 2017
weaverryan added a commit that referenced this pull request May 20, 2017
* 3.2: (39 commits)
  updating instance
  Avoid backticks in shell scripts
  Update optional_dependencies.rst
  Fix xml blocks
  pass only strings to loadUserByUsername()
  Fix Authenticator Class (getCredentials) example
  Documented addAnnotatedClassesToCompile() and the use of class patterns
  Added the picture that shows how GuardAuthenticationListener calls Authentication Guard methods.
  [#7205] minor tweak
  Simplified the use of transChoice()
  [#7875] minor tweaks
  Minor fix
  Minor changes
  Properly show all events and describe guard events
  [#7891] remove not needed sentence
  [#7773] fix line length
  Add helpful remarks on custom DataCollector
  Remove use of deprecated security.exception_listener.class parameter
  Update resources.rst
  Fix incoherent ut8mb4 collation in Doctrine setup
  ...
weaverryan added a commit that referenced this pull request May 20, 2017
* 3.4: (40 commits)
  Adding an article to explain the 3.3 changes, and how to upgrade
  updating instance
  Avoid backticks in shell scripts
  Update optional_dependencies.rst
  Fix xml blocks
  pass only strings to loadUserByUsername()
  Fix Authenticator Class (getCredentials) example
  Documented addAnnotatedClassesToCompile() and the use of class patterns
  Added the picture that shows how GuardAuthenticationListener calls Authentication Guard methods.
  [#7205] minor tweak
  Simplified the use of transChoice()
  [#7875] minor tweaks
  Minor fix
  Minor changes
  Properly show all events and describe guard events
  [#7891] remove not needed sentence
  [#7773] fix line length
  Add helpful remarks on custom DataCollector
  Remove use of deprecated security.exception_listener.class parameter
  Update resources.rst
  ...
@cnotin cnotin deleted the patch-1 branch June 8, 2018 23:42
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