diff --git a/docker/conformance.sql b/docker/conformance.sql index 42aa9078..bf09b946 100644 --- a/docker/conformance.sql +++ b/docker/conformance.sql @@ -65,6 +65,7 @@ CREATE TABLE oidc_access_token ( authorization_details TEXT NULL, bound_client_id TEXT NULL, bound_redirect_uri TEXT NULL, + issuer_state TEXT NULL, CONSTRAINT FK_43C1650EA76ED395 FOREIGN KEY (user_id) REFERENCES oidc_user (id) ON DELETE CASCADE, CONSTRAINT FK_43C1650E19EB6921 FOREIGN KEY (client_id) @@ -92,6 +93,7 @@ CREATE TABLE oidc_auth_code ( authorization_details TEXT NULL, bound_client_id TEXT NULL, bound_redirect_uri TEXT NULL, + issuer_state TEXT NULL, CONSTRAINT FK_97D32CA7A76ED395 FOREIGN KEY (user_id) REFERENCES oidc_user (id) ON DELETE CASCADE, CONSTRAINT FK_97D32CA719EB6921 FOREIGN KEY (client_id) diff --git a/src/Controllers/VerifiableCredentials/CredentialIssuerCredentialController.php b/src/Controllers/VerifiableCredentials/CredentialIssuerCredentialController.php index 728356dd..c4ee2e85 100644 --- a/src/Controllers/VerifiableCredentials/CredentialIssuerCredentialController.php +++ b/src/Controllers/VerifiableCredentials/CredentialIssuerCredentialController.php @@ -10,6 +10,7 @@ use SimpleSAML\Module\oidc\Entities\AccessTokenEntity; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\AccessTokenRepository; +use SimpleSAML\Module\oidc\Repositories\IssuerStateRepository; use SimpleSAML\Module\oidc\Repositories\UserRepository; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Services\LoggerService; @@ -53,6 +54,7 @@ public function __construct( protected readonly RequestParamsResolver $requestParamsResolver, protected readonly UserRepository $userRepository, protected readonly Did $did, + protected readonly IssuerStateRepository $issuerStateRepository, ) { if (!$this->moduleConfig->getVerifiableCredentialEnabled()) { $this->loggerService->warning('Verifiable Credential capabilities not enabled.'); @@ -121,6 +123,30 @@ public function credential(Request $request): Response ); } + $issuerState = $accessToken->getIssuerState(); + if (!is_string($issuerState)) { + $this->loggerService->error( + 'CredentialIssuerCredentialController::credential: Issuer state missing in access token.', + ['access_token' => $accessToken], + ); + return $this->routes->newJsonErrorResponse( + 'invalid_token', + 'Issuer state missing in access token.', + 401, + ); + } + + if ($this->issuerStateRepository->findValid($issuerState) === null) { + $this->loggerService->warning( + 'CredentialIssuerCredentialController::credential: Issuer state not valid.', + ['issuer_state' => $issuerState], + ); + return $this->routes->newJsonErrorResponse( + 'invalid_token', + 'Issuer state not valid.', + ); + } + if ( isset($requestData[ClaimsEnum::CredentialConfigurationId->value]) && isset($requestData[ClaimsEnum::CredentialIdentifier->value]) @@ -652,7 +678,11 @@ public function credential(Request $request): Response throw new OpenIdException('Invalid credential format ID.'); } - $this->loggerService->debug('response', [ + $this->loggerService->debug('Revoking issuer state.', ['issuerState' => $issuerState]); + ; + $this->issuerStateRepository->revoke($issuerState); + + $this->loggerService->debug('Returning credential response.', [ 'credentials' => [ ['credential' => $verifiableCredential->getToken()], ], diff --git a/src/Entities/AccessTokenEntity.php b/src/Entities/AccessTokenEntity.php index 834902a1..2fef7aaa 100644 --- a/src/Entities/AccessTokenEntity.php +++ b/src/Entities/AccessTokenEntity.php @@ -74,6 +74,7 @@ public function __construct( protected readonly ?array $authorizationDetails = null, protected readonly ?string $boundClientId = null, protected readonly ?string $boundRedirectUri = null, + protected readonly ?string $issuerState = null, ) { $this->setIdentifier($id); $this->setClient($clientEntity); @@ -125,6 +126,7 @@ public function getState(): array null, 'bound_client_id' => $this->boundClientId, 'bound_redirect_uri' => $this->boundRedirectUri, + 'issuer_state' => $this->issuerState, ]; } @@ -166,6 +168,9 @@ protected function convertToJWT(): Token ->expiresAt($this->getExpiryDateTime()) ->relatedTo((string) $this->getUserIdentifier()) ->withClaim('scopes', $this->getScopes()); + if ($this->issuerState !== null) { + $jwtBuilder = $jwtBuilder->withClaim('issuer_state', $this->issuerState); + } return $this->jsonWebTokenBuilderService->getSignedProtocolJwt($jwtBuilder); } @@ -189,4 +194,9 @@ public function getBoundRedirectUri(): ?string { return $this->boundRedirectUri; } + + public function getIssuerState(): ?string + { + return $this->issuerState; + } } diff --git a/src/Entities/AuthCodeEntity.php b/src/Entities/AuthCodeEntity.php index e8e10f30..2cacb7e1 100644 --- a/src/Entities/AuthCodeEntity.php +++ b/src/Entities/AuthCodeEntity.php @@ -49,6 +49,7 @@ public function __construct( protected readonly ?array $authorizationDetails = null, protected readonly ?string $boundClientId = null, protected readonly ?string $boundRedirectUri = null, + protected readonly ?string $issuerState = null, ) { $this->identifier = $id; $this->client = $client; @@ -81,6 +82,7 @@ public function getState(): array null, 'bound_client_id' => $this->boundClientId, 'bound_redirect_uri' => $this->boundRedirectUri, + 'issuer_state' => $this->issuerState, ]; } @@ -113,4 +115,9 @@ public function getBoundRedirectUri(): ?string { return $this->boundRedirectUri; } + + public function getIssuerState(): ?string + { + return $this->issuerState; + } } diff --git a/src/Factories/Entities/AccessTokenEntityFactory.php b/src/Factories/Entities/AccessTokenEntityFactory.php index b3ed8f3a..2c1a5417 100644 --- a/src/Factories/Entities/AccessTokenEntityFactory.php +++ b/src/Factories/Entities/AccessTokenEntityFactory.php @@ -40,6 +40,7 @@ public function fromData( ?array $authorizationDetails = null, ?string $boundClientId = null, ?string $boundRedirectUri = null, + ?string $issuerState = null, ): AccessTokenEntity { return new AccessTokenEntity( $id, @@ -56,6 +57,7 @@ public function fromData( authorizationDetails: $authorizationDetails, boundClientId: $boundClientId, boundRedirectUri: $boundRedirectUri, + issuerState: $issuerState, ); } diff --git a/src/Factories/Entities/AuthCodeEntityFactory.php b/src/Factories/Entities/AuthCodeEntityFactory.php index 4449e74b..0304b804 100644 --- a/src/Factories/Entities/AuthCodeEntityFactory.php +++ b/src/Factories/Entities/AuthCodeEntityFactory.php @@ -31,6 +31,7 @@ public function fromData( ?string $userIdentifier = null, ?string $redirectUri = null, ?string $nonce = null, + ?string $issuerState = null, bool $isRevoked = false, ?FlowTypeEnum $flowTypeEnum = null, ?string $txCode = null, @@ -52,6 +53,7 @@ public function fromData( $authorizationDetails, $boundClientId, $boundRedirectUri, + $issuerState, ); } @@ -94,6 +96,7 @@ public function fromState(array $state): AuthCodeEntity $isRevoked = (bool) $state['is_revoked']; $flowType = empty($state['flow_type']) ? null : FlowTypeEnum::tryFrom((string)$state['flow_type']); $txCode = empty($state['tx_code']) ? null : (string)$state['tx_code']; + $issuerState = empty($state['issuer_state']) ? null : (string)$state['issuer_state']; /** @psalm-suppress MixedAssignment */ $authorizationDetails = isset($state['authorization_details']) && is_string($state['authorization_details']) ? @@ -112,6 +115,7 @@ public function fromState(array $state): AuthCodeEntity $userIdentifier, $redirectUri, $nonce, + $issuerState, $isRevoked, $flowType, $txCode, diff --git a/src/Factories/RequestRulesManagerFactory.php b/src/Factories/RequestRulesManagerFactory.php index a05bf4f0..27de4e3c 100644 --- a/src/Factories/RequestRulesManagerFactory.php +++ b/src/Factories/RequestRulesManagerFactory.php @@ -10,7 +10,6 @@ use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\ClientRepository; use SimpleSAML\Module\oidc\Repositories\CodeChallengeVerifiersRepository; -use SimpleSAML\Module\oidc\Repositories\IssuerStateRepository; use SimpleSAML\Module\oidc\Repositories\ScopeRepository; use SimpleSAML\Module\oidc\Server\RequestRules\RequestRulesManager; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\AcrValuesRule; @@ -66,7 +65,6 @@ public function __construct( private readonly JwksResolver $jwksResolver, private readonly FederationParticipationValidator $federationParticipationValidator, private readonly SspBridge $sspBridge, - private readonly IssuerStateRepository $issuerStateRepository, private readonly ?FederationCache $federationCache = null, private readonly ?ProtocolCache $protocolCache = null, ) { @@ -89,6 +87,7 @@ private function getDefaultRules(): array { return [ new StateRule($this->requestParamsResolver, $this->helpers), + new IssuerStateRule($this->requestParamsResolver, $this->helpers), new ClientRule( $this->requestParamsResolver, $this->helpers, @@ -147,7 +146,6 @@ private function getDefaultRules(): array $this->protocolCache, ), new CodeVerifierRule($this->requestParamsResolver, $this->helpers), - new IssuerStateRule($this->requestParamsResolver, $this->helpers, $this->issuerStateRepository), new AuthorizationDetailsRule($this->requestParamsResolver, $this->helpers, $this->moduleConfig), new ClientIdRule($this->requestParamsResolver, $this->helpers), ]; diff --git a/src/Repositories/AccessTokenRepository.php b/src/Repositories/AccessTokenRepository.php index 9d55097b..ac535490 100644 --- a/src/Repositories/AccessTokenRepository.php +++ b/src/Repositories/AccessTokenRepository.php @@ -113,7 +113,8 @@ public function persistNewAccessToken(OAuth2AccessTokenEntityInterface $accessTo flow_type, authorization_details, bound_client_id, - bound_redirect_uri + bound_redirect_uri, + issuer_state ) " . "VALUES ( :id, @@ -127,7 +128,8 @@ public function persistNewAccessToken(OAuth2AccessTokenEntityInterface $accessTo :flow_type, :authorization_details, :bound_client_id, - :bound_redirect_uri + :bound_redirect_uri, + :issuer_state )", $this->getTableName(), ); @@ -267,7 +269,7 @@ private function update(AccessTokenEntity $accessTokenEntity): void . "client_id = :client_id, is_revoked = :is_revoked, auth_code_id = :auth_code_id, " . "requested_claims = :requested_claims, flow_type = :flow_type, " . "authorization_details = :authorization_details, bound_client_id = :bound_client_id, " . - "bound_redirect_uri = :bound_redirect_uri WHERE id = :id", + "bound_redirect_uri = :bound_redirect_uri, issuer_state = :issuer_state WHERE id = :id", $this->getTableName(), ); diff --git a/src/Repositories/AuthCodeRepository.php b/src/Repositories/AuthCodeRepository.php index a24afdf3..f083cfc8 100644 --- a/src/Repositories/AuthCodeRepository.php +++ b/src/Repositories/AuthCodeRepository.php @@ -84,7 +84,8 @@ public function persistNewAuthCode(OAuth2AuthCodeEntityInterface $authCodeEntity tx_code, authorization_details, bound_client_id, - bound_redirect_uri + bound_redirect_uri, + issuer_state ) VALUES ( :id, :scopes, @@ -98,7 +99,8 @@ public function persistNewAuthCode(OAuth2AuthCodeEntityInterface $authCodeEntity :tx_code, :authorization_details, :bound_client_id, - :bound_redirect_uri + :bound_redirect_uri, + :issuer_state ) EOS, $this->getTableName(), @@ -224,7 +226,8 @@ private function update(AuthCodeEntity $authCodeEntity): void tx_code = :tx_code, authorization_details = :authorization_details, bound_client_id = :bound_client_id, - bound_redirect_uri = :bound_redirect_uri + bound_redirect_uri = :bound_redirect_uri, + issuer_state = :issuer_state WHERE id = :id EOS , diff --git a/src/Server/Grants/AuthCodeGrant.php b/src/Server/Grants/AuthCodeGrant.php index 53315271..5693e220 100644 --- a/src/Server/Grants/AuthCodeGrant.php +++ b/src/Server/Grants/AuthCodeGrant.php @@ -27,6 +27,7 @@ use SimpleSAML\Module\oidc\Entities\Interfaces\AccessTokenEntityInterface; use SimpleSAML\Module\oidc\Entities\Interfaces\AuthCodeEntityInterface; use SimpleSAML\Module\oidc\Entities\Interfaces\RefreshTokenEntityInterface; +use SimpleSAML\Module\oidc\Entities\ScopeEntity; use SimpleSAML\Module\oidc\Entities\UserEntity; use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\AuthCodeEntityFactory; @@ -347,6 +348,7 @@ protected function issueOidcAuthCode( $userIdentifier, $redirectUri, $authorizationRequest->getNonce(), + $authorizationRequest->getIssuerState(), flowTypeEnum: $flowType, authorizationDetails: $authorizationRequest->getAuthorizationDetails(), boundClientId: $authorizationRequest->getBoundClientId(), @@ -615,6 +617,7 @@ public function respondToAccessTokenRequest( $storedAuthCodeEntity->getAuthorizationDetails(), $storedAuthCodeEntity->getBoundClientId(), $storedAuthCodeEntity->getBoundRedirectUri(), + $storedAuthCodeEntity->getIssuerState(), ); $this->getEmitter()->emit(new RequestEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request)); $responseType->setAccessToken($accessToken); @@ -893,6 +896,28 @@ public function validateAuthorizationRequestWithRequestRules( ); $authorizationRequest->setAuthorizationDetails($authorizationDetails); + // TODO This is a band-aid fix for having credential claims in the userinfo endpoint when + // only VCI authorizationDetails are supplied. This requires configuring a matching OIDC scope + // that has all the credential type claims as well. + if (is_array($authorizationDetails)) { + /** @psalm-suppress MixedAssignment */ + foreach ($authorizationDetails as $authorizationDetail) { + if ( + is_array($authorizationDetail) && + (isset($authorizationDetail['type'])) && + ($authorizationDetail['type']) === 'openid_credential' + ) { + /** @psalm-suppress MixedAssignment */ + $credentialConfigurationId = $authorizationDetail['credential_configuration_id'] ?? null; + if (is_string($credentialConfigurationId)) { + $scopes[] = new ScopeEntity($credentialConfigurationId); + } + } + } + $this->loggerService->debug('authorizationDetails Resolved Scopes: ', ['scopes' => $scopes]); + $authorizationRequest->setScopes($scopes); + } + // Check if we are using a generic client for this request. This can happen for non-registered clients // in VCI flows. This can be removed once the VCI clients (wallets) are properly registered using DCR. if ($client->isGeneric()) { diff --git a/src/Server/Grants/Traits/IssueAccessTokenTrait.php b/src/Server/Grants/Traits/IssueAccessTokenTrait.php index 8ac538e3..6e8f47b6 100644 --- a/src/Server/Grants/Traits/IssueAccessTokenTrait.php +++ b/src/Server/Grants/Traits/IssueAccessTokenTrait.php @@ -55,6 +55,7 @@ protected function issueAccessToken( ?array $authorizationDetails = null, ?string $boundClientId = null, ?string $boundRedirectUri = null, + ?string $issuerState = null, ): AccessTokenEntityInterface { $maxGenerationAttempts = AbstractGrant::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS; @@ -79,6 +80,7 @@ protected function issueAccessToken( authorizationDetails: $authorizationDetails, boundClientId: $boundClientId, boundRedirectUri: $boundRedirectUri, + issuerState: $issuerState, ); $this->accessTokenRepository->persistNewAccessToken($accessToken); return $accessToken; diff --git a/src/Server/RequestRules/Rules/IssuerStateRule.php b/src/Server/RequestRules/Rules/IssuerStateRule.php index f309b8ba..7ba9bf2d 100644 --- a/src/Server/RequestRules/Rules/IssuerStateRule.php +++ b/src/Server/RequestRules/Rules/IssuerStateRule.php @@ -5,27 +5,15 @@ namespace SimpleSAML\Module\oidc\Server\RequestRules\Rules; use Psr\Http\Message\ServerRequestInterface; -use SimpleSAML\Module\oidc\Helpers; -use SimpleSAML\Module\oidc\Repositories\IssuerStateRepository; -use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Result; use SimpleSAML\Module\oidc\Services\LoggerService; -use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum; use SimpleSAML\OpenID\Codebooks\ParamsEnum; class IssuerStateRule extends AbstractRule { - public function __construct( - RequestParamsResolver $requestParamsResolver, - Helpers $helpers, - protected readonly IssuerStateRepository $issuerStateRepository, - ) { - parent::__construct($requestParamsResolver, $helpers); - } - /** * @inheritDoc */ @@ -37,25 +25,12 @@ public function checkRule( bool $useFragmentInHttpErrorResponses = false, array $allowedServerRequestMethods = [HttpMethodsEnum::GET], ): ?ResultInterface { - $loggerService->debug('IssuerStateRule::checkRule'); - $issuerState = $this->requestParamsResolver->getAsStringBasedOnAllowedMethods( ParamsEnum::IssuerState->value, $request, $allowedServerRequestMethods, ); - if ($issuerState === null) { - return null; - } - - if ($this->issuerStateRepository->findValid($issuerState) === null) { - $loggerService->error('IssuerStateRule: Invalid issuer state: ' . $issuerState); - throw OidcServerException::invalidRequest(ParamsEnum::IssuerState->value); - } - - $loggerService->debug('IssuerStateRule: Valid issuer state: ' . $issuerState); - return new Result($this->getKey(), $issuerState); } } diff --git a/src/Server/Validators/BearerTokenValidator.php b/src/Server/Validators/BearerTokenValidator.php index 3eaded18..b1afbd23 100644 --- a/src/Server/Validators/BearerTokenValidator.php +++ b/src/Server/Validators/BearerTokenValidator.php @@ -72,13 +72,11 @@ public function setPublicKey(CryptKey $key): void */ protected function initJwtConfiguration(): void { + /** @psalm-suppress ArgumentTypeCoercion */ $this->jwtConfiguration = Configuration::forSymmetricSigner( $this->moduleConfig->getProtocolSigner(), InMemory::plainText('empty', 'empty'), - ); - - /** @psalm-suppress DeprecatedMethod, ArgumentTypeCoercion */ - $this->jwtConfiguration->setValidationConstraints( + )->withValidationConstraints( new StrictValidAt(new SystemClock(new DateTimeZone(date_default_timezone_get()))), new SignedWith( $this->moduleConfig->getProtocolSigner(), diff --git a/src/Services/DatabaseMigration.php b/src/Services/DatabaseMigration.php index 8256ba90..06020ef2 100644 --- a/src/Services/DatabaseMigration.php +++ b/src/Services/DatabaseMigration.php @@ -199,6 +199,16 @@ public function migrate(): void $this->version20250917163000(); $this->database->write("INSERT INTO $versionsTablename (version) VALUES ('20250917163000')"); } + + if (!in_array('20251021000001', $versions, true)) { + $this->version20251021000001(); + $this->database->write("INSERT INTO $versionsTablename (version) VALUES ('20251021000001')"); + } + + if (!in_array('20251021000002', $versions, true)) { + $this->version20251021000002(); + $this->database->write("INSERT INTO $versionsTablename (version) VALUES ('20251021000002')"); + } } private function versionsTableName(): string @@ -678,6 +688,26 @@ private function version20250917163000(): void ,); } + private function version20251021000001(): void + { + $authCodeTableName = $this->database->applyPrefix(AuthCodeRepository::TABLE_NAME); + $this->database->write(<<< EOT + ALTER TABLE {$authCodeTableName} + ADD issuer_state TEXT NULL +EOT + ,); + } + + private function version20251021000002(): void + { + $accessTokenTableName = $this->database->applyPrefix(AccessTokenRepository::TABLE_NAME); + $this->database->write(<<< EOT + ALTER TABLE {$accessTokenTableName} + ADD issuer_state TEXT NULL +EOT + ,); + } + /** * @param string[] $columnNames */ diff --git a/tests/integration/src/Repositories/AccessTokenRepositoryTest.php b/tests/integration/src/Repositories/AccessTokenRepositoryTest.php index 1eab1395..6ace21ac 100644 --- a/tests/integration/src/Repositories/AccessTokenRepositoryTest.php +++ b/tests/integration/src/Repositories/AccessTokenRepositoryTest.php @@ -120,6 +120,7 @@ public function setUp(): void 'authorization_details' => null, 'bound_client_id' => null, 'bound_redirect_uri' => null, + 'issuer_state' => null, ]; $this->accessTokenEntityMock = $this->createMock(AccessTokenEntity::class); diff --git a/tests/unit/src/Entities/AuthCodeEntityTest.php b/tests/unit/src/Entities/AuthCodeEntityTest.php index 877420eb..6b0be802 100644 --- a/tests/unit/src/Entities/AuthCodeEntityTest.php +++ b/tests/unit/src/Entities/AuthCodeEntityTest.php @@ -106,6 +106,7 @@ public function testCanGetState(): void 'authorization_details' => null, 'bound_client_id' => null, 'bound_redirect_uri' => null, + 'issuer_state' => null, ], ); }