Optimized authentication middleware handling

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
 

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"

middleware/Auth/ApiKeyAuthMiddleware.php

@@ -1,14 +1,13 @@
 <?php
 
-namespace Grocy\Middleware;
+namespace Grocy\Middleware\Auth;
 
 use DI\Container;
 use Grocy\Services\ApiKeyService;
 use Psr\Http\Message\ResponseFactoryInterface;
 use Psr\Http\Message\ServerRequestInterface as Request;
-use Slim\Routing\RouteContext;
 
-class ApiKeyAuthMiddleware extends AuthMiddleware
+class ApiKeyAuthMiddleware extends BaseAuthMiddleware
 {
 	public function __construct(Container $container, ResponseFactoryInterface $responseFactory)
 	{
@@ -16,26 +15,18 @@ class ApiKeyAuthMiddleware extends AuthMiddleware
 		$this->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))
 				{

middleware/Auth/BaseAuthMiddleware.php

@@ -1,34 +1,38 @@
 <?php
 
-namespace Grocy\Middleware;
+namespace Grocy\Middleware\Auth;
 
+use Grocy\Middleware\BaseMiddleware;
 use Grocy\Services\SessionService;
 use Psr\Http\Message\ResponseInterface as Response;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use Psr\Http\Server\RequestHandlerInterface as RequestHandler;
 use Slim\Routing\RouteContext;
 
-abstract class AuthMiddleware extends BaseMiddleware
+abstract class BaseAuthMiddleware extends BaseMiddleware
 {
+	protected ?string $RouteName = null;
+	protected bool $IsApiRoute = false;
+
 	public function __invoke(Request $request, RequestHandler $handler): Response
 	{
 		$routeContext = RouteContext::fromRequest($request);
 		$route = $routeContext->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);
 }

middleware/Auth/DefaultAuthMiddleware.php

@@ -0,0 +1,68 @@
+<?php
+
+namespace Grocy\Middleware\Auth;
+
+use Grocy\Services\DatabaseService;
+use Grocy\Services\SessionService;
+use Psr\Http\Message\ServerRequestInterface as Request;
+
+class DefaultAuthMiddleware extends BaseAuthMiddleware
+{
+	protected function AuthenticateRequest(Request $request)
+	{
+		if ($this->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;
+		}
+	}
+}

middleware/Auth/LdapAuthMiddleware.php

@@ -1,43 +1,36 @@
 <?php
 
-namespace Grocy\Middleware;
+namespace Grocy\Middleware\Auth;
 
 use Grocy\Services\DatabaseService;
 use Grocy\Services\SessionService;
 use Grocy\Services\UsersService;
 use Psr\Http\Message\ServerRequestInterface as Request;
 
-class LdapAuthMiddleware extends AuthMiddleware
+class LdapAuthMiddleware extends BaseAuthMiddleware
 {
-	public function authenticate(Request $request)
+	public function AuthenticateRequest(Request $request)
 	{
 		define('GROCY_EXTERNALLY_MANAGED_AUTHENTICATION', true);
 
-		// TODO: Reuse DefaultAuthMiddleware->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;
 			}
 		}

middleware/Auth/ReverseProxyAuthMiddleware.php

@@ -1,25 +1,26 @@
 <?php
 
-namespace Grocy\Middleware;
+namespace Grocy\Middleware\Auth;
 
 use Grocy\Services\DatabaseService;
 use Grocy\Services\UsersService;
 use Psr\Http\Message\ServerRequestInterface as Request;
 
-class ReverseProxyAuthMiddleware extends AuthMiddleware
+class ReverseProxyAuthMiddleware extends BaseAuthMiddleware
 {
-	public function authenticate(Request $request)
+	public function AuthenticateRequest(Request $request)
 	{
 		define('GROCY_EXTERNALLY_MANAGED_AUTHENTICATION', true);
 
-		$db = DatabaseService::GetInstance()->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)
 		{

middleware/Auth/SessionAuthMiddleware.php

@@ -1,23 +1,23 @@
 <?php
 
-namespace Grocy\Middleware;
+namespace Grocy\Middleware\Auth;
 
 use Grocy\Services\SessionService;
 use Psr\Http\Message\ServerRequestInterface as Request;
 
-class SessionAuthMiddleware extends AuthMiddleware
+class SessionAuthMiddleware extends BaseAuthMiddleware
 {
-	public function authenticate(Request $request)
+	public function AuthenticateRequest(Request $request)
 	{
 		$sessionService = SessionService::GetInstance();
 
-		if (!isset($_COOKIE[SessionService::SESSION_COOKIE_NAME]) || !$sessionService->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;
 		}
 	}
 

middleware/DefaultAuthMiddleware.php

@@ -1,62 +0,0 @@
-<?php
-
-namespace Grocy\Middleware;
-
-use Grocy\Services\DatabaseService;
-use Grocy\Services\SessionService;
-use Psr\Http\Message\ServerRequestInterface as Request;
-
-class DefaultAuthMiddleware extends AuthMiddleware
-{
-	protected function authenticate(Request $request)
-	{
-		// 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;
-	}
-
-	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;
-		}
-	}
-}

migrations/0023.sql

@@ -1 +1 @@
-DELETE FROM sessions
+DELETE FROM sessions;

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()));

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);
 	}

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);
 	}

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
 	</script>
 </head>