-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Derivate kernel.secret
from the decryption secret when its env var is not defined
#56985
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
Conversation
0e69ffd
to
23a87c7
Compare
…hen its env var is not defined
23a87c7
to
4749871
Compare
kernel.secret
from the decryption secret when its env var is not defined
Thank you @nicolas-grekas. |
About this:
Let's improve the error message in Symfony code to display a much more detailed error message explaining that you want to use a feature that needs a secret, that is a random alpha numeric string that you need to create, mention the recommended length, a link to the docs explaining it, etc. Ideally, we could also add a command to generate a good secret value and update the value in Thanks! |
…ng (yceruto) This PR was merged into the 7.2 branch. Discussion ---------- [FrameworkBundle] Lazy `kernel.secret` parameter resolving | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT #56985 and symfony/recipes#1317 following up The goal of this PR is to fix the current compiler-errors about a missing `kernel.secret` parameter when it's not set at all. Thus, improving the first-time experience with minimalistic apps. Commits ------- 0284011 Lazy kernel.secret parameter resolving
…ng (yceruto) This PR was merged into the 7.2 branch. Discussion ---------- [FrameworkBundle] Lazy `kernel.secret` parameter resolving | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT symfony/symfony#56985 and symfony/recipes#1317 following up The goal of this PR is to fix the current compiler-errors about a missing `kernel.secret` parameter when it's not set at all. Thus, improving the first-time experience with minimalistic apps. Commits ------- 0284011717 Lazy kernel.secret parameter resolving
…ng (yceruto) This PR was merged into the 7.2 branch. Discussion ---------- [FrameworkBundle] Lazy `kernel.secret` parameter resolving | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT symfony/symfony#56985 and symfony/recipes#1317 following up The goal of this PR is to fix the current compiler-errors about a missing `kernel.secret` parameter when it's not set at all. Thus, improving the first-time experience with minimalistic apps. Commits ------- 0284011717 Lazy kernel.secret parameter resolving
…tainer non-empty parameters (yceruto) This PR was merged into the 7.2 branch. Discussion ---------- [DependencyInjection][FrameworkBundle] Introducing container non-empty parameters | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT This new iteration (following up #57462, #56985 and symfony/recipes#1317) is about to improve the DX when we're dealing with optional parameters (this is the case for `kernel.secret` now and likely others out there) . Nicolas regarding your comment on #57462 (comment), I tried, but after some tests I realized that the impact of deprecating the `kernel.secret` is huge and, in some cases, counterproductive, as we used to reference that parameter in many configurations, see https://github.com/search?q=language%3APHP+%25kernel.secret%25+&type=code&p=3, which is currently a convenient way to share a config value. So I gave this new concept for container parameters a try. Basically, a non-empty parameter is one that must exist and cannot be [empty](https://www.php.net/manual/en/function.empty.php). It's evaluated when the `ParameterBag::get()` method is invoked. Additionally, we can now connect the parameter with its source by passing a custom error message with details on how to proceed if it fails, thus improving the DX. This is what we can achieve with this feature: ```php $container = new ContainerBuilder(); if (isset($config['secret'])) { $container->setParameter('app.secret', $config['secret']); } // NEW $container->nonEmptyParameter('app.secret', 'Did you forget to configure the "app.secret" option?'); $container->register('security_service', 'SecurityService') ->setArguments([new Parameter('app.secret')]) ->setPublic(true) ; ``` when the `security_service` is initiated/used, the `app.secret` parameter will be evaluated based on the non-empty conditions. If it's missing or empty, a helpful exception message will be thrown. Before (case when it's missing): ``` You have requested a non-existent parameter "app.secret". ``` After: ``` You have requested a non-existent parameter "app.secret". Did you forget to configure the "app.secret" option? ``` This would also address our concern about third-party services depending on the `kernel.secret` parameter when `APP_SECRET` is empty (and the `secrets` option is disabled). In that case, even if they are not checking for empty secret value in their own, it'll fail. Commits ------- 98156f7 Introducing container non-empty parameters
…tainer non-empty parameters (yceruto) This PR was merged into the 7.2 branch. Discussion ---------- [DependencyInjection][FrameworkBundle] Introducing container non-empty parameters | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT This new iteration (following up symfony/symfony#57462, symfony/symfony#56985 and symfony/recipes#1317) is about to improve the DX when we're dealing with optional parameters (this is the case for `kernel.secret` now and likely others out there) . Nicolas regarding your comment on symfony/symfony#57462 (comment), I tried, but after some tests I realized that the impact of deprecating the `kernel.secret` is huge and, in some cases, counterproductive, as we used to reference that parameter in many configurations, see https://github.com/search?q=language%3APHP+%25kernel.secret%25+&type=code&p=3, which is currently a convenient way to share a config value. So I gave this new concept for container parameters a try. Basically, a non-empty parameter is one that must exist and cannot be [empty](https://www.php.net/manual/en/function.empty.php). It's evaluated when the `ParameterBag::get()` method is invoked. Additionally, we can now connect the parameter with its source by passing a custom error message with details on how to proceed if it fails, thus improving the DX. This is what we can achieve with this feature: ```php $container = new ContainerBuilder(); if (isset($config['secret'])) { $container->setParameter('app.secret', $config['secret']); } // NEW $container->nonEmptyParameter('app.secret', 'Did you forget to configure the "app.secret" option?'); $container->register('security_service', 'SecurityService') ->setArguments([new Parameter('app.secret')]) ->setPublic(true) ; ``` when the `security_service` is initiated/used, the `app.secret` parameter will be evaluated based on the non-empty conditions. If it's missing or empty, a helpful exception message will be thrown. Before (case when it's missing): ``` You have requested a non-existent parameter "app.secret". ``` After: ``` You have requested a non-existent parameter "app.secret". Did you forget to configure the "app.secret" option? ``` This would also address our concern about third-party services depending on the `kernel.secret` parameter when `APP_SECRET` is empty (and the `secrets` option is disabled). In that case, even if they are not checking for empty secret value in their own, it'll fail. Commits ------- 98156f7d64 Introducing container non-empty parameters
I'm pursuing the goal of making
APP_SECRET
empty in the default recipe. See symfony/recipes#1314 for background.At the moment,
kernel.secret
is used for remember-be, login-links and ESI. This means that when you start a project, you don't need it. But once you do enable those features, you'll get an "APP_SECRET env var not found" error message.I think we can live with this error and the related DX. We need good doc of course.
Still, in order to make DX a bit smoother, I propose to derivate APP_SECRET from SYMFONY_DECRYPTION_SECRET when it's set.
This is what this PR does.
Of course, we should also document that creating a separate
APP_SECRET
is likely a good idea.FTR, here is how one can trivially generate a value for APP_SECRET and put it in the vault, thus fixing #38021: