This PR was merged into the 4.4 branch.
Discussion
----------
[FrameworkBundle] debug:autowiring: Fix wrong display when using class_alias
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | None
| License | MIT
| Doc PR | not needed
Imagine that `FooInterface` is an alias, but it is deprecated and so has a `class_alias` to `BarInterface`. Currently, `debug:autowiring` will actually print that's the autowiring alias is `BarInterface`, despite there being no such id in the container.
@nicolas-grekas originally (on purpose) made the 2nd argument to `Descriptor::getClassDescription()` be passed by reference *for* this exact feature - 56aab09b01 - but I can't figure out why. This change (which effectively removes the by-reference modifying) made no existing tests fail.
Discovered this because the whole deprecated`Doctrine\Common\Persistence\ManagerRegistry` vs newer `Doctrine\Persistence\ManagerRegistry` causes the issue.
Thanks!
Commits
-------
d34b437ce0 Fixing a bug where class_alias would cause incorrect items in debug:autowiring
This PR was merged into the 4.4 branch.
Discussion
----------
[Mailer] Add a comment to avoid more wrong PRs on this piece of code
| Q | A
| ------------- | ---
| Branch? | 4.4 <!-- see below -->
| Bug fix? | no
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Numerous bogus PRs <!-- prefix each issue number with "Fix #", if any -->
| License | MIT
| Doc PR | n/a
Commits
-------
444e616f6b [Mailer] Add a comment to avoid more wrong PRs on this piece of code
This PR was squashed before being merged into the 5.1-dev branch (closes#36525).
Discussion
----------
Improve SQS interoperability
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | NA
| License | MIT
| Doc PR | NA
The Symfony Messenger component provides a SerializerInterface to encode/decode the `Envelope`, this can be used to improve the Interoperability (see [article from jolicode](https://jolicode.com/blog/symfony-messenger-et-linteroperabilite) (french))
Sadly, the current implementation of SQS adapter json_encode the elements of the `Envelope` (`string body` + `string[] headers`) and store everything in the SQS message `Body`. That partially defect the interoperability: 3rd party have to also wrap (unwrap) message form json_encoded Body.
This PR leverage the AWS SQS `Body` and `MessageAttribute` properties to store message information:
```yaml
# before
SQS Message:
Body: {"body": "hello world", "headers": {"foo": "bar"}}
MessageAttributes: {}
# after
SQS Message:
Body: hello world
MessageAttributes:
foor: bar
```
Commits
-------
00d84c125e Improve SQS interoperability
This PR was merged into the 5.0 branch.
Discussion
----------
[FrameworkBundle] remove getContainer overwrites in tests
| Q | A
| ------------- | ---
| Branch? | 5.0
| Bug fix? | no
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets |
| Doc PR |
Since 5.0 the parent implementation throws an exception anyway making the overwrites obsolete, see https://github.com/symfony/symfony/pull/31202
Commits
-------
5ef9390b2f remove getContainer overwrites in tests
This PR was merged into the 4.4 branch.
Discussion
----------
[DependencyInjection][ServiceSubscriber] Support late aliases
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
A service subscriber that references a service that is aliased after optimization passes (after ResolveReferencesToAliasesPass technically) end up being dumped with the real service and not the alias.
I would consider it a bug but @nicolas-grekas told me it's a feature for him, this is why I'm submitting this on master.
@nicolas-grekas, feel free to close this one and open with your solution since you definitely know the subject better.
Commits
-------
24150370c3 [DependencyInjection][ServiceSubscriber] Support late aliases
This PR was merged into the 5.1-dev branch.
Discussion
----------
[OptionsResolver] remove not needed BC layer
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Commits
-------
1452619a52 remove not needed BC layer
This PR was merged into the 5.0 branch.
Discussion
----------
[Profiler] Fix profiler nullable string type
| Q | A
| ------------- | ---
| Branch? | 5.0
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | n/a
| License | MIT
| Doc PR | -
This PR fixes nullable string types in setter for the Profile class.
The detected issue comes from [the Profiler class](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/Profiler/Profiler.php#L149) :
```php
$profile->setIp($request->getClientIp()); // string or null
```
The corresponding return types for the Profile getters are allready good:
```php
/**
* Returns the IP.
*
* @return string|null The IP
*/
public function getIp()
{
return $this->ip;
}
```
Commits
-------
b5d406117d Fix profiler nullable string type
* 5.0:
[FrameworkBundle] Fix session.attribute_bag service definition
[Routing] Remove unused properties from the Route annotation
[Routing] Add missing _locale requirements
Update LdapBindAuthenticationProvider.php
Add reproducer to for hit after update expire cacheItem
[Cache] fix FilesystemTagAwareAdapter failing when a tag link preexists
* 4.4:
[FrameworkBundle] Fix session.attribute_bag service definition
[Routing] Remove unused properties from the Route annotation
[Routing] Add missing _locale requirements
Update LdapBindAuthenticationProvider.php
Add reproducer to for hit after update expire cacheItem
[Cache] fix FilesystemTagAwareAdapter failing when a tag link preexists
This PR was merged into the 3.4 branch.
Discussion
----------
[Security/Core] fix escape for username in LdapBindAuthenticationProvider.php
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| License | MIT
I think that when we call `ldap_search()` as definitely it will do the `$this->ldap->query()` call, the proper filter applied should be `LdapInterface::ESCAPE_FILTER` as documented in
https://www.php.net/manual/en/function.ldap-escape.php while `LdapInterface::ESCAPE_DN` should be used for `dn` only
This simple change should fix, I'm sorry if I'm wrong.
Commits
-------
4bda68a9a2 Update LdapBindAuthenticationProvider.php
This PR was merged into the 3.4 branch.
Discussion
----------
[FrameworkBundle] Fix session.attribute_bag service definition
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | https://github.com/symfony/symfony/issues/36465
| License | MIT
| Doc PR | -
It looks like in https://github.com/symfony/symfony/pull/36063, the fact the the `getAttributeBag` method is private was forgotten. It cannot be used as the factory method. I guess we can make it public. Should it maybe marked `@internal`?
Commits
-------
76072c6424 [FrameworkBundle] Fix session.attribute_bag service definition
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Security] Apply left-over review comments from #33558
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | n/a
This applies the review comments of @noniagriconomie in #33558. It's mostly doc fixes and one extra security-safeguard by resetting the plaintext password early (similair to what is done in `PasswordCredentials`).
Commits
-------
be3a9a93f0 Applied left-over review comments from #33558
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Security] Fix missing nullable in CsrfTokenBadge
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
Related to #33558 I noticed a minor 🤏 bug with the method return-type.
Commits
-------
5cb633c0dd Update CsrfTokenBadge.php
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Notifier] Fix error handling for Free mobile
| Q | A
| ------------- | ---
| Branch? | master
| 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 #", if any -->
| License | MIT
| Doc PR | n/a
Looks like the error handling code for Free mobile is a copy/paste from Twilio.
/cc @noniagriconomie
Commits
-------
6167ce4961 [Notifier] Fix error handling for Free mobile
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Notifier] Throw an exception when the Slack DSN is not valid
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes-ish
| New feature? | yes-ish <!-- 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 #", if any -->
| License | MIT
| Doc PR | n/a
Improved errors in case of a DSN issue.
+ proper error for the Slack DSN when path is empty (will help catch when people haven't updated their Slack DSN for 5.1).
Commits
-------
6b1a64a642 [Notifier] Throw an exception when the Slack DSN is not valid
This PR was merged into the 4.4 branch.
Discussion
----------
[Routing] Remove unused properties from the Route annotation
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
From what I see those properties are totally unused.
Commits
-------
9ac1c76fd5 [Routing] Remove unused properties from the Route annotation
This PR was squashed before being merged into the 5.1-dev branch.
Discussion
----------
[Security] AuthenticatorManager to make "authenticators" first-class security
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | tbd
The tl;dr
---
The old authentication listener + authentication provider system was replaced by a new "authenticator" system (similar to Guard authentication). All existing "auth systems" (e.g. `form_login` are now written as an "authenticator" in core).
Instead of each "authentication system" registering its own listener in the `Firewall`, there is now only one listener: `AuthenticatorManagerListener`
* `Firewall` -> executes `AuthenticatorManagerListener`
* `AuthenticatorManagerListener` -> calls `AuthenticatorManager`
* `AuthenticatorManager` -> calls each authenticator
This PR contains *no deprecations* and the "new system" is *marked as experimental*. This allows to continue to develop the new Security system during the 5.x release cycle without disturbing Symfony users. In 5.4, we can deprecate "old" Security and remove it completely in 6.0.
Important Decisions
---
* A) **The new authentication manager - `AuthenticatorManager` - now dispatches 3 important "hook" events**:
* `VerifyAuthenticatorCredentialsEvent`: occurs at the point when a "password" needs to be checked. Allows us to centralize password checking, CSRF validation, password upgrading and the "user checker" logic.
* `LoginSuccessEvent`: Dispatched after a successful authentication. E.g. used by remember me listener.
* `LoginFailedEvent`: Dispatched after an unsuccessful authentication. Also used by remember me (and in theory could be used for login throttling).
* B) **`getCredentials()`, `getUser()` and `checkCredentials()` methods from old Guard are gone: their logic is centralized**.
Authenticators now have an `authenticate(Request $request): PassportInterface` method. A passport contains the user object, the credentials and any other add-in Security badges (e.g. CSRF):
```php
public function authenticate(Request $request): PassportInterface
{
return new Passport(
$user,
new PasswordCredentials($request->get('_password')),
[
new CsrfBadge($request->get('_token'))
]
);
}
```
All badges (including the credentials) need to be resolved by listeners to `VerifyAuthenticatorCredentialsEvent`. There is build-in core support for the following badges/credentials:
* `PasswordCredentials`: validated using the password encoder factory
* `CustomCredentials`: allows a closure to do credentials checking
* `CsrfTokenBadge`: automatic CSRF token verification
* `PasswordUpgradeBadge`: enables password migration
* `RememberMeBadge`: enables remember-me support for this authenticator
* C) **`AuthenticatorManager` contains all logic to authenticate**
As authenticators always relate to HTTP, the `AuthenticatorManager` contains all logic to authenticate. It has three methods, the most important two are:
* `authenticateRequest(Request $request): TokenInterface`: Doing what is previously done by a listener and an authentication provider;
* `authenticateUser(UserInterface $user, AuthenticatorInterface $authenticator, Request $request, array $badges = [])` for manual login in e.g. a controller.
* D) **One AuthenticatorManager per firewall**
In the old system, there was 1 authentication manager containing all providers and each firewall had a specific firewall listener. In the new system, each firewall has a specific authentication manager.
* E) **Pre-authentication tokens are dropped.**
As everything is now handled inside `AuthenticatorManager` and everything is stored in the Security `Passport`, there was no need for a token anymore (removing lots of confusion about what information is inside the token).
This change deprecates 2 authentication calls: one in `AuthorizationChecker#isGranted()` and one in `AccessListener`. These seem now to be mis-used to reload users (e.g. re-authenticate the user after you change their roles). This (some "way" to change a user's roles *without* logging them out) needs to be "fixed"/added in another PR.
* F) **The remember me service now uses *all* user providers**
Previously, only user providers of authentication providers listening on that firewall were used. This change is due to practical reasons and we don't think it is common to have 2 user providers supporting the same user instance. In any case, you can always explicitly configure the user provider under `remember_me`.
* G) **Auth Providers No Longer Clear the Token on Auth Failure**
Previously, authentication providers did `$this->tokenStorage->setToken(null)` upon authentication failure. This is not yet implemented: our reasoning is that if you've authenticated successfully using e.g. the login form, why should you be logged out if you visit the same login form and enter wrong credentials?
The pre-authenticated authenticators are an exception here, they do reset the token upon authentication failure, just like the old system.
* H) **CSRF Generator Service ID No Longer Configurable**
The old Form login authentication provider allowed you to configure the CSRF generator service ID. This is no longer possible with the automated CSRF listener. This feature was introduced in the first CSRF commit and didn't get any updates ever since, so we don't think this feature is required. This could also be accomplished by checking CSRF manually in your authenticator, instead of using the automated check.
Future Considerations
---
* Remove Security sub-components: Move CSRF to `Symfony\Component\Csrf` (just like mime); Deprecated Guard; Put HTTP + Core as `symfony/security`. This means moving the new classes to `Symfony\Component\Security`
* Convert LDAP to the new system
* This is fixed (and merged) by #36243 <s>There is a need for some listeners to listen for events on one firewall, but not another (e.g. `RememberMeListener`). This is now fixed by checking the `$providerKey`. We thought it might be nice to introduce a feature to the event dispatcher:</s>
* <s>Create one event dispatcher per firewall;</s>
* <s>Extend the `kernel.event_subscriber` tag, so that you can optionally specify the dispatcher service ID (to allow listening on events for a specific dispatcher);</s>
* <s>Add a listener that always also triggers the events on the main event dispatcher, in case you want a listener that is listening on all firewalls.</s>
* Drop the `AnonymousToken` and `AnonymousAuthenticator`: Anonymous authentication has never made much sense and complicates things (e.g. the user can be a string). For access control, an anonymous user has the same meaning as an un-authenticated one (`null`). This require changes in the `AccessListener` and `AuthorizationChecker` and probably also a new Security attribute (to replace `IS_AUTHENTICATED_ANONYMOUSLY`). Related issues: #34909, #30609
> **How to test**
> 1. Install the Symfony demo application (or any Symfony application)
> 2. Clone my Symfony fork (`git clone git@github.com:wouterj/symfony`) and checkout my branch (`git checkout security/deprecate-providers-listeners`)
> 3. Use the link utility to link my fork to the Symfony application: `/path/to/symfony-fork/link /path/to/project`
> 4. Enable the new system by setting `security.enable_authenticator_manager` to `true`
Commits
-------
b1e040f311 Rename providerKey to firewallName for more consistent naming
50224aa285 Introduce Passport & Badges to extend authenticators
9ea32c4ed3 Also use authentication failure/success handlers in FormLoginAuthenticator
0fe5083a3e Added JSON login authenticator
7ef6a7ab03 Use the firewall event dispatcher
95edc806a1 Added pre-authenticated authenticators (X.509 & REMOTE_USER)
f5e11e5f32 Reverted changes to the Guard component
ba3754a80f Differentiate between interactive and non-interactive authenticators
6b9d78d5e0 Added tests
59f49b20ca Rename AuthenticatingListener
60d396f2d1 Added automatically CSRF protected authenticators
bf1a452e94 Merge AuthenticatorManager and AuthenticatorHandler
44cc76fec2 Use one AuthenticatorManager per firewall
09bed16d3d Only load old manager if new system is disabled
ddf430fc1e Added remember me functionality
1c810d5d2a Added support for lazy firewalls
7859977324 Removed all mentions of 'guard' in the new system
999ec2795f Refactor to an event based authentication approach
b14a5e8c52 Moved new authenticator to the HTTP namespace
b923e4c4f6 Enabled remember me for the GuardManagerListener
873b949cf9 Mark new core authenticators as experimental
4c06236933 Fixes after testing in Demo application
fa4b3ec213 Implemented password migration for the new authenticators
5efa892395 Create a new core AuthenticatorInterface
50132587a1 Add provider key in PreAuthenticationGuardToken
526f75608b Added GuardManagerListener
a172bacaa6 Added FormLogin and Anonymous authenticators
9b7fddd10c Integrated GuardAuthenticationManager in the SecurityBundle
a6890dbcf0 Created HttpBasicAuthenticator and some Guard traits
c321f4d73a Created GuardAuthenticationManager to make Guard first-class Security
Nicolas Grekas
Thomas Calvet
Fabien Potencier
Robin Chalas
Jérémy Derussé
Dimitri Gritsajuk
Tobias Schultze
Wouter de Jong
Laurent Masforné
Ryan Weaver
Sebastiaan Stok
