[Security] Deprecate remaining anonymous checks #42510
Conversation
chalasr
changed the title
chalasr
changed the title
wouterj
force-pushed
the
pull-41613/anon
branch
from
273a924 to
e77f4c8
Compare
Nyholm
left a comment
Nyholm
left a comment
There was a problem hiding this comment.
Thank you. Well done
After first review, It looks good. Just waiting for it to remove WIP.
src/Symfony/Component/Security/Core/Authentication/AuthenticationTrustResolverInterface.php
Outdated
Show resolved
Hide resolved
|
Thanks for the reviews @Nyholm!
The WIP is just because of the intentionally broken tests. Everything except |
|
The most common usage of firewalls:
main:
pattern: ^/
form_login:
login_path: login
check_path: login
access_control:
- { path: ^/login, roles: IS_AUTHENTICATED_ANONYMOUSLY }
- { path: ^/another_public_resource, roles: IS_AUTHENTICATED_ANONYMOUSLY }
- { path: ^/, roles: IS_AUTHENTICATED_FULLY }What is the new way to achieve that? How should one allow a single route to be accessed without being authenticated? |
|
#42423 has been merged now. |
wouterj
force-pushed
the
pull-41613/anon
branch
2 times, most recently
from
d6de7d5 to
ab23e6f
Compare
|
Good idea @chalasr. The great thing: this made me realize that I was suggesting a wrong replacement. I've added your example to the UPGRADE guide. Meanwhile, I also fixed the tests and updated the I think I have to disagree with fabbot here, their suggestion doesn't make sense: (I know we like to use the pixels at the right side of the screen, but I'm going to need a much much wider screen to be able to read this line 😄 ) diff -ru src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php
--- src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php 2021-08-14 09:17:15.170647398 +0000
+++ src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php 2021-08-14 09:17:18.381551885 +0000
@@ -416,11 +416,9 @@
$tokenStorage = new TokenStorage();
$usageIndex = $session->getUsageIndex();
- $tokenStorage = new UsageTrackingTokenStorage($tokenStorage, new class(
- (new \ReflectionClass(UsageTrackingTokenStorage::class))->hasMethod('getSession') ? [
- 'request_stack' => function () use ($requestStack) {
- return $requestStack;
- }] : [
+ $tokenStorage = new UsageTrackingTokenStorage($tokenStorage, new class((new \ReflectionClass(UsageTrackingTokenStorage::class))->hasMethod('getSession') ? ['request_stack' => function () use ($requestStack) {
+ return $requestStack;
+ }] : [
// BC for symfony/framework-bundle < 5.3
'session' => function () use ($session) {
return $session; |
wouterj
force-pushed
the
pull-41613/anon
branch
from
ab23e6f to
e3aca7f
Compare
wouterj
changed the title
|
Thank you @wouterj. |
…:isAuthenticated()` non-virtual (chalasr) This PR was merged into the 6.0 branch. Discussion ---------- [Security] Make `AuthenticationTrustResolverInterface::isAuthenticated()` non-virtual | Q | A | ------------- | --- | Branch? | 6.0 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - The method has been added in #42510 (5.4) as a replacement for `isAnonymous()`. Commits ------- 32952ca [Security] Make `AuthenticationTrustResolverInterface::isAuthenticated()` non-virtual
5 participants
Deprecates the remaining checks for anonymous found in #41613. It's WIP because the tests are failing until #42423 is merged and this PR is rebased (didn't update one test to avoid merge conflicts).
Besides this, it also introduced
IS_AUTHENTICATEDandAuthenticationTrustResolver::isAutenticated(). Previously,IS_AUTHENTICATED_ANONYMOUSLYwas considered to be the "bottom type" for authenticated requests. As this is no longer true,IS_AUTHENTICATED_REMEMBERMEis now the new "bottom type". I suggest we use an explicit bottom type (the ones introduced) instead to avoid another such update if we change something with remember me. It's also more clear on the exact intent of the check.