From 28cab68e3470878fdb1aebe81136b78d9b5ead0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Wed, 8 Jul 2026 09:45:15 +0200 Subject: [PATCH 1/5] Initial login_hint implementation --- docs/6-oidc-upgrade.md | 9 ++ src/Factories/RequestRulesManagerFactory.php | 2 + src/Server/Grants/AuthCodeGrant.php | 6 ++ src/Server/Grants/ImplicitGrant.php | 5 + .../RequestRules/Rules/LoginHintRule.php | 41 ++++++++ .../RequestTypes/AuthorizationRequest.php | 15 +++ src/Services/AuthenticationService.php | 25 ++++- .../RequestRules/Rules/LoginHintRuleTest.php | 95 +++++++++++++++++++ .../Services/AuthenticationServiceTest.php | 48 ++++++++++ 9 files changed, 245 insertions(+), 1 deletion(-) create mode 100644 src/Server/RequestRules/Rules/LoginHintRule.php create mode 100644 tests/unit/src/Server/RequestRules/Rules/LoginHintRuleTest.php diff --git a/docs/6-oidc-upgrade.md b/docs/6-oidc-upgrade.md index 43ec6a76..f4ee32bd 100644 --- a/docs/6-oidc-upgrade.md +++ b/docs/6-oidc-upgrade.md @@ -204,6 +204,15 @@ language tags). Note: use the canonical SimpleSAMLphp locale codes in (`pt-br`, `zh-tw`), which SimpleSAMLphp itself warns against, are not fully supported by `ui_locales` on the authorization endpoint, because the language cookie is validated against the raw configured codes. +- Support for the `login_hint` parameter on the authorization endpoint +(previously ignored). The parameter carries a hint about the login identifier +the End-User might use to log in. When present, it is propagated to the +SimpleSAMLphp login page as the pre-filled username (using the standard +`core:username` state key consumed by the core UserPass login form). Per +specification this is best-effort: authentication sources that do not use the +core login form simply ignore it, and an incorrect value can be corrected by +the user (or the login fails with invalid credentials). No error is raised if +the parameter is present but unused. - Logging has been improved for authentication flows. It should now be easier to find information about what went wrong by looking at the relevant log entries. diff --git a/src/Factories/RequestRulesManagerFactory.php b/src/Factories/RequestRulesManagerFactory.php index 699488fc..86b00b49 100644 --- a/src/Factories/RequestRulesManagerFactory.php +++ b/src/Factories/RequestRulesManagerFactory.php @@ -25,6 +25,7 @@ use SimpleSAML\Module\oidc\Server\RequestRules\Rules\CodeVerifierRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\IdTokenHintRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\IssuerStateRule; +use SimpleSAML\Module\oidc\Server\RequestRules\Rules\LoginHintRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\MaxAgeRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\PostLogoutRedirectUriRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\PromptRule; @@ -171,6 +172,7 @@ private function getDefaultRules(): array ), new PostLogoutRedirectUriRule($this->requestParamsResolver, $this->helpers, $this->clientRepository), new UiLocalesRule($this->requestParamsResolver, $this->helpers), + new LoginHintRule($this->requestParamsResolver, $this->helpers), new AcrValuesRule($this->requestParamsResolver, $this->helpers), new ScopeOfflineAccessRule($this->requestParamsResolver, $this->helpers), new ClientAuthenticationRule( diff --git a/src/Server/Grants/AuthCodeGrant.php b/src/Server/Grants/AuthCodeGrant.php index e083ea8c..2e106df8 100644 --- a/src/Server/Grants/AuthCodeGrant.php +++ b/src/Server/Grants/AuthCodeGrant.php @@ -55,6 +55,7 @@ use SimpleSAML\Module\oidc\Server\RequestRules\Rules\CodeChallengeRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\CodeVerifierRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\IssuerStateRule; +use SimpleSAML\Module\oidc\Server\RequestRules\Rules\LoginHintRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\MaxAgeRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\PromptRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\RequestedClaimsRule; @@ -869,6 +870,7 @@ public function validateAuthorizationRequestWithRequestRules( IssuerStateRule::class, AuthorizationDetailsRule::class, UiLocalesRule::class, + LoginHintRule::class, ]; // Since we have already validated redirect_uri, and we have state, make it available for other checkers. @@ -990,6 +992,10 @@ public function validateAuthorizationRequestWithRequestRules( $this->loggerService->debug('AuthCodeGrant: UI locales: ', ['uiLocales' => $uiLocales]); $authorizationRequest->setUiLocales($uiLocales); + $loginHint = $resultBag->getOrFail(LoginHintRule::class)->getValue(); + $this->loggerService->debug('AuthCodeGrant: Login hint: ', ['loginHint' => $loginHint]); + $authorizationRequest->setLoginHint($loginHint); + $authorizationRequest->setIsVciRequest($isVciAuthorizationCodeRequest); $flowType = $isVciAuthorizationCodeRequest ? diff --git a/src/Server/Grants/ImplicitGrant.php b/src/Server/Grants/ImplicitGrant.php index df837dc2..27d2e609 100644 --- a/src/Server/Grants/ImplicitGrant.php +++ b/src/Server/Grants/ImplicitGrant.php @@ -25,6 +25,7 @@ use SimpleSAML\Module\oidc\Server\RequestRules\Rules\AddClaimsToIdTokenRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRedirectUriRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRule; +use SimpleSAML\Module\oidc\Server\RequestRules\Rules\LoginHintRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\MaxAgeRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\PromptRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\RequestedClaimsRule; @@ -135,6 +136,7 @@ public function validateAuthorizationRequestWithRequestRules( RequestedClaimsRule::class, AcrValuesRule::class, UiLocalesRule::class, + LoginHintRule::class, ]; $this->requestRulesManager->predefineResultBag($resultBag); @@ -194,6 +196,9 @@ public function validateAuthorizationRequestWithRequestRules( $uiLocales = $resultBag->getOrFail(UiLocalesRule::class)->getValue(); $authorizationRequest->setUiLocales($uiLocales); + $loginHint = $resultBag->getOrFail(LoginHintRule::class)->getValue(); + $authorizationRequest->setLoginHint($loginHint); + $responseMode = $resultBag->getOrFail(ResponseModeRule::class)->getValue(); $authorizationRequest->setResponseMode($responseMode); diff --git a/src/Server/RequestRules/Rules/LoginHintRule.php b/src/Server/RequestRules/Rules/LoginHintRule.php new file mode 100644 index 00000000..f984fb60 --- /dev/null +++ b/src/Server/RequestRules/Rules/LoginHintRule.php @@ -0,0 +1,41 @@ + + */ +class LoginHintRule extends AbstractRule +{ + /** + * @inheritDoc + * + * @param ResponseModeInterface $responseMode + * @param HttpMethodsEnum[] $allowedServerRequestMethods + */ + public function checkRule( + ServerRequestInterface $request, + ResultBagInterface $currentResultBag, + LoggerService $loggerService, + array $data = [], + ResponseModeInterface $responseMode = new QueryResponseMode(), + array $allowedServerRequestMethods = [HttpMethodsEnum::GET], + ): ?Result { + return new Result($this->getKey(), $this->requestParamsResolver->getAsStringBasedOnAllowedMethods( + ParamsEnum::LoginHint->value, + $request, + $allowedServerRequestMethods, + )); + } +} diff --git a/src/Server/RequestTypes/AuthorizationRequest.php b/src/Server/RequestTypes/AuthorizationRequest.php index 6c2cb018..dc3747d6 100644 --- a/src/Server/RequestTypes/AuthorizationRequest.php +++ b/src/Server/RequestTypes/AuthorizationRequest.php @@ -42,6 +42,11 @@ class AuthorizationRequest extends OAuth2AuthorizationRequest */ protected ?string $uiLocales = null; + /** + * Hint about the login identifier the End-User might use to log in, as requested using the login_hint parameter. + */ + protected ?string $loginHint = null; + /** * ACR used during authn. */ @@ -241,6 +246,16 @@ public function setUiLocales(?string $uiLocales): void $this->uiLocales = $uiLocales; } + public function getLoginHint(): ?string + { + return $this->loginHint; + } + + public function setLoginHint(?string $loginHint): void + { + $this->loginHint = $loginHint; + } + public function getAcr(): ?string { return $this->acr; diff --git a/src/Services/AuthenticationService.php b/src/Services/AuthenticationService.php index dcad70d0..8a2b84a8 100644 --- a/src/Services/AuthenticationService.php +++ b/src/Services/AuthenticationService.php @@ -100,7 +100,7 @@ public function processRequest( $this->authSourceId = $authSimple->getAuthSource()->getAuthId(); if (! $authSimple->isAuthenticated()) { - $this->authenticate($authSimple); + $this->authenticate($authSimple, $this->resolveLoginParams($authorizationRequest)); } elseif ($this->sessionService->getIsAuthnPerformedInPreviousRequest()) { $this->sessionService->setIsAuthnPerformedInPreviousRequest(false); @@ -272,6 +272,29 @@ public function getSessionId(): ?string return $this->sessionService->getCurrentSession()->getSessionId(); } + /** + * Resolve additional login parameters to pass to the authentication source, based on the authorization request. + * + * Currently this propagates the login_hint authorization request parameter to the SimpleSAMLphp login page as + * the pre-filled username (the standard 'core:username' state key consumed by the core UserPass login form). + * Per specification this is a hint about the identifier the End-User might use to log in, so it is best-effort: + * auth sources that do not use the core login form simply ignore it, and an incorrect value is corrected by the + * user (or the login simply fails with invalid credentials). + * + * @return array + */ + protected function resolveLoginParams(OAuth2AuthorizationRequestInterface $authorizationRequest): array + { + if ( + !$authorizationRequest instanceof AuthorizationRequest || + ($loginHint = $authorizationRequest->getLoginHint()) === null + ) { + return []; + } + + return ['core:username' => $loginHint]; + } + /** * @throws Error\BadRequest * @throws Error\NotFound diff --git a/tests/unit/src/Server/RequestRules/Rules/LoginHintRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/LoginHintRuleTest.php new file mode 100644 index 00000000..62e7bd7f --- /dev/null +++ b/tests/unit/src/Server/RequestRules/Rules/LoginHintRuleTest.php @@ -0,0 +1,95 @@ +requestStub = $this->createStub(ServerRequestInterface::class); + $this->requestStub->method('getMethod')->willReturn('GET'); + + $this->resultBagStub = $this->createStub(ResultBagInterface::class); + $this->loggerServiceStub = $this->createStub(LoggerService::class); + $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + $this->helpers = new Helpers(); + $this->responseModeStub = $this->createStub(ResponseModeInterface::class); + } + + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ): LoginHintRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + + return new LoginHintRule( + $requestParamsResolver, + $helpers, + ); + } + + /** + * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException + */ + public function testCheckRuleReturnsResultWhenParamSet() + { + $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods')->willReturn('user@example.org'); + + $result = $this->sut()->checkRule( + $this->requestStub, + $this->resultBagStub, + $this->loggerServiceStub, + [], + $this->responseModeStub, + ) ?? + new Result(LoginHintRule::class); + + $this->assertEquals('user@example.org', $result->getValue()); + } + + /** + * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException + */ + public function testCheckRuleReturnsNullWhenParamNotSet() + { + $this->requestStub->method('getQueryParams')->willReturn([]); + + $result = $this->sut()->checkRule( + $this->requestStub, + $this->resultBagStub, + $this->loggerServiceStub, + [], + $this->responseModeStub, + ) ?? + new Result(LoginHintRule::class); + + $this->assertNull($result->getValue()); + } +} diff --git a/tests/unit/src/Services/AuthenticationServiceTest.php b/tests/unit/src/Services/AuthenticationServiceTest.php index a6538d9c..c8bceac2 100644 --- a/tests/unit/src/Services/AuthenticationServiceTest.php +++ b/tests/unit/src/Services/AuthenticationServiceTest.php @@ -433,6 +433,54 @@ public function testItProcessesRequest(bool $isAuthnPer): void ); } + /** + * When the user is not yet authenticated and a login_hint is present in the authorization request, it must be + * propagated to the authentication source as the pre-filled username (the 'core:username' login parameter). + * + * @throws \Throwable + */ + public function testItPropagatesLoginHintToAuthentication(): void + { + $authenticationServiceMock = $this->getMockBuilder(AuthenticationService::class) + ->enableOriginalConstructor() + ->setConstructorArgs([ + $this->userRepositoryMock, + $this->authSimpleFactoryMock, + $this->clientRepositoryMock, + $this->opMetadataService, + $this->sessionServiceMock, + $this->claimTranslatorExtractorMock, + $this->moduleConfigMock, + $this->processingChainFactoryMock, + $this->stateServiceMock, + $this->requestParamsResolverMock, + $this->userEntityFactoryMock, + $this->routesMock, + new UserIdentifierResolver(), + ]) + ->onlyMethods(['runAuthProcs', 'prepareStateArray', 'authenticate']) + ->getMock(); + + $this->authSimpleMock->method('getAuthSource')->willReturn($this->authSourceMock); + $this->authSourceMock->method('getAuthId')->willReturn(self::AUTH_SOURCE); + $this->authSimpleMock->expects($this->once())->method('isAuthenticated')->willReturn(false); + $this->authorizationRequestMock->method('getClient')->willReturn($this->clientEntityMock); + $this->authorizationRequestMock->method('getLoginHint')->willReturn('user@example.org'); + $authenticationServiceMock->method('prepareStateArray')->willReturn(self::STATE); + + $authenticationServiceMock->expects($this->once()) + ->method('authenticate') + ->with($this->authSimpleMock, ['core:username' => 'user@example.org']); + + $this->assertSame( + $authenticationServiceMock->processRequest( + $this->serverRequestMock, + $this->authorizationRequestMock, + ), + self::STATE, + ); + } + /** * @throws NoState */ From 9c39ca7a129e250bdd3eb9bb639f16d238c9fbe0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Wed, 8 Jul 2026 10:02:02 +0200 Subject: [PATCH 2/5] Keep login_hint in reauthentication scenario --- docs/6-oidc-upgrade.md | 3 +- .../RequestRules/Rules/AbstractRule.php | 28 +++++ src/Server/RequestRules/Rules/MaxAgeRule.php | 1 + src/Server/RequestRules/Rules/PromptRule.php | 1 + src/Services/AuthenticationService.php | 7 +- .../RequestRules/Rules/MaxAgeRuleTest.php | 35 ++++++ .../RequestRules/Rules/PromptRuleTest.php | 119 ++++++++++++++++++ 7 files changed, 192 insertions(+), 2 deletions(-) create mode 100644 tests/unit/src/Server/RequestRules/Rules/PromptRuleTest.php diff --git a/docs/6-oidc-upgrade.md b/docs/6-oidc-upgrade.md index f4ee32bd..78838580 100644 --- a/docs/6-oidc-upgrade.md +++ b/docs/6-oidc-upgrade.md @@ -212,7 +212,8 @@ SimpleSAMLphp login page as the pre-filled username (using the standard specification this is best-effort: authentication sources that do not use the core login form simply ignore it, and an incorrect value can be corrected by the user (or the login fails with invalid credentials). No error is raised if -the parameter is present but unused. +the parameter is present but unused. This also applies to forced +re-authentication triggered by `prompt=login` or an expired `max_age`. - Logging has been improved for authentication flows. It should now be easier to find information about what went wrong by looking at the relevant log entries. diff --git a/src/Server/RequestRules/Rules/AbstractRule.php b/src/Server/RequestRules/Rules/AbstractRule.php index fe4eb48f..b67c101b 100644 --- a/src/Server/RequestRules/Rules/AbstractRule.php +++ b/src/Server/RequestRules/Rules/AbstractRule.php @@ -7,6 +7,7 @@ use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\RequestRuleInterface; +use SimpleSAML\Module\oidc\Services\AuthenticationService; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum; use SimpleSAML\OpenID\Codebooks\ParamsEnum; @@ -53,4 +54,31 @@ protected function isOidcAuthorizationRequest( return in_array(ScopesEnum::OpenId->value, explode(' ', $scope), true); } + + /** + * Propagate the login_hint authorization request parameter (if present) into the given login params as the + * pre-filled username, so it survives a rule-initiated re-authentication (e.g. prompt=login or expired max_age), + * where the normal AuthenticationService propagation path is bypassed. Best-effort, mirroring + * AuthenticationService::resolveLoginParams(). + * + * @param array $loginParams + * @param \SimpleSAML\OpenID\Codebooks\HttpMethodsEnum[] $allowedServerRequestMethods + */ + protected function addLoginHintParam( + array &$loginParams, + ServerRequestInterface $request, + array $allowedServerRequestMethods = [HttpMethodsEnum::GET], + ): void { + $loginHint = $this->requestParamsResolver->getAsStringBasedOnAllowedMethods( + ParamsEnum::LoginHint->value, + $request, + $allowedServerRequestMethods, + ); + + if ($loginHint === null) { + return; + } + + $loginParams[AuthenticationService::LOGIN_PARAM_USERNAME] = $loginHint; + } } diff --git a/src/Server/RequestRules/Rules/MaxAgeRule.php b/src/Server/RequestRules/Rules/MaxAgeRule.php index 89caab14..1ee2d023 100644 --- a/src/Server/RequestRules/Rules/MaxAgeRule.php +++ b/src/Server/RequestRules/Rules/MaxAgeRule.php @@ -117,6 +117,7 @@ public function checkRule( $this->sspBridge->utils()->http()->getSelfURLNoQuery(), $requestParams, ); + $this->addLoginHintParam($loginParams, $request, $allowedServerRequestMethods); $this->authenticationService->authenticateForClient($client, $loginParams); } diff --git a/src/Server/RequestRules/Rules/PromptRule.php b/src/Server/RequestRules/Rules/PromptRule.php index 276d94c0..597bb035 100644 --- a/src/Server/RequestRules/Rules/PromptRule.php +++ b/src/Server/RequestRules/Rules/PromptRule.php @@ -105,6 +105,7 @@ public function checkRule( $this->sspBridge->utils()->http()->getSelfURLNoQuery(), $requestParams, ); + $this->addLoginHintParam($loginParams, $request, $allowedServerRequestMethods); $this->authenticationService->authenticateForClient($client, $loginParams); } diff --git a/src/Services/AuthenticationService.php b/src/Services/AuthenticationService.php index 8a2b84a8..56c248f4 100644 --- a/src/Services/AuthenticationService.php +++ b/src/Services/AuthenticationService.php @@ -46,6 +46,11 @@ class AuthenticationService { + /** + * Login parameter (state array key) used to pre-fill the username on the SimpleSAMLphp core UserPass login form. + */ + public const string LOGIN_PARAM_USERNAME = 'core:username'; + /** * ID of auth source used during authn. */ @@ -292,7 +297,7 @@ protected function resolveLoginParams(OAuth2AuthorizationRequestInterface $autho return []; } - return ['core:username' => $loginHint]; + return [self::LOGIN_PARAM_USERNAME => $loginHint]; } /** diff --git a/tests/unit/src/Server/RequestRules/Rules/MaxAgeRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/MaxAgeRuleTest.php index 920d3528..c7b99a7f 100644 --- a/tests/unit/src/Server/RequestRules/Rules/MaxAgeRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/MaxAgeRuleTest.php @@ -10,17 +10,21 @@ use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Auth\Simple; use SimpleSAML\Module\oidc\Bridges\SspBridge; +use SimpleSAML\Module\oidc\Bridges\SspBridge\Utils as SspBridgeUtils; use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface; use SimpleSAML\Module\oidc\Factories\AuthSimpleFactory; use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\RequestRules\Result; use SimpleSAML\Module\oidc\Server\RequestRules\ResultBag; +use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRedirectUriRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\MaxAgeRule; +use SimpleSAML\Module\oidc\Server\RequestRules\Rules\StateRule; use SimpleSAML\Module\oidc\Server\ResponseModes\ResponseModeInterface; use SimpleSAML\Module\oidc\Services\AuthenticationService; use SimpleSAML\Module\oidc\Services\LoggerService; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; +use SimpleSAML\Utils\HTTP as SspHttp; #[CoversClass(MaxAgeRule::class)] class MaxAgeRuleTest extends TestCase @@ -111,4 +115,35 @@ public function testDefaultMaxAgeNotExpiredReturnsAuthInstant(): void $this->assertNotNull($this->checkRule()); } + + public function testExpiredMaxAgeReAuthenticatesAndPropagatesLoginHint(): void + { + $this->resultBag->add(new Result(ClientRedirectUriRule::class, 'https://rp.example.org/cb')); + $this->resultBag->add(new Result(StateRule::class, 'state123')); + $this->requestParamsResolverMock->method('getAllBasedOnAllowedMethods') + ->willReturn(['max_age' => 0, 'login_hint' => 'user@example.org']); + $this->requestParamsResolverMock->method('getAsStringBasedOnAllowedMethods') + ->willReturn('user@example.org'); + $this->clientMock->method('getRequireAuthTime')->willReturn(false); + $this->authSimpleMock->method('isAuthenticated')->willReturn(true); + // Authenticated well before the (zero) max_age window, so re-authentication is enforced. + $this->authSimpleMock->method('getAuthData')->willReturn(time() - 3600); + + $httpMock = $this->createMock(SspHttp::class); + $httpMock->method('getSelfURLNoQuery')->willReturn('https://op.example.org/authorize'); + $httpMock->method('addURLParameters')->willReturn('https://op.example.org/authorize?max_age=0'); + $utilsMock = $this->createMock(SspBridgeUtils::class); + $utilsMock->method('http')->willReturn($httpMock); + $this->sspBridgeMock->method('utils')->willReturn($utilsMock); + + $this->authenticationServiceMock->expects($this->once()) + ->method('authenticateForClient') + ->with( + $this->clientMock, + $this->callback(fn(array $loginParams): bool => + ($loginParams['core:username'] ?? null) === 'user@example.org'), + ); + + $this->checkRule(); + } } diff --git a/tests/unit/src/Server/RequestRules/Rules/PromptRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/PromptRuleTest.php new file mode 100644 index 00000000..f121ceec --- /dev/null +++ b/tests/unit/src/Server/RequestRules/Rules/PromptRuleTest.php @@ -0,0 +1,119 @@ +requestParamsResolverMock = $this->createMock(RequestParamsResolver::class); + $this->authSimpleFactoryMock = $this->createMock(AuthSimpleFactory::class); + $this->authenticationServiceMock = $this->createMock(AuthenticationService::class); + $this->sspBridgeMock = $this->createMock(SspBridge::class); + $this->authSimpleMock = $this->createMock(Simple::class); + $this->clientMock = $this->createMock(ClientEntityInterface::class); + $this->loggerServiceMock = $this->createMock(LoggerService::class); + $this->requestMock = $this->createMock(ServerRequestInterface::class); + $this->responseModeMock = $this->createMock(ResponseModeInterface::class); + + $this->authSimpleFactoryMock->method('build')->willReturn($this->authSimpleMock); + + $this->resultBag = new ResultBag(); + $this->resultBag->add(new Result(ClientRule::class, $this->clientMock)); + $this->resultBag->add(new Result(ClientRedirectUriRule::class, 'https://rp.example.org/cb')); + $this->resultBag->add(new Result(StateRule::class, 'state123')); + } + + protected function sut(): PromptRule + { + return new PromptRule( + $this->requestParamsResolverMock, + new Helpers(), + $this->authSimpleFactoryMock, + $this->authenticationServiceMock, + $this->sspBridgeMock, + ); + } + + protected function checkRule(): ?Result + { + return $this->sut()->checkRule( + $this->requestMock, + $this->resultBag, + $this->loggerServiceMock, + [], + $this->responseModeMock, + ); + } + + public function testReturnsNullWhenNoPromptParam(): void + { + $this->requestParamsResolverMock->method('getAllBasedOnAllowedMethods')->willReturn([]); + $this->authenticationServiceMock->expects($this->never())->method('authenticateForClient'); + + $this->assertNull($this->checkRule()); + } + + public function testPromptLoginReAuthenticatesAndPropagatesLoginHint(): void + { + $this->requestParamsResolverMock->method('getAllBasedOnAllowedMethods') + ->willReturn(['prompt' => 'login', 'login_hint' => 'user@example.org']); + $this->requestParamsResolverMock->method('getAsStringBasedOnAllowedMethods') + ->willReturn('user@example.org'); + $this->authSimpleMock->method('isAuthenticated')->willReturn(true); + + $httpMock = $this->createMock(SspHttp::class); + $httpMock->method('getSelfURLNoQuery')->willReturn('https://op.example.org/authorize'); + $httpMock->method('addURLParameters') + ->willReturn('https://op.example.org/authorize?login_hint=user@example.org'); + $utilsMock = $this->createMock(SspBridgeUtils::class); + $utilsMock->method('http')->willReturn($httpMock); + $this->sspBridgeMock->method('utils')->willReturn($utilsMock); + + $this->authenticationServiceMock->expects($this->once()) + ->method('authenticateForClient') + ->with( + $this->clientMock, + $this->callback(fn(array $loginParams): bool => + ($loginParams['core:username'] ?? null) === 'user@example.org'), + ); + + $this->assertNull($this->checkRule()); + } +} From 47ca9f73c765c2ec4e1ff3e5680b7bfc9f8de1f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Wed, 8 Jul 2026 13:40:56 +0200 Subject: [PATCH 3/5] Don't log the login hint --- src/Server/Grants/AuthCodeGrant.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Server/Grants/AuthCodeGrant.php b/src/Server/Grants/AuthCodeGrant.php index 2e106df8..9b8dbff9 100644 --- a/src/Server/Grants/AuthCodeGrant.php +++ b/src/Server/Grants/AuthCodeGrant.php @@ -993,7 +993,9 @@ public function validateAuthorizationRequestWithRequestRules( $authorizationRequest->setUiLocales($uiLocales); $loginHint = $resultBag->getOrFail(LoginHintRule::class)->getValue(); - $this->loggerService->debug('AuthCodeGrant: Login hint: ', ['loginHint' => $loginHint]); + // Only log presence, not the value: login_hint is commonly an email or username (PII). + $loginHintPresent = $loginHint !== null; + $this->loggerService->debug('AuthCodeGrant: Login hint present: ', ['loginHintPresent' => $loginHintPresent]); $authorizationRequest->setLoginHint($loginHint); From 21cfc1099b9ddb625a681704f30a818aa4ee072c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Wed, 8 Jul 2026 15:34:56 +0200 Subject: [PATCH 4/5] Update docs --- src/Server/Grants/AuthCodeGrant.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Server/Grants/AuthCodeGrant.php b/src/Server/Grants/AuthCodeGrant.php index 9b8dbff9..0e99f66e 100644 --- a/src/Server/Grants/AuthCodeGrant.php +++ b/src/Server/Grants/AuthCodeGrant.php @@ -993,7 +993,7 @@ public function validateAuthorizationRequestWithRequestRules( $authorizationRequest->setUiLocales($uiLocales); $loginHint = $resultBag->getOrFail(LoginHintRule::class)->getValue(); - // Only log presence, not the value: login_hint is commonly an email or username (PII). + // Only log presence, not the value: login_hint is commonly a PII. $loginHintPresent = $loginHint !== null; $this->loggerService->debug('AuthCodeGrant: Login hint present: ', ['loginHintPresent' => $loginHintPresent]); $authorizationRequest->setLoginHint($loginHint); From 1f7dbfe125be0d13f525d03e888e8afe95c442a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Ivan=C4=8Di=C4=87?= Date: Wed, 8 Jul 2026 15:49:19 +0200 Subject: [PATCH 5/5] Reuse LoginHintRule result --- src/Server/Grants/AuthCodeGrant.php | 4 ++- src/Server/Grants/ImplicitGrant.php | 4 ++- .../RequestRules/Rules/AbstractRule.php | 28 ------------------- src/Server/RequestRules/Rules/MaxAgeRule.php | 6 +++- src/Server/RequestRules/Rules/PromptRule.php | 6 +++- .../RequestRules/Rules/MaxAgeRuleTest.php | 4 +-- .../RequestRules/Rules/PromptRuleTest.php | 4 +-- 7 files changed, 20 insertions(+), 36 deletions(-) diff --git a/src/Server/Grants/AuthCodeGrant.php b/src/Server/Grants/AuthCodeGrant.php index 0e99f66e..604aac6f 100644 --- a/src/Server/Grants/AuthCodeGrant.php +++ b/src/Server/Grants/AuthCodeGrant.php @@ -858,6 +858,9 @@ public function validateAuthorizationRequestWithRequestRules( ClientIdRule::class, ResponseTypeRule::class, RequestObjectRule::class, + // LoginHintRule must run before PromptRule and MaxAgeRule, which consume its result when they + // trigger re-authentication (prompt=login / expired max_age) to pre-fill the username. + LoginHintRule::class, PromptRule::class, MaxAgeRule::class, ScopeRule::class, @@ -870,7 +873,6 @@ public function validateAuthorizationRequestWithRequestRules( IssuerStateRule::class, AuthorizationDetailsRule::class, UiLocalesRule::class, - LoginHintRule::class, ]; // Since we have already validated redirect_uri, and we have state, make it available for other checkers. diff --git a/src/Server/Grants/ImplicitGrant.php b/src/Server/Grants/ImplicitGrant.php index 27d2e609..14312917 100644 --- a/src/Server/Grants/ImplicitGrant.php +++ b/src/Server/Grants/ImplicitGrant.php @@ -127,6 +127,9 @@ public function validateAuthorizationRequestWithRequestRules( $rulesToExecute = [ ScopeRule::class, RequestObjectRule::class, + // LoginHintRule must run before PromptRule and MaxAgeRule, which consume its result when they + // trigger re-authentication (prompt=login / expired max_age) to pre-fill the username. + LoginHintRule::class, PromptRule::class, MaxAgeRule::class, RequiredOpenIdScopeRule::class, @@ -136,7 +139,6 @@ public function validateAuthorizationRequestWithRequestRules( RequestedClaimsRule::class, AcrValuesRule::class, UiLocalesRule::class, - LoginHintRule::class, ]; $this->requestRulesManager->predefineResultBag($resultBag); diff --git a/src/Server/RequestRules/Rules/AbstractRule.php b/src/Server/RequestRules/Rules/AbstractRule.php index b67c101b..fe4eb48f 100644 --- a/src/Server/RequestRules/Rules/AbstractRule.php +++ b/src/Server/RequestRules/Rules/AbstractRule.php @@ -7,7 +7,6 @@ use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\RequestRuleInterface; -use SimpleSAML\Module\oidc\Services\AuthenticationService; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum; use SimpleSAML\OpenID\Codebooks\ParamsEnum; @@ -54,31 +53,4 @@ protected function isOidcAuthorizationRequest( return in_array(ScopesEnum::OpenId->value, explode(' ', $scope), true); } - - /** - * Propagate the login_hint authorization request parameter (if present) into the given login params as the - * pre-filled username, so it survives a rule-initiated re-authentication (e.g. prompt=login or expired max_age), - * where the normal AuthenticationService propagation path is bypassed. Best-effort, mirroring - * AuthenticationService::resolveLoginParams(). - * - * @param array $loginParams - * @param \SimpleSAML\OpenID\Codebooks\HttpMethodsEnum[] $allowedServerRequestMethods - */ - protected function addLoginHintParam( - array &$loginParams, - ServerRequestInterface $request, - array $allowedServerRequestMethods = [HttpMethodsEnum::GET], - ): void { - $loginHint = $this->requestParamsResolver->getAsStringBasedOnAllowedMethods( - ParamsEnum::LoginHint->value, - $request, - $allowedServerRequestMethods, - ); - - if ($loginHint === null) { - return; - } - - $loginParams[AuthenticationService::LOGIN_PARAM_USERNAME] = $loginHint; - } } diff --git a/src/Server/RequestRules/Rules/MaxAgeRule.php b/src/Server/RequestRules/Rules/MaxAgeRule.php index 1ee2d023..4eebce56 100644 --- a/src/Server/RequestRules/Rules/MaxAgeRule.php +++ b/src/Server/RequestRules/Rules/MaxAgeRule.php @@ -117,7 +117,11 @@ public function checkRule( $this->sspBridge->utils()->http()->getSelfURLNoQuery(), $requestParams, ); - $this->addLoginHintParam($loginParams, $request, $allowedServerRequestMethods); + // Propagate login_hint (resolved earlier by LoginHintRule) as the pre-filled username. + $loginHint = $currentResultBag->getOrFail(LoginHintRule::class)->getValue(); + if ($loginHint !== null) { + $loginParams[AuthenticationService::LOGIN_PARAM_USERNAME] = $loginHint; + } $this->authenticationService->authenticateForClient($client, $loginParams); } diff --git a/src/Server/RequestRules/Rules/PromptRule.php b/src/Server/RequestRules/Rules/PromptRule.php index 597bb035..6c88db03 100644 --- a/src/Server/RequestRules/Rules/PromptRule.php +++ b/src/Server/RequestRules/Rules/PromptRule.php @@ -105,7 +105,11 @@ public function checkRule( $this->sspBridge->utils()->http()->getSelfURLNoQuery(), $requestParams, ); - $this->addLoginHintParam($loginParams, $request, $allowedServerRequestMethods); + // Propagate login_hint (resolved earlier by LoginHintRule) as the pre-filled username. + $loginHint = $currentResultBag->getOrFail(LoginHintRule::class)->getValue(); + if ($loginHint !== null) { + $loginParams[AuthenticationService::LOGIN_PARAM_USERNAME] = $loginHint; + } $this->authenticationService->authenticateForClient($client, $loginParams); } diff --git a/tests/unit/src/Server/RequestRules/Rules/MaxAgeRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/MaxAgeRuleTest.php index c7b99a7f..1927a6fa 100644 --- a/tests/unit/src/Server/RequestRules/Rules/MaxAgeRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/MaxAgeRuleTest.php @@ -18,6 +18,7 @@ use SimpleSAML\Module\oidc\Server\RequestRules\ResultBag; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRedirectUriRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRule; +use SimpleSAML\Module\oidc\Server\RequestRules\Rules\LoginHintRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\MaxAgeRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\StateRule; use SimpleSAML\Module\oidc\Server\ResponseModes\ResponseModeInterface; @@ -120,10 +121,9 @@ public function testExpiredMaxAgeReAuthenticatesAndPropagatesLoginHint(): void { $this->resultBag->add(new Result(ClientRedirectUriRule::class, 'https://rp.example.org/cb')); $this->resultBag->add(new Result(StateRule::class, 'state123')); + $this->resultBag->add(new Result(LoginHintRule::class, 'user@example.org')); $this->requestParamsResolverMock->method('getAllBasedOnAllowedMethods') ->willReturn(['max_age' => 0, 'login_hint' => 'user@example.org']); - $this->requestParamsResolverMock->method('getAsStringBasedOnAllowedMethods') - ->willReturn('user@example.org'); $this->clientMock->method('getRequireAuthTime')->willReturn(false); $this->authSimpleMock->method('isAuthenticated')->willReturn(true); // Authenticated well before the (zero) max_age window, so re-authentication is enforced. diff --git a/tests/unit/src/Server/RequestRules/Rules/PromptRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/PromptRuleTest.php index f121ceec..60a98b8d 100644 --- a/tests/unit/src/Server/RequestRules/Rules/PromptRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/PromptRuleTest.php @@ -18,6 +18,7 @@ use SimpleSAML\Module\oidc\Server\RequestRules\ResultBag; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRedirectUriRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\ClientRule; +use SimpleSAML\Module\oidc\Server\RequestRules\Rules\LoginHintRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\PromptRule; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\StateRule; use SimpleSAML\Module\oidc\Server\ResponseModes\ResponseModeInterface; @@ -92,10 +93,9 @@ public function testReturnsNullWhenNoPromptParam(): void public function testPromptLoginReAuthenticatesAndPropagatesLoginHint(): void { + $this->resultBag->add(new Result(LoginHintRule::class, 'user@example.org')); $this->requestParamsResolverMock->method('getAllBasedOnAllowedMethods') ->willReturn(['prompt' => 'login', 'login_hint' => 'user@example.org']); - $this->requestParamsResolverMock->method('getAsStringBasedOnAllowedMethods') - ->willReturn('user@example.org'); $this->authSimpleMock->method('isAuthenticated')->willReturn(true); $httpMock = $this->createMock(SspHttp::class);