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

Update to Symfony 7.2 #1529

Closed
wants to merge 6 commits into from
Closed

Update to Symfony 7.2 #1529

wants to merge 6 commits into from

Conversation

javiereguiluz
Copy link
Member

No description provided.

@javiereguiluz
Copy link
Member Author

I have two questions:

@nicolas-grekas do you agree on using an empty value for the APP_SECRET env var in .env file as done in the recipe?

@stof do you know which is the best way to solve this PHPStan issue?

Error: Method App\Entity\User::getUserIdentifier() should return non-empty-string but returns string.
 ------ ----------------------------------------------------------- 
  Line   src/Entity/User.php                                        
 ------ ----------------------------------------------------------- 
  85     Method App\Entity\User::getUserIdentifier() should return  
         non-empty-string but returns string.                       
 ------ ----------------------------------------------------------- 

Thanks!

@stof
Copy link
Member

stof commented Sep 30, 2024

@javiereguiluz annotating the User entity to tell phpstan that the identifier property contains a non-empty-string and not just any string

@nicolas-grekas
Copy link
Member

A non empty kernel.secret is required for remember_me, so we cannot empty it, unless we also comment out the remember me feature in security.yaml

@javiereguiluz
Copy link
Member Author

@stof I tried adding the non-empty-string but it doesn't work. Maybe it's because the property can also be null. See

private ?string $username = null;

@nicolas-grekas if I use an empty APP_SECRET, the app works as expected, including the "Remember Me" feature. Should I see an exception instead? What am I missing?

@stof
Copy link
Member

stof commented Sep 30, 2024

@javiereguiluz aren't you seeing a deprecation at least ?

@javiereguiluz
Copy link
Member Author

No. Now I also removed APP_SECRET from .env.test and I don't see a deprecation when running tests either.

@nicolas-grekas
Copy link
Member

That's a bug, fixed by symfony/symfony#58419
If we want to have an empty APP_SECRET but still use remember me, we could configure a persistent remember strategy.

@nicolas-grekas
Copy link
Member

If we want to have an empty APP_SECRET but still use remember me, we could configure a persistent remember strategy.

Actually that's not enough because ESI/fragments need the secret also.

nicolas-grekas added a commit to symfony/symfony that referenced this pull request Oct 1, 2024
…ompute `kernel.secret` (nicolas-grekas)

This PR was merged into the 7.2 branch.

Discussion
----------

[FrameworkBundle] Never hash the empty decryption key to compute `kernel.secret`

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

As spotted by `@javiereguiluz` in symfony/demo#1529 (comment)

Commits
-------

b6d6bc1 [FrameworkBundle] Never hash the empty decryption key to compute kernel.secret
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Oct 1, 2024
…ompute `kernel.secret` (nicolas-grekas)

This PR was merged into the 7.2 branch.

Discussion
----------

[FrameworkBundle] Never hash the empty decryption key to compute `kernel.secret`

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

As spotted by `@javiereguiluz` in symfony/demo#1529 (comment)

Commits
-------

b6d6bc1fa33 [FrameworkBundle] Never hash the empty decryption key to compute kernel.secret
@javiereguiluz
Copy link
Member Author

Closing in favor of #1532.

javiereguiluz added a commit that referenced this pull request Oct 3, 2024
This PR was merged into the main branch.

Discussion
----------

Upgrade to Symfony 7.2

This PR is based on #1529 with the following changes:

- composer.lock is no committed, so that people always get the latest dependencies (note that this is how e.g. laravel installation works, so we're not inventing anything here, this is proven strategy.)
- because composer.lock is not committed, flex will run recipes that are not found in symfony.lock. In this PR, I removed listing symfony/framework-bundle there, so that its recipe is run when installing the demo app. As a result and thanks to symfony/recipes#1342, flex will generate APP_SECRET in .env.local
- I also synced all recipes meanwhile

Commits
-------

10afb78 Sync with latest recipes
8fc2dd4 Remove some unneeded config options
74d38a5 Remove the value of the APP_SECRET env var
71ff694 Update symfony/framework-bundle recipe
305f15c Update doctrine/doctrine-bundle recipe
837583a Update dependencies
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.

3 participants