diff --git a/README.md b/README.md index e372db67..34810467 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,7 @@ configuration. Currently, the following OIDF features are supported: * automatic client registration using a Request Object (passing it by value) +* federation participation limiting based on Trust Marks * endpoint for issuing configuration entity statement (statement about itself) * fetch endpoint for issuing statements about subordinates (registered clients) diff --git a/UPGRADE.md b/UPGRADE.md index 14ac992c..9cc95e34 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -10,10 +10,11 @@ - Key rollover support - you can now define additional (new) private / public key pair which will be published on relevant JWKS endpoint or contained in JWKS property. In this way, you can "announce" new public key which can then be fetched by RPs, and do the switch between "old" and "new" key pair when you find appropriate. -- OpenID capabilities - - New federation endpoints: - - endpoint for issuing configuration entity statement (statement about itself) - - fetch endpoint for issuing statements about subordinates (registered clients) +- OpenID Federation capabilities: + - Automatic client registration using a Request Object (passing it by value) + - Federation participation limiting based on Trust Marks + - Endpoint for issuing configuration entity statement (statement about itself) + - Fetch endpoint for issuing statements about subordinates (registered clients) - Clients can now be configured with new properties: - Entity Identifier - Supported OpenID Federation Registration Types diff --git a/composer.json b/composer.json index 3c229366..f1c3a1a6 100644 --- a/composer.json +++ b/composer.json @@ -31,7 +31,7 @@ "psr/container": "^2.0", "psr/log": "^3", "simplesamlphp/composer-module-installer": "^1.3", - "simplesamlphp/openid": "dev-master", + "simplesamlphp/openid": "^0", "spomky-labs/base64url": "^2.0", "symfony/expression-language": "^6.3", "symfony/psr-http-message-bridge": "^7.1", diff --git a/rector.php b/rector.php index a65521ec..5fefbb9f 100644 --- a/rector.php +++ b/rector.php @@ -16,7 +16,7 @@ ]); $rectorConfig->paths([ - // TODO mivanci also go trough commented out paths... + // TODO v7 mivanci also go trough commented out paths... //__DIR__ . '/docker', //__DIR__ . '/hooks', //__DIR__ . '/public', diff --git a/routing/routes/routes.php b/routing/routes/routes.php index d014c72d..f05ac6ac 100644 --- a/routing/routes/routes.php +++ b/routing/routes/routes.php @@ -96,8 +96,4 @@ $routes->add(RoutesEnum::FederationFetch->name, RoutesEnum::FederationFetch->value) ->controller([EntityStatementController::class, 'fetch']) ->methods([HttpMethodsEnum::GET->value]); - - // TODO mivanci delete - $routes->add('test', 'test') - ->controller(\SimpleSAML\Module\oidc\Controllers\Federation\Test::class); }; diff --git a/src/Bridges/SspBridge/Utils.php b/src/Bridges/SspBridge/Utils.php index 7b7a3705..f201faa6 100644 --- a/src/Bridges/SspBridge/Utils.php +++ b/src/Bridges/SspBridge/Utils.php @@ -4,6 +4,7 @@ namespace SimpleSAML\Module\oidc\Bridges\SspBridge; +use SimpleSAML\Utils\Attributes; use SimpleSAML\Utils\Auth; use SimpleSAML\Utils\Config; use SimpleSAML\Utils\HTTP; @@ -15,6 +16,7 @@ class Utils protected static ?HTTP $http = null; protected static ?Random $random = null; protected static ?Auth $auth = null; + protected static ?Attributes $attributes = null; public function config(): Config { @@ -35,4 +37,9 @@ public function auth(): Auth { return self::$auth ??= new Auth(); } + + public function attributes(): Attributes + { + return self::$attributes ??= new Attributes(); + } } diff --git a/src/Controllers/Admin/ClientController.php b/src/Controllers/Admin/ClientController.php index 903a9712..ec0f2c74 100644 --- a/src/Controllers/Admin/ClientController.php +++ b/src/Controllers/Admin/ClientController.php @@ -299,7 +299,7 @@ public function edit(Request $request): Response } /** - * TODO mivanci Move to ClientEntityFactory::fromRegistrationData on dynamic client registration implementation. + * TODO v7 mivanci Move to ClientEntityFactory::fromRegistrationData on dynamic client registration implementation. * @throws \SimpleSAML\Module\oidc\Exceptions\OidcException */ protected function buildClientEntityFromFormData( diff --git a/src/Controllers/EndSessionController.php b/src/Controllers/EndSessionController.php index dbe6e910..267aa57f 100644 --- a/src/Controllers/EndSessionController.php +++ b/src/Controllers/EndSessionController.php @@ -41,7 +41,7 @@ public function __construct( */ public function __invoke(ServerRequestInterface $request): Response { - // TODO Back-Channel Logout: https://openid.net/specs/openid-connect-backchannel-1_0.html + // TODO v7 Back-Channel Logout: https://openid.net/specs/openid-connect-backchannel-1_0.html // [] Refresh tokens issued without the offline_access property to a session being logged out SHOULD // be revoked. Refresh tokens issued with the offline_access property normally SHOULD NOT be revoked. // - offline_access scope is now handled. @@ -147,7 +147,7 @@ public static function logoutHandler(): void $sessionLogoutTickets = $sessionLogoutTicketStore->getAll(); if (!empty($sessionLogoutTickets)) { - // TODO low mivanci This could brake since interface does not mandate type. Move to strong typing. + // TODO v7 low mivanci This could brake since interface does not mandate type. Move to strong typing. /** @var array $sessionLogoutTicket */ foreach ($sessionLogoutTickets as $sessionLogoutTicket) { $sid = (string)$sessionLogoutTicket['sid']; diff --git a/src/Controllers/Federation/EntityStatementController.php b/src/Controllers/Federation/EntityStatementController.php index 25419f5b..cf7cee86 100644 --- a/src/Controllers/Federation/EntityStatementController.php +++ b/src/Controllers/Federation/EntityStatementController.php @@ -95,7 +95,7 @@ public function configuration(): Response )), ClaimsEnum::FederationFetchEndpoint->value => $this->moduleConfig->getModuleUrl(RoutesEnum::FederationFetch->value), - // TODO mivanci Add when ready. Use ClaimsEnum for keys. + // TODO v7 mivanci Add when ready. Use ClaimsEnum for keys. // https://openid.net/specs/openid-federation-1_0.html#name-federation-entity //'federation_list_endpoint', //'federation_resolve_endpoint', @@ -149,7 +149,7 @@ public function configuration(): Response $builder = $builder->withClaim(ClaimsEnum::TrustMarks->value, $trustMarks); } - // TODO mivanci Continue + // TODO v7 mivanci Continue // Remaining claims, add if / when ready. // * crit @@ -235,14 +235,14 @@ public function fetch(Request $request): Response ClaimsEnum::PostLogoutRedirectUris->value => $client->getPostLogoutRedirectUri(), ], )), - // TODO mivanci Continue + // 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 mivanci Continue + // TODO v7 mivanci Continue // Note: claims which can be present in subordinate statements: // * metadata_policy // * constraints diff --git a/src/Controllers/Federation/Test.php b/src/Controllers/Federation/Test.php deleted file mode 100644 index 8c10fd36..00000000 --- a/src/Controllers/Federation/Test.php +++ /dev/null @@ -1,152 +0,0 @@ -coreFactory->build()); -// $t = 'eyJ0eXAiOiJlbnRpdHktc3RhdGVtZW50K2p3dCIsImFsZyI6IlJTMjU2Iiwia2lkIjoiYzRhZmYzY2M3NDM5MWI3M2UxM2FhODE2OTdkYmYzODIifQ.eyJpc3MiOiJodHRwczovLzgyLWRhcC5sb2NhbGhvc3QubWFya29pdmFuY2ljLmZyb20uaHIiLCJpYXQiOjE3MjY4NTM0NTUsImp0aSI6IjQ0ZDQyNDQxOGIyZDEwOWY4M2FhNDMzY2Y0YTVhODNiMTI4YjgzZDZiZDExOTRjMDI1NTgzMTQ1YmZkMjNjMzZjZDg1Y2UzMzBjN2ZlOTc4Iiwic3ViIjoiaHR0cHM6Ly84Mi1kYXAubG9jYWxob3N0Lm1hcmtvaXZhbmNpYy5mcm9tLmhyIiwiZXhwIjoxNzI2OTM5ODU1LCJqd2tzIjp7ImtleXMiOlt7Imt0eSI6IlJTQSIsIm4iOiJzTHpnc0NiaW40Y0l1YUlFZ0w3QzBvaXZSazNyN09HSTBUdWJ0TFBYMkJiMmI5QmtPVElUcnhqSjIwenVVblVLbUJ5eGdyaFJUZGtVWW9EcFJOenVIUENyeVdwU0NQSDB5SUZPUVdxbEFxWHEzXzJheHcwTzlCMFVYVzYzQWNaRVBERVlVWGFsNHNaazE3OG9ZMTNhUlk0Um9NZm8yZkZ1cDlyb2RpSFJqU0gweWsxS2tEOWR5NjZGM1ZmaTF6SHRGQzhkV000clE5cW1OS3pyVFpXMzFsVmQ3N3ZvajZsNE1BOFlYWFVuM2dVMHRocUxMRFI3WnhJcFdUcU1VbzVDRXFJZ0pZS0FRUG5sZldvQ2JiMVhWSl9qMFNQZzZ0M29GNTUwNGd3SFp3M1dDSHJEbUxzdTdpa29CcmdrRWZnS05ISWlra3hXalB0bGNmbmlXUjl2b1EiLCJlIjoiQVFBQiIsImtpZCI6ImM0YWZmM2NjNzQzOTFiNzNlMTNhYTgxNjk3ZGJmMzgyIiwidXNlIjoic2lnIiwiYWxnIjoiUlMyNTYifV19LCJtZXRhZGF0YSI6eyJmZWRlcmF0aW9uX2VudGl0eSI6eyJvcmdhbml6YXRpb25fbmFtZSI6IkZvbyBjb3JwIiwiY29udGFjdHMiOlsiSm9obiBEb2UgamRvZUBleGFtcGxlLm9yZyJdLCJsb2dvX3VyaSI6Imh0dHBzOi8vZXhhbXBsZS5vcmcvbG9nbyIsInBvbGljeV91cmkiOiJodHRwczovL2V4YW1wbGUub3JnL3BvbGljeSIsImhvbWVwYWdlX3VyaSI6Imh0dHBzOi8vZXhhbXBsZS5vcmciLCJmZWRlcmF0aW9uX2ZldGNoX2VuZHBvaW50IjoiaHR0cHM6Ly84Mi1kYXAubG9jYWxob3N0Lm1hcmtvaXZhbmNpYy5mcm9tLmhyL3NpbXBsZXNhbWxwaHAvc2ltcGxlc2FtbHBocC0yLjIvbW9kdWxlLnBocC9vaWRjL2ZlZGVyYXRpb24vZmV0Y2gifSwib3BlbmlkX3Byb3ZpZGVyIjp7Imlzc3VlciI6Imh0dHBzOi8vODItZGFwLmxvY2FsaG9zdC5tYXJrb2l2YW5jaWMuZnJvbS5ociIsImF1dGhvcml6YXRpb25fZW5kcG9pbnQiOiJodHRwczovLzgyLWRhcC5sb2NhbGhvc3QubWFya29pdmFuY2ljLmZyb20uaHIvc2ltcGxlc2FtbHBocC9zaW1wbGVzYW1scGhwLTIuMi9tb2R1bGUucGhwL29pZGMvYXV0aG9yaXphdGlvbiIsInRva2VuX2VuZHBvaW50IjoiaHR0cHM6Ly84Mi1kYXAubG9jYWxob3N0Lm1hcmtvaXZhbmNpYy5mcm9tLmhyL3NpbXBsZXNhbWxwaHAvc2ltcGxlc2FtbHBocC0yLjIvbW9kdWxlLnBocC9vaWRjL3Rva2VuIiwidXNlcmluZm9fZW5kcG9pbnQiOiJodHRwczovLzgyLWRhcC5sb2NhbGhvc3QubWFya29pdmFuY2ljLmZyb20uaHIvc2ltcGxlc2FtbHBocC9zaW1wbGVzYW1scGhwLTIuMi9tb2R1bGUucGhwL29pZGMvdXNlcmluZm8iLCJlbmRfc2Vzc2lvbl9lbmRwb2ludCI6Imh0dHBzOi8vODItZGFwLmxvY2FsaG9zdC5tYXJrb2l2YW5jaWMuZnJvbS5oci9zaW1wbGVzYW1scGhwL3NpbXBsZXNhbWxwaHAtMi4yL21vZHVsZS5waHAvb2lkYy9lbmQtc2Vzc2lvbiIsImp3a3NfdXJpIjoiaHR0cHM6Ly84Mi1kYXAubG9jYWxob3N0Lm1hcmtvaXZhbmNpYy5mcm9tLmhyL3NpbXBsZXNhbWxwaHAvc2ltcGxlc2FtbHBocC0yLjIvbW9kdWxlLnBocC9vaWRjL2p3a3MiLCJzY29wZXNfc3VwcG9ydGVkIjpbIm9wZW5pZCIsIm9mZmxpbmVfYWNjZXNzIiwicHJvZmlsZSIsImVtYWlsIiwiYWRkcmVzcyIsInBob25lIiwiaHJFZHVQZXJzb25VbmlxdWVJRCIsInVpZCIsImNuIiwic24iLCJnaXZlbk5hbWUiLCJtYWlsIiwidGVsZXBob25lTnVtYmVyIiwiaHJFZHVQZXJzb25FeHRlbnNpb25OdW1iZXIiLCJtb2JpbGUiLCJmYWNzaW1pbGVUZWxlcGhvbmVOdW1iZXIiLCJockVkdVBlcnNvblVuaXF1ZU51bWJlciIsImhyRWR1UGVyc29uT0lCIiwiaHJFZHVQZXJzb25EYXRlT2ZCaXJ0aCIsImhyRWR1UGVyc29uR2VuZGVyIiwianBlZ1Bob3RvIiwidXNlckNlcnRpZmljYXRlIiwibGFiZWxlZFVSSSIsImhyRWR1UGVyc29uUHJvZmVzc2lvbmFsU3RhdHVzIiwiaHJFZHVQZXJzb25BY2FkZW1pY1N0YXR1cyIsImhyRWR1UGVyc29uU2NpZW5jZUFyZWEiLCJockVkdVBlcnNvbkFmZmlsaWF0aW9uIiwiaHJFZHVQZXJzb25QcmltYXJ5QWZmaWxpYXRpb24iLCJockVkdVBlcnNvblN0dWRlbnRDYXRlZ29yeSIsImhyRWR1UGVyc29uRXhwaXJlRGF0ZSIsImhyRWR1UGVyc29uVGl0bGUiLCJockVkdVBlcnNvblJvbGUiLCJockVkdVBlcnNvblN0YWZmQ2F0ZWdvcnkiLCJockVkdVBlcnNvbkdyb3VwTWVtYmVyIiwibyIsImhyRWR1UGVyc29uSG9tZU9yZyIsIm91Iiwicm9vbU51bWJlciIsInBvc3RhbEFkZHJlc3MiLCJsIiwicG9zdGFsQ29kZSIsInN0cmVldCIsImhvbWVQb3N0YWxBZGRyZXNzIiwiaG9tZVRlbGVwaG9uZU51bWJlciIsImhyRWR1UGVyc29uQ29tbVVSSSIsImhyRWR1UGVyc29uUHJpdmFjeSIsImhyRWR1UGVyc29uUGVyc2lzdGVudElEIiwiZGlzcGxheU5hbWUiLCJzY2hhY1VzZXJQcmVzZW5jZUlEIiwiaHJFZHVQZXJzb25DYXJkTnVtIiwiZm9ybWF0ZWRUZXN0Il0sInJlc3BvbnNlX3R5cGVzX3N1cHBvcnRlZCI6WyJjb2RlIiwidG9rZW4iLCJpZF90b2tlbiIsImlkX3Rva2VuIHRva2VuIl0sInN1YmplY3RfdHlwZXNfc3VwcG9ydGVkIjpbInB1YmxpYyJdLCJpZF90b2tlbl9zaWduaW5nX2FsZ192YWx1ZXNfc3VwcG9ydGVkIjpbIlJTMjU2Il0sImNvZGVfY2hhbGxlbmdlX21ldGhvZHNfc3VwcG9ydGVkIjpbInBsYWluIiwiUzI1NiJdLCJ0b2tlbl9lbmRwb2ludF9hdXRoX21ldGhvZHNfc3VwcG9ydGVkIjpbImNsaWVudF9zZWNyZXRfcG9zdCIsImNsaWVudF9zZWNyZXRfYmFzaWMiLCJwcml2YXRlX2tleV9qd3QiXSwicmVxdWVzdF9wYXJhbWV0ZXJfc3VwcG9ydGVkIjp0cnVlLCJyZXF1ZXN0X29iamVjdF9zaWduaW5nX2FsZ192YWx1ZXNfc3VwcG9ydGVkIjpbIm5vbmUiLCJSUzI1NiJdLCJyZXF1ZXN0X3VyaV9wYXJhbWV0ZXJfc3VwcG9ydGVkIjpmYWxzZSwiZ3JhbnRfdHlwZXNfc3VwcG9ydGVkIjpbImF1dGhvcml6YXRpb25fY29kZSIsInJlZnJlc2hfdG9rZW4iXSwiY2xhaW1zX3BhcmFtZXRlcl9zdXBwb3J0ZWQiOnRydWUsImFjcl92YWx1ZXNfc3VwcG9ydGVkIjpbIjEiLCIwIl0sImJhY2tjaGFubmVsX2xvZ291dF9zdXBwb3J0ZWQiOnRydWUsImJhY2tjaGFubmVsX2xvZ291dF9zZXNzaW9uX3N1cHBvcnRlZCI6dHJ1ZSwiY2xpZW50X3JlZ2lzdHJhdGlvbl90eXBlc19zdXBwb3J0ZWQiOlsiYXV0b21hdGljIl0sInJlcXVlc3RfYXV0aGVudGljYXRpb25fbWV0aG9kc19zdXBwb3J0ZWQiOnsiYXV0aG9yaXphdGlvbl9lbmRwb2ludCI6WyJyZXF1ZXN0X29iamVjdCJdfSwicmVxdWVzdF9hdXRoZW50aWNhdGlvbl9zaWduaW5nX2FsZ192YWx1ZXNfc3VwcG9ydGVkIjpbIlJTMjU2Il19fSwiYXV0aG9yaXR5X2hpbnRzIjpbImh0dHBzOi8vZWR1Z2Fpbi5vcmcvIiwiaHR0cHM6Ly84Mi1kYXAubG9jYWxob3N0Lm1hcmtvaXZhbmNpYy5mcm9tLmhyL3NpbXBsZXNhbWxwaHAvc2ltcGxlc2FtbHBocC0yLjIvbW9kdWxlLnBocC9vaWRjLyJdfQ.QOC5hPVzoGe5jJ4o_TkYMyRPWyd7HxqD4flduSAKhF1MIVRkBxgDfV3G1obJd875MsCq_Syb9wZfTP544-nY0z6ulSZm1L08ymzSlWwltcDW-l8rSjuCXErX5UDFNzBwc8ht7F7FfWpNCHrn6-A6t-m5E588IueGZfCqQrKUHRzsObQ8ZCNCkU_hjXgkM-FyERu2_Dnle9wpQ1GszOpNAJAuyMUfissgkokBRrXWwvDbj_7yA8prhgoLhOqtqf_ljMXlx_RggWknd-3zqvBi3U3msHwNnBCQ25E_TH7V_2onASfVOjr2TxyZ5diSkBqoSU9Vqr3bmH3cmcFodu_mvg'; -// $es = $this->federation->entityStatementFetcher()->fromNetwork('https://82-dap.localhost.markoivancic.from.hr/simplesamlphp/simplesamlphp-2.2/module.php/oidc/.well-known/openid-federation'); -// $es = $this->federation->entityStatementFetcher()->fromNetwork('https://maiv1.incubator.geant.org/.well-known/openid-federation'); -// dd($es->getPayload(), $es->verifyWithKeySet()); - -// $this->federationCache->cache->clear(); - //$this->protocolCache->set('value', 10, 'test'); - //dd($this->protocolCache, $this->protocolCache->get(null, 'test')); - - -// $requestObjectFactory = (new Core())->requestObjectFactory(); - - // {"alg":"none"}, {"iss":"joe", - // "exp":1300819380, - // "http://example.com/is_root":true} -// $unprotectedJws = 'eyJhbGciOiJub25lIn0.' . -// 'eyJpc3MiOiJqb2UiLA0KICJleHAiOjEzMDA4MTkzODAsDQogImh0dHA6Ly9leGFtcGxlLmNvbS9pc19yb290Ijp0cnVlfQ.'; - -// $requestObject = $requestObjectFactory->fromToken($unprotectedJws); - -// dd($requestObject, $requestObject->getPayload(), $requestObject->getHeader()); - $this->federationCache?->cache->clear(); - - $trustChain = $this->federation - ->trustChainResolver() - ->for( - 'https://08-dap.localhost.markoivancic.from.hr/openid/entities/ALeaf/', -// 'https://trust-anchor.testbed.oidcfed.incubator.geant.org/oidc/rp/', -// 'https://relying-party-php.testbed.oidcfed.incubator.geant.org/', -// 'https://gorp.testbed.oidcfed.incubator.geant.org', -// 'https://maiv1.incubator.geant.org', - [ -// 'https://trust-anchor.testbed.oidcfed.incubator.geant.org/', - 'https://08-dap.localhost.markoivancic.from.hr/openid/entities/ABTrustAnchor/', -// 'https://08-dap.localhost.markoivancic.from.hr/openid/entities/CTrustAnchor/', - ], - ) - //->getAll(); - ->getShortestByTrustAnchorPriority( - 'https://08-dap.localhost.markoivancic.from.hr/openid/entities/ABTrustAnchor/', - ); - - $leaf = $trustChain->getResolvedLeaf(); - $trustAnchor = $trustChain->getResolvedTrustAnchor(); - - $this->federationParticipationValidator->validateForAllOfLimit( - ['https://08-dap.localhost.markoivancic.from.hr/openid/entities/ATrustMarkIssuer/trust-mark/member'], - $leaf, - $trustAnchor, - ); - - dd($leaf->getPayload()); - - $this->federation->trustMarkValidator()->fromCacheOrDoForTrustMarkId( - 'https://08-dap.localhost.markoivancic.from.hr/openid/entities/ATrustMarkIssuer/trust-mark/member', - $leaf, - $trustAnchor, - ); - -// $leafFederationJwks = $leaf->getJwks(); -// dd($leafFederationJwks); -// /** @psalm-suppress PossiblyNullArgument */ - $resolvedMetadata = $trustChain->getResolvedMetadata(EntityTypesEnum::OpenIdRelyingParty); - $clientEntity = $this->clientEntityFactory->fromRegistrationData( - $resolvedMetadata, - RegistrationTypeEnum::FederatedAutomatic, - ); -// dd($resolvedMetadata, $clientEntity); - $jwksUri = $resolvedMetadata['jwks_uri'] ?? null; - $signedJwksUri = $resolvedMetadata['signed_jwks_uri'] ?? null; -// dd($leaf, $leafFederationJwks, $resolvedMetadata, $jwksUri, $signedJwksUri); -// $cachedJwks = $jwksUri ? $this->jwks->jwksFetcher()->fromCache($jwksUri) : null; -// $jwks = $jwksUri ? $this->jwks->jwksFetcher()->fromJwksUri($jwksUri) : null; - -//$leafFederationJwks = [ -// 'keys' => -// [ -// 0 => -// [ -// 'alg' => 'RS256', -// 'use' => 'sig', -// 'kty' => 'RSA', -// 'n' => 'pJgG9F_lwc2cFEC1l6q0fjJYxKPbtVGqJpDggDpDR8MgfbH0jUZP_RvhJGpl_09Bp-PfibLiwxchHZlrCx-fHQyGMaBRivUfq_p12ECEXMaFUcasCP6cyNrDfa5Uchumau4WeC21nYI1NMawiMiWFcHpLCQ7Ul8NMaCM_dkeruhm_xG0ZCqfwu30jOyCsnZdE0izJwPTfBRLpLyivu8eHpwjoIzmwqo8H-ZsbqR0vdRu20-MNS78ppTxwK3QmJhU6VO2r730F6WH9xJd_XUDuVeM4_6Z6WVDXw3kQF-jlpfcssPP303nbqVmfFZSUgS8buToErpMqevMIKREShsjMQ', -// 'e' => 'AQAB', -// 'kid' => 'F4VFObNusj3PHmrHxpqh4GNiuFHlfh-2s6xMJ95fLYA', -// ], -// ], -//]; -// $signedJwksUri = 'https://08-dap.localhost.markoivancic.from.hr/openid/entities/ALeaf/signed-jwks'; - $signedJwks = $signedJwksUri ? $this->jwks->jwksFetcher() - ->fromSignedJwksUri($signedJwksUri, $leafFederationJwks) : null; - $cachedSignedJwks = $signedJwksUri ? $this->jwks->jwksFetcher()->fromCache($signedJwksUri) : null; - dd($signedJwksUri, $cachedSignedJwks, $signedJwks); -// dd( -// $signedJwksUri, -// $cachedSignedJwks, -// $signedJwks, -// ); - - return new JsonResponse( - $trustChain->getResolvedMetadata(EntityTypesEnum::OpenIdRelyingParty), - ); - } -} diff --git a/src/Factories/FormFactory.php b/src/Factories/FormFactory.php index 4a164d74..13e9c3b2 100644 --- a/src/Factories/FormFactory.php +++ b/src/Factories/FormFactory.php @@ -18,13 +18,19 @@ use Nette\Forms\Form; use SimpleSAML\Error\Exception; +use SimpleSAML\Module\oidc\Bridges\SspBridge; use SimpleSAML\Module\oidc\Forms\Controls\CsrfProtection; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; class FormFactory { - public function __construct(private readonly ModuleConfig $moduleConfig, protected CsrfProtection $csrfProtection) - { + public function __construct( + protected readonly ModuleConfig $moduleConfig, + protected readonly CsrfProtection $csrfProtection, + protected readonly SspBridge $sspBridge, + protected readonly Helpers $helpers, + ) { } /** @@ -39,6 +45,11 @@ public function build(string $classname): Form } /** @psalm-suppress UnsafeInstantiation */ - return new $classname($this->moduleConfig, $this->csrfProtection); + return new $classname( + $this->moduleConfig, + $this->csrfProtection, + $this->sspBridge, + $this->helpers, + ); } } diff --git a/src/Factories/Grant/AuthCodeGrantFactory.php b/src/Factories/Grant/AuthCodeGrantFactory.php index f519b687..a72a53c4 100644 --- a/src/Factories/Grant/AuthCodeGrantFactory.php +++ b/src/Factories/Grant/AuthCodeGrantFactory.php @@ -18,6 +18,7 @@ use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\AuthCodeEntityFactory; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\AccessTokenRepository; use SimpleSAML\Module\oidc\Repositories\AuthCodeRepository; @@ -39,6 +40,7 @@ public function __construct( private readonly AccessTokenEntityFactory $accessTokenEntityFactory, private readonly AuthCodeEntityFactory $authCodeEntityFactory, private readonly RefreshTokenIssuer $refreshTokenIssuer, + private readonly Helpers $helpers, ) { } @@ -57,6 +59,7 @@ public function build(): AuthCodeGrant $this->accessTokenEntityFactory, $this->authCodeEntityFactory, $this->refreshTokenIssuer, + $this->helpers, ); $authCodeGrant->setRefreshTokenTTL($this->moduleConfig->getRefreshTokenDuration()); diff --git a/src/Factories/RequestRulesManagerFactory.php b/src/Factories/RequestRulesManagerFactory.php index 8aa57b32..bf28d4da 100644 --- a/src/Factories/RequestRulesManagerFactory.php +++ b/src/Factories/RequestRulesManagerFactory.php @@ -4,6 +4,7 @@ namespace SimpleSAML\Module\oidc\Factories; +use SimpleSAML\Module\oidc\Bridges\SspBridge; use SimpleSAML\Module\oidc\Factories\Entities\ClientEntityFactory; use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; @@ -60,6 +61,7 @@ public function __construct( private readonly Helpers $helpers, private readonly JwksResolver $jwksResolver, private readonly FederationParticipationValidator $federationParticipationValidator, + private readonly SspBridge $sspBridge, private readonly ?FederationCache $federationCache = null, private readonly ?ProtocolCache $protocolCache = null, ) { @@ -81,43 +83,64 @@ public function build(?array $rules = null): RequestRulesManager private function getDefaultRules(): array { return [ - new StateRule($this->requestParamsResolver), + new StateRule($this->requestParamsResolver, $this->helpers), new ClientIdRule( $this->requestParamsResolver, + $this->helpers, $this->clientRepository, $this->moduleConfig, $this->clientEntityFactory, $this->federation, - $this->helpers, $this->jwksResolver, $this->federationParticipationValidator, $this->federationCache, ), - new RedirectUriRule($this->requestParamsResolver), - new RequestObjectRule($this->requestParamsResolver, $this->jwksResolver), - new PromptRule($this->requestParamsResolver, $this->authSimpleFactory, $this->authenticationService), - new MaxAgeRule($this->requestParamsResolver, $this->authSimpleFactory, $this->authenticationService), - new ScopeRule($this->requestParamsResolver, $this->scopeRepository, $this->helpers), - new RequiredOpenIdScopeRule($this->requestParamsResolver), - new CodeChallengeRule($this->requestParamsResolver), - new CodeChallengeMethodRule($this->requestParamsResolver, $this->codeChallengeVerifiersRepository), - new RequestedClaimsRule($this->requestParamsResolver, $this->claimTranslatorExtractor), - new AddClaimsToIdTokenRule($this->requestParamsResolver), - new RequiredNonceRule($this->requestParamsResolver), - new ResponseTypeRule($this->requestParamsResolver), - new IdTokenHintRule($this->requestParamsResolver, $this->moduleConfig, $this->cryptKeyFactory), - new PostLogoutRedirectUriRule($this->requestParamsResolver, $this->clientRepository), - new UiLocalesRule($this->requestParamsResolver), - new AcrValuesRule($this->requestParamsResolver), - new ScopeOfflineAccessRule($this->requestParamsResolver), + new RedirectUriRule($this->requestParamsResolver, $this->helpers), + new RequestObjectRule($this->requestParamsResolver, $this->helpers, $this->jwksResolver), + new PromptRule( + $this->requestParamsResolver, + $this->helpers, + $this->authSimpleFactory, + $this->authenticationService, + $this->sspBridge, + ), + new MaxAgeRule( + $this->requestParamsResolver, + $this->helpers, + $this->authSimpleFactory, + $this->authenticationService, + $this->sspBridge, + ), + new ScopeRule($this->requestParamsResolver, $this->helpers, $this->scopeRepository), + new RequiredOpenIdScopeRule($this->requestParamsResolver, $this->helpers), + new CodeChallengeRule($this->requestParamsResolver, $this->helpers), + new CodeChallengeMethodRule( + $this->requestParamsResolver, + $this->helpers, + $this->codeChallengeVerifiersRepository, + ), + new RequestedClaimsRule($this->requestParamsResolver, $this->helpers, $this->claimTranslatorExtractor), + new AddClaimsToIdTokenRule($this->requestParamsResolver, $this->helpers), + new RequiredNonceRule($this->requestParamsResolver, $this->helpers), + new ResponseTypeRule($this->requestParamsResolver, $this->helpers), + new IdTokenHintRule( + $this->requestParamsResolver, + $this->helpers, + $this->moduleConfig, + $this->cryptKeyFactory, + ), + new PostLogoutRedirectUriRule($this->requestParamsResolver, $this->helpers, $this->clientRepository), + new UiLocalesRule($this->requestParamsResolver, $this->helpers), + new AcrValuesRule($this->requestParamsResolver, $this->helpers), + new ScopeOfflineAccessRule($this->requestParamsResolver, $this->helpers), new ClientAuthenticationRule( $this->requestParamsResolver, + $this->helpers, $this->moduleConfig, $this->jwksResolver, - $this->helpers, $this->protocolCache, ), - new CodeVerifierRule($this->requestParamsResolver), + new CodeVerifierRule($this->requestParamsResolver, $this->helpers), ]; } } diff --git a/src/Forms/ClientForm.php b/src/Forms/ClientForm.php index f4afb751..05f54d3b 100644 --- a/src/Forms/ClientForm.php +++ b/src/Forms/ClientForm.php @@ -20,6 +20,7 @@ use SimpleSAML\Locale\Translate; use SimpleSAML\Module\oidc\Bridges\SspBridge; use SimpleSAML\Module\oidc\Forms\Controls\CsrfProtection; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\OpenID\Codebooks\ClientRegistrationTypesEnum; use Traversable; @@ -62,6 +63,7 @@ public function __construct( protected readonly ModuleConfig $moduleConfig, protected CsrfProtection $csrfProtection, protected SspBridge $sspBridge, + protected Helpers $helpers, ) { parent::__construct(); @@ -217,14 +219,14 @@ public function getValues(string|object|bool|null $returnType = null, ?array $co $values = parent::getValues(self::TYPE_ARRAY); // Sanitize redirect_uri and allowed_origin - $values['redirect_uri'] = $this->convertTextToArrayWithLinesAsValues((string)$values['redirect_uri']); + $values['redirect_uri'] = $this->helpers->str()->convertTextToArray((string)$values['redirect_uri']); if (! $values['is_confidential'] && isset($values['allowed_origin'])) { - $values['allowed_origin'] = $this->convertTextToArrayWithLinesAsValues((string)$values['allowed_origin']); + $values['allowed_origin'] = $this->helpers->str()->convertTextToArray((string)$values['allowed_origin']); } else { $values['allowed_origin'] = []; } $values['post_logout_redirect_uri'] = - $this->convertTextToArrayWithLinesAsValues((string)$values['post_logout_redirect_uri']); + $this->helpers->str()->convertTextToArray((string)$values['post_logout_redirect_uri']); $bclUri = trim((string)$values['backchannel_logout_uri']); $values['backchannel_logout_uri'] = empty($bclUri) ? null : $bclUri; @@ -423,18 +425,6 @@ protected function getScopes(): array ); } - /** - * TODO mivanci Move to Str helper. - * @return string[] - */ - protected function convertTextToArrayWithLinesAsValues(string $text): array - { - return array_filter( - preg_split("/[\t\r\n]+/", $text), - fn(string $line): bool => !empty(trim($line)), - ); - } - /** * @return string[] */ diff --git a/src/Helpers.php b/src/Helpers.php index 7a5e153a..5a55e766 100644 --- a/src/Helpers.php +++ b/src/Helpers.php @@ -9,6 +9,7 @@ use SimpleSAML\Module\oidc\Helpers\DateTime; use SimpleSAML\Module\oidc\Helpers\Http; use SimpleSAML\Module\oidc\Helpers\Random; +use SimpleSAML\Module\oidc\Helpers\Scope; use SimpleSAML\Module\oidc\Helpers\Str; class Helpers @@ -19,6 +20,7 @@ class Helpers protected static ?Str $str = null; protected static ?Arr $arr = null; protected static ?Random $random = null; + protected static ?Scope $scope = null; public function http(): Http { @@ -51,4 +53,9 @@ public function random(): Random { return static::$random ??= new Random(); } + + public function scope(): Scope + { + return static::$scope ??= new Scope(); + } } diff --git a/src/Helpers/Arr.php b/src/Helpers/Arr.php index c7df69ac..96cadff8 100644 --- a/src/Helpers/Arr.php +++ b/src/Helpers/Arr.php @@ -6,6 +6,23 @@ class Arr { + /** + * Find item in array using the given callable. + * + * @return mixed|null + */ + public function findByCallback(array $arr, callable $fn): mixed + { + /** @psalm-suppress MixedAssignment */ + foreach ($arr as $x) { + if (call_user_func($fn, $x) === true) { + return $x; + } + } + + return null; + } + /** * @param array $values * @return string[] diff --git a/src/Utils/ScopeHelper.php b/src/Helpers/Scope.php similarity index 82% rename from src/Utils/ScopeHelper.php rename to src/Helpers/Scope.php index 339b6ffb..bdd36a72 100644 --- a/src/Utils/ScopeHelper.php +++ b/src/Helpers/Scope.php @@ -2,18 +2,18 @@ declare(strict_types=1); -namespace SimpleSAML\Module\oidc\Utils; +namespace SimpleSAML\Module\oidc\Helpers; use League\OAuth2\Server\Entities\ScopeEntityInterface; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; -class ScopeHelper +class Scope { /** * @param \League\OAuth2\Server\Entities\ScopeEntityInterface[] $scopes * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException */ - public static function scopeExists(array $scopes, string $scopeIdentifier): bool + public function exists(array $scopes, string $scopeIdentifier): bool { foreach ($scopes as $scope) { if (! $scope instanceof ScopeEntityInterface) { diff --git a/src/ModuleConfig.php b/src/ModuleConfig.php index 730758a2..1e50289c 100644 --- a/src/ModuleConfig.php +++ b/src/ModuleConfig.php @@ -234,7 +234,7 @@ public function config(): Configuration return $this->moduleConfig; } - // TODO mivanci Move to dedicated \SimpleSAML\Module\oidc\Utils\Routes::getModuleUrl + // TODO mivanci v7 Move to dedicated \SimpleSAML\Module\oidc\Utils\Routes::getModuleUrl public function getModuleUrl(?string $path = null): string { $base = $this->sspBridge->module()->getModuleURL(self::MODULE_NAME); diff --git a/src/Server/Grants/AuthCodeGrant.php b/src/Server/Grants/AuthCodeGrant.php index e056081e..dfaac1cf 100644 --- a/src/Server/Grants/AuthCodeGrant.php +++ b/src/Server/Grants/AuthCodeGrant.php @@ -27,6 +27,7 @@ use SimpleSAML\Module\oidc\Entities\UserEntity; use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\AuthCodeEntityFactory; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Repositories\Interfaces\AccessTokenRepositoryInterface; use SimpleSAML\Module\oidc\Repositories\Interfaces\AuthCodeRepositoryInterface; use SimpleSAML\Module\oidc\Repositories\Interfaces\RefreshTokenRepositoryInterface; @@ -58,9 +59,7 @@ use SimpleSAML\Module\oidc\Server\ResponseTypes\Interfaces\NonceResponseTypeInterface; use SimpleSAML\Module\oidc\Server\ResponseTypes\Interfaces\SessionIdResponseTypeInterface; use SimpleSAML\Module\oidc\Server\TokenIssuers\RefreshTokenIssuer; -use SimpleSAML\Module\oidc\Utils\Arr; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; -use SimpleSAML\Module\oidc\Utils\ScopeHelper; use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum; use SimpleSAML\OpenID\Codebooks\ParamsEnum; @@ -165,6 +164,7 @@ public function __construct( AccessTokenEntityFactory $accessTokenEntityFactory, protected AuthCodeEntityFactory $authCodeEntityFactory, protected RefreshTokenIssuer $refreshTokenIssuer, + protected Helpers $helpers, ) { parent::__construct($authCodeRepository, $refreshTokenRepository, $authCodeTTL); @@ -211,7 +211,7 @@ public function isOidcCandidate( OAuth2AuthorizationRequest $authorizationRequest, ): bool { // Check if the scopes contain 'oidc' scope - return (bool) Arr::find( + return (bool) $this->helpers->arr()->findByCallback( $authorizationRequest->getScopes(), fn(ScopeEntityInterface $scope) => $scope->getIdentifier() === 'openid', ); @@ -554,7 +554,7 @@ public function respondToAccessTokenRequest( } // Release refresh token if it is requested by using offline_access scope. - if (ScopeHelper::scopeExists($scopes, 'offline_access')) { + if ($this->helpers->scope()->exists($scopes, 'offline_access')) { // Issue and persist new refresh token if given $refreshToken = $this->issueRefreshToken($accessToken, $authCodePayload->auth_code_id); diff --git a/src/Server/RequestRules/Rules/AbstractRule.php b/src/Server/RequestRules/Rules/AbstractRule.php index e9ba45ac..3882cdf9 100644 --- a/src/Server/RequestRules/Rules/AbstractRule.php +++ b/src/Server/RequestRules/Rules/AbstractRule.php @@ -4,13 +4,16 @@ namespace SimpleSAML\Module\oidc\Server\RequestRules\Rules; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\RequestRuleInterface; use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; abstract class AbstractRule implements RequestRuleInterface { - public function __construct(protected RequestParamsResolver $requestParamsResolver) - { + public function __construct( + protected RequestParamsResolver $requestParamsResolver, + protected Helpers $helpers, + ) { } /** diff --git a/src/Server/RequestRules/Rules/ClientAuthenticationRule.php b/src/Server/RequestRules/Rules/ClientAuthenticationRule.php index cb92a1ce..62b522ca 100644 --- a/src/Server/RequestRules/Rules/ClientAuthenticationRule.php +++ b/src/Server/RequestRules/Rules/ClientAuthenticationRule.php @@ -26,12 +26,12 @@ class ClientAuthenticationRule extends AbstractRule public function __construct( RequestParamsResolver $requestParamsResolver, + Helpers $helpers, protected ModuleConfig $moduleConfig, protected JwksResolver $jwksResolver, - protected Helpers $helpers, protected ?ProtocolCache $protocolCache, ) { - parent::__construct($requestParamsResolver); + parent::__construct($requestParamsResolver, $helpers); } /** diff --git a/src/Server/RequestRules/Rules/ClientIdRule.php b/src/Server/RequestRules/Rules/ClientIdRule.php index 9905cf82..b377377f 100644 --- a/src/Server/RequestRules/Rules/ClientIdRule.php +++ b/src/Server/RequestRules/Rules/ClientIdRule.php @@ -34,16 +34,16 @@ class ClientIdRule extends AbstractRule public function __construct( RequestParamsResolver $requestParamsResolver, + Helpers $helpers, protected ClientRepository $clientRepository, protected ModuleConfig $moduleConfig, protected ClientEntityFactory $clientEntityFactory, protected Federation $federation, - protected Helpers $helpers, protected JwksResolver $jwksResolver, protected FederationParticipationValidator $federationParticipationValidator, protected ?FederationCache $federationCache = null, ) { - parent::__construct($requestParamsResolver); + parent::__construct($requestParamsResolver, $helpers); } /** @@ -132,8 +132,8 @@ public function checkRule( throw OidcServerException::invalidRequest(ParamsEnum::Request->value, 'Client ID is not valid URI.'); // We are ready to resolve trust chain. - // TODO mivanci Request Object can contain trust_chain claim, so also implement resolving using that claim. Note - // that this is only possible if we have JWKS configured for common TA, so we can check TA Configuration + // TODO mivanci v7 Request Object can contain trust_chain claim, so also implement resolving using that claim. + // Note that this is only possible if we have JWKS configured for common TA, so we can check TA Configuration // signature. try { $trustChain = $this->federation->trustChainResolver()->for( diff --git a/src/Server/RequestRules/Rules/CodeChallengeMethodRule.php b/src/Server/RequestRules/Rules/CodeChallengeMethodRule.php index 5f69a134..ed087d3e 100644 --- a/src/Server/RequestRules/Rules/CodeChallengeMethodRule.php +++ b/src/Server/RequestRules/Rules/CodeChallengeMethodRule.php @@ -5,6 +5,7 @@ namespace SimpleSAML\Module\oidc\Server\RequestRules\Rules; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Repositories\CodeChallengeVerifiersRepository; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; @@ -19,9 +20,10 @@ class CodeChallengeMethodRule extends AbstractRule { public function __construct( RequestParamsResolver $requestParamsResolver, + Helpers $helpers, protected CodeChallengeVerifiersRepository $codeChallengeVerifiersRepository, ) { - parent::__construct($requestParamsResolver); + parent::__construct($requestParamsResolver, $helpers); } /** diff --git a/src/Server/RequestRules/Rules/IdTokenHintRule.php b/src/Server/RequestRules/Rules/IdTokenHintRule.php index 97a77575..c1160b01 100644 --- a/src/Server/RequestRules/Rules/IdTokenHintRule.php +++ b/src/Server/RequestRules/Rules/IdTokenHintRule.php @@ -10,6 +10,7 @@ use Lcobucci\JWT\Validation\Constraint\SignedWith; use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Module\oidc\Factories\CryptKeyFactory; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; @@ -25,10 +26,11 @@ class IdTokenHintRule extends AbstractRule { public function __construct( RequestParamsResolver $requestParamsResolver, + Helpers $helpers, protected ModuleConfig $moduleConfig, protected CryptKeyFactory $cryptKeyFactory, ) { - parent::__construct($requestParamsResolver); + parent::__construct($requestParamsResolver, $helpers); } /** @@ -56,7 +58,7 @@ public function checkRule( return new Result($this->getKey(), $idTokenHintParam); } - // TODO mivanci Fix: unmockable services... inject instead. + // TODO v7 mivanci Fix: unmockable services... inject instead. $privateKey = $this->cryptKeyFactory->buildPrivateKey(); $publicKey = $this->cryptKeyFactory->buildPublicKey(); /** @psalm-suppress ArgumentTypeCoercion */ diff --git a/src/Server/RequestRules/Rules/MaxAgeRule.php b/src/Server/RequestRules/Rules/MaxAgeRule.php index 695c9f50..38c4a809 100644 --- a/src/Server/RequestRules/Rules/MaxAgeRule.php +++ b/src/Server/RequestRules/Rules/MaxAgeRule.php @@ -5,7 +5,9 @@ namespace SimpleSAML\Module\oidc\Server\RequestRules\Rules; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Bridges\SspBridge; use SimpleSAML\Module\oidc\Factories\AuthSimpleFactory; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultInterface; @@ -15,16 +17,17 @@ use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum; use SimpleSAML\OpenID\Codebooks\ParamsEnum; -use SimpleSAML\Utils\HTTP; class MaxAgeRule extends AbstractRule { public function __construct( RequestParamsResolver $requestParamsResolver, + Helpers $helpers, private readonly AuthSimpleFactory $authSimpleFactory, private readonly AuthenticationService $authenticationService, + private readonly SspBridge $sspBridge, ) { - parent::__construct($requestParamsResolver); + parent::__construct($requestParamsResolver, $helpers); } /** @@ -86,11 +89,12 @@ public function checkRule( if ($isExpired) { unset($requestParams['prompt']); $loginParams = []; - // TODO mivanci Move to SspBridge - $loginParams['ReturnTo'] = (new HTTP()) - ->addURLParameters((new HTTP())->getSelfURLNoQuery(), $requestParams); + $loginParams['ReturnTo'] = $this->sspBridge->utils()->http()->addURLParameters( + $this->sspBridge->utils()->http()->getSelfURLNoQuery(), + $requestParams, + ); - $this->authenticationService->authenticate($request, $loginParams); + $this->authenticationService->authenticate($client, $loginParams); } return new Result($this->getKey(), $lastAuth); diff --git a/src/Server/RequestRules/Rules/PostLogoutRedirectUriRule.php b/src/Server/RequestRules/Rules/PostLogoutRedirectUriRule.php index db0c13fa..d27dace8 100644 --- a/src/Server/RequestRules/Rules/PostLogoutRedirectUriRule.php +++ b/src/Server/RequestRules/Rules/PostLogoutRedirectUriRule.php @@ -5,6 +5,7 @@ namespace SimpleSAML\Module\oidc\Server\RequestRules\Rules; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Repositories\ClientRepository; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; @@ -19,9 +20,10 @@ class PostLogoutRedirectUriRule extends AbstractRule { public function __construct( RequestParamsResolver $requestParamsResolver, + Helpers $helpers, protected ClientRepository $clientRepository, ) { - parent::__construct($requestParamsResolver); + parent::__construct($requestParamsResolver, $helpers); } /** diff --git a/src/Server/RequestRules/Rules/PromptRule.php b/src/Server/RequestRules/Rules/PromptRule.php index de8137b6..60fd38cc 100644 --- a/src/Server/RequestRules/Rules/PromptRule.php +++ b/src/Server/RequestRules/Rules/PromptRule.php @@ -6,7 +6,9 @@ use League\OAuth2\Server\Exception\OAuthServerException; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Bridges\SspBridge; use SimpleSAML\Module\oidc\Factories\AuthSimpleFactory; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultInterface; @@ -15,16 +17,17 @@ use SimpleSAML\Module\oidc\Utils\RequestParamsResolver; use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum; use SimpleSAML\OpenID\Codebooks\ParamsEnum; -use SimpleSAML\Utils\HTTP; class PromptRule extends AbstractRule { public function __construct( RequestParamsResolver $requestParamsResolver, + Helpers $helpers, private readonly AuthSimpleFactory $authSimpleFactory, private readonly AuthenticationService $authenticationService, + private readonly SspBridge $sspBridge, ) { - parent::__construct($requestParamsResolver); + parent::__construct($requestParamsResolver, $helpers); } /** @@ -80,11 +83,12 @@ public function checkRule( if (in_array('login', $prompt, true) && $authSimple->isAuthenticated()) { unset($requestParams[ParamsEnum::Prompt->value]); $loginParams = []; - // TODO mivanci move to SSP Bridge - $loginParams['ReturnTo'] = (new HTTP()) - ->addURLParameters((new HTTP())->getSelfURLNoQuery(), $requestParams); + $loginParams['ReturnTo'] = $this->sspBridge->utils()->http()->addURLParameters( + $this->sspBridge->utils()->http()->getSelfURLNoQuery(), + $requestParams, + ); - $this->authenticationService->authenticate($request, $loginParams); + $this->authenticationService->authenticate($client, $loginParams); } return null; diff --git a/src/Server/RequestRules/Rules/RequestObjectRule.php b/src/Server/RequestRules/Rules/RequestObjectRule.php index 944bb19e..a1f74a24 100644 --- a/src/Server/RequestRules/Rules/RequestObjectRule.php +++ b/src/Server/RequestRules/Rules/RequestObjectRule.php @@ -5,6 +5,7 @@ namespace SimpleSAML\Module\oidc\Server\RequestRules\Rules; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultInterface; @@ -19,9 +20,10 @@ class RequestObjectRule extends AbstractRule { public function __construct( RequestParamsResolver $requestParamsResolver, + Helpers $helpers, protected JwksResolver $jwksResolver, ) { - parent::__construct($requestParamsResolver); + parent::__construct($requestParamsResolver, $helpers); } /** diff --git a/src/Server/RequestRules/Rules/RequestedClaimsRule.php b/src/Server/RequestRules/Rules/RequestedClaimsRule.php index fee6c21c..d8b27970 100644 --- a/src/Server/RequestRules/Rules/RequestedClaimsRule.php +++ b/src/Server/RequestRules/Rules/RequestedClaimsRule.php @@ -5,6 +5,7 @@ namespace SimpleSAML\Module\oidc\Server\RequestRules\Rules; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Result; @@ -18,9 +19,10 @@ class RequestedClaimsRule extends AbstractRule { public function __construct( RequestParamsResolver $requestParamsResolver, + Helpers $helpers, private readonly ClaimTranslatorExtractor $claimExtractor, ) { - parent::__construct($requestParamsResolver); + parent::__construct($requestParamsResolver, $helpers); } diff --git a/src/Server/RequestRules/Rules/ResponseTypeRule.php b/src/Server/RequestRules/Rules/ResponseTypeRule.php index a7cc81e5..30acb5ad 100644 --- a/src/Server/RequestRules/Rules/ResponseTypeRule.php +++ b/src/Server/RequestRules/Rules/ResponseTypeRule.php @@ -38,7 +38,7 @@ public function checkRule( throw OidcServerException::invalidRequest('Missing response_type or client_id'); } - // TODO consider checking for supported response types, for example, from configuration... + // TODO v7 consider checking for supported response types, for example, from configuration... return new Result($this->getKey(), $requestParams[ParamsEnum::ResponseType->value]); } diff --git a/src/Server/RequestRules/Rules/ScopeOfflineAccessRule.php b/src/Server/RequestRules/Rules/ScopeOfflineAccessRule.php index 127ab59e..ae67f588 100644 --- a/src/Server/RequestRules/Rules/ScopeOfflineAccessRule.php +++ b/src/Server/RequestRules/Rules/ScopeOfflineAccessRule.php @@ -10,7 +10,6 @@ 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\ScopeHelper; use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum; class ScopeOfflineAccessRule extends AbstractRule @@ -37,7 +36,7 @@ public function checkRule( $validScopes = $currentResultBag->getOrFail(ScopeRule::class)->getValue(); // Check if offline_access scope is used. If not, we don't have to check anything else. - if (! ScopeHelper::scopeExists($validScopes, 'offline_access')) { + if (! $this->helpers->scope()->exists($validScopes, 'offline_access')) { return new Result($this->getKey(), false); } diff --git a/src/Server/RequestRules/Rules/ScopeRule.php b/src/Server/RequestRules/Rules/ScopeRule.php index 4ad66244..e1eb7884 100644 --- a/src/Server/RequestRules/Rules/ScopeRule.php +++ b/src/Server/RequestRules/Rules/ScopeRule.php @@ -21,10 +21,10 @@ class ScopeRule extends AbstractRule { public function __construct( RequestParamsResolver $requestParamsResolver, + Helpers $helpers, protected ScopeRepositoryInterface $scopeRepository, - protected Helpers $helpers, ) { - parent::__construct($requestParamsResolver); + parent::__construct($requestParamsResolver, $helpers); } /** diff --git a/src/Services/AuthContextService.php b/src/Services/AuthContextService.php index 10af36de..b0d6e6db 100644 --- a/src/Services/AuthContextService.php +++ b/src/Services/AuthContextService.php @@ -6,10 +6,9 @@ use RuntimeException; use SimpleSAML\Auth\Simple; +use SimpleSAML\Module\oidc\Bridges\SspBridge; use SimpleSAML\Module\oidc\Factories\AuthSimpleFactory; use SimpleSAML\Module\oidc\ModuleConfig; -use SimpleSAML\Utils\Attributes; -use SimpleSAML\Utils\Auth; /** * Provide contextual authentication information for administration interface. @@ -28,13 +27,13 @@ class AuthContextService public function __construct( private readonly ModuleConfig $moduleConfig, private readonly AuthSimpleFactory $authSimpleFactory, + private readonly SspBridge $sspBridge, ) { } public function isSspAdmin(): bool { - // TODO mivanci make bridge to SSP utility classes (search for SSP namespace through the codebase) - return (new Auth())->isAdmin(); + return $this->sspBridge->utils()->auth()->isAdmin(); } /** @@ -45,7 +44,10 @@ public function getAuthUserId(): string { $simple = $this->authenticate(); $userIdAttr = $this->moduleConfig->getUserIdentifierAttribute(); - return (string)(new Attributes())->getExpectedAttribute($simple->getAttributes(), $userIdAttr); + return (string)$this->sspBridge->utils()->attributes()->getExpectedAttribute( + $simple->getAttributes(), + $userIdAttr, + ); } /** diff --git a/src/Services/AuthenticationService.php b/src/Services/AuthenticationService.php index 7d8c7afa..4dbba7d9 100644 --- a/src/Services/AuthenticationService.php +++ b/src/Services/AuthenticationService.php @@ -89,14 +89,14 @@ public function processRequest( ServerRequestInterface $request, OAuth2AuthorizationRequest $authorizationRequest, ): array { - // TODO mivanci Fix: client has already been resolved up to this point, but we are again fetching it from DB. + // TODO mivanci v7 Fix: client has already been resolved up to this point, but we are again fetching it from DB. $oidcClient = $this->helpers->client()->getFromRequest($request, $this->clientRepository); $authSimple = $this->authSimpleFactory->build($oidcClient); $this->authSourceId = $authSimple->getAuthSource()->getAuthId(); if (! $authSimple->isAuthenticated()) { - $this->authenticate($request); + $this->authenticate($oidcClient); } elseif ($this->sessionService->getIsAuthnPerformedInPreviousRequest()) { $this->sessionService->setIsAuthnPerformedInPreviousRequest(false); @@ -268,10 +268,6 @@ public function getSessionId(): ?string } /** - * @param ServerRequestInterface $request - * @param array $loginParams - * - * @return void * @throws Error\BadRequest * @throws Error\NotFound * @throws \JsonException @@ -279,12 +275,10 @@ public function getSessionId(): ?string */ public function authenticate( - ServerRequestInterface $request, + ClientEntityInterface $clientEntity, array $loginParams = [], ): void { - // TODO mivanci Fix: client has already been resolved up to this point, but we are again fetching it from DB. - $oidcClient = $this->helpers->client()->getFromRequest($request, $this->clientRepository); - $authSimple = $this->authSimpleFactory->build($oidcClient); + $authSimple = $this->authSimpleFactory->build($clientEntity); $this->sessionService->setIsCookieBasedAuthn(false); $this->sessionService->setIsAuthnPerformedInPreviousRequest(true); diff --git a/src/Services/Container.php b/src/Services/Container.php index 78f57483..d08a82cb 100644 --- a/src/Services/Container.php +++ b/src/Services/Container.php @@ -131,14 +131,29 @@ public function __construct() $authSimpleFactory = new AuthSimpleFactory($moduleConfig); $this->services[AuthSimpleFactory::class] = $authSimpleFactory; - $authContextService = new AuthContextService($moduleConfig, $authSimpleFactory); + $sspBridge = new SspBridge(); + $this->services[SspBridge::class] = $sspBridge; + + $authContextService = new AuthContextService( + $moduleConfig, + $authSimpleFactory, + $sspBridge, + ); $this->services[AuthContextService::class] = $authContextService; $session = Session::getSessionFromRequest(); $this->services[Session::class] = $session; + $helpers = new Helpers(); + $this->services[Helpers::class] = $helpers; + $csrfProtection = new CsrfProtection('{oidc:client:csrf_error}', $session); - $formFactory = new FormFactory($moduleConfig, $csrfProtection); + $formFactory = new FormFactory( + $moduleConfig, + $csrfProtection, + $sspBridge, + $helpers, + ); $this->services[FormFactory::class] = $formFactory; $jsonWebKeySetService = new JsonWebKeySetService($moduleConfig); @@ -150,9 +165,6 @@ public function __construct() $sessionMessagesService = new SessionMessagesService($session); $this->services[SessionMessagesService::class] = $sessionMessagesService; - $sspBridge = new SspBridge(); - $this->services[SspBridge::class] = $sspBridge; - $oidcMenu = new Menu(); $this->services[Menu::class] = $oidcMenu; @@ -193,8 +205,6 @@ public function __construct() $stateService = new StateService(); $this->services[StateService::class] = $stateService; - $helpers = new Helpers(); - $core = new Core(); $this->services[Core::class] = $core; $classInstanceBuilder = new ClassInstanceBuilder(); @@ -350,43 +360,43 @@ public function __construct() $this->services[FederationParticipationValidator::class] = $federationParticipationValidator; $requestRules = [ - new StateRule($requestParamsResolver), + new StateRule($requestParamsResolver, $helpers), new ClientIdRule( $requestParamsResolver, + $helpers, $clientRepository, $moduleConfig, $clientEntityFactory, $federation, - $helpers, $jwksResolver, $federationParticipationValidator, $federationCache, ), - new RedirectUriRule($requestParamsResolver), - new RequestObjectRule($requestParamsResolver, $jwksResolver), - new PromptRule($requestParamsResolver, $authSimpleFactory, $authenticationService), - new MaxAgeRule($requestParamsResolver, $authSimpleFactory, $authenticationService), - new ScopeRule($requestParamsResolver, $scopeRepository, $helpers), - new RequiredOpenIdScopeRule($requestParamsResolver), - new CodeChallengeRule($requestParamsResolver), - new CodeChallengeMethodRule($requestParamsResolver, $codeChallengeVerifiersRepository), - new RequestedClaimsRule($requestParamsResolver, $claimTranslatorExtractor), - new AddClaimsToIdTokenRule($requestParamsResolver), - new RequiredNonceRule($requestParamsResolver), - new ResponseTypeRule($requestParamsResolver), - new IdTokenHintRule($requestParamsResolver, $moduleConfig, $cryptKeyFactory), - new PostLogoutRedirectUriRule($requestParamsResolver, $clientRepository), - new UiLocalesRule($requestParamsResolver), - new AcrValuesRule($requestParamsResolver), - new ScopeOfflineAccessRule($requestParamsResolver), + new RedirectUriRule($requestParamsResolver, $helpers), + new RequestObjectRule($requestParamsResolver, $helpers, $jwksResolver), + new PromptRule($requestParamsResolver, $helpers, $authSimpleFactory, $authenticationService, $sspBridge), + new MaxAgeRule($requestParamsResolver, $helpers, $authSimpleFactory, $authenticationService, $sspBridge), + new ScopeRule($requestParamsResolver, $helpers, $scopeRepository), + new RequiredOpenIdScopeRule($requestParamsResolver, $helpers), + new CodeChallengeRule($requestParamsResolver, $helpers), + new CodeChallengeMethodRule($requestParamsResolver, $helpers, $codeChallengeVerifiersRepository), + new RequestedClaimsRule($requestParamsResolver, $helpers, $claimTranslatorExtractor), + new AddClaimsToIdTokenRule($requestParamsResolver, $helpers), + new RequiredNonceRule($requestParamsResolver, $helpers), + new ResponseTypeRule($requestParamsResolver, $helpers), + new IdTokenHintRule($requestParamsResolver, $helpers, $moduleConfig, $cryptKeyFactory), + new PostLogoutRedirectUriRule($requestParamsResolver, $helpers, $clientRepository), + new UiLocalesRule($requestParamsResolver, $helpers), + new AcrValuesRule($requestParamsResolver, $helpers), + new ScopeOfflineAccessRule($requestParamsResolver, $helpers), new ClientAuthenticationRule( $requestParamsResolver, + $helpers, $moduleConfig, $jwksResolver, - $helpers, $protocolCache, ), - new CodeVerifierRule($requestParamsResolver), + new CodeVerifierRule($requestParamsResolver, $helpers), ]; $requestRuleManager = new RequestRulesManager($requestRules, $loggerService); $this->services[RequestRulesManager::class] = $requestRuleManager; @@ -431,6 +441,7 @@ public function __construct() $accessTokenEntityFactory, $authCodeEntityFactory, $refreshTokenIssuer, + $helpers, ); $this->services[AuthCodeGrant::class] = $authCodeGrantFactory->build(); diff --git a/src/Services/JsonWebTokenBuilderService.php b/src/Services/JsonWebTokenBuilderService.php index cf4135b5..c371a10c 100644 --- a/src/Services/JsonWebTokenBuilderService.php +++ b/src/Services/JsonWebTokenBuilderService.php @@ -11,10 +11,10 @@ use Lcobucci\JWT\Signer; use Lcobucci\JWT\Signer\Key\InMemory; use Lcobucci\JWT\UnencryptedToken; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Utils\FingerprintGenerator; -use SimpleSAML\Module\oidc\Utils\UniqueIdentifierGenerator; use SimpleSAML\OpenID\Codebooks\ClaimsEnum; class JsonWebTokenBuilderService @@ -37,6 +37,7 @@ class JsonWebTokenBuilderService */ public function __construct( protected ModuleConfig $moduleConfig = new ModuleConfig(), + protected Helpers $helpers = new Helpers(), ) { $this->protocolJwtConfig = Configuration::forAsymmetricSigner( $this->moduleConfig->getProtocolSigner(), @@ -97,7 +98,7 @@ public function getDefaultJwtBuilder(Configuration $configuration): Builder return $configuration->builder(ChainedFormatter::withUnixTimestampDates()) ->issuedBy($this->moduleConfig->getIssuer()) ->issuedAt(new DateTimeImmutable('now')) - ->identifiedBy(UniqueIdentifierGenerator::hitMe()); + ->identifiedBy($this->helpers->random()->getIdentifier()); } /** diff --git a/src/Utils/Arr.php b/src/Utils/Arr.php deleted file mode 100644 index 32032d4c..00000000 --- a/src/Utils/Arr.php +++ /dev/null @@ -1,31 +0,0 @@ - {% for scope, claims in moduleConfig.getScopes %} {{ scope }}{{ loop.last ? '' : ', ' }} - {# TODO mivanci Add claims or extract scopes to sepparate page. #} + {# TODO v7 mivanci Add claims or extract scopes to sepparate page. #} {% endfor %}

diff --git a/tests/unit/src/Bridges/SspBridge/UtilsTest.php b/tests/unit/src/Bridges/SspBridge/UtilsTest.php index 3fc4941d..e9cb8be9 100644 --- a/tests/unit/src/Bridges/SspBridge/UtilsTest.php +++ b/tests/unit/src/Bridges/SspBridge/UtilsTest.php @@ -7,6 +7,7 @@ use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use SimpleSAML\Module\oidc\Bridges\SspBridge\Utils; +use SimpleSAML\Utils\Attributes; use SimpleSAML\Utils\Auth; use SimpleSAML\Utils\Config; use SimpleSAML\Utils\HTTP; @@ -44,4 +45,9 @@ public function testCanBuildAuthInstance(): void { $this->assertInstanceOf(Auth::class, $this->sut()->auth()); } + + public function testCanBuileAttributesInstance(): void + { + $this->assertInstanceOf(Attributes::class, $this->sut()->attributes()); + } } diff --git a/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php b/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php index a2fdc291..e2170067 100644 --- a/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php +++ b/tests/unit/src/Controllers/Federation/EntityStatementControllerTest.php @@ -99,7 +99,7 @@ public function testCanGetConfigurationStatement(): void $this->moduleConfigMock->expects($this->once())->method('getFederationEnabled')->willReturn(true); $this->federationCacheMock->expects($this->once())->method('get')->willReturn(null); - // TODO mivanci + // TODO v7 mivanci $this->markTestIncomplete('Move to simplesamlphp/openid library for building entity statements.'); } } diff --git a/tests/unit/src/Forms/ClientFormTest.php b/tests/unit/src/Forms/ClientFormTest.php index fbe200dc..e8f1f944 100644 --- a/tests/unit/src/Forms/ClientFormTest.php +++ b/tests/unit/src/Forms/ClientFormTest.php @@ -6,19 +6,21 @@ use DateTimeImmutable; use Laminas\Diactoros\ServerRequest; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\TestDox; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use SimpleSAML\Module\oidc\Bridges\SspBridge; use SimpleSAML\Module\oidc\Codebooks\RegistrationTypeEnum; use SimpleSAML\Module\oidc\Forms\ClientForm; use SimpleSAML\Module\oidc\Forms\Controls\CsrfProtection; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; -/** - * @covers \SimpleSAML\Module\oidc\Forms\ClientForm - */ +#[CoversClass(ClientForm::class)] +#[UsesClass(Helpers::class)] class ClientFormTest extends TestCase { protected MockObject $csrfProtectionMock; @@ -29,6 +31,7 @@ class ClientFormTest extends TestCase protected MockObject $sspBridgeMock; protected MockObject $sspBridgeAuthMock; protected MockObject $sspBridgeAuthSourceMock; + protected Helpers $helpers; protected array $clientDataSample; @@ -42,6 +45,7 @@ public function setUp(): void $this->moduleConfigMock = $this->createMock(ModuleConfig::class); $this->serverRequestMock = $this->createMock(ServerRequest::class); $this->sspBridgeMock = $this->createMock(SspBridge::class); + $this->helpers = new Helpers(); $this->sspBridgeAuthMock = $this->createMock(SspBridge\Auth::class); $this->sspBridgeMock->method('auth')->willReturn($this->sspBridgeAuthMock); @@ -85,15 +89,18 @@ protected function sut( ?ModuleConfig $moduleConfig = null, ?CsrfProtection $csrfProtection = null, ?SspBridge $sspBridge = null, + ?Helpers $helpers = null, ): ClientForm { $moduleConfig ??= $this->moduleConfigMock; $csrfProtection ??= $this->csrfProtectionMock; $sspBridge ??= $this->sspBridgeMock; + $helpers ??= $this->helpers; return new ClientForm( $moduleConfig, $csrfProtection, $sspBridge, + $helpers, ); } diff --git a/tests/unit/src/Helpers/ArrTest.php b/tests/unit/src/Helpers/ArrTest.php index e3555dd9..cac1613a 100644 --- a/tests/unit/src/Helpers/ArrTest.php +++ b/tests/unit/src/Helpers/ArrTest.php @@ -16,6 +16,22 @@ protected function sut(): Arr return new Arr(); } + public function testCanFindByCallback(): void + { + $this->assertSame( + 'a', + $this->sut()->findByCallback( + ['a', 'b', 'c'], + fn($item): bool => $item === 'a', + ), + ); + + $this->assertNull($this->sut()->findByCallback( + ['a', 'b', 'c'], + fn($item): bool => $item === 'd', + )); + } + public function testEnsureStringValues(): void { $this->assertSame( diff --git a/tests/unit/src/Utils/ScopeHelperTest.php b/tests/unit/src/Helpers/ScopeTest.php similarity index 67% rename from tests/unit/src/Utils/ScopeHelperTest.php rename to tests/unit/src/Helpers/ScopeTest.php index 06736cf6..814d9792 100644 --- a/tests/unit/src/Utils/ScopeHelperTest.php +++ b/tests/unit/src/Helpers/ScopeTest.php @@ -2,18 +2,17 @@ declare(strict_types=1); -namespace SimpleSAML\Test\Module\oidc\unit\Utils; +namespace SimpleSAML\Test\Module\oidc\unit\Helpers; use League\OAuth2\Server\Entities\ScopeEntityInterface; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; +use SimpleSAML\Module\oidc\Helpers\Scope; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; -use SimpleSAML\Module\oidc\Utils\ScopeHelper; -/** - * @covers \SimpleSAML\Module\oidc\Utils\ScopeHelper - */ -class ScopeHelperTest extends TestCase +#[CoversClass(Scope::class)] +class ScopeTest extends TestCase { protected Stub $scopeEntityOpenIdStub; protected Stub $scopeEntityProfileStub; @@ -34,20 +33,25 @@ protected function setUp(): void ]; } + protected function sut(): Scope + { + return new Scope(); + } + /** * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException */ public function testCanCheckScopeExistence(): void { - $this->assertTrue(ScopeHelper::scopeExists($this->scopeEntitiesArray, 'openid')); - $this->assertTrue(ScopeHelper::scopeExists($this->scopeEntitiesArray, 'profile')); - $this->assertFalse(ScopeHelper::scopeExists($this->scopeEntitiesArray, 'invalid')); + $this->assertTrue($this->sut()->exists($this->scopeEntitiesArray, 'openid')); + $this->assertTrue($this->sut()->exists($this->scopeEntitiesArray, 'profile')); + $this->assertFalse($this->sut()->exists($this->scopeEntitiesArray, 'invalid')); } public function testThrowsForInvalidScopeEntity(): void { $this->expectException(OidcServerException::class); - ScopeHelper::scopeExists(['invalid'], 'test'); + $this->sut()->exists(['invalid'], 'test'); } } diff --git a/tests/unit/src/HelpersTest.php b/tests/unit/src/HelpersTest.php index 643ad853..9fb4271d 100644 --- a/tests/unit/src/HelpersTest.php +++ b/tests/unit/src/HelpersTest.php @@ -16,6 +16,7 @@ #[UsesClass(Helpers\Str::class)] #[UsesClass(Helpers\Arr::class)] #[UsesClass(Helpers\Random::class)] +#[UsesClass(Helpers\Scope::class)] class HelpersTest extends TestCase { protected function sut(): Helpers @@ -31,5 +32,6 @@ public function testCanBuildHelpers(): void $this->assertInstanceOf(Helpers\Str::class, $this->sut()->str()); $this->assertInstanceOf(Helpers\Arr::class, $this->sut()->arr()); $this->assertInstanceOf(Helpers\Random::class, $this->sut()->random()); + $this->assertInstanceOf(Helpers\Scope::class, $this->sut()->scope()); } } diff --git a/tests/unit/src/Server/Grants/AuthCodeGrantTest.php b/tests/unit/src/Server/Grants/AuthCodeGrantTest.php index a0f1b8ed..4479ddf3 100644 --- a/tests/unit/src/Server/Grants/AuthCodeGrantTest.php +++ b/tests/unit/src/Server/Grants/AuthCodeGrantTest.php @@ -9,6 +9,7 @@ use PHPUnit\Framework\TestCase; use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; use SimpleSAML\Module\oidc\Factories\Entities\AuthCodeEntityFactory; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\Interfaces\AccessTokenRepositoryInterface; use SimpleSAML\Module\oidc\Repositories\Interfaces\AuthCodeRepositoryInterface; @@ -33,6 +34,7 @@ class AuthCodeGrantTest extends TestCase protected Stub $accessTokenEntityFactoryStub; protected Stub $authCodeEntityFactoryStub; protected Stub $refreshTokenIssuerStub; + protected Stub $helpersStub; /** * @throws \Exception @@ -49,6 +51,7 @@ protected function setUp(): void $this->accessTokenEntityFactoryStub = $this->createStub(AccessTokenEntityFactory::class); $this->authCodeEntityFactoryStub = $this->createStub(AuthcodeEntityFactory::class); $this->refreshTokenIssuerStub = $this->createStub(RefreshTokenIssuer::class); + $this->helpersStub = $this->createStub(Helpers::class); } /** @@ -68,6 +71,7 @@ public function testCanCreateInstance(): void $this->accessTokenEntityFactoryStub, $this->authCodeEntityFactoryStub, $this->refreshTokenIssuerStub, + $this->helpersStub, ), ); } diff --git a/tests/unit/src/Server/RequestRules/Rules/AcrValuesRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/AcrValuesRuleTest.php index 3bc4eea0..302eaa30 100644 --- a/tests/unit/src/Server/RequestRules/Rules/AcrValuesRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/AcrValuesRuleTest.php @@ -7,6 +7,7 @@ use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Result; @@ -24,6 +25,7 @@ class AcrValuesRuleTest extends TestCase protected Stub $resultStub; protected Stub $loggerServiceStub; protected Stub $requestParamsResolverStub; + protected Helpers $helpers; /** * @throws \Exception @@ -35,11 +37,20 @@ protected function setUp(): void $this->resultStub = $this->createStub(ResultInterface::class); $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + $this->helpers = new Helpers(); } - protected function mock(): AcrValuesRule - { - return new AcrValuesRule($this->requestParamsResolverStub); + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ): AcrValuesRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + + return new AcrValuesRule( + $requestParamsResolver, + $helpers, + ); } /** @@ -47,7 +58,7 @@ protected function mock(): AcrValuesRule */ public function testNoAcrIsSetIfAcrValuesNotRequested(): void { - $result = $this->mock()->checkRule( + $result = $this->sut()->checkRule( $this->requestStub, $this->resultBagStub, $this->loggerServiceStub, @@ -64,7 +75,7 @@ public function testPopulatesAcrValuesFromClaimsParameter(): void $this->resultStub->method('getValue')->willReturn($claims); $this->resultBagStub->method('get')->willReturn($this->resultStub); - $result = $this->mock()->checkRule( + $result = $this->sut()->checkRule( $this->requestStub, $this->resultBagStub, $this->loggerServiceStub, @@ -81,7 +92,7 @@ public function testPopulatesAcrValuesFromAcrValuesRequestParameter(): void { $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods')->willReturn('1 0'); - $result = $this->mock()->checkRule( + $result = $this->sut()->checkRule( $this->requestStub, $this->resultBagStub, $this->loggerServiceStub, diff --git a/tests/unit/src/Server/RequestRules/Rules/AddClaimsToIdTokenRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/AddClaimsToIdTokenRuleTest.php index 20d8a55e..fb54da45 100644 --- a/tests/unit/src/Server/RequestRules/Rules/AddClaimsToIdTokenRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/AddClaimsToIdTokenRuleTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; +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\AddClaimsToIdTokenRule; @@ -22,6 +23,7 @@ class AddClaimsToIdTokenRuleTest extends TestCase { protected Stub $requestStub; protected Stub $requestParamsResolverStub; + protected Helpers $helpers; protected array $requestParams = [ 'client_id' => 'client123', @@ -57,11 +59,20 @@ protected function setUp(): void $this->resultBag = new ResultBag(); $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + $this->helpers = new Helpers(); } - protected function mock(): AddClaimsToIdTokenRule - { - return new AddClaimsToIdTokenRule($this->requestParamsResolverStub); + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ): AddClaimsToIdTokenRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + + return new AddClaimsToIdTokenRule( + $requestParamsResolver, + $helpers, + ); } /** @@ -72,7 +83,7 @@ public function testAddClaimsToIdTokenRuleTest($responseType) { $this->resultBag->add(new Result(ResponseTypeRule::class, $responseType)); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub) ?? + $result = $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub) ?? new Result(AddClaimsToIdTokenRule::class, null); $this->assertTrue($result->getValue()); } @@ -92,7 +103,7 @@ public function testDoNotAddClaimsToIdTokenRuleTest($responseType) { $this->resultBag->add(new Result(ResponseTypeRule::class, $responseType)); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub) ?? + $result = $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub) ?? new Result(AddClaimsToIdTokenRule::class, null); $this->assertFalse($result->getValue()); @@ -117,6 +128,6 @@ public static function invalidResponseTypeProvider(): array public function testAddClaimsToIdTokenRuleThrowsWithNoResponseTypeParamTest() { $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); } } diff --git a/tests/unit/src/Server/RequestRules/Rules/ClientIdRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/ClientIdRuleTest.php index fda3d8ae..9556d8d3 100644 --- a/tests/unit/src/Server/RequestRules/Rules/ClientIdRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/ClientIdRuleTest.php @@ -66,11 +66,11 @@ protected function sut(): ClientIdRule { return new ClientIdRule( $this->requestParamsResolverStub, + $this->helpersStub, $this->clientRepositoryStub, $this->moduleConfigStub, $this->clientEntityFactoryStub, $this->federationStub, - $this->helpersStub, $this->jwksResolverStub, $this->federationParticipationValidatorStub, $this->federationCacheStub, diff --git a/tests/unit/src/Server/RequestRules/Rules/CodeChallengeMethodRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/CodeChallengeMethodRuleTest.php index bd7906ea..4d0217d7 100644 --- a/tests/unit/src/Server/RequestRules/Rules/CodeChallengeMethodRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/CodeChallengeMethodRuleTest.php @@ -9,6 +9,7 @@ use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Repositories\CodeChallengeVerifiersRepository; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; @@ -34,6 +35,7 @@ class CodeChallengeMethodRuleTest extends TestCase protected Stub $loggerServiceStub; protected Stub $requestParamsResolverStub; protected MockObject $codeChallengeVerifiersRepositoryMock; + protected Helpers $helpers; /** * @throws \Exception */ @@ -46,13 +48,22 @@ protected function setUp(): void $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); $this->codeChallengeVerifiersRepositoryMock = $this->createMock(CodeChallengeVerifiersRepository::class); + $this->helpers = new Helpers(); } - protected function mock(): CodeChallengeMethodRule - { + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ?CodeChallengeVerifiersRepository $codeChallengeVerifiersRepository = null, + ): CodeChallengeMethodRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + $codeChallengeVerifiersRepository ??= $this->codeChallengeVerifiersRepositoryMock; + return new CodeChallengeMethodRule( - $this->requestParamsResolverStub, - $this->codeChallengeVerifiersRepositoryMock, + $requestParamsResolver, + $helpers, + $codeChallengeVerifiersRepository, ); } @@ -64,7 +75,7 @@ public function testCheckRuleRedirectUriDependency(): void { $resultBag = new ResultBag(); $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -76,7 +87,7 @@ public function testCheckRuleStateDependency(): void $resultBag = new ResultBag(); $resultBag->add($this->redirectUriResult); $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -89,7 +100,7 @@ public function testCheckRuleWithInvalidCodeChallengeMethodThrows(): void $this->codeChallengeVerifiersRepositoryMock->expects($this->once())->method('has') ->with('invalid')->willReturn(false); $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -102,7 +113,7 @@ public function testCheckRuleForValidCodeChallengeMethod(): void $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods')->willReturn('plain'); $this->codeChallengeVerifiersRepositoryMock->expects($this->once())->method('has') ->with('plain')->willReturn(true); - $result = $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $result = $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); $this->assertInstanceOf(ResultInterface::class, $result); $this->assertSame('plain', $result->getValue()); diff --git a/tests/unit/src/Server/RequestRules/Rules/CodeChallengeRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/CodeChallengeRuleTest.php index 1ace20d8..671badb7 100644 --- a/tests/unit/src/Server/RequestRules/Rules/CodeChallengeRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/CodeChallengeRuleTest.php @@ -9,6 +9,7 @@ use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultInterface; @@ -37,6 +38,7 @@ class CodeChallengeRuleTest extends TestCase protected Stub $requestParamsResolverStub; protected Stub $clientStub; protected Result $clientIdResult; + protected Helpers $helpers; /** * @throws \Exception @@ -51,11 +53,20 @@ protected function setUp(): void $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); $this->clientStub = $this->createStub(ClientEntityInterface::class); $this->clientIdResult = new Result(ClientIdRule::class, $this->clientStub); + $this->helpers = new Helpers(); } - protected function mock(): CodeChallengeRule - { - return new CodeChallengeRule($this->requestParamsResolverStub); + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ): CodeChallengeRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + + return new CodeChallengeRule( + $requestParamsResolver, + $helpers, + ); } /** @@ -66,7 +77,7 @@ public function testCheckRuleRedirectUriDependency(): void { $resultBag = new ResultBag(); $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -78,7 +89,7 @@ public function testCheckRuleStateDependency(): void $resultBag = new ResultBag(); $resultBag->add($this->redirectUriResult); $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -89,7 +100,7 @@ public function testCheckRuleNoCodeReturnsNullForConfidentialClients(): void $this->clientStub->method('isConfidential')->willReturn(true); $resultBag = $this->prepareValidResultBag(); $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods')->willReturn(null); - $result = $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $result = $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); $this->assertInstanceOf(ResultInterface::class, $result); $this->assertNull($result->getValue()); } @@ -102,7 +113,7 @@ public function testCheckRuleInvalidCodeChallengeThrows(): void $resultBag = $this->prepareValidResultBag(); $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods')->willReturn('too-short'); $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -114,7 +125,7 @@ public function testCheckRuleForValidCodeChallenge(): void $resultBag = $this->prepareValidResultBag(); $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods')->willReturn($this->codeChallenge); - $result = $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $result = $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); $this->assertInstanceOf(ResultInterface::class, $result); $this->assertSame($this->codeChallenge, $result->getValue()); diff --git a/tests/unit/src/Server/RequestRules/Rules/IdTokenHintRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/IdTokenHintRuleTest.php index d07f525e..bee541f9 100644 --- a/tests/unit/src/Server/RequestRules/Rules/IdTokenHintRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/IdTokenHintRuleTest.php @@ -13,6 +13,7 @@ use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Module\oidc\Factories\CryptKeyFactory; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Result; @@ -42,6 +43,7 @@ class IdTokenHintRuleTest extends TestCase protected Stub $loggerServiceStub; protected Stub $requestParamsResolverStub; + protected Helpers $helpers; public static function setUpBeforeClass(): void { @@ -78,20 +80,33 @@ protected function setUp(): void $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + + $this->helpers = new Helpers(); } - protected function mock(): IdTokenHintRule - { + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ?ModuleConfig $moduleConfig = null, + ?CryptKeyFactory $cryptKeyFactory = null, + ): IdTokenHintRule { + + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + $moduleConfig ??= $this->moduleConfigStub; + $cryptKeyFactory ??= $this->cryptKeyFactoryStub; + return new IdTokenHintRule( - $this->requestParamsResolverStub, - $this->moduleConfigStub, - $this->cryptKeyFactoryStub, + $requestParamsResolver, + $helpers, + $moduleConfig, + $cryptKeyFactory, ); } public function testConstruct(): void { - $this->assertInstanceOf(IdTokenHintRule::class, $this->mock()); + $this->assertInstanceOf(IdTokenHintRule::class, $this->sut()); } /** @@ -100,7 +115,7 @@ public function testConstruct(): void */ public function testCheckRuleIsNullWhenParamNotSet(): void { - $result = $this->mock()->checkRule( + $result = $this->sut()->checkRule( $this->requestStub, $this->resultBagStub, $this->loggerServiceStub, @@ -116,7 +131,7 @@ public function testCheckRuleThrowsForMalformedIdToken(): void { $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods')->willReturn('malformed'); $this->expectException(Throwable::class); - $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); } /** @@ -133,7 +148,7 @@ public function testCheckRuleThrowsForIdTokenWithInvalidSignature(): void $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods')->willReturn($invalidSignatureJwt); $this->expectException(Throwable::class); - $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); } /** @@ -150,7 +165,7 @@ public function testCheckRuleThrowsForIdTokenWithInvalidIssuer(): void )->toString(); $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods')->willReturn($invalidIssuerJwt); $this->expectException(Throwable::class); - $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); } /** @@ -166,7 +181,7 @@ public function testCheckRulePassesForValidIdToken(): void )->toString(); $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods')->willReturn($idToken); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? + $result = $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? new Result(IdTokenHintRule::class); $this->assertInstanceOf(UnencryptedToken::class, $result->getValue()); diff --git a/tests/unit/src/Server/RequestRules/Rules/PostLogoutRedirectUriRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/PostLogoutRedirectUriRuleTest.php index 2ffccbc3..b14a3a57 100644 --- a/tests/unit/src/Server/RequestRules/Rules/PostLogoutRedirectUriRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/PostLogoutRedirectUriRuleTest.php @@ -12,6 +12,7 @@ use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Repositories\ClientRepository; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; @@ -45,6 +46,7 @@ class PostLogoutRedirectUriRuleTest extends TestCase protected Stub $loggerServiceStub; protected Stub $requestParamsResolverStub; + protected Helpers $helpers; public static function setUpBeforeClass(): void { @@ -74,13 +76,23 @@ protected function setUp(): void $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + + $this->helpers = new Helpers(); } - protected function mock(): PostLogoutRedirectUriRule - { + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ?ClientRepository $clientRepository = null, + ): PostLogoutRedirectUriRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + $clientRepository ??= $this->clientRepositoryStub; + return new PostLogoutRedirectUriRule( - $this->requestParamsResolverStub, - $this->clientRepositoryStub, + $requestParamsResolver, + $helpers, + $clientRepository, ); } @@ -90,7 +102,7 @@ protected function mock(): PostLogoutRedirectUriRule */ public function testCheckRuleReturnsNullIfNoParamSet(): void { - $result = $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? + $result = $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? (new Result(PostLogoutRedirectUriRule::class)); $this->assertNull($result->getValue()); @@ -106,7 +118,7 @@ public function testCheckRuleThrowsWhenIdTokenHintNotAvailable(): void $this->expectException(Throwable::class); - $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? + $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? (new Result(PostLogoutRedirectUriRule::class)); } @@ -131,7 +143,7 @@ public function testCheckRuleThrowsWhenAudClaimNotValid(): void $this->expectException(Throwable::class); - $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? + $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? (new Result(PostLogoutRedirectUriRule::class)); } @@ -159,7 +171,7 @@ public function testCheckRuleThrowsWhenClientNotFound(): void $this->expectException(Throwable::class); - $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? + $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? (new Result(PostLogoutRedirectUriRule::class)); } @@ -192,7 +204,7 @@ public function testCheckRuleThrowsWhenPostLogoutRegisteredUriNotRegistered(): v $this->expectException(Throwable::class); - $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? + $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? (new Result(PostLogoutRedirectUriRule::class)); } @@ -224,7 +236,7 @@ public function testCheckRuleReturnsForRegisteredPostLogoutRedirectUri(): void new Result(IdTokenHintRule::class, $jwt), ); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? + $result = $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? (new Result(PostLogoutRedirectUriRule::class)); $this->assertEquals(self::$postLogoutRedirectUri, $result->getValue()); diff --git a/tests/unit/src/Server/RequestRules/Rules/RedirectUriRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/RedirectUriRuleTest.php index 5fdb1e71..3650edde 100644 --- a/tests/unit/src/Server/RequestRules/Rules/RedirectUriRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/RedirectUriRuleTest.php @@ -9,6 +9,7 @@ use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Result; @@ -30,6 +31,7 @@ class RedirectUriRuleTest extends TestCase protected string $redirectUri = 'https://some-redirect-uri.org'; protected Stub $loggerServiceStub; protected Stub $requestParamsResolverStub; + protected Helpers $helpers; /** @@ -42,11 +44,20 @@ protected function setUp(): void $this->requestStub = $this->createStub(ServerRequestInterface::class); $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + $this->helpers = new Helpers(); } - protected function mock(): RedirectUriRule - { - return new RedirectUriRule($this->requestParamsResolverStub); + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ): RedirectUriRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + + return new RedirectUriRule( + $requestParamsResolver, + $helpers, + ); } /** @@ -56,7 +67,7 @@ protected function mock(): RedirectUriRule public function testCheckRuleClientIdDependency(): void { $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); } /** @@ -67,7 +78,7 @@ public function testCheckRuleWithInvalidClientDependancy(): void { $this->resultBag->add(new Result(ClientIdRule::class, 'invalid')); $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); } /** @@ -78,7 +89,7 @@ public function testCheckRuleRedirectUriNotSetThrows(): void $resultBag = $this->prepareValidResultBag(); $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -90,7 +101,7 @@ public function testCheckRuleDifferentClientRedirectUriThrows(): void $resultBag = $this->prepareValidResultBag(); $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -104,7 +115,7 @@ public function testCheckRuleDifferentClientRedirectUriArrayThrows(): void $this->resultBag->add(new Result(ClientIdRule::class, $this->clientStub)); $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); } /** @@ -117,7 +128,7 @@ public function testCheckRuleWithValidRedirectUri(): void $resultBag = $this->prepareValidResultBag(); - $result = $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $result = $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); $this->assertInstanceOf(ResultInterface::class, $result); $this->assertSame($this->redirectUri, $result->getValue()); diff --git a/tests/unit/src/Server/RequestRules/Rules/RequestObjectRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/RequestObjectRuleTest.php index 1c3b97b4..69c9392c 100644 --- a/tests/unit/src/Server/RequestRules/Rules/RequestObjectRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/RequestObjectRuleTest.php @@ -10,6 +10,7 @@ use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Result; use SimpleSAML\Module\oidc\Server\RequestRules\ResultBag; @@ -31,6 +32,7 @@ class RequestObjectRuleTest extends TestCase protected Stub $requestStub; protected Stub $loggerServiceStub; protected MockObject $jwksResolverMock; + protected Helpers $helpers; protected function setUp(): void { @@ -46,24 +48,33 @@ protected function setUp(): void $this->requestStub = $this->createStub(ServerRequestInterface::class); $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->jwksResolverMock = $this->createMock(JwksResolver::class); + $this->helpers = new Helpers(); } - protected function mock(): RequestObjectRule - { + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ?JwksResolver $jwksResolver = null, + ): RequestObjectRule { + $requestParamsResolver ??= $this->requestParamsResolverMock; + $helpers ??= $this->helpers; + $jwksResolver ??= $this->jwksResolverMock; + return new RequestObjectRule( - $this->requestParamsResolverMock, - $this->jwksResolverMock, + $requestParamsResolver, + $helpers, + $jwksResolver, ); } public function testCanCreateInstance(): void { - $this->assertInstanceOf(RequestObjectRule::class, $this->mock()); + $this->assertInstanceOf(RequestObjectRule::class, $this->sut()); } public function testRequestParamCanBeAbsent(): void { - $result = $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); + $result = $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); $this->assertNull($result); } @@ -74,7 +85,7 @@ public function testUnprotectedRequestParamCanBeUsed(): void $this->requestParamsResolverMock->expects($this->once())->method('parseRequestObjectToken') ->with('token')->willReturn($this->requestObjectMock); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); + $result = $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); $this->assertInstanceOf(Result::class, $result); $this->assertIsArray($result->getValue()); $this->assertNotEmpty($result->getValue()); @@ -89,7 +100,7 @@ public function testMissingClientJwksThrows(): void $this->clientStub->expects($this->once())->method('getJwks')->willReturn(null); $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); } public function testThrowsForInvalidRequestObject(): void @@ -105,7 +116,7 @@ public function testThrowsForInvalidRequestObject(): void ->willReturn(['jwks']); $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); } public function testReturnsValidRequestObject(): void @@ -121,7 +132,7 @@ public function testReturnsValidRequestObject(): void ->with($this->clientStub) ->willReturn(['jwks']); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); + $result = $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub); $this->assertInstanceOf(Result::class, $result); $this->assertIsArray($result->getValue()); diff --git a/tests/unit/src/Server/RequestRules/Rules/RequestedClaimsRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/RequestedClaimsRuleTest.php index 74648c30..20d165c1 100644 --- a/tests/unit/src/Server/RequestRules/Rules/RequestedClaimsRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/RequestedClaimsRuleTest.php @@ -10,6 +10,7 @@ use SimpleSAML\Module\oidc\Entities\ClaimSetEntity; use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface; use SimpleSAML\Module\oidc\Factories\Entities\ClaimSetEntityFactory; +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\ClientIdRule; @@ -31,6 +32,7 @@ class RequestedClaimsRuleTest extends TestCase protected static string $userIdAttr = 'uid'; protected Stub $requestParamsResolverStub; protected Stub $claimSetEntityFactoryStub; + protected Helpers $helpers; /** @@ -53,13 +55,23 @@ protected function setUp(): void $claimSetEntityStub->method('getClaims')->willReturn($claims); return $claimSetEntityStub; }); + + $this->helpers = new Helpers(); } - protected function mock(): RequestedClaimsRule - { + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ?ClaimTranslatorExtractor $claimTranslatorExtractor = null, + ): RequestedClaimsRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + $claimTranslatorExtractor ??= new ClaimTranslatorExtractor(self::$userIdAttr, $this->claimSetEntityFactoryStub); + return new RequestedClaimsRule( - $this->requestParamsResolverStub, - new ClaimTranslatorExtractor(self::$userIdAttr, $this->claimSetEntityFactoryStub), + $requestParamsResolver, + $helpers, + $claimTranslatorExtractor, ); } @@ -68,7 +80,7 @@ protected function mock(): RequestedClaimsRule */ public function testNoRequestedClaims(): void { - $result = $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); + $result = $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); $this->assertNull($result); } @@ -101,7 +113,7 @@ public function testWithClaims(): void $this->requestParamsResolverStub->method('getBasedOnAllowedMethods')->willReturn(json_encode($requestedClaims)); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); + $result = $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); $this->assertNotNull($result); $this->assertEquals($expectedClaims, $result->getValue()); } @@ -120,7 +132,7 @@ public function testOnlyWithNonStandardClaimRequest(): void $requestedClaims = $expectedClaims; $this->requestParamsResolverStub->method('getBasedOnAllowedMethods')->willReturn(json_encode($requestedClaims)); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); + $result = $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); $this->assertNotNull($result); $this->assertEquals($expectedClaims, $result->getValue()); } diff --git a/tests/unit/src/Server/RequestRules/Rules/RequiredNonceRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/RequiredNonceRuleTest.php index 72cd83c0..8a6d377d 100644 --- a/tests/unit/src/Server/RequestRules/Rules/RequiredNonceRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/RequiredNonceRuleTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Result; use SimpleSAML\Module\oidc\Server\RequestRules\ResultBag; @@ -27,6 +28,7 @@ class RequiredNonceRuleTest extends TestCase protected Result $stateResult; protected Stub $requestStub; + protected Helpers $helpers; protected array $requestQueryParams = [ 'nonce' => 'nonce123', @@ -51,11 +53,20 @@ protected function setUp(): void $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + $this->helpers = new Helpers(); } - protected function mock(): RequiredNonceRule - { - return new RequiredNonceRule($this->requestParamsResolverStub); + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ): RequiredNonceRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + + return new RequiredNonceRule( + $requestParamsResolver, + $helpers, + ); } /** @@ -66,7 +77,7 @@ public function testCheckRuleRedirectUriDependency(): void { $resultBag = new ResultBag(); $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -78,7 +89,7 @@ public function testCheckRuleStateDependency(): void $resultBag = new ResultBag(); $resultBag->add($this->redirectUriResult); $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -90,7 +101,7 @@ public function testCheckRulePassesWhenNonceIsPresent() $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods') ->willReturn($this->requestQueryParams['nonce']); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub) ?? + $result = $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub) ?? new Result(RequiredNonceRule::class, null); $this->assertEquals($this->requestQueryParams['nonce'], $result->getValue()); @@ -103,6 +114,6 @@ public function testCheckRuleThrowsWhenNonceIsNotPresent() { $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); } } diff --git a/tests/unit/src/Server/RequestRules/Rules/RequiredOpenIdScopeRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/RequiredOpenIdScopeRuleTest.php index d9455abe..9f6dcedf 100644 --- a/tests/unit/src/Server/RequestRules/Rules/RequiredOpenIdScopeRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/RequiredOpenIdScopeRuleTest.php @@ -9,6 +9,7 @@ use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Module\oidc\Entities\ScopeEntity; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Result; use SimpleSAML\Module\oidc\Server\RequestRules\ResultBag; @@ -34,6 +35,7 @@ class RequiredOpenIdScopeRuleTest extends TestCase protected Stub $loggerServiceStub; protected Stub $requestParamsResolverStub; + protected Helpers $helpers; /** * @throws \Exception @@ -50,12 +52,19 @@ protected function setUp(): void $this->scopeResult = new Result(ScopeRule::class, $this->scopeEntities); $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + $this->helpers = new Helpers(); } - protected function mock(): RequiredOpenIdScopeRule - { + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ): RequiredOpenIdScopeRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + return new RequiredOpenIdScopeRule( - $this->requestParamsResolverStub, + $requestParamsResolver, + $helpers, ); } @@ -67,7 +76,7 @@ public function testCheckRuleRedirectUriDependency(): void { $resultBag = new ResultBag(); $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -79,7 +88,7 @@ public function testCheckRuleStateDependency(): void $resultBag = new ResultBag(); $resultBag->add($this->redirectUriResult); $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } /** @@ -93,7 +102,7 @@ public function testCheckRulePassesWhenOpenIdScopeIsPresent() $resultBag->add($this->stateResult); $resultBag->add($this->scopeResult); - $result = $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub) ?? + $result = $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub) ?? new Result(RequiredOpenIdScopeRule::class, null); $this->assertTrue($result->getValue()); @@ -114,6 +123,6 @@ public function testCheckRuleThrowsWhenOpenIdScopeIsNotPresent() $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub); } } diff --git a/tests/unit/src/Server/RequestRules/Rules/ResponseTypeRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/ResponseTypeRuleTest.php index a1688b6d..ecd2d1e6 100644 --- a/tests/unit/src/Server/RequestRules/Rules/ResponseTypeRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/ResponseTypeRuleTest.php @@ -7,6 +7,7 @@ use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Result; use SimpleSAML\Module\oidc\Server\RequestRules\ResultBag; @@ -21,6 +22,7 @@ class ResponseTypeRuleTest extends TestCase { protected Stub $requestStub; protected Stub $requestParamsResolverStub; + protected Helpers $helpers; protected array $requestParams = [ 'client_id' => 'client123', @@ -58,11 +60,20 @@ protected function setUp(): void $this->resultBag = new ResultBag(); $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + $this->helpers = new Helpers(); } - protected function mock(): ResponseTypeRule - { - return new ResponseTypeRule($this->requestParamsResolverStub); + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ): ResponseTypeRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + + return new ResponseTypeRule( + $requestParamsResolver, + $helpers, + ); } /** @@ -73,7 +84,7 @@ public function testResponseTypeRuleTest($responseType) { $this->requestParams['response_type'] = $responseType; $this->requestParamsResolverStub->method('getAllBasedOnAllowedMethods')->willReturn($this->requestParams); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub) ?? + $result = $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub) ?? new Result(ResponseTypeRule::class, null); $this->assertSame($responseType, $result->getValue()); } @@ -92,6 +103,6 @@ public function testResponseTypeRuleThrowsWithNoResponseTypeParamTest() unset($params['response_type']); $this->requestParamsResolverStub->method('getAllBasedOnAllowedMethods')->willReturn($params); $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); + $this->sut()->checkRule($this->requestStub, $this->resultBag, $this->loggerServiceStub); } } diff --git a/tests/unit/src/Server/RequestRules/Rules/ScopeOfflineAccessRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/ScopeOfflineAccessRuleTest.php index 5b0a216d..e9bddd38 100644 --- a/tests/unit/src/Server/RequestRules/Rules/ScopeOfflineAccessRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/ScopeOfflineAccessRuleTest.php @@ -11,6 +11,7 @@ use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Configuration; use SimpleSAML\Module\oidc\Entities\Interfaces\ClientEntityInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; @@ -37,6 +38,7 @@ class ScopeOfflineAccessRuleTest extends TestCase protected Stub $moduleConfigStub; protected Stub $openIdConfigurationStub; protected Stub $requestParamsResolverStub; + protected Helpers $helpers; /** * @throws \Exception @@ -65,18 +67,28 @@ protected function setUp(): void $this->moduleConfigStub = $this->createStub(ModuleConfig::class); $this->openIdConfigurationStub = $this->createStub(Configuration::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + + $this->helpers = new Helpers(); } - protected function mock(): ScopeOfflineAccessRule - { - return new ScopeOfflineAccessRule($this->requestParamsResolverStub); + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ): ScopeOfflineAccessRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + + return new ScopeOfflineAccessRule( + $requestParamsResolver, + $helpers, + ); } public function testCanCreateInstance(): void { $this->assertInstanceOf( ScopeOfflineAccessRule::class, - $this->mock(), + $this->sut(), ); } @@ -103,7 +115,7 @@ public function testReturnsFalseWhenOfflineAccessScopeNotPresent(): void $this->moduleConfigStub->method('config') ->willReturn($this->openIdConfigurationStub); - $result = $this->mock()->checkRule($this->serverRequestStub, $this->resultBagMock, $this->loggerServiceMock); + $result = $this->sut()->checkRule($this->serverRequestStub, $this->resultBagMock, $this->loggerServiceMock); $this->assertNotNull($result); $this->assertFalse($result->getValue()); @@ -134,7 +146,7 @@ public function testThrowsWhenClientDoesntHaveOfflineAccessScopeRegistered(): vo $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->serverRequestStub, $this->resultBagMock, $this->loggerServiceMock); + $this->sut()->checkRule($this->serverRequestStub, $this->resultBagMock, $this->loggerServiceMock); } /** @@ -161,7 +173,7 @@ public function testReturnsTrueWhenClientDoesHaveOfflineAccessScopeRegistered(): $this->moduleConfigStub->method('config') ->willReturn($this->openIdConfigurationStub); - $result = $this->mock()->checkRule($this->serverRequestStub, $this->resultBagMock, $this->loggerServiceMock); + $result = $this->sut()->checkRule($this->serverRequestStub, $this->resultBagMock, $this->loggerServiceMock); $this->assertNotNull($result); $this->assertTrue($result->getValue()); diff --git a/tests/unit/src/Server/RequestRules/Rules/ScopeRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/ScopeRuleTest.php index 47761bcf..c40143b9 100644 --- a/tests/unit/src/Server/RequestRules/Rules/ScopeRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/ScopeRuleTest.php @@ -69,18 +69,25 @@ protected function setUp(): void $this->helpersStub->method('str')->willReturn($this->strHelperMock); } - protected function mock(): ScopeRule - { + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ?ScopeRepositoryInterface $scopeRepository = null, + ): ScopeRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpersStub; + $scopeRepository ??= $this->scopeRepositoryStub; + return new ScopeRule( - $this->requestParamsResolverStub, - $this->scopeRepositoryStub, - $this->helpersStub, + $requestParamsResolver, + $helpers, + $scopeRepository, ); } public function testConstruct(): void { - $this->assertInstanceOf(ScopeRule::class, $this->mock()); + $this->assertInstanceOf(ScopeRule::class, $this->sut()); } /** @@ -91,7 +98,7 @@ public function testCheckRuleRedirectUriDependency(): void { $resultBag = new ResultBag(); $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub, $this->data); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub, $this->data); } /** @@ -103,7 +110,7 @@ public function testCheckRuleStateDependency(): void $resultBag = new ResultBag(); $resultBag->add($this->redirectUriResult); $this->expectException(LogicException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub, $this->data); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub, $this->data); } /** @@ -127,7 +134,7 @@ public function testValidScopes(): void ), ); - $result = $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub, $this->data); + $result = $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub, $this->data); $this->assertInstanceOf(ResultInterface::class, $result); $this->assertIsArray($result->getValue()); $this->assertSame($this->scopeEntities['openid'], $result->getValue()[0]); @@ -154,7 +161,7 @@ public function testInvalidScopeThrows(): void ); $this->expectException(OidcServerException::class); - $this->mock()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub, $this->data); + $this->sut()->checkRule($this->requestStub, $resultBag, $this->loggerServiceStub, $this->data); } protected function prepareValidResultBag(): ResultBag diff --git a/tests/unit/src/Server/RequestRules/Rules/StateRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/StateRuleTest.php index 10fae4c3..aa38de1b 100644 --- a/tests/unit/src/Server/RequestRules/Rules/StateRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/StateRuleTest.php @@ -7,6 +7,7 @@ use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultInterface; use SimpleSAML\Module\oidc\Server\RequestRules\ResultBag; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\StateRule; @@ -21,6 +22,7 @@ class StateRuleTest extends TestCase { protected Stub $loggerServiceStub; protected Stub $requestParamsResolverStub; + protected Helpers $helpers; /** * @throws \Exception @@ -29,16 +31,25 @@ public function setUp(): void { $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + $this->helpers = new Helpers(); } - protected function mock(): StateRule - { - return new StateRule($this->requestParamsResolverStub); + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ): StateRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + + return new StateRule( + $requestParamsResolver, + $helpers, + ); } public function testGetKey(): void { - $this->assertSame(StateRule::class, $this->mock()->getKey()); + $this->assertSame(StateRule::class, $this->sut()->getKey()); } /** @@ -54,7 +65,7 @@ public function testCheckRuleHasValue(): void $resultBag = new ResultBag(); $data = []; - $result = $this->mock()->checkRule($request, $resultBag, $this->loggerServiceStub, $data); + $result = $this->sut()->checkRule($request, $resultBag, $this->loggerServiceStub, $data); $this->assertInstanceOf(ResultInterface::class, $result); $this->assertSame($value, $result->getValue()); @@ -70,7 +81,7 @@ public function testCheckRulePostMethod(): void $this->requestParamsResolverStub->method('getAsStringBasedOnAllowedMethods')->willReturn(null); $resultBag = new ResultBag(); - $result = $this->mock()->checkRule( + $result = $this->sut()->checkRule( $request, $resultBag, $this->loggerServiceStub, diff --git a/tests/unit/src/Server/RequestRules/Rules/UiLocalesRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/UiLocalesRuleTest.php index 681f5f04..a4eefd8c 100644 --- a/tests/unit/src/Server/RequestRules/Rules/UiLocalesRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/UiLocalesRuleTest.php @@ -7,6 +7,7 @@ use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; +use SimpleSAML\Module\oidc\Helpers; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Result; use SimpleSAML\Module\oidc\Server\RequestRules\Rules\UiLocalesRule; @@ -22,6 +23,7 @@ class UiLocalesRuleTest extends TestCase protected Stub $resultBagStub; protected Stub $loggerServiceStub; protected Stub $requestParamsResolverStub; + protected Helpers $helpers; /** * @throws \Exception @@ -34,11 +36,20 @@ protected function setUp(): void $this->resultBagStub = $this->createStub(ResultBagInterface::class); $this->loggerServiceStub = $this->createStub(LoggerService::class); $this->requestParamsResolverStub = $this->createStub(RequestParamsResolver::class); + $this->helpers = new Helpers(); } - protected function mock(): UiLocalesRule - { - return new UiLocalesRule($this->requestParamsResolverStub); + protected function sut( + ?RequestParamsResolver $requestParamsResolver = null, + ?Helpers $helpers = null, + ): UiLocalesRule { + $requestParamsResolver ??= $this->requestParamsResolverStub; + $helpers ??= $this->helpers; + + return new UiLocalesRule( + $requestParamsResolver, + $helpers, + ); } /** @@ -48,7 +59,7 @@ public function testCheckRuleReturnsResultWhenParamSet() { $this->requestParamsResolverStub->method('getBasedOnAllowedMethods')->willReturn('en'); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? + $result = $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? new Result(UiLocalesRule::class); $this->assertEquals('en', $result->getValue()); @@ -61,7 +72,7 @@ public function testCheckRuleReturnsNullWhenParamNotSet() { $this->requestStub->method('getQueryParams')->willReturn([]); - $result = $this->mock()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? + $result = $this->sut()->checkRule($this->requestStub, $this->resultBagStub, $this->loggerServiceStub) ?? new Result(UiLocalesRule::class); $this->assertNull($result->getValue()); diff --git a/tests/unit/src/Services/AuthContextServiceTest.php b/tests/unit/src/Services/AuthContextServiceTest.php index 494a9a03..0fcf54fb 100644 --- a/tests/unit/src/Services/AuthContextServiceTest.php +++ b/tests/unit/src/Services/AuthContextServiceTest.php @@ -10,9 +10,11 @@ use SimpleSAML\Auth\Simple; use SimpleSAML\Configuration; use SimpleSAML\Error\Exception; +use SimpleSAML\Module\oidc\Bridges\SspBridge; use SimpleSAML\Module\oidc\Factories\AuthSimpleFactory; use SimpleSAML\Module\oidc\ModuleConfig; use SimpleSAML\Module\oidc\Services\AuthContextService; +use SimpleSAML\Utils\Attributes; /** * @covers \SimpleSAML\Module\oidc\Services\AuthContextService @@ -28,6 +30,9 @@ class AuthContextServiceTest extends TestCase protected MockObject $moduleConfigMock; protected MockObject $authSimpleService; protected MockObject $authSimpleFactory; + protected MockObject $sspBridgeMock; + protected MockObject $sspBridgeUtilsMock; + protected MockObject $sspBridgeUtilsAttributesMock; /** * @throws \PHPUnit\Framework\MockObject\Exception @@ -52,13 +57,27 @@ protected function setUp(): void $this->authSimpleFactory = $this->createMock(AuthSimpleFactory::class); $this->authSimpleFactory->method('getDefaultAuthSource')->willReturn($this->authSimpleService); + + $this->sspBridgeMock = $this->createMock(SspBridge::class); + $this->sspBridgeUtilsMock = $this->createMock(SspBridge\Utils::class); + $this->sspBridgeMock->method('utils')->willReturn($this->sspBridgeUtilsMock); + $this->sspBridgeUtilsAttributesMock = $this->createMock(Attributes::class); + $this->sspBridgeUtilsMock->method('attributes')->willReturn($this->sspBridgeUtilsAttributesMock); } - protected function prepareMockedInstance(): AuthContextService - { + protected function sut( + ?ModuleConfig $moduleConfig = null, + ?AuthSimpleFactory $authSimpleFactory = null, + ?SspBridge $sspBridge = null, + ): AuthContextService { + $moduleConfig ??= $this->moduleConfigMock; + $authSimpleFactory ??= $this->authSimpleFactory; + $sspBridge ??= $this->sspBridgeMock; + return new AuthContextService( - $this->moduleConfigMock, - $this->authSimpleFactory, + $moduleConfig, + $authSimpleFactory, + $sspBridge, ); } @@ -66,7 +85,7 @@ public function testItIsInitializable(): void { $this->assertInstanceOf( AuthContextService::class, - $this->prepareMockedInstance(), + $this->sut(), ); } @@ -77,9 +96,15 @@ public function testItReturnsUsername(): void { $this->moduleConfigMock->method('getUserIdentifierAttribute')->willReturn('idAttribute'); $this->authSimpleService->method('getAttributes')->willReturn(self::AUTHORIZED_USER); + $this->sspBridgeUtilsAttributesMock->expects($this->once())->method('getExpectedAttribute') + ->with( + self::AUTHORIZED_USER, + 'idAttribute', + ) + ->willReturn(self::AUTHORIZED_USER['idAttribute'][0]); $this->assertSame( - $this->prepareMockedInstance()->getAuthUserId(), + $this->sut()->getAuthUserId(), 'myUsername', ); } @@ -94,8 +119,12 @@ public function testItThrowsWhenNoUsername(): void ->willReturn('attributeNotSet'); $this->authSimpleService->method('getAttributes')->willReturn(self::AUTHORIZED_USER); + $this->sspBridgeUtilsAttributesMock->expects($this->once())->method('getExpectedAttribute') + ->with(self::AUTHORIZED_USER) + ->willThrowException(new Exception('error')); + $this->expectException(Exception::class); - $this->prepareMockedInstance()->getAuthUserId(); + $this->sut()->getAuthUserId(); } /** @@ -108,7 +137,7 @@ public function testPermissionsOk(): void ->willReturn($this->permissions); $this->authSimpleService->method('getAttributes')->willReturn(self::AUTHORIZED_USER); - $this->prepareMockedInstance()->requirePermission('client'); + $this->sut()->requirePermission('client'); $this->expectNotToPerformAssertions(); } @@ -121,7 +150,7 @@ public function testItThrowsIfNotAuthorizedForPermission(): void ->with(ModuleConfig::OPTION_ADMIN_UI_PERMISSIONS, null) ->willReturn($this->permissions); $this->expectException(RuntimeException::class); - $this->prepareMockedInstance()->requirePermission('no-match'); + $this->sut()->requirePermission('no-match'); } /** @@ -141,7 +170,7 @@ public function testItThrowsForWrongEntitlements(): void ); $this->expectException(RuntimeException::class); - $this->prepareMockedInstance()->requirePermission('client'); + $this->sut()->requirePermission('client'); } /** @@ -160,7 +189,7 @@ public function testItThrowsForNotHavingEntitlementAttribute(): void ); $this->expectException(RuntimeException::class); - $this->prepareMockedInstance()->requirePermission('client'); + $this->sut()->requirePermission('client'); } /** @@ -173,6 +202,6 @@ public function testThrowsForNotHavingEnabledPermissions(): void ->willReturn(Configuration::loadFromArray([])); $this->expectException(RuntimeException::class); - $this->prepareMockedInstance()->requirePermission('client'); + $this->sut()->requirePermission('client'); } } diff --git a/tests/unit/src/Services/AuthenticationServiceTest.php b/tests/unit/src/Services/AuthenticationServiceTest.php index fd9b2024..deae3f3d 100644 --- a/tests/unit/src/Services/AuthenticationServiceTest.php +++ b/tests/unit/src/Services/AuthenticationServiceTest.php @@ -331,11 +331,8 @@ public function testGetAuthenticateUserItThrowsIfClaimsNotExist(): void public function testItAuthenticates(): void { $this->authSimpleMock->expects($this->once())->method('login')->with([]); - $this->clientHelperMock->expects($this->once()) - ->method('getFromRequest') - ->willReturn($this->clientEntityMock); - $this->mock()->authenticate($this->serverRequestMock); + $this->mock()->authenticate($this->clientEntityMock); } /** diff --git a/tests/unit/src/Utils/UniqueIdentifierGeneratorTest.php b/tests/unit/src/Utils/UniqueIdentifierGeneratorTest.php deleted file mode 100644 index b8a15931..00000000 --- a/tests/unit/src/Utils/UniqueIdentifierGeneratorTest.php +++ /dev/null @@ -1,25 +0,0 @@ -assertNotEquals($id1, $id2); - } -}