Skip to content

Commit 72fd8e3

Browse files
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 <[email protected]>
1 parent 4283f47 commit 72fd8e3

File tree

1 file changed

+31
-6
lines changed

1 file changed

+31
-6
lines changed

lib/base.php

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,13 +1156,31 @@ public static function handleRequest(): void {
11561156
}
11571157

11581158
/**
1159-
* Check login: apache auth, auth token, basic auth
1159+
* Attempts to authenticate the current request using multiple authentication mechanisms.
1160+
*
1161+
* Tries authentication in the following order: Apache authentication, app API login, token-based login,
1162+
* cookie-based login, and HTTP Basic authentication. Returns true if any method authenticates the user
1163+
* successfully, otherwise false.
1164+
*
1165+
* If the request contains the 'X-Nextcloud-Federation' header, authentication will be skipped.
1166+
*
1167+
* @param OCP\IRequest $request The current HTTP request object.
1168+
* @return bool True if authentication succeeds, false otherwise.
1169+
* @throws \Exception Passes through any unexpected exceptions from underlying authentication methods.
1170+
*
1171+
* @security Every authentication method is invoked in priority order, and early return is used on first success.
1172+
* Headers from federation requests are explicitly rejected.
11601173
*/
11611174
public static function handleLogin(OCP\IRequest $request): bool {
1175+
// Talk federated users have no user backend; auth handled via Talk
11621176
if ($request->getHeader('X-Nextcloud-Federation')) {
11631177
return false;
11641178
}
1179+
11651180
$userSession = Server::get(\OC\User\Session::class);
1181+
$throttler = Server::get(IThrottler::class);
1182+
1183+
// Try different authentication methods in order of preference
11661184
if (OC_User::handleApacheAuth()) {
11671185
return true;
11681186
}
@@ -1172,15 +1190,22 @@ public static function handleLogin(OCP\IRequest $request): bool {
11721190
if ($userSession->tryTokenLogin($request)) {
11731191
return true;
11741192
}
1175-
if (isset($_COOKIE['nc_username'])
1176-
&& isset($_COOKIE['nc_token'])
1177-
&& isset($_COOKIE['nc_session_id'])
1178-
&& $userSession->loginWithCookie($_COOKIE['nc_username'], $_COOKIE['nc_token'], $_COOKIE['nc_session_id'])) {
1193+
if (
1194+
$request->getCookie('nc_username') !== null &&
1195+
$request->getCookie('nc_token') !== null &&
1196+
$request->getCookie('nc_session_id') !== null &&
1197+
$userSession->loginWithCookie(
1198+
$request->getCookie('nc_username'),
1199+
$request->getCookie('nc_token'),
1200+
$request->getCookie('nc_session_id')
1201+
)
1202+
) {
11791203
return true;
11801204
}
1181-
if ($userSession->tryBasicAuthLogin($request, Server::get(IThrottler::class))) {
1205+
if ($userSession->tryBasicAuthLogin($request, $throttler))) {
11821206
return true;
11831207
}
1208+
11841209
return false;
11851210
}
11861211

0 commit comments

Comments
 (0)