From 9688be42792647a2792b81cbc3f14692968d9f53 Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 29 Dec 2025 12:14:32 -0500 Subject: [PATCH 1/4] refactor(auth): improve robustness and readability of handleLogin - Use $request->getCookie() instead of $_COOKIE directly for consistency. - Assign dependencies via variables up front for readability. - Note exceptions from each login method is possible and are not handled here / must be handled by callers. Just minor refinements to readability, maintainability, and some aspects of robustness. Signed-off-by: Josh --- lib/base.php | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/lib/base.php b/lib/base.php index b656158b53070..b1e371b8089f3 100644 --- a/lib/base.php +++ b/lib/base.php @@ -1156,13 +1156,31 @@ public static function handleRequest(): void { } /** - * Check login: apache auth, auth token, basic auth + * Attempts to authenticate the current request using multiple authentication mechanisms. + * + * Tries authentication in the following order: Apache authentication, app API login, token-based login, + * cookie-based login, and HTTP Basic authentication. Returns true if any method authenticates the user + * successfully, otherwise false. + * + * If the request contains the 'X-Nextcloud-Federation' header, authentication will be skipped. + * + * @param OCP\IRequest $request The current HTTP request object. + * @return bool True if authentication succeeds, false otherwise. + * @throws \Exception Passes through any unexpected exceptions from underlying authentication methods. + * + * @security Every authentication method is invoked in priority order, and early return is used on first success. + * Headers from federation requests are explicitly rejected. */ public static function handleLogin(OCP\IRequest $request): bool { + // Talk federated users have no user backend; auth handled via Talk if ($request->getHeader('X-Nextcloud-Federation')) { return false; } + $userSession = Server::get(\OC\User\Session::class); + $throttler = Server::get(IThrottler::class); + + // Try different authentication methods in order of preference if (OC_User::handleApacheAuth()) { return true; } @@ -1172,15 +1190,22 @@ public static function handleLogin(OCP\IRequest $request): bool { if ($userSession->tryTokenLogin($request)) { return true; } - if (isset($_COOKIE['nc_username']) - && isset($_COOKIE['nc_token']) - && isset($_COOKIE['nc_session_id']) - && $userSession->loginWithCookie($_COOKIE['nc_username'], $_COOKIE['nc_token'], $_COOKIE['nc_session_id'])) { + if ( + $request->getCookie('nc_username') !== null && + $request->getCookie('nc_token') !== null && + $request->getCookie('nc_session_id') !== null && + $userSession->loginWithCookie( + $request->getCookie('nc_username'), + $request->getCookie('nc_token'), + $request->getCookie('nc_session_id') + ) + ) { return true; } - if ($userSession->tryBasicAuthLogin($request, Server::get(IThrottler::class))) { + if ($userSession->tryBasicAuthLogin($request, $throttler))) { return true; } + return false; } From d0cb033ba697677349bc0d5eb7afeba402e9f151 Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 30 Dec 2025 07:40:46 -0500 Subject: [PATCH 2/4] chore: fix typo Signed-off-by: Josh --- lib/base.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/base.php b/lib/base.php index b1e371b8089f3..1f3e830cd09c4 100644 --- a/lib/base.php +++ b/lib/base.php @@ -1202,7 +1202,7 @@ public static function handleLogin(OCP\IRequest $request): bool { ) { return true; } - if ($userSession->tryBasicAuthLogin($request, $throttler))) { + if ($userSession->tryBasicAuthLogin($request, $throttler)) { return true; } From 9378c2f6063bf2a01c5a395213b18871fca57603 Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 30 Dec 2025 09:38:07 -0500 Subject: [PATCH 3/4] chore: fixup for lint Signed-off-by: Josh --- lib/base.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/base.php b/lib/base.php index 1f3e830cd09c4..ef18b465c2981 100644 --- a/lib/base.php +++ b/lib/base.php @@ -1191,10 +1191,10 @@ public static function handleLogin(OCP\IRequest $request): bool { return true; } if ( - $request->getCookie('nc_username') !== null && - $request->getCookie('nc_token') !== null && - $request->getCookie('nc_session_id') !== null && - $userSession->loginWithCookie( + $request->getCookie('nc_username') !== null + && $request->getCookie('nc_token') !== null + && $request->getCookie('nc_session_id') !== null + && $userSession->loginWithCookie( $request->getCookie('nc_username'), $request->getCookie('nc_token'), $request->getCookie('nc_session_id') From fbac3a7c7d01374f2e118195b100c30b613b6163 Mon Sep 17 00:00:00 2001 From: Josh Date: Sun, 11 Jan 2026 09:15:51 -0500 Subject: [PATCH 4/4] chore: updater per review input Signed-off-by: Josh --- lib/base.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/base.php b/lib/base.php index ef18b465c2981..6b2d0aaf25c7f 100644 --- a/lib/base.php +++ b/lib/base.php @@ -1190,15 +1190,14 @@ public static function handleLogin(OCP\IRequest $request): bool { if ($userSession->tryTokenLogin($request)) { return true; } + $username = $request->getCookie('nc_username'); + $token = $request->getCookie('nc_token'); + $sessionId = $request->getCookie('nc_session_id'); if ( - $request->getCookie('nc_username') !== null - && $request->getCookie('nc_token') !== null - && $request->getCookie('nc_session_id') !== null - && $userSession->loginWithCookie( - $request->getCookie('nc_username'), - $request->getCookie('nc_token'), - $request->getCookie('nc_session_id') - ) + $username !== null + && $token !== null + && $sessionId !== null + && $userSession->loginWithCookie($username, $token, $sessionId) ) { return true; }