diff --git a/docker/conformance.sql b/docker/conformance.sql index 2037da69..1bbcf6cc 100644 --- a/docker/conformance.sql +++ b/docker/conformance.sql @@ -30,6 +30,7 @@ INSERT INTO oidc_migration_versions VALUES('20250917163000'); INSERT INTO oidc_migration_versions VALUES('20251021000001'); INSERT INTO oidc_migration_versions VALUES('20251021000002'); INSERT INTO oidc_migration_versions VALUES('20260109000001'); +INSERT INTO oidc_migration_versions VALUES('20260218163000'); CREATE TABLE oidc_user ( id VARCHAR(191) PRIMARY KEY NOT NULL, claims TEXT, @@ -59,17 +60,16 @@ CREATE TABLE oidc_client ( updated_at TIMESTAMP NULL DEFAULT NULL, created_at TIMESTAMP NULL DEFAULT NULL, expires_at TIMESTAMP NULL DEFAULT NULL, - is_federated BOOLEAN NOT NULL DEFAULT false, is_generic BOOLEAN NOT NULL DEFAULT false, extra_metadata TEXT NULL ); -- Used 'httpd' host for back-channel logout url (https://httpd:8443/test/a/simplesamlphp-module-oidc/backchannel_logout) -- since this is the hostname of conformance server while running in container environment -INSERT INTO oidc_client VALUES('_55a99a1d298da921cb27d700d4604352e51171ebc4','_8967dd97d07cc59db7055e84ac00e79005157c1132','Conformance Client 1',replace('Client 1 for Conformance Testing https://openid.net/certification/connect_op_testing/\n','\n',char(10)),'example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","address","phone","offline_access"]',1,1,NULL,'["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/post_logout_redirect"]','https://httpd:8443/test/a/simplesamlphp-module-oidc/backchannel_logout',NULL,NULL, NULL, NULL, NULL, NULL, 'manual', NULL, NULL, NULL, false, false, NULL); -INSERT INTO oidc_client VALUES('_34efb61060172a11d62101bc804db789f8f9100b0e','_91a4607a1c10ba801268929b961b3f6c067ff82d21','Conformance Client 2','','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","offline_access"]',1,1,NULL,NULL,NULL,NULL,NULL, NULL, NULL, NULL, NULL, 'manual', NULL, NULL, NULL, false, false, NULL); -INSERT INTO oidc_client VALUES('_0afb7d18e54b2de8205a93e38ca119e62ee321d031','_944e73bbeec7850d32b68f1b5c780562c955967e4e','Conformance Client 3','Client for client_secret_post','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email"]',1,1,NULL,NULL,NULL,NULL,NULL, NULL, NULL, NULL, NULL, 'manual', NULL, NULL, NULL, false, false, NULL); -INSERT INTO oidc_client VALUES('_8957eda35234902ba8343c0cdacac040310f17dfca','_322d16999f9da8b5abc9e9c0c08e853f60f4dc4804','RP-Initiated Logout Client','Client for testing RP-Initiated Logout','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","address","phone"]',1,1,NULL,'["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/post_logout_redirect"]',NULL,NULL,NULL, NULL, NULL, NULL, NULL, 'manual', NULL, NULL, NULL, false, false, NULL); -INSERT INTO oidc_client VALUES('_9fe2f7589ece1b71f5ef75a91847d71bc5125ec2a6','_3c0beb20194179c01d7796c6836f62801e9ed4b368','Back-Channel Logout Client','Client for testing Back-Channel Logout','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","address","phone"]',1,1,NULL,'["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/post_logout_redirect"]','https://httpd:8443/test/a/simplesamlphp-module-oidc/backchannel_logout',NULL,NULL, NULL, NULL, NULL, NULL, 'manual', NULL, NULL, NULL, false, false, NULL); +INSERT INTO oidc_client VALUES('_55a99a1d298da921cb27d700d4604352e51171ebc4','_8967dd97d07cc59db7055e84ac00e79005157c1132','Conformance Client 1',replace('Client 1 for Conformance Testing https://openid.net/certification/connect_op_testing/\n','\n',char(10)),'example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","address","phone","offline_access"]',1,1,NULL,'["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/post_logout_redirect"]','https://httpd:8443/test/a/simplesamlphp-module-oidc/backchannel_logout',NULL,NULL, NULL, NULL, NULL, NULL, 'manual', NULL, NULL, NULL, false, NULL); +INSERT INTO oidc_client VALUES('_34efb61060172a11d62101bc804db789f8f9100b0e','_91a4607a1c10ba801268929b961b3f6c067ff82d21','Conformance Client 2','','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","offline_access"]',1,1,NULL,NULL,NULL,NULL,NULL, NULL, NULL, NULL, NULL, 'manual', NULL, NULL, NULL, false, NULL); +INSERT INTO oidc_client VALUES('_0afb7d18e54b2de8205a93e38ca119e62ee321d031','_944e73bbeec7850d32b68f1b5c780562c955967e4e','Conformance Client 3','Client for client_secret_post','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email"]',1,1,NULL,NULL,NULL,NULL,NULL, NULL, NULL, NULL, NULL, 'manual', NULL, NULL, NULL, false, NULL); +INSERT INTO oidc_client VALUES('_8957eda35234902ba8343c0cdacac040310f17dfca','_322d16999f9da8b5abc9e9c0c08e853f60f4dc4804','RP-Initiated Logout Client','Client for testing RP-Initiated Logout','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","address","phone"]',1,1,NULL,'["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/post_logout_redirect"]',NULL,NULL,NULL, NULL, NULL, NULL, NULL, 'manual', NULL, NULL, NULL, false, NULL); +INSERT INTO oidc_client VALUES('_9fe2f7589ece1b71f5ef75a91847d71bc5125ec2a6','_3c0beb20194179c01d7796c6836f62801e9ed4b368','Back-Channel Logout Client','Client for testing Back-Channel Logout','example-userpass','["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/callback","https:\/\/www.certification.openid.net\/test\/a\/simplesamlphp-module-oidc\/callback"]','["openid","profile","email","address","phone"]',1,1,NULL,'["https:\/\/localhost.emobix.co.uk:8443\/test\/a\/simplesamlphp-module-oidc\/post_logout_redirect"]','https://httpd:8443/test/a/simplesamlphp-module-oidc/backchannel_logout',NULL,NULL, NULL, NULL, NULL, NULL, 'manual', NULL, NULL, NULL, false, NULL); CREATE TABLE oidc_access_token ( id VARCHAR(191) PRIMARY KEY NOT NULL, scopes TEXT, diff --git a/docs/6-oidc-upgrade.md b/docs/6-oidc-upgrade.md index b78d1e9c..1c2ef70f 100644 --- a/docs/6-oidc-upgrade.md +++ b/docs/6-oidc-upgrade.md @@ -5,6 +5,9 @@ apply those relevant to your deployment. ## Version 6 to 7 +As the database schema has been updated, you will have to run the DB migrations +to bring your local database schema up to date. + New features: - Instance can now be configured to support multiple algorithms and signature @@ -80,10 +83,20 @@ This would allow multiple values (array of values) for standard claims which have a single value by specification. All [standard claims](https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims) are now hardcoded to have a single value, even when the 'are_multiple_claim_values_allowed' option is enabled. +- OpenID Federation specific endpoints for subordinate listing and fetching +statements about subordinates are removed, as the final specification +explicitly states that leaf entities must not have those endpoints. +This effectively means that this OP implementation can only be a leaf entity +in the federation context, and not a federation operator or intermediary entity. Medium impact changes: Low-impact changes: +- Client property `is_federated` has been removed, as the OP implementation +can now only be a leaf entity in the federation context, and not a federation +operator or intermediary entity. Previously, this property was used to +indicate whether the client is a federated client or not, but now it is not +needed since the OP implementation can only be a leaf entity ## Version 5 to 6 diff --git a/routing/routes/routes.php b/routing/routes/routes.php index 16b7b01c..58a82096 100644 --- a/routing/routes/routes.php +++ b/routing/routes/routes.php @@ -17,7 +17,6 @@ use SimpleSAML\Module\oidc\Controllers\ConfigurationDiscoveryController; use SimpleSAML\Module\oidc\Controllers\EndSessionController; use SimpleSAML\Module\oidc\Controllers\Federation\EntityStatementController; -use SimpleSAML\Module\oidc\Controllers\Federation\SubordinateListingsController; use SimpleSAML\Module\oidc\Controllers\JwksController; use SimpleSAML\Module\oidc\Controllers\OAuth2\OAuth2ServerConfigurationController; use SimpleSAML\Module\oidc\Controllers\UserInfoController; @@ -114,14 +113,6 @@ ->controller([EntityStatementController::class, 'configuration']) ->methods([HttpMethodsEnum::GET->value]); - $routes->add(RoutesEnum::FederationFetch->name, RoutesEnum::FederationFetch->value) - ->controller([EntityStatementController::class, 'fetch']) - ->methods([HttpMethodsEnum::GET->value]); - - $routes->add(RoutesEnum::FederationList->name, RoutesEnum::FederationList->value) - ->controller([SubordinateListingsController::class, 'list']) - ->methods([HttpMethodsEnum::GET->value]); - /***************************************************************************************************************** * OpenID for Verifiable Credential Issuance ****************************************************************************************************************/ diff --git a/src/Controllers/Admin/ClientController.php b/src/Controllers/Admin/ClientController.php index 6cbf67eb..7f7ff7ef 100644 --- a/src/Controllers/Admin/ClientController.php +++ b/src/Controllers/Admin/ClientController.php @@ -348,7 +348,6 @@ protected function buildClientEntityFromFormData( $jwksUri = empty($data[ClientEntity::KEY_JWKS_URI]) ? null : (string)$data[ClientEntity::KEY_JWKS_URI]; $signedJwksUri = empty($data[ClientEntity::KEY_SIGNED_JWKS_URI]) ? null : (string)$data[ClientEntity::KEY_SIGNED_JWKS_URI]; - $isFederated = (bool)$data[ClientEntity::KEY_IS_FEDERATED]; $idTokenSignedResponseAlg = isset($data[ClaimsEnum::IdTokenSignedResponseAlg->value]) && is_string($data[ClaimsEnum::IdTokenSignedResponseAlg->value]) ? @@ -382,7 +381,6 @@ protected function buildClientEntityFromFormData( $updatedAt, $createdAt, $expiresAt, - $isFederated, $isGeneric, $extraMetadata, ); diff --git a/src/Controllers/Federation/EntityStatementController.php b/src/Controllers/Federation/EntityStatementController.php index 7e089776..7b522edf 100644 --- a/src/Controllers/Federation/EntityStatementController.php +++ b/src/Controllers/Federation/EntityStatementController.php @@ -6,7 +6,6 @@ use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; -use SimpleSAML\Module\oidc\Repositories\ClientRepository; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Services\LoggerService; use SimpleSAML\Module\oidc\Services\OpMetadataService; @@ -16,11 +15,9 @@ use SimpleSAML\OpenID\Codebooks\ClientRegistrationTypesEnum; use SimpleSAML\OpenID\Codebooks\ContentTypesEnum; use SimpleSAML\OpenID\Codebooks\EntityTypesEnum; -use SimpleSAML\OpenID\Codebooks\ErrorsEnum; use SimpleSAML\OpenID\Codebooks\HttpHeadersEnum; use SimpleSAML\OpenID\Federation; use SimpleSAML\OpenID\Jwks; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; class EntityStatementController @@ -35,7 +32,6 @@ public function __construct( protected readonly ModuleConfig $moduleConfig, protected readonly Jwks $jwks, protected readonly OpMetadataService $opMetadataService, - protected readonly ClientRepository $clientRepository, protected readonly Helpers $helpers, protected readonly Routes $routes, protected readonly Federation $federation, @@ -224,113 +220,6 @@ public function configuration(): Response return $this->prepareEntityStatementResponse($entityConfigurationToken); } - public function fetch(Request $request): Response - { - $subject = $request->query->getString(ClaimsEnum::Sub->value); - - if (empty($subject)) { - return $this->routes->newJsonErrorResponse( - ErrorsEnum::InvalidRequest->value, - sprintf('Missing parameter %s', ClaimsEnum::Sub->value), - 400, - ); - } - - /** @var non-empty-string $subject */ - - $cachedSubordinateStatement = $this->federationCache?->get( - null, - self::KEY_RP_SUBORDINATE_ENTITY_STATEMENT, - $subject, - ); - - if (!is_null($cachedSubordinateStatement)) { - return $this->prepareEntityStatementResponse((string)$cachedSubordinateStatement); - } - - $client = $this->clientRepository->findFederatedByEntityIdentifier($subject); - if (empty($client)) { - return $this->routes->newJsonErrorResponse( - ErrorsEnum::NotFound->value, - sprintf('Subject not found (%s)', $subject), - 404, - ); - } - - $jwks = $client->getFederationJwks(); - if (empty($jwks)) { - return $this->routes->newJsonErrorResponse( - ErrorsEnum::InvalidClient->value, - sprintf('Subject does not contain JWKS claim (%s)', $subject), - 401, - ); - } - - $currentTimestamp = $this->helpers->dateTime()->getUtc()->getTimestamp(); - - $payload = [ - ClaimsEnum::Iss->value => $this->moduleConfig->getIssuer(), - ClaimsEnum::Iat->value => $currentTimestamp, - ClaimsEnum::Jti->value => $this->helpers->random()->getIdentifier(), - - ClaimsEnum::Sub->value => $subject, - ClaimsEnum::Exp->value => $this->helpers->dateTime()->getUtc()->add( - $this->moduleConfig->getFederationEntityStatementDuration(), - )->getTimestamp(), - ClaimsEnum::Jwks->value => $jwks, - ClaimsEnum::Metadata->value => [ - EntityTypesEnum::OpenIdRelyingParty->value => [ - ClaimsEnum::ClientName->value => $client->getName(), - ClaimsEnum::ClientId->value => $client->getIdentifier(), - ClaimsEnum::RedirectUris->value => $client->getRedirectUris(), - ClaimsEnum::Scope->value => implode(' ', $client->getScopes()), - ClaimsEnum::ClientRegistrationTypes->value => $client->getClientRegistrationTypes(), - // Optional claims... - ...(array_filter( - [ - ClaimsEnum::BackChannelLogoutUri->value => $client->getBackChannelLogoutUri(), - ClaimsEnum::PostLogoutRedirectUris->value => $client->getPostLogoutRedirectUri(), - ], - )), - // TODO v7 mivanci Continue - // https://openid.net/specs/openid-connect-registration-1_0.html#ClientMetadata - // https://www.iana.org/assignments/oauth-parameters/oauth-parameters.xhtml#client-metadata - ], - ], - ]; - - // TODO v7 mivanci Continue - // Note: claims which can be present in subordinate statements: - // * metadata_policy - // * constraints - // * metadata_policy_crit - - $signingKeyPair = $this->moduleConfig - ->getFederationSignatureKeyPairBag() - ->getFirstOrFail(); - - - $header = [ - ClaimsEnum::Kid->value => $signingKeyPair->getKeyPair()->getKeyId(), - ]; - - $subordinateStatementToken = $this->federation->entityStatementFactory()->fromData( - $signingKeyPair->getKeyPair()->getPrivateKey(), - $signingKeyPair->getSignatureAlgorithm(), - $payload, - $header, - )->getToken(); - - $this->federationCache?->set( - $subordinateStatementToken, - $this->moduleConfig->getFederationEntityStatementCacheDurationForProduced(), - self::KEY_RP_SUBORDINATE_ENTITY_STATEMENT, - $subject, - ); - - return $this->prepareEntityStatementResponse($subordinateStatementToken); - } - protected function prepareEntityStatementResponse(string $entityStatementToken): Response { return $this->routes->newResponse( diff --git a/src/Controllers/Federation/SubordinateListingsController.php b/src/Controllers/Federation/SubordinateListingsController.php deleted file mode 100644 index ef6b6a65..00000000 --- a/src/Controllers/Federation/SubordinateListingsController.php +++ /dev/null @@ -1,69 +0,0 @@ -moduleConfig->getFederationEnabled()) { - throw OidcServerException::forbidden('federation capabilities not enabled'); - } - } - - public function list(Request $request): Response - { - // If an unsupported query parameter is provided, we have to respond with an error: "If the responder does not - // support this feature, it MUST use the HTTP status code 400 and the content type application/json, with - // the error code unsupported_parameter." - - // Currently, we don't support any of the mentioned params in the spec, so let's return an error for - // any of them. - $unsupportedParams = [ - ParamsEnum::EntityType->value, - ParamsEnum::TrustMarked->value, - ParamsEnum::TrustMarkType->value, - ParamsEnum::Intermediate->value, - ]; - - $requestedParams = array_keys($request->query->all()); - - if (!empty($intersectedParams = array_intersect($unsupportedParams, $requestedParams))) { - return $this->routes->newJsonErrorResponse( - ErrorsEnum::UnsupportedParameter->value, - 'Unsupported parameter: ' . implode(', ', $intersectedParams), - 400, - ); - } - - $subordinateEntityIdList = array_filter(array_map( - function (ClientEntityInterface $clientEntity): ?string { - return $clientEntity->getEntityIdentifier(); - }, - $this->clientRepository->findAllFederated(), - )); - - return $this->routes->newJsonResponse( - $subordinateEntityIdList, - headers: ['Access-Control-Allow-Origin' => '*'], - ); - } -} diff --git a/src/Entities/ClientEntity.php b/src/Entities/ClientEntity.php index 3e901beb..0d5add92 100644 --- a/src/Entities/ClientEntity.php +++ b/src/Entities/ClientEntity.php @@ -51,7 +51,6 @@ class ClientEntity implements ClientEntityInterface public const KEY_UPDATED_AT = 'updated_at'; public const KEY_CREATED_AT = 'created_at'; public const KEY_EXPIRES_AT = 'expires_at'; - public const KEY_IS_FEDERATED = 'is_federated'; public const KEY_IS_GENERIC = 'is_generic'; public const KEY_EXTRA_METADATA = 'extra_metadata'; @@ -95,7 +94,6 @@ class ClientEntity implements ClientEntityInterface private ?DateTimeImmutable $updatedAt; private ?DateTimeImmutable $createdAt; private ?DateTimeImmutable $expiresAt; - private bool $isFederated; private bool $isGeneric; private ?array $extraMetadata; @@ -130,7 +128,6 @@ public function __construct( ?DateTimeImmutable $updatedAt = null, ?DateTimeImmutable $createdAt = null, ?DateTimeImmutable $expiresAt = null, - bool $isFederated = false, bool $isGeneric = false, ?array $extraMetadata = null, ) { @@ -156,7 +153,6 @@ public function __construct( $this->updatedAt = $updatedAt; $this->createdAt = $createdAt; $this->expiresAt = $expiresAt; - $this->isFederated = $isFederated; $this->isGeneric = $isGeneric; $this->extraMetadata = $extraMetadata; } @@ -196,7 +192,6 @@ public function getState(): array self::KEY_UPDATED_AT => $this->getUpdatedAt()?->format('Y-m-d H:i:s'), self::KEY_CREATED_AT => $this->getCreatedAt()?->format('Y-m-d H:i:s'), self::KEY_EXPIRES_AT => $this->getExpiresAt()?->format('Y-m-d H:i:s'), - self::KEY_IS_FEDERATED => $this->isFederated(), self::KEY_IS_GENERIC => $this->isGeneric(), self::KEY_EXTRA_METADATA => is_null($this->extraMetadata) ? null : @@ -229,7 +224,6 @@ public function toArray(): array self::KEY_UPDATED_AT => $this->updatedAt, self::KEY_CREATED_AT => $this->createdAt, self::KEY_EXPIRES_AT => $this->expiresAt, - self::KEY_IS_FEDERATED => $this->isFederated, self::KEY_IS_GENERIC => $this->isGeneric, // Extra metadata @@ -368,11 +362,6 @@ public function isExpired(): bool return $this->expiresAt !== null && $this->expiresAt < new DateTimeImmutable(); } - public function isFederated(): bool - { - return $this->isFederated; - } - public function isGeneric(): bool { return $this->isGeneric; diff --git a/src/Entities/Interfaces/ClientEntityInterface.php b/src/Entities/Interfaces/ClientEntityInterface.php index ac72cf71..dea9ff66 100644 --- a/src/Entities/Interfaces/ClientEntityInterface.php +++ b/src/Entities/Interfaces/ClientEntityInterface.php @@ -78,7 +78,6 @@ public function getUpdatedAt(): ?DateTimeImmutable; public function getCreatedAt(): ?DateTimeImmutable; public function getExpiresAt(): ?DateTimeImmutable; public function isExpired(): bool; - public function isFederated(): bool; public function isGeneric(): bool; public function getExtraMetadata(): array; diff --git a/src/Factories/Entities/ClientEntityFactory.php b/src/Factories/Entities/ClientEntityFactory.php index 7934b091..54358388 100644 --- a/src/Factories/Entities/ClientEntityFactory.php +++ b/src/Factories/Entities/ClientEntityFactory.php @@ -61,7 +61,6 @@ public function fromData( ?DateTimeImmutable $updatedAt = null, ?DateTimeImmutable $createdAt = null, ?DateTimeImmutable $expiresAt = null, - bool $isFederated = false, bool $isGeneric = false, ?array $extraMetadata = null, ): ClientEntityInterface { @@ -88,7 +87,6 @@ public function fromData( $updatedAt, $createdAt, $expiresAt, - $isFederated, $isGeneric, $extraMetadata, ); @@ -189,7 +187,6 @@ public function fromRegistrationData( // $expiresAt = $expiresAt; - $isFederated = $existingClient?->isFederated() ?? false; $isGeneric = $existingClient?->isGeneric() ?? false; $extraMetadata = $existingClient?->getExtraMetadata() ?? []; @@ -229,7 +226,6 @@ public function fromRegistrationData( $updatedAt, $createdAt, $expiresAt, - $isFederated, $isGeneric, $extraMetadata, ); @@ -360,7 +356,6 @@ public function fromState(array $state): ClientEntityInterface $expiresAt = empty($state[ClientEntity::KEY_EXPIRES_AT]) ? null : $this->helpers->dateTime()->getUtc((string)$state[ClientEntity::KEY_EXPIRES_AT]); - $isFederated = (bool)$state[ClientEntity::KEY_IS_FEDERATED]; $isGeneric = (bool)$state[ClientEntity::KEY_IS_GENERIC]; /** @var ?mixed[] $extraMetadata */ @@ -391,7 +386,6 @@ public function fromState(array $state): ClientEntityInterface $updatedAt, $createdAt, $expiresAt, - $isFederated, $isGeneric, $extraMetadata, ); diff --git a/src/Forms/ClientForm.php b/src/Forms/ClientForm.php index 96a55347..64c28196 100644 --- a/src/Forms/ClientForm.php +++ b/src/Forms/ClientForm.php @@ -417,9 +417,6 @@ protected function buildForm(): void $this->addText('signed_jwks_uri', 'Signed JWKS URI') ->setHtmlAttribute('class', 'full-width'); - $this->addCheckbox('is_federated', '{oidc:client:is_federated}') - ->setHtmlAttribute('class', 'full-width'); - // TODO mivanci Properly fetch the list of supported algos $this->addSelect('id_token_signed_response_alg', Translate::noop('ID Token Signing Algorithm')) ->setHtmlAttribute('class', 'full-width') diff --git a/src/Repositories/ClientRepository.php b/src/Repositories/ClientRepository.php index 718a0e1f..b6524d0a 100644 --- a/src/Repositories/ClientRepository.php +++ b/src/Repositories/ClientRepository.php @@ -199,7 +199,6 @@ public function findFederatedByEntityIdentifier( if ( is_null($clientEntity->getEntityIdentifier()) || (! $clientEntity->isEnabled()) || - (! $clientEntity->isFederated()) || (!is_array($clientEntity->getFederationJwks())) || $clientEntity->isExpired() ) { @@ -270,12 +269,10 @@ public function findAllFederated(?string $owner = null): array WHERE entity_identifier IS NOT NULL AND federation_jwks IS NOT NULL AND - is_enabled = :is_enabled AND - is_federated = :is_federated + is_enabled = :is_enabled EOS, [ 'is_enabled' => [true, PDO::PARAM_BOOL], - 'is_federated' => [true, PDO::PARAM_BOOL], ], $owner, ); @@ -361,7 +358,6 @@ public function add(ClientEntityInterface $client): void updated_at, created_at, expires_at, - is_federated, is_generic, extra_metadata ) @@ -388,7 +384,6 @@ public function add(ClientEntityInterface $client): void :updated_at, :created_at, :expires_at, - :is_federated, :is_generic, :extra_metadata ) @@ -462,7 +457,6 @@ public function update(ClientEntityInterface $client, ?string $owner = null): vo updated_at = :updated_at, created_at = :created_at, expires_at = :expires_at, - is_federated = :is_federated, is_generic = :is_generic, extra_metadata = :extra_metadata WHERE id = :id @@ -557,12 +551,10 @@ protected function preparePdoState(array $state): array { $isEnabled = (bool)($state[ClientEntity::KEY_IS_ENABLED] ?? false); $isConfidential = (bool)($state[ClientEntity::KEY_IS_CONFIDENTIAL] ?? false); - $isFederated = (bool)($state[ClientEntity::KEY_IS_FEDERATED] ?? false); $isGeneric = (bool)($state[ClientEntity::KEY_IS_GENERIC] ?? false); $state[ClientEntity::KEY_IS_ENABLED] = [$isEnabled, PDO::PARAM_BOOL]; $state[ClientEntity::KEY_IS_CONFIDENTIAL] = [$isConfidential, PDO::PARAM_BOOL]; - $state[ClientEntity::KEY_IS_FEDERATED] = [$isFederated, PDO::PARAM_BOOL]; $state[ClientEntity::KEY_IS_GENERIC] = [$isGeneric, PDO::PARAM_BOOL]; return $state; diff --git a/src/Services/DatabaseMigration.php b/src/Services/DatabaseMigration.php index ae026915..2eadd226 100644 --- a/src/Services/DatabaseMigration.php +++ b/src/Services/DatabaseMigration.php @@ -214,6 +214,11 @@ public function migrate(): void $this->version20260109000001(); $this->database->write("INSERT INTO $versionsTablename (version) VALUES ('20260109000001')"); } + + if (!in_array('20260218163000', $versions, true)) { + $this->version20260218163000(); + $this->database->write("INSERT INTO $versionsTablename (version) VALUES ('20260218163000')"); + } } private function versionsTableName(): string @@ -723,6 +728,17 @@ private function version20260109000001(): void ,); } + + private function version20260218163000(): void + { + $clientTableName = $this->database->applyPrefix(ClientRepository::TABLE_NAME); + $this->database->write(<<< EOT + ALTER TABLE {$clientTableName} + DROP COLUMN is_federated; +EOT + ,); + } + /** * @param string[] $columnNames */ diff --git a/templates/clients/includes/form.twig b/templates/clients/includes/form.twig index 27cce2a5..51b1a869 100644 --- a/templates/clients/includes/form.twig +++ b/templates/clients/includes/form.twig @@ -152,27 +152,6 @@

{{ 'OpenID Federation Related Properties'|trans }}

- - {% trans %}In order for an entity to participate in federation contexts (for example, to be listed as subordinate to this OP), it must have an Entity Identifier and Federation JWKS set. {% endtrans %} - - - - - - {% trans %}Choose if the client is allowed to participate in federation context or not.{% endtrans %} - {{ form.entity_identifier.control | raw }} diff --git a/templates/clients/show.twig b/templates/clients/show.twig index 5aa6fc71..307de574 100644 --- a/templates/clients/show.twig +++ b/templates/clients/show.twig @@ -234,14 +234,6 @@ - - - {{ 'Is Federated'|trans }} - - - {{ (client.isFederated ? 'Yes' : 'No')|trans }} - - {{ 'Entity Identifier'|trans }} diff --git a/tests/unit/src/Controllers/Admin/ClientControllerTest.php b/tests/unit/src/Controllers/Admin/ClientControllerTest.php index f591f966..c223d899 100644 --- a/tests/unit/src/Controllers/Admin/ClientControllerTest.php +++ b/tests/unit/src/Controllers/Admin/ClientControllerTest.php @@ -86,7 +86,6 @@ class ClientControllerTest extends TestCase ], 'jwks_uri' => 'https://example.com/jwks', 'signed_jwks_uri' => 'https://example.com/signed-jwks', - 'is_federated' => true, ]; protected function setUp(): void diff --git a/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php b/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php index 0c7416dd..eeda4fe0 100644 --- a/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php +++ b/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php @@ -10,7 +10,6 @@ use SimpleSAML\Module\oidc\Controllers\Federation\EntityStatementController; use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; -use SimpleSAML\Module\oidc\Repositories\ClientRepository; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Services\LoggerService; use SimpleSAML\Module\oidc\Services\OpMetadataService; @@ -25,7 +24,6 @@ class EntityStatementControllerTest extends TestCase protected MockObject $moduleConfigMock; protected MockObject $jwksMock; protected MockObject $opMetadataServiceMock; - protected MockObject $clientRepositoryMock; protected MockObject $helpersMock; protected MockObject $routesMock; protected MockObject $federationMock; @@ -38,7 +36,6 @@ protected function setUp(): void $this->moduleConfigMock = $this->createMock(ModuleConfig::class); $this->jwksMock = $this->createMock(Jwks::class); $this->opMetadataServiceMock = $this->createMock(OpMetadataService::class); - $this->clientRepositoryMock = $this->createMock(ClientRepository::class); $this->helpersMock = $this->createMock(Helpers::class); $this->routesMock = $this->createMock(Routes::class); $this->federationMock = $this->createMock(Federation::class); @@ -50,7 +47,6 @@ protected function sut( ?ModuleConfig $moduleConfig = null, ?Jwks $jwks = null, ?OpMetadataService $opMetadataService = null, - ?ClientRepository $clientRepository = null, ?Helpers $helpers = null, ?Routes $routes = null, ?Federation $federation = null, @@ -60,7 +56,6 @@ protected function sut( $moduleConfig ??= $this->moduleConfigMock; $jwks ??= $this->jwksMock; $opMetadataService ??= $this->opMetadataServiceMock; - $clientRepository ??= $this->clientRepositoryMock; $helpers ??= $this->helpersMock; $routes ??= $this->routesMock; $federation ??= $this->federationMock; @@ -71,7 +66,6 @@ protected function sut( $moduleConfig, $jwks, $opMetadataService, - $clientRepository, $helpers, $routes, $federation, diff --git a/tests/unit/src/Controllers/Federation/SubordinateListingsControllerTest.php b/tests/unit/src/Controllers/Federation/SubordinateListingsControllerTest.php deleted file mode 100644 index d1396f1c..00000000 --- a/tests/unit/src/Controllers/Federation/SubordinateListingsControllerTest.php +++ /dev/null @@ -1,110 +0,0 @@ -moduleConfigMock = $this->createMock(ModuleConfig::class); - $this->clientRepositoryMock = $this->createMock(ClientRepository::class); - $this->routesMock = $this->createMock(Routes::class); - - $this->isFederationEnabled = true; - - $this->requestMock = $this->createMock(Request::class); - $this->requestQueryMock = $this->createMock(ParameterBag::class); - $this->requestMock->query = $this->requestQueryMock; - } - - public function sut( - ?ModuleConfig $moduleConfig = null, - ?ClientRepository $clientRepository = null, - ?Routes $routes = null, - ?bool $federationEnabled = null, - ): SubordinateListingsController { - $federationEnabled = $federationEnabled ?? $this->isFederationEnabled; - $this->moduleConfigMock->method('getFederationEnabled')->willReturn($federationEnabled); - - $moduleConfig = $moduleConfig ?? $this->moduleConfigMock; - $clientRepository = $clientRepository ?? $this->clientRepositoryMock; - $routes = $routes ?? $this->routesMock; - - return new SubordinateListingsController( - $moduleConfig, - $clientRepository, - $routes, - ); - } - - public function testCanConstruct(): void - { - $this->assertInstanceOf(SubordinateListingsController::class, $this->sut()); - } - - public function testThrowsIfFederationNotEnabled(): void - { - $this->expectException(OidcServerException::class); - $this->expectExceptionMessage('refused'); - - $this->sut(federationEnabled: false); - } - - public function testCanListFederatedEntities(): void - { - $client = $this->createMock(ClientEntityInterface::class); - $client->method('getEntityIdentifier')->willReturn('entity-id'); - - $federatedEntities = [ - $client, - ]; - - $this->clientRepositoryMock->expects($this->once())->method('findAllFederated') - ->willReturn($federatedEntities); - - $this->routesMock->expects($this->once())->method('newJsonResponse') - ->with([ - $client->getEntityIdentifier(), - ]); - - $this->sut()->list($this->requestMock); - } - - public function testListReturnsErrorOnUnsuportedQueryParameter(): void - { - $this->requestQueryMock->method('all')->willReturn([ - ParamsEnum::EntityType->value => 'something', - ]); - - $this->routesMock->expects($this->once())->method('newJsonErrorResponse') - ->with(ErrorsEnum::UnsupportedParameter->value); - - $this->sut()->list($this->requestMock); - } -} diff --git a/tests/unit/src/Entities/ClientEntityTest.php b/tests/unit/src/Entities/ClientEntityTest.php index ba082935..acca0582 100644 --- a/tests/unit/src/Entities/ClientEntityTest.php +++ b/tests/unit/src/Entities/ClientEntityTest.php @@ -37,7 +37,6 @@ class ClientEntityTest extends TestCase protected ?DateTimeImmutable $updatedAt = null; protected ?DateTimeImmutable $createdAt = null; protected ?DateTimeImmutable $expiresAt = null; - protected bool $isFederated = false; protected bool $isGeneric = false; protected function setUp(): void @@ -59,7 +58,6 @@ protected function setUp(): void 'updated_at' => null, 'created_at' => null, 'expires_at' => null, - 'is_federated' => false, 'is_generic' => false, ]; } @@ -93,7 +91,6 @@ public function mock(): ClientEntity $this->updatedAt, $this->createdAt, $this->expiresAt, - $this->isFederated, $this->isGeneric, ); } @@ -185,7 +182,6 @@ public function testCanGetState(): void 'updated_at' => null, 'created_at' => null, 'expires_at' => null, - 'is_federated' => $this->state['is_federated'], 'is_generic' => $this->state['is_generic'], 'extra_metadata' => null, ], @@ -223,7 +219,6 @@ public function testCanExportAsArray(): void 'updated_at' => null, 'created_at' => null, 'expires_at' => null, - 'is_federated' => false, 'is_generic' => false, 'id_token_signed_response_alg' => null, ], diff --git a/tests/unit/src/Forms/ClientFormTest.php b/tests/unit/src/Forms/ClientFormTest.php index 1134c0b3..425c3d2e 100644 --- a/tests/unit/src/Forms/ClientFormTest.php +++ b/tests/unit/src/Forms/ClientFormTest.php @@ -80,7 +80,6 @@ public function setUp(): void ['date' => '2024-12-01 11:54:12.000000', 'timezone_type' => 3, 'timezone' => 'UTC',], ), 'expires_at' => null, - 'is_federated' => false, 'allowed_origin' => [], ]; } diff --git a/tests/unit/src/Repositories/ClientRepositoryTest.php b/tests/unit/src/Repositories/ClientRepositoryTest.php index 5da45064..57f3893e 100644 --- a/tests/unit/src/Repositories/ClientRepositoryTest.php +++ b/tests/unit/src/Repositories/ClientRepositoryTest.php @@ -406,7 +406,7 @@ public function testCanFindByEntityIdentifier(): void public function testCanFindFederatedByEntityIdentifier(): void { - $client = self::getClient(id: 'clientId', entityId: 'entityId', isFederated: true, federationJwks: []); + $client = self::getClient(id: 'clientId', entityId: 'entityId', federationJwks: []); $this->repository->add($client); $this->clientEntityFactoryMock->expects($this->once())->method('fromState')->willReturn($client); @@ -436,7 +436,7 @@ public function testCanNotFindFederatedByEntityIdentifierIfMissingFederationAttr public function testCanFindAllFederated(): void { - $client = self::getClient(id: 'clientId', entityId: 'entityId', isFederated: true, federationJwks: []); + $client = self::getClient(id: 'clientId', entityId: 'entityId', federationJwks: []); $this->repository->add($client); $this->clientEntityFactoryMock->expects($this->atLeastOnce())->method('fromState')->willReturn($client); @@ -469,7 +469,6 @@ public static function getClient( bool $confidential = false, ?string $owner = null, ?string $entityId = null, - bool $isFederated = false, ?array $federationJwks = null, ): ClientEntityInterface { return new ClientEntity( @@ -485,7 +484,6 @@ public static function getClient( owner: $owner, entityIdentifier: $entityId, federationJwks: $federationJwks, - isFederated: $isFederated, ); } }