This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[SecurityBundle] Empty line starting with dash under "access_control" causes all rules to be skipped
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Fix#40235 ... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
When the IDE by mistake puts an empty line in `access_control` in security.yaml there is no warning that we have an empty row, making the rest of routes defined, to be ignored and possible to be accessed by anyone that can authenticate no matter the role.
# How to reproduce the issue
- git clone git@github.com:monteiro/symfony-issue-40235.git
- composer install
- symfony server:start
- open 127.0.0.1:8000/admin with username: "john_user" and password "123456"
- Since that user has only ROLE_USER should not be able to access the route... but because there is an empty line in "access_control" in `security.yaml`, "by mistake" it is possible to access the protected `ROLE_ADMIN` route.
Commits
-------
ee26ce5987 [SecurityBundle] Empty line starting with dash under "access_control" causes all rules to be skipped
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[Cache] Apply NullAdapter as Null Object
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Fix https://github.com/symfony/symfony/issues/40753
| License | MIT
<!--| Doc PR | symfony/symfony-docs#... <!-- required for new features -->
There is a problem with the NullAdapter if I have to add an expression to work:
```php
$adapter = new NullAdapter();
$item = new CacheItem();
$item->set('FooBar');
if (!$adapter->save($item) && !($adapter instanceof NullAdapter)) {
throw new Exception('Uoh oh');
}
```
So the goal here is to modify the methods that are causing a problem to behave as a Null Object.
Commits
-------
f6818eb7ac [Cache] Apply NullAdapter as Null Object
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[HttpKernel] Minor fixes and tweaks in the Symfony Welcome Page
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | (not needed)
The current "Welcome Page" has a minor issue related to CSS flexbox. See how the "debug mode" text looks in smaller screens:
![before](https://user-images.githubusercontent.com/73419/114359439-4620d300-9b74-11eb-85c6-ee7afdb937df.png)
The solution is to wrap the contents in a HTML element such as `<p>`, but this PR also does some other minor tweaks. This is how it'd look now:
![after](https://user-images.githubusercontent.com/73419/114359535-5cc72a00-9b74-11eb-86c4-07fba89b8e8f.png)
Commits
-------
ee49cfe2b9 [HttpKernel] Minor fixes and tweaks in the Symfony Welcome Page
* 5.2:
[HttpClient][PHPDoc] Fix 2 remaining return mixed
[Cache] [FrameworkBundle] Fix logging for TagAwareAdapter
[Route] Better inline requirements and defaults parsing
Simplified condition and removed unused code from AbstractSessionListener::onKernelRequest
[PhpUnitBridge] Fix phpunit symlink on Windows
[Yaml] Fixed infinite loop when parser goes through an additional and invalid closing tag
[Form] Fix 'invalid_message' use in multiple ChoiceType
* 4.4:
[HttpClient][PHPDoc] Fix 2 remaining return mixed
[Cache] [FrameworkBundle] Fix logging for TagAwareAdapter
[Route] Better inline requirements and defaults parsing
Simplified condition and removed unused code from AbstractSessionListener::onKernelRequest
[PhpUnitBridge] Fix phpunit symlink on Windows
[Yaml] Fixed infinite loop when parser goes through an additional and invalid closing tag
This PR was merged into the 5.3-dev branch.
Discussion
----------
[DependencyInjection] Add env() and EnvConfigurator in the PHP-DSL
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Recently, I have been using env var processors a lot. This is a proposition to improve the DX a bit when you use the PHP-DSL to configure services.
Firstly, I am "annoyed" by the fact that I can do `param('my_param')` but not `env('MY_ENV')`.
Secondly, long chains of env var processors (eg: `%env(default:my_param:key:path:url:MY_ENV_VAR)` have two issues:
- you must construct and read them in "reverse"
- some env var processor are actually composed of 2 parts (key:path), you don't distinguish them easily from the rest
Before:
```php
->arg('$myArg', '%env(default:my_param:key:path:url:MY_ENV_VAR)%')
```
After:
```php
->arg(
'$myArg',
env('MY_ENV_VAR')
->url()
->key('path')
->default('my_param')
)
```
Custom env var processor would be callable with `->custom('my_custom_env_var_processor')` or you could extend the configurator and add your own methods.
WDYT?
Commits
-------
5f0fe3235f [DependencyInjection] Add env() and EnvConfigurator in the PHP-DSL
Remove ! symbol from requirements and defaults array keys in Route class. Leave ! symbol in Route compiled path for correct token creation
Added some inline route settings tests
This PR was merged into the 4.4 branch.
Discussion
----------
[PhpUnitBridge] Fix phpunit symlink on Windows
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
If the symlink to `.phpunit/phpunit` already exists, unlink is called to remove it. But this fails on Windows (because it is a directory and requires `rmdir`), which in turn causes the subsequent `symlink` call to fail (because it already exists).
Additionally, creating symlinks on Windows requires Administrator permissions (generally), so `.phpunit/phpunit` can never be created for ordinary Users.
This PR uses a junction instead of a symlink on Windows. It also fixes some issues with stderror output and adds some argument escaping.
Commits
-------
ff8093246b [PhpUnitBridge] Fix phpunit symlink on Windows
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[Security] Rework the remember me system
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fixes part of #39308
| License | MIT
| Doc PR | tbd
As I said in #39308, I want to change the remember me system in Symfony 5.3. The remember me system has a couple big "problems":
1. **It's hardwired into some Security classes** like `ContextListener`. The `RememberMeFactory` adds a `setRememberMe()` method call to the DI config and the context listener calls methods on this. This is very coupled, instead of the decoupled nature of the rest of security.
2. **Conditional conditions are combined with cookie creation in one class**. This is especially hard in e.g. 2FA (where setting the cookie should be done after 2FA is completed, which is currently near impossible as it's directly bound to the conditional of being called after logging in).
The changes
---
* The first commits harden the current functional test suite of remember me, to avoid breaking it.
* I discovered a lot of similarity between remember me tokens and login links. That's why I've extracted the shared logic into a generic `SignatureHasher` in the 3rd commit.
* I then remodelled `RememberMeAuthenticator` to the login link system, which I think improves a lot and at least improves problem (2) - as the conditionals (`RememberMeAuthenticator`) is split from the cookie creation (`RememberMeHandlerInterface`).
* Finally, I added a new event (`TokenDeauthenticatedEvent`) to the `ContextListener` to avoid direct coupling - solving problem (1).
This removes any usage of remember me services, which can be deprecated along with the rest of the security system.
Usage
---
As with the authenticator manager: **Nothing changes in the configuration**
Usage of persistent token providers has been improved. First, configuration is provided (setting up services is no longer needed):
```yaml
# before
services:
Symfony\Bridge\Doctrine\Security\RememberMe\DoctrineTokenProvider:
autowire: true
security:
firewalls:
main:
remember_me:
# ...
token_provider: 'Symfony\Bridge\Doctrine\Security\RememberMe\DoctrineTokenProvider'
# after
security:
firewalls:
main:
remember_me:
# ...
token_provider:
doctrine: true
```
Furthermore, a schema listener is created. Whenever the doctrine token provider is used, `make:migration`/`doctrine:schema:update` will automatically create the required table.
Some advanced usage of Remember me is also improved a lot (there is no real "before" here, consider looking at scheb/2fa to get an idea of the before). A few use-cases I took into account:
* If you ever need to **programmatically create a remember me cookie**, you can autowire `RememberMeHandlerInterface` and use `createRememberMeCookie($user)`. This will make sure the remember me cookie is set on the final response (using the `ResponseListener`)
* The `RememberMeListener` previously was responsible for both determining if a cookie must be set and setting the cookie. This is now split in 2 listeners (checking is done by `RememberMeConditionsListener`). If `RememberMeBadge` is enabled, the cookie is set and otherwise it isn't. This allows e.g. SchebTwoFactorBundle to create a listener that catches whether remember me was requested, but suppress it until the 2nd factor is completed.
Todo
---
* [x] Update UPGRADE and CHANGELOG
* [x] Show before/after examples
* [x] Investigate the conditional event registering of `ContextListener`. This forces to inject both the firewall and the global event dispatcher at the moment.
* Make sure old remember me tokens still function. As remember me tokens are long lived, we may need to provide backwards compatibility for at least Symfony 6.x. **Update: it was decided to not include this for now: https://github.com/symfony/symfony/pull/40145#issuecomment-785819607**
cc `@scheb` `@weaverryan` as you both initiated this PR by sharing the problems with the current design.
Commits
-------
15670419d4 [Security] Rework the remember me system
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[Console] Deprecate Helper::strlen() for width() and length()
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | yes
| Deprecations? | yes
| Tickets | Follow up form #40698
| License | MIT
| Doc PR |
This PR will deprecated `Helper::strlen()` since it is actually calculating the width. I remove the `@internal` on `Helper::width()` and a `Helper::length()`. I will also deprecate `Helper::strlenWithoutDecoration()` because you should use `Helper::removeDecoration()` instead.
Commits
-------
3c24aa9d47 [Console] Deprecate Helper::strlen() for width() and length()
This PR was merged into the 5.2 branch.
Discussion
----------
[Form] Fix 'invalid_message' use in multiple ChoiceType
| Q | A
| ------------- | ---
| Branch? | 5.2<!-- see below -->
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Fix#40636 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | <!-- required for new features -->
<!--
Replace this notice by a short README for your feature/bugfix. This will help people
understand your PR and can be used as a start for the documentation.
Additionally (see https://symfony.com/releases):
- Always add tests and ensure they pass.
- Never break backward compatibility (see https://symfony.com/bc).
- Bug fixes must be submitted against the lowest maintained branch where they apply
(lowest branches are regularly merged to upper ones so they get the fixes too.)
- Features and deprecations must be submitted against branch 5.x.
- Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
-->
`invalid_message` option were not take into account anymore since v5.2.4. This PR intends to fix this. The option `invalid_message` is now passed to the `POST_SUBMIT` callback, for multiple ChoiceType.
Commits
-------
f2516840c8 [Form] Fix 'invalid_message' use in multiple ChoiceType
This PR was merged into the 4.4 branch.
Discussion
----------
[Yaml] Fixed infinite loop when parser goes through an additional and invalid closing tag
| Q | A
| ------------- | ---
| Branch? | 4.4 and above
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Fix#40706 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | <!-- required for new features -->
Instead of letting the parser goes in an infinite loop because it can't get the right closing tag, throw an exception when the additional and invalid closing tag is found
Commits
-------
d5f8c887a2 [Yaml] Fixed infinite loop when parser goes through an additional and invalid closing tag
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
[Notifier] LightSMS duplicated $errorCode variable fix
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40712, #40733
| License | MIT
| Doc PR | -
Removed duplicated variable $errorCode.
Many thanks for:
@OskarStark, @jderusse and special thanks for @chalasr for fast rebase course at night :)))
Commits
-------
867769ede4 [Notifier] LightSMS duplicated $errorCode variable fix
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Notifier] Added missing changelog
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
I found this to be missing
Commits
-------
41198cccb5 [Notifier] Added missing changelog
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Notifier] Fix LightSms package name
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | n/a <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | n/a
As the namespace is LightSms and not Lightsms.
Commits
-------
2d80665a4a [Notifier] Fix LightSms package name
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Security] Add concept of required passport badges
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | n
| Tickets | Fix#39305
| License | MIT
| Doc PR | tbd
A badge on a passport is a critical security element, it determines which security checks are run during authentication. Using the `required_badges` setting, applications can make sure the expected security checks are run.
Commits
-------
01c3bf9604 [Security] Add concept of required passport badges
This PR was merged into the 4.4 branch.
Discussion
----------
[PHPDoc] Fix some union type cases
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
While working on https://github.com/symfony/symfony/issues/40154, I discovered some PHPDoc issues, I'm going to comment in the review. Upper branches will need some fixes too.
Commits
-------
dd1481642b [PHPDoc] Fix some union type cases
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Serializer] Construct annotations using named arguments
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | yes
| Tickets | N/A
| License | MIT
| Doc PR | Not needed
This is the same as #40266, but applied to the serializer annotations.
This PR proposes to bump the `doctrine/annotations` library to 1.12 to gain access to its emulation layer for named arguments. Furthermore, constructing any of the serializer's annotation classes the old way by passing an array of parameters is deprecated.
### Reasons for this change
The constructors of our annotation classes have become unnecessarily complicated because we have to support two ways of calling them:
* An array of parameters, passed as first argument, because that's the default behavior `doctrine/annotations`.
* A set of named arguments because that's how PHP 8 attributes work.
Since we can now tell the Doctrine annotation reader to use named arguments as well, we can simplify the constructors of our annotations significantly.
### Drawback
After this change, there is no easy way anymore to construct instances of most of the annotation classes directly on PHP 7. The PR has been built under the assumption that instances of this class are usually created using either Doctrine annotations or a PHP 8 attribute. Thus, most applications should be unaffected by this change.
Commits
-------
c11666264d [Serializer] Construct annotations using named arguments
This PR was merged into the 5.2 branch.
Discussion
----------
[Console] Add Helper::width() and Helper::length()
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Close#40697Fix#40634, fix#40635
| License | MIT
| Doc PR |
This PR will add add a Helper::strwidth() and a Helper::strlength(). Same with with the Helper::strlenWithoutDecoration(). It does not deprecate anything. That is done in #40695
With this PR we dont have to revert the emoji issue (ie close#40697)
FYI @grasmash, I used your tests from #40635
Commits
-------
d9ea4c597c Add test.
dc02ab3d74 [Console] Add Helper::strwidth() and Helper::strlength()
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Notifier] [CS] [5.x] Replace easy occurrences of ?: with ??
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
https://github.com/symfony/symfony/pull/40729 on 5.x
Commits
-------
726075c177 [CS] [5.x] Replace easy occurrences of ?: with ??
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Notifier] Mercure bridge: bump mercure dependency to 0.5
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| License | MIT
| Doc PR | https://github.com/symfony/symfony-docs/pull/15179
Bump Mercure bridge's `symfony/mercure` dependency to 0.5 to deal with hubs instead of publishers.
---
To be able to use `HandlerRegistry::all` method, this PR needs https://github.com/symfony/mercure/pull/50 to be merged and released.
Commits
-------
498f96f1a8 Drop support of mercure:^0.4
d3306fdc92 add support for symfony/mercure:^0.5
This PR was merged into the 4.4 branch.
Discussion
----------
[Debug][ErrorHandler] Avoid warning with Xdebug 3 with develop mode disabled
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#40677
| License | MIT
As reported in #40677, this fix is needed when Xdebug 3 is enabled but its `develop` mode is off.
Commits
-------
0e0639b129 Avoid warning with Xdebug 3 with develop mode disabled
This PR was merged into the 5.2 branch.
Discussion
----------
[Security] [Passport] improve dx and document AuthenticationException
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | N/A
| License | MIT
| Doc PR | tbd
`Passport::getUser()` (Instance of `UserPassportInterface::class`) throws an `AuthenticationException::class`
if a user does not exist. Let's document that for better DX and visibility.
Use case:
- User login w/ a `username` that does not exist (custom json authenticator)
- Attempt Authentication...
- Auth failed `LoginFailureEvent` dispatched
- snippet below:
```php
// Userland\LoginFailureEventSubscriber::class
public function dispatchFailure(LoginFailureEvent $event): void
{
$user = $event->getPassport()->getUser();
$message = new UserlandMessage($user);
$this->messageBus->dispatch($message);
}
```
- `401` status is returned.
The above subscriber fails silently because a `UsernameNotFoundException` was ultimately thrown from `UserBadge::getUser()`.
Commits
-------
97ceba0f5d improve dx and document auth exception
A badge on a passport is a critical security element, it determines which
security checks are run during authentication. Using the `required_badges`
setting, applications can make sure the expected security checks are run.
This PR was merged into the 5.3-dev branch.
Discussion
----------
[Mailer][Notifier] added missing gitattributes and gitignore
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| License | MIT
just added missing .gitattributes and .gitignore to be consistent with other components
Commits
-------
00ac6d4b45 added missing gitattributes and gitignore
* 5.2:
added missing gitattributes and gitignore
allow CurlHttpClient on Windows
[Security] Mark ExpiredLoginLinkStorage as experimental
remove references to "Silex"
This PR was merged into the 5.2 branch.
Discussion
----------
[Security] Mark ExpiredLoginLinkStorage as experimental
| Q | A
| ------------- | ---
| Branch? | 5.2
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
We missed marking this as experimental together with the other "LoginLink" features.
This PR follows the discussion in https://github.com/symfony/symfony/pull/40145/files#r589072524
Commits
-------
76a88f2211 [Security] Mark ExpiredLoginLinkStorage as experimental
This PR was squashed before being merged into the 5.3-dev branch.
Discussion
----------
LoginLink create get Request Locale
| Q | A
| ------------- | ---
| Branch? | 5.x
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| License | MIT
| Doc PR | symfony/symfony-docs#15163
While writing the documentation for PR#40153 I noticed that the RequestContext :: fromRequest method does not add the '_locale' parameter automaticaly. This new pull request to set _locale parameter to the RequestContext.
Commits
-------
433d76bfe1 LoginLink create get Request Locale