No more support for custom anon/remember tokens based on FQCN #26981

linaori · 2018-04-19T10:59:58Z

Q	A
Branch?	master
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	yes
Tests pass?	yes
Fixed tickets	#26940
License	MIT
Doc PR	~

This PR deprecates the ability to configure a custom anonymous and remember me token class, via the AuthenticationTrustResolver. The only change required if you have changed the token classes like this, is to extend the Anonymous/RememberMe token classes.

javiereguiluz · 2018-04-19T11:18:39Z

src/Symfony/Component/Security/Core/Authentication/AuthenticationTrustResolver.php

-        return $token instanceof $this->anonymousClass;
+        if (null !== $this->anonymousClass) {
+            if ($isAnonymous = ($token instanceof $this->anonymousClass)) {
+                @trigger_error(sprintf('Configuring a custom anonymous token class is deprecated as of 4.2 and will not be possible in 5.0. To fix this, please extend the %s instead.', AnonymousToken::class), E_USER_DEPRECATED);


...extend the %s instead -> ...extend the %s class instead ? (in other messages too)

javiereguiluz · 2018-04-19T11:22:14Z

src/Symfony/Component/Security/Core/Authentication/AuthenticationTrustResolver.php

-        return $token instanceof $this->anonymousClass;
+        if (null !== $this->anonymousClass) {
+            if ($isAnonymous = ($token instanceof $this->anonymousClass)) {
+                @trigger_error(sprintf('Configuring a custom anonymous token class is deprecated as of 4.2 and will not be possible in 5.0. To fix this, please extend the %s instead.', AnonymousToken::class), E_USER_DEPRECATED);


In the first occurrence of a Symfony version we add Symfony to not mistake this deprecation message with any other bundle. The format would be:

Configuring a custom anonymous token class is deprecated since Symfony 4.1 and will throw an exception in 5.0. To fix this, extend the %s class instead.

Also, the "...please..." is not necessary. We usually don't put "please" in this kind of messages.

Taluu · 2018-04-21T23:17:22Z

src/Symfony/Component/Security/Core/Authentication/AuthenticationTrustResolver.php

-        return $token instanceof $this->anonymousClass;
+        if (null !== $this->anonymousClass) {
+            if (($isAnonymous = ($token instanceof $this->anonymousClass)) && !$token instanceof AnonymousToken) {
+                @trigger_error(sprintf('Configuring a custom anonymous token class is deprecated since Symfony 4.2 and will not be possible in 5.0. To fix this, have the %s class extend the %s class instead.', \get_class($token), AnonymousToken::class), E_USER_DEPRECATED);


how about making this check in the constructor with a is_a or somehting like that ?

You mean checking the type of the string passed into the constructor? I think that's an option

Yeah, maybe we should have 2 deprecations: trigger one when you pass a custom class in the constructor; and trigger another when you pass a token that doesn't extend AnonymousToken/RememberMeToken.
Early triggering of deprecations makes the upgrade path probably easier.

Yeah @iltar I meant checking the type in the constructor. Two deprecations as suggested by @wouterj looks good too.

linaori · 2018-04-23T06:28:35Z

I've moved the deprecation to the constructor, I don't think it's needed runtime as it's the same check.

nicolas-grekas · 2018-05-21T18:21:41Z

rebase needed

linaori · 2018-05-22T06:51:24Z

done

fabpot · 2018-05-22T07:05:23Z

UPGRADE-4.2.md

@@ -5,3 +5,15 @@ Security
 --------

 * Using the `has_role()` function in security expressions is deprecated, use the `is_granted()` function instead.
+ * Passing custom class names for to the 


Looks like for should be removed here.

fabpot · 2018-05-22T07:05:36Z

UPGRADE-4.2.md

@@ -5,3 +5,15 @@ Security
 --------

 * Using the `has_role()` function in security expressions is deprecated, use the `is_granted()` function instead.
+ * Passing custom class names for to the 
+   `Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver` to define
+   custom anonymouse and remember me token classes, is deprecated as of 4.2 and will be


typo anonymous

fabpot · 2018-05-22T07:06:12Z

UPGRADE-4.2.md

+ * Passing custom class names for to the 
+   `Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver` to define
+   custom anonymouse and remember me token classes, is deprecated as of 4.2 and will be
+   removed in 5.0. To use custom tokens extend the existing AnonymousToken and RememberMeToken.


The "and will be removed in 5.0" should be removed (same in the other paragraph). We decided to remove them all in deprecation as this should be "obvious".

fabpot · 2018-05-22T07:06:27Z

UPGRADE-4.2.md

+ * Passing custom class names for to the 
+   `Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver` to define
+   custom anonymouse and remember me token classes, is deprecated as of 4.2 and will be
+   removed in 5.0. To use custom tokens extend the existing AnonymousToken and RememberMeToken.


Can you use the FQCN here instead?

fabpot · 2018-05-22T07:06:52Z

src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

+
+ * Using the `security.authentication.trust_resolver.anonymous_class` and 
+   `security.authentication.trust_resolver.rememberme_class` parameters to define
+   the token classes, is deprecated as of 4.2 and will be removed in 5.0. To use


and will be removed in 5.0 to be removed: )

fabpot · 2018-05-22T07:07:02Z

src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

+ * Using the `security.authentication.trust_resolver.anonymous_class` and 
+   `security.authentication.trust_resolver.rememberme_class` parameters to define
+   the token classes, is deprecated as of 4.2 and will be removed in 5.0. To use
+   custom tokens extend the existing AnonymousToken and RememberMeToken.


fabpot · 2018-05-22T07:07:26Z

src/Symfony/Component/Security/CHANGELOG.md

+* Passing custom class names for to the 
+  `Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver` to define
+  custom anonymouse and remember me token classes, is deprecated as of 4.2 and will be
+  removed in 5.0. To use custom tokens extend the existing AnonymousToken


missing comma after tokens and missing end dot.

fabpot · 2018-05-22T07:07:45Z

src/Symfony/Component/Security/Core/Authentication/AuthenticationTrustResolver.php

    {
        $this->anonymousClass = $anonymousClass;
        $this->rememberMeClass = $rememberMeClass;
+
+        if (null !== $anonymousClass && !is_a($anonymousClass, AnonymousToken::class, true)) {
+            @trigger_error(sprintf('Configuring a custom anonymous token class is deprecated since Symfony 4.2 and will not be possible in 5.0. To fix this, have the %s class extend the %s class instead, and remove the %s constructor argument.', $anonymousClass, AnonymousToken::class, self::class), E_USER_DEPRECATED);


Same here, the "will be removed" part must be, well, removed.

fabpot

Sorry, I missed some more changes during my first review. Hopefully, that's the last round of CS tweaks.

fabpot · 2018-05-22T07:23:14Z

UPGRADE-4.2.md

@@ -5,3 +5,16 @@ Security
 --------

 * Using the `has_role()` function in security expressions is deprecated, use the `is_granted()` function instead.
+ * Passing custom class names to the
+   `Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver` to define
+   custom anonymous and remember me token classes, is deprecated as of 4.2. To


sorry, forget one thing: we don't need the "as of 4.2" here as the file is specifically for 4.2.

fabpot · 2018-05-22T07:23:28Z

UPGRADE-4.2.md

+   `Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver` to define
+   custom anonymous and remember me token classes, is deprecated as of 4.2. To
+   use custom tokens, extend the existing `Symfony\Component\Security\Core\Authentication\Token\AnonymousToken`
+   and `Symfony\Component\Security\Core\Authentication\Token\RememberMeToken`.


as in "and/or", or replace the and by or?

fabpot · 2018-05-22T07:23:48Z

UPGRADE-4.2.md

+
+ * Using the `security.authentication.trust_resolver.anonymous_class` and 
+   `security.authentication.trust_resolver.rememberme_class` parameters to define
+   the token classes, is deprecated as of 4.2. To use


fabpot · 2018-05-22T07:23:57Z

src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

+
+ * Using the `security.authentication.trust_resolver.anonymous_class` and 
+   `security.authentication.trust_resolver.rememberme_class` parameters to define
+   the token classes, is deprecated as of 4.2. To use


fabpot · 2018-05-22T07:24:15Z

src/Symfony/Component/Security/CHANGELOG.md

@@ -6,6 +6,11 @@ CHANGELOG

 * added the `is_granted()` function in security expressions
 * deprecated the `has_role()` function in security expressions, use `is_granted()` instead
+* Passing custom class names to the
+  `Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver` to define
+  custom anonymous and remember me token classes, is deprecated as of 4.2. To


same here as the section if for 4.2 anyway.

fabpot · 2018-05-22T07:25:17Z

src/Symfony/Component/Security/Core/Authentication/AuthenticationTrustResolver.php

    {
        $this->anonymousClass = $anonymousClass;
        $this->rememberMeClass = $rememberMeClass;
+
+        if (null !== $anonymousClass && !is_a($anonymousClass, AnonymousToken::class, true)) {
+            @trigger_error(sprintf('Configuring a custom anonymous token class is deprecated since Symfony 4.2. To fix this, have the %s class extend the %s class instead, and remove the %s constructor argument.', $anonymousClass, AnonymousToken::class, self::class), E_USER_DEPRECATED);


We try to have only one sentence for deprecations: Configuring a custom anonymous token class is deprecated since Symfony 4.2; have the "%s" class extend the "%s" class instead, and remove the "%s" constructor argument. (note the quotes around class names and parameters).

That makes sense, I'll update it 👍

Noticed that the first argument of the sprintf was the wrong one of for the remember me as well, it received the $anonymousClass instead (fixed)

fabpot · 2018-05-27T07:22:02Z

Thank you @iltar.

…on FQCN (Iltar van der Berg) This PR was squashed before being merged into the 4.2-dev branch (closes #26981). Discussion ---------- No more support for custom anon/remember tokens based on FQCN | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #26940 | License | MIT | Doc PR | ~ This PR deprecates the ability to configure a custom anonymous and remember me token class, via the AuthenticationTrustResolver. The only change required _if_ you have changed the token classes like this, is to extend the Anonymous/RememberMe token classes. Commits ------- 860d454 No more support for custom anon/remember tokens based on FQCN

This PR was squashed before being merged into the 5.0-dev branch (closes #32160). Discussion ---------- Removed legacy code and cleanup | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - See symfony/symfony#32074, symfony/symfony#26981 labels: `HttpFoundation`, `Form`, `Security`, `SecurityBundle`, `Validator` Commits ------- 7b99fb45bb Removed legacy code and cleanup

This PR was squashed before being merged into the 5.0-dev branch (closes #32160). Discussion ---------- Removed legacy code and cleanup | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - See #32074, #26981 labels: `HttpFoundation`, `Form`, `Security`, `SecurityBundle`, `Validator` Commits ------- 7b99fb4 Removed legacy code and cleanup

…esolver (jzawadzki) This PR was submitted for the 5.0 branch but it was merged into the 4.3 branch instead (closes #12721). Discussion ---------- Removed constructor parameters from AuthenticationTrustResolver As per symfony/symfony#26981 there are no constructor arguments in `AuthenticationTrustResolver`. Commits ------- f046117 Removed constructor parameters from AuthenticationTrustResolver

carsonbot added Status: Needs Review Deprecation labels Apr 19, 2018

javiereguiluz added the Security label Apr 19, 2018

javiereguiluz added this to the 4.2 milestone Apr 19, 2018

javiereguiluz reviewed Apr 19, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Apr 21, 2018

wouterj approved these changes Apr 21, 2018

View reviewed changes

Taluu approved these changes Apr 21, 2018

View reviewed changes

aschempp approved these changes Apr 22, 2018

View reviewed changes

fabpot requested changes May 22, 2018

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels May 22, 2018

fabpot requested changes May 22, 2018

View reviewed changes

carsonbot removed the Status: Needs Work label May 22, 2018

fabpot approved these changes May 22, 2018

View reviewed changes

carsonbot added the Status: Reviewed label May 22, 2018

chalasr approved these changes May 27, 2018

View reviewed changes

No more support for custom anon/remember tokens based on FQCN

860d454

fabpot merged commit 860d454 into symfony:master May 27, 2018

nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018

This was referenced Nov 3, 2018

Release v4.2.0-BETA1 #29072

Closed

Release v4.2.0-BETA1 #29073

Merged

linaori deleted the feature/trust-resolver-tokens branch February 8, 2019 13:37

yceruto mentioned this pull request Jun 24, 2019

Removed legacy code and cleanup #32160

Merged

jzawadzki mentioned this pull request Nov 27, 2019

Removed constructor parameters from AuthenticationTrustResolver symfony/symfony-docs#12721

Merged

wouterj mentioned this pull request Feb 25, 2020

[Security] Deprecated AuthenticationTrustResolver #35860

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

No more support for custom anon/remember tokens based on FQCN #26981

No more support for custom anon/remember tokens based on FQCN #26981

linaori commented Apr 19, 2018

javiereguiluz Apr 19, 2018

javiereguiluz Apr 19, 2018

Taluu Apr 21, 2018

linaori Apr 22, 2018

wouterj Apr 22, 2018

Taluu Apr 22, 2018

linaori commented Apr 23, 2018

nicolas-grekas commented May 21, 2018

linaori commented May 22, 2018

fabpot May 22, 2018

fabpot May 22, 2018

fabpot May 22, 2018

fabpot May 22, 2018

fabpot May 22, 2018

fabpot May 22, 2018

fabpot May 22, 2018

fabpot May 22, 2018

fabpot left a comment

fabpot May 22, 2018

fabpot May 22, 2018

linaori May 22, 2018

fabpot May 22, 2018

fabpot May 22, 2018

fabpot May 22, 2018

fabpot May 22, 2018

linaori May 22, 2018

linaori May 22, 2018

fabpot commented May 27, 2018

No more support for custom anon/remember tokens based on FQCN #26981

No more support for custom anon/remember tokens based on FQCN #26981

Conversation

linaori commented Apr 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linaori commented Apr 23, 2018

nicolas-grekas commented May 21, 2018

linaori commented May 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented May 27, 2018