From 0cfaf60c0666e8967590d0dada80caa7f9b61b02 Mon Sep 17 00:00:00 2001 From: Lynn Date: Fri, 20 Dec 2019 16:07:22 +0100 Subject: [PATCH] Use supportsClass where possible --- Core/Tests/User/ChainUserProviderTest.php | 61 ++++++++++++++++++++- Core/User/ChainUserProvider.php | 4 ++ Http/Firewall/ContextListener.php | 7 ++- Http/Tests/Firewall/ContextListenerTest.php | 28 +++++++--- 4 files changed, 88 insertions(+), 12 deletions(-) diff --git a/Core/Tests/User/ChainUserProviderTest.php b/Core/Tests/User/ChainUserProviderTest.php index 1592bcd2f..cf4909dfe 100644 --- a/Core/Tests/User/ChainUserProviderTest.php +++ b/Core/Tests/User/ChainUserProviderTest.php @@ -68,24 +68,49 @@ public function testRefreshUser() $provider1 = $this->getProvider(); $provider1 ->expects($this->once()) - ->method('refreshUser') - ->willThrowException(new UnsupportedUserException('unsupported')) + ->method('supportsClass') + ->willReturn(false) ; $provider2 = $this->getProvider(); + $provider2 + ->expects($this->once()) + ->method('supportsClass') + ->willReturn(true) + ; + $provider2 + ->expects($this->once()) + ->method('refreshUser') + ->willThrowException(new UnsupportedUserException('unsupported')) + ; + + $provider3 = $this->getProvider(); + $provider3 + ->expects($this->once()) + ->method('supportsClass') + ->willReturn(true) + ; + + $provider3 ->expects($this->once()) ->method('refreshUser') ->willReturn($account = $this->getAccount()) ; - $provider = new ChainUserProvider([$provider1, $provider2]); + $provider = new ChainUserProvider([$provider1, $provider2, $provider3]); $this->assertSame($account, $provider->refreshUser($this->getAccount())); } public function testRefreshUserAgain() { $provider1 = $this->getProvider(); + $provider1 + ->expects($this->once()) + ->method('supportsClass') + ->willReturn(true) + ; + $provider1 ->expects($this->once()) ->method('refreshUser') @@ -93,6 +118,12 @@ public function testRefreshUserAgain() ; $provider2 = $this->getProvider(); + $provider2 + ->expects($this->once()) + ->method('supportsClass') + ->willReturn(true) + ; + $provider2 ->expects($this->once()) ->method('refreshUser') @@ -107,6 +138,12 @@ public function testRefreshUserThrowsUnsupportedUserException() { $this->expectException('Symfony\Component\Security\Core\Exception\UnsupportedUserException'); $provider1 = $this->getProvider(); + $provider1 + ->expects($this->once()) + ->method('supportsClass') + ->willReturn(true) + ; + $provider1 ->expects($this->once()) ->method('refreshUser') @@ -114,6 +151,12 @@ public function testRefreshUserThrowsUnsupportedUserException() ; $provider2 = $this->getProvider(); + $provider2 + ->expects($this->once()) + ->method('supportsClass') + ->willReturn(true) + ; + $provider2 ->expects($this->once()) ->method('refreshUser') @@ -171,6 +214,12 @@ public function testSupportsClassWhenNotSupported() public function testAcceptsTraversable() { $provider1 = $this->getProvider(); + $provider1 + ->expects($this->once()) + ->method('supportsClass') + ->willReturn(true) + ; + $provider1 ->expects($this->once()) ->method('refreshUser') @@ -178,6 +227,12 @@ public function testAcceptsTraversable() ; $provider2 = $this->getProvider(); + $provider2 + ->expects($this->once()) + ->method('supportsClass') + ->willReturn(true) + ; + $provider2 ->expects($this->once()) ->method('refreshUser') diff --git a/Core/User/ChainUserProvider.php b/Core/User/ChainUserProvider.php index 02ce08464..5ea8150a3 100644 --- a/Core/User/ChainUserProvider.php +++ b/Core/User/ChainUserProvider.php @@ -73,6 +73,10 @@ public function refreshUser(UserInterface $user) foreach ($this->providers as $provider) { try { + if (!$provider->supportsClass(\get_class($user))) { + continue; + } + return $provider->refreshUser($user); } catch (UnsupportedUserException $e) { // try next one diff --git a/Http/Firewall/ContextListener.php b/Http/Firewall/ContextListener.php index ea9f51f92..6a05ee517 100644 --- a/Http/Firewall/ContextListener.php +++ b/Http/Firewall/ContextListener.php @@ -168,12 +168,17 @@ protected function refreshUser(TokenInterface $token) $userNotFoundByProvider = false; $userDeauthenticated = false; + $userClass = \get_class($user); foreach ($this->userProviders as $provider) { if (!$provider instanceof UserProviderInterface) { throw new \InvalidArgumentException(sprintf('User provider "%s" must implement "%s".', \get_class($provider), UserProviderInterface::class)); } + if (!$provider->supportsClass($userClass)) { + continue; + } + try { $refreshedUser = $provider->refreshUser($user); $newToken = clone $token; @@ -233,7 +238,7 @@ protected function refreshUser(TokenInterface $token) return null; } - throw new \RuntimeException(sprintf('There is no user provider for user "%s".', \get_class($user))); + throw new \RuntimeException(sprintf('There is no user provider for user "%s".', $userClass)); } private function safelyUnserialize($serializedToken) diff --git a/Http/Tests/Firewall/ContextListenerTest.php b/Http/Tests/Firewall/ContextListenerTest.php index acab7087c..74c459604 100644 --- a/Http/Tests/Firewall/ContextListenerTest.php +++ b/Http/Tests/Firewall/ContextListenerTest.php @@ -256,7 +256,7 @@ public function testIfTokenIsDeauthenticatedTriggersDeprecations() { $tokenStorage = new TokenStorage(); $refreshedUser = new User('foobar', 'baz'); - $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)]); + $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider($refreshedUser)]); $this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser()); } @@ -265,7 +265,7 @@ public function testIfTokenIsDeauthenticated() { $tokenStorage = new TokenStorage(); $refreshedUser = new User('foobar', 'baz'); - $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)], null, true); + $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider($refreshedUser)], null, true); $this->assertNull($tokenStorage->getToken()); } @@ -287,7 +287,7 @@ public function testRememberMeGetsCanceledIfTokenIsDeauthenticated() $rememberMeServices = $this->createMock(RememberMeServicesInterface::class); $rememberMeServices->expects($this->once())->method('loginFail'); - $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)], null, true, $rememberMeServices); + $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider($refreshedUser)], null, true, $rememberMeServices); $this->assertNull($tokenStorage->getToken()); } @@ -296,7 +296,7 @@ public function testTryAllUserProvidersUntilASupportingUserProviderIsFound() { $tokenStorage = new TokenStorage(); $refreshedUser = new User('foobar', 'baz'); - $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)], $refreshedUser); + $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider($refreshedUser)], $refreshedUser); $this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser()); } @@ -313,7 +313,7 @@ public function testNextSupportingUserProviderIsTriedIfPreviousSupportingUserPro public function testTokenIsSetToNullIfNoUserWasLoadedByTheRegisteredUserProviders() { $tokenStorage = new TokenStorage(); - $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(), new SupportingUserProvider()]); + $this->handleEventWithPreviousSession($tokenStorage, [new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider()]); $this->assertNull($tokenStorage->getToken()); } @@ -321,14 +321,14 @@ public function testTokenIsSetToNullIfNoUserWasLoadedByTheRegisteredUserProvider public function testRuntimeExceptionIsThrownIfNoSupportingUserProviderWasRegistered() { $this->expectException('RuntimeException'); - $this->handleEventWithPreviousSession(new TokenStorage(), [new NotSupportingUserProvider(), new NotSupportingUserProvider()]); + $this->handleEventWithPreviousSession(new TokenStorage(), [new NotSupportingUserProvider(false), new NotSupportingUserProvider(true)]); } public function testAcceptsProvidersAsTraversable() { $tokenStorage = new TokenStorage(); $refreshedUser = new User('foobar', 'baz'); - $this->handleEventWithPreviousSession($tokenStorage, new \ArrayObject([new NotSupportingUserProvider(), new SupportingUserProvider($refreshedUser)]), $refreshedUser); + $this->handleEventWithPreviousSession($tokenStorage, new \ArrayObject([new NotSupportingUserProvider(true), new NotSupportingUserProvider(false), new SupportingUserProvider($refreshedUser)]), $refreshedUser); $this->assertSame($refreshedUser, $tokenStorage->getToken()->getUser()); } @@ -383,6 +383,14 @@ private function handleEventWithPreviousSession(TokenStorageInterface $tokenStor class NotSupportingUserProvider implements UserProviderInterface { + /** @var bool */ + private $throwsUnsupportedException; + + public function __construct($throwsUnsupportedException) + { + $this->throwsUnsupportedException = $throwsUnsupportedException; + } + public function loadUserByUsername($username) { throw new UsernameNotFoundException(); @@ -390,7 +398,11 @@ public function loadUserByUsername($username) public function refreshUser(UserInterface $user) { - throw new UnsupportedUserException(); + if ($this->throwsUnsupportedException) { + throw new UnsupportedUserException(); + } + + return $user; } public function supportsClass($class)