From fee6826137b2ab1ee77ba866efab069a4f1ef604 Mon Sep 17 00:00:00 2001 From: Alexander Menshchikov Date: Sat, 8 Jun 2024 19:16:23 +0300 Subject: [PATCH] Updates + compatibility table --- .github/workflows/audit.yml | 18 +- .php-cs-fixer.php | 7 + README.md | 6 + composer.json | 4 +- phpstan.neon | 2 - phpunit.xml.dist | 60 +- src/Controller/Login.php | 12 +- .../Security/SamlLogoutListener.php | 25 +- src/Idp/IdpResolver.php | 4 +- src/Onelogin/AuthArgumentResolver.php | 10 +- .../Passport/Badge/SamlAttributesBadge.php | 4 +- .../Http/Authenticator/SamlAuthenticator.php | 8 +- src/Security/User/SamlUserFactory.php | 6 +- tests/Controller/LoginTest.php | 91 +- tests/Controller/MetadataTest.php | 4 +- .../Compiler/AuthRegistryCompilerPassTest.php | 4 +- .../EntityManagerCompilerPassTest.php | 4 +- .../DependencyInjection/ConfigurationTest.php | 41 +- .../NbgrpOneloginSamlExtensionTest.php | 4 +- .../Security/Factory/SamlFactoryTest.php | 4 +- .../SamlUserProviderFactoryTest.php | 4 +- .../Security/SamlLogoutListenerTest.php | 77 +- .../User/DeferredUserListenerTest.php | 10 +- .../EventListener/User/UserListenersTest.php | 118 ++- tests/Idp/IdpResolverTest.php | 21 +- tests/Onelogin/AuthArgumentResolverTest.php | 8 +- tests/Onelogin/AuthFactoryTest.php | 4 +- tests/Onelogin/AuthRegistryTest.php | 10 +- .../SamlAuthenticationSuccessHandlerTest.php | 43 +- .../Passport/Badge/DeferredEventBadgeTest.php | 6 +- .../Badge/SamlAttributesBadgeTest.php | 27 +- .../Authenticator/SamlAuthenticatorTest.php | 973 +++++++++--------- .../Authenticator/Token/SamlTokenTest.php | 31 +- tests/Security/User/SamlUserFactoryTest.php | 4 +- tests/Security/User/SamlUserProviderTest.php | 4 +- 35 files changed, 829 insertions(+), 829 deletions(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index 1503539..d08f9d6 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -21,7 +21,7 @@ jobs: uses: actions/checkout@v4 if: github.event_name == 'pull_request' with: - ref: ${{ github.event.pull_request.head.ref }} + ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 0 - name: Setup PHP @@ -38,29 +38,23 @@ jobs: run: echo "DIR=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT - name: Cache dependencies - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ${{ steps.composer-cache.outputs.DIR }} key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }} restore-keys: ${{ runner.os }}-composer- - name: Install dependencies - run: | - echo "::group::composer install" - composer install --no-interaction - echo "::endgroup::" - echo "::group::phpunit install" - vendor/bin/simple-phpunit install - echo "::endgroup::" + run: composer install --no-interaction - name: Auditor - uses: docker://nbgrp/auditor:0.23.1 + uses: docker://nbgrp/auditor:0.26.0 - name: Tests - run: vendor/bin/simple-phpunit + run: vendor/bin/phpunit - name: Codecov - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 with: token: ${{ secrets.CODECOV_TOKEN }} files: ./clover.xml diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php index de45edd..0188ace 100644 --- a/.php-cs-fixer.php +++ b/.php-cs-fixer.php @@ -76,6 +76,13 @@ 'function', ], ], + 'phpdoc_order' => [ + 'order' => [ + 'param', + 'throws', + 'return', + ], + ], 'phpdoc_separation' => false, 'phpdoc_summary' => false, 'phpdoc_to_comment' => false, diff --git a/README.md b/README.md index 1a2cf9c..11408b9 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,12 @@ > use [hslavich/oneloginsaml-bundle](https://github.com/hslavich/OneloginSamlBundle) > which this bundle based on. +### Compatibility +| Branch | Symfony | +|---------|-----------| +| 1.x | Symfony 6 | +| **2.x** | Symfony 7 | + ## Installation ``` diff --git a/composer.json b/composer.json index 6c22fa3..0687b3e 100644 --- a/composer.json +++ b/composer.json @@ -32,8 +32,8 @@ }, "require-dev": { "doctrine/orm": "^2.3 || ^3", - "symfony/event-dispatcher": "^7", - "symfony/phpunit-bridge": "^7" + "phpunit/phpunit": "^11.2", + "symfony/event-dispatcher": "^7" }, "autoload": { "psr-4": { diff --git a/phpstan.neon b/phpstan.neon index f73ea60..fb7a0dd 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -3,8 +3,6 @@ parameters: paths: - src - tests - bootstrapFiles: - - vendor/bin/.phpunit/phpunit/vendor/autoload.php checkMissingIterableValueType: false checkGenericClassInNonGenericObjectType: false diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 85d6a0a..93b0f16 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,39 +1,31 @@ + + + + + + + - + + + ./tests/ + + - - - - - + + + + + - - - ./tests/ - - - - - - src - - - src/Resources - src/NbgrpOneloginSamlBundle.php - - - - - - - - - + + + src + + + src/Resources + src/NbgrpOneloginSamlBundle.php + + diff --git a/src/Controller/Login.php b/src/Controller/Login.php index a9a0e0c..3a1309d 100644 --- a/src/Controller/Login.php +++ b/src/Controller/Login.php @@ -16,10 +16,10 @@ use Symfony\Component\Security\Http\SecurityRequestAttributes; #[AsController] -class Login +readonly class Login { public function __construct( - private readonly FirewallMap $firewallMap, + private FirewallMap $firewallMap, ) {} public function __invoke(Request $request, Auth $auth): RedirectResponse @@ -51,7 +51,7 @@ public function __invoke(Request $request, Auth $auth): RedirectResponse private function getTargetPath(Request $request, SessionInterface $session): ?string { $firewallName = $this->firewallMap->getFirewallConfig($request)?->getName(); - if (!$firewallName) { + if ($firewallName === null || $firewallName === '') { throw new ServiceUnavailableHttpException(message: 'Unknown firewall.'); } @@ -62,15 +62,13 @@ private function getTargetPath(Request $request, SessionInterface $session): ?st private function processLoginAndGetRedirectUrl(Auth $auth, ?string $targetPath, ?SessionInterface $session): string { $redirectUrl = $auth->login(returnTo: $targetPath, stay: true); - if ($redirectUrl === null) { - throw new \RuntimeException('Login cannot be performed: Auth did not returned redirect url.'); - } $security = $auth->getSettings()->getSecurityData(); - if (($security['rejectUnsolicitedResponsesWithInResponseTo'] ?? false) && $session instanceof SessionInterface) { + if (($security['rejectUnsolicitedResponsesWithInResponseTo'] ?? false) !== false && $session instanceof SessionInterface) { $session->set(SamlAuthenticator::LAST_REQUEST_ID, $auth->getLastRequestID()); } + // @phan-suppress-next-line PhanPossiblyNullTypeReturn return $redirectUrl; } } diff --git a/src/EventListener/Security/SamlLogoutListener.php b/src/EventListener/Security/SamlLogoutListener.php index 5ef8be9..a0e63ec 100644 --- a/src/EventListener/Security/SamlLogoutListener.php +++ b/src/EventListener/Security/SamlLogoutListener.php @@ -17,18 +17,18 @@ /** * Process Single Logout by current OneLogin Auth service on user logout. */ -final class SamlLogoutListener +final readonly class SamlLogoutListener { public function __construct( - private readonly AuthRegistryInterface $authRegistry, - private readonly IdpResolverInterface $idpResolver, + private AuthRegistryInterface $authRegistry, + private IdpResolverInterface $idpResolver, ) {} #[AsEventListener(LogoutEvent::class)] public function processSingleLogout(LogoutEvent $event): void { $authService = $this->getAuthService($event->getRequest()); - if (!$authService) { + if ($authService === null) { return; } @@ -40,20 +40,23 @@ public function processSingleLogout(LogoutEvent $event): void try { $authService->processSLO(); } catch (\OneLogin\Saml2\Error) { - if (!empty($authService->getSLOurl())) { - /** @var string|null $sessionIndex */ - $sessionIndex = $token->hasAttribute(SamlAuthenticator::SESSION_INDEX_ATTRIBUTE) - ? $token->getAttribute(SamlAuthenticator::SESSION_INDEX_ATTRIBUTE) - : null; - $authService->logout(null, [], $token->getUserIdentifier(), $sessionIndex); + $sloUrl = $authService->getSLOurl(); + if ($sloUrl === null || $sloUrl === '') { + return; } + + /** @var string|null $sessionIndex */ + $sessionIndex = $token->hasAttribute(SamlAuthenticator::SESSION_INDEX_ATTRIBUTE) + ? $token->getAttribute(SamlAuthenticator::SESSION_INDEX_ATTRIBUTE) + : null; + $authService->logout(null, [], $token->getUserIdentifier(), $sessionIndex); } } private function getAuthService(Request $request): ?Auth { $idp = $this->idpResolver->resolve($request); - if (!$idp) { + if ($idp === null || $idp === '') { return $this->authRegistry->getDefaultService(); } diff --git a/src/Idp/IdpResolver.php b/src/Idp/IdpResolver.php index c694941..41fc20c 100644 --- a/src/Idp/IdpResolver.php +++ b/src/Idp/IdpResolver.php @@ -7,10 +7,10 @@ use Symfony\Component\HttpFoundation\Request; -final class IdpResolver implements IdpResolverInterface +final readonly class IdpResolver implements IdpResolverInterface { public function __construct( - private readonly string $idpParameterName, + private string $idpParameterName, ) {} public function resolve(Request $request): ?string diff --git a/src/Onelogin/AuthArgumentResolver.php b/src/Onelogin/AuthArgumentResolver.php index 52aafde..b3ea592 100644 --- a/src/Onelogin/AuthArgumentResolver.php +++ b/src/Onelogin/AuthArgumentResolver.php @@ -17,11 +17,11 @@ * Yields the OneLogin Auth instance for current request * (default or according to an idp parameter). */ -final class AuthArgumentResolver implements ValueResolverInterface +final readonly class AuthArgumentResolver implements ValueResolverInterface { public function __construct( - private readonly AuthRegistryInterface $authRegistry, - private readonly IdpResolverInterface $idpResolver, + private AuthRegistryInterface $authRegistry, + private IdpResolverInterface $idpResolver, ) {} public function resolve(Request $request, ArgumentMetadata $argument): iterable @@ -31,12 +31,12 @@ public function resolve(Request $request, ArgumentMetadata $argument): iterable } $idp = $this->idpResolver->resolve($request); - if ($idp && !$this->authRegistry->hasService($idp)) { + if (($idp !== null && $idp !== '') && !$this->authRegistry->hasService($idp)) { throw new BadRequestHttpException('There is no OneLogin PHP toolkit settings for IdP "'.$idp.'". See nbgrp_onelogin_saml config ("onelogin_settings" section).'); } try { - yield $idp + yield ($idp !== null && $idp !== '') ? $this->authRegistry->getService($idp) : $this->authRegistry->getDefaultService(); } catch (\RuntimeException $exception) { diff --git a/src/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadge.php b/src/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadge.php index 5148024..5b3a01a 100644 --- a/src/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadge.php +++ b/src/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadge.php @@ -10,10 +10,10 @@ /** * Allows to add SAML attributes to a passport. */ -class SamlAttributesBadge implements BadgeInterface +readonly class SamlAttributesBadge implements BadgeInterface { public function __construct( - private readonly array $attributes, + private array $attributes, ) {} public function getAttributes(): array diff --git a/src/Security/Http/Authenticator/SamlAuthenticator.php b/src/Security/Http/Authenticator/SamlAuthenticator.php index 83772c3..482a365 100644 --- a/src/Security/Http/Authenticator/SamlAuthenticator.php +++ b/src/Security/Http/Authenticator/SamlAuthenticator.php @@ -68,7 +68,7 @@ public function start(Request $request, ?AuthenticationException $authException { $uri = $this->httpUtils->generateUri($request, (string) $this->options['login_path']); $idp = $this->idpResolver->resolve($request); - if ($idp) { + if ($idp !== null && $idp !== '') { $uri .= '?'.$this->idpParameterName.'='.$idp; } @@ -126,7 +126,7 @@ protected function processResponse(Auth $oneLoginAuth, SessionInterface $session { $requestId = null; $security = $oneLoginAuth->getSettings()->getSecurityData(); - if ($security['rejectUnsolicitedResponsesWithInResponseTo'] ?? false) { + if (($security['rejectUnsolicitedResponsesWithInResponseTo'] ?? false) !== false) { /** @var string $requestId */ $requestId = $session->get(self::LAST_REQUEST_ID); } @@ -179,7 +179,7 @@ function (string $identifier) use ($deferredEventBadge, $attributes) { protected function extractAttributes(Auth $oneLoginAuth): array { - $attributes = $this->options['use_attribute_friendly_name'] ?? false + $attributes = ($this->options['use_attribute_friendly_name'] ?? false) !== false ? $oneLoginAuth->getAttributesWithFriendlyName() : $oneLoginAuth->getAttributes(); $attributes[self::SESSION_INDEX_ATTRIBUTE] = $oneLoginAuth->getSessionIndex(); @@ -215,7 +215,7 @@ private function getOneLoginAuth(Request $request): Auth { try { $idp = $this->idpResolver->resolve($request); - $authService = $idp + $authService = ($idp !== null && $idp !== '') ? $this->authRegistry->getService($idp) : $this->authRegistry->getDefaultService(); } catch (\RuntimeException $exception) { diff --git a/src/Security/User/SamlUserFactory.php b/src/Security/User/SamlUserFactory.php index dee41e6..08c0147 100644 --- a/src/Security/User/SamlUserFactory.php +++ b/src/Security/User/SamlUserFactory.php @@ -7,15 +7,15 @@ use Symfony\Component\Security\Core\User\UserInterface; -final class SamlUserFactory implements SamlUserFactoryInterface +final readonly class SamlUserFactory implements SamlUserFactoryInterface { /** * @param class-string $userClass * @param array $mapping */ public function __construct( - private readonly string $userClass, - private readonly array $mapping, + private string $userClass, + private array $mapping, ) {} public function createUser(string $identifier, array $attributes): UserInterface diff --git a/tests/Controller/LoginTest.php b/tests/Controller/LoginTest.php index d8b1a7e..35b5d90 100644 --- a/tests/Controller/LoginTest.php +++ b/tests/Controller/LoginTest.php @@ -9,6 +9,8 @@ use Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\SamlAuthenticator; use OneLogin\Saml2\Auth; use OneLogin\Saml2\Settings; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Bundle\SecurityBundle\Security\FirewallConfig; use Symfony\Bundle\SecurityBundle\Security\FirewallMap; @@ -19,15 +21,39 @@ use Symfony\Component\Security\Http\SecurityRequestAttributes; /** - * @covers \Nbgrp\OneloginSamlBundle\Controller\Login - * * @internal */ +#[CoversClass(Login::class)] final class LoginTest extends TestCase { + public static function provideErrorExceptionCases(): iterable + { + yield 'From attributes' => [ + 'request' => (static function () { + $request = Request::create('/login'); + $request->attributes->set(SecurityRequestAttributes::AUTHENTICATION_ERROR, new \Exception('Error from attributes')); + + return $request; + })(), + 'expectedMessage' => 'Error from attributes', + ]; + + yield 'From session' => [ + 'request' => (static function () { + $request = Request::create('/login'); + $session = new Session(new MockArraySessionStorage()); + $session->set(SecurityRequestAttributes::AUTHENTICATION_ERROR, new \Exception('Error from session')); + $request->setSession($session); + + return $request; + })(), + 'expectedMessage' => 'Error from session', + ]; + } + public function testInvokeWithRejectUnsolicitedResponsesWithInResponseTo(): void { - $firewallMap = $this->createStub(FirewallMap::class); + $firewallMap = self::createStub(FirewallMap::class); $firewallMap ->method('getFirewallConfig') ->willReturn(new FirewallConfig('foo', 'bar')) @@ -67,7 +93,7 @@ public function testInvokeWithRejectUnsolicitedResponsesWithInResponseTo(): void public function testInvokeWithoutRejectUnsolicitedResponsesWithInResponseTo(): void { - $firewallMap = $this->createStub(FirewallMap::class); + $firewallMap = self::createStub(FirewallMap::class); $firewallMap ->method('getFirewallConfig') ->willReturn(new FirewallConfig('foo', 'bar')) @@ -105,9 +131,7 @@ public function testInvokeWithoutRejectUnsolicitedResponsesWithInResponseTo(): v self::assertFalse($session->has(SamlAuthenticator::LAST_REQUEST_ID)); } - /** - * @dataProvider provideErrorExceptionCases - */ + #[DataProvider('provideErrorExceptionCases')] public function testErrorException(Request $request, string $expectedMessage): void { $firewallMap = $this->createMock(FirewallMap::class); @@ -116,7 +140,7 @@ public function testErrorException(Request $request, string $expectedMessage): v ->willReturn(new FirewallConfig('foo', 'bar')) ; - $auth = $this->createStub(Auth::class); + $auth = self::createStub(Auth::class); $controller = new Login($firewallMap); @@ -125,55 +149,6 @@ public function testErrorException(Request $request, string $expectedMessage): v $controller($request, $auth); } - public function provideErrorExceptionCases(): iterable - { - yield 'From attributes' => [ - 'request' => (static function () { - $request = Request::create('/login'); - $request->attributes->set(SecurityRequestAttributes::AUTHENTICATION_ERROR, new \Exception('Error from attributes')); - - return $request; - })(), - 'expectedMessage' => 'Error from attributes', - ]; - - yield 'From session' => [ - 'request' => (static function () { - $request = Request::create('/login'); - $session = new Session(new MockArraySessionStorage()); - $session->set(SecurityRequestAttributes::AUTHENTICATION_ERROR, new \Exception('Error from session')); - $request->setSession($session); - - return $request; - })(), - 'expectedMessage' => 'Error from session', - ]; - } - - public function testAuthLoginWithoutRedirectUrlException(): void - { - $firewallMap = $this->createMock(FirewallMap::class); - $firewallMap - ->method('getFirewallConfig') - ->willReturn(new FirewallConfig('foo', 'bar')) - ; - - $auth = $this->createMock(Auth::class); - $auth - ->method('login') - ->willReturn(null) - ; - - $request = Request::create('/login'); - $request->setSession(new Session(new MockArraySessionStorage())); - - $controller = new Login($firewallMap); - - $this->expectException(\RuntimeException::class); - $this->expectExceptionMessage('Login cannot be performed: Auth did not returned redirect url.'); - $controller($request, $auth); - } - public function testUnknownFirewallException(): void { $firewallMap = $this->createMock(FirewallMap::class); @@ -182,7 +157,7 @@ public function testUnknownFirewallException(): void ->willReturn(null) ; - $auth = $this->createStub(Auth::class); + $auth = self::createStub(Auth::class); $controller = new Login($firewallMap); $request = Request::create('/login'); diff --git a/tests/Controller/MetadataTest.php b/tests/Controller/MetadataTest.php index d87c849..e019fa5 100644 --- a/tests/Controller/MetadataTest.php +++ b/tests/Controller/MetadataTest.php @@ -8,13 +8,13 @@ use Nbgrp\OneloginSamlBundle\Controller\Metadata; use OneLogin\Saml2\Auth; use OneLogin\Saml2\Settings; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; /** - * @covers \Nbgrp\OneloginSamlBundle\Controller\Metadata - * * @internal */ +#[CoversClass(Metadata::class)] final class MetadataTest extends TestCase { public function testInvoke(): void diff --git a/tests/DependencyInjection/Compiler/AuthRegistryCompilerPassTest.php b/tests/DependencyInjection/Compiler/AuthRegistryCompilerPassTest.php index 8beba6c..09dabae 100644 --- a/tests/DependencyInjection/Compiler/AuthRegistryCompilerPassTest.php +++ b/tests/DependencyInjection/Compiler/AuthRegistryCompilerPassTest.php @@ -7,15 +7,15 @@ use Nbgrp\OneloginSamlBundle\DependencyInjection\Compiler\AuthRegistryCompilerPass; use Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistryInterface; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; /** - * @covers \Nbgrp\OneloginSamlBundle\DependencyInjection\Compiler\AuthRegistryCompilerPass - * * @internal */ +#[CoversClass(AuthRegistryCompilerPass::class)] final class AuthRegistryCompilerPassTest extends TestCase { public function testSuccessProcess(): void diff --git a/tests/DependencyInjection/Compiler/EntityManagerCompilerPassTest.php b/tests/DependencyInjection/Compiler/EntityManagerCompilerPassTest.php index ec4a660..a0b30dd 100644 --- a/tests/DependencyInjection/Compiler/EntityManagerCompilerPassTest.php +++ b/tests/DependencyInjection/Compiler/EntityManagerCompilerPassTest.php @@ -8,16 +8,16 @@ use Nbgrp\OneloginSamlBundle\DependencyInjection\Compiler\EntityManagerCompilerPass; use Nbgrp\OneloginSamlBundle\EventListener\User\UserCreatedListener; use Nbgrp\OneloginSamlBundle\EventListener\User\UserModifiedListener; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; /** - * @covers \Nbgrp\OneloginSamlBundle\DependencyInjection\Compiler\EntityManagerCompilerPass - * * @internal */ +#[CoversClass(EntityManagerCompilerPass::class)] final class EntityManagerCompilerPassTest extends TestCase { public function testNoProcess(): void diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index bbeaf91..8c688db 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -6,28 +6,21 @@ namespace Nbgrp\Tests\OneloginSamlBundle\DependencyInjection; use Nbgrp\OneloginSamlBundle\DependencyInjection\Configuration; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\Definition\Processor; /** - * @covers \Nbgrp\OneloginSamlBundle\DependencyInjection\Configuration - * * @internal */ +#[CoversClass(Configuration::class)] final class ConfigurationTest extends TestCase { private Processor $processor; - /** - * @dataProvider provideValidConfigCases - */ - public function testValidConfig(array $config, array $expected): void - { - self::assertSame($expected, $this->processor->processConfiguration(new Configuration(), [$config])); - } - - public function provideValidConfigCases(): iterable + public static function provideValidConfigCases(): iterable { yield 'Simple configuration' => [ 'config' => [ @@ -301,17 +294,7 @@ public function provideValidConfigCases(): iterable ]; } - /** - * @dataProvider provideConfigWithInvalidOneLoginSettingsExceptionCases - */ - public function testConfigWithInvalidOneLoginSettingsException(array $config, string $expectedMessage): void - { - $this->expectException(InvalidConfigurationException::class); - $this->expectExceptionMessage($expectedMessage); - $this->processor->processConfiguration(new Configuration(), [$config]); - } - - public function provideConfigWithInvalidOneLoginSettingsExceptionCases(): iterable + public static function provideConfigWithInvalidOneLoginSettingsExceptionCases(): iterable { yield 'Empty idp OneLogin settings' => [ 'config' => [ @@ -717,6 +700,20 @@ public function provideConfigWithInvalidOneLoginSettingsExceptionCases(): iterab ]; } + #[DataProvider('provideValidConfigCases')] + public function testValidConfig(array $config, array $expected): void + { + self::assertSame($expected, $this->processor->processConfiguration(new Configuration(), [$config])); + } + + #[DataProvider('provideConfigWithInvalidOneLoginSettingsExceptionCases')] + public function testConfigWithInvalidOneLoginSettingsException(array $config, string $expectedMessage): void + { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage($expectedMessage); + $this->processor->processConfiguration(new Configuration(), [$config]); + } + protected function setUp(): void { $this->processor = new Processor(); diff --git a/tests/DependencyInjection/NbgrpOneloginSamlExtensionTest.php b/tests/DependencyInjection/NbgrpOneloginSamlExtensionTest.php index 370c69e..fbd1df8 100644 --- a/tests/DependencyInjection/NbgrpOneloginSamlExtensionTest.php +++ b/tests/DependencyInjection/NbgrpOneloginSamlExtensionTest.php @@ -12,14 +12,14 @@ use Nbgrp\OneloginSamlBundle\Idp\IdpResolverInterface; use Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistryInterface; use Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\SamlAuthenticator; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerBuilder; /** - * @covers \Nbgrp\OneloginSamlBundle\DependencyInjection\NbgrpOneloginSamlExtension - * * @internal */ +#[CoversClass(NbgrpOneloginSamlExtension::class)] final class NbgrpOneloginSamlExtensionTest extends TestCase { public function testLoad(): void diff --git a/tests/DependencyInjection/Security/Factory/SamlFactoryTest.php b/tests/DependencyInjection/Security/Factory/SamlFactoryTest.php index fbec932..cc568f6 100644 --- a/tests/DependencyInjection/Security/Factory/SamlFactoryTest.php +++ b/tests/DependencyInjection/Security/Factory/SamlFactoryTest.php @@ -10,6 +10,7 @@ use Nbgrp\OneloginSamlBundle\EventListener\User\UserModifiedListener; use Nbgrp\OneloginSamlBundle\Security\Http\Authentication\SamlAuthenticationSuccessHandler; use Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\SamlAuthenticator; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\DependencyInjection\ChildDefinition; @@ -18,10 +19,9 @@ use Symfony\Component\DependencyInjection\Reference; /** - * @covers \Nbgrp\OneloginSamlBundle\DependencyInjection\Security\Factory\SamlFactory - * * @internal */ +#[CoversClass(SamlFactory::class)] final class SamlFactoryTest extends TestCase { private SamlFactory $factory; diff --git a/tests/DependencyInjection/Security/UserProvider/SamlUserProviderFactoryTest.php b/tests/DependencyInjection/Security/UserProvider/SamlUserProviderFactoryTest.php index 16ed884..42b07ff 100644 --- a/tests/DependencyInjection/Security/UserProvider/SamlUserProviderFactoryTest.php +++ b/tests/DependencyInjection/Security/UserProvider/SamlUserProviderFactoryTest.php @@ -7,16 +7,16 @@ use Nbgrp\OneloginSamlBundle\DependencyInjection\Security\UserProvider\SamlUserProviderFactory; use Nbgrp\Tests\OneloginSamlBundle\TestUser; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\DependencyInjection\ContainerBuilder; /** - * @covers \Nbgrp\OneloginSamlBundle\DependencyInjection\Security\UserProvider\SamlUserProviderFactory - * * @internal */ +#[CoversClass(SamlUserProviderFactory::class)] final class SamlUserProviderFactoryTest extends TestCase { private SamlUserProviderFactory $factory; diff --git a/tests/EventListener/Security/SamlLogoutListenerTest.php b/tests/EventListener/Security/SamlLogoutListenerTest.php index d2c7546..adc13e6 100644 --- a/tests/EventListener/Security/SamlLogoutListenerTest.php +++ b/tests/EventListener/Security/SamlLogoutListenerTest.php @@ -9,48 +9,31 @@ use Nbgrp\OneloginSamlBundle\Idp\IdpResolver; use Nbgrp\OneloginSamlBundle\Idp\IdpResolverInterface; use Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistry; +use Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistryInterface; use Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\SamlAuthenticator; use Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\Token\SamlToken; use Nbgrp\Tests\OneloginSamlBundle\TestUser; use OneLogin\Saml2\Auth; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Http\Event\LogoutEvent; /** - * @covers \Nbgrp\OneloginSamlBundle\EventListener\Security\SamlLogoutListener - * * @internal */ +#[CoversClass(SamlLogoutListener::class)] final class SamlLogoutListenerTest extends TestCase { - /** - * @dataProvider provideCases - */ - public function test(AuthRegistry $authRegistry, IdpResolverInterface $ipdResolver, Request $request, ?TokenInterface $token): void - { - $event = $this->createMock(LogoutEvent::class); - $event - ->method('getRequest') - ->willReturn($request) - ; - $event - ->expects($token ? self::once() : self::never()) - ->method('getToken') - ->willReturn($token) - ; - - (new SamlLogoutListener($authRegistry, $ipdResolver))->processSingleLogout($event); - } - - public function provideCases(): iterable + public static function provideCases(): iterable { yield 'No Auth service' => [ - 'authRegistry' => (function (): AuthRegistry { - $auth = $this->createMock(Auth::class); + 'authRegistry' => static function (TestCase $case): AuthRegistryInterface { + $auth = $case->createMock(Auth::class); $auth - ->expects(self::never()) + ->expects($case->never()) ->method('processSLO') ; @@ -58,17 +41,17 @@ public function provideCases(): iterable $authRegistry->addService('foo', $auth); return $authRegistry; - })(), + }, 'ipdResolver' => new IdpResolver('idp'), 'request' => Request::create('/logout', 'GET', ['idp' => 'unknown']), 'token' => null, ]; yield 'Custom Auth service without SAML token' => [ - 'authRegistry' => (function (): AuthRegistry { - $auth = $this->createMock(Auth::class); + 'authRegistry' => static function (TestCase $case): AuthRegistryInterface { + $auth = $case->createMock(Auth::class); $auth - ->expects(self::never()) + ->expects($case->never()) ->method('processSLO') ; @@ -76,15 +59,15 @@ public function provideCases(): iterable $authRegistry->addService('foo', $auth); return $authRegistry; - })(), + }, 'ipdResolver' => new IdpResolver('idp'), 'request' => Request::create('/logout', 'GET', ['idp' => 'foo']), - 'token' => $this->createStub(TokenInterface::class), + 'token' => self::createStub(TokenInterface::class), ]; yield 'Logout without session index' => [ - 'authRegistry' => (function (): AuthRegistry { - $auth = $this->createMock(Auth::class); + 'authRegistry' => static function (TestCase $case): AuthRegistryInterface { + $auth = $case->createMock(Auth::class); $auth ->method('processSLO') ->willThrowException(new \OneLogin\Saml2\Error('error')) @@ -102,15 +85,15 @@ public function provideCases(): iterable $authRegistry->addService('foo', $auth); return $authRegistry; - })(), + }, 'ipdResolver' => new IdpResolver('idp'), 'request' => Request::create('/logout'), 'token' => new SamlToken(new TestUser('tester'), 'foo', [], []), ]; yield 'Logout with session index' => [ - 'authRegistry' => (function (): AuthRegistry { - $auth = $this->createMock(Auth::class); + 'authRegistry' => static function (TestCase $case): AuthRegistryInterface { + $auth = $case->createMock(Auth::class); $auth ->method('processSLO') ->willThrowException(new \OneLogin\Saml2\Error('error')) @@ -128,10 +111,30 @@ public function provideCases(): iterable $authRegistry->addService('foo', $auth); return $authRegistry; - })(), + }, 'ipdResolver' => new IdpResolver('idp'), 'request' => Request::create('/logout'), 'token' => new SamlToken(new TestUser('tester'), 'foo', [], [SamlAuthenticator::SESSION_INDEX_ATTRIBUTE => 'session_index']), ]; } + + /** + * @param callable(TestCase): AuthRegistryInterface $authRegistry + */ + #[DataProvider('provideCases')] + public function test(callable $authRegistry, IdpResolverInterface $ipdResolver, Request $request, ?TokenInterface $token): void + { + $event = $this->createMock(LogoutEvent::class); + $event + ->method('getRequest') + ->willReturn($request) + ; + $event + ->expects($token ? self::once() : self::never()) + ->method('getToken') + ->willReturn($token) + ; + + (new SamlLogoutListener($authRegistry($this), $ipdResolver))->processSingleLogout($event); + } } diff --git a/tests/EventListener/User/DeferredUserListenerTest.php b/tests/EventListener/User/DeferredUserListenerTest.php index 8f814d2..ed1f83c 100644 --- a/tests/EventListener/User/DeferredUserListenerTest.php +++ b/tests/EventListener/User/DeferredUserListenerTest.php @@ -8,6 +8,7 @@ use Nbgrp\OneloginSamlBundle\Event\AbstractUserEvent; use Nbgrp\OneloginSamlBundle\EventListener\User\DeferredUserListener; use Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\Passport\Badge\DeferredEventBadge; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; @@ -16,16 +17,15 @@ use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; /** - * @covers \Nbgrp\OneloginSamlBundle\EventListener\User\DeferredUserListener - * * @internal */ +#[CoversClass(DeferredUserListener::class)] final class DeferredUserListenerTest extends TestCase { public function testWithoutBadge(): void { $event = new CheckPassportEvent( - $this->createStub(AuthenticatorInterface::class), + self::createStub(AuthenticatorInterface::class), new SelfValidatingPassport(new UserBadge('tester')), ); @@ -41,7 +41,7 @@ public function testWithoutBadge(): void public function testBadgeWithoutEvent(): void { $event = new CheckPassportEvent( - $this->createStub(AuthenticatorInterface::class), + self::createStub(AuthenticatorInterface::class), new SelfValidatingPassport(new UserBadge('tester'), [new DeferredEventBadge()]), ); @@ -62,7 +62,7 @@ public function testSuccessfulEventDispatching(): void $deferredEventBadge->setEvent($deferredEvent); $event = new CheckPassportEvent( - $this->createStub(AuthenticatorInterface::class), + self::createStub(AuthenticatorInterface::class), new SelfValidatingPassport(new UserBadge('tester'), [$deferredEventBadge]), ); diff --git a/tests/EventListener/User/UserListenersTest.php b/tests/EventListener/User/UserListenersTest.php index 54bd2dd..52a3ce8 100644 --- a/tests/EventListener/User/UserListenersTest.php +++ b/tests/EventListener/User/UserListenersTest.php @@ -6,79 +6,89 @@ namespace Nbgrp\Tests\OneloginSamlBundle\EventListener\User; use Doctrine\ORM\EntityManagerInterface; +use Nbgrp\OneloginSamlBundle\Event\AbstractUserEvent; use Nbgrp\OneloginSamlBundle\Event\UserCreatedEvent; use Nbgrp\OneloginSamlBundle\Event\UserModifiedEvent; +use Nbgrp\OneloginSamlBundle\EventListener\User\AbstractUserListener; use Nbgrp\OneloginSamlBundle\EventListener\User\UserCreatedListener; use Nbgrp\OneloginSamlBundle\EventListener\User\UserModifiedListener; use Nbgrp\Tests\OneloginSamlBundle\TestUser; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\User\UserInterface; /** - * @covers \Nbgrp\OneloginSamlBundle\Event\AbstractUserEvent - * @covers \Nbgrp\OneloginSamlBundle\EventListener\User\AbstractUserListener - * @covers \Nbgrp\OneloginSamlBundle\EventListener\User\UserCreatedListener - * @covers \Nbgrp\OneloginSamlBundle\EventListener\User\UserModifiedListener - * * @internal */ +#[CoversClass(AbstractUserEvent::class)] +#[CoversClass(AbstractUserListener::class)] +#[CoversClass(UserCreatedListener::class)] +#[CoversClass(UserModifiedListener::class)] final class UserListenersTest extends TestCase { - /** - * @dataProvider userListenerProvider - */ - public function testUserCreatedListener(?EntityManagerInterface $entityManager, bool $needPersist, UserInterface $user): void + public static function provideUserListenerCases(): iterable { - (new UserCreatedListener($entityManager, $needPersist))(new UserCreatedEvent($user)); + yield 'needPersist false' => [ + 'entityManager' => static function (TestCase $case): EntityManagerInterface { + $entityManager = $case->createMock(EntityManagerInterface::class); + $entityManager + ->expects($case->never()) + ->method('persist') + ; + $entityManager + ->expects($case->never()) + ->method('flush') + ; + + return $entityManager; + }, + 'needPersist' => false, + 'user' => new TestUser('tester'), + ]; + + $user = new TestUser('tester'); + yield 'Success' => [ + 'entityManager' => static function (TestCase $case) use ($user): EntityManagerInterface { + $entityManager = $case->createMock(EntityManagerInterface::class); + $entityManager + ->expects($case->once()) + ->method('persist') + ->with($user) + ; + $entityManager + ->expects($case->once()) + ->method('flush') + ; + + return $entityManager; + }, + 'needPersist' => true, + 'user' => $user, + ]; } /** - * @dataProvider userListenerProvider + * @param callable(TestCase): EntityManagerInterface $entityManager */ - public function testUserModifiedListener(?EntityManagerInterface $entityManager, bool $needPersist, UserInterface $user): void - { - (new UserModifiedListener($entityManager, $needPersist))(new UserModifiedEvent($user)); + #[DataProvider('provideUserListenerCases')] + public function testUserCreatedListener( + callable $entityManager, + bool $needPersist, + UserInterface $user, + ): void { + (new UserCreatedListener($entityManager($this), $needPersist))(new UserCreatedEvent($user)); } - public function userListenerProvider(): iterable - { - yield 'needPersist false' => (function (): array { - $entityManager = $this->createMock(EntityManagerInterface::class); - $entityManager - ->expects(self::never()) - ->method('persist') - ; - $entityManager - ->expects(self::never()) - ->method('flush') - ; - - return [ - 'entityManager' => $entityManager, - 'needPersist' => false, - 'user' => new TestUser('tester'), - ]; - })(); - - yield 'Success' => (function (): array { - $user = new TestUser('tester'); - - $entityManager = $this->createMock(EntityManagerInterface::class); - $entityManager - ->expects(self::once()) - ->method('persist') - ->with($user) - ; - $entityManager - ->expects(self::once()) - ->method('flush') - ; - - return [ - 'entityManager' => $entityManager, - 'needPersist' => true, - 'user' => $user, - ]; - })(); + /** + * @param callable(TestCase): EntityManagerInterface $entityManager + */ + #[DataProvider('provideUserListenerCases')] + public function testUserModifiedListener( + callable $entityManager, + bool $needPersist, + UserInterface $user, + ): void { + (new UserModifiedListener($entityManager($this), $needPersist))(new UserModifiedEvent($user)); } } diff --git a/tests/Idp/IdpResolverTest.php b/tests/Idp/IdpResolverTest.php index 6dae595..47da736 100644 --- a/tests/Idp/IdpResolverTest.php +++ b/tests/Idp/IdpResolverTest.php @@ -6,27 +6,20 @@ namespace Nbgrp\Tests\OneloginSamlBundle\Idp; use Nbgrp\OneloginSamlBundle\Idp\IdpResolver; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; /** - * @covers \Nbgrp\OneloginSamlBundle\Idp\IdpResolver - * * @internal */ +#[CoversClass(IdpResolver::class)] final class IdpResolverTest extends TestCase { private IdpResolver $resolver; - /** - * @dataProvider provideResolveCases - */ - public function testResolve(Request $request, ?string $expected): void - { - self::assertSame($expected, $this->resolver->resolve($request)); - } - - public function provideResolveCases(): iterable + public static function provideResolveCases(): iterable { yield 'Request with ipd in query' => [ 'request' => new Request(['idp' => 'query-idp']), @@ -44,6 +37,12 @@ public function provideResolveCases(): iterable ]; } + #[DataProvider('provideResolveCases')] + public function testResolve(Request $request, ?string $expected): void + { + self::assertSame($expected, $this->resolver->resolve($request)); + } + protected function setUp(): void { $this->resolver = new IdpResolver('idp'); diff --git a/tests/Onelogin/AuthArgumentResolverTest.php b/tests/Onelogin/AuthArgumentResolverTest.php index 0b056fb..4df3da8 100644 --- a/tests/Onelogin/AuthArgumentResolverTest.php +++ b/tests/Onelogin/AuthArgumentResolverTest.php @@ -9,6 +9,7 @@ use Nbgrp\OneloginSamlBundle\Onelogin\AuthArgumentResolver; use Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistry; use OneLogin\Saml2\Auth; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; @@ -16,12 +17,11 @@ use Symfony\Component\HttpKernel\Exception\ServiceUnavailableHttpException; /** - * @covers \Nbgrp\OneloginSamlBundle\Onelogin\AuthArgumentResolver - * * @internal * * @psalm-suppress MixedArgumentTypeCoercion */ +#[CoversClass(AuthArgumentResolver::class)] final class AuthArgumentResolverTest extends TestCase { public function testResolve(): void @@ -29,10 +29,10 @@ public function testResolve(): void $authRegistry = new AuthRegistry(); $idpResolver = new IdpResolver('idp'); - $defaultAuth = $this->createStub(Auth::class); + $defaultAuth = self::createStub(Auth::class); $authRegistry->addService('default', $defaultAuth); - $additionalAuth = $this->createStub(Auth::class); + $additionalAuth = self::createStub(Auth::class); $authRegistry->addService('additional', $additionalAuth); $resolver = new AuthArgumentResolver($authRegistry, $idpResolver); diff --git a/tests/Onelogin/AuthFactoryTest.php b/tests/Onelogin/AuthFactoryTest.php index 0475ee5..fd55d2c 100644 --- a/tests/Onelogin/AuthFactoryTest.php +++ b/tests/Onelogin/AuthFactoryTest.php @@ -6,15 +6,15 @@ namespace Nbgrp\Tests\OneloginSamlBundle\Onelogin; use Nbgrp\OneloginSamlBundle\Onelogin\AuthFactory; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; /** - * @covers \Nbgrp\OneloginSamlBundle\Onelogin\AuthFactory - * * @internal */ +#[CoversClass(AuthFactory::class)] final class AuthFactoryTest extends TestCase { public function testReplace(): void diff --git a/tests/Onelogin/AuthRegistryTest.php b/tests/Onelogin/AuthRegistryTest.php index 907ebc4..b6fa3ca 100644 --- a/tests/Onelogin/AuthRegistryTest.php +++ b/tests/Onelogin/AuthRegistryTest.php @@ -8,23 +8,23 @@ use Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistry; use Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistryInterface; use OneLogin\Saml2\Auth; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; /** - * @covers \Nbgrp\OneloginSamlBundle\Onelogin\AuthRegistry - * * @internal */ +#[CoversClass(AuthRegistry::class)] final class AuthRegistryTest extends TestCase { private AuthRegistryInterface $registry; public function testRegistry(): void { - $defaultAuth = $this->createStub(Auth::class); + $defaultAuth = self::createStub(Auth::class); $this->registry->addService('default', $defaultAuth); - $additionalAuth = $this->createStub(Auth::class); + $additionalAuth = self::createStub(Auth::class); $this->registry->addService('additional', $additionalAuth); self::assertTrue($this->registry->hasService('default')); @@ -44,7 +44,7 @@ public function testGetNotExistsServiceException(): void public function testAddExistenceServiceException(): void { - $defaultAuth = $this->createStub(Auth::class); + $defaultAuth = self::createStub(Auth::class); $this->registry->addService('default', $defaultAuth); $this->expectException(\OverflowException::class); diff --git a/tests/Security/Http/Authentication/SamlAuthenticationSuccessHandlerTest.php b/tests/Security/Http/Authentication/SamlAuthenticationSuccessHandlerTest.php index 1980f16..da9930d 100644 --- a/tests/Security/Http/Authentication/SamlAuthenticationSuccessHandlerTest.php +++ b/tests/Security/Http/Authentication/SamlAuthenticationSuccessHandlerTest.php @@ -6,6 +6,8 @@ namespace Nbgrp\Tests\OneloginSamlBundle\Security\Http\Authentication; use Nbgrp\OneloginSamlBundle\Security\Http\Authentication\SamlAuthenticationSuccessHandler; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -14,30 +16,12 @@ use Symfony\Component\Security\Http\HttpUtils; /** - * @covers \Nbgrp\OneloginSamlBundle\Security\Http\Authentication\SamlAuthenticationSuccessHandler - * * @internal */ +#[CoversClass(SamlAuthenticationSuccessHandler::class)] final class SamlAuthenticationSuccessHandlerTest extends TestCase { - /** - * @dataProvider provideHandlerCases - */ - public function testHandler(array $options, Request $request, string $expectedLocation): void - { - $token = $this->createStub(TokenInterface::class); - $urlGenerator = $this->createConfiguredMock(UrlGeneratorInterface::class, [ - 'generate' => 'http://localhost/login', - ]); - $handler = new SamlAuthenticationSuccessHandler(new HttpUtils($urlGenerator), $options); - $response = $handler->onAuthenticationSuccess($request, $token); - - self::assertNotNull($response); - self::assertSame(Response::HTTP_FOUND, $response->getStatusCode()); - self::assertSame($expectedLocation, $response->headers->get('Location')); - } - - public function provideHandlerCases(): iterable + public static function provideHandlerCases(): iterable { yield 'Always use default target path' => [ 'options' => [ @@ -84,11 +68,26 @@ public function provideHandlerCases(): iterable ]; } + #[DataProvider('provideHandlerCases')] + public function testHandler(array $options, Request $request, string $expectedLocation): void + { + $token = self::createStub(TokenInterface::class); + $urlGenerator = $this->createConfiguredMock(UrlGeneratorInterface::class, [ + 'generate' => 'http://localhost/login', + ]); + $handler = new SamlAuthenticationSuccessHandler(new HttpUtils($urlGenerator), $options); + $response = $handler->onAuthenticationSuccess($request, $token); + + self::assertNotNull($response); + self::assertSame(Response::HTTP_FOUND, $response->getStatusCode()); + self::assertSame($expectedLocation, $response->headers->get('Location')); + } + public function testEmptyRelayState(): void { $request = Request::create('/', 'GET', ['RelayState' => '']); - $token = $this->createStub(TokenInterface::class); - $handler = new SamlAuthenticationSuccessHandler(new HttpUtils($this->createStub(UrlGeneratorInterface::class))); + $token = self::createStub(TokenInterface::class); + $handler = new SamlAuthenticationSuccessHandler(new HttpUtils(self::createStub(UrlGeneratorInterface::class))); $this->expectException(\InvalidArgumentException::class); $this->expectExceptionMessage('Cannot redirect to an empty URL'); diff --git a/tests/Security/Http/Authenticator/Passport/Badge/DeferredEventBadgeTest.php b/tests/Security/Http/Authenticator/Passport/Badge/DeferredEventBadgeTest.php index 0807529..0b073f9 100644 --- a/tests/Security/Http/Authenticator/Passport/Badge/DeferredEventBadgeTest.php +++ b/tests/Security/Http/Authenticator/Passport/Badge/DeferredEventBadgeTest.php @@ -6,14 +6,14 @@ namespace Nbgrp\Tests\OneloginSamlBundle\Security\Http\Authenticator\Passport\Badge; use Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\Passport\Badge\DeferredEventBadge; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Symfony\Contracts\EventDispatcher\Event; /** - * @covers \Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\Passport\Badge\DeferredEventBadge - * * @internal */ +#[CoversClass(DeferredEventBadge::class)] final class DeferredEventBadgeTest extends TestCase { public function testEmptyBadge(): void @@ -31,7 +31,7 @@ public function testEventBadge(): void self::assertFalse($badge->isResolved()); - $event = $this->createStub(Event::class); + $event = self::createStub(Event::class); $badge->setEvent($event); self::assertSame($event, $badge->getEvent()); diff --git a/tests/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadgeTest.php b/tests/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadgeTest.php index c587c66..445df02 100644 --- a/tests/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadgeTest.php +++ b/tests/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadgeTest.php @@ -6,27 +6,17 @@ namespace Nbgrp\Tests\OneloginSamlBundle\Security\Http\Authenticator\Passport\Badge; use Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\Passport\Badge\SamlAttributesBadge; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; /** - * @covers \Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\Passport\Badge\SamlAttributesBadge - * * @internal */ +#[CoversClass(SamlAttributesBadge::class)] final class SamlAttributesBadgeTest extends TestCase { - /** - * @dataProvider provideBadgeCases - */ - public function testBadge(array $attributes): void - { - $badge = new SamlAttributesBadge($attributes); - - self::assertSame($attributes, $badge->getAttributes()); - self::assertTrue($badge->isResolved()); - } - - public function provideBadgeCases(): iterable + public static function provideBadgeCases(): iterable { yield 'Empty attributes' => [ 'attributes' => [], @@ -39,4 +29,13 @@ public function provideBadgeCases(): iterable ], ]; } + + #[DataProvider('provideBadgeCases')] + public function testBadge(array $attributes): void + { + $badge = new SamlAttributesBadge($attributes); + + self::assertSame($attributes, $badge->getAttributes()); + self::assertTrue($badge->isResolved()); + } } diff --git a/tests/Security/Http/Authenticator/SamlAuthenticatorTest.php b/tests/Security/Http/Authenticator/SamlAuthenticatorTest.php index 8ecefd7..5759c71 100644 --- a/tests/Security/Http/Authenticator/SamlAuthenticatorTest.php +++ b/tests/Security/Http/Authenticator/SamlAuthenticatorTest.php @@ -20,6 +20,8 @@ use OneLogin\Saml2\Auth; use OneLogin\Saml2\Settings; use OneLogin\Saml2\Utils; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\Request; @@ -41,26 +43,12 @@ use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; /** - * @covers \Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\SamlAuthenticator - * * @internal */ +#[CoversClass(SamlAuthenticator::class)] final class SamlAuthenticatorTest extends TestCase { - /** - * @dataProvider provideSupportsCases - */ - public function testSupports(Request $request, bool $expectedSupports): void - { - $authenticator = $this->createSamlAuthenticator( - httpUtils: new HttpUtils(), - options: ['check_path' => '/check'], - ); - - self::assertSame($expectedSupports, $authenticator->supports($request)); - } - - public function provideSupportsCases(): iterable + public static function provideSupportsCases(): iterable { yield 'GET request' => [ 'request' => Request::create('/'), @@ -78,24 +66,7 @@ public function provideSupportsCases(): iterable ]; } - /** - * @dataProvider provideStartCases - */ - public function testStart(Request $request, string $idpParameterName, string $expectedLocation): void - { - $authenticator = $this->createSamlAuthenticator( - httpUtils: new HttpUtils(), - idpResolver: new IdpResolver($idpParameterName), - options: ['login_path' => '/login'], - idpParameterName: $idpParameterName, - ); - $response = $authenticator->start($request); - - self::assertSame(Response::HTTP_FOUND, $response->getStatusCode()); - self::assertSame($expectedLocation, $response->headers->get('Location')); - } - - public function provideStartCases(): iterable + public static function provideStartCases(): iterable { yield 'Without idp' => [ 'request' => Request::create('/'), @@ -110,6 +81,456 @@ public function provideStartCases(): iterable ]; } + public static function provideAuthenticateOneLoginErrorsExceptionCases(): iterable + { + yield 'Default Auth service + OneLogin auth error' => [ + 'idpResolver' => static fn (TestCase $case): IdpResolverInterface => $case->createConfiguredMock(IdpResolverInterface::class, [ + 'resolve' => null, + ]), + 'authRegistry' => static function (TestCase $case): AuthRegistryInterface { + $auth = $case->createConfiguredMock(Auth::class, [ + 'getErrors' => ['invalid something'], + 'getLastErrorReason' => 'error reason', + ]); + $auth + ->expects($case->once()) + ->method('processResponse') + ; + $settingsMock = $case->createMock(Settings::class); + $settingsMock + ->method('getSecurityData') + ->willReturn([]) + ; + $auth + ->expects($case->once()) + ->method('getSettings') + ->willReturn($settingsMock) + ; + $authRegistry = new AuthRegistry(); + $authRegistry->addService('foo', $auth); + + return $authRegistry; + }, + 'expectedMessage' => 'error reason', + ]; + + yield 'Custom Auth service + undefined OneLogin auth error' => [ + 'idpResolver' => static fn (TestCase $case): IdpResolverInterface => $case->createConfiguredMock(IdpResolverInterface::class, [ + 'resolve' => 'custom', + ]), + 'authRegistry' => static function (TestCase $case): AuthRegistryInterface { + $auth = $case->createConfiguredMock(Auth::class, [ + 'getErrors' => ['invalid something'], + 'getLastErrorReason' => null, + ]); + $auth + ->expects($case->once()) + ->method('processResponse') + ; + $settingsMock = $case->createMock(Settings::class); + $settingsMock + ->method('getSecurityData') + ->willReturn([]) + ; + $auth + ->expects($case->once()) + ->method('getSettings') + ->willReturn($settingsMock) + ; + $authRegistry = new AuthRegistry(); + $authRegistry->addService('custom', $auth); + + return $authRegistry; + }, + 'expectedMessage' => 'Undefined OneLogin auth error.', + ]; + } + + public static function provideAuthenticateExceptionCases(): iterable + { + yield 'SAML attributes without identifier attribute' => [ + 'auth' => static function (TestCase $case): Auth { + $settingsMock = $case->createMock(Settings::class); + $settingsMock + ->method('getSecurityData') + ->willReturn([]) + ; + $auth = $case->createConfiguredMock(Auth::class, [ + 'getAttributes' => [], + 'getSessionIndex' => 'session_index', + 'getSettings' => $settingsMock, + ]); + $auth + ->expects($case->never()) + ->method('getNameId') + ; + + return $auth; + }, + 'userProvider' => null, + 'samlUserFactory' => null, + 'options' => [ + 'identifier_attribute' => 'username', + ], + 'expectedException' => \RuntimeException::class, + 'expectedMessage' => 'Attribute "username" not found in SAML data.', + ]; + + yield 'SAML attributes with invalid identifier attribute' => [ + 'auth' => static function (TestCase $case): Auth { + $settingsMock = $case->createMock(Settings::class); + $settingsMock + ->method('getSecurityData') + ->willReturn([]) + ; + $auth = $case->createConfiguredMock(Auth::class, [ + 'getAttributes' => [ + 'username' => [], + ], + 'getSessionIndex' => 'session_index', + 'getSettings' => $settingsMock, + ]); + $auth + ->expects($case->never()) + ->method('getNameId') + ; + + return $auth; + }, + 'userProvider' => null, + 'samlUserFactory' => null, + 'options' => [ + 'identifier_attribute' => 'username', + ], + 'expectedException' => \RuntimeException::class, + 'expectedMessage' => 'Attribute "username" does not contain valid user identifier.', + ]; + + yield 'User not found without SAML user factory' => [ + 'auth' => static function (TestCase $case): Auth { + $settingsMock = $case->createMock(Settings::class); + $settingsMock + ->method('getSecurityData') + ->willReturn([]) + ; + $auth = $case->createConfiguredMock(Auth::class, [ + 'getAttributes' => [], + 'getSessionIndex' => 'session_index', + 'getSettings' => $settingsMock, + 'getNameId' => 'tester_id', + ]); + $auth + ->expects($case->never()) + ->method('getAttributesWithFriendlyName') + ; + + return $auth; + }, + 'userProvider' => static function (TestCase $case): UserProviderInterface { + $userProvider = $case->createMock(UserProviderInterface::class); + $userProvider + ->method('loadUserByIdentifier') + ->willThrowException(new UserNotFoundException()) + ; + + return $userProvider; + }, + 'samlUserFactory' => null, + 'options' => [], + 'expectedException' => UserNotFoundException::class, + 'expectedMessage' => null, + ]; + + yield 'User not found + SAML user factory exception' => [ + 'auth' => static function (TestCase $case): Auth { + $settingsMock = $case->createMock(Settings::class); + $settingsMock + ->method('getSecurityData') + ->willReturn([]) + ; + $auth = $case->createConfiguredMock(Auth::class, [ + 'getAttributes' => [], + 'getSessionIndex' => 'session_index', + 'getSettings' => $settingsMock, + 'getNameId' => 'tester_id', + ]); + $auth + ->expects($case->never()) + ->method('getAttributesWithFriendlyName') + ; + + return $auth; + }, + 'userProvider' => static function (TestCase $case): UserProviderInterface { + $userProvider = $case->createMock(UserProviderInterface::class); + $userProvider + ->method('loadUserByIdentifier') + ->willThrowException(new UserNotFoundException()) + ; + + return $userProvider; + }, + 'samlUserFactory' => static function (TestCase $case): SamlUserFactoryInterface { + $samlUserFactory = $case->createMock(SamlUserFactoryInterface::class); + $samlUserFactory + ->method('createUser') + ->willThrowException(new \Exception()) + ; + + return $samlUserFactory; + }, + 'options' => [], + 'expectedException' => AuthenticationException::class, + 'expectedMessage' => 'The authentication failed.', + ]; + } + + public static function provideSuccessAuthenticateCases(): iterable + { + yield 'Not attribute friendly name + user identifier from OneLogin auth' => [ + 'auth' => static function (TestCase $case): Auth { + $settingsMock = $case->createMock(Settings::class); + $settingsMock + ->method('getSecurityData') + ->willReturn([]) + ; + $auth = $case->createConfiguredMock(Auth::class, [ + 'getAttributes' => [ + 'username' => 'tester', + 'email' => 'tester@example.com', + ], + 'getSessionIndex' => 'session_index', + 'getSettings' => $settingsMock, + 'getNameId' => 'tester_id', + ]); + $auth + ->expects($case->never()) + ->method('getAttributesWithFriendlyName') + ; + $auth + ->method('processResponse') + ->with(null) + ; + + return $auth; + }, + 'userProvider' => static function (TestCase $case): UserProviderInterface { + $userProvider = $case->createMock(UserProviderInterface::class); + $userProvider + ->method('loadUserByIdentifier') + ->with('tester_id') + ->willReturn(new TestUser('tester_id')) + ; + + return $userProvider; + }, + 'samlUserFactory' => null, + 'eventDispatcher' => null, + 'options' => [ + 'use_attribute_friendly_name' => false, + ], + 'lastRequestId' => null, + 'useProxyVars' => false, + 'expectedUserIdentifier' => 'tester_id', + 'expectedSamlAttributes' => [ + 'username' => 'tester', + 'email' => 'tester@example.com', + SamlAuthenticator::SESSION_INDEX_ATTRIBUTE => 'session_index', + ], + 'expectedUseProxyVars' => false, + ]; + + yield 'Attribute friendly name + user identifier from SAML attributes (array) + SamlUser created' => [ + 'auth' => static function (TestCase $case): Auth { + $settingsMock = $case->createMock(Settings::class); + $settingsMock + ->method('getSecurityData') + ->willReturn(['rejectUnsolicitedResponsesWithInResponseTo' => false]) + ; + $auth = $case->createConfiguredMock(Auth::class, [ + 'getAttributesWithFriendlyName' => [ + 'username' => ['tester_attribute'], + 'email' => 'tester@example.com', + ], + 'getSessionIndex' => 'session_index', + 'getSettings' => $settingsMock, + ]); + $auth + ->expects($case->never()) + ->method('getAttributes') + ; + $auth + ->expects($case->never()) + ->method('getNameId') + ; + $auth + ->method('processResponse') + ->with(null) + ; + + return $auth; + }, + 'userProvider' => static function (TestCase $case): UserProviderInterface { + $userProvider = $case->createMock(UserProviderInterface::class); + $userProvider + ->method('loadUserByIdentifier') + ->willThrowException(new UserNotFoundException()) + ; + + return $userProvider; + }, + 'samlUserFactory' => static function (TestCase $case): SamlUserFactoryInterface { + $user = $case->createConfiguredMock(SamlUserInterface::class, [ + 'getUserIdentifier' => 'tester_attribute', + ]); + $user + ->expects($case->never()) + ->method('setSamlAttributes') + ; + + $samlUserFactory = $case->createMock(SamlUserFactoryInterface::class); + $samlUserFactory + ->method('createUser') + ->with('tester_attribute', [ + 'username' => ['tester_attribute'], + 'email' => 'tester@example.com', + SamlAuthenticator::SESSION_INDEX_ATTRIBUTE => 'session_index', + ]) + ->willReturn($user) + ; + + return $samlUserFactory; + }, + 'eventDispatcher' => static function (TestCase $case): EventDispatcherInterface { + $eventDispatcher = $case->createMock(EventDispatcherInterface::class); + $eventDispatcher + ->expects($case->once()) + ->method('dispatch') + ->with(self::isInstanceOf(UserCreatedEvent::class)) + ; + + return $eventDispatcher; + }, + 'options' => [ + 'use_attribute_friendly_name' => true, + 'identifier_attribute' => 'username', + ], + 'lastRequestId' => null, + 'useProxyVars' => false, + 'expectedUserIdentifier' => 'tester_attribute', + 'expectedSamlAttributes' => [ + 'username' => ['tester_attribute'], + 'email' => 'tester@example.com', + SamlAuthenticator::SESSION_INDEX_ATTRIBUTE => 'session_index', + ], + 'expectedUseProxyVars' => false, + ]; + + yield 'Attribute friendly name + user identifier from SAML attributes (string) + SamlUser modified + InResponseTo' => [ + 'auth' => static function (TestCase $case): Auth { + $settingsMock = $case->createMock(Settings::class); + $settingsMock + ->method('getSecurityData') + ->willReturn(['rejectUnsolicitedResponsesWithInResponseTo' => true]) + ; + $auth = $case->createConfiguredMock(Auth::class, [ + 'getAttributesWithFriendlyName' => [ + 'username' => 'tester_attribute', + 'email' => 'tester@example.com', + ], + 'getSessionIndex' => 'session_index', + 'getSettings' => $settingsMock, + ]); + $auth + ->expects($case->never()) + ->method('getAttributes') + ; + $auth + ->expects($case->never()) + ->method('getNameId') + ; + $auth + ->method('processResponse') + ->with('requestID') + ; + + return $auth; + }, + 'userProvider' => static function (TestCase $case): UserProviderInterface { + $user = $case->createConfiguredMock(SamlUserInterface::class, [ + 'getUserIdentifier' => 'tester_attribute', + ]); + $user + ->method('setSamlAttributes') + ->with([ + 'username' => 'tester_attribute', + 'email' => 'tester@example.com', + SamlAuthenticator::SESSION_INDEX_ATTRIBUTE => 'session_index', + ]) + ; + + $userProvider = $case->createMock(UserProviderInterface::class); + $userProvider + ->method('loadUserByIdentifier') + ->with('tester_attribute') + ->willReturn($user) + ; + + return $userProvider; + }, + 'samlUserFactory' => null, + 'eventDispatcher' => static function (TestCase $case): EventDispatcherInterface { + $eventDispatcher = $case->createMock(EventDispatcherInterface::class); + $eventDispatcher + ->expects($case->once()) + ->method('dispatch') + ->with(self::isInstanceOf(UserModifiedEvent::class)) + ; + + return $eventDispatcher; + }, + 'options' => [ + 'use_attribute_friendly_name' => true, + 'identifier_attribute' => 'username', + ], + 'lastRequestId' => 'requestID', + 'useProxyVars' => true, + 'expectedUserIdentifier' => 'tester_attribute', + 'expectedSamlAttributes' => [ + 'username' => 'tester_attribute', + 'email' => 'tester@example.com', + SamlAuthenticator::SESSION_INDEX_ATTRIBUTE => 'session_index', + ], + 'expectedUseProxyVars' => true, + ]; + } + + #[DataProvider('provideSupportsCases')] + public function testSupports(Request $request, bool $expectedSupports): void + { + $authenticator = $this->createSamlAuthenticator( + httpUtils: new HttpUtils(), + options: ['check_path' => '/check'], + ); + + self::assertSame($expectedSupports, $authenticator->supports($request)); + } + + #[DataProvider('provideStartCases')] + public function testStart(Request $request, string $idpParameterName, string $expectedLocation): void + { + $authenticator = $this->createSamlAuthenticator( + httpUtils: new HttpUtils(), + idpResolver: new IdpResolver($idpParameterName), + options: ['login_path' => '/login'], + idpParameterName: $idpParameterName, + ); + $response = $authenticator->start($request); + + self::assertSame(Response::HTTP_FOUND, $response->getStatusCode()); + self::assertSame($expectedLocation, $response->headers->get('Location')); + } + public function testAuthenticateSessionException(): void { $authenticator = $this->createSamlAuthenticator(); @@ -121,11 +542,13 @@ public function testAuthenticateSessionException(): void } /** - * @dataProvider provideAuthenticateOneLoginErrorsExceptionCases + * @param callable(TestCase): IdpResolverInterface $idpResolver + * @param callable(TestCase): AuthRegistryInterface $authRegistry */ + #[DataProvider('provideAuthenticateOneLoginErrorsExceptionCases')] public function testAuthenticateOneLoginErrorsException( - IdpResolverInterface $idpResolver, - AuthRegistryInterface $authRegistry, + callable $idpResolver, + callable $authRegistry, string $expectedMessage, ): void { $request = Request::create('/'); @@ -138,8 +561,8 @@ public function testAuthenticateOneLoginErrorsException( ; $authenticator = $this->createSamlAuthenticator( - idpResolver: $idpResolver, - authRegistry: $authRegistry, + idpResolver: $idpResolver($this), + authRegistry: $authRegistry($this), logger: $logger, ); @@ -149,73 +572,6 @@ public function testAuthenticateOneLoginErrorsException( $authenticator->authenticate($request); } - public function provideAuthenticateOneLoginErrorsExceptionCases(): iterable - { - yield 'Default Auth service + OneLogin auth error' => (function (): array { - $idpResolver = $this->createConfiguredMock(IdpResolverInterface::class, [ - 'resolve' => null, - ]); - $auth = $this->createConfiguredMock(Auth::class, [ - 'getErrors' => ['invalid something'], - 'getLastErrorReason' => 'error reason', - ]); - $auth - ->expects(self::once()) - ->method('processResponse') - ; - $settingsMock = $this->createMock(Settings::class); - $settingsMock - ->method('getSecurityData') - ->willReturn([]) - ; - $auth - ->expects(self::once()) - ->method('getSettings') - ->willReturn($settingsMock) - ; - $authRegistry = new AuthRegistry(); - $authRegistry->addService('foo', $auth); - - return [ - 'idpResolver' => $idpResolver, - 'authRegistry' => $authRegistry, - 'expectedMessage' => 'error reason', - ]; - })(); - - yield 'Custom Auth service + undefined OneLogin auth error' => (function (): array { - $idpResolver = $this->createConfiguredMock(IdpResolverInterface::class, [ - 'resolve' => 'custom', - ]); - $auth = $this->createConfiguredMock(Auth::class, [ - 'getErrors' => ['invalid something'], - 'getLastErrorReason' => null, - ]); - $auth - ->expects(self::once()) - ->method('processResponse') - ; - $settingsMock = $this->createMock(Settings::class); - $settingsMock - ->method('getSecurityData') - ->willReturn([]) - ; - $auth - ->expects(self::once()) - ->method('getSettings') - ->willReturn($settingsMock) - ; - $authRegistry = new AuthRegistry(); - $authRegistry->addService('custom', $auth); - - return [ - 'idpResolver' => $idpResolver, - 'authRegistry' => $authRegistry, - 'expectedMessage' => 'Undefined OneLogin auth error.', - ]; - })(); - } - public function testAuthenticateWithoutAuthServiceException(): void { $request = Request::create('/'); @@ -236,13 +592,17 @@ public function testAuthenticateWithoutAuthServiceException(): void } /** - * @dataProvider provideSuccessAuthenticateCases + * @param callable(TestCase): Auth $auth + * @param ?callable(TestCase): UserProviderInterface $userProvider + * @param ?callable(TestCase): SamlUserFactoryInterface $samlUserFactory + * @param ?callable(TestCase): EventDispatcherInterface $eventDispatcher */ + #[DataProvider('provideSuccessAuthenticateCases')] public function testSuccessAuthenticate( - Auth $auth, - ?UserProviderInterface $userProvider, - ?SamlUserFactoryInterface $samlUserFactory, - ?EventDispatcherInterface $eventDispatcher, + callable $auth, + ?callable $userProvider, + ?callable $samlUserFactory, + ?callable $eventDispatcher, array $options, ?string $lastRequestId, bool $useProxyVars, @@ -262,15 +622,15 @@ public function testSuccessAuthenticate( ]); $authRegistry = new AuthRegistry(); - $authRegistry->addService('foo', $auth); + $authRegistry->addService('foo', $auth($this)); $authenticator = $this->createSamlAuthenticator( - userProvider: $userProvider, + userProvider: $userProvider !== null ? $userProvider($this) : null, idpResolver: $idpResolver, authRegistry: $authRegistry, options: $options, - samlUserFactory: $samlUserFactory, - useProxyVars: $expectedUseProxyVars, + samlUserFactory: $samlUserFactory !== null ? $samlUserFactory($this) : null, + useProxyVars: $useProxyVars, ); self::assertFalse(Utils::getProxyVars()); @@ -282,7 +642,7 @@ public function testSuccessAuthenticate( $badge = $passport->getBadge(SamlAttributesBadge::class); self::assertSame($expectedSamlAttributes, $badge->getAttributes()); - if (!$eventDispatcher) { + if ($eventDispatcher === null) { return; } @@ -294,226 +654,20 @@ public function testSuccessAuthenticate( $deferredEvent = $deferredEventBadge->getEvent(); self::assertInstanceOf(Event::class, $deferredEvent); - $eventDispatcher->dispatch($deferredEvent); - } - - public function provideSuccessAuthenticateCases(): iterable - { - yield 'Not attribute friendly name + user identifier from OneLogin auth' => (function (): array { - $settingsMock = $this->createMock(Settings::class); - $settingsMock - ->method('getSecurityData') - ->willReturn([]) - ; - $auth = $this->createConfiguredMock(Auth::class, [ - 'getAttributes' => [ - 'username' => 'tester', - 'email' => 'tester@example.com', - ], - 'getSessionIndex' => 'session_index', - 'getSettings' => $settingsMock, - 'getNameId' => 'tester_id', - ]); - $auth - ->expects(self::never()) - ->method('getAttributesWithFriendlyName') - ; - $auth - ->method('processResponse') - ->with(null) - ; - - $userProvider = $this->createMock(UserProviderInterface::class); - $userProvider - ->method('loadUserByIdentifier') - ->with('tester_id') - ->willReturn(new TestUser('tester_id')) - ; - - return [ - 'auth' => $auth, - 'userProvider' => $userProvider, - 'samlUserFactory' => null, - 'eventDispatcher' => null, - 'options' => [ - 'use_attribute_friendly_name' => false, - ], - 'lastRequestId' => null, - 'useProxyVars' => false, - 'expectedUserIdentifier' => 'tester_id', - 'expectedSamlAttributes' => [ - 'username' => 'tester', - 'email' => 'tester@example.com', - SamlAuthenticator::SESSION_INDEX_ATTRIBUTE => 'session_index', - ], - 'expectedUseProxyVars' => false, - ]; - })(); - - yield 'Attribute friendly name + user identifier from SAML attributes (array) + SamlUser created' => (function (): array { - $settingsMock = $this->createMock(Settings::class); - $settingsMock - ->method('getSecurityData') - ->willReturn(['rejectUnsolicitedResponsesWithInResponseTo' => false]) - ; - $auth = $this->createConfiguredMock(Auth::class, [ - 'getAttributesWithFriendlyName' => [ - 'username' => ['tester_attribute'], - 'email' => 'tester@example.com', - ], - 'getSessionIndex' => 'session_index', - 'getSettings' => $settingsMock, - ]); - $auth - ->expects(self::never()) - ->method('getAttributes') - ; - $auth - ->expects(self::never()) - ->method('getNameId') - ; - $auth - ->method('processResponse') - ->with(null) - ; - - $userProvider = $this->createMock(UserProviderInterface::class); - $userProvider - ->method('loadUserByIdentifier') - ->willThrowException(new UserNotFoundException()) - ; - - $user = $this->createConfiguredMock(SamlUserInterface::class, [ - 'getUserIdentifier' => 'tester_attribute', - ]); - $user - ->expects(self::never()) - ->method('setSamlAttributes') - ; - - $samlUserFactory = $this->createMock(SamlUserFactoryInterface::class); - $samlUserFactory - ->method('createUser') - ->with('tester_attribute', [ - 'username' => ['tester_attribute'], - 'email' => 'tester@example.com', - SamlAuthenticator::SESSION_INDEX_ATTRIBUTE => 'session_index', - ]) - ->willReturn($user) - ; - - $eventDispatcher = $this->createMock(EventDispatcherInterface::class); - $eventDispatcher - ->expects(self::once()) - ->method('dispatch') - ->with(self::isInstanceOf(UserCreatedEvent::class)) - ; - - return [ - 'auth' => $auth, - 'userProvider' => $userProvider, - 'samlUserFactory' => $samlUserFactory, - 'eventDispatcher' => $eventDispatcher, - 'options' => [ - 'use_attribute_friendly_name' => true, - 'identifier_attribute' => 'username', - ], - 'lastRequestId' => null, - 'useProxyVars' => false, - 'expectedUserIdentifier' => 'tester_attribute', - 'expectedSamlAttributes' => [ - 'username' => ['tester_attribute'], - 'email' => 'tester@example.com', - SamlAuthenticator::SESSION_INDEX_ATTRIBUTE => 'session_index', - ], - 'expectedUseProxyVars' => false, - ]; - })(); - - yield 'Attribute friendly name + user identifier from SAML attributes (string) + SamlUser modified + InResponseTo' => (function (): array { - $settingsMock = $this->createMock(Settings::class); - $settingsMock - ->method('getSecurityData') - ->willReturn(['rejectUnsolicitedResponsesWithInResponseTo' => true]) - ; - $auth = $this->createConfiguredMock(Auth::class, [ - 'getAttributesWithFriendlyName' => [ - 'username' => 'tester_attribute', - 'email' => 'tester@example.com', - ], - 'getSessionIndex' => 'session_index', - 'getSettings' => $settingsMock, - ]); - $auth - ->expects(self::never()) - ->method('getAttributes') - ; - $auth - ->expects(self::never()) - ->method('getNameId') - ; - $auth - ->method('processResponse') - ->with('requestID') - ; - - $user = $this->createConfiguredMock(SamlUserInterface::class, [ - 'getUserIdentifier' => 'tester_attribute', - ]); - $user - ->method('setSamlAttributes') - ->with([ - 'username' => 'tester_attribute', - 'email' => 'tester@example.com', - SamlAuthenticator::SESSION_INDEX_ATTRIBUTE => 'session_index', - ]) - ; - - $userProvider = $this->createMock(UserProviderInterface::class); - $userProvider - ->method('loadUserByIdentifier') - ->with('tester_attribute') - ->willReturn($user) - ; - - $eventDispatcher = $this->createMock(EventDispatcherInterface::class); - $eventDispatcher - ->expects(self::once()) - ->method('dispatch') - ->with(self::isInstanceOf(UserModifiedEvent::class)) - ; - - return [ - 'auth' => $auth, - 'userProvider' => $userProvider, - 'samlUserFactory' => null, - 'eventDispatcher' => $eventDispatcher, - 'options' => [ - 'use_attribute_friendly_name' => true, - 'identifier_attribute' => 'username', - ], - 'lastRequestId' => 'requestID', - 'useProxyVars' => true, - 'expectedUserIdentifier' => 'tester_attribute', - 'expectedSamlAttributes' => [ - 'username' => 'tester_attribute', - 'email' => 'tester@example.com', - SamlAuthenticator::SESSION_INDEX_ATTRIBUTE => 'session_index', - ], - 'expectedUseProxyVars' => true, - ]; - })(); + $eventDispatcher($this)->dispatch($deferredEvent); } /** - * @dataProvider provideAuthenticateExceptionCases - * - * @param class-string<\Throwable> $expectedException + * @param callable(TestCase): Auth $auth + * @param ?callable(TestCase): UserProviderInterface $userProvider + * @param ?callable(TestCase): SamlUserFactoryInterface $samlUserFactory + * @param class-string<\Throwable> $expectedException */ + #[DataProvider('provideAuthenticateExceptionCases')] public function testAuthenticateException( - Auth $auth, - ?UserProviderInterface $userProvider, - ?SamlUserFactoryInterface $samlUserFactory, + callable $auth, + ?callable $userProvider, + ?callable $samlUserFactory, array $options, string $expectedException, ?string $expectedMessage, @@ -526,14 +680,14 @@ public function testAuthenticateException( ]); $authRegistry = new AuthRegistry(); - $authRegistry->addService('foo', $auth); + $authRegistry->addService('foo', $auth($this)); $authenticator = $this->createSamlAuthenticator( - userProvider: $userProvider, + userProvider: $userProvider !== null ? $userProvider($this) : null, idpResolver: $idpResolver, authRegistry: $authRegistry, options: $options, - samlUserFactory: $samlUserFactory, + samlUserFactory: $samlUserFactory !== null ? $samlUserFactory($this) : null, ); $this->expectException($expectedException); @@ -544,139 +698,6 @@ public function testAuthenticateException( $authenticator->authenticate($request)->getUser(); } - public function provideAuthenticateExceptionCases(): iterable - { - yield 'SAML attributes without identifier attribute' => (function (): array { - $settingsMock = $this->createMock(Settings::class); - $settingsMock - ->method('getSecurityData') - ->willReturn([]) - ; - $auth = $this->createConfiguredMock(Auth::class, [ - 'getAttributes' => [], - 'getSessionIndex' => 'session_index', - 'getSettings' => $settingsMock, - ]); - $auth - ->expects(self::never()) - ->method('getNameId') - ; - - return [ - 'auth' => $auth, - 'userProvider' => null, - 'samlUserFactory' => null, - 'options' => [ - 'identifier_attribute' => 'username', - ], - 'expectedException' => \RuntimeException::class, - 'expectedMessage' => 'Attribute "username" not found in SAML data.', - ]; - })(); - - yield 'SAML attributes with invalid identifier attribute' => (function (): array { - $settingsMock = $this->createMock(Settings::class); - $settingsMock - ->method('getSecurityData') - ->willReturn([]) - ; - $auth = $this->createConfiguredMock(Auth::class, [ - 'getAttributes' => [ - 'username' => [], - ], - 'getSessionIndex' => 'session_index', - 'getSettings' => $settingsMock, - ]); - $auth - ->expects(self::never()) - ->method('getNameId') - ; - - return [ - 'auth' => $auth, - 'userProvider' => null, - 'samlUserFactory' => null, - 'options' => [ - 'identifier_attribute' => 'username', - ], - 'expectedException' => \RuntimeException::class, - 'expectedMessage' => 'Attribute "username" does not contain valid user identifier.', - ]; - })(); - - yield 'User not found without SAML user factory' => (function (): array { - $settingsMock = $this->createMock(Settings::class); - $settingsMock - ->method('getSecurityData') - ->willReturn([]) - ; - $auth = $this->createConfiguredMock(Auth::class, [ - 'getAttributes' => [], - 'getSessionIndex' => 'session_index', - 'getSettings' => $settingsMock, - 'getNameId' => 'tester_id', - ]); - $auth - ->expects(self::never()) - ->method('getAttributesWithFriendlyName') - ; - - $userProvider = $this->createMock(UserProviderInterface::class); - $userProvider - ->method('loadUserByIdentifier') - ->willThrowException(new UserNotFoundException()) - ; - - return [ - 'auth' => $auth, - 'userProvider' => $userProvider, - 'samlUserFactory' => null, - 'options' => [], - 'expectedException' => UserNotFoundException::class, - 'expectedMessage' => null, - ]; - })(); - - yield 'User not found + SAML user factory exception' => (function (): array { - $settingsMock = $this->createMock(Settings::class); - $settingsMock - ->method('getSecurityData') - ->willReturn([]) - ; - $auth = $this->createConfiguredMock(Auth::class, [ - 'getAttributes' => [], - 'getSessionIndex' => 'session_index', - 'getSettings' => $settingsMock, - 'getNameId' => 'tester_id', - ]); - $auth - ->expects(self::never()) - ->method('getAttributesWithFriendlyName') - ; - - $userProvider = $this->createMock(UserProviderInterface::class); - $userProvider - ->method('loadUserByIdentifier') - ->willThrowException(new UserNotFoundException()) - ; - - $samlUserFactory = $this->createMock(SamlUserFactoryInterface::class); - $samlUserFactory - ->method('createUser') - ->willThrowException(new \Exception()) - ; - - return [ - 'auth' => $auth, - 'userProvider' => $userProvider, - 'samlUserFactory' => $samlUserFactory, - 'options' => [], - 'expectedException' => AuthenticationException::class, - 'expectedMessage' => 'The authentication failed.', - ]; - })(); - } - public function testCreateToken(): void { $authenticator = $this->createSamlAuthenticator(); @@ -707,8 +728,8 @@ public function testCreateTokenWithoutSamlAttributesBadgeException(): void public function testOnAuthenticationSuccess(): void { - $request = $this->createStub(Request::class); - $token = $this->createStub(TokenInterface::class); + $request = self::createStub(Request::class); + $token = self::createStub(TokenInterface::class); $authenticationSuccessHandler = $this->createMock(AuthenticationSuccessHandlerInterface::class); $authenticationSuccessHandler @@ -726,7 +747,7 @@ public function testOnAuthenticationSuccess(): void public function testOnAuthenticationFailure(): void { - $request = $this->createStub(Request::class); + $request = self::createStub(Request::class); $exception = new AuthenticationException(); $authenticationFailureHandler = $this->createMock(AuthenticationFailureHandlerInterface::class); @@ -757,12 +778,12 @@ private function createSamlAuthenticator( bool $useProxyVars = false, ): SamlAuthenticator { return new SamlAuthenticator( - $httpUtils ?? $this->createStub(HttpUtils::class), - $userProvider ?? $this->createStub(UserProviderInterface::class), - $idpResolver ?? $this->createStub(IdpResolverInterface::class), - $authRegistry ?? $this->createStub(AuthRegistryInterface::class), - $authenticationSuccessHandler ?? $this->createStub(AuthenticationSuccessHandlerInterface::class), - $authenticationFailureHandler ?? $this->createStub(AuthenticationFailureHandlerInterface::class), + $httpUtils ?? self::createStub(HttpUtils::class), + $userProvider ?? self::createStub(UserProviderInterface::class), + $idpResolver ?? self::createStub(IdpResolverInterface::class), + $authRegistry ?? self::createStub(AuthRegistryInterface::class), + $authenticationSuccessHandler ?? self::createStub(AuthenticationSuccessHandlerInterface::class), + $authenticationFailureHandler ?? self::createStub(AuthenticationFailureHandlerInterface::class), $options, $samlUserFactory, $logger, diff --git a/tests/Security/Http/Authenticator/Token/SamlTokenTest.php b/tests/Security/Http/Authenticator/Token/SamlTokenTest.php index 104c34e..9a43ad7 100644 --- a/tests/Security/Http/Authenticator/Token/SamlTokenTest.php +++ b/tests/Security/Http/Authenticator/Token/SamlTokenTest.php @@ -7,29 +7,17 @@ use Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\Token\SamlToken; use Nbgrp\Tests\OneloginSamlBundle\TestUser; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; /** - * @covers \Nbgrp\OneloginSamlBundle\Security\Http\Authenticator\Token\SamlToken - * * @internal */ +#[CoversClass(SamlToken::class)] final class SamlTokenTest extends TestCase { - /** - * @dataProvider provideTokenCases - */ - public function testToken(array $attributes): void - { - $user = new TestUser('tester'); - $token = new SamlToken($user, 'fwname', ['ROLE_USER', 'ROLE_EXTRA'], $attributes); - - self::assertSame($token->getUserIdentifier(), 'tester'); - self::assertSame($token->getRoleNames(), ['ROLE_USER', 'ROLE_EXTRA']); - self::assertSame($token->getAttributes(), $attributes); - } - - public function provideTokenCases(): iterable + public static function provideTokenCases(): iterable { yield 'Empty attributes' => [ 'attributes' => [], @@ -42,4 +30,15 @@ public function provideTokenCases(): iterable ], ]; } + + #[DataProvider('provideTokenCases')] + public function testToken(array $attributes): void + { + $user = new TestUser('tester'); + $token = new SamlToken($user, 'fwname', ['ROLE_USER', 'ROLE_EXTRA'], $attributes); + + self::assertSame($token->getUserIdentifier(), 'tester'); + self::assertSame($token->getRoleNames(), ['ROLE_USER', 'ROLE_EXTRA']); + self::assertSame($token->getAttributes(), $attributes); + } } diff --git a/tests/Security/User/SamlUserFactoryTest.php b/tests/Security/User/SamlUserFactoryTest.php index e22ab61..c9ccfc1 100644 --- a/tests/Security/User/SamlUserFactoryTest.php +++ b/tests/Security/User/SamlUserFactoryTest.php @@ -7,13 +7,13 @@ use Nbgrp\OneloginSamlBundle\Security\User\SamlUserFactory; use Nbgrp\Tests\OneloginSamlBundle\TestUser; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; /** - * @covers \Nbgrp\OneloginSamlBundle\Security\User\SamlUserFactory - * * @internal */ +#[CoversClass(SamlUserFactory::class)] final class SamlUserFactoryTest extends TestCase { public function testCreateUser(): void diff --git a/tests/Security/User/SamlUserProviderTest.php b/tests/Security/User/SamlUserProviderTest.php index 4969ab4..d9aec6f 100644 --- a/tests/Security/User/SamlUserProviderTest.php +++ b/tests/Security/User/SamlUserProviderTest.php @@ -7,16 +7,16 @@ use Nbgrp\OneloginSamlBundle\Security\User\SamlUserProvider; use Nbgrp\Tests\OneloginSamlBundle\TestUser; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Exception\UnsupportedUserException; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\UserInterface; /** - * @covers \Nbgrp\OneloginSamlBundle\Security\User\SamlUserProvider - * * @internal */ +#[CoversClass(SamlUserProvider::class)] final class SamlUserProviderTest extends TestCase { public function testLoadUserByIdentifier(): void