From 73f9ab74a6f724db8f8e808cb294f74a8fdf3217 Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sat, 2 Nov 2024 17:00:32 +0100 Subject: [PATCH] refactor: moved EntraID related code into own class (#3202) --- docs/administration.md | 2 +- phpmyfaq/assets/templates/admin/login.twig | 2 +- phpmyfaq/assets/templates/default/login.twig | 2 +- phpmyfaq/index.php | 2 +- phpmyfaq/services/azure/callback.php | 21 ++- phpmyfaq/services/azure/index.php | 8 +- phpmyfaq/services/azure/logout.php | 8 +- phpmyfaq/src/Bootstrap.php | 1 + phpmyfaq/src/phpMyFAQ/Auth/AuthEntraId.php | 21 ++- .../Auth/{Azure => EntraId}/OAuth.php | 20 +-- .../src/phpMyFAQ/Auth/EntraId/Session.php | 120 ++++++++++++++++++ .../src/phpMyFAQ/Session/AbstractSession.php | 38 ++++++ phpmyfaq/src/phpMyFAQ/User/UserSession.php | 89 ------------- .../Auth/{Azure => EntraId}/OAuthTest.php | 16 +-- 14 files changed, 215 insertions(+), 135 deletions(-) rename phpmyfaq/src/phpMyFAQ/Auth/{Azure => EntraId}/OAuth.php (87%) create mode 100644 phpmyfaq/src/phpMyFAQ/Auth/EntraId/Session.php create mode 100644 phpmyfaq/src/phpMyFAQ/Session/AbstractSession.php rename tests/phpMyFAQ/Auth/{Azure => EntraId}/OAuthTest.php (89%) diff --git a/docs/administration.md b/docs/administration.md index 7fc9b481ae..52f5deca7a 100644 --- a/docs/administration.md +++ b/docs/administration.md @@ -485,7 +485,7 @@ Follow these steps to create an App Registration in Microsoft Azure: 1. In the "Name" field, provide a name for your App Registration, e.g. "phpMyFAQ". 2. Choose the supported account types that your application will authenticate: "Accounts in this organizational directory only" -3. In the "Redirect URI" section, specify the redirect URI where Entra ID will send authentication responses: `http://www.example.com/faq/services/azure/callback.php` +3. In the "Redirect URI" section, specify the redirect URI where Entra ID will send authentication responses: `http://www.example.com/faq/services/entra-id/callback.php` 4. Click the "Register" button to create the App Registration. **Step 5: Configure Authentication** diff --git a/phpmyfaq/assets/templates/admin/login.twig b/phpmyfaq/assets/templates/admin/login.twig index 0b9829e309..411754c4c2 100644 --- a/phpmyfaq/assets/templates/admin/login.twig +++ b/phpmyfaq/assets/templates/admin/login.twig @@ -65,7 +65,7 @@ {% endif %} {% if hasSignInWithMicrosoftActive %} - + {{ msgSignInWithMicrosoft }} diff --git a/phpmyfaq/assets/templates/default/login.twig b/phpmyfaq/assets/templates/default/login.twig index dddb3fc874..3f5f0eb104 100644 --- a/phpmyfaq/assets/templates/default/login.twig +++ b/phpmyfaq/assets/templates/default/login.twig @@ -68,7 +68,7 @@ {% endif %} {% if useSignInWithMicrosoft %} - + {{ 'msgSignInWithMicrosoft' | translate }} diff --git a/phpmyfaq/index.php b/phpmyfaq/index.php index f539d75957..e5158b8803 100755 --- a/phpmyfaq/index.php +++ b/phpmyfaq/index.php @@ -232,7 +232,7 @@ } if ($faqConfig->isSignInWithMicrosoftActive() && $user->getUserAuthSource() === 'azure') { - $redirect = new RedirectResponse($faqConfig->getDefaultUrl() . 'services/azure/logout.php'); + $redirect = new RedirectResponse($faqConfig->getDefaultUrl() . 'services/entra-id/logout.php'); $redirect->send(); } diff --git a/phpmyfaq/services/azure/callback.php b/phpmyfaq/services/azure/callback.php index b5105903ba..ef631e4235 100644 --- a/phpmyfaq/services/azure/callback.php +++ b/phpmyfaq/services/azure/callback.php @@ -16,14 +16,20 @@ */ use phpMyFAQ\Auth\AuthEntraId; -use phpMyFAQ\Auth\Azure\OAuth; +use phpMyFAQ\Auth\EntraId\OAuth; +use phpMyFAQ\Auth\EntraId\Session as EntraIdSession; use phpMyFAQ\Configuration; use phpMyFAQ\Enums\AuthenticationSourceType; use phpMyFAQ\Filter; use phpMyFAQ\User\CurrentUser; -use phpMyFAQ\User\UserSession; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface; +use Symfony\Component\HttpFoundation\Session\Session; +use Symfony\Component\HttpFoundation\Session\Storage\PhpBridgeSessionStorage; + +ini_set('display_errors', '1'); +ini_set('display_startup_errors', '1'); +error_reporting(-1); if (session_status() !== PHP_SESSION_ACTIVE) { session_start(); @@ -48,13 +54,16 @@ $code = Filter::filterInput(INPUT_GET, 'code', FILTER_SANITIZE_SPECIAL_CHARS); $error = Filter::filterInput(INPUT_GET, 'error_description', FILTER_SANITIZE_SPECIAL_CHARS); -$session = new UserSession($faqConfig); -$oAuth = new OAuth($faqConfig, $session); +$session = new Session(new PhpBridgeSessionStorage()); +$session->start(); + +$entraIdSession = new EntraIdSession($faqConfig, $session); +$oAuth = new OAuth($faqConfig, $entraIdSession); $auth = new AuthEntraId($faqConfig, $oAuth); $redirect = new RedirectResponse($faqConfig->getDefaultUrl()); -if ($session->getCurrentSessionKey()) { +if ($entraIdSession->getCurrentSessionKey()) { try { $token = $oAuth->getOAuthToken($code); $oAuth->setToken($token)->setAccessToken($token->access_token)->setRefreshToken($token->refresh_token); @@ -81,7 +90,7 @@ $user->setTokenData([ 'refresh_token' => $oAuth->getRefreshToken(), 'access_token' => $oAuth->getAccessToken(), - 'code_verifier' => $session->get(UserSession::ENTRA_ID_OAUTH_VERIFIER), + 'code_verifier' => $entraIdSession->get(EntraIdSession::ENTRA_ID_OAUTH_VERIFIER), 'jwt' => $oAuth->getToken() ]); $user->setSuccess(true); diff --git a/phpmyfaq/services/azure/index.php b/phpmyfaq/services/azure/index.php index d766a6eaf7..554c84c000 100644 --- a/phpmyfaq/services/azure/index.php +++ b/phpmyfaq/services/azure/index.php @@ -16,9 +16,9 @@ */ use phpMyFAQ\Auth\AuthEntraId; -use phpMyFAQ\Auth\Azure\OAuth; +use phpMyFAQ\Auth\EntraId\OAuth; +use phpMyFAQ\Auth\EntraId\Session as EntraIdSession; use phpMyFAQ\Configuration; -use phpMyFAQ\User\UserSession; // // Prepend and start the PHP session @@ -34,8 +34,8 @@ $faqConfig = Configuration::getConfigurationInstance(); -$session = new UserSession($faqConfig); -$oAuth = new OAuth($faqConfig, $session); +$enraIdSession = new EntraIdSession($faqConfig, $session); +$oAuth = new OAuth($faqConfig, $enraIdSession); $auth = new AuthEntraId($faqConfig, $oAuth); try { diff --git a/phpmyfaq/services/azure/logout.php b/phpmyfaq/services/azure/logout.php index 69dd0c531a..8fd95d56c7 100644 --- a/phpmyfaq/services/azure/logout.php +++ b/phpmyfaq/services/azure/logout.php @@ -16,9 +16,9 @@ */ use phpMyFAQ\Auth\AuthEntraId; -use phpMyFAQ\Auth\Azure\OAuth; +use phpMyFAQ\Auth\EntraId\OAuth; +use phpMyFAQ\Auth\EntraId\Session as EntraIdSession; use phpMyFAQ\Configuration; -use phpMyFAQ\User\UserSession; // // Prepend and start the PHP session @@ -34,8 +34,8 @@ $faqConfig = Configuration::getConfigurationInstance(); -$session = new UserSession($faqConfig); -$oAuth = new OAuth($faqConfig, $session); +$enraIdSession = new EntraIdSession($faqConfig, $session); +$oAuth = new OAuth($faqConfig, $enraIdSession); $auth = new AuthEntraId($faqConfig, $oAuth); $auth->logout(); diff --git a/phpmyfaq/src/Bootstrap.php b/phpmyfaq/src/Bootstrap.php index 504e2b2466..eed5947f76 100644 --- a/phpmyfaq/src/Bootstrap.php +++ b/phpmyfaq/src/Bootstrap.php @@ -195,6 +195,7 @@ } else { $ldap = null; } + // // Connect to Elasticsearch if enabled // diff --git a/phpmyfaq/src/phpMyFAQ/Auth/AuthEntraId.php b/phpmyfaq/src/phpMyFAQ/Auth/AuthEntraId.php index 2543cf0959..bac969cd66 100644 --- a/phpmyfaq/src/phpMyFAQ/Auth/AuthEntraId.php +++ b/phpmyfaq/src/phpMyFAQ/Auth/AuthEntraId.php @@ -18,12 +18,12 @@ namespace phpMyFAQ\Auth; use phpMyFAQ\Auth; -use phpMyFAQ\Auth\Azure\OAuth; +use phpMyFAQ\Auth\EntraId\OAuth; +use phpMyFAQ\Auth\EntraId\Session; use phpMyFAQ\Configuration; use phpMyFAQ\Core\Exception; use phpMyFAQ\Enums\AuthenticationSourceType; use phpMyFAQ\User; -use phpMyFAQ\User\UserSession; use SensitiveParameter; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -34,8 +34,6 @@ */ class AuthEntraId extends Auth implements AuthDriverInterface { - private readonly UserSession $session; - private string $oAuthVerifier = ''; private string $oAuthChallenge; @@ -49,10 +47,11 @@ class AuthEntraId extends Auth implements AuthDriverInterface /** * @inheritDoc */ - public function __construct(Configuration $configuration, private readonly OAuth $oAuth) - { + public function __construct( + Configuration $configuration, + private readonly OAuth $oAuth + ) { $this->configuration = $configuration; - $this->session = new UserSession($configuration); parent::__construct($configuration); } @@ -128,16 +127,16 @@ public function isValidLogin(string $login, ?array $optionalData = []): int public function authorize(): void { $this->createOAuthChallenge(); - $this->session->setCurrentSessionKey(); - $this->session->set(UserSession::ENTRA_ID_OAUTH_VERIFIER, $this->oAuthVerifier); - $this->session->setCookie(UserSession::ENTRA_ID_OAUTH_VERIFIER, $this->oAuthVerifier, 7200, false); + $this->oAuth->getSession()->setCurrentSessionKey(); + $this->oAuth->getSession()->set(Session::ENTRA_ID_OAUTH_VERIFIER, $this->oAuthVerifier); + $this->oAuth->getSession()->setCookie(Session::ENTRA_ID_OAUTH_VERIFIER, $this->oAuthVerifier, 7200, false); $oAuthURL = sprintf( 'https://login.microsoftonline.com/%s/oauth2/v2.0/authorize' . '?response_type=code&client_id=%s&redirect_uri=%s&scope=%s&code_challenge=%s&code_challenge_method=%s', AAD_OAUTH_TENANTID, AAD_OAUTH_CLIENTID, - urlencode($this->configuration->getDefaultUrl() . 'services/azure/callback.php'), + urlencode($this->configuration->getDefaultUrl() . 'services/entra-id/callback.php'), AAD_OAUTH_SCOPE, $this->oAuthChallenge, self::ENTRAID_CHALLENGE_METHOD diff --git a/phpmyfaq/src/phpMyFAQ/Auth/Azure/OAuth.php b/phpmyfaq/src/phpMyFAQ/Auth/EntraId/OAuth.php similarity index 87% rename from phpmyfaq/src/phpMyFAQ/Auth/Azure/OAuth.php rename to phpmyfaq/src/phpMyFAQ/Auth/EntraId/OAuth.php index 57a94a6e98..f67b851a67 100644 --- a/phpmyfaq/src/phpMyFAQ/Auth/Azure/OAuth.php +++ b/phpmyfaq/src/phpMyFAQ/Auth/EntraId/OAuth.php @@ -15,10 +15,9 @@ * @since 2022-09-09 */ -namespace phpMyFAQ\Auth\Azure; +namespace phpMyFAQ\Auth\EntraId; use phpMyFAQ\Configuration; -use phpMyFAQ\User\UserSession; use stdClass; use Symfony\Component\HttpClient\HttpClient; use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface; @@ -43,7 +42,7 @@ class OAuth /** * Constructor. */ - public function __construct(private readonly Configuration $configuration, private readonly UserSession $session) + public function __construct(private readonly Configuration $configuration, private readonly Session $session) { $this->client = HttpClient::create(); } @@ -66,17 +65,17 @@ public function getOAuthToken(string $code): stdClass { $url = 'https://login.microsoftonline.com/' . AAD_OAUTH_TENANTID . '/oauth2/v2.0/token'; - if ($this->session->get(UserSession::ENTRA_ID_OAUTH_VERIFIER) !== '') { - $codeVerifier = $this->session->get(UserSession::ENTRA_ID_OAUTH_VERIFIER); + if ($this->session->get(Session::ENTRA_ID_OAUTH_VERIFIER) !== '') { + $codeVerifier = $this->session->get(Session::ENTRA_ID_OAUTH_VERIFIER); } else { - $codeVerifier = $this->session->getCookie(UserSession::ENTRA_ID_OAUTH_VERIFIER); + $codeVerifier = $this->session->getCookie(Session::ENTRA_ID_OAUTH_VERIFIER); } $response = $this->client->request('POST', $url, [ 'body' => [ 'grant_type' => 'authorization_code', 'client_id' => AAD_OAUTH_CLIENTID, - 'redirect_uri' => $this->configuration->getDefaultUrl() . 'services/azure/callback.php', + 'redirect_uri' => $this->configuration->getDefaultUrl() . 'services/entra-id/callback.php', 'code' => $code, 'code_verifier' => $codeVerifier, 'client_secret' => AAD_OAUTH_SECRET @@ -118,10 +117,15 @@ public function setToken(stdClass $token): OAuth { $idToken = base64_decode(explode('.', (string) $token->id_token)[1]); $this->token = json_decode($idToken, null, 512, JSON_THROW_ON_ERROR); - $this->session->set(UserSession::ENTRA_ID_JWT, json_encode($this->token, JSON_THROW_ON_ERROR)); + $this->session->set(Session::ENTRA_ID_JWT, json_encode($this->token, JSON_THROW_ON_ERROR)); return $this; } + public function getSession(): Session + { + return $this->session; + } + public function getRefreshToken(): ?string { return $this->refreshToken; diff --git a/phpmyfaq/src/phpMyFAQ/Auth/EntraId/Session.php b/phpmyfaq/src/phpMyFAQ/Auth/EntraId/Session.php new file mode 100644 index 0000000000..078c43f825 --- /dev/null +++ b/phpmyfaq/src/phpMyFAQ/Auth/EntraId/Session.php @@ -0,0 +1,120 @@ +createCurrentSessionKey(); + } + + /** + * Creates the current UUID session key + */ + public function createCurrentSessionKey(): void + { + $this->currentSessionKey = $this->uuid(); + } + + /** + * Returns the current UUID session key + */ + public function getCurrentSessionKey(): ?string + { + return $this->currentSessionKey ?? $this->session->get(self::ENTRA_ID_SESSION_KEY); + } + + /** + * Sets the current UUID session key + * + * @throws Exception + */ + public function setCurrentSessionKey(): Session + { + if (!isset($this->currentSessionKey)) { + $this->createCurrentSessionKey(); + } + + $this->session->set(self::ENTRA_ID_SESSION_KEY, $this->currentSessionKey); + + return $this; + } + + /** + * Store the Session ID into a persistent cookie expiring + * 3600 seconds after the page request. + * + * @param string $name Cookie name + * @param int|string|null $sessionId Session ID + * @param int $timeout Cookie timeout + */ + public function setCookie(string $name, int|string|null $sessionId, int $timeout = 3600, bool $strict = true): void + { + $request = Request::createFromGlobals(); + + Cookie::create($name) + ->withValue($sessionId ?? '') + ->withExpires($request->server->get('REQUEST_TIME') + $timeout) + ->withPath(dirname($request->server->get('SCRIPT_NAME'))) + ->withDomain(parse_url($this->configuration->getDefaultUrl(), PHP_URL_HOST)) + ->withSameSite($strict ? 'strict' : '') + ->withSecure($request->isSecure()) + ->withHttpOnly(); + } + + /** + * Returns the value of a cookie. + * + * @param string $name Cookie name + */ + public function getCookie(string $name): string + { + $request = Request::createFromGlobals(); + return $request->cookies->get($name, ''); + } + + /** + * Returns a UUID Version 4 compatible universally unique identifier. + */ + public function uuid(): string + { + try { + return sprintf( + '%04x%04x-%04x-%04x-%04x-%04x%04x%04x', + random_int(0, 0xffff), + random_int(0, 0xffff), + random_int(0, 0xffff), + random_int(0, 0x0fff) | 0x4000, + random_int(0, 0x3fff) | 0x8000, + random_int(0, 0xffff), + random_int(0, 0xffff), + random_int(0, 0xffff) + ); + } catch (RandomException $e) { + $this->configuration->getLogger()->error('Cannot generate UUID: ' . $e->getMessage()); + return ''; + } + } +} diff --git a/phpmyfaq/src/phpMyFAQ/Session/AbstractSession.php b/phpmyfaq/src/phpMyFAQ/Session/AbstractSession.php new file mode 100644 index 0000000000..34cdef05df --- /dev/null +++ b/phpmyfaq/src/phpMyFAQ/Session/AbstractSession.php @@ -0,0 +1,38 @@ + + * @copyright 2023-2024 phpMyFAQ Team + * @license https://www.mozilla.org/MPL/2.0/ Mozilla Public License Version 2.0 + * @link https://www.phpmyfaq.de + * @since 2023-02-19 + */ + +namespace phpMyFAQ\Session; + +use phpMyFAQ\Configuration; +use Symfony\Component\HttpFoundation\Session\Session; + +class AbstractSession +{ + public function __construct(private readonly Configuration $configuration, private readonly Session $session) + { + } + + public function get(string $key): mixed + { + return $this->session->get($key); + } + + public function set(string $key, mixed $value): void + { + $this->session->set($key, $value); + } +} diff --git a/phpmyfaq/src/phpMyFAQ/User/UserSession.php b/phpmyfaq/src/phpMyFAQ/User/UserSession.php index f4d75522b8..3dd7aefd0d 100644 --- a/phpmyfaq/src/phpMyFAQ/User/UserSession.php +++ b/phpmyfaq/src/phpMyFAQ/User/UserSession.php @@ -17,7 +17,6 @@ namespace phpMyFAQ\User; -use Exception; use phpMyFAQ\Configuration; use phpMyFAQ\Database; use phpMyFAQ\Enums\SessionActionType; @@ -45,24 +44,12 @@ class UserSession /** @var string Name of the session GET parameter */ final public const KEY_NAME_SESSION_ID = 'sid'; - /** @var string EntraID session key */ - final public const ENTRA_ID_SESSION_KEY = 'pmf-entra-id-session-key'; - - /** @var string */ - final public const ENTRA_ID_OAUTH_VERIFIER = 'pmf-entra-id-oauth-verifier'; - - /** @var string */ - final public const ENTRA_ID_JWT = 'pmf-entra-id-jwt'; - private ?int $currentSessionId = null; - private string $currentSessionKey; - private ?CurrentUser $currentUser = null; public function __construct(private readonly Configuration $configuration) { - $this->createCurrentSessionKey(); } /** @@ -91,48 +78,6 @@ public function setCurrentUser(CurrentUser $currentUser): UserSession return $this; } - /** - * Returns the current UUID session key - */ - public function getCurrentSessionKey(): ?string - { - return $this->currentSessionKey ?? $this->get(self::ENTRA_ID_SESSION_KEY); - } - - /** - * Sets the current UUID session key - * - * @throws Exception - */ - public function setCurrentSessionKey(): UserSession - { - if (!isset($this->currentSessionKey)) { - $this->createCurrentSessionKey(); - } - - $this->set(self::ENTRA_ID_SESSION_KEY, $this->currentSessionKey); - - return $this; - } - - /** - * Creates the current UUID session key - */ - public function createCurrentSessionKey(): void - { - $this->currentSessionKey = $this->uuid(); - } - - public function set(string $key, string $value): void - { - $_SESSION[$key] = $value; - } - - public function get(string $key): string - { - return $_SESSION[$key] ?? ''; - } - /** * Checks the Session ID. * @@ -299,38 +244,4 @@ public function setCookie(string $name, int|string|null $sessionId, int $timeout ->withSecure($request->isSecure()) ->withHttpOnly(); } - - /** - * Returns the value of a cookie. - * - * @param string $name Cookie name - */ - public function getCookie(string $name): string - { - $request = Request::createFromGlobals(); - return $request->cookies->get($name, ''); - } - - /** - * Returns a UUID Version 4 compatible universally unique identifier. - */ - public function uuid(): string - { - try { - return sprintf( - '%04x%04x-%04x-%04x-%04x-%04x%04x%04x', - random_int(0, 0xffff), - random_int(0, 0xffff), - random_int(0, 0xffff), - random_int(0, 0x0fff) | 0x4000, - random_int(0, 0x3fff) | 0x8000, - random_int(0, 0xffff), - random_int(0, 0xffff), - random_int(0, 0xffff) - ); - } catch (RandomException $e) { - $this->configuration->getLogger()->error('Cannot generate UUID: ' . $e->getMessage()); - return ''; - } - } } diff --git a/tests/phpMyFAQ/Auth/Azure/OAuthTest.php b/tests/phpMyFAQ/Auth/EntraId/OAuthTest.php similarity index 89% rename from tests/phpMyFAQ/Auth/Azure/OAuthTest.php rename to tests/phpMyFAQ/Auth/EntraId/OAuthTest.php index 0feae8caa8..8deb280409 100644 --- a/tests/phpMyFAQ/Auth/Azure/OAuthTest.php +++ b/tests/phpMyFAQ/Auth/EntraId/OAuthTest.php @@ -1,9 +1,8 @@ mockClient = $this->createMock(HttpClientInterface::class); - $this->mockConfiguration = $this->createMock(Configuration::class); - $this->mockSession = $this->createMock(UserSession::class); + $this->mockSession = $this->createMock(Session::class); + $mockConfiguration = $this->createMock(Configuration::class); - $this->oAuth = new OAuth($this->mockConfiguration, $this->mockSession); + $this->oAuth = new OAuth($mockConfiguration, $this->mockSession); } /** @@ -50,7 +48,7 @@ public function testGetOAuthTokenSuccess(): void $this->mockSession->expects($this->exactly(1)) ->method('get') - ->with(UserSession::ENTRA_ID_OAUTH_VERIFIER) + ->with(Session::ENTRA_ID_OAUTH_VERIFIER) ->willReturnOnConsecutiveCalls('', 'code_verifier'); $this->mockClient->expects($this->once()) @@ -115,7 +113,7 @@ public function testSetToken(): void $this->mockSession->expects($this->once()) ->method('set') - ->with(UserSession::ENTRA_ID_JWT, $this->stringContains('John Doe')); + ->with(Session::ENTRA_ID_JWT, $this->stringContains('John Doe')); $this->oAuth->setToken($token);