Skip to content

Conversation

@lmcd
Copy link
Contributor

@lmcd lmcd commented Dec 26, 2012

After reading #6463, I think it's becoming increasingly important that kernel.secret is absolutely set to something secure.

I have seen (and also been guilty of for quick builds) projects that don't bother changing the secret. This PR adds a warning to the logger when this is detected.

Perhaps a compiler pass could be used to unsubscribe from the event once the issue has been resolved.

Note: threw this together in a few mins... might not comply with conventions

Choose a reason for hiding this comment

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

$logger can't be null. because you will have error on $this->logger->alert

Copy link
Member

Choose a reason for hiding this comment

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

Well, the logic should be fixed. The logger is optional everywhere in Symfony.

@fabpot
Copy link
Member

fabpot commented Jan 7, 2013

Closing in favor of #6598

@fabpot fabpot closed this Jan 7, 2013
fabpot added a commit that referenced this pull request Jan 7, 2013
This PR was merged into the master branch.

Commits
-------

f5290b9 [FrameworkBundle] Force users to set "kernel.secret" to something different than default "ThisTokenIsNotSoSecretChangeIt"

Discussion
----------

[RFC][BC][FrameworkBundle] Force users to set "kernel.secret" to something unique

Bug fix: kinda*
Feature addition: no
BC break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #6480
License of the code: MIT

This PR is to show different approach for "fix" suggested in #6480, as IMO there is no real point for "yet another listener" =)

This PR also introduces BC break for all users that used default value for `kernel.secret`, but IMO it's worth it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants