This PR was merged into the 5.1-dev branch.
Discussion
----------
[Security] Added LDAP support to Authenticator system
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
The last missing authenticator in the new system 🎉
I have no experience with LDAP at all and I didn't succeed in setting up a server locally. So I can't test whether this works, but the unit test works (and also tested in a real app, while adding a `dd()` call in the listener).
---
I want to share with you the current state of Security LDAP, how this PR implements it and a possible other solution (which I think I would prefer most). Is there anyone who can share their opinions on this? (hopefully @weaverryan and @csarrazi can share their opinion, as they have most experience on this topic)
1. **Current Solution: An LDAP authentication provider + duplicated `SecurityFactory` classes**
LDAP is done in one centralized authentication provider. This provider is configured by security factories for each core factory (e.g. `form_login` becomes `form_login_ldap`, `http_basic` becomes `http_basic_ldap`).
2. **Implementation in this PR: A listener is executed before the default `VerifyCredentialsListener`, to verify `PasswordCredentials`**
This listener must be configured for each specific authenticator wanting to use LDAP. This is a technique similar to (1). It's a bit difficult to use this for your own authenticator (you need to configure a custom listener service) and still needs the duplicated factory classes
3. **Proposal: Introduce a `LdapCredentials` class and always register a listener**
If an authentictor returns `LdapCredentials`, it'll be checked using the LDAP verification listener. This is the easiest for custom authenticators and would remove the duplicated factories, I can imagine `form_login` getting a new `ldap` sub option to configure the settings.
The main disadvantage (I think) is that we would need to make `LdapCredentials` configure all options: ldap service, dnString, searchDn, searchPassword & queryString. Especially passing around the ldap service seems a bit weird. The main questions here are: Is it weird to pass all these things in the `LdapCredentials`? And, do we really need to support having multiple LDAP configuration sets for different authenticators? Or can we e.g. add a global `security.ldap` configuration, that registers the listener for all authenticators returning `LdapCredentials`?
Commits
-------
20962e604a [Security] Added LDAP support to Authenticator system
This PR was squashed before being merged into the 5.1-dev branch.
Discussion
----------
[Translations] Throw exception if xFileLoader dependencies don't exist.
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix#36658
| License | MIT
| Doc PR |
`XliffFileLoader` & `QtFileLoader` both require `XmlUtils::class` from the `Config` component. This PR throws a friendly exception is the `Config` component does not exist.
Original idea by @xabbuh was to throw the exception from the __constructor. This PR throws the exception from the `load()` method to be consistent with the `YamlFileLoader::class`. But that can easily be changed.
Commits
-------
627e476eb4 [Translations] Throw exception if xFileLoader dependencies don't exist.
This PR was squashed before being merged into the 5.1-dev branch.
Discussion
----------
[AmazonSqsMessenger] Use AsyncAws to handle SQS communication
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | /
| License | MIT
| Doc PR | /
Similar to #35992 this PR use AsyncAws to handle Sqs messages sent/receive
It move complexity of authentication/streaming outside Symfony while keeping HttpClient integration.
Commits
-------
7c4888eed1 [AmazonSqsMessenger] Use AsyncAws to handle SQS communication
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Messenger] Fix messenger:failed:remove can not remove single message
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36659
| License | MIT
| Doc PR | -
Fix this error:
```
count(): Parameter must be an array or an object that implements Countable
```
When calling `messenger:failed:remove` command from other code with single id
Commits
-------
e66cd97ec3 [Messenger] Fix messenger:failed:remove can not remove single message
This PR was merged into the 5.1-dev branch.
Discussion
----------
Add support of PHP8 static return type for withers
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | https://github.com/orgs/symfony/projects/1#card-35852557
| License | MIT
| Doc PR |
Commits
-------
04fdf05cff Add support of PHP8 static return type for withers
This PR was merged into the 5.1-dev branch.
Discussion
----------
[DI] allow loading and dumping tags with an attribute named "name"
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
This is a minor feature added for consistency: using PHP, we can already define tags with an attribute named `"name"`. But then, we cannot dump such definitions in YAML nor XML since we don't have a syntax to declare such tags in these formats.
I spotted this while looking at a dumped container: we already use an attribute named `"name"` on two tags: `cache.pool` and `workflow.definition`. Currently, the dumped XML is wrong because of this.
This PR enables the following new syntaxes (the current style still works as usual):
- in YAML, consistently with the new syntax for method calls:
```yaml
tags:
- cache.pool: { name: my_cache_pool }
```
- in XML:
```xml
<tag name="my_cache_pool">cache.pool</tag>
```
Commits
-------
b023e4cac3 [DI] allow loading and dumping tags with an attribute named "name"
This PR was merged into the 5.1-dev branch.
Discussion
----------
[HttpKernel] make kernels implementing `WarmableInterface` be part of the cache warmup stage
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
This allows your kernel to return extra classes to preload also (which was my main motivation for creating this PR actually.)
```php
// ...
use Symfony\Component\HttpKernel\CacheWarmer\WarmableInterface;
// ...
class Kernel ... implements ..., WarmableInterface
{
// ...
public function warmUp(string $cacheDir): array
{
// ...
return [
SomeClassToPreload::class,
AnotherClassClassToPreload::class,
$cacheDir.'/some-file-to-preload.php',
// ...
];
}
// ...
}
```
Commits
-------
649e530356 [HttpKernel] make kernels implementing `WarmableInterface` be part of the cache warmup stage
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Form] deprecate not using a rounding mode
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| Deprecations? | yes
| Tickets |
| License | MIT
| Doc PR |
Commits
-------
25ba1a241d deprecate not using a rounding mode
This PR was squashed before being merged into the 5.1-dev branch.
Discussion
----------
[Mailer] Use AsyncAws to handle SES requests
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | #33183, #35468 and #35037
| License | MIT
| Doc PR | TODO
alternative to #33326
This PR replace the native code to call AWS SES by the new [AsyncAws](https://github.com/async-aws/aws) project maintained by @Nyholm and me.
This removes complexity of signing request, and adds new features likes:
- authentication via .aws/config.ini, Instance profile, WebIdentity (K8S service account)
- usesignature V4 (the one recommanded by the Official SDK )
- fully compatible with API (uses the official AWS SDK interface contract to generate classes)
Because it's based on `symfony/http-client`, it's fully integrable with Symfony application.
Commits
-------
21243874bc [Mailer] Use AsyncAws to handle SES requests
This PR was merged into the 4.4 branch.
Discussion
----------
[Translation] Fix for translation:update command updating ICU messages
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36459
| License | MIT
If `translation:update` command executed with option `--domain=messages` – it ignore `messages-intl-icu` file and just create new `messages`
Method `TranslationUpdateCommand::filterCatalogue()` on `MessageCatalogue::all()` method to get all messages for domain
But `MessageCatalogue::all()` method disredard `intl-icu` domains and simply merge all.
[Translation] added $strict parameter for MessageCatalogueInterface::all() to be able to get only defined domain messages
[FrameworkBundle] modified translation:update command to respect intl-icu domain
Commits
-------
567cee5f02 [Translation] Fix for translation:update command updating ICU messages
* Anonymous users are actual to unauthenticated users, both are now represented by no token
* Added a PUBLIC_ACCESS Security attribute to be used in access_control
* Deprecated "anonymous: lazy" in favor of "lazy: true"
This PR was squashed before being merged into the 3.4 branch (closes#36627).
Discussion
----------
[Validator] fix lazy property usage.
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36343
| License | MIT
| Doc PR |
This attempts to fix a large regression introduced in #36343, which broke recursing values returned from `getter` Constraints, because they are now wrapped in in a `LazyProperty`. The `LazyProperty` needs to be evaluated because some checks are done on the type of `$value`, i.e `is_array` etc... in `validateGenericNode`.
I'm concerned that the original PR didn't really add sufficient test coverage for the introduction of `LazyProperty`, and I'm not 100% sure that I've caught all the cases where the `instanceof` check are needed in this PR.
For the tests, I added the `@dataProvider getConstraintMethods` to every test that hit the problem area of code.
~~The only issue is that my fixed has broken the test introduced in #36343, `testGroupedMethodConstraintValidateInSequence`.~~
~~I think I need @HeahDude to help me work through this. Maybe there is a more simple solution, one that doesn't require doing `instanceof LazyPropery` checks in multiple places, because this feels very brittle.~~
EDIT: fixed that test.
Commits
-------
281861e788 [Validator] fix lazy property usage.
This PR was merged into the 4.4 branch.
Discussion
----------
[Form] provide a useful message when extension types don't match
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36610
| License | MIT
| Doc PR |
Commits
-------
88d836643a provide a useful message when extension types don't match
This PR was merged into the 4.4 branch.
Discussion
----------
[Serializer] do not transform empty \Traversable to Array
| 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 | na
| License | MIT
| Doc PR | na
Today, using `PRESERVE_EMPTY_OBJECTS` ([introduced in 4.0](f28e826627)), the JSON serialization of:
```php
<?php
$object = [];
$object['foo'] = new \ArrayObject();
$object['bar'] = new \ArrayObject(['notempty']);
$object['baz'] = new \ArrayObject(['nested' => new \ArrayObject()]);
```
Outputs:
```json
{"foo":[],"bar":["notempty"],"baz":{"nested":[]}}
```
Instead of the expected:
```json
{"foo":{},"bar":["notempty"],"baz":{"nested":{}}}
```
This issue comes from the Serializer that transforms `Traversable` to an Array [here](11a707200d/src/Symfony/Component/Serializer/Serializer.php (L159)). Also, the `AbstractObjectNormalizer` [doesn't support Traversable](11a707200d/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php (L134)), but he allows to preserve empty objects.
I propose this patch where the fix doesn't transform a `Traversable` to an Array. I see another way to patch this in which we could allow empty Traversable in the `AbstractObjectNormalizer` (not sure it's better though). See attached [other-fix.patch](https://github.com/symfony/symfony/files/4539865/other-fix.log) to see the alternative patch.
Commits
-------
e5c20293fa Fix serializer do not transform empty \Traversable to Array
This PR was merged into the 5.1-dev branch.
Discussion
----------
Add missing port SQS Host Header request
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | /
| License | MIT
| Doc PR | /
When user provides a custom endpoint, the port is missing from the `Host` headers, leading to wrong URL when calling `getQueueUrl`
Commits
-------
41165beb48 Add missing port SQS Host Header request
This PR was merged into the 4.4 branch.
Discussion
----------
[Cache] Fixed not supported Redis eviction policies
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | no
| License | MIT
| Doc PR | no
**Steps to reproduce:**
1. Define the following redis service on SymfonyCloud:
```
rediscache:
type: redis:5.0
size: S
configuration:
maxmemory_policy: allkeys-lru
```
2. Deploy the change
**Expected result:**
No redis cache will be populated
**Actual result:**
Following exception is thrown:
```
[2020-04-28T05:35:58.440403-04:00] php.CRITICAL: Uncaught Error: Return value of Symfony\Component\Cache\Adapter\RedisTagAwareAdapter::doSave() must be of the type array, bool returned {"exception":"[object] (TypeError(code: 0): Return value of Symfony\\Component\\Cache\\Adapter\\RedisTagAwareAdapter::doSave() must be of the type array, bool returned at /app/vendor/symfony/cache/Adapter/RedisTagAwareAdapter.php:100)"} []
```
Commits
-------
3d6e942da5 [Cache] Fixed not supported Redis eviction policies
This PR was merged into the 5.1-dev branch.
Discussion
----------
[FrameworkBundle][CacheWarmupCommand] Append files to preload
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Files returned by the warmUp() method must be added to the preload file.
I also added checks to avoid empty preload calls in the preload file like this:
```
$classes = [];
Preloader::preload($classes);
```
Commits
-------
a82c7ab4c0 [FrameworkBundle][CacheWarmupCommand] Append files to preload
* 5.0:
updated VERSION for 3.4.40
update CONTRIBUTORS for 3.4.40
updated CHANGELOG for 3.4.40
[WebProfilerBundle] changed label of peak memory usage in the time & memory panels (MB into MiB)
add tests for the ConstraintViolationBuilder class
Improve dirname usage
[PhpUnitBridge] Use COMPOSER_BINARY env var if available
Allow invalidateTags calls to be traced by data collector
[YAML] escape DEL(\x7f)
fix compatibility with phpunit 9
[Cache] skip APCu in chains when the backend is disabled
[Mailer] Add a comment to avoid more wrong PRs on this piece of code
[Form] apply automatically step=1 for datetime-local input
remove getContainer overwrites in tests
Fixing a bug where class_alias would cause incorrect items in debug:autowiring
[DependencyInjection][ServiceSubscriber] Support late aliases
Fix profiler nullable string type
* 4.4:
updated VERSION for 3.4.40
update CONTRIBUTORS for 3.4.40
updated CHANGELOG for 3.4.40
[WebProfilerBundle] changed label of peak memory usage in the time & memory panels (MB into MiB)
add tests for the ConstraintViolationBuilder class
Improve dirname usage
[PhpUnitBridge] Use COMPOSER_BINARY env var if available
Allow invalidateTags calls to be traced by data collector
[YAML] escape DEL(\x7f)
fix compatibility with phpunit 9
[Cache] skip APCu in chains when the backend is disabled
[Mailer] Add a comment to avoid more wrong PRs on this piece of code
[Form] apply automatically step=1 for datetime-local input
Fixing a bug where class_alias would cause incorrect items in debug:autowiring
[DependencyInjection][ServiceSubscriber] Support late aliases
* 3.4:
updated VERSION for 3.4.40
update CONTRIBUTORS for 3.4.40
updated CHANGELOG for 3.4.40
[WebProfilerBundle] changed label of peak memory usage in the time & memory panels (MB into MiB)
add tests for the ConstraintViolationBuilder class
Improve dirname usage
[PhpUnitBridge] Use COMPOSER_BINARY env var if available
[YAML] escape DEL(\x7f)
fix compatibility with phpunit 9
[Cache] skip APCu in chains when the backend is disabled
[Form] apply automatically step=1 for datetime-local input
This PR was merged into the 4.4 branch.
Discussion
----------
[Cache] Allow invalidateTags calls to be traced by data collector
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | #34810
| License | MIT
| Doc PR |
`TraceableTagAwareAdapter` is not used in the fullstack framework since tag aware pools don't have the `cache.pool` tag (it's the decorated adapter that has it). This PR aims to use `TraceableTagAwareAdapter` when a pool is configured with `tags: true`
Commits
-------
28fdb3a879 Allow invalidateTags calls to be traced by data collector
This PR was merged into the 5.1-dev branch.
Discussion
----------
[DI] add syntax to stack decorators
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix#30599
| License | MIT
| Doc PR | -
Declare this:
```yaml
services:
my_stack_of_decorators:
stack:
- class: App\ExternalDecorator
- class: App\InternalDecorator
- class: App\DecoratoredClass
```
And get this:
![image](https://user-images.githubusercontent.com/243674/78615803-b8c8e580-7872-11ea-95c2-22cb78f88ca8.png)
The PR is now ready with support for Yaml, XML and the PHP-DSL. It needs #36388, #36392 and #36389 to pass, and relates to #36390 to be DX-friendly.
The new syntax now supports composable stacks - i.e stack you can reuse in the middle of another stack.
RIP middleware, simple decorators FTW :)
From the test cases:
```yaml
services:
reusable_stack:
stack:
- class: stdClass
properties:
label: A
inner: '@.inner'
- class: stdClass
properties:
label: B
inner: '@.inner'
concrete_stack:
stack:
- parent: reusable_stack
- class: stdClass
properties:
label: C
```
This will create a service similar to:
```php
(object) [
'label' => 'A',
'inner' => (object) [
'label' => 'B',
'inner' => (object) [
'label' => 'C',
]
],
];
```
When used together with autowiring, this is enough to declare a stack of decorators:
```yaml
services:
my_processing_stack:
stack:
- App\ExternalDecorator: ~
- App\InternalDecorator: ~
- App\TheDecoratedClass: ~
```
See fixtures for the other configuration formats.
See also https://twitter.com/nicolasgrekas/status/1248198573998604288
Todo:
- [x] rebase on top of #36388, #36392 and #36389 once they are merged
- [x] test declaring deeper nested stacks
Commits
-------
98eeeae3d1 [DI] add syntax to stack decorators
This PR was merged into the 5.1-dev branch.
Discussion
----------
[DI] fix definition and usage of AbstractArgument
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Reading https://symfony.com/blog/new-in-symfony-5-1-abstract-service-arguments and the comments there made me realize that the current implementation is not generic enough. Abstract arguments can be found anywhere, not only as service arguments. Also, `AbstractArgument` instances should not convey the key/id since that makes them harder to use in the PHP-DSL.
Commits
-------
abb463c749 [DI] fix definition and usage of AbstractArgument
This PR was squashed before being merged into the 5.1-dev branch.
Discussion
----------
[Serializer] Add an @Ignore annotation
| Q | A
| ------------- | ---
| Branch? | master
Bug fix? | no
| New feature? | yes <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks? | no <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass? | yes <!-- please add some, will be required by reviewers -->
| Fixed tickets | #24071
| License | MIT
| Doc PR | n/a
Add an `@Ignore` annotation to configure [ignored attributes](https://symfony.com/doc/current/components/serializer.html#ignoring-attributes) in a convenient way, as well as the related XML and YAML loaders.
TODO:
* [x] Add tests
Commits
-------
8526d7c050 [Serializer] Add an @Ignore annotation
This PR was merged into the 5.1-dev branch.
Discussion
----------
[String] Add locale-sensitive map for slugging symbols
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | Fix#36383
| License | MIT
By default chars '@' and '&' are respectively replaced by 'at' and 'and' (so limited by enlgish language).
I had an $options arguments to 'slug' method to replace chars with your own logic.
Commits
-------
1331584fa1 [String] Add locale-sensitive map for slugging symbols
This PR was merged into the 3.4 branch.
Discussion
----------
[Cache] skip APCu in chains when the backend is disabled
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix#34962
| License | MIT
| Doc PR | -
I think this should do it.
Commits
-------
5a7208481d [Cache] skip APCu in chains when the backend is disabled
This PR was merged into the 5.1-dev branch.
Discussion
----------
[DI] skip preloading dependencies of non-preloaded services
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Suggested by @stof on Slack: this improves preloading by propagating the `container.no_preload` tag to services that are referenced only by not-preloaded services.
The benefit is double:
1. this fixes potential over-preloading
2. this requires less work from the community: no need to add the tag anymore most of the time
As a corollary, listeners of console events are tagged with `container.no_preload` automatically now.
Commits
-------
add867020a [DI] skip preloading dependencies of non-preloaded services
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 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 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 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
The AuthenticatorManager now performs the whole authentication process. This
allows for manual authentication without duplicating or publicly exposing parts
of the process.
This to remove confusion between the new system and Guard. When using the new
system, guard should not be installed. Guard did however influence the idea
behind the new system. Thus keeping the mentions of "guard" makes it confusing
to use the new system.
This allows more flexibility for the authentication manager (to e.g. implement
login throttling, easier remember me, etc). It is also a known design pattern
in Symfony HttpKernel.
This removes the introduced dependency on Guard from core. It also allows an
easier migration path, as the complete Guard subcomponent can now be deprecated
later in the 5.x life.
This is an iteration on the AuthenticatorInterface of the Guard, to allow more
flexibility so it can be used as a real replaced of the authentication
providers and listeners.
This PR was merged into the 4.4 branch.
Discussion
----------
[Cache] CacheItem with tag is never a hit after expired
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes/no
| New feature? | no
| Deprecations? | no
| Tickets | Fix#36458
| License | MIT
| Doc PR | symfony/symfony-docs#... <!-- required for new features -->
It seems like a tag cacheItem is never a hit again. Not sure how fix this but the cache component is really hard to debug 🙈 .
It need to be somewhere generally as all TagAware caches are effected:
```
1) Symfony\Component\Cache\Tests\Adapter\FilesystemTagAwareAdapterTest::testRefreshAfterExpires
Failed asserting that false is true.
/home/travis/build/symfony/symfony/src/Symfony/Component/Cache/Tests/Traits/TagAwareTestTrait.php:194
2) Symfony\Component\Cache\Tests\Adapter\PredisTagAwareClusterAdapterTest::testRefreshAfterExpires
Failed asserting that true is false.
/home/travis/build/symfony/symfony/src/Symfony/Component/Cache/Tests/Traits/TagAwareTestTrait.php:183
3) Symfony\Component\Cache\Tests\Adapter\RedisTagAwareAdapterTest::testRefreshAfterExpires
Failed asserting that true is false.
/home/travis/build/symfony/symfony/src/Symfony/Component/Cache/Tests/Traits/TagAwareTestTrait.php:183
4) Symfony\Component\Cache\Tests\Adapter\RedisTagAwareClusterAdapterTest::testRefreshAfterExpires
Failed asserting that true is false.
/home/travis/build/symfony/symfony/src/Symfony/Component/Cache/Tests/Traits/TagAwareTestTrait.php:183
```
Commits
-------
d082eca7dd Add reproducer to for hit after update expire cacheItem
f815b011c3 [Cache] fix FilesystemTagAwareAdapter failing when a tag link preexists
* 4.4:
[HttpFoundation] workaround PHP bug in the session module
[SecurityBundle] fix accepting env vars in remember-me configurations
[Form] Fixed handling groups sequence validation
[Cache] Avoid memory leak in TraceableAdapter::reset()
This PR was merged into the 3.4 branch.
Discussion
----------
[Form] Fixed handling groups sequence validation
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | FIx https://github.com/symfony/symfony/issues/9939#issuecomment-607459505, Fix#35556
| License | MIT
| Doc PR | ~
This is not the same as the original issue fixed by #36245, that was reported in https://github.com/symfony/symfony/issues/9939#issuecomment-607459505.
The form also fails to cascade sequence validation properly because each nested field is validated against the sequence, and one can fail at a step independently from another which could failed in another step. I've added a lot of tests to ensure this is working properly and tested in a website skeleton too.
This PR aims to close#35556 which tries to fix the same issue but afterwards in its implementation as said in https://github.com/symfony/symfony/pull/35556#discussion_r379289230.
Commits
-------
dfb61c204c [Form] Fixed handling groups sequence validation
This PR was merged into the 5.1-dev branch.
Discussion
----------
[RedisMessengerBridge] Add a delete_after_ack option
This allows Messenger to clean up processed messages from memory, avoiding a mem "leak" in redis
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | https://github.com/symfony/symfony/issues/33715
| License | MIT
| Doc PR | symfony/symfony-docs#... TODO - will pile it on to https://github.com/symfony/symfony-docs/pull/11869 as it kinda binds together and a bigger refactor of the docs here is much needed to avoid all these gotchas
Right now by default a redis transport for messenger will leak memory as all messages stay in redis forever. You can configure `stream_max_entries` to automatically trim to a max of X entries, but that means if you have big peaks in messages you might start losing messages which have not been processed.
This PR provides an alternative to that, by deleting message as they are processed. This is ideal as it avoids having to find the right number for `stream_max_entries` (do you want to risk losing data or use more memory than needed on average?). The only catch is that if you have multiple groups consuming the same stream, the first one processing a message will delete it, so other groups will not see it. For that reason `setup()` attempts to detect this and fails hard if it is misconfigured to prevent data loss.
Commits
-------
7c416a7173 [RedisMessengerBridge] Add a delete_after_ack option to automatically clean up processed messages from memory
This PR was squashed before being merged into the 5.1-dev branch.
Discussion
----------
[Messenger] Add FIFO support to the SQS transport
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets | no
| License | MIT
| Doc PR | --
https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/FIFO-queues.html
Commits
-------
37601753f1 [Messenger] Add FIFO support to the SQS transport
This PR was squashed before being merged into the 5.1-dev branch.
Discussion
----------
[Cache] Added context to log messages
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR | n/a
In my application logs, I've got many entries like:
> Failed to save key "foobar" of type string.
I know it is related to the cache, But I dont know from what adapter. I use a chain of Array, Apcu and Redis. This PR adds some context to that log entry so I know which one of my cache adapter that fails.
Commits
-------
a4d9e0fc94 [Cache] Added context to log messages
This PR was merged into the 5.1-dev branch.
Discussion
----------
Use ExpectDeprecationTrait
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| Deprecations? | no
| Tickets | -
| License | MIT
| Doc PR | -
Commits
-------
08febef500 Use ExpectDeprecationTrait
This PR was merged into the 3.4 branch.
Discussion
----------
[Cache] Avoid memory leak in TraceableAdapter::reset()
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets |
| License | MIT
| Doc PR |
When we call `ServicesResetter::reset()`, we want to reset the
application to its initial states. We don't want a memory leak :p
Commits
-------
15a8610c0c [Cache] Avoid memory leak in TraceableAdapter::reset()
* 5.0:
[PhpUnitBridge] add PolyfillTestCaseTrait::expectExceptionMessageMatches to provide FC with recent phpunit versions
[Messenger] Make sure redis transports are initialized correctly
Remove return type for Twig function workflow_metadata()
[DI] fix typo
* 4.4:
[PhpUnitBridge] add PolyfillTestCaseTrait::expectExceptionMessageMatches to provide FC with recent phpunit versions
[Messenger] Make sure redis transports are initialized correctly
Remove return type for Twig function workflow_metadata()
[DI] fix typo
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[PhpUnitBridge] add PolyfillTestCaseTrait::expectExceptionMessageMatches to provide FC with recent phpunit versions
| 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 | na
| License | MIT
| Doc PR | na
expectExceptionMessageRegExp is deprecated coming phpunit 8.5.3 see https://github.com/sebastianbergmann/phpunit/issues/4133
Not sure if I need to add something else lmk.
Commits
-------
cfd5a29eaf [PhpUnitBridge] add PolyfillTestCaseTrait::expectExceptionMessageMatches to provide FC with recent phpunit versions
This PR was merged into the 5.1-dev branch.
Discussion
----------
[HttpFoundation] Add InputBag
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | yes
| License | MIT
When ppl read a request attribute, they never check if an array is returned
This means many apps just fail with a 500 when adding `[]` in the query string.
This PR turns them to 400 basically (with a deprecation for now)
Commits
-------
0a2ef70c04 [HttpFoundation] add InputBag
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Mailer][Messenger] add return statement for MessageHandler
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | yes
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets |
| License | MIT
| Doc PR |
By returning the result of the transporter in `MessageHandler` we can get hold of `SentMessage` from `HandledStamp::getResult()`.
![image](https://user-images.githubusercontent.com/4582866/79046122-3bed9100-7c0f-11ea-878f-65a6eb610758.png)
Commits
-------
7854cb488e [Mailer][Messenger] add return statement for MessageHandler
This PR was merged into the 3.4 branch.
Discussion
----------
[Form] RepeatedType should always have inner types mapped
| Q | A
| ------------- | ---
| Branch? | 3.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Doc PR| https://github.com/symfony/symfony-docs/pull/13519 |
| Tickets | Fix#36410
| License | MIT
Always set mapped=true to override inner type mapped setting.
Throw an exception if inner types of RepeatedType has mapped=false
Commits
-------
728cd66a13 RepeatedType should always have inner types mapped