diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index c9090aa..f2e92c2 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -7,21 +7,21 @@ jobs: name: Audit strategy: matrix: - php-version: [ '8.1', '8.2', '8.3' ] + php-version: [ '8.1', '8.2', '8.3', '8.4' ] fail-fast: false runs-on: ubuntu-latest steps: - name: Checkout (Push) - uses: actions/checkout@v4 + uses: actions/checkout@v5 if: github.event_name == 'push' with: fetch-depth: 0 - name: Checkout (PR) - uses: actions/checkout@v4 + uses: actions/checkout@v5 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,7 +38,7 @@ 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') }} @@ -54,13 +54,13 @@ jobs: echo "::endgroup::" - name: Auditor - uses: docker://nbgrp/auditor:0.21.0 + uses: docker://nbgrp/auditor:0.31.0 - name: Tests - run: vendor/bin/simple-phpunit + run: SYMFONY_DEPRECATIONS_HELPER="max[total]=11&max[direct]=0&max[indirect]=0&max[self]=1" vendor/bin/simple-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 1aa9117..82a491f 100644 --- a/.php-cs-fixer.php +++ b/.php-cs-fixer.php @@ -12,18 +12,18 @@ 'src/DependencyInjection/Configuration.php', 'src/DependencyInjection/Security/UserProvider/SamlUserProviderFactory.php', 'src/Resources/config/services.php', - ]) + ]), ) ->setRiskyAllowed(true) ->setRules([ // base presets - '@PER' => true, + '@PER-CS' => true, '@PhpCsFixer' => true, '@Symfony' => true, '@PHP81Migration' => true, // risky presets - '@PER:risky' => true, + '@PER-CS:risky' => true, '@PhpCsFixer:risky' => true, '@Symfony:risky' => true, '@PHP80Migration:risky' => true, @@ -31,10 +31,12 @@ // presets tuning 'blank_line_after_opening_tag' => false, 'blank_line_before_statement' => [ - 'statements' => ['case', 'default', 'return', 'throw', 'try'], + 'statements' => ['case', 'default', 'declare', 'return', 'throw', 'try'], ], 'comment_to_phpdoc' => [ 'ignored_tags' => [ + 'phan-suppress-current-line', + 'phan-suppress-next-line', 'see', 'todo', ], @@ -74,6 +76,15 @@ 'function', ], ], + 'php_unit_data_provider_static' => false, + 'php_unit_test_case_static_method_calls' => false, + '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..d5ed30f 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,7 @@ nbgrp_onelogin_saml: wantNameId: false wantNameIdEncrypted: false requestedAuthnContext: true + requestedAuthnContextComparison: 'exact' wantXMLValidation: false relaxDestinationValidation: false destinationStrictlyMatches: true diff --git a/phpcs.xml b/phpcs.xml index cd21c9f..38ccd5c 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -69,8 +69,10 @@ - + + + @@ -183,14 +185,15 @@ - - - + + + + diff --git a/phpstan.neon b/phpstan.neon index f73ea60..f2d0706 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -3,8 +3,11 @@ parameters: paths: - src - tests - bootstrapFiles: - - vendor/bin/.phpunit/phpunit/vendor/autoload.php - checkMissingIterableValueType: false - checkGenericClassInNonGenericObjectType: false + treatPhpDocTypesAsCertain: false + + ignoreErrors: + - + identifier: missingType.iterableValue + - + identifier: missingType.generics diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 85d6a0a..79bdf3a 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -9,7 +9,6 @@ - diff --git a/psalm.xml b/psalm.xml index f6e3dd9..91aaf71 100644 --- a/psalm.xml +++ b/psalm.xml @@ -1,6 +1,7 @@ + + + + + + + + diff --git a/src/Controller/Login.php b/src/Controller/Login.php index a9a0e0c..ac72a1c 100644 --- a/src/Controller/Login.php +++ b/src/Controller/Login.php @@ -47,11 +47,11 @@ public function __invoke(Request $request, Auth $auth): RedirectResponse return new RedirectResponse($this->processLoginAndGetRedirectUrl($auth, $targetPath, $session)); } - /** @psalm-suppress MixedInferredReturnType, MixedReturnStatement */ + /** @psalm-suppress MixedReturnStatement */ 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/DependencyInjection/Compiler/AuthRegistryCompilerPass.php b/src/DependencyInjection/Compiler/AuthRegistryCompilerPass.php index b6ee84f..af0a3f5 100644 --- a/src/DependencyInjection/Compiler/AuthRegistryCompilerPass.php +++ b/src/DependencyInjection/Compiler/AuthRegistryCompilerPass.php @@ -18,6 +18,7 @@ */ class AuthRegistryCompilerPass implements CompilerPassInterface { + #[\Override] public function process(ContainerBuilder $container): void { $authRegistry = $container->getDefinition(AuthRegistryInterface::class); diff --git a/src/DependencyInjection/Compiler/EntityManagerCompilerPass.php b/src/DependencyInjection/Compiler/EntityManagerCompilerPass.php index 5ba02bf..ed7e588 100644 --- a/src/DependencyInjection/Compiler/EntityManagerCompilerPass.php +++ b/src/DependencyInjection/Compiler/EntityManagerCompilerPass.php @@ -14,6 +14,7 @@ */ class EntityManagerCompilerPass implements CompilerPassInterface { + #[\Override] public function process(ContainerBuilder $container): void { if (!$container->hasParameter('nbgrp_onelogin_saml.entity_manager')) { diff --git a/src/DependencyInjection/Compiler/ProxyVarsCompilerPass.php b/src/DependencyInjection/Compiler/ProxyVarsCompilerPass.php index e55e5c1..b278c20 100644 --- a/src/DependencyInjection/Compiler/ProxyVarsCompilerPass.php +++ b/src/DependencyInjection/Compiler/ProxyVarsCompilerPass.php @@ -19,6 +19,7 @@ */ class ProxyVarsCompilerPass implements CompilerPassInterface { + #[\Override] public function process(ContainerBuilder $container): void { $useProxyVars = $container->getParameter('nbgrp_onelogin_saml.use_proxy_vars'); diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 933f2f3..ac3c94f 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -22,7 +22,7 @@ public function getConfigTreeBuilder(): TreeBuilder $rootNode = $treeBuilder->getRootNode(); // @formatter:off - /** @phpstan-ignore-next-line */ + // @phpstan-ignore-next-line $rootNode ->info('nb:group OneLogin PHP Symfony Bundle configuration') ->children() @@ -51,7 +51,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->scalarNode('binding') ->validate() - ->ifTrue(static fn ($value): bool => !str_starts_with($value, 'urn:oasis:names:tc:SAML:2.0:bindings:')) + ->ifTrue(static fn ($value): bool => !str_starts_with((string) $value, 'urn:oasis:names:tc:SAML:2.0:bindings:')) ->thenInvalid('invalid value.') ->end() ->end() @@ -63,7 +63,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->scalarNode('responseUrl')->end() ->scalarNode('binding') ->validate() - ->ifTrue(static fn ($value): bool => !str_starts_with($value, 'urn:oasis:names:tc:SAML:2.0:bindings:')) + ->ifTrue(static fn ($value): bool => !str_starts_with((string) $value, 'urn:oasis:names:tc:SAML:2.0:bindings:')) ->thenInvalid('invalid value.') ->end() ->end() @@ -100,7 +100,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->scalarNode('binding') ->validate() - ->ifTrue(static fn ($value): bool => !str_starts_with($value, 'urn:oasis:names:tc:SAML:2.0:bindings:')) + ->ifTrue(static fn ($value): bool => !str_starts_with((string) $value, 'urn:oasis:names:tc:SAML:2.0:bindings:')) ->thenInvalid('invalid value.') ->end() ->end() @@ -133,7 +133,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->scalarNode('binding') ->validate() - ->ifTrue(static fn ($value): bool => !str_starts_with($value, 'urn:oasis:names:tc:SAML:2.0:bindings:')) + ->ifTrue(static fn ($value): bool => !str_starts_with((string) $value, 'urn:oasis:names:tc:SAML:2.0:bindings:')) ->thenInvalid('invalid value.') ->end() ->end() @@ -141,7 +141,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->scalarNode('NameIDFormat') ->validate() - ->ifTrue(static fn ($value): bool => !(str_starts_with($value, 'urn:oasis:names:tc:SAML:1.1:nameid-format:') || str_starts_with($value, 'urn:oasis:names:tc:SAML:2.0:nameid-format:'))) + ->ifTrue(static fn ($value): bool => !(str_starts_with((string) $value, 'urn:oasis:names:tc:SAML:1.1:nameid-format:') || str_starts_with((string) $value, 'urn:oasis:names:tc:SAML:2.0:nameid-format:'))) ->thenInvalid('invalid value.') ->end() ->end() @@ -174,10 +174,19 @@ public function getConfigTreeBuilder(): TreeBuilder ->thenInvalid('must be an array or a boolean.') ->end() ->validate() - ->ifTrue(static fn ($value) => \is_array($value) && array_filter($value, static fn ($item): bool => !str_starts_with($item, 'urn:oasis:names:tc:SAML:2.0:ac:classes:'))) + // @phpstan-ignore-next-line + ->ifTrue(static fn ($value) => \is_array($value) && array_filter($value, static fn ($item): bool => !str_starts_with((string) $item, 'urn:oasis:names:tc:SAML:2.0:ac:classes:'))) ->thenInvalid('invalid value.') ->end() ->end() + ->enumNode('requestedAuthnContextComparison') + ->values([ + 'exact', + 'minimum', + 'maximum', + 'better', + ]) + ->end() ->booleanNode('wantXMLValidation')->end() ->booleanNode('relaxDestinationValidation')->end() ->booleanNode('destinationStrictlyMatches')->end() @@ -285,8 +294,8 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ->validate() - ->ifTrue(static fn ($value): bool => empty($value['organization'])) - ->then(static fn ($value): array => array_diff_key($value, ['organization' => null])) + ->ifTrue(static fn (array $value): bool => empty($value['organization'])) + ->then(static fn (array $value): array => array_diff_key($value, ['organization' => null])) ->end() ->end() ->end() diff --git a/src/DependencyInjection/NbgrpOneloginSamlExtension.php b/src/DependencyInjection/NbgrpOneloginSamlExtension.php index 7582d42..6de688e 100644 --- a/src/DependencyInjection/NbgrpOneloginSamlExtension.php +++ b/src/DependencyInjection/NbgrpOneloginSamlExtension.php @@ -17,6 +17,7 @@ class NbgrpOneloginSamlExtension extends Extension { /** @psalm-suppress MixedArgument */ + #[\Override] public function load(array $configs, ContainerBuilder $container): void { $loader = new PhpFileLoader($container, new FileLocator(\dirname(__DIR__).'/Resources/config')); diff --git a/src/DependencyInjection/Security/Factory/SamlFactory.php b/src/DependencyInjection/Security/Factory/SamlFactory.php index 756aec8..5b10bef 100644 --- a/src/DependencyInjection/Security/Factory/SamlFactory.php +++ b/src/DependencyInjection/Security/Factory/SamlFactory.php @@ -30,17 +30,20 @@ public function __construct() $this->addOption('success_handler', SamlAuthenticationSuccessHandler::class); } + #[\Override] public function getPriority(): int { return self::PRIORITY; } + #[\Override] public function getKey(): string { return 'saml'; } /** @psalm-suppress MixedArgument */ + #[\Override] public function createAuthenticator(ContainerBuilder $container, string $firewallName, array $config, string $userProviderId): string { $authenticatorId = 'security.authenticator.saml.'.$firewallName; diff --git a/src/DependencyInjection/Security/UserProvider/SamlUserProviderFactory.php b/src/DependencyInjection/Security/UserProvider/SamlUserProviderFactory.php index 7e7f10b..65fd2ab 100644 --- a/src/DependencyInjection/Security/UserProvider/SamlUserProviderFactory.php +++ b/src/DependencyInjection/Security/UserProvider/SamlUserProviderFactory.php @@ -14,6 +14,7 @@ class SamlUserProviderFactory implements UserProviderFactoryInterface { + #[\Override] public function create(ContainerBuilder $container, string $id, array $config): void { $container @@ -23,6 +24,7 @@ public function create(ContainerBuilder $container, string $id, array $config): ; } + #[\Override] public function getKey(): string { return 'saml'; @@ -31,17 +33,18 @@ public function getKey(): string /** * @suppress PhanUndeclaredMethod */ + #[\Override] public function addConfiguration(NodeDefinition $builder): void { // @formatter:off - /** @phpstan-ignore-next-line */ + // @phpstan-ignore-next-line $builder ->children() ->scalarNode('user_class') ->isRequired() ->cannotBeEmpty() ->validate() - ->ifTrue(static fn ($value) => !is_a($value, UserInterface::class, true)) + ->ifTrue(static fn ($value) => !is_a((string) $value, UserInterface::class, true)) ->thenInvalid('You should provide user class implementing '.UserInterface::class.' interface.') ->end() ->end() diff --git a/src/EventListener/Security/SamlLogoutListener.php b/src/EventListener/Security/SamlLogoutListener.php index abc8dbe..8e19b7d 100644 --- a/src/EventListener/Security/SamlLogoutListener.php +++ b/src/EventListener/Security/SamlLogoutListener.php @@ -33,27 +33,30 @@ public function processSingleLogout(LogoutEvent $event): void } $authService = $this->getAuthService($event->getRequest()); - if (!$authService) { + if ($authService === null) { return; } 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..c3bd656 100644 --- a/src/Idp/IdpResolver.php +++ b/src/Idp/IdpResolver.php @@ -13,6 +13,7 @@ public function __construct( private readonly string $idpParameterName, ) {} + #[\Override] public function resolve(Request $request): ?string { if ($request->query->has($this->idpParameterName)) { diff --git a/src/NbgrpOneloginSamlBundle.php b/src/NbgrpOneloginSamlBundle.php index 0f4b1e6..be78c7c 100644 --- a/src/NbgrpOneloginSamlBundle.php +++ b/src/NbgrpOneloginSamlBundle.php @@ -18,6 +18,7 @@ */ class NbgrpOneloginSamlBundle extends Bundle { + #[\Override] public function build(ContainerBuilder $container): void { parent::build($container); diff --git a/src/Onelogin/AuthArgumentResolver.php b/src/Onelogin/AuthArgumentResolver.php index 52aafde..86bfe17 100644 --- a/src/Onelogin/AuthArgumentResolver.php +++ b/src/Onelogin/AuthArgumentResolver.php @@ -24,6 +24,7 @@ public function __construct( private readonly IdpResolverInterface $idpResolver, ) {} + #[\Override] public function resolve(Request $request, ArgumentMetadata $argument): iterable { if ($argument->getType() !== Auth::class) { @@ -31,12 +32,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/Onelogin/AuthRegistry.php b/src/Onelogin/AuthRegistry.php index ed171a3..57ffbe2 100644 --- a/src/Onelogin/AuthRegistry.php +++ b/src/Onelogin/AuthRegistry.php @@ -14,6 +14,7 @@ final class AuthRegistry implements AuthRegistryInterface */ private array $services = []; + #[\Override] public function addService(string $key, Auth $auth): self { if (\array_key_exists($key, $this->services)) { @@ -25,16 +26,19 @@ public function addService(string $key, Auth $auth): self return $this; } + #[\Override] public function hasService(string $key): bool { return \array_key_exists($key, $this->services); } + #[\Override] public function getService(string $key): Auth { return $this->services[$key] ?? throw new \OutOfBoundsException('Auth service for key "'.$key.'" does not exists.'); } + #[\Override] public function getDefaultService(): Auth { if (empty($this->services)) { diff --git a/src/Security/Http/Authentication/SamlAuthenticationSuccessHandler.php b/src/Security/Http/Authentication/SamlAuthenticationSuccessHandler.php index 9209907..d11e6e6 100644 --- a/src/Security/Http/Authentication/SamlAuthenticationSuccessHandler.php +++ b/src/Security/Http/Authentication/SamlAuthenticationSuccessHandler.php @@ -18,6 +18,7 @@ class SamlAuthenticationSuccessHandler extends DefaultAuthenticationSuccessHandl public const RELAY_STATE = 'RelayState'; /** @psalm-suppress MixedArrayAccess */ + #[\Override] protected function determineTargetUrl(Request $request): string { if ($this->options['always_use_default_target_path']) { diff --git a/src/Security/Http/Authenticator/Passport/Badge/DeferredEventBadge.php b/src/Security/Http/Authenticator/Passport/Badge/DeferredEventBadge.php index 076d674..8058904 100644 --- a/src/Security/Http/Authenticator/Passport/Badge/DeferredEventBadge.php +++ b/src/Security/Http/Authenticator/Passport/Badge/DeferredEventBadge.php @@ -30,6 +30,7 @@ public function getEvent(): ?Event } } + #[\Override] public function isResolved(): bool { return $this->resolved; diff --git a/src/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadge.php b/src/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadge.php index 5148024..4ba9c2b 100644 --- a/src/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadge.php +++ b/src/Security/Http/Authenticator/Passport/Badge/SamlAttributesBadge.php @@ -21,6 +21,7 @@ public function getAttributes(): array return $this->attributes; } + #[\Override] public function isResolved(): bool { return true; diff --git a/src/Security/Http/Authenticator/SamlAuthenticator.php b/src/Security/Http/Authenticator/SamlAuthenticator.php index 83772c3..41199d1 100644 --- a/src/Security/Http/Authenticator/SamlAuthenticator.php +++ b/src/Security/Http/Authenticator/SamlAuthenticator.php @@ -58,23 +58,26 @@ public function __construct( private readonly bool $useProxyVars, ) {} + #[\Override] public function supports(Request $request): ?bool { return $request->isMethod('POST') && $this->httpUtils->checkRequestPath($request, (string) $this->options['check_path']); } + #[\Override] public function start(Request $request, ?AuthenticationException $authException = null): Response { $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; } return new RedirectResponse($uri); } + #[\Override] public function authenticate(Request $request): Passport { if (!$request->hasSession()) { @@ -96,10 +99,11 @@ public function authenticate(Request $request): Passport return $this->createPassport($oneLoginAuth); } + #[\Override] public function createToken(Passport $passport, string $firewallName): TokenInterface { if (!$passport->hasBadge(SamlAttributesBadge::class)) { - throw new LogicException(sprintf('Passport should contains a "%s" badge.', SamlAttributesBadge::class)); + throw new LogicException(\sprintf('Passport should contains a "%s" badge.', SamlAttributesBadge::class)); } $badge = $passport->getBadge(SamlAttributesBadge::class); @@ -112,11 +116,13 @@ public function createToken(Passport $passport, string $firewallName): TokenInte return new SamlToken($passport->getUser(), $firewallName, $passport->getUser()->getRoles(), $attributes); } + #[\Override] public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response { return $this->successHandler->onAuthenticationSuccess($request, $token); } + #[\Override] public function onAuthenticationFailure(Request $request, AuthenticationException $exception): ?Response { return $this->failureHandler->onAuthenticationFailure($request, $exception); @@ -126,7 +132,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 +185,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 +221,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..2f84ace 100644 --- a/src/Security/User/SamlUserFactory.php +++ b/src/Security/User/SamlUserFactory.php @@ -18,6 +18,7 @@ public function __construct( private readonly array $mapping, ) {} + #[\Override] public function createUser(string $identifier, array $attributes): UserInterface { $user = new $this->userClass($identifier); diff --git a/src/Security/User/SamlUserProvider.php b/src/Security/User/SamlUserProvider.php index bd1dfbb..ffa9813 100644 --- a/src/Security/User/SamlUserProvider.php +++ b/src/Security/User/SamlUserProvider.php @@ -12,7 +12,7 @@ /** * Just instantiates user object with providing identifier and default roles. * - * @template-covariant TUser of UserInterface + * @template TUser of UserInterface * * @template-implements UserProviderInterface */ @@ -30,11 +30,13 @@ public function __construct( } } + #[\Override] public function loadUserByIdentifier(string $identifier): UserInterface { return new $this->userClass($identifier, $this->defaultRoles); } + #[\Override] public function refreshUser(UserInterface $user): UserInterface { if (!$user instanceof $this->userClass) { @@ -44,6 +46,7 @@ public function refreshUser(UserInterface $user): UserInterface return $user; } + #[\Override] public function supportsClass(string $class): bool { return is_a($class, $this->userClass, true); diff --git a/tests/Controller/LoginTest.php b/tests/Controller/LoginTest.php index d8b1a7e..e3c44a5 100644 --- a/tests/Controller/LoginTest.php +++ b/tests/Controller/LoginTest.php @@ -150,30 +150,6 @@ public function provideErrorExceptionCases(): iterable ]; } - 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); diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index bbeaf91..f9a9050 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -125,6 +125,7 @@ public function provideValidConfigCases(): iterable 'wantNameId' => false, 'wantNameIdEncrypted' => false, 'requestedAuthnContext' => false, + 'requestedAuthnContextComparison' => 'exact', 'wantXMLValidation' => false, 'relaxDestinationValidation' => false, 'destinationStrictlyMatches' => false, @@ -241,6 +242,7 @@ public function provideValidConfigCases(): iterable 'wantNameId' => false, 'wantNameIdEncrypted' => false, 'requestedAuthnContext' => false, + 'requestedAuthnContextComparison' => 'exact', 'wantXMLValidation' => false, 'relaxDestinationValidation' => false, 'destinationStrictlyMatches' => false,