Skip to content

Commit

Permalink
Remove BC layer for Symfony < 5.4
Browse files Browse the repository at this point in the history
  • Loading branch information
stloyd committed Aug 20, 2023
1 parent f676ef3 commit 70c4620
Show file tree
Hide file tree
Showing 15 changed files with 31 additions and 214 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

use HWI\Bundle\OAuthBundle\Security\Http\Authenticator\OAuthAuthenticator;
use HWI\Bundle\OAuthBundle\Security\Http\Firewall\RefreshAccessTokenListener;
use HWI\Bundle\OAuthBundle\Security\Http\Firewall\RefreshAccessTokenListenerOld;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AbstractFactory;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AuthenticatorFactoryInterface;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FirewallListenerFactoryInterface;
Expand Down Expand Up @@ -96,18 +95,9 @@ public function createListeners(ContainerBuilder $container, string $firewallNam
$listenerDef = $container->setDefinition($listenerId, new ChildDefinition('hwi_oauth.context_listener.abstract_token_refresher'));

$listenerDef->addMethodCall('setResourceOwnerMap', [$this->createResourceOwnerMapReference($firewallName)]);

if ($container->hasDefinition($authenticatorId)) {
// new auth manager
$listenerDef
->setClass(RefreshAccessTokenListener::class)
->replaceArgument(0, new Reference($authenticatorId));
} else {
// old auth manager
$listenerDef
->setClass(RefreshAccessTokenListenerOld::class)
->replaceArgument(0, new Reference($providerId));
}
$listenerDef
->setClass(RefreshAccessTokenListener::class)
->replaceArgument(0, new Reference($authenticatorId));

return [$listenerId];
}
Expand Down
7 changes: 2 additions & 5 deletions src/DependencyInjection/Security/Factory/OAuthFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
namespace HWI\Bundle\OAuthBundle\DependencyInjection\Security\Factory;

use HWI\Bundle\OAuthBundle\Security\Core\Authentication\Provider\OAuthProvider;
use HWI\Bundle\OAuthBundle\Security\Http\Firewall\RefreshAccessTokenListenerOld;
use HWI\Bundle\OAuthBundle\Security\Http\Firewall\RefreshAccessTokenListener;
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AbstractFactory;
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
Expand Down Expand Up @@ -110,8 +110,6 @@ protected function createAuthProvider(ContainerBuilder $container, $id, $config,
->addArgument(new Reference('security.token_storage'))
;

$this->createRefreshTokenListeners($container, $providerId, $id);

$this->firewallNames[$id] = true;

return $providerId;
Expand Down Expand Up @@ -186,8 +184,7 @@ protected function createRefreshTokenListeners(ContainerBuilder $container, stri
$listenerDef = $container->setDefinition($listenerId, new ChildDefinition('hwi_oauth.context_listener.abstract_token_refresher'));

$listenerDef
->addMethodCall('setResourceOwnerMap', [$this->createResourceOwnerMapReference($id)])
->setClass(RefreshAccessTokenListenerOld::class)
->setClass(RefreshAccessTokenListener::class)
->replaceArgument(0, new Reference($providerId));
}

Expand Down
17 changes: 1 addition & 16 deletions src/HWIOAuthBundle.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,12 @@
namespace HWI\Bundle\OAuthBundle;

use HWI\Bundle\OAuthBundle\DependencyInjection\CompilerPass\EnableRefreshOAuthTokenListenerCompilerPass;
use HWI\Bundle\OAuthBundle\DependencyInjection\CompilerPass\RefreshOAuthTokenCompilerPass;
use HWI\Bundle\OAuthBundle\DependencyInjection\CompilerPass\ResourceOwnerCompilerPass;
use HWI\Bundle\OAuthBundle\DependencyInjection\Security\Factory\OAuthAuthenticatorFactory;
use HWI\Bundle\OAuthBundle\DependencyInjection\Security\Factory\OAuthFactory;
use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\ExtensionInterface;
use Symfony\Component\HttpKernel\Bundle\Bundle;
use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface;

/**
* @author Geoffrey Bachelet <[email protected]>
Expand All @@ -37,19 +34,7 @@ public function build(ContainerBuilder $container)

/** @var SecurityExtension $extension */
$extension = $container->getExtension('security');

$firewallNames = $this->extension->getFirewallNames();

if (method_exists($extension, 'addAuthenticatorFactory')) {
$extension->addAuthenticatorFactory(new OAuthAuthenticatorFactory($firewallNames));
} elseif (interface_exists(AuthenticationProviderInterface::class)) {
// @phpstan-ignore-next-line Symfony 4.4 BC layer
$extension->addSecurityListenerFactory(new OAuthFactory($firewallNames));
$container->addCompilerPass(new RefreshOAuthTokenCompilerPass());
} else {
// @phpstan-ignore-next-line Symfony < 5.4 BC layer
$extension->addSecurityListenerFactory(new OAuthAuthenticatorFactory($firewallNames));
}
$extension->addAuthenticatorFactory(new OAuthAuthenticatorFactory($this->extension->getFirewallNames()));

$container->addCompilerPass(new ResourceOwnerCompilerPass());
$container->addCompilerPass(new EnableRefreshOAuthTokenListenerCompilerPass());
Expand Down
2 changes: 1 addition & 1 deletion src/OAuth/Response/SensioConnectUserResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ final class SensioConnectUserResponse extends AbstractUserResponse
*/
public function getUserIdentifier(): string
{
/** @var \DOMAttr $attribute */
/** @var \DOMAttr|null $attribute */
$attribute = $this->data->attributes->getNamedItem('id');
if (null === $attribute->value) {
throw new \InvalidArgumentException('User identifier was not found in response.');
Expand Down
38 changes: 7 additions & 31 deletions src/Security/Core/User/EntityUserProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use HWI\Bundle\OAuthBundle\OAuth\Response\UserResponseInterface;
use Symfony\Component\PropertyAccess\PropertyAccess;
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
use Symfony\Component\Security\Core\Exception\UserNotFoundException;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface;
Expand Down Expand Up @@ -57,10 +56,7 @@ public function loadUserByIdentifier(string $identifier): UserInterface
$user = $this->findUser(['username' => $identifier]);

if (!$user) {
$exception = new UserNotFoundException(sprintf("User '%s' not found.", $identifier));
$exception->setUserIdentifier($identifier);

throw $exception;
throw $this->createUserNotFoundException($identifier, sprintf("User '%s' not found.", $username));

Check failure on line 59 in src/Security/Core/User/EntityUserProvider.php

View workflow job for this annotation

GitHub Actions / phpstan

Undefined variable: $username
}

return $user;
Expand All @@ -75,12 +71,7 @@ public function loadUserByIdentifier(string $identifier): UserInterface
*/
public function loadUserByUsername($username)
{
$user = $this->findUser(['username' => $username]);
if (!$user) {
throw $this->createUserNotFoundException($username, sprintf("User '%s' not found.", $username));
}

return $user;
return $this->loadUserByIdentifier($username);
}

public function loadUserByOAuthUserResponse(UserResponseInterface $response): ?UserInterface
Expand Down Expand Up @@ -109,11 +100,10 @@ public function refreshUser(UserInterface $user): ?UserInterface

$userId = $accessor->getValue($user, $identifier);

// @phpstan-ignore-next-line Symfony <5.4 BC layer
$username = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername();
$userIdentifier = $user->getUserIdentifier();

if (null === $user = $this->findUser([$identifier => $userId])) {
throw $this->createUserNotFoundException($username, sprintf('User with ID "%d" could not be reloaded.', $userId));
throw $this->createUserNotFoundException($userIdentifier, sprintf('User with ID "%d" could not be reloaded.', $userId));
}

return $user;
Expand All @@ -133,24 +123,10 @@ private function findUser(array $criteria): ?UserInterface
return $this->repository->findOneBy($criteria);
}

/**
* @return UsernameNotFoundException|UserNotFoundException
*/
private function createUserNotFoundException(string $username, string $message)
private function createUserNotFoundException(string $identifier, string $message): UserNotFoundException
{
if (class_exists(UserNotFoundException::class)) {
$exception = new UserNotFoundException($message);
$exception->setUserIdentifier($username);
} else {
if (!class_exists(UsernameNotFoundException::class)) {
throw new \RuntimeException('Unsupported Symfony version used!');
}

$exception = new UsernameNotFoundException($message);
if (method_exists($exception, 'setUsername')) {
$exception->setUsername($username); // @phpstan-ignore-this-line Symfony <5.4 BC layer
}
}
$exception = new UserNotFoundException($message);
$exception->setUserIdentifier($identifier);

return $exception;
}
Expand Down
5 changes: 1 addition & 4 deletions src/Security/Core/User/OAuthUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ public function eraseCredentials(): bool

public function equals(UserInterface $user): bool
{
// @phpstan-ignore-next-line Symfony <5.4 BC layer
$username = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername();

return $username === $this->username;
return $user->getUserIdentifier() === $this->username;
}
}
5 changes: 1 addition & 4 deletions src/Security/Core/User/OAuthUserProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,7 @@ public function refreshUser(UserInterface $user): UserInterface
throw new UnsupportedUserException(sprintf('Unsupported user class "%s"', \get_class($user)));
}

// @phpstan-ignore-next-line Symfony <5.4 BC layer
$username = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername();

return $this->loadUserByUsername($username);
return $this->loadUserByUsername($user->getUserIdentifier());
}

public function supportsClass($class): bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function __construct(OAuthToken $token, array $badges = [])

$userBadge = class_exists(UserBadge::class)
? new UserBadge(
method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(),
$user->getUserIdentifier(),
static function () use ($user) { return $user; }
)
: $user;
Expand Down
39 changes: 0 additions & 39 deletions src/Security/Http/Firewall/RefreshAccessTokenListenerOld.php

This file was deleted.

6 changes: 3 additions & 3 deletions tests/DependencyInjection/HWIOAuthExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ public function testConfigurationPassValidOAuth2WithPaths(): void
}

/**
* @legacy
* @group legacy
*/
public function testConfigurationPassValidOAuth1WithClass(): void
{
Expand Down Expand Up @@ -312,7 +312,7 @@ public function testConfigurationPassValidOAuth1WithClass(): void
}

/**
* @legacy
* @group legacy
*/
public function testConfigurationPassValidOAuth2WithClassOnly(): void
{
Expand All @@ -333,7 +333,7 @@ public function testConfigurationPassValidOAuth2WithClassOnly(): void
}

/**
* @legacy
* @group legacy
*/
public function testConfigurationPassValidOAuth2WithPathsAndClass(): void
{
Expand Down
4 changes: 2 additions & 2 deletions tests/OAuth/Response/PathUserResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public function testGetUserIdentifier(): void
$this->responseObject->setPaths($paths);
$this->responseObject->setData(json_encode(['id' => 666]));

$this->assertEquals('666', $this->responseObject->getUserIdentifier());
$this->assertEquals('666', $this->responseObject->getUsername());
}

public function testGetUserIdentifierWithoutResponseThrowsException(): void
Expand Down Expand Up @@ -126,7 +126,7 @@ public function testGetUsername(): void
public function testGetUsernameWithoutResponseReturnsNull(): void
{
$this->responseObject->setPaths(['identifier' => 'id']);
$this->assertNull($this->responseObject->getUsername());
$this->assertNull($this->responseObject->getUserIdentifier());
}

public function testGetNickname(): void
Expand Down
9 changes: 0 additions & 9 deletions tests/Security/Core/User/EntityUserProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,6 @@ public function testLoadUserByIdentifierThrowsExceptionWhenUserIsNotFound(): voi
$provider->loadUserByIdentifier('asm89');
}

public function testLoadUserByUsernameThrowsExceptionWhenUserIsNull(): void
{
$this->assertUserNotFoundException();
$this->expectExceptionMessage("User 'asm89' not found.");

$provider = $this->createEntityUserProvider();
$provider->loadUserByUsername('asm89');
}

public function testLoadUserByOAuthUserResponseThrowsExceptionWhenNoPropertyIsConfigured(): void
{
$this->expectException(\RuntimeException::class);
Expand Down
Loading

0 comments on commit 70c4620

Please sign in to comment.