diff --git a/config/module_oidc.php.dist b/config/module_oidc.php.dist index b68d2491..30489c86 100644 --- a/config/module_oidc.php.dist +++ b/config/module_oidc.php.dist @@ -232,6 +232,25 @@ $config = [ * claim in the attribute-to-claim translation table (you will probably want * to use this attribute as the 'sub' claim since it designates a unique * identifier for the user). + * + * This option can be a single attribute name (string), or an array of + * prioritized attribute names. The array form is useful in scenarios with + * multiple heterogeneous IdPs (e.g., eduGAIN inter-federation), where not + * every IdP is able (or willing) to release the same identifier attribute. + * When an array is given, the attributes are consulted in order. The + * first one that is actually present in the released attributes is used, + * both as the internal user identifier and as the default source for the + * 'sub' claim. + * + * Single value example: + * ModuleConfig::OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE => 'uid', + * + * Prioritized list example: + * ModuleConfig::OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE => [ + * 'eduPersonPrincipalName', + * 'eduPersonUniqueId', + * 'uid', + * ], */ ModuleConfig::OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE => 'uid', diff --git a/docs/6-oidc-upgrade.md b/docs/6-oidc-upgrade.md index c04106a5..71f3db71 100644 --- a/docs/6-oidc-upgrade.md +++ b/docs/6-oidc-upgrade.md @@ -47,6 +47,8 @@ Federation) and applies the matching signing rules; when present, the `aud` and - Require Pushed Authorization Requests (`require_pushed_authorization_requests`) - Require Signed Request Object (`require_signed_request_object`) - Registered Request URIs (`request_uris`) +See the [configuration guide](3-oidc-configuration.md#pushed-authorization-requests-par-and-request-objects) +for details. - Support for the OAuth 2.0 Form Post Response Mode (`response_mode=form_post`). The OP now supports three response modes - `query`, `fragment`, and `form_post`. With `form_post`, the authorization response parameters are @@ -87,9 +89,16 @@ performance. Note that changing the encryption key (including switching from the secret salt to a Key, or rotating a Key) invalidates all outstanding encrypted artifacts (existing authorization codes, refresh tokens, and PAR request URIs will be rejected), so only set or change it during a planned maintenance window. - -See the [configuration guide](3-oidc-configuration.md#pushed-authorization-requests-par-and-request-objects) -for details. +- The user identifier attribute option +(`ModuleConfig::OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE`, `useridattr`) can now be +configured either as a single attribute name (string, as before) or as an array +of prioritized attribute names. This is useful in scenarios with multiple +heterogeneous IdPs (for example, eduGAIN inter-federation), where not every IdP +is able (or willing) to release the same identifier attribute. When an array is +given, the attributes are consulted in priority order and the first one actually +present in the released attributes is used, both as the internal user identifier +and as the default source for the `sub` claim. The single-string form continues +to work unchanged, so existing configurations are unaffected. New configuration options: diff --git a/routing/services/services.yml b/routing/services/services.yml index cc6bd158..16e0d221 100644 --- a/routing/services/services.yml +++ b/routing/services/services.yml @@ -103,6 +103,7 @@ services: SimpleSAML\Module\oidc\Utils\FederationParticipationValidator: ~ SimpleSAML\Module\oidc\Utils\Routes: ~ SimpleSAML\Module\oidc\Utils\RequestParamsResolver: ~ + SimpleSAML\Module\oidc\Utils\UserIdentifierResolver: ~ SimpleSAML\Module\oidc\Utils\ClassInstanceBuilder: ~ SimpleSAML\Module\oidc\Utils\JwksResolver: ~ SimpleSAML\Module\oidc\Utils\AuthenticatedOAuth2ClientResolver: ~ diff --git a/src/Factories/ClaimTranslatorExtractorFactory.php b/src/Factories/ClaimTranslatorExtractorFactory.php index f8f06388..cbe848c8 100644 --- a/src/Factories/ClaimTranslatorExtractorFactory.php +++ b/src/Factories/ClaimTranslatorExtractorFactory.php @@ -67,10 +67,10 @@ public function build(): ClaimTranslatorExtractor } } - $userIdAttr = $this->moduleConfig->getUserIdentifierAttribute(); + $userIdAttrs = $this->moduleConfig->getUserIdentifierAttributes(); return new ClaimTranslatorExtractor( - $userIdAttr, + $userIdAttrs, $this->claimSetEntityFactory, $claimSet, $translatorTable, diff --git a/src/Factories/CredentialOfferUriFactory.php b/src/Factories/CredentialOfferUriFactory.php index e052ba97..c8e572c7 100644 --- a/src/Factories/CredentialOfferUriFactory.php +++ b/src/Factories/CredentialOfferUriFactory.php @@ -21,6 +21,7 @@ use SimpleSAML\Module\oidc\Repositories\IssuerStateRepository; use SimpleSAML\Module\oidc\Repositories\UserRepository; use SimpleSAML\Module\oidc\Services\LoggerService; +use SimpleSAML\Module\oidc\Utils\UserIdentifierResolver; use SimpleSAML\OpenID\Codebooks\ClaimsEnum; use SimpleSAML\OpenID\Codebooks\GrantTypesEnum; use SimpleSAML\OpenID\Exceptions\OpenIdException; @@ -42,6 +43,7 @@ public function __construct( protected readonly EmailFactory $emailFactory, protected readonly IssuerStateEntityFactory $issuerStateEntityFactory, protected readonly IssuerStateRepository $issuerStateRepository, + protected readonly UserIdentifierResolver $userIdentifierResolver, ) { } @@ -136,16 +138,14 @@ public function buildPreAuthorized( $userId = null; try { - /** @psalm-suppress MixedAssignment */ - $userId = $this->sspBridge->utils()->attributes()->getExpectedAttribute( + $userId = $this->userIdentifierResolver->resolve( + $this->moduleConfig->getUserIdentifierAttributes(), $userAttributes, - $this->moduleConfig->getUserIdentifierAttribute(), ); - if (!is_scalar($userId)) { - throw new RuntimeException('User identifier attribute value is not a string.'); + if ($userId === null) { + throw new RuntimeException('User identifier attribute value is not available.'); } - $userId = strval($userId); } catch (\Throwable $e) { $this->loggerService->warning( 'Could not extract user identifier from user attributes: ' . $e->getMessage(), diff --git a/src/ModuleConfig.php b/src/ModuleConfig.php index c4362192..e8fb2b39 100644 --- a/src/ModuleConfig.php +++ b/src/ModuleConfig.php @@ -409,11 +409,36 @@ public function getDefaultAuthSourceId(): string } /** + * Get the ordered list of candidate user identifier attributes. + * + * The option may be configured either as a single string (legacy) or as an + * array of prioritized attribute names. In heterogeneous IdP scenarios (e.g. + * eduGAIN inter-federation) not every IdP releases the same identifier, so + * the list is consulted in order and the first attribute that is actually + * present in the released attributes is used. + * + * @return string[] * @throws \Exception */ + public function getUserIdentifierAttributes(): array + { + $value = $this->config()->getOptionalArrayizeString( + ModuleConfig::OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE, + ['uid'], + ); + + return array_values(array_filter($value, 'is_string')); + } + + /** + * Returns the primary (first) configured user ID candidate. + * @throws \SimpleSAML\Error\ConfigurationError + * @deprecated Use getUserIdentifierAttributes(). + */ public function getUserIdentifierAttribute(): string { - return $this->config()->getString(ModuleConfig::OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE); + return $this->getUserIdentifierAttributes()[0] + ?? throw new ConfigurationError('No user identifier attribute configured.'); } public function getSupportedAlgorithms(): SupportedAlgorithms diff --git a/src/Services/AuthContextService.php b/src/Services/AuthContextService.php index dd482a3f..ce3b0056 100644 --- a/src/Services/AuthContextService.php +++ b/src/Services/AuthContextService.php @@ -6,9 +6,9 @@ use RuntimeException; use SimpleSAML\Auth\Simple; -use SimpleSAML\Module\oidc\Bridges\SspBridge; use SimpleSAML\Module\oidc\Factories\AuthSimpleFactory; use SimpleSAML\Module\oidc\ModuleConfig; +use SimpleSAML\Module\oidc\Utils\UserIdentifierResolver; /** * Provide contextual authentication information for administration interface. @@ -28,7 +28,7 @@ class AuthContextService public function __construct( private readonly ModuleConfig $moduleConfig, private readonly AuthSimpleFactory $authSimpleFactory, - private readonly SspBridge $sspBridge, + private readonly UserIdentifierResolver $userIdentifierResolver, ) { } @@ -39,11 +39,17 @@ public function __construct( public function getAuthUserId(): string { $simple = $this->authenticate(); - $userIdAttr = $this->moduleConfig->getUserIdentifierAttribute(); - return (string)$this->sspBridge->utils()->attributes()->getExpectedAttribute( - $simple->getAttributes(), - $userIdAttr, - ); + $userIdAttrs = $this->moduleConfig->getUserIdentifierAttributes(); + $userId = $this->userIdentifierResolver->resolve($userIdAttrs, $simple->getAttributes()); + + if ($userId === null) { + throw new RuntimeException(sprintf( + 'None of the configured user identifier attributes (%s) are available.', + implode(', ', $userIdAttrs), + )); + } + + return $userId; } /** diff --git a/src/Services/AuthenticationService.php b/src/Services/AuthenticationService.php index 73322b7b..b4061577 100644 --- a/src/Services/AuthenticationService.php +++ b/src/Services/AuthenticationService.php @@ -42,6 +42,7 @@ use SimpleSAML\Module\oidc\Utils\ClaimTranslatorExtractor; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; use SimpleSAML\Module\oidc\Utils\Routes; +use SimpleSAML\Module\oidc\Utils\UserIdentifierResolver; class AuthenticationService { @@ -51,9 +52,10 @@ class AuthenticationService private ?string $authSourceId = null; /** - * @var string + * Ordered list of candidate user identifier attributes. + * @var string[] */ - private string $userIdAttr; + private array $userIdAttrs; /** * @throws \Exception @@ -71,8 +73,9 @@ public function __construct( private readonly RequestParamsResolver $requestParamsResolver, private readonly UserEntityFactory $userEntityFactory, private readonly Routes $routes, + private readonly UserIdentifierResolver $userIdentifierResolver, ) { - $this->userIdAttr = $this->moduleConfig->getUserIdentifierAttribute(); + $this->userIdAttrs = $this->moduleConfig->getUserIdentifierAttributes(); } /** @@ -136,20 +139,19 @@ public function getAuthenticateUser( $claims = $state['Attributes']; - if (!array_key_exists($this->userIdAttr, $claims) || !is_array($claims[$this->userIdAttr])) { - $attr = implode(', ', array_keys($claims)); + $userId = $this->userIdentifierResolver->resolve($this->userIdAttrs, $claims); + + if ($userId === null) { throw new Error\Exception( sprintf( - 'User identifier attribute `%s` does not exist in the user attribute state.' . + 'None of the configured user identifier attributes (%s) exist in the user attribute state.' . ' Available attributes are: %s.', - $this->userIdAttr, - $attr, + implode(', ', $this->userIdAttrs), + implode(', ', array_keys($claims)), ), ); } - $userId = (string)$claims[$this->userIdAttr][0]; - $user = $this->userRepository->getUserEntityByIdentifier($userId); if ($user) { diff --git a/src/Utils/ClaimTranslatorExtractor.php b/src/Utils/ClaimTranslatorExtractor.php index 5e1ac974..3a35d206 100644 --- a/src/Utils/ClaimTranslatorExtractor.php +++ b/src/Utils/ClaimTranslatorExtractor.php @@ -156,19 +156,25 @@ class ClaimTranslatorExtractor /** * ClaimTranslatorExtractor constructor. * + * @param string[] $userIdAttrs Ordered list of candidate user identifier attributes. * @param \SimpleSAML\Module\oidc\Entities\Interfaces\ClaimSetEntityInterface[] $claimSets * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException */ public function __construct( - string $userIdAttr, + array $userIdAttrs, protected readonly ClaimSetEntityFactory $claimSetEntityFactory, array $claimSets = [], array $translationTable = [], protected array $allowedMultiValueClaims = [], ) { - // By default, add the userIdAttribute as one of the attribute for 'sub' claim. - /** @psalm-suppress MixedArgument */ - array_unshift($this->translationTable['sub'], $userIdAttr); + // By default, add the user identifier attribute(s) as attributes for the + // 'sub' claim, preserving the configured priority order (the translation + // resolves to the first present attribute). array_reverse keeps the + // configured order after successive array_unshift() prepends. + foreach (array_reverse($userIdAttrs) as $userIdAttr) { + /** @psalm-suppress MixedArgument */ + array_unshift($this->translationTable['sub'], $userIdAttr); + } $this->translationTable = array_merge($this->translationTable, $translationTable); diff --git a/src/Utils/UserIdentifierResolver.php b/src/Utils/UserIdentifierResolver.php new file mode 100644 index 00000000..ee7eb53c --- /dev/null +++ b/src/Utils/UserIdentifierResolver.php @@ -0,0 +1,56 @@ + $attributes Released attributes (each value an array of values). + * @return string|null The first resolved identifier value, or null if none of the candidates match. + */ + public function resolve(array $candidates, array $attributes): ?string + { + foreach ($candidates as $candidate) { + if ( + !array_key_exists($candidate, $attributes) || + !is_array($attributes[$candidate]) || + $attributes[$candidate] === [] + ) { + continue; + } + + /** @psalm-suppress MixedAssignment */ + $value = reset($attributes[$candidate]); + + if (is_scalar($value) && (string)$value !== '') { + return (string)$value; + } + } + + return null; + } +} diff --git a/templates/config/protocol.twig b/templates/config/protocol.twig index 79db95df..11762b98 100644 --- a/templates/config/protocol.twig +++ b/templates/config/protocol.twig @@ -44,7 +44,7 @@

{{ 'Default Authentication Source'|trans }}: {{ moduleConfig.getDefaultAuthSourceId }}
- {{ 'User Identifier Attribute'|trans }}: {{ moduleConfig.getUserIdentifierAttribute }} + {{ 'User Identifier Attribute'|trans }}: {{ moduleConfig.getUserIdentifierAttributes|join(', ') }}

{{ 'Authentication Processing Filters'|trans }}: diff --git a/tests/unit/src/ModuleConfigTest.php b/tests/unit/src/ModuleConfigTest.php index 0c396925..a8cc8870 100644 --- a/tests/unit/src/ModuleConfigTest.php +++ b/tests/unit/src/ModuleConfigTest.php @@ -236,6 +236,20 @@ public function testCanGetUserIdentifierAttribute(): void $this->assertEquals('sample', $this->sut()->getUserIdentifierAttribute()); } + public function testCanGetUserIdentifierAttributesFromString(): void + { + $this->overrides[ModuleConfig::OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE] = 'sample'; + $this->assertEquals(['sample'], $this->sut()->getUserIdentifierAttributes()); + } + + public function testCanGetUserIdentifierAttributesFromArray(): void + { + $this->overrides[ModuleConfig::OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE] = ['ePPN', 'uid']; + $this->assertEquals(['ePPN', 'uid'], $this->sut()->getUserIdentifierAttributes()); + // The deprecated single accessor returns the primary (first) candidate. + $this->assertEquals('ePPN', $this->sut()->getUserIdentifierAttribute()); + } + public function testCanGetCommonFederationOptions(): void { $this->assertFalse($this->sut()->getFederationEnabled()); diff --git a/tests/unit/src/Server/RequestRules/Rules/RequestedClaimsRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/RequestedClaimsRuleTest.php index 33caca10..5f7561ec 100644 --- a/tests/unit/src/Server/RequestRules/Rules/RequestedClaimsRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/RequestedClaimsRuleTest.php @@ -30,7 +30,8 @@ class RequestedClaimsRuleTest extends TestCase protected Stub $requestStub; protected string $redirectUri = 'https://some-redirect-uri.org'; protected Stub $loggerServiceStub; - protected static string $userIdAttr = 'uid'; + /** @var string[] */ + protected static array $userIdAttrs = ['uid']; protected Stub $requestParamsResolverStub; protected Stub $claimSetEntityFactoryStub; protected Helpers $helpers; @@ -69,7 +70,10 @@ protected function sut( ): RequestedClaimsRule { $requestParamsResolver ??= $this->requestParamsResolverStub; $helpers ??= $this->helpers; - $claimTranslatorExtractor ??= new ClaimTranslatorExtractor(self::$userIdAttr, $this->claimSetEntityFactoryStub); + $claimTranslatorExtractor ??= new ClaimTranslatorExtractor( + self::$userIdAttrs, + $this->claimSetEntityFactoryStub, + ); return new RequestedClaimsRule( $requestParamsResolver, diff --git a/tests/unit/src/Server/ResponseTypes/TokenResponseTest.php b/tests/unit/src/Server/ResponseTypes/TokenResponseTest.php index cf9a1b24..79058fca 100644 --- a/tests/unit/src/Server/ResponseTypes/TokenResponseTest.php +++ b/tests/unit/src/Server/ResponseTypes/TokenResponseTest.php @@ -113,7 +113,7 @@ protected function setUp(): void $this->coreMock->method('idTokenFactory')->willReturn($this->idTokenFactoryMock); $this->idTokenBuilder = new IdTokenBuilder( - new ClaimTranslatorExtractor(self::USER_ID_ATTR, $this->claimSetEntityFactoryStub), + new ClaimTranslatorExtractor([self::USER_ID_ATTR], $this->claimSetEntityFactoryStub), $this->coreMock, $this->moduleConfigMock, ); diff --git a/tests/unit/src/Services/AuthContextServiceTest.php b/tests/unit/src/Services/AuthContextServiceTest.php index 0fcf54fb..8cc7f36b 100644 --- a/tests/unit/src/Services/AuthContextServiceTest.php +++ b/tests/unit/src/Services/AuthContextServiceTest.php @@ -9,12 +9,10 @@ use RuntimeException; use SimpleSAML\Auth\Simple; use SimpleSAML\Configuration; -use SimpleSAML\Error\Exception; -use SimpleSAML\Module\oidc\Bridges\SspBridge; use SimpleSAML\Module\oidc\Factories\AuthSimpleFactory; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Services\AuthContextService; -use SimpleSAML\Utils\Attributes; +use SimpleSAML\Module\oidc\Utils\UserIdentifierResolver; /** * @covers \SimpleSAML\Module\oidc\Services\AuthContextService @@ -30,9 +28,6 @@ class AuthContextServiceTest extends TestCase protected MockObject $moduleConfigMock; protected MockObject $authSimpleService; protected MockObject $authSimpleFactory; - protected MockObject $sspBridgeMock; - protected MockObject $sspBridgeUtilsMock; - protected MockObject $sspBridgeUtilsAttributesMock; /** * @throws \PHPUnit\Framework\MockObject\Exception @@ -57,27 +52,21 @@ protected function setUp(): void $this->authSimpleFactory = $this->createMock(AuthSimpleFactory::class); $this->authSimpleFactory->method('getDefaultAuthSource')->willReturn($this->authSimpleService); - - $this->sspBridgeMock = $this->createMock(SspBridge::class); - $this->sspBridgeUtilsMock = $this->createMock(SspBridge\Utils::class); - $this->sspBridgeMock->method('utils')->willReturn($this->sspBridgeUtilsMock); - $this->sspBridgeUtilsAttributesMock = $this->createMock(Attributes::class); - $this->sspBridgeUtilsMock->method('attributes')->willReturn($this->sspBridgeUtilsAttributesMock); } protected function sut( ?ModuleConfig $moduleConfig = null, ?AuthSimpleFactory $authSimpleFactory = null, - ?SspBridge $sspBridge = null, + ?UserIdentifierResolver $userIdentifierResolver = null, ): AuthContextService { $moduleConfig ??= $this->moduleConfigMock; $authSimpleFactory ??= $this->authSimpleFactory; - $sspBridge ??= $this->sspBridgeMock; + $userIdentifierResolver ??= new UserIdentifierResolver(); return new AuthContextService( $moduleConfig, $authSimpleFactory, - $sspBridge, + $userIdentifierResolver, ); } @@ -94,14 +83,8 @@ public function testItIsInitializable(): void */ public function testItReturnsUsername(): void { - $this->moduleConfigMock->method('getUserIdentifierAttribute')->willReturn('idAttribute'); + $this->moduleConfigMock->method('getUserIdentifierAttributes')->willReturn(['idAttribute']); $this->authSimpleService->method('getAttributes')->willReturn(self::AUTHORIZED_USER); - $this->sspBridgeUtilsAttributesMock->expects($this->once())->method('getExpectedAttribute') - ->with( - self::AUTHORIZED_USER, - 'idAttribute', - ) - ->willReturn(self::AUTHORIZED_USER['idAttribute'][0]); $this->assertSame( $this->sut()->getAuthUserId(), @@ -109,21 +92,24 @@ public function testItReturnsUsername(): void ); } - public function testItThrowsWhenNoUsername(): void + public function testItRespectsCandidatePriority(): void { - $this->oidcConfigurationMock->method('getOptionalConfigItem') - ->with(ModuleConfig::OPTION_ADMIN_UI_PERMISSIONS, null) - ->willReturn($this->permissions); - $this->oidcConfigurationMock->method('getString') - ->with(ModuleConfig::OPTION_AUTH_USER_IDENTIFIER_ATTRIBUTE) - ->willReturn('attributeNotSet'); + $this->moduleConfigMock->method('getUserIdentifierAttributes') + ->willReturn(['nonExisting', 'idAttribute']); $this->authSimpleService->method('getAttributes')->willReturn(self::AUTHORIZED_USER); - $this->sspBridgeUtilsAttributesMock->expects($this->once())->method('getExpectedAttribute') - ->with(self::AUTHORIZED_USER) - ->willThrowException(new Exception('error')); + $this->assertSame( + 'myUsername', + $this->sut()->getAuthUserId(), + ); + } - $this->expectException(Exception::class); + public function testItThrowsWhenNoUsername(): void + { + $this->moduleConfigMock->method('getUserIdentifierAttributes')->willReturn(['attributeNotSet']); + $this->authSimpleService->method('getAttributes')->willReturn(self::AUTHORIZED_USER); + + $this->expectException(RuntimeException::class); $this->sut()->getAuthUserId(); } diff --git a/tests/unit/src/Services/AuthenticationServiceTest.php b/tests/unit/src/Services/AuthenticationServiceTest.php index e22e7ee1..a6538d9c 100644 --- a/tests/unit/src/Services/AuthenticationServiceTest.php +++ b/tests/unit/src/Services/AuthenticationServiceTest.php @@ -35,6 +35,7 @@ use SimpleSAML\Module\oidc\Utils\ClaimTranslatorExtractor; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; use SimpleSAML\Module\oidc\Utils\Routes; +use SimpleSAML\Module\oidc\Utils\UserIdentifierResolver; use SimpleSAML\Session; /** @@ -114,7 +115,7 @@ protected function setUp(): void $this->authSimpleMock->method('getAuthDataArray')->willReturn(self::AUTH_DATA); $this->clientEntityMock->method('getAuthSourceId')->willReturn(self::AUTH_SOURCE); $this->clientEntityMock->method('toArray')->willReturn(self::CLIENT_ENTITY); - $this->moduleConfigMock->method('getUserIdentifierAttribute')->willReturn(self::USER_ID_ATTR); + $this->moduleConfigMock->method('getUserIdentifierAttributes')->willReturn([self::USER_ID_ATTR]); $this->opMetadataService->method('getMetadata')->willReturn(self::OIDC_OP_METADATA); $this->processingChainFactoryMock->method('build')->willReturn($this->processingChainMock); $this->serverRequestMock->method('getQueryParams')->willReturn(self::AUTHZ_REQUEST_PARAMS); @@ -154,6 +155,7 @@ public function mock(): AuthenticationService $this->requestParamsResolverMock, $this->userEntityFactoryMock, $this->routesMock, + new UserIdentifierResolver(), ], )->onlyMethods([]) ->getMock(); @@ -319,7 +321,7 @@ public function testGetAuthenticateUserItThrowsIfClaimsNotExist(): void unset($invalidState['Attributes'][self::USER_ID_ATTR]); $this->expectException(Exception::class); - $this->expectExceptionMessageMatches("/User identifier attribute /"); + $this->expectExceptionMessageMatches("/None of the configured user identifier attributes/"); $this->mock()->getAuthenticateUser($invalidState); } @@ -405,6 +407,7 @@ public function testItProcessesRequest(bool $isAuthnPer): void $this->requestParamsResolverMock, $this->userEntityFactoryMock, $this->routesMock, + new UserIdentifierResolver(), ]) ->onlyMethods(['runAuthProcs', 'prepareStateArray']) ->getMock(); @@ -496,6 +499,7 @@ public function testItRunAuthProcs(): void $this->requestParamsResolverMock, $this->userEntityFactoryMock, $this->routesMock, + new UserIdentifierResolver(), ) extends AuthenticationService { public function runAuthProcsPublic(array &$state): void { @@ -542,6 +546,7 @@ public function testRunAuthProcsAppliesPerClientAuthProcFilters(): void $this->requestParamsResolverMock, $this->userEntityFactoryMock, $this->routesMock, + new UserIdentifierResolver(), ) extends AuthenticationService { public function runAuthProcsPublic(array &$state): void { diff --git a/tests/unit/src/Utils/ClaimTranslatorExtractorTest.php b/tests/unit/src/Utils/ClaimTranslatorExtractorTest.php index d7f54b54..60ec77fe 100644 --- a/tests/unit/src/Utils/ClaimTranslatorExtractorTest.php +++ b/tests/unit/src/Utils/ClaimTranslatorExtractorTest.php @@ -16,7 +16,8 @@ */ class ClaimTranslatorExtractorTest extends TestCase { - protected static string $userIdAttr = 'uid'; + /** @var string[] */ + protected static array $userIdAttrs = ['uid']; protected Stub $claimSetEntityFactoryStub; protected function setUp(): void @@ -39,7 +40,7 @@ protected function mock( array $allowedMultiValueClaims = [], ): ClaimTranslatorExtractor { return new ClaimTranslatorExtractor( - self::$userIdAttr, + self::$userIdAttrs, $this->claimSetEntityFactoryStub, $claimSets, $translationTable, diff --git a/tests/unit/src/Utils/UserIdentifierResolverTest.php b/tests/unit/src/Utils/UserIdentifierResolverTest.php new file mode 100644 index 00000000..5d39d0bb --- /dev/null +++ b/tests/unit/src/Utils/UserIdentifierResolverTest.php @@ -0,0 +1,83 @@ +assertSame( + 'user-1', + $this->sut()->resolve(['uid'], ['uid' => ['user-1']]), + ); + } + + public function testRespectsCandidatePriority(): void + { + $attributes = [ + 'uid' => ['fallback'], + 'eduPersonPrincipalName' => ['preferred'], + ]; + + $this->assertSame( + 'preferred', + $this->sut()->resolve(['eduPersonPrincipalName', 'uid'], $attributes), + ); + } + + public function testFallsBackToLaterCandidateWhenEarlierMissing(): void + { + $this->assertSame( + 'fallback', + $this->sut()->resolve(['eduPersonPrincipalName', 'uid'], ['uid' => ['fallback']]), + ); + } + + public function testSkipsCandidatesWithEmptyValue(): void + { + $attributes = [ + 'eduPersonPrincipalName' => [''], + 'uid' => ['present'], + ]; + + $this->assertSame( + 'present', + $this->sut()->resolve(['eduPersonPrincipalName', 'uid'], $attributes), + ); + } + + public function testReturnsNullWhenNoCandidateMatches(): void + { + $this->assertNull( + $this->sut()->resolve(['eduPersonPrincipalName', 'uid'], ['mail' => ['user@example.org']]), + ); + } + + public function testReturnsNullForNonArrayOrEmptyAttributeValues(): void + { + $this->assertNull( + $this->sut()->resolve(['uid'], ['uid' => []]), + ); + } + + public function testUsesFirstValueOfMultiValuedAttribute(): void + { + $this->assertSame( + 'first', + $this->sut()->resolve(['uid'], ['uid' => ['first', 'second']]), + ); + } +}