From d941daae42ed510d6d002cef45c0c2ad13194178 Mon Sep 17 00:00:00 2001 From: Bernd Bestel Date: Sun, 16 Aug 2026 17:28:56 +0200 Subject: [PATCH] Optimized authentication middleware handling --- changelog/82_UNRELEASED_xxxx-xx-xx.md | 6 +- config-dist.php | 11 +-- .../{ => Auth}/ApiKeyAuthMiddleware.php | 27 +++----- .../BaseAuthMiddleware.php} | 37 +++++----- middleware/Auth/DefaultAuthMiddleware.php | 68 +++++++++++++++++++ middleware/{ => Auth}/LdapAuthMiddleware.php | 43 +++++------- .../{ => Auth}/ReverseProxyAuthMiddleware.php | 22 +++--- .../{ => Auth}/SessionAuthMiddleware.php | 12 ++-- middleware/DefaultAuthMiddleware.php | 62 ----------------- migrations/0023.sql | 2 +- routes.php | 2 +- services/ApiKeyService.php | 4 +- services/SessionService.php | 4 +- views/layout/default.blade.php | 4 +- 14 files changed, 149 insertions(+), 155 deletions(-) rename middleware/{ => Auth}/ApiKeyAuthMiddleware.php (71%) rename middleware/{AuthMiddleware.php => Auth/BaseAuthMiddleware.php} (67%) create mode 100644 middleware/Auth/DefaultAuthMiddleware.php rename middleware/{ => Auth}/LdapAuthMiddleware.php (69%) rename middleware/{ => Auth}/ReverseProxyAuthMiddleware.php (76%) rename middleware/{ => Auth}/SessionAuthMiddleware.php (58%) delete mode 100644 middleware/DefaultAuthMiddleware.php diff --git a/changelog/82_UNRELEASED_xxxx-xx-xx.md b/changelog/82_UNRELEASED_xxxx-xx-xx.md index 26237ebe..1b853a12 100644 --- a/changelog/82_UNRELEASED_xxxx-xx-xx.md +++ b/changelog/82_UNRELEASED_xxxx-xx-xx.md @@ -1,8 +1,4 @@ -> ⚠️ xxxBREAKING CHANGESxxx - -> ❗ xxxImportant upgrade informationXXX - -> 💡 xxxMinor upgrade informationXXX +> ⚠️ Authentication middleware was reorganized, review your `AUTH_CLASS` setting (see the default reference in `config-dist.php` as usual) ### New Feature: xxxx diff --git a/config-dist.php b/config-dist.php index f5b82e4a..348e3275 100644 --- a/config-dist.php +++ b/config-dist.php @@ -85,13 +85,16 @@ Setting('ENTRY_PAGE', 'stock'); // places where user context is needed will then use the default (first existing) user Setting('DISABLE_AUTH', false); -// Either "Grocy\Middleware\DefaultAuthMiddleware", "Grocy\Middleware\ReverseProxyAuthMiddleware" -// or any class that implements Grocy\Middleware\AuthMiddleware -Setting('AUTH_CLASS', 'Grocy\Middleware\DefaultAuthMiddleware'); +// A valid fully qualified class name of the authentication middlware to use: +// Grocy\Middleware\Auth\DefaultAuthMiddleware: The default which uses the users you create in Grocy +// Grocy\Middleware\Auth\ReverseProxyAuthMiddleware: When your reverse proxy handles authentication (see options below) +// Grocy\Middleware\Auth\LdapAuthMiddleware: When you want to use your existing LDAP server (see options below) +// or any other class that implements Grocy\Middleware\Auth\BaseAuthMiddleware +Setting('AUTH_CLASS', 'Grocy\Middleware\Auth\DefaultAuthMiddleware'); // Options when using ReverseProxyAuthMiddleware Setting('REVERSE_PROXY_AUTH_HEADER', 'REMOTE_USER'); // The name of the HTTP header which your reverse proxy uses to pass the username (on successful authentication) -Setting('REVERSE_PROXY_AUTH_USE_ENV', false); // Set to true if the username is passed as environment variable +Setting('REVERSE_PROXY_AUTH_USE_ENV', false); // Set to true if the username is passed as an environment variable // Options when using LdapAuthMiddleware Setting('LDAP_ADDRESS', ''); // Example value "ldap://vm-dc2019.local.berrnd.net" diff --git a/middleware/ApiKeyAuthMiddleware.php b/middleware/Auth/ApiKeyAuthMiddleware.php similarity index 71% rename from middleware/ApiKeyAuthMiddleware.php rename to middleware/Auth/ApiKeyAuthMiddleware.php index 6345b33d..e7e5abdb 100644 --- a/middleware/ApiKeyAuthMiddleware.php +++ b/middleware/Auth/ApiKeyAuthMiddleware.php @@ -1,14 +1,13 @@ ApiKeyHeaderName = $this->AppContainer->get('ApiKeyHeaderName'); } - protected $ApiKeyHeaderName; + protected readonly string $ApiKeyHeaderName; - public function authenticate(Request $request) + public function AuthenticateRequest(Request $request) { - $routeContext = RouteContext::fromRequest($request); - $route = $routeContext->getRoute(); - $routeName = $route->getName(); - - $validApiKey = true; + $validApiKey = false; $usedApiKey = null; - $apiKeyService = new ApiKeyService(); - // First check of the API key in the configured header - if (!$request->hasHeader($this->ApiKeyHeaderName) || !$apiKeyService->IsValidApiKey($request->getHeaderLine($this->ApiKeyHeaderName))) - { - $validApiKey = false; - } - else + // First check the key in the configured header + if ($request->hasHeader($this->ApiKeyHeaderName) && $apiKeyService->IsValidApiKey($request->getHeaderLine($this->ApiKeyHeaderName))) { + $validApiKey = true; $usedApiKey = $request->getHeaderLine($this->ApiKeyHeaderName); } @@ -49,7 +40,7 @@ class ApiKeyAuthMiddleware extends AuthMiddleware // Handling of special purpose API keys if (!$validApiKey) { - if ($routeName === 'calendar-ical') + if ($this->RouteName === 'calendar-ical') { if ($request->getQueryParam('secret') !== null && $apiKeyService->IsValidApiKey($request->getQueryParam('secret'), ApiKeyService::API_KEY_TYPE_SPECIAL_PURPOSE_CALENDAR_ICAL)) { diff --git a/middleware/AuthMiddleware.php b/middleware/Auth/BaseAuthMiddleware.php similarity index 67% rename from middleware/AuthMiddleware.php rename to middleware/Auth/BaseAuthMiddleware.php index fbc494ed..df88260e 100644 --- a/middleware/AuthMiddleware.php +++ b/middleware/Auth/BaseAuthMiddleware.php @@ -1,34 +1,38 @@ getRoute(); - $routeName = $route->getName(); - $isApiRoute = string_starts_with($request->getUri()->getPath(), '/api/'); + $this->RouteName = $route->getName(); + $this->IsApiRoute = string_starts_with($request->getUri()->getPath(), '/api/'); - if ($routeName === 'root') - { - return $handler->handle($request); - } - elseif ($routeName === 'login') + if ($this->RouteName === 'root' || $this->RouteName === 'login') { + // Root and Login routes are public/unauthenticated + define('GROCY_AUTHENTICATED', false); return $handler->handle($request); } if (GROCY_MODE === 'dev' || GROCY_MODE === 'demo' || GROCY_MODE === 'prerelease' || GROCY_IS_EMBEDDED_INSTALL || GROCY_DISABLE_AUTH) { + // These modes use default user context (without authentication) only + $sessionService = SessionService::GetInstance(); $user = $sessionService->GetDefaultUser(); @@ -40,15 +44,16 @@ abstract class AuthMiddleware extends BaseMiddleware } else { - $user = $this->authenticate($request); + // Normal authentication flow (up to specific middleware implementation) + + $user = $this->AuthenticateRequest($request); if ($user === null) { define('GROCY_AUTHENTICATED', false); - $response = $this->ResponseFactory->createResponse(); - if ($isApiRoute) + if ($this->IsApiRoute) { return $response->withStatus(401); } @@ -69,7 +74,7 @@ abstract class AuthMiddleware extends BaseMiddleware } } - protected static function SetSessionCookie($sessionKey) + protected static function SetSessionCookie(string $sessionKey) { // Cookie never expires, session validity is up to SessionService setcookie(SessionService::SESSION_COOKIE_NAME, $sessionKey, PHP_INT_SIZE == 4 ? PHP_INT_MAX : PHP_INT_MAX >> 32); @@ -78,14 +83,14 @@ abstract class AuthMiddleware extends BaseMiddleware /** * @param array $postParams * @return bool True/False if the provided credentials were valid - * @throws \Exception Throws an \Exception if an error happened during credentials processing or if this AuthMiddleware doesn't provide credentials processing (e. g. handles this externally) + * @throws \Exception Throws an \Exception if an error happened during credentials processing or if this authentication middlware doesn't provide credentials processing (e.g. handles this externally) */ abstract public static function ProcessLogin(array $postParams); /** * @param Request $request * @return mixed|null the user row or null if the request is not authenticated - * @throws \Exception Throws an \Exception if config is invalid. + * @throws \Exception Throws an \Exception if authentaction config is invalid */ - abstract protected function authenticate(Request $request); + abstract protected function AuthenticateRequest(Request $request); } diff --git a/middleware/Auth/DefaultAuthMiddleware.php b/middleware/Auth/DefaultAuthMiddleware.php new file mode 100644 index 00000000..a726a4d5 --- /dev/null +++ b/middleware/Auth/DefaultAuthMiddleware.php @@ -0,0 +1,68 @@ +IsApiRoute) + { + // Session cookie or API Key is ok + $auth = new ApiKeyAuthMiddleware($this->AppContainer, $this->ResponseFactory); + $user = $auth->AuthenticateRequest($request); + if ($user !== null) + { + return $user; + } + + $auth = new SessionAuthMiddleware($this->AppContainer, $this->ResponseFactory); + $user = $auth->AuthenticateRequest($request); + return $user; + } + else + { + // Only session cookie is ok + $auth = new SessionAuthMiddleware($this->AppContainer, $this->ResponseFactory); + $user = $auth->AuthenticateRequest($request); + return $user; + } + } + + public static function ProcessLogin(array $postParams) + { + if (empty($postParams['username']) || empty($postParams['password'])) + { + return false; + } + + $db = DatabaseService::GetInstance()->GetDbConnection(); + + $user = $db->users()->where('username', $postParams['username'])->fetch(); + $inputPassword = $postParams['password']; + $stayLoggedInPermanently = $postParams['stay_logged_in'] == 'on'; + + if ($user !== null && password_verify($inputPassword, $user->password)) + { + $sessionKey = SessionService::GetInstance()->CreateSession($user->id, $stayLoggedInPermanently); + self::SetSessionCookie($sessionKey); + + if (password_needs_rehash($user->password, PASSWORD_ARGON2ID)) + { + $user->update([ + 'password' => password_hash($inputPassword, PASSWORD_ARGON2ID) + ]); + } + + return true; + } + else + { + return false; + } + } +} diff --git a/middleware/LdapAuthMiddleware.php b/middleware/Auth/LdapAuthMiddleware.php similarity index 69% rename from middleware/LdapAuthMiddleware.php rename to middleware/Auth/LdapAuthMiddleware.php index b99f951b..611fa240 100644 --- a/middleware/LdapAuthMiddleware.php +++ b/middleware/Auth/LdapAuthMiddleware.php @@ -1,43 +1,36 @@ authenticate somehow - - // First try to authenticate by API key - $auth = new ApiKeyAuthMiddleware($this->AppContainer, $this->ResponseFactory); - $user = $auth->authenticate($request); - if ($user !== null) - { - return $user; - } - - // Then by session cookie - $auth = new SessionAuthMiddleware($this->AppContainer, $this->ResponseFactory); - $user = $auth->authenticate($request); - return $user; + $auth = new DefaultAuthMiddleware($this->AppContainer, $this->ResponseFactory); + return $auth->AuthenticateRequest($request); } public static function ProcessLogin(array $postParams) { + if (empty($postParams['username']) || empty($postParams['password'])) + { + return false; + } + if ($connect = ldap_connect(GROCY_LDAP_ADDRESS)) { ldap_set_option($connect, LDAP_OPT_PROTOCOL_VERSION, 3); ldap_set_option($connect, LDAP_OPT_REFERRALS, 0); - // bind with service account to retrieve user DN - if ($bind = ldap_bind($connect, GROCY_LDAP_BIND_DN, GROCY_LDAP_BIND_PW)) + // Bind with service account to retrieve user DN + if (ldap_bind($connect, GROCY_LDAP_BIND_DN, GROCY_LDAP_BIND_PW)) { $filter = '(&(' . GROCY_LDAP_UID_ATTR . '=' . $postParams['username'] . ')' . GROCY_LDAP_USER_FILTER . ')'; @@ -61,20 +54,19 @@ class LdapAuthMiddleware extends AuthMiddleware if (is_null($ldapDistinguishedName)) { // User not found + ldap_close($connect); return false; } } else { // Bind authentication failed - return false; + throw new \Exception('LDAP error: ' . ldap_error($connect)); } - // bind with user account to validate password - if ($bind = ldap_bind($connect, $ldapDistinguishedName, $postParams['password'])) + // Bind with user account to validate password + if (ldap_bind($connect, $ldapDistinguishedName, $postParams['password'])) { - ldap_close($connect); - $db = DatabaseService::GetInstance()->GetDbConnection(); $user = $db->users()->where('username', $ldapUidAttribute)->fetch(); if ($user == null) @@ -89,9 +81,8 @@ class LdapAuthMiddleware extends AuthMiddleware } else { - ldap_close($connect); - // User authentication failed + ldap_close($connect); return false; } } diff --git a/middleware/ReverseProxyAuthMiddleware.php b/middleware/Auth/ReverseProxyAuthMiddleware.php similarity index 76% rename from middleware/ReverseProxyAuthMiddleware.php rename to middleware/Auth/ReverseProxyAuthMiddleware.php index b31e4f68..3a6426e1 100644 --- a/middleware/ReverseProxyAuthMiddleware.php +++ b/middleware/Auth/ReverseProxyAuthMiddleware.php @@ -1,25 +1,26 @@ GetDbConnection(); - - // API key authentication is also ok - $auth = new ApiKeyAuthMiddleware($this->AppContainer, $this->ResponseFactory); - $user = $auth->authenticate($request); - if ($user !== null) + // When the reverse proxy is configured to be bypassed for API routes, try to use regular ApiKeyAuth + if ($this->IsApiRoute) { - return $user; + $auth = new ApiKeyAuthMiddleware($this->AppContainer, $this->ResponseFactory); + $user = $auth->AuthenticateRequest($request); + if ($user !== null) + { + return $user; + } } if (GROCY_REVERSE_PROXY_AUTH_USE_ENV) @@ -48,6 +49,7 @@ class ReverseProxyAuthMiddleware extends AuthMiddleware $username = $username[0]; } + $db = DatabaseService::GetInstance()->GetDbConnection(); $user = $db->users()->where('username', $username)->fetch(); if ($user == null) { diff --git a/middleware/SessionAuthMiddleware.php b/middleware/Auth/SessionAuthMiddleware.php similarity index 58% rename from middleware/SessionAuthMiddleware.php rename to middleware/Auth/SessionAuthMiddleware.php index e04b0cf3..97ef7500 100644 --- a/middleware/SessionAuthMiddleware.php +++ b/middleware/Auth/SessionAuthMiddleware.php @@ -1,23 +1,23 @@ IsValidSession($_COOKIE[SessionService::SESSION_COOKIE_NAME])) + if (isset($_COOKIE[SessionService::SESSION_COOKIE_NAME]) && $sessionService->IsValidSession($_COOKIE[SessionService::SESSION_COOKIE_NAME])) { - return null; + return $sessionService->GetUserBySessionKey($_COOKIE[SessionService::SESSION_COOKIE_NAME]); } else { - return $sessionService->GetUserBySessionKey($_COOKIE[SessionService::SESSION_COOKIE_NAME]); + return null; } } diff --git a/middleware/DefaultAuthMiddleware.php b/middleware/DefaultAuthMiddleware.php deleted file mode 100644 index 9b8e0676..00000000 --- a/middleware/DefaultAuthMiddleware.php +++ /dev/null @@ -1,62 +0,0 @@ -AppContainer, $this->ResponseFactory); - $user = $auth->authenticate($request); - - if ($user !== null) - { - return $user; - } - - // Then by session cookie - $auth = new SessionAuthMiddleware($this->AppContainer, $this->ResponseFactory); - $user = $auth->authenticate($request); - return $user; - } - - public static function ProcessLogin(array $postParams) - { - if (isset($postParams['username']) && isset($postParams['password'])) - { - $db = DatabaseService::GetInstance()->GetDbConnection(); - - $user = $db->users()->where('username', $postParams['username'])->fetch(); - $inputPassword = $postParams['password']; - $stayLoggedInPermanently = $postParams['stay_logged_in'] == 'on'; - - if ($user !== null && password_verify($inputPassword, $user->password)) - { - $sessionKey = SessionService::GetInstance()->CreateSession($user->id, $stayLoggedInPermanently); - self::SetSessionCookie($sessionKey); - - if (password_needs_rehash($user->password, PASSWORD_ARGON2ID)) - { - $user->update([ - 'password' => password_hash($inputPassword, PASSWORD_ARGON2ID) - ]); - } - - return true; - } - else - { - return false; - } - } - else - { - return false; - } - } -} diff --git a/migrations/0023.sql b/migrations/0023.sql index 6479ee13..bc9dae68 100644 --- a/migrations/0023.sql +++ b/migrations/0023.sql @@ -1 +1 @@ -DELETE FROM sessions +DELETE FROM sessions; diff --git a/routes.php b/routes.php index 51dd5a03..376c6937 100644 --- a/routes.php +++ b/routes.php @@ -269,7 +269,7 @@ $app->group('/api', function (RouteCollectorProxy $group) // For CORS preflight OPTIONS requests -$app->options('/api/{routes:.+}', function (Request $request, Response $response): Response +$app->any('/api/{routes:.+}', function (Request $request, Response $response): Response { return $response; })->add(new CorsMiddleware($container, $app->getResponseFactory())); diff --git a/services/ApiKeyService.php b/services/ApiKeyService.php index c236ef6c..8514ea4d 100644 --- a/services/ApiKeyService.php +++ b/services/ApiKeyService.php @@ -9,7 +9,7 @@ class ApiKeyService extends BaseService public function CreateApiKey(string $keyType = self::API_KEY_TYPE_DEFAULT, ?string $description = null) { - $newApiKey = $this->GenerateApiKey(); + $newApiKey = $this->GenerateKey(); $apiKeyRow = $this->DB->api_keys()->createRow([ 'api_key' => $newApiKey, @@ -98,7 +98,7 @@ class ApiKeyService extends BaseService $this->DB->api_keys()->where('api_key', $apiKey)->delete(); } - private function GenerateApiKey() + private function GenerateKey() { return RandomString(50); } diff --git a/services/SessionService.php b/services/SessionService.php index cc885064..c40bc8f2 100644 --- a/services/SessionService.php +++ b/services/SessionService.php @@ -8,7 +8,7 @@ class SessionService extends BaseService public function CreateSession($userId, $stayLoggedInPermanently = false) { - $newSessionKey = $this->GenerateSessionKey(); + $newSessionKey = $this->GenerateKey(); $expires = date('Y-m-d H:i:s', time() + 2592000); // Default is that sessions expire in 30 days @@ -76,7 +76,7 @@ class SessionService extends BaseService $this->DB->sessions()->where('session_key', $sessionKey)->delete(); } - private function GenerateSessionKey() + private function GenerateKey() { return RandomString(50); } diff --git a/views/layout/default.blade.php b/views/layout/default.blade.php index 81c45b6c..a81edd14 100644 --- a/views/layout/default.blade.php +++ b/views/layout/default.blade.php @@ -119,12 +119,12 @@ }; @if (GROCY_AUTHENTICATED) - Grocy.UserSettings = {!! json_encode($userSettings) !!}; Grocy.UserId = {{ GROCY_USER_ID }}; + Grocy.UserSettings = {!! json_encode($userSettings) !!}; Grocy.UserPermissions = {!! json_encode($permissions) !!}; @else - Grocy.UserSettings = { }; Grocy.UserId = -1; + Grocy.UserSettings = { }; @endif