diff --git a/README.md b/README.md index 797bdfea..6b3c0e94 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ PHP version requirement changes in minor releases for SimpleSAMLphp. | OIDC module | Tested SimpleSAMLphp | PHP | Note | |:------------|:---------------------|:------:|-----------------------------| -| v6.\* | v2.2.\* | \>=8.2 | Recommended | +| v6.\* | v2.3.\* | \>=8.2 | Recommended | | v5.\* | v2.1.\* | \>=8.1 | | | v4.\* | v2.0.\* | \>=8.0 | | | v3.\* | v2.0.\* | \>=7.4 | Abandoned from August 2023. | @@ -329,7 +329,7 @@ docker run --name ssp-oidc-dev \ --mount type=bind,source="$(pwd)/docker/ssp/oidc_module.crt",target=/var/simplesamlphp/cert/oidc_module.crt,readonly \ --mount type=bind,source="$(pwd)/docker/ssp/oidc_module.key",target=/var/simplesamlphp/cert/oidc_module.key,readonly \ --mount type=bind,source="$(pwd)/docker/apache-override.cf",target=/etc/apache2/sites-enabled/ssp-override.cf,readonly \ - -p 443:443 cirrusid/simplesamlphp:v2.2.2 + -p 443:443 cirrusid/simplesamlphp:v2.3.5 ``` Visit https://localhost/simplesaml/ and confirm you get the default page. diff --git a/UPGRADE.md b/UPGRADE.md index a6f42115..9a766943 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -91,7 +91,7 @@ has been refactored: - upgraded to v5 of lcobucci/jwt https://github.com/lcobucci/jwt - upgraded to v3 of laminas/laminas-diactoros https://github.com/laminas/laminas-diactoros -- SimpleSAMLphp version used during development was bumped to v2.2 +- SimpleSAMLphp version used during development was bumped to v2.3 - In Authorization Code Flow, a new validation was added which checks for 'openid' value in 'scope' parameter. Up to now, 'openid' value was dynamically added if not present. In Implicit Code Flow this validation was already present. diff --git a/composer.json b/composer.json index 8c09654f..69de0580 100644 --- a/composer.json +++ b/composer.json @@ -43,7 +43,7 @@ "friendsofphp/php-cs-fixer": "^3", "phpunit/phpunit": "^10", "rector/rector": "^0.18.3", - "simplesamlphp/simplesamlphp": "2.2.*", + "simplesamlphp/simplesamlphp": "2.3.*", "simplesamlphp/simplesamlphp-test-framework": "^1.5", "squizlabs/php_codesniffer": "^3", "vimeo/psalm": "^5", @@ -56,9 +56,10 @@ }, "sort-packages": true, "allow-plugins": { - "simplesamlphp/composer-module-installer": true, "dealerdirect/phpcodesniffer-composer-installer": true, - "phpstan/extension-installer": true + "phpstan/extension-installer": true, + "simplesamlphp/composer-module-installer": true, + "simplesamlphp/composer-xmlprovider-installer": true }, "cache-dir": "build/composer" }, diff --git a/docker/Dockerfile b/docker/Dockerfile index c8a12a77..46543010 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,5 +1,5 @@ -#FROM cirrusid/simplesamlphp:v2.2.2 -FROM cicnavi/simplesamlphp:dev +FROM cirrusid/simplesamlphp:v2.3.5 +#FROM cicnavi/simplesamlphp:dev RUN apt-get update && apt-get install -y sqlite3 # Prepopulate the DB with items needed for testing diff --git a/src/Admin/Menu.php b/src/Admin/Menu.php index 0c5e15a6..0ccbb8ae 100644 --- a/src/Admin/Menu.php +++ b/src/Admin/Menu.php @@ -20,7 +20,7 @@ public function __construct(Item ...$items) array_push($this->items, ...$items); } - public function addItem(Item $menuItem, int $offset = null): void + public function addItem(Item $menuItem, ?int $offset = null): void { $offset ??= count($this->items); diff --git a/src/Entities/AccessTokenEntity.php b/src/Entities/AccessTokenEntity.php index 5873044e..67630acd 100644 --- a/src/Entities/AccessTokenEntity.php +++ b/src/Entities/AccessTokenEntity.php @@ -65,11 +65,11 @@ public function __construct( DateTimeImmutable $expiryDateTime, CryptKey $privateKey, protected JsonWebTokenBuilderService $jsonWebTokenBuilderService, - int|string $userIdentifier = null, - string $authCodeId = null, - array $requestedClaims = null, - bool $isRevoked = false, - Configuration $jwtConfiguration = null, + int|string|null $userIdentifier = null, + ?string $authCodeId = null, + ?array $requestedClaims = null, + ?bool $isRevoked = false, + ?Configuration $jwtConfiguration = null, ) { $this->setIdentifier($id); $this->setClient($clientEntity); diff --git a/src/Entities/AuthCodeEntity.php b/src/Entities/AuthCodeEntity.php index c96488c7..c0bf7c0a 100644 --- a/src/Entities/AuthCodeEntity.php +++ b/src/Entities/AuthCodeEntity.php @@ -40,9 +40,9 @@ public function __construct( OAuth2ClientEntityInterface $client, array $scopes, DateTimeImmutable $expiryDateTime, - string $userIdentifier = null, - string $redirectUri = null, - string $nonce = null, + ?string $userIdentifier = null, + ?string $redirectUri = null, + ?string $nonce = null, bool $isRevoked = false, ) { $this->identifier = $id; diff --git a/src/Factories/Entities/AccessTokenEntityFactory.php b/src/Factories/Entities/AccessTokenEntityFactory.php index f656fa12..f4672e98 100644 --- a/src/Factories/Entities/AccessTokenEntityFactory.php +++ b/src/Factories/Entities/AccessTokenEntityFactory.php @@ -31,10 +31,10 @@ public function fromData( OAuth2ClientEntityInterface $clientEntity, array $scopes, DateTimeImmutable $expiryDateTime, - int|string $userIdentifier = null, - string $authCodeId = null, - array $requestedClaims = null, - bool $isRevoked = false, + int|string|null $userIdentifier = null, + ?string $authCodeId = null, + ?array $requestedClaims = null, + ?bool $isRevoked = false, ): AccessTokenEntity { return new AccessTokenEntity( $id, diff --git a/src/Factories/Entities/AuthCodeEntityFactory.php b/src/Factories/Entities/AuthCodeEntityFactory.php index 30d65939..be0cdee2 100644 --- a/src/Factories/Entities/AuthCodeEntityFactory.php +++ b/src/Factories/Entities/AuthCodeEntityFactory.php @@ -27,9 +27,9 @@ public function fromData( OAuth2ClientEntityInterface $client, array $scopes, DateTimeImmutable $expiryDateTime, - string $userIdentifier = null, - string $redirectUri = null, - string $nonce = null, + ?string $userIdentifier = null, + ?string $redirectUri = null, + ?string $nonce = null, bool $isRevoked = false, ): AuthCodeEntity { return new AuthCodeEntity( diff --git a/src/Factories/Entities/ScopeEntityFactory.php b/src/Factories/Entities/ScopeEntityFactory.php index 36e4da7f..b12ef45a 100644 --- a/src/Factories/Entities/ScopeEntityFactory.php +++ b/src/Factories/Entities/ScopeEntityFactory.php @@ -13,8 +13,8 @@ class ScopeEntityFactory */ public function fromData( string $identifier, - string $description = null, - string $icon = null, + ?string $description = null, + ?string $icon = null, array $claims = [], ): ScopeEntity { return new ScopeEntity( diff --git a/src/Factories/TemplateFactory.php b/src/Factories/TemplateFactory.php index de0d223c..a3039779 100644 --- a/src/Factories/TemplateFactory.php +++ b/src/Factories/TemplateFactory.php @@ -49,7 +49,7 @@ public function __construct( public function build( string $templateName, array $data = [], - string $activeHrefPath = null, + ?string $activeHrefPath = null, ?bool $includeDefaultMenuItems = null, ?bool $showMenu = null, ?bool $showModuleName = null, diff --git a/src/ModuleConfig.php b/src/ModuleConfig.php index 6196ddb2..973d1f16 100644 --- a/src/ModuleConfig.php +++ b/src/ModuleConfig.php @@ -118,7 +118,7 @@ class ModuleConfig public function __construct( string $fileName = self::DEFAULT_FILE_NAME, // Primarily used for easy (unit) testing overrides. array $overrides = [], // Primarily used for easy (unit) testing overrides. - Configuration $sspConfig = null, + ?Configuration $sspConfig = null, private readonly SspBridge $sspBridge = new SspBridge(), ) { $this->moduleConfig = Configuration::loadFromArray( @@ -225,7 +225,7 @@ public function config(): Configuration } // TODO mivanci Move to dedicated \SimpleSAML\Module\oidc\Utils\Routes::getModuleUrl - public function getModuleUrl(string $path = null): string + public function getModuleUrl(?string $path = null): string { $base = $this->sspBridge->module()->getModuleURL(self::MODULE_NAME); diff --git a/src/Repositories/AccessTokenRepository.php b/src/Repositories/AccessTokenRepository.php index 3e1ac577..4298211e 100644 --- a/src/Repositories/AccessTokenRepository.php +++ b/src/Repositories/AccessTokenRepository.php @@ -63,10 +63,10 @@ public function getNewToken( OAuth2ClientEntityInterface $clientEntity, array $scopes, $userIdentifier = null, - string $authCodeId = null, - array $requestedClaims = null, - string $id = null, - DateTimeImmutable $expiryDateTime = null, + ?string $authCodeId = null, + ?array $requestedClaims = null, + ?string $id = null, + ?DateTimeImmutable $expiryDateTime = null, ): AccessTokenEntityInterface { if (!is_null($userIdentifier)) { $userIdentifier = (string)$userIdentifier; diff --git a/src/Repositories/Interfaces/AccessTokenRepositoryInterface.php b/src/Repositories/Interfaces/AccessTokenRepositoryInterface.php index dae29026..18453a20 100644 --- a/src/Repositories/Interfaces/AccessTokenRepositoryInterface.php +++ b/src/Repositories/Interfaces/AccessTokenRepositoryInterface.php @@ -29,7 +29,7 @@ public function getNewToken( OAuth2ClientEntityInterface $clientEntity, array $scopes, $userIdentifier = null, - string $authCodeId = null, - array $requestedClaims = null, + ?string $authCodeId = null, + ?array $requestedClaims = null, ): AccessTokenEntityInterface; } diff --git a/src/Server/AuthorizationServer.php b/src/Server/AuthorizationServer.php index 70d946e1..4c444e70 100644 --- a/src/Server/AuthorizationServer.php +++ b/src/Server/AuthorizationServer.php @@ -49,8 +49,8 @@ public function __construct( ScopeRepositoryInterface $scopeRepository, CryptKey|string $privateKey, Key|string $encryptionKey, - ResponseTypeInterface $responseType = null, - RequestRulesManager $requestRulesManager = null, + ?ResponseTypeInterface $responseType = null, + ?RequestRulesManager $requestRulesManager = null, ) { parent::__construct( $clientRepository, diff --git a/src/Server/Exceptions/OidcServerException.php b/src/Server/Exceptions/OidcServerException.php index 695b5e69..5a9be60d 100644 --- a/src/Server/Exceptions/OidcServerException.php +++ b/src/Server/Exceptions/OidcServerException.php @@ -6,6 +6,7 @@ use League\OAuth2\Server\Exception\OAuthServerException; use Psr\Http\Message\ResponseInterface; +use SimpleSAML\OpenID\Codebooks\ErrorsEnum; use Throwable; use function http_build_query; @@ -57,10 +58,10 @@ public function __construct( int $code, string $errorType, int $httpStatusCode = 400, - string $hint = null, - string $redirectUri = null, - Throwable $previous = null, - string $state = null, + ?string $hint = null, + ?string $redirectUri = null, + ?Throwable $previous = null, + ?string $state = null, ) { parent::__construct($message, $code, $errorType, $httpStatusCode, $hint, $redirectUri, $previous); @@ -93,8 +94,8 @@ public function __construct( * @return self */ public static function unsupportedResponseType( - string $redirectUri = null, - string $state = null, + ?string $redirectUri = null, + ?string $state = null, bool $useFragment = false, ): OidcServerException { $errorMessage = 'The response type is not supported by the authorization server.'; @@ -117,7 +118,7 @@ public static function unsupportedResponseType( public static function invalidScope( $scope, $redirectUri = null, - string $state = null, + ?string $state = null, bool $useFragment = false, ): OidcServerException { // OAuthServerException correctly implements this error, however, it misses state parameter. @@ -142,9 +143,9 @@ public static function invalidScope( public static function invalidRequest( $parameter, $hint = null, - Throwable $previous = null, - string $redirectUri = null, - string $state = null, + ?Throwable $previous = null, + ?string $redirectUri = null, + ?string $state = null, bool $useFragment = false, ): OidcServerException { $e = parent::invalidRequest($parameter, $hint, $previous); @@ -167,8 +168,8 @@ public static function invalidRequest( public static function accessDenied( $hint = null, $redirectUri = null, - Throwable $previous = null, - string $state = null, + ?Throwable $previous = null, + ?string $state = null, bool $useFragment = false, ): OidcServerException { $e = parent::accessDenied($hint, $redirectUri, $previous); @@ -190,10 +191,10 @@ public static function accessDenied( * @return self */ public static function loginRequired( - string $hint = null, - string $redirectUri = null, - Throwable $previous = null, - string $state = null, + ?string $hint = null, + ?string $redirectUri = null, + ?Throwable $previous = null, + ?string $state = null, bool $useFragment = false, ): OidcServerException { $errorMessage = "End-User is not already authenticated."; @@ -216,10 +217,10 @@ public static function loginRequired( * @return self */ public static function requestNotSupported( - string $hint = null, - string $redirectUri = null, - Throwable $previous = null, - string $state = null, + ?string $hint = null, + ?string $redirectUri = null, + ?Throwable $previous = null, + ?string $state = null, bool $useFragment = false, ): OidcServerException { $errorMessage = "Request object not supported."; @@ -239,21 +240,30 @@ public static function requestNotSupported( * @return self * @psalm-suppress LessSpecificImplementedReturnType */ - public static function invalidRefreshToken($hint = null, Throwable $previous = null): OidcServerException + public static function invalidRefreshToken($hint = null, ?Throwable $previous = null): OidcServerException { return new self('The refresh token is invalid.', 8, 'invalid_grant', 400, $hint, null, $previous); } public static function invalidTrustChain( - string $hint = null, - string $redirectUri = null, - Throwable $previous = null, - string $state = null, + ?string $hint = null, + ?string $redirectUri = null, + ?Throwable $previous = null, + ?string $state = null, bool $useFragment = false, ): OidcServerException { $errorMessage = 'Trust chain validation failed.'; - $e = new self($errorMessage, 12, 'trust_chain_validation_failed', 400, $hint, $redirectUri, $previous, $state); + $e = new self( + $errorMessage, + 12, + ErrorsEnum::InvalidTrustChain->value, + 400, + $hint, + $redirectUri, + $previous, + $state, + ); $e->useFragmentInHttpResponses($useFragment); return $e; @@ -268,7 +278,7 @@ public static function invalidTrustChain( * @return self * @psalm-suppress LessSpecificImplementedReturnType */ - public static function forbidden(string $hint = null, Throwable $previous = null): OidcServerException + public static function forbidden(?string $hint = null, ?Throwable $previous = null): OidcServerException { return new self( 'Request understood, but refused to process it.', @@ -304,7 +314,7 @@ public function setPayload(array $payload): void /** * @param string|null $redirectUri Set to string, or unset it with null */ - public function setRedirectUri(string $redirectUri = null): void + public function setRedirectUri(?string $redirectUri = null): void { $this->redirectUri = $redirectUri; } @@ -337,7 +347,7 @@ public function getRedirectUri(): ?string /** * @param string|null $state Set to string, or unset it with null */ - public function setState(string $state = null): void + public function setState(?string $state = null): void { if ($state === null) { unset($this->payload['state']); diff --git a/src/Server/Grants/AuthCodeGrant.php b/src/Server/Grants/AuthCodeGrant.php index aec720b9..5d73bcaf 100644 --- a/src/Server/Grants/AuthCodeGrant.php +++ b/src/Server/Grants/AuthCodeGrant.php @@ -314,7 +314,7 @@ protected function issueOidcAuthCode( string $userIdentifier, string $redirectUri, array $scopes = [], - string $nonce = null, + ?string $nonce = null, ): AuthCodeEntityInterface { $maxGenerationAttempts = self::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS; @@ -748,7 +748,7 @@ public function validateAuthorizationRequestWithCheckerResultBag( */ protected function issueRefreshToken( OAuth2AccessTokenEntityInterface $accessToken, - string $authCodeId = null, + ?string $authCodeId = null, ): ?RefreshTokenEntityInterface { if (! is_a($accessToken, AccessTokenEntityInterface::class)) { throw OidcServerException::serverError('Unexpected access token entity type.'); diff --git a/src/Server/Grants/Traits/IssueAccessTokenTrait.php b/src/Server/Grants/Traits/IssueAccessTokenTrait.php index 742c756b..6660ec92 100644 --- a/src/Server/Grants/Traits/IssueAccessTokenTrait.php +++ b/src/Server/Grants/Traits/IssueAccessTokenTrait.php @@ -48,8 +48,8 @@ protected function issueAccessToken( ClientEntityInterface $client, $userIdentifier = null, array $scopes = [], - string $authCodeId = null, - array $requestedClaims = null, + ?string $authCodeId = null, + ?array $requestedClaims = null, ): AccessTokenEntityInterface { $maxGenerationAttempts = AbstractGrant::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS; diff --git a/src/Server/LogoutHandlers/BackChannelLogoutHandler.php b/src/Server/LogoutHandlers/BackChannelLogoutHandler.php index a0987572..e1fb8478 100644 --- a/src/Server/LogoutHandlers/BackChannelLogoutHandler.php +++ b/src/Server/LogoutHandlers/BackChannelLogoutHandler.php @@ -29,7 +29,7 @@ public function __construct( * @param \GuzzleHttp\HandlerStack|null $handlerStack For easier testing * @throws \League\OAuth2\Server\Exception\OAuthServerException */ - public function handle(array $relyingPartyAssociations, HandlerStack $handlerStack = null): void + public function handle(array $relyingPartyAssociations, ?HandlerStack $handlerStack = null): void { $clientConfig = ['timeout' => 3, 'verify' => false, 'handler' => $handlerStack]; diff --git a/src/Server/TokenIssuers/RefreshTokenIssuer.php b/src/Server/TokenIssuers/RefreshTokenIssuer.php index 8aad35d2..f136dded 100644 --- a/src/Server/TokenIssuers/RefreshTokenIssuer.php +++ b/src/Server/TokenIssuers/RefreshTokenIssuer.php @@ -35,7 +35,7 @@ public function __construct( public function issue( Oauth2TokenEntityInterface $accessToken, DateInterval $refreshTokenTtl, - string $authCodeId = null, + ?string $authCodeId = null, int $maxGenerationAttempts = self::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS, ): ?RefreshTokenEntityInterface { if (! is_a($accessToken, AccessTokenEntityInterface::class)) { diff --git a/src/Server/Validators/BearerTokenValidator.php b/src/Server/Validators/BearerTokenValidator.php index c6a80572..94c7b183 100644 --- a/src/Server/Validators/BearerTokenValidator.php +++ b/src/Server/Validators/BearerTokenValidator.php @@ -44,7 +44,7 @@ class BearerTokenValidator extends OAuth2BearerTokenValidator public function __construct( AccessTokenRepositoryInterface $accessTokenRepository, CryptKey $publicKey, - DateInterval $jwtValidAtDateLeeway = null, + ?DateInterval $jwtValidAtDateLeeway = null, protected LoggerService $loggerService = new LoggerService(), ) { parent::__construct($accessTokenRepository, $jwtValidAtDateLeeway); diff --git a/src/Services/DatabaseMigration.php b/src/Services/DatabaseMigration.php index 8f4f3a74..a4936e88 100644 --- a/src/Services/DatabaseMigration.php +++ b/src/Services/DatabaseMigration.php @@ -30,7 +30,7 @@ class DatabaseMigration { private readonly Database $database; - public function __construct(Database $database = null) + public function __construct(?Database $database = null) { $this->database = $database ?? Database::getInstance(); } diff --git a/tests/unit/src/Repositories/UserRepositoryTest.php b/tests/unit/src/Repositories/UserRepositoryTest.php index fc2e7270..ec2189b0 100644 --- a/tests/unit/src/Repositories/UserRepositoryTest.php +++ b/tests/unit/src/Repositories/UserRepositoryTest.php @@ -79,11 +79,11 @@ protected function setUp(): void } protected function mock( - ModuleConfig|MockObject $moduleConfig = null, - Database|MockObject $database = null, - ProtocolCache|MockObject $protocolCache = null, - Helpers|MockObject $helpers = null, - UserEntityFactory|MockObject $userEntityFactory = null, + ?ModuleConfig $moduleConfig = null, + ?Database $database = null, + ?ProtocolCache $protocolCache = null, + ?Helpers $helpers = null, + ?UserEntityFactory $userEntityFactory = null, ): UserRepository { $moduleConfig ??= $this->moduleConfigMock; $database ??= $this->database; // Let's use real database instance for tests by default. diff --git a/tests/unit/src/Utils/RequestParamsResolverTest.php b/tests/unit/src/Utils/RequestParamsResolverTest.php index da084d8e..0cbc269a 100644 --- a/tests/unit/src/Utils/RequestParamsResolverTest.php +++ b/tests/unit/src/Utils/RequestParamsResolverTest.php @@ -57,9 +57,9 @@ protected function setUp(): void } protected function mock( - MockObject $helpersMock = null, - MockObject $coreMock = null, - MockObject $federationMock = null, + ?MockObject $helpersMock = null, + ?MockObject $coreMock = null, + ?MockObject $federationMock = null, ): RequestParamsResolver { $helpersMock ??= $this->helpersMock; $coreMock ??= $this->coreMock;