From e36d4e5b682cbd393bfbc0023a1f6cb4e0293c5a Mon Sep 17 00:00:00 2001 From: Ronald Marfoldi Date: Tue, 25 Feb 2025 12:09:41 +0100 Subject: [PATCH 1/7] Upgrade --- .github/workflows/php.yml | 6 +++--- Dockerfile | 2 +- composer.json | 4 ++-- src/Security/Authentication/ApiTokenAuthenticator.php | 1 - src/Util/StatelessTokenUtil.php | 2 +- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index 9014db8..dfc9bee 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -10,10 +10,10 @@ jobs: strategy: matrix: include: - - php-version: 8.2 - docker-image: 'anzusystems/php:3.3.0-php82-cli' - php-version: 8.3 - docker-image: 'anzusystems/php:3.3.0-php83-cli' + docker-image: 'anzusystems/php:4.0.0-php83-cli' + - php-version: 8.4 + docker-image: 'anzusystems/php:4.0.0-php84-cli' name: PHP ${{ matrix.php-version }} runs-on: ubuntu-latest diff --git a/Dockerfile b/Dockerfile index f08d6db..998f337 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM anzusystems/php:3.1.0-php83-cli +FROM anzusystems/php:4.0.0-php83-cli # ### Basic arguments and variables ARG DOCKER_USER_ID diff --git a/composer.json b/composer.json index 59d33a0..a69d45f 100644 --- a/composer.json +++ b/composer.json @@ -11,7 +11,7 @@ } ], "require": { - "php": ">=8.2", + "php": ">=8.3", "ext-json": "*", "ext-redis": "*", "anzusystems/common-bundle": "^7.0|^8.0", @@ -24,7 +24,7 @@ "slevomat/coding-standard": "^8.5", "symfony/test-pack": "^1.0", "symplify/easy-coding-standard": "^11.1", - "vimeo/psalm": "^5.0" + "vimeo/psalm": "^6.0" }, "autoload": { "psr-4": { diff --git a/src/Security/Authentication/ApiTokenAuthenticator.php b/src/Security/Authentication/ApiTokenAuthenticator.php index 291f3ad..7e80801 100644 --- a/src/Security/Authentication/ApiTokenAuthenticator.php +++ b/src/Security/Authentication/ApiTokenAuthenticator.php @@ -14,7 +14,6 @@ use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\CustomCredentials; use Symfony\Component\Security\Http\Authenticator\Passport\Passport; -use Symfony\Component\String\AbstractString; use function Symfony\Component\String\u; /** diff --git a/src/Util/StatelessTokenUtil.php b/src/Util/StatelessTokenUtil.php index 4d77cb2..1fc9464 100644 --- a/src/Util/StatelessTokenUtil.php +++ b/src/Util/StatelessTokenUtil.php @@ -35,7 +35,7 @@ public function isValidForRequest(Request $request, string $hash): bool if ($this->enabled) { return hash_equals( known_string: $this->createHashForRequest($request), - user_string: base64_decode(urldecode($hash), strict: true), + user_string: (string) base64_decode(urldecode($hash), strict: true), ); } From 7c414d01eb955599664813d804f4a22ef121d0fe Mon Sep 17 00:00:00 2001 From: Ronald Marfoldi Date: Tue, 25 Feb 2025 13:30:31 +0100 Subject: [PATCH 2/7] Add timestamp to login redirect --- src/Controller/Api/AbstractAuthController.php | 2 +- src/Controller/Api/JsonCredentialsAuthController.php | 2 +- src/Controller/Api/OAuth2AuthController.php | 4 +++- .../Process/OAuth2/GrantAccessByOAuth2TokenProcess.php | 8 +++++++- src/Util/HttpUtil.php | 4 ++++ 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/Controller/Api/AbstractAuthController.php b/src/Controller/Api/AbstractAuthController.php index 8bbf6e8..e759a18 100644 --- a/src/Controller/Api/AbstractAuthController.php +++ b/src/Controller/Api/AbstractAuthController.php @@ -12,7 +12,7 @@ use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Routing\Annotation\Route; +use Symfony\Component\Routing\Attribute\Route; use Symfony\Component\Security\Http\Event\LogoutEvent; #[OA\Tag('Authorization')] diff --git a/src/Controller/Api/JsonCredentialsAuthController.php b/src/Controller/Api/JsonCredentialsAuthController.php index 2e7d8be..6f3fc4b 100644 --- a/src/Controller/Api/JsonCredentialsAuthController.php +++ b/src/Controller/Api/JsonCredentialsAuthController.php @@ -8,7 +8,7 @@ use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Routing\Annotation\Route; +use Symfony\Component\Routing\Attribute\Route; #[Route(name: 'auth_')] #[OA\Tag('Authorization')] diff --git a/src/Controller/Api/OAuth2AuthController.php b/src/Controller/Api/OAuth2AuthController.php index d388fcb..37691ef 100644 --- a/src/Controller/Api/OAuth2AuthController.php +++ b/src/Controller/Api/OAuth2AuthController.php @@ -8,13 +8,14 @@ use AnzuSystems\AuthBundle\Domain\Process\OAuth2\GrantAccessByOAuth2TokenProcess; use AnzuSystems\AuthBundle\Domain\Process\RefreshTokenProcess; use AnzuSystems\AuthBundle\Util\StatelessTokenUtil; +use AnzuSystems\Contracts\Exception\AnzuException; use AnzuSystems\SerializerBundle\Exception\SerializerException; use OpenApi\Attributes as OA; use Psr\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Routing\Annotation\Route; +use Symfony\Component\Routing\Attribute\Route; #[Route(name: 'auth_')] #[OA\Tag('Authorization')] @@ -42,6 +43,7 @@ public function login(Request $request): RedirectResponse /** * @throws SerializerException + * @throws AnzuException */ #[Route('authorize', name: 'authorize', methods: [Request::METHOD_GET])] public function callbackAuthorize(Request $request): Response diff --git a/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php b/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php index 3953895..5152a10 100644 --- a/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php +++ b/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php @@ -32,6 +32,8 @@ final class GrantAccessByOAuth2TokenProcess public const AUTH_METHOD_SSO_ID = 'sso_id'; public const AUTH_METHOD_SSO_EMAIL = 'sso_email'; + public const LOGIN_STATE_QUERY_PARAM = 'loginState'; + public const TIMESTAMP_QUERY_PARAM = 'timestamp'; public function __construct( private readonly OAuth2HttpClient $OAuth2HttpClient, @@ -114,7 +116,11 @@ private function logException(Request $request, Throwable $throwable): void private function createRedirectResponseForRequest(Request $request, UserOAuthLoginState $loginState): RedirectResponse { $redirectUrl = $this->httpUtil->getAuthRedirectUrlFromRequest($request); - $redirectUrl .= '?loginState=' . $loginState->toString(); + $redirectUrl .= '?'; + $redirectUrl .= http_build_query([ + self::LOGIN_STATE_QUERY_PARAM => $loginState->toString(), + self::TIMESTAMP_QUERY_PARAM => time(), + ]); return new RedirectResponse($redirectUrl); } diff --git a/src/Util/HttpUtil.php b/src/Util/HttpUtil.php index b7067a0..85916c3 100644 --- a/src/Util/HttpUtil.php +++ b/src/Util/HttpUtil.php @@ -23,6 +23,7 @@ final class HttpUtil { private const COOKIE_DEVICE_ID_TTL = 31_536_000; // 1 year + private const COOKIE_JWT_SUB_TTL = 60; public function __construct( private readonly CookieConfiguration $cookieConfiguration, @@ -101,6 +102,9 @@ public function storeJwtOnResponse(Response $response, Token $token, DateTimeImm } $lifetime = $expiresAt?->getTimestamp() ?? $this->jwtConfiguration->getLifetime(); + // We want to prevent a situation where a cookie is still stored and sent with a request, but when the server validates it, it is already invalid. + // To avoid this, we subtract a few seconds from the expiration time and allow the user to refresh the token earlier. + $lifetime -= self::COOKIE_JWT_SUB_TTL; $payloadCookie = $this->createCookie( $this->cookieConfiguration->getJwtPayloadCookieName(), $header . '.' . $claims, From ba0caf9d4405699ba5642157303f3637373a6eb7 Mon Sep 17 00:00:00 2001 From: Ronald Marfoldi Date: Mon, 3 Mar 2025 12:36:47 +0100 Subject: [PATCH 3/7] Fix public method --- .../Process/GrantAccessOnResponseProcess.php | 14 ++++---- .../GrantAccessByOAuth2TokenProcess.php | 32 +++++++++---------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/Domain/Process/GrantAccessOnResponseProcess.php b/src/Domain/Process/GrantAccessOnResponseProcess.php index 6ba9b08..d408d0f 100644 --- a/src/Domain/Process/GrantAccessOnResponseProcess.php +++ b/src/Domain/Process/GrantAccessOnResponseProcess.php @@ -7,6 +7,7 @@ use AnzuSystems\AuthBundle\Configuration\CookieConfiguration; use AnzuSystems\AuthBundle\Configuration\JwtConfiguration; use AnzuSystems\AuthBundle\Contracts\RefreshTokenStorageInterface; +use AnzuSystems\AuthBundle\Domain\Process\OAuth2\GrantAccessByOAuth2TokenProcess; use AnzuSystems\AuthBundle\Model\DeviceDto; use AnzuSystems\AuthBundle\Model\RefreshTokenDto; use AnzuSystems\AuthBundle\Util\HttpUtil; @@ -17,14 +18,14 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -final class GrantAccessOnResponseProcess +final readonly class GrantAccessOnResponseProcess { public function __construct( - private readonly CookieConfiguration $cookieConfiguration, - private readonly JwtConfiguration $jwtConfiguration, - private readonly JwtUtil $jwtUtil, - private readonly HttpUtil $httpUtil, - private readonly RefreshTokenStorageInterface $refreshTokenStorage, + private CookieConfiguration $cookieConfiguration, + private JwtConfiguration $jwtConfiguration, + private JwtUtil $jwtUtil, + private HttpUtil $httpUtil, + private RefreshTokenStorageInterface $refreshTokenStorage, ) { } @@ -52,6 +53,7 @@ public function execute(string $userId, Request $request, Response $response = n $response->setData([ 'access_token' => $jwt->toString(), 'refresh_token' => $refreshTokenDto->toString(), + GrantAccessByOAuth2TokenProcess::TIMESTAMP_QUERY_PARAM => time(), ]); } diff --git a/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php b/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php index 5152a10..ad7d7c1 100644 --- a/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php +++ b/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php @@ -30,10 +30,10 @@ final class GrantAccessByOAuth2TokenProcess { use SerializerAwareTrait; - public const AUTH_METHOD_SSO_ID = 'sso_id'; - public const AUTH_METHOD_SSO_EMAIL = 'sso_email'; - public const LOGIN_STATE_QUERY_PARAM = 'loginState'; - public const TIMESTAMP_QUERY_PARAM = 'timestamp'; + public const string AUTH_METHOD_SSO_ID = 'sso_id'; + public const string AUTH_METHOD_SSO_EMAIL = 'sso_email'; + public const string LOGIN_STATE_QUERY_PARAM = 'loginState'; + public const string TIMESTAMP_QUERY_PARAM = 'timestamp'; public function __construct( private readonly OAuth2HttpClient $OAuth2HttpClient, @@ -100,6 +100,18 @@ public function execute(Request $request): Response } } + public function createRedirectResponseForRequest(Request $request, UserOAuthLoginState $loginState): RedirectResponse + { + $redirectUrl = $this->httpUtil->getAuthRedirectUrlFromRequest($request); + $redirectUrl .= '?'; + $redirectUrl .= http_build_query([ + self::LOGIN_STATE_QUERY_PARAM => $loginState->toString(), + self::TIMESTAMP_QUERY_PARAM => time(), + ]); + + return new RedirectResponse($redirectUrl); + } + /** * @throws SerializerException */ @@ -113,18 +125,6 @@ private function logException(Request $request, Throwable $throwable): void $this->appLogger->error('[Authorization] ' . $throwable->getMessage(), $arrayContext); } - private function createRedirectResponseForRequest(Request $request, UserOAuthLoginState $loginState): RedirectResponse - { - $redirectUrl = $this->httpUtil->getAuthRedirectUrlFromRequest($request); - $redirectUrl .= '?'; - $redirectUrl .= http_build_query([ - self::LOGIN_STATE_QUERY_PARAM => $loginState->toString(), - self::TIMESTAMP_QUERY_PARAM => time(), - ]); - - return new RedirectResponse($redirectUrl); - } - /** * @throws AnzuException * @throws UnsuccessfulAccessTokenRequestException From f4f1766fdf41914e87ec4f1202b7244a6a842696 Mon Sep 17 00:00:00 2001 From: Ronald Marfoldi Date: Mon, 3 Mar 2025 16:23:06 +0100 Subject: [PATCH 4/7] upgrade --- .../Process/OAuth2/GrantAccessByOAuth2TokenProcess.php | 5 +++-- src/Util/HttpUtil.php | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php b/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php index ad7d7c1..eaf20aa 100644 --- a/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php +++ b/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php @@ -63,10 +63,11 @@ public function execute(Request $request): Response return $this->createRedirectResponseForRequest($request, UserOAuthLoginState::FailureSsoCommunicationFailed); } - if ($accessTokenDto->getJwt()) { + $jwt = $accessTokenDto->getJwt(); + if ($jwt) { // validate jwt try { - $this->validateOAuth2AccessTokenProcess->execute($accessTokenDto->getJwt()); + $this->validateOAuth2AccessTokenProcess->execute($jwt); } catch (InvalidJwtException $exception) { $this->logException($request, $exception); diff --git a/src/Util/HttpUtil.php b/src/Util/HttpUtil.php index 85916c3..112ae9a 100644 --- a/src/Util/HttpUtil.php +++ b/src/Util/HttpUtil.php @@ -22,8 +22,8 @@ final class HttpUtil { - private const COOKIE_DEVICE_ID_TTL = 31_536_000; // 1 year - private const COOKIE_JWT_SUB_TTL = 60; + private const int COOKIE_DEVICE_ID_TTL = 31_536_000; // 1 year + private const int COOKIE_JWT_SUB_TTL = 60; public function __construct( private readonly CookieConfiguration $cookieConfiguration, From 474bd583e6a8b1bbe38a2438598a7e2cf26a2ffc Mon Sep 17 00:00:00 2001 From: Ronald Marfoldi Date: Mon, 3 Mar 2025 17:00:19 +0100 Subject: [PATCH 5/7] upgrade --- .github/workflows/php.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index dfc9bee..6ab4c57 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -37,7 +37,7 @@ jobs: run: composer install --prefer-dist --no-progress --no-ansi --no-interaction --no-scripts - name: Run Security check - run: local-php-security-checker --path=composer.lock + run: composer audit --no-scripts - name: Run ECS style check run: vendor/bin/ecs check -vv From a4c4d3a0d9b161b9d96790c4ccd16b484e04f2d2 Mon Sep 17 00:00:00 2001 From: Ronald Marfoldi Date: Mon, 3 Mar 2025 17:03:56 +0100 Subject: [PATCH 6/7] upgrade --- psalm.xml | 15 +-------------- src/Model/SsoUserDto.php | 3 +++ 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/psalm.xml b/psalm.xml index 3f79157..73e8920 100644 --- a/psalm.xml +++ b/psalm.xml @@ -23,22 +23,9 @@ + - - - - - - - - - - - - - - diff --git a/src/Model/SsoUserDto.php b/src/Model/SsoUserDto.php index c575d0f..6ef04e3 100644 --- a/src/Model/SsoUserDto.php +++ b/src/Model/SsoUserDto.php @@ -6,6 +6,9 @@ use AnzuSystems\SerializerBundle\Attributes\Serialize; +/** + * @psalm-suppress ClassMustBeFinal + */ class SsoUserDto { #[Serialize] From e3c76f1c93b5ea7f1d9adeb8737fc392ce4654ce Mon Sep 17 00:00:00 2001 From: Ronald Marfoldi Date: Tue, 15 Apr 2025 15:17:57 +0200 Subject: [PATCH 7/7] Fix conflicting access token cache for fetching user info from SSO. --- src/Configuration/OAuth2Configuration.php | 2 +- .../GrantAccessByOAuth2TokenProcess.php | 4 +-- src/HttpClient/OAuth2HttpClient.php | 32 +++++++++++++++---- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/Configuration/OAuth2Configuration.php b/src/Configuration/OAuth2Configuration.php index 7fca5a4..a7fd766 100644 --- a/src/Configuration/OAuth2Configuration.php +++ b/src/Configuration/OAuth2Configuration.php @@ -41,7 +41,7 @@ public function getSsoAuthorizeUrl(): string return $this->ssoAuthorizeUrl; } - public function getSsoUserInfoUrl(?string $userId): string + public function getSsoUserInfoUrl(?string $userId = null): string { if (!$userId) { return $this->ssoUserInfoUrl; diff --git a/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php b/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php index eaf20aa..9349285 100644 --- a/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php +++ b/src/Domain/Process/OAuth2/GrantAccessByOAuth2TokenProcess.php @@ -135,7 +135,7 @@ private function getAuthUser(AccessTokenDto $accessTokenDto): ?AnzuAuthUserInter { if (self::AUTH_METHOD_SSO_EMAIL === $this->authMethod) { // fetch user info - $ssoUser = $this->OAuth2HttpClient->getSsoUserInfo(); + $ssoUser = $this->OAuth2HttpClient->getCurrentSsoUserInfo($accessTokenDto); return $this->oAuth2AuthUserRepository->findOneBySsoEmail($ssoUser->getEmail()); } @@ -149,7 +149,7 @@ private function getAuthUser(AccessTokenDto $accessTokenDto): ?AnzuAuthUserInter } // otherwise fetch user info - $ssoUser = $this->OAuth2HttpClient->getSsoUserInfo(); + $ssoUser = $this->OAuth2HttpClient->getCurrentSsoUserInfo($accessTokenDto); return $this->oAuth2AuthUserRepository->findOneBySsoUserId($ssoUser->getId()); } diff --git a/src/HttpClient/OAuth2HttpClient.php b/src/HttpClient/OAuth2HttpClient.php index f16e0b4..a1265f8 100644 --- a/src/HttpClient/OAuth2HttpClient.php +++ b/src/HttpClient/OAuth2HttpClient.php @@ -20,7 +20,7 @@ final class OAuth2HttpClient { - private const CLIENT_SERVICE_ACCESS_TOKEN_CACHE_KEY = 'sso_access_token_client_service'; + private const string CLIENT_SERVICE_ACCESS_TOKEN_CACHE_KEY = 'sso_access_token_client_service'; public function __construct( private readonly HttpClientInterface $client, @@ -34,24 +34,20 @@ public function __construct( */ public function requestAccessTokenByAuthCode(string $code): AccessTokenDto { - $accessToken = $this->sendTokenRequest($this->configuration->getSsoAccessTokenUrl(), [ + return $this->sendTokenRequest($this->configuration->getSsoAccessTokenUrl(), [ 'grant_type' => 'authorization_code', 'code' => $code, 'client_id' => $this->configuration->getSsoClientId(), 'client_secret' => $this->configuration->getSsoClientSecret(), 'redirect_uri' => $this->configuration->getSsoRedirectUrl(), ]); - - $this->storeAccessTokenToCache($this->getAccessTokenCacheItem(), $accessToken); - - return $accessToken; } /** * @throws UnsuccessfulAccessTokenRequestException * @throws UnsuccessfulUserInfoRequestException */ - public function getSsoUserInfo(?string $id = null): SsoUserDto + public function getSsoUserInfo(string $id): SsoUserDto { try { $response = $this->client->request( @@ -70,6 +66,28 @@ public function getSsoUserInfo(?string $id = null): SsoUserDto } } + /** + * @throws UnsuccessfulUserInfoRequestException + */ + public function getCurrentSsoUserInfo(AccessTokenDto $token): SsoUserDto + { + try { + $response = $this->client->request( + method: Request::METHOD_GET, + url: $this->configuration->getSsoUserInfoUrl(), + options: [ + 'auth_bearer' => $token->getAccessToken(), + ] + ); + + return $this->serializer->deserialize($response->getContent(), $this->configuration->getSsoUserInfoClass()); + } catch (ExceptionInterface $exception) { + throw UnsuccessfulUserInfoRequestException::create('User info request failed!', $exception); + } catch (SerializerException $exception) { + throw UnsuccessfulUserInfoRequestException::create('User info response deserialization failed!', $exception); + } + } + public function getSsoUserInfoByEmail(string $email): SsoUserDto { try {