diff --git a/composer.json b/composer.json index 84fd2e48..abe3ddeb 100644 --- a/composer.json +++ b/composer.json @@ -26,18 +26,18 @@ "laminas/laminas-diactoros": "^3", "laminas/laminas-httphandlerrunner": "^2", "lcobucci/jwt": "^5.3", - "league/oauth2-server": "^8.5.3", + "league/oauth2-server": "^9.4", "nette/forms": "^3", "psr/container": "^2.0", "psr/log": "^3", + "psr/simple-cache": "^3", "simplesamlphp/composer-module-installer": "^1.3", "simplesamlphp/openid": "~0.3.8", "spomky-labs/base64url": "^2.0", + "symfony/cache": "^7.4", "symfony/expression-language": "^7.4", "symfony/psr-http-message-bridge": "^7.4", - "web-token/jwt-framework": "^3.4.10", - "symfony/cache": "^7.4", - "psr/simple-cache": "^3" + "web-token/jwt-framework": "^3.4.10" }, "require-dev": { "friendsofphp/php-cs-fixer": "^3", diff --git a/src/Entities/AccessTokenEntity.php b/src/Entities/AccessTokenEntity.php index 9b46904c..55c42ae5 100644 --- a/src/Entities/AccessTokenEntity.php +++ b/src/Entities/AccessTokenEntity.php @@ -75,13 +75,22 @@ public function __construct( protected readonly ?string $boundRedirectUri = null, protected readonly ?string $issuerState = null, ) { + if ($id === '') { + throw new \InvalidArgumentException('Access token identifier cannot be empty.'); + } + $this->setIdentifier($id); $this->setClient($clientEntity); foreach ($scopes as $scope) { $this->addScope($scope); } $this->setExpiryDateTime($expiryDateTime); - $this->setUserIdentifier($userIdentifier); + if (!is_null($userIdentifier)) { + $userIdentifier = (string)$userIdentifier; + } + if (!empty($userIdentifier)) { + $this->setUserIdentifier($userIdentifier); + } $this->setAuthCodeId($authCodeId); $this->setRequestedClaims($requestedClaims ?? []); if ($isRevoked) { @@ -134,16 +143,16 @@ public function getState(): array */ public function __toString(): string { - return $this->stringRepresentation = $this->convertToJWT()->getToken(); + return $this->toString(); } /** * Get string representation of access token at the moment of casting it to string. - * @return string|null String representation or null if it was not cast to string yet. + * @return string String representation of the access token. */ - public function toString(): ?string + public function toString(): string { - return $this->stringRepresentation; + return $this->stringRepresentation ??= $this->convertToJWT()->getToken(); } /** @@ -162,7 +171,7 @@ protected function convertToJWT(): ParsedJws $payload = array_filter([ ClaimsEnum::Iss->value => $this->moduleConfig->getIssuer(), ClaimsEnum::Iat->value => $currentTimestamp, - ClaimsEnum::Jti->value => (string)$this->getIdentifier(), + ClaimsEnum::Jti->value => $this->getIdentifier(), ClaimsEnum::Aud->value => $this->getClient()->getIdentifier(), ClaimsEnum::Nbf->value => $currentTimestamp, ClaimsEnum::Exp->value => $this->expiryDateTime->getTimestamp(), diff --git a/src/Entities/AuthCodeEntity.php b/src/Entities/AuthCodeEntity.php index 2cacb7e1..f68361cc 100644 --- a/src/Entities/AuthCodeEntity.php +++ b/src/Entities/AuthCodeEntity.php @@ -51,11 +51,15 @@ public function __construct( protected readonly ?string $boundRedirectUri = null, protected readonly ?string $issuerState = null, ) { + if ($id === '') { + throw new \InvalidArgumentException('Authorization code identifier cannot be empty.'); + } + $this->identifier = $id; $this->client = $client; $this->scopes = $scopes; $this->expiryDateTime = $expiryDateTime; - $this->userIdentifier = $userIdentifier; + $this->userIdentifier = $userIdentifier === '' ? null : $userIdentifier; $this->redirectUri = $redirectUri; $this->nonce = $nonce; $this->isRevoked = $isRevoked; diff --git a/src/Entities/ClientEntity.php b/src/Entities/ClientEntity.php index 23a5c09b..f7772452 100644 --- a/src/Entities/ClientEntity.php +++ b/src/Entities/ClientEntity.php @@ -162,6 +162,10 @@ public function __construct( ?array $extraMetadata = null, ?string $registrationAccessToken = null, ) { + if ($identifier === '') { + throw new \InvalidArgumentException('Client identifier cannot be empty.'); + } + $this->identifier = $identifier; $this->secret = $secret; $this->name = $name; diff --git a/src/Entities/Interfaces/EntityStringRepresentationInterface.php b/src/Entities/Interfaces/EntityStringRepresentationInterface.php index 4305f5a2..da5510e5 100644 --- a/src/Entities/Interfaces/EntityStringRepresentationInterface.php +++ b/src/Entities/Interfaces/EntityStringRepresentationInterface.php @@ -9,5 +9,5 @@ interface EntityStringRepresentationInterface /** * Generate string representation of entity. */ - public function toString(): ?string; + public function toString(): string; } diff --git a/src/Entities/RefreshTokenEntity.php b/src/Entities/RefreshTokenEntity.php index de12e766..08a08b37 100644 --- a/src/Entities/RefreshTokenEntity.php +++ b/src/Entities/RefreshTokenEntity.php @@ -38,6 +38,10 @@ public function __construct( ?string $authCodeId = null, bool $isRevoked = false, ) { + if ($id === '') { + throw new \InvalidArgumentException('Refresh token identifier cannot be empty.'); + } + $this->setIdentifier($id); $this->setExpiryDateTime($expiryDateTime); $this->setAccessToken($accessTokenEntity); diff --git a/src/Entities/ScopeEntity.php b/src/Entities/ScopeEntity.php index 648c8d4e..c3639409 100644 --- a/src/Entities/ScopeEntity.php +++ b/src/Entities/ScopeEntity.php @@ -35,6 +35,10 @@ public function __construct( protected ?string $icon = null, protected array $claims = [], ) { + if ($identifier === '') { + throw new \InvalidArgumentException('Scope identifier cannot be empty.'); + } + $this->identifier = $identifier; } @@ -58,6 +62,6 @@ public function getClaims(): array public function jsonSerialize(): string { - return (string) $this->getIdentifier(); + return $this->getIdentifier(); } } diff --git a/src/Entities/UserEntity.php b/src/Entities/UserEntity.php index e62a73c3..db7f34f4 100644 --- a/src/Entities/UserEntity.php +++ b/src/Entities/UserEntity.php @@ -26,12 +26,20 @@ */ class UserEntity implements UserEntityInterface, MementoInterface, ClaimSetInterface { + /** @var non-empty-string */ + private readonly string $identifier; + public function __construct( - private readonly string $identifier, + string $identifier, private readonly DateTimeImmutable $createdAt, private DateTimeImmutable $updatedAt, private array $claims = [], ) { + if ($identifier === '') { + throw new \InvalidArgumentException('User identifier cannot be empty.'); + } + + $this->identifier = $identifier; } /** diff --git a/src/Factories/Entities/ClientEntityFactory.php b/src/Factories/Entities/ClientEntityFactory.php index 1394db0a..0261464a 100644 --- a/src/Factories/Entities/ClientEntityFactory.php +++ b/src/Factories/Entities/ClientEntityFactory.php @@ -168,8 +168,10 @@ public function fromRegistrationData( $isDcrUpdate = $existingClient !== null && $registrationType === RegistrationTypeEnum::Dynamic; $metadataFallbackClient = $isDcrUpdate ? null : $existingClient; - $id = $clientIdentifier ?? $existingClient?->getIdentifier() ?? - $this->sspBridge->utils()->random()->generateID(); + $id = $clientIdentifier ?: $existingClient?->getIdentifier(); + if (empty($id)) { + $id = $this->sspBridge->utils()->random()->generateID(); + } $secret = $existingClient?->getSecret() ?? $this->sspBridge->utils()->random()->generateID(); diff --git a/src/Repositories/AccessTokenRepository.php b/src/Repositories/AccessTokenRepository.php index 422aa238..01565e7d 100644 --- a/src/Repositories/AccessTokenRepository.php +++ b/src/Repositories/AccessTokenRepository.php @@ -61,15 +61,12 @@ public function getTableName(): string public function getNewToken( OAuth2ClientEntityInterface $clientEntity, array $scopes, - $userIdentifier = null, + ?string $userIdentifier = null, ?string $authCodeId = null, ?array $requestedClaims = null, ?string $id = null, ?DateTimeImmutable $expiryDateTime = null, ): AccessTokenEntityInterface { - if (!is_null($userIdentifier)) { - $userIdentifier = (string)$userIdentifier; - } if (empty($userIdentifier)) { $userIdentifier = null; } @@ -145,7 +142,7 @@ public function persistNewAccessToken(OAuth2AccessTokenEntityInterface $accessTo $this->helpers->dateTime()->getSecondsToExpirationTime( $accessTokenEntity->getExpiryDateTime()->getTimestamp(), ), - $this->getCacheKey((string)$accessTokenEntity->getIdentifier()), + $this->getCacheKey($accessTokenEntity->getIdentifier()), ); } @@ -184,7 +181,7 @@ public function findById(string $tokenId): ?AccessTokenEntity $this->helpers->dateTime()->getSecondsToExpirationTime( $accessTokenEntity->getExpiryDateTime()->getTimestamp(), ), - $this->getCacheKey((string)$accessTokenEntity->getIdentifier()), + $this->getCacheKey($accessTokenEntity->getIdentifier()), ); return $accessTokenEntity; @@ -195,7 +192,7 @@ public function findById(string $tokenId): ?AccessTokenEntity * @throws \JsonException * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException */ - public function revokeAccessToken($tokenId): void + public function revokeAccessToken(string $tokenId): void { $accessToken = $this->findById($tokenId); @@ -227,7 +224,7 @@ public function revokeByAuthCodeId(string $authCodeId): void * {@inheritdoc} * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException */ - public function isAccessTokenRevoked($tokenId): bool + public function isAccessTokenRevoked(string $tokenId): bool { $accessToken = $this->findById($tokenId); @@ -286,7 +283,7 @@ private function update(AccessTokenEntity $accessTokenEntity): void $this->helpers->dateTime()->getSecondsToExpirationTime( $accessTokenEntity->getExpiryDateTime()->getTimestamp(), ), - $this->getCacheKey((string)$accessTokenEntity->getIdentifier()), + $this->getCacheKey($accessTokenEntity->getIdentifier()), ); } diff --git a/src/Repositories/AuthCodeRepository.php b/src/Repositories/AuthCodeRepository.php index e8cd0920..bebf8433 100644 --- a/src/Repositories/AuthCodeRepository.php +++ b/src/Repositories/AuthCodeRepository.php @@ -117,7 +117,7 @@ public function persistNewAuthCode(OAuth2AuthCodeEntityInterface $authCodeEntity $this->helpers->dateTime()->getSecondsToExpirationTime( $authCodeEntity->getExpiryDateTime()->getTimestamp(), ), - $this->getCacheKey((string)$authCodeEntity->getIdentifier()), + $this->getCacheKey($authCodeEntity->getIdentifier()), ); } @@ -155,7 +155,7 @@ public function findById(string $codeId): ?AuthCodeEntity $this->helpers->dateTime()->getSecondsToExpirationTime( $authCodeEntity->getExpiryDateTime()->getTimestamp(), ), - $this->getCacheKey((string)$authCodeEntity->getIdentifier()), + $this->getCacheKey($authCodeEntity->getIdentifier()), ); return $authCodeEntity; @@ -166,7 +166,7 @@ public function findById(string $codeId): ?AuthCodeEntity * @throws \Exception * @throws \JsonException */ - public function revokeAuthCode($codeId): void + public function revokeAuthCode(string $codeId): void { $authCode = $this->findById($codeId); @@ -182,7 +182,7 @@ public function revokeAuthCode($codeId): void * {@inheritdoc} * @throws \Exception */ - public function isAuthCodeRevoked($codeId): bool + public function isAuthCodeRevoked(string $codeId): bool { $authCode = $this->findById($codeId); @@ -245,7 +245,7 @@ private function update(AuthCodeEntity $authCodeEntity): void $this->helpers->dateTime()->getSecondsToExpirationTime( $authCodeEntity->getExpiryDateTime()->getTimestamp(), ), - $this->getCacheKey((string)$authCodeEntity->getIdentifier()), + $this->getCacheKey($authCodeEntity->getIdentifier()), ); } diff --git a/src/Repositories/ClientRepository.php b/src/Repositories/ClientRepository.php index 7f240daf..46458d32 100644 --- a/src/Repositories/ClientRepository.php +++ b/src/Repositories/ClientRepository.php @@ -15,6 +15,7 @@ */ namespace SimpleSAML\Module\oidc\Repositories; +use League\OAuth2\Server\Entities\ClientEntityInterface as OAuth2ClientEntityInterface; use League\OAuth2\Server\Repositories\ClientRepositoryInterface; use PDO; use SimpleSAML\Database; @@ -48,7 +49,7 @@ public function getTableName(): string * @throws \JsonException * @throws \League\OAuth2\Server\Exception\OAuthServerException */ - public function getClientEntity($clientIdentifier) + public function getClientEntity(string $clientIdentifier): ?OAuth2ClientEntityInterface { $client = $this->findById($clientIdentifier); @@ -72,7 +73,7 @@ public function getClientEntity($clientIdentifier) * @throws \JsonException * @throws \League\OAuth2\Server\Exception\OAuthServerException */ - public function validateClient($clientIdentifier, $clientSecret, $grantType): bool + public function validateClient(string $clientIdentifier, ?string $clientSecret, ?string $grantType): bool { $client = $this->getClientEntity($clientIdentifier); diff --git a/src/Repositories/Interfaces/AccessTokenRepositoryInterface.php b/src/Repositories/Interfaces/AccessTokenRepositoryInterface.php index 18453a20..c6ba6f16 100644 --- a/src/Repositories/Interfaces/AccessTokenRepositoryInterface.php +++ b/src/Repositories/Interfaces/AccessTokenRepositoryInterface.php @@ -20,7 +20,7 @@ public function revokeByAuthCodeId(string $authCodeId): void; * * @param \League\OAuth2\Server\Entities\ClientEntityInterface $clientEntity * @param \League\OAuth2\Server\Entities\ScopeEntityInterface[] $scopes - * @param mixed $userIdentifier + * @param string|null $userIdentifier * @param string|null $authCodeId * @param array|null $requestedClaims Any requested claims * @return \SimpleSAML\Module\oidc\Entities\Interfaces\AccessTokenEntityInterface @@ -28,7 +28,7 @@ public function revokeByAuthCodeId(string $authCodeId): void; public function getNewToken( OAuth2ClientEntityInterface $clientEntity, array $scopes, - $userIdentifier = null, + ?string $userIdentifier = null, ?string $authCodeId = null, ?array $requestedClaims = null, ): AccessTokenEntityInterface; diff --git a/src/Repositories/RefreshTokenRepository.php b/src/Repositories/RefreshTokenRepository.php index 63876843..c2a02a31 100644 --- a/src/Repositories/RefreshTokenRepository.php +++ b/src/Repositories/RefreshTokenRepository.php @@ -57,7 +57,7 @@ public function getTableName(): string /** * {@inheritdoc} */ - public function getNewRefreshToken(): RefreshTokenEntityInterface + public function getNewRefreshToken(): ?RefreshTokenEntityInterface { throw new RuntimeException('Not implemented. Use RefreshTokenEntityFactory instead.'); } @@ -88,7 +88,7 @@ public function persistNewRefreshToken(OAuth2RefreshTokenEntityInterface $refres $this->helpers->dateTime()->getSecondsToExpirationTime( $refreshTokenEntity->getExpiryDateTime()->getTimestamp(), ), - $this->getCacheKey((string)$refreshTokenEntity->getIdentifier()), + $this->getCacheKey($refreshTokenEntity->getIdentifier()), ); } @@ -127,7 +127,7 @@ public function findById(string $tokenId): ?RefreshTokenEntityInterface $this->helpers->dateTime()->getSecondsToExpirationTime( $refreshTokenEntity->getExpiryDateTime()->getTimestamp(), ), - $this->getCacheKey((string)$refreshTokenEntity->getIdentifier()), + $this->getCacheKey($refreshTokenEntity->getIdentifier()), ); return $refreshTokenEntity; @@ -137,7 +137,7 @@ public function findById(string $tokenId): ?RefreshTokenEntityInterface * {@inheritdoc} * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException */ - public function revokeRefreshToken($tokenId): void + public function revokeRefreshToken(string $tokenId): void { $refreshToken = $this->findById($tokenId); @@ -168,7 +168,7 @@ public function revokeByAuthCodeId(string $authCodeId): void * {@inheritdoc} * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException */ - public function isRefreshTokenRevoked($tokenId): bool + public function isRefreshTokenRevoked(string $tokenId): bool { $refreshToken = $this->findById($tokenId); diff --git a/src/Repositories/ScopeRepository.php b/src/Repositories/ScopeRepository.php index fbc62aaa..3814a229 100644 --- a/src/Repositories/ScopeRepository.php +++ b/src/Repositories/ScopeRepository.php @@ -38,7 +38,7 @@ public function __construct( * {@inheritdoc} * @throws \Exception */ - public function getScopeEntityByIdentifier($identifier): ScopeEntity|ScopeEntityInterface|null + public function getScopeEntityByIdentifier(string $identifier): ScopeEntity|ScopeEntityInterface|null { $scopes = $this->moduleConfig->getScopes(); @@ -68,9 +68,10 @@ public function getScopeEntityByIdentifier($identifier): ScopeEntity|ScopeEntity */ public function finalizeScopes( array $scopes, - $grantType, + string $grantType, OAuth2ClientEntityInterface $clientEntity, - $userIdentifier = null, + ?string $userIdentifier = null, + ?string $authCodeId = null, ): array { if (!$clientEntity instanceof ClientEntity) { return []; diff --git a/src/Repositories/UserRepository.php b/src/Repositories/UserRepository.php index faf7da06..12722ef5 100644 --- a/src/Repositories/UserRepository.php +++ b/src/Repositories/UserRepository.php @@ -99,9 +99,9 @@ public function getUserEntityByIdentifier(string $identifier): ?UserEntity * @throws \Exception */ public function getUserEntityByUserCredentials( - $username, - $password, - $grantType, + string $username, + string $password, + string $grantType, OAuth2ClientEntityInterface $clientEntity, ): ?UserEntityInterface { throw new Exception('Not supported'); diff --git a/src/Server/AuthorizationServer.php b/src/Server/AuthorizationServer.php index 40ae9029..ddcdc74c 100644 --- a/src/Server/AuthorizationServer.php +++ b/src/Server/AuthorizationServer.php @@ -7,10 +7,11 @@ use Defuse\Crypto\Key; use League\OAuth2\Server\AuthorizationServer as OAuth2AuthorizationServer; use League\OAuth2\Server\CryptKey; +use League\OAuth2\Server\CryptKeyInterface; use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ClientRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; -use League\OAuth2\Server\RequestTypes\AuthorizationRequest as OAuth2AuthorizationRequest; +use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface as OAuth2AuthorizationRequestInterface; use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface; use LogicException; use Psr\Http\Message\ServerRequestInterface; @@ -31,6 +32,9 @@ use SimpleSAML\Module\oidc\Services\LoggerService; use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum; +/** + * @psalm-suppress PropertyNotSetInConstructor + */ class AuthorizationServer extends OAuth2AuthorizationServer { /** @psalm-suppress PossiblyUnusedProperty Private property in parent. */ @@ -39,10 +43,10 @@ class AuthorizationServer extends OAuth2AuthorizationServer protected RequestRulesManager $requestRulesManager; /** - * @var \League\OAuth2\Server\CryptKey + * @var \League\OAuth2\Server\CryptKeyInterface * @psalm-suppress PropertyNotSetInConstructor */ - protected $publicKey; + protected CryptKeyInterface $publicKey; /** * @inheritDoc @@ -80,7 +84,7 @@ public function __construct( * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException * @throws \Throwable */ - public function validateAuthorizationRequest(ServerRequestInterface $request): OAuth2AuthorizationRequest + public function validateAuthorizationRequest(ServerRequestInterface $request): OAuth2AuthorizationRequestInterface { $this->loggerService?->debug('AuthorizationServer::validateAuthorizationRequest'); diff --git a/src/Server/Exceptions/OidcServerException.php b/src/Server/Exceptions/OidcServerException.php index 1878ad7e..5c3eac80 100644 --- a/src/Server/Exceptions/OidcServerException.php +++ b/src/Server/Exceptions/OidcServerException.php @@ -16,46 +16,17 @@ class OidcServerException extends OAuthServerException { - /** - * @var array - */ - protected array $payload; - - /** - * @var int - * @psalm-suppress PossiblyUnusedProperty Property is private in parent. - */ - protected int $httpStatusCode; - - /** - * @var string - * @psalm-suppress PossiblyUnusedProperty Property is private in parent. - */ - protected string $errorType; - /** * @var null|string */ - protected ?string $redirectUri; + protected ?string $redirectUri = null; /** * @var null|ResponseModeInterface */ protected ?ResponseModeInterface $responseMode = null; - /** - * Throw a new exception. - * - * @param string $message Error message - * @param int $code Error code - * @param string $errorType Error type - * @param int $httpStatusCode HTTP status code to send (default = 400) - * @param null|string $hint A helper hint - * @param null|string $redirectUri An HTTP URI to redirect the user back to - * @param \Throwable|null $previous Previous exception - * @param string|null $state - */ - public function __construct( + private static function create( string $message, int $code, string $errorType, @@ -65,13 +36,11 @@ public function __construct( ?Throwable $previous = null, ?string $state = null, ?ResponseModeInterface $responseMode = null, - ) { - parent::__construct($message, $code, $errorType, $httpStatusCode, $hint, $redirectUri, $previous); + ): static { + $exception = new static($message, $code, $errorType, $httpStatusCode, $hint, $redirectUri, $previous); - $this->httpStatusCode = $httpStatusCode; - $this->errorType = $errorType; - $this->redirectUri = $redirectUri; - $this->responseMode = $responseMode; + $exception->redirectUri = $redirectUri; + $exception->responseMode = $responseMode; if ($hint !== null) { $message .= ' (' . $hint . ')'; @@ -86,7 +55,9 @@ public function __construct( $payload['state'] = $state; } - $this->payload = $payload; + $exception->setPayload($payload); + + return $exception; } /** @@ -95,7 +66,7 @@ public function __construct( * @param string|null $redirectUri * @param string|null $state * @param \SimpleSAML\Module\oidc\Server\ResponseModes\ResponseModeInterface|null $responseMode - * @return OidcServerException + * @return static */ public static function unsupportedResponseType( ?string $redirectUri = null, @@ -105,7 +76,7 @@ public static function unsupportedResponseType( $errorMessage = 'The response type is not supported by the authorization server.'; $hint = 'Check that all required parameters have been provided'; - return new self( + return self::create( $errorMessage, 2, 'unsupported_response_type', @@ -125,15 +96,15 @@ public static function unsupportedResponseType( * @param string|null $redirectUri An HTTP URI to redirect the user back to * @param string|null $state * @param \SimpleSAML\Module\oidc\Server\ResponseModes\ResponseModeInterface|null $responseMode - * @return OidcServerException + * @return static * @psalm-suppress LessSpecificImplementedReturnType */ public static function invalidScope( - $scope, - $redirectUri = null, + string $scope, + ?string $redirectUri = null, ?string $state = null, ?ResponseModeInterface $responseMode = null, - ): OidcServerException { + ): static { if (empty($scope)) { $hint = 'Specify a scope in the request or set a default scope'; } else { @@ -143,7 +114,7 @@ public static function invalidScope( ); } - $e = new self( + $e = self::create( 'The requested scope is invalid, unknown, or malformed', 5, 'invalid_scope', @@ -167,21 +138,31 @@ public static function invalidScope( * @param string|null $redirectUri * @param string|null $state * @param \SimpleSAML\Module\oidc\Server\ResponseModes\ResponseModeInterface|null $responseMode - * @return OidcServerException + * @return static * @psalm-suppress LessSpecificImplementedReturnType */ public static function invalidRequest( - $parameter, - $hint = null, + string $parameter, + ?string $hint = null, ?Throwable $previous = null, ?string $redirectUri = null, ?string $state = null, ?ResponseModeInterface $responseMode = null, - ): OidcServerException { + ): static { $errorMessage = 'The request is missing a required parameter, includes an invalid parameter value, ' . 'includes a parameter more than once, or is otherwise malformed.'; $hint = ($hint === null) ? \sprintf('Check the `%s` parameter', $parameter) : $hint; - $e = new self($errorMessage, 9, 'invalid_request', 400, $hint, $redirectUri, $previous, $state, $responseMode); + $e = self::create( + $errorMessage, + 9, + 'invalid_request', + 400, + $hint, + $redirectUri, + $previous, + $state, + $responseMode, + ); return $e; } @@ -192,17 +173,17 @@ public static function invalidRequest( * @param \Throwable|null $previous * @param string|null $state * @param \SimpleSAML\Module\oidc\Server\ResponseModes\ResponseModeInterface|null $responseMode - * @return OidcServerException + * @return static * @psalm-suppress LessSpecificImplementedReturnType */ public static function accessDenied( - $hint = null, - $redirectUri = null, + ?string $hint = null, + ?string $redirectUri = null, ?Throwable $previous = null, ?string $state = null, ?ResponseModeInterface $responseMode = null, - ): OidcServerException { - $e = new self( + ): static { + $e = self::create( 'The resource owner or authorization server denied the request.', 9, 'access_denied', @@ -233,8 +214,8 @@ public static function unauthorizedClient( ?Throwable $previous = null, ?string $state = null, ?ResponseModeInterface $responseMode = null, - ): OidcServerException { - return new self( + ): static { + return self::create( 'The client is not authorized to request a token using this method.', 10, 'unauthorized_client', @@ -256,7 +237,7 @@ public static function unauthorizedClient( * @param string|null $state * @param \SimpleSAML\Module\oidc\Server\ResponseModes\ResponseModeInterface|null $responseMode * - * @return OidcServerException + * @return static */ public static function loginRequired( ?string $hint = null, @@ -267,7 +248,17 @@ public static function loginRequired( ): OidcServerException { $errorMessage = "End-User is not already authenticated."; - $e = new self($errorMessage, 6, 'login_required', 400, $hint, $redirectUri, $previous, $state, $responseMode); + $e = self::create( + $errorMessage, + 6, + 'login_required', + 400, + $hint, + $redirectUri, + $previous, + $state, + $responseMode, + ); return $e; } @@ -281,7 +272,7 @@ public static function loginRequired( * @param string|null $state * @param \SimpleSAML\Module\oidc\Server\ResponseModes\ResponseModeInterface|null $responseMode * - * @return OidcServerException + * @return static */ public static function requestNotSupported( ?string $hint = null, @@ -292,7 +283,7 @@ public static function requestNotSupported( ): OidcServerException { $errorMessage = "Request object not supported."; - $e = new self( + $e = self::create( $errorMessage, 7, 'request_not_supported', @@ -313,12 +304,12 @@ public static function requestNotSupported( * @param string|null $hint * @param \Throwable|null $previous * - * @return OidcServerException + * @return static * @psalm-suppress LessSpecificImplementedReturnType */ - public static function invalidRefreshToken($hint = null, ?Throwable $previous = null): OidcServerException + public static function invalidRefreshToken(?string $hint = null, ?Throwable $previous = null): static { - return new self('The refresh token is invalid.', 8, 'invalid_grant', 400, $hint, null, $previous); + return self::create('The refresh token is invalid.', 8, 'invalid_grant', 400, $hint, null, $previous); } public static function invalidTrustChain( @@ -330,7 +321,7 @@ public static function invalidTrustChain( ): OidcServerException { $errorMessage = 'Trust chain validation failed.'; - $e = new self( + $e = self::create( $errorMessage, 12, ErrorsEnum::InvalidTrustChain->value, @@ -356,7 +347,7 @@ public static function invalidTrustChain( */ public static function forbidden(?string $hint = null, ?Throwable $previous = null): OidcServerException { - return new self( + return self::create( 'Request understood, but refused to process it.', 11, 'forbidden', @@ -385,7 +376,7 @@ public static function invalidClientMetadata( ?string $hint = null, ?Throwable $previous = null, ): OidcServerException { - return new self( + return self::create( 'The value of one of the client metadata fields is invalid and the server has rejected this request.', 13, ErrorsEnum::InvalidClientMetadata->value, @@ -414,7 +405,7 @@ public static function invalidRedirectUri( ?string $hint = null, ?Throwable $previous = null, ): OidcServerException { - return new self( + return self::create( 'The value of one or more redirect_uris is invalid.', 14, ErrorsEnum::InvalidRedirectUri->value, @@ -428,21 +419,21 @@ public static function invalidRedirectUri( /** * Returns the current payload. * - * @return array + * @return array */ public function getPayload(): array { - return $this->payload; + return parent::getPayload(); } /** * Updates the current payload. * - * @param array $payload + * @param array $payload */ public function setPayload(array $payload): void { - $this->payload = $payload; + parent::setPayload($payload); } /** @@ -484,11 +475,15 @@ public function getRedirectUri(): ?string public function setState(?string $state = null): void { if ($state === null) { - unset($this->payload['state']); + $payload = $this->getPayload(); + unset($payload['state']); + $this->setPayload($payload); return; } - $this->payload['state'] = $state; + $payload = $this->getPayload(); + $payload['state'] = $state; + $this->setPayload($payload); } /** @@ -505,7 +500,6 @@ public function generateHttpResponse( $useFragment = false, $jsonOptions = 0, ): ResponseInterface { - /** @var array $headers */ $headers = $this->getHttpHeaders(); $payload = $this->getPayload(); diff --git a/src/Server/Grants/AuthCodeGrant.php b/src/Server/Grants/AuthCodeGrant.php index acc95cc3..7b1ea5df 100644 --- a/src/Server/Grants/AuthCodeGrant.php +++ b/src/Server/Grants/AuthCodeGrant.php @@ -17,6 +17,7 @@ use League\OAuth2\Server\Repositories\AuthCodeRepositoryInterface as OAuth2AuthCodeRepositoryInterface; use League\OAuth2\Server\RequestEvent; use League\OAuth2\Server\RequestTypes\AuthorizationRequest as OAuth2AuthorizationRequest; +use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface as OAuth2AuthorizationRequestInterface; use League\OAuth2\Server\ResponseTypes\AbstractResponseType; use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface; use LogicException; @@ -80,6 +81,9 @@ use function array_key_exists; +/** + * @psalm-suppress PropertyNotSetInConstructor + */ class AuthCodeGrant extends OAuth2AuthCodeGrant implements // phpcs:ignore PkceEnabledGrantTypeInterface, @@ -95,60 +99,6 @@ class AuthCodeGrant extends OAuth2AuthCodeGrant implements /** @var \League\OAuth2\Server\CodeChallengeVerifiers\CodeChallengeVerifierInterface[] */ protected array $codeChallengeVerifiers = []; - /** - * @var \League\OAuth2\Server\Repositories\AuthCodeRepositoryInterface - * @psalm-suppress PropertyNotSetInConstructor - */ - protected $authCodeRepository; - - /** - * @var \League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface - * @psalm-suppress PropertyNotSetInConstructor - */ - protected $accessTokenRepository; - - /** - * @var \League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface - * @psalm-suppress PropertyNotSetInConstructor - */ - protected $refreshTokenRepository; - - /** - * @var bool - * @psalm-suppress PropertyNotSetInConstructor - */ - protected $revokeRefreshTokens; - - /** - * @var string - * @psalm-suppress PropertyNotSetInConstructor - */ - protected $defaultScope; - - /** - * @var \League\OAuth2\Server\Repositories\UserRepositoryInterface - * @psalm-suppress PropertyNotSetInConstructor - */ - protected $userRepository; - - /** - * @var \League\OAuth2\Server\Repositories\ScopeRepositoryInterface - * @psalm-suppress PropertyNotSetInConstructor - */ - protected $scopeRepository; - - /** - * @var \League\OAuth2\Server\Repositories\ClientRepositoryInterface - * @psalm-suppress PropertyNotSetInConstructor - */ - protected $clientRepository; - - /** - * @var \League\OAuth2\Server\CryptKey - * @psalm-suppress PropertyNotSetInConstructor - */ - protected $privateKey; - /** @var HttpMethodsEnum[] */ protected array $allowedAuthorizationHttpMethods = [HttpMethodsEnum::GET, HttpMethodsEnum::POST]; @@ -241,7 +191,7 @@ public function isOidcCandidate( * @throws \JsonException */ public function completeAuthorizationRequest( - OAuth2AuthorizationRequest $authorizationRequest, + OAuth2AuthorizationRequestInterface $authorizationRequest, ): ResponseTypeInterface { if ($authorizationRequest instanceof AuthorizationRequest) { return $this->completeOidcAuthorizationRequest($authorizationRequest); @@ -267,7 +217,7 @@ public function completeOidcAuthorizationRequest( } $finalRedirectUri = $authorizationRequest->getRedirectUri() - ?? $this->getClientRedirectUri($authorizationRequest); + ?? $this->getAuthorizationRequestClientRedirectUri($authorizationRequest); if ($authorizationRequest->isAuthorizationApproved() !== true) { // The user denied the client, redirect them back with an error @@ -378,8 +328,9 @@ protected function issueOidcAuthCode( * * @return string */ - protected function getClientRedirectUri(OAuth2AuthorizationRequest $authorizationRequest): string - { + protected function getAuthorizationRequestClientRedirectUri( + OAuth2AuthorizationRequest $authorizationRequest, + ): string { $rediretctUri = $authorizationRequest->getClient()->getRedirectUri(); if (is_array($rediretctUri)) { @@ -579,8 +530,13 @@ public function respondToAccessTokenRequest( $this->validateAuthorizationCode($authCodePayload, $client, $request, $storedAuthCodeEntity); + $authCodeScopes = $authCodePayload->scopes; + if (is_array($authCodeScopes)) { + $authCodeScopes = array_values(array_filter($authCodeScopes, 'is_string')); + } + $scopes = $this->scopeRepository->finalizeScopes( - $this->validateScopes($authCodePayload->scopes), + $this->validateScopes($authCodeScopes), $this->getIdentifier(), $client, $authCodePayload->user_id, @@ -772,7 +728,7 @@ protected function validateAuthorizationCode( public function validateAuthorizationRequestWithRequestRules( ServerRequestInterface $request, ResultBagInterface $resultBag, - ): OAuth2AuthorizationRequest { + ): OAuth2AuthorizationRequestInterface { $this->loggerService->debug('AuthCodeGrant::validateAuthorizationRequestWithRequestRules'); $rulesToExecute = [ diff --git a/src/Server/Grants/ImplicitGrant.php b/src/Server/Grants/ImplicitGrant.php index 7d6f2901..70437f01 100644 --- a/src/Server/Grants/ImplicitGrant.php +++ b/src/Server/Grants/ImplicitGrant.php @@ -6,7 +6,7 @@ use DateInterval; use League\OAuth2\Server\Grant\ImplicitGrant as OAuth2ImplicitGrant; -use League\OAuth2\Server\RequestTypes\AuthorizationRequest as OAuth2AuthorizationRequest; +use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface as OAuth2AuthorizationRequestInterface; use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface; use LogicException; use Psr\Http\Message\ServerRequestInterface; @@ -51,12 +51,6 @@ class ImplicitGrant extends OAuth2ImplicitGrant implements AuthorizationValidata /** @var HttpMethodsEnum[] */ protected array $allowedAuthorizationHttpMethods = [HttpMethodsEnum::GET, HttpMethodsEnum::POST]; - /** - * @psalm-suppress PropertyNotSetInConstructor - * @var \League\OAuth2\Server\CryptKey - */ - protected $privateKey; - public function __construct( protected IdTokenBuilder $idTokenBuilder, protected DateInterval $accessTokenTTL, @@ -98,14 +92,14 @@ public function canRespondToAuthorizationRequest(ServerRequestInterface $request /** * {@inheritdoc} - * @param \League\OAuth2\Server\RequestTypes\AuthorizationRequest $authorizationRequest + * @param \League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface $authorizationRequest * @return \League\OAuth2\Server\ResponseTypes\ResponseTypeInterface * @throws \League\OAuth2\Server\Exception\OAuthServerException * @throws \League\OAuth2\Server\Exception\UniqueTokenIdentifierConstraintViolationException * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException */ public function completeAuthorizationRequest( - OAuth2AuthorizationRequest $authorizationRequest, + OAuth2AuthorizationRequestInterface $authorizationRequest, ): ResponseTypeInterface { if ($authorizationRequest instanceof AuthorizationRequest) { return $this->completeOidcAuthorizationRequest($authorizationRequest); @@ -121,7 +115,7 @@ public function completeAuthorizationRequest( public function validateAuthorizationRequestWithRequestRules( ServerRequestInterface $request, ResultBagInterface $resultBag, - ): OAuth2AuthorizationRequest { + ): OAuth2AuthorizationRequestInterface { $rulesToExecute = [ ScopeRule::class, RequestObjectRule::class, @@ -253,7 +247,7 @@ private function completeOidcAuthorizationRequest(AuthorizationRequest $authoriz if ($authorizationRequest->shouldReturnAccessTokenInAuthorizationResponse()) { $addAccessTokenHashToIdToken = true; - $responseParams['access_token'] = $accessToken->toString() ?? (string) $accessToken; + $responseParams['access_token'] = $accessToken->toString(); $responseParams['token_type'] = 'Bearer'; $responseParams['expires_in'] = $accessToken->getExpiryDateTime()->getTimestamp() - time(); } diff --git a/src/Server/Grants/Interfaces/AuthorizationValidatableWithRequestRules.php b/src/Server/Grants/Interfaces/AuthorizationValidatableWithRequestRules.php index c0e1d49b..06c7cdcb 100644 --- a/src/Server/Grants/Interfaces/AuthorizationValidatableWithRequestRules.php +++ b/src/Server/Grants/Interfaces/AuthorizationValidatableWithRequestRules.php @@ -4,7 +4,7 @@ namespace SimpleSAML\Module\oidc\Server\Grants\Interfaces; -use League\OAuth2\Server\RequestTypes\AuthorizationRequest as OAuth2AuthorizationRequest; +use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface as OAuth2AuthorizationRequestInterface; use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Module\oidc\Server\RequestRules\Interfaces\ResultBagInterface; @@ -18,5 +18,5 @@ interface AuthorizationValidatableWithRequestRules public function validateAuthorizationRequestWithRequestRules( ServerRequestInterface $request, ResultBagInterface $resultBag, - ): OAuth2AuthorizationRequest; + ): OAuth2AuthorizationRequestInterface; } diff --git a/src/Server/Grants/PreAuthCodeGrant.php b/src/Server/Grants/PreAuthCodeGrant.php index 2a364e1b..3bfbee18 100644 --- a/src/Server/Grants/PreAuthCodeGrant.php +++ b/src/Server/Grants/PreAuthCodeGrant.php @@ -9,6 +9,7 @@ use League\OAuth2\Server\Entities\ClientEntityInterface as OAuth2ClientEntityInterface; use League\OAuth2\Server\RequestEvent; use League\OAuth2\Server\RequestTypes\AuthorizationRequest as OAuth2AuthorizationRequest; +use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface as OAuth2AuthorizationRequestInterface; use League\OAuth2\Server\ResponseTypes\RedirectResponse; use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface; use Psr\Http\Message\ServerRequestInterface; @@ -26,6 +27,9 @@ use SimpleSAML\OpenID\Codebooks\GrantTypesEnum; use SimpleSAML\OpenID\Codebooks\ParamsEnum; +/** + * @psalm-suppress PropertyNotSetInConstructor + */ class PreAuthCodeGrant extends AuthCodeGrant { public function getIdentifier(): string @@ -59,7 +63,7 @@ public function isOidcCandidate( * @throws \JsonException */ public function completeAuthorizationRequest( - OAuth2AuthorizationRequest $authorizationRequest, + OAuth2AuthorizationRequestInterface $authorizationRequest, ): ResponseTypeInterface { throw OidcServerException::serverError('Not implemented'); } @@ -263,7 +267,7 @@ protected function validateAuthorizationCode( public function validateAuthorizationRequestWithRequestRules( ServerRequestInterface $request, ResultBagInterface $resultBag, - ): OAuth2AuthorizationRequest { + ): OAuth2AuthorizationRequestInterface { throw OidcServerException::serverError('Not implemented'); } diff --git a/src/Server/Grants/RefreshTokenGrant.php b/src/Server/Grants/RefreshTokenGrant.php index 31492d74..813b7b87 100644 --- a/src/Server/Grants/RefreshTokenGrant.php +++ b/src/Server/Grants/RefreshTokenGrant.php @@ -24,34 +24,13 @@ use function json_decode; use function time; +/** + * @psalm-suppress PropertyNotSetInConstructor + */ class RefreshTokenGrant extends OAuth2RefreshTokenGrant { use IssueAccessTokenTrait; - /** @psalm-suppress PropertyNotSetInConstructor */ - protected $revokeRefreshTokens; - - /** @psalm-suppress PropertyNotSetInConstructor */ - protected $defaultScope; - - /** @psalm-suppress PropertyNotSetInConstructor */ - protected $privateKey; - - /** @psalm-suppress PropertyNotSetInConstructor */ - protected $userRepository; - - /** @psalm-suppress PropertyNotSetInConstructor */ - protected $authCodeRepository; - - /** @psalm-suppress PropertyNotSetInConstructor */ - protected $scopeRepository; - - /** @psalm-suppress PropertyNotSetInConstructor */ - protected $accessTokenRepository; - - /** @psalm-suppress PropertyNotSetInConstructor */ - protected $clientRepository; - public function __construct( RefreshTokenRepositoryInterface $refreshTokenRepository, AccessTokenEntityFactory $accessTokenEntityFactory, @@ -92,7 +71,7 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity * @throws \JsonException * @throws \SimpleSAML\Module\oidc\Server\Exceptions\OidcServerException */ - protected function validateOldRefreshToken(ServerRequestInterface $request, $clientId): array + protected function validateOldRefreshToken(ServerRequestInterface $request, string $clientId): array { $encryptedRefreshToken = $this->getRequestParameter('refresh_token', $request); if (is_null($encryptedRefreshToken)) { @@ -111,6 +90,8 @@ protected function validateOldRefreshToken(ServerRequestInterface $request, $cli if (! is_array($refreshTokenData)) { throw OidcServerException::invalidRefreshToken('Refresh token has unexpected type'); } + + /** @var array $refreshTokenData */ if ($refreshTokenData['client_id'] !== $clientId) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::REFRESH_TOKEN_CLIENT_FAILED, $request)); throw OidcServerException::invalidRefreshToken('Refresh token is not linked to client'); @@ -151,7 +132,7 @@ protected function validateOldRefreshToken(ServerRequestInterface $request, $cli 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 6e8f47b6..3e7b3407 100644 --- a/src/Server/Grants/Traits/IssueAccessTokenTrait.php +++ b/src/Server/Grants/Traits/IssueAccessTokenTrait.php @@ -8,7 +8,6 @@ use DateTimeImmutable; use League\OAuth2\Server\Entities\ClientEntityInterface; use League\OAuth2\Server\Exception\UniqueTokenIdentifierConstraintViolationException; -use League\OAuth2\Server\Grant\AbstractGrant; use SimpleSAML\Module\oidc\Codebooks\FlowTypeEnum; use SimpleSAML\Module\oidc\Entities\Interfaces\AccessTokenEntityInterface; use SimpleSAML\Module\oidc\Factories\Entities\AccessTokenEntityFactory; @@ -23,16 +22,6 @@ */ trait IssueAccessTokenTrait { - /** - * @psalm-suppress MissingPropertyType - */ - protected $accessTokenRepository; - - /** - * @var \League\OAuth2\Server\CryptKey - */ - protected $privateKey; - protected AccessTokenEntityFactory $accessTokenEntityFactory; /** @@ -57,7 +46,7 @@ protected function issueAccessToken( ?string $boundRedirectUri = null, ?string $issuerState = null, ): AccessTokenEntityInterface { - $maxGenerationAttempts = AbstractGrant::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS; + $maxGenerationAttempts = self::MAX_RANDOM_TOKEN_GENERATION_ATTEMPTS; /** Since we are using our own repository interface, check for proper type. */ if (! is_a($this->accessTokenRepository, AccessTokenRepositoryInterface::class)) { @@ -102,5 +91,5 @@ protected function issueAccessToken( * * @return string */ - abstract protected function generateUniqueIdentifier($length = 40); + abstract protected function generateUniqueIdentifier(int $length = 40): string; } diff --git a/src/Server/RequestTypes/AuthorizationRequest.php b/src/Server/RequestTypes/AuthorizationRequest.php index bc969f10..4ad40bec 100644 --- a/src/Server/RequestTypes/AuthorizationRequest.php +++ b/src/Server/RequestTypes/AuthorizationRequest.php @@ -82,8 +82,15 @@ public static function fromOAuth2AuthorizationRequest( $authorizationRequest->setClient($oAuth2authorizationRequest->getClient()); $authorizationRequest->setRedirectUri($oAuth2authorizationRequest->getRedirectUri()); $authorizationRequest->setScopes($oAuth2authorizationRequest->getScopes()); - $authorizationRequest->setCodeChallenge($oAuth2authorizationRequest->getCodeChallenge()); - $authorizationRequest->setCodeChallengeMethod($oAuth2authorizationRequest->getCodeChallengeMethod()); + $codeChallenge = $oAuth2authorizationRequest->getCodeChallenge(); + if ($codeChallenge !== null) { + $authorizationRequest->setCodeChallenge($codeChallenge); + } + + $codeChallengeMethod = $oAuth2authorizationRequest->getCodeChallengeMethod(); + if ($codeChallengeMethod !== null) { + $authorizationRequest->setCodeChallengeMethod($codeChallengeMethod); + } $state = $oAuth2authorizationRequest->getState(); if (null !== $state) { diff --git a/src/Server/ResponseTypes/HtmlResponse.php b/src/Server/ResponseTypes/HtmlResponse.php index 36b7d4ea..52c3204e 100644 --- a/src/Server/ResponseTypes/HtmlResponse.php +++ b/src/Server/ResponseTypes/HtmlResponse.php @@ -11,20 +11,12 @@ class HtmlResponse extends AbstractResponseType { private string $html = ''; - /** - * @param string $html - */ - public function setHtml($html): void + public function setHtml(string $html): void { $this->html = $html; } - /** - * @param ResponseInterface $response - * - * @return ResponseInterface - */ - public function generateHttpResponse(ResponseInterface $response) + public function generateHttpResponse(ResponseInterface $response): ResponseInterface { $response->getBody()->write($this->html); diff --git a/src/Server/ResponseTypes/TokenResponse.php b/src/Server/ResponseTypes/TokenResponse.php index 19ecdace..000331ac 100644 --- a/src/Server/ResponseTypes/TokenResponse.php +++ b/src/Server/ResponseTypes/TokenResponse.php @@ -16,7 +16,7 @@ namespace SimpleSAML\Module\oidc\Server\ResponseTypes; -use League\OAuth2\Server\CryptKey; +use League\OAuth2\Server\CryptKeyInterface; use League\OAuth2\Server\Entities\AccessTokenEntityInterface; use League\OAuth2\Server\ResponseTypes\BearerTokenResponse; use RuntimeException; @@ -37,6 +37,8 @@ * @license http://opensource.org/licenses/MIT MIT * * @see https://github.com/steverhoades/oauth2-openid-connect-server/blob/master/src/IdTokenResponse.php + * + * @psalm-suppress PropertyNotSetInConstructor */ class TokenResponse extends BearerTokenResponse implements // phpcs:ignore @@ -56,22 +58,10 @@ class TokenResponse extends BearerTokenResponse implements protected ?string $sessionId = null; - /** - * @var \League\OAuth2\Server\Entities\AccessTokenEntityInterface - * @psalm-suppress PropertyNotSetInConstructor - */ - protected $accessToken; - - /** - * @var \League\OAuth2\Server\Entities\RefreshTokenEntityInterface - * @psalm-suppress PropertyNotSetInConstructor - */ - protected $refreshToken; - public function __construct( private readonly IdentityProviderInterface $identityProvider, protected IdTokenBuilder $idTokenBuilder, - CryptKey $privateKey, + CryptKeyInterface $privateKey, protected LoggerService $loggerService, ) { $this->privateKey = $privateKey; @@ -120,7 +110,7 @@ protected function prepareIdTokenExtraParam(AccessTokenEntity $accessToken): arr throw OidcServerException::accessDenied('No user identifier present in AccessToken.'); } - $userEntity = $this->identityProvider->getUserEntityByIdentifier((string)$userIdentifier); + $userEntity = $this->identityProvider->getUserEntityByIdentifier($userIdentifier); if (empty($userEntity)) { throw OidcServerException::accessDenied('No user available for provided user identifier.'); diff --git a/src/Services/AuthenticationService.php b/src/Services/AuthenticationService.php index b4061577..dcad70d0 100644 --- a/src/Services/AuthenticationService.php +++ b/src/Services/AuthenticationService.php @@ -17,7 +17,7 @@ namespace SimpleSAML\Module\oidc\Services; use League\OAuth2\Server\Entities\ClientEntityInterface as OAuth2ClientEntityInterface; -use League\OAuth2\Server\RequestTypes\AuthorizationRequest as OAuth2AuthorizationRequest; +use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface as OAuth2AuthorizationRequestInterface; use Psr\Http\Message\ServerRequestInterface; use SimpleSAML\Auth\ProcessingChain; use SimpleSAML\Auth\Simple; @@ -80,7 +80,7 @@ public function __construct( /** * @param ServerRequestInterface $request - * @param OAuth2AuthorizationRequest $authorizationRequest + * @param OAuth2AuthorizationRequestInterface $authorizationRequest * * @return array * @throws Error\AuthSource @@ -92,7 +92,7 @@ public function __construct( */ public function processRequest( ServerRequestInterface $request, - OAuth2AuthorizationRequest $authorizationRequest, + OAuth2AuthorizationRequestInterface $authorizationRequest, ): array { $oidcClient = $authorizationRequest->getClient(); $authSimple = $this->authSimpleFactory->build($oidcClient); @@ -179,11 +179,11 @@ public function getAuthenticateUser( /** * @param array|null $state * - * @return OAuth2AuthorizationRequest + * @return OAuth2AuthorizationRequestInterface * @throws Exception */ - public function getAuthorizationRequestFromState(array|null $state): OAuth2AuthorizationRequest + public function getAuthorizationRequestFromState(array|null $state): OAuth2AuthorizationRequestInterface { if (!isset($state['authorizationRequest'])) { throw new Exception('Authorization Request is not set.'); @@ -191,7 +191,7 @@ public function getAuthorizationRequestFromState(array|null $state): OAuth2Autho if ($state['authorizationRequest'] instanceof AuthorizationRequest) { return $state['authorizationRequest']; - } elseif ($state['authorizationRequest'] instanceof OAuth2AuthorizationRequest) { + } elseif ($state['authorizationRequest'] instanceof OAuth2AuthorizationRequestInterface) { return $state['authorizationRequest']; } else { throw new Exception('Authorization Request is not valid.'); @@ -202,7 +202,7 @@ public function getAuthorizationRequestFromState(array|null $state): OAuth2Autho * @param Simple $authSimple * @param OAuth2ClientEntityInterface $client * @param ServerRequestInterface $request - * @param OAuth2AuthorizationRequest $authorizationRequest + * @param OAuth2AuthorizationRequestInterface $authorizationRequest * * @return array * @throws Error\AuthSource @@ -212,7 +212,7 @@ public function prepareStateArray( Simple $authSimple, OAuth2ClientEntityInterface $client, ServerRequestInterface $request, - OAuth2AuthorizationRequest $authorizationRequest, + OAuth2AuthorizationRequestInterface $authorizationRequest, ): array { $state = $authSimple->getAuthDataArray(); diff --git a/src/Services/IdTokenBuilder.php b/src/Services/IdTokenBuilder.php index 87adba25..2eb7ec8b 100644 --- a/src/Services/IdTokenBuilder.php +++ b/src/Services/IdTokenBuilder.php @@ -147,9 +147,7 @@ public function generateAccessTokenHash(AccessTokenEntityInterface $accessToken, EntityStringRepresentationInterface::class); } - // Try to use toString() so that it uses the string representation if - // it was already cast to string, otherwise, use the cast version. - $accessTokenString = $accessToken->toString() ?? (string) $accessToken; + $accessTokenString = $accessToken->toString(); return Base64Url::encode( substr( diff --git a/tests/unit/src/Entities/AccessTokenEntityTest.php b/tests/unit/src/Entities/AccessTokenEntityTest.php index d7e37417..76e0c284 100644 --- a/tests/unit/src/Entities/AccessTokenEntityTest.php +++ b/tests/unit/src/Entities/AccessTokenEntityTest.php @@ -129,12 +129,12 @@ public function testHasProperState(): void public function testHasImmutableStringRepresentation(): void { $instance = $this->mock(); - $this->assertNull($instance->toString()); - $stringRepresentation = (string) $instance; + $stringRepresentation = $instance->toString(); $this->assertIsString($instance->toString()); + $this->assertSame($stringRepresentation, (string) $instance); $this->assertSame($stringRepresentation, $instance->toString()); } } diff --git a/tests/unit/src/Server/RequestRules/Rules/ClientRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/ClientRuleTest.php index b356e43d..91a2d178 100644 --- a/tests/unit/src/Server/RequestRules/Rules/ClientRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/ClientRuleTest.php @@ -102,7 +102,7 @@ public function testCheckRuleEmptyClientIdThrows(): void public function testCheckRuleInvalidClientThrows(): void { $this->requestParamsResolverStub->method('getBasedOnAllowedMethods')->willReturn('123'); - $this->clientRepositoryStub->method('getClientEntity')->willReturn('invalid'); + $this->clientRepositoryStub->method('getClientEntity')->willReturn(null); $this->expectException(OidcServerException::class); $this->sut()->checkRule( $this->requestStub, diff --git a/tests/unit/src/Server/RequestRules/Rules/ScopeRuleTest.php b/tests/unit/src/Server/RequestRules/Rules/ScopeRuleTest.php index 0cdc985a..c5b8e6eb 100644 --- a/tests/unit/src/Server/RequestRules/Rules/ScopeRuleTest.php +++ b/tests/unit/src/Server/RequestRules/Rules/ScopeRuleTest.php @@ -141,11 +141,9 @@ public function testValidScopes(): void ->willReturn(['openid', 'profile']); $this->scopeRepositoryStub ->method('getScopeEntityByIdentifier') - ->willReturn( - $this->onConsecutiveCalls( - $this->scopeEntities['openid'], - $this->scopeEntities['profile'], - ), + ->willReturnOnConsecutiveCalls( + $this->scopeEntities['openid'], + $this->scopeEntities['profile'], ); $result = $this->sut()->checkRule( @@ -174,11 +172,7 @@ public function testInvalidScopeThrows(): void ->willReturn(['openid']); $this->scopeRepositoryStub ->method('getScopeEntityByIdentifier') - ->willReturn( - $this->onConsecutiveCalls( - 'invalid-scope-entity', - ), - ); + ->willReturn(null); $this->expectException(OidcServerException::class); $this->sut()->checkRule(