Commit Graph

48750 Commits

Author SHA1 Message Date
Thomas Calvet
3e80e461a9 [DependencyInjection] Add a mechanism to deprecate public services to private 2020-05-04 15:38:14 +02:00
Nicolas Grekas
cf0d086d14 bug #36678 [WebProfiler] Do not add src-elem CSP directives if they do not exist (ndench)
This PR was merged into the 3.4 branch.

Discussion
----------

[WebProfiler] Do not add src-elem CSP directives if they do not exist

| Q             | A
| ------------- | ---
| Branch?       | 3.4, 4.4, 5.0
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #36643
| License       | MIT
| Doc PR        | n/a

In the latest 3.4.*, 4.4.* and 5.0.* branches the `script-src-elem` and `style-src-elem` directives are added to the Content-Security-Policy header if they don't exist by copying the `default-src`. This causes browsers to ignore the `script-src` and `style-src` directives which likely contain scripts and styles the developer wanted to allow.

As mentioned in the fixed ticket, we shouldn't be adding these directives if they don't exist because the browser will automatically fallback to `script-src` and `style-src` which we have already added `unsafe-inlen` and the `nonce-*` to.

This will need to be merged into 3.4, 4.4 and 5.0, but I was unsure which branch I am meant to base it off to start with. I've put it on 4.4 but can move it to another if required.

Commits
-------

d9c47087c9 [WebProfiler] Do not add src-elem CSP directives if they do not exist
2020-05-04 15:33:45 +02:00
Nicolas Grekas
78a7f4682f bug #36501 [DX] Show the ParseException message in all YAML file loaders (fancyweb)
This PR was merged into the 3.4 branch.

Discussion
----------

[DX] Show the ParseException message in all YAML file loaders

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This PR synchronizes the exception message in the Routing, Validator and Translation YAML file loaders with the DependencyInjection YAML file loader behavior. Adding the ParseException message is a big DX gain because it highlights the problem directly instead of having to scroll down 7 previous exceptions.

I'm targetting 3.4 because DX can be considered as a bug fix AFAIK.

Commits
-------

fc6cf3d3c6 [DX] Show the ParseException message in YAML file loaders
2020-05-04 15:29:28 +02:00
Nathan Dench
d9c47087c9 [WebProfiler] Do not add src-elem CSP directives if they do not exist 2020-05-04 15:18:19 +02:00
Fabien Potencier
bb77914a26 bug #36683 [Yaml] fix parse error when unindented collections contain a comment (wdiesveld)
This PR was squashed before being merged into the 3.4 branch.

Discussion
----------

[Yaml] fix parse error when unindented collections contain a comment

| Q             | A
| ------------- | ---
| Branch?       | 5.0
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #36558
| License       | MIT

### Problem
The method `Parser::getNextEmbedBlock` did not determine the yaml-block correctly when there was a comment before the first unindented collection item. This was caused by the fact that the check for unindented collection items was done for the _first line of the block only_. So in case this first line is a comment, this check will result in _false_, while in fact the parser is in an unindented collection.

### Solution
In the solution I implemented the parser will check for comment lines as well. As long as the loop encounters a comment line, it will check (in the next iteration) whether the line is an unindented collection item. So this check will be done until all comments before the first uncommented item are parsed.

Commits
-------

58bb2c52ac [Yaml] fix parse error when unindented collections contain a comment
2020-05-04 14:50:49 +02:00
Wouter Diesveld
58bb2c52ac [Yaml] fix parse error when unindented collections contain a comment 2020-05-04 14:50:41 +02:00
Fabien Potencier
0a7fa8f35d minor #36647 Execute docker dependent tests with github actions (jakzal)
This PR was merged into the 4.4 branch.

Discussion
----------

Execute docker dependent tests with github actions

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fixes #36427
| License       | MIT
| Doc PR        | -

* redis, memcached, rabbitmq and vulcain dependent tests moved to the github action
* run on PHP 7.1 and 7.4 only
* use the `integration` group for all tests that depend on docker services
* do not exclude the `integration` group on Travis, but make sure tests that depend on docker services are skipped properly

[<img width="1222" alt="image" src="https://user-images.githubusercontent.com/190447/80806323-48339100-8bb2-11ea-95cd-5ce773c74ce6.png">](https://github.com/jakzal/symfony/runs/636461875?check_suite_focus=true)

Commits
-------

d710c1b654 Execute docker dependent tests with github actions
2020-05-04 14:48:54 +02:00
Jakub Zalas
d710c1b654
Execute docker dependent tests with github actions 2020-05-04 12:09:26 +01:00
Nicolas Grekas
1ac5f68810 [FrameworkBundle] use the router context by default for assets 2020-05-04 10:37:33 +02:00
Fabien Potencier
469d82d6e2 bug #36672 [Validator] Skip validation when email is an empty object (acrobat)
This PR was merged into the 3.4 branch.

Discussion
----------

[Validator] Skip validation when email is an empty object

| Q             | A
| ------------- | ---
| Branch?       | 3.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       | <!-- prefix each issue number with "Fix #", if any -->
| License       | MIT
| Doc PR        | <!-- required for new features -->

When the value passed to the email validator is an empty object the validator is still called and will mark the value as invalid. The object should be skipped in this case, as it is also done in the `UrlValidator`

bfdbb244fe/src/Symfony/Component/Validator/Constraints/UrlValidator.php (L59-L62)

<!--
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 master.
-->

Commits
-------

de5d68ef2a Skip validation when email is an empty object
2020-05-04 09:50:33 +02:00
Fabien Potencier
f94d9397e2 minor #36675 Update exception.html.php (bosunski)
This PR was submitted for the master branch but it was merged into the 4.4 branch instead.

Discussion
----------

Update exception.html.php

| Q             | A
| ------------- | ---
| Branch?       | master for features / 3.4, 4.4 or 5.0 for bug fixes <!-- 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       | NIL <!-- prefix each issue number with "Fix #", if any -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- 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 master.
-->

Removed the second use of `count` and replaced with the result of the first call to `count()`.

In other words: the use of `$last = count($exceptionAsArray) - 1;` is rewritten as `$last = $exceptionAsArrayCount - 1;` to use the result of the last count.

Commits
-------

1c9162d2ad Update exception.html.php
2020-05-04 09:47:26 +02:00
Olatunbosun Egberinde
1c9162d2ad Update exception.html.php 2020-05-04 09:47:19 +02:00
Fabien Potencier
106408ba83 minor #36639 [Inflector] Fix testPluralize() arguments names (fancyweb)
This PR was merged into the 4.4 branch.

Discussion
----------

[Inflector] Fix testPluralize() arguments names

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This is minor but the testPluralize() method arguments names are inversed ($plural contains the singular, $singular contains the plural, etc.). I lost some time on this because I thought the test was wrong 😅 I guess it's better to have a test that is easier to understand.

Commits
-------

fb42f98315 [Inflector] Fix testPluralize() arguments names
2020-05-04 09:46:09 +02:00
Fabien Potencier
e3d2a50c35 minor #36680 [3.4][Inflector] Improve testSingularize() argument name (fancyweb)
This PR was merged into the 3.4 branch.

Discussion
----------

[3.4][Inflector] Improve testSingularize() argument name

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | https://github.com/symfony/symfony/pull/36639#discussion_r418192900
| License       | MIT
| Doc PR        | -

Follow up of https://github.com/symfony/symfony/pull/36639 on 3.4 to keep both tests methods synchronized.

Commits
-------

75405247be [3.4][Inflector] Improve testSingularize() argument name
2020-05-04 09:44:57 +02:00
Fabien Potencier
e9be7418a3 feature #36651 [FrameworkBundle] Allow configuring the default base URI with a DSN (nicolas-grekas)
This PR was merged into the 5.1-dev branch.

Discussion
----------

[FrameworkBundle] Allow configuring the default base URI with a DSN

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fixes #35121, replaces #35580, partially reverts #35281
| License       | MIT
| Doc PR        | -

Instead of defining 3-4 parameters, this PR enables using a single DSN to configure the default URL context (for commands mainly):
```
framework:
	router:
		base_uri: 'https://my.host:8443/base-path/'
```

When using parameters directly, one can now set the same absolute URI in the `router.request_context.base_url` parameter, this will provide the same benefit.

Commits
-------

250fa7e979 [FrameworkBundle] Allow configuring the default base URI with a DSN
2020-05-04 09:42:27 +02:00
Nicolas Grekas
250fa7e979 [FrameworkBundle] Allow configuring the default base URI with a DSN 2020-05-04 09:14:05 +02:00
Thomas Calvet
75405247be [3.4][Inflector] Improve testSingularize() argument name 2020-05-04 09:08:14 +02:00
Thomas Calvet
fb42f98315 [Inflector] Fix testPluralize() arguments names 2020-05-04 09:00:39 +02:00
Nicolas Grekas
119b81035b bug #36673 [PhpUnitBridge] fix PHP 5.3 compat again (nicolas-grekas)
This PR was merged into the 3.4 branch.

Discussion
----------

[PhpUnitBridge] fix PHP 5.3 compat again

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Commits
-------

065a8cee5f [PhpUnitBridge] fix PHP 5.3 compat again
2020-05-03 23:54:48 +02:00
Nicolas Grekas
065a8cee5f [PhpUnitBridge] fix PHP 5.3 compat again 2020-05-03 23:44:38 +02:00
Jeroen Thora
de5d68ef2a
Skip validation when email is an empty object 2020-05-03 21:36:20 +02:00
Fabien Potencier
09645a9103 feature #36600 [Security] Added LDAP support to Authenticator system (wouterj)
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
2020-05-03 19:24:55 +02:00
Fabien Potencier
956d547078 feature #35453 [Messenger] Add option to stop the worker after a message failed (micheh)
This PR was squashed before being merged into the 5.1-dev branch.

Discussion
----------

[Messenger] Add option to stop the worker after a message failed

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #34414
| License       | MIT
| Doc PR        |

This pull request adds a new option to the `messenger:consume` command, to stop the worker after a specified amount of failed messages was handled by the worker.

Commits
-------

ea79206470 [Messenger] Add option to stop the worker after a message failed
2020-05-03 19:22:48 +02:00
Michel Hunziker
ea79206470 [Messenger] Add option to stop the worker after a message failed 2020-05-03 19:22:38 +02:00
Fabien Potencier
362c5d4700 minor #36670 [Translations] Throw exception if xFileLoader dependencies don't exist. (jrushlow)
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.
2020-05-03 19:18:39 +02:00
Jesse Rushlow
627e476eb4 [Translations] Throw exception if xFileLoader dependencies don't exist. 2020-05-03 19:18:32 +02:00
Wouter de Jong
20962e604a [Security] Added LDAP support to Authenticator system 2020-05-03 12:56:20 -04:00
Fabien Potencier
017420b511 feature #36094 [AmazonSqsMessenger] Use AsyncAws to handle SQS communication (jderusse)
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
2020-05-03 18:22:12 +02:00
Jérémy Derussé
7c4888eed1 [AmazonSqsMessenger] Use AsyncAws to handle SQS communication 2020-05-03 18:22:01 +02:00
Fabien Potencier
87a570185e bug #36660 [Messenger] Fix messenger:failed:remove can not remove single message (tienvx)
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
2020-05-03 17:44:00 +02:00
Fabien Potencier
eebb3efa2c feature #36636 Add support of PHP8 static return type for withers (l-vo)
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
2020-05-03 16:47:09 +02:00
Fabien Potencier
d6aa205b47 feature #36586 [DI] allow loading and dumping tags with an attribute named "name" (nicolas-grekas)
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"
2020-05-03 16:42:23 +02:00
Fabien Potencier
07a0334947 feature #36599 [HttpKernel] make kernels implementing WarmableInterface be part of the cache warmup stage (nicolas-grekas)
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
2020-05-03 16:39:00 +02:00
Fabien Potencier
3a6f8ca3e4 bug #36578 [Form] deprecate not using a rounding mode (xabbuh)
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
2020-05-03 16:35:45 +02:00
Fabien Potencier
669b7f132b feature #35992 [Mailer] Use AsyncAws to handle SES requests (jderusse)
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
2020-05-03 16:23:47 +02:00
Jérémy Derussé
21243874bc [Mailer] Use AsyncAws to handle SES requests 2020-05-03 16:23:41 +02:00
Fabien Potencier
a5ae434a92 bug #36505 [Translation] Fix for translation:update command updating ICU messages (artemoliynyk)
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
2020-05-03 10:46:12 +02:00
Fabien Potencier
09f90792e7 feature #36574 [Security] Removed anonymous in the new security system (wouterj)
This PR was merged into the 5.1-dev branch.

Discussion
----------

[Security] Removed anonymous in the new security system

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | tbd

This was one of the "Future considerations" of #33558:

> 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

This new experimental system is probably a once-in-a-lifetime change to make this change. @weaverryan and I have had some brainstorming about this. Some reasons why we think it makes 100% sense to do this change:

* From a Security perspective, **a user that is not authenticated is similar to an "unknown" user**: They both have no rights at all.
* **The higher level consequences of the AnonymousToken are confusing and inconsistent**:
  * It's hard to explain people new to Symfony Security that not being logged in still means you're authenticated within the Symfony app
  * To counter this, some higher level APIs explicitly mark anonymous tokens as not being authenticated, see e.g. the [`is_authenticated()` expression language function](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Core/Authorization/ExpressionLanguageProvider.php#L33-L37)
  * The anonymous authentication resulted in the `IS_AUTHENTICATED` security attribute being removed from #35854, as there was no clear consensus on what its meaning should be
* **Spring Security, which is where this originated from, makes Anonymous a very special case**:

  > Finally, there is an AnonymousAuthenticationFilter, which is chained after the normal authentication mechanisms and automatically adds an AnonymousAuthenticationToken to the SecurityContextHolder if there is no existing Authentication held there.
  >
  > Note that there is no real conceptual difference between a user who is “anonymously authenticated” and an unauthenticated user. Spring Security's anonymous authentication just gives you a more convenient way to configure your access-control attributes. Calls to servlet API calls such as getCallerPrincipal, for example, will still return null even though there is actually an anonymous authentication object in the SecurityContextHolder.
* Symfony uses AnonymousToken much more than "just for convience in access-control attributes". **Removing anonymous tokens allows us to move towards only allowing `UserInterface` users**: #34909

---

Removing anonymous tokens do have an impact on `AccessListener` and `AuthorizationChecker`. These currently throw an exception if there is no token in the storage, instead of treating them like "unknown users" (i.e. no roles). See #30609 on a RFC about removing this exception. We can also see e.g. the [Twig `is_granted()` function explicitly catching this exception](https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Twig/Extension/SecurityExtension.php#L37-L52).

* **To make the changes in `AccessListener` and `AuthorizationChecker` BC, a flag has been added - default enabled - to throw an exception when no token is present** (which is automatically disabled when the new system is used). In Symfony 5.4 (or whenever the new system is no longer experimental), we can deprecate this flag and in 6.0 we can never throw the exception anymore.
* **`anonymous: lazy` has been deprecated in favor of `{ anonymous: true, lazy: true }`** This fixes the dependency on `AnonymousFactory` from the `SecurityExtension` and allows removing the `anonymous` option.
* **Introduced `PUBLIC_ACCESS` Security attribute** as alternative of `IS_AUTHENTICATED_ANONYMOUSLY`. Both work in the new system, the latter only triggers a deprecation notice (but may be usefull to allow switching back and forth between old and new system).

cc @javiereguiluz you might be interested, as I recently talked with you about this topic

Commits
-------

ac84a6c5d9 Removed AnonymousToken from the authenticator system
2020-05-03 08:43:51 +02:00
Wouter de Jong
ac84a6c5d9 Removed AnonymousToken from the authenticator system
* 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"
2020-05-03 08:43:40 +02:00
Fabien Potencier
28bb74cd50 bug #36661 [SecurityBundle] Fixed entry point service ID resolving and multiple guard entry points (wouterj)
This PR was squashed before being merged into the 5.1-dev branch.

Discussion
----------

[SecurityBundle] Fixed entry point service ID resolving and multiple guard entry points

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | n/a

@fabpot I am not able to reproduce [the error you reported](https://github.com/symfony/symfony/pull/36575#issuecomment-622272051) in any of my demo applications or in the tests introduced in this PR. The error indicates that no entry point is configured in your application, can you maybe try out this patch (given it now makes a hard error when more than one guard is used)? If it still doesn't work, can you maybe share your firewall configuration?

---

_build failures are unrelated_

Commits
-------

c75659350e Do not make AbstractFactory internal and revert method rename
6870a18803 Fixed entry point resolving and guard entry point configuration
2020-05-03 08:36:54 +02:00
Fabien Potencier
1308dd5553 feature #36666 [Security] Renamed VerifyAuthenticatorCredentialsEvent to CheckPassportEvent (wouterj)
This PR was merged into the 5.1-dev branch.

Discussion
----------

[Security] Renamed VerifyAuthenticatorCredentialsEvent to CheckPassportEvent

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #36608
| License       | MIT
| Doc PR        | -

This event was named long before we introduced the concept of passports. Listeners on this event check the user, the credentials and any badges of the Security passport. I think `CheckPassportEvent` makes the most sense (more than `CheckCredentialsEvent`).

Also, I managed to break fabbot in the large PR. Just checked all new classes and added license headers in case they were missing (fabbot complained about most of them in this PR already).

Commits
-------

5ba4d1de86 Renamed VerifyAuthenticatorCredentialsEvent to CheckPassportEvent
2020-05-03 08:34:00 +02:00
Fabien Potencier
bfdbb244fe minor #36667 fix sr_Latn translation (kaznovac)
This PR was merged into the 3.4 branch.

Discussion
----------

fix sr_Latn translation

*negative* translated as positive

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

Commits
-------

0da177a224 fix sr_Latn translation
2020-05-03 08:32:27 +02:00
Marko Kaznovac
0da177a224
fix sr_Latn translation
*negative* translated as positive
2020-05-03 00:06:24 +02:00
Wouter de Jong
c75659350e Do not make AbstractFactory internal and revert method rename 2020-05-02 20:58:17 +02:00
Wouter de Jong
5ba4d1de86 Renamed VerifyAuthenticatorCredentialsEvent to CheckPassportEvent 2020-05-02 20:51:30 +02:00
Wouter de Jong
6870a18803 Fixed entry point resolving and guard entry point configuration 2020-05-02 15:04:30 +02:00
Christian Flothmann
aee10cd44a bug #36627 [Validator] fix lazy property usage. (bendavies)
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.
2020-05-02 08:43:10 +02:00
Ben Davies
281861e788 [Validator] fix lazy property usage. 2020-05-02 08:43:03 +02:00
Christian Flothmann
d765a0999d minor #36654 Fix Form annotation (VincentLanglet)
This PR was merged into the 3.4 branch.

Discussion
----------

Fix Form annotation

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

Symfony form component provide his own exception `OutOfBoundsException` which is implementing `ExceptionInterface`. Form are returning this custom `OutOfBoundsException`, but the phpDoc say it's an original `\OutOfBoundsException`, which is not extending the `ExceptionInterface`.

This can provide issue with static analysis tools.

Commits
-------

67b744929f Fix annotation
2020-05-02 08:35:59 +02:00
Christian Flothmann
a804333b25 minor #36613 [Form] provide a useful message when extension types don't match (xabbuh)
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
2020-05-02 08:23:09 +02:00