nextcloud / nextcloud/server

loginWithApache() unconditionally re-runs full filesystem setup and keygen on every request, with no cross-request reuse

Open
#62,973 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

0. Needs triage feature: authentication performance 🚀 technical debt
Dominant language
PHP
Stars
36.9k
Forks
5.2k
Avg merge
2d 3h
Merged PRs (30d)
713

Description

Follow-up from nextcloud/user_oidc#1452 and nextcloud/user_oidc#1486, at the maintainer's suggestion there ("Your setupFS issue is worth filing separately").

What happens

OC_User::loginWithApache() calls OC_Util::setupFS($uid) and \OC::$server->getUserFolder($uid) unconditionally, on every request where the guard self::getUser() !== $uid is true:

// lib/private/legacy/OC_User.php
if (self::getUser() !== $uid) {
    ...
    $userSession->createSessionToken($request, $uid, $uid, $password);
    $userSession->createRememberMeToken($userSession->getUser());
    ...
    OC_Util::setupFS($uid);
    ...
    \OC::$server->getUserFolder($uid);
}

OC_Util::setupFS() goes straight to SetupManager::setupForUser():

// lib/private/legacy/OC_Util.php
$setupManager->setupForUser($userObject);

setupForUser()'s only guard is isSetupComplete(), backed by $this->setupUsersComplete — an in-memory array scoped to the current SetupManager instance, i.e. one request:

// lib/private/Files/SetupManager.php
public function isSetupComplete(IUser $user): bool {
    return in_array($user->getUID(), $this->setupUsersComplete, true);
}

So there is no cross-request reuse at all on this path. Contrast with the other full-setup trigger, SetupManager::setupForPath() (reached lazily via Root::getUserFolder()'s LazyUserFolder), which checks a 5-minute distributed cache (fullSetupRequired(), fs_mount_cache_duration) before doing a full setup. loginWithApache()'s call bypasses that cache entirely — it always does the full enumeration, every time the guard is open.

createSessionToken() also unconditionally generates a fresh RSA-2048 keypair (PublicKeyTokenProvider::newToken()openssl_pkey_new()), with no reuse of any existing token for the same uid.

Who hits this

Any IApacheBackend implementation authenticates through this path — most commonly bearer-token backends (user_oidc's Backend::getCurrentUserId()) where a client sends a token without a session cookie on every request. Each such request re-triggers the full block, because there is nothing that lets the guard recognize "this exact uid was fully set up 200ms ago in a different request."

Other login paths already avoid this. Session::completeLogin()'s isToken branch hard-codes $firstTimeLogin = false, and prepareUserLogin() only calls setupFS()/getUserFolder()/copySkeleton() when $firstTimeLogin is true:

// lib/private/User/Session.php
protected function prepareUserLogin($firstTimeLogin, $refreshCsrfToken = true) {
    ...
    if ($firstTimeLogin) {
        OC_Util::setupFS($user);
        ...
    }
}

loginWithApache() has no equivalent distinction between "this user's very first login ever" and "this user was already fully set up a moment ago in another request."

Measured impact

Captured via the built-in profiler app, on a real deployment, isolated (non-concurrent) bearer requests through this path:

  • fs:setup:user:full: 98.70 ms and 148.61 ms in two separate captures, dominated by mount-provider enumeration (Collectives_Mount_MountProvider, Files_Sharing_MountProvider, GroupFolders_Mount_MountProvider — none relevant to the request being served in either case).
  • The token-creation queries themselves (SELECT/DELETE×2/SELECT/INSERT/UPDATE against oc_authtoken) totaled under 0.02 ms combined — negligible; the keygen and mount setup are the actual cost, not the DB writes.
  • Under concurrent load (10 simultaneous bearer requests, same uid), this cost compounds — one capture showed 148 ms of internal work but a multi-second gap between profiler-internal timing and actual TTFB, consistent with several requests hitting this unconditional path at once and contending for DB/worker resources. Not confirmed as directly caused by this specific code path rather than general load, but consistent with it.

Suggested direction

Something that lets loginWithApache() distinguish "already fully set up very recently" from "first sighting," without reintroducing the correctness bug in #1452 (i.e. without setting the session user before the guard runs). Two shapes that seem plausible, not fully evaluated:

  • Have OC_Util::setupFS() / the login-time call check the same distributed cache SetupManager::setupForPath() already uses (fs_mount_cache_duration), instead of only the per-request isSetupComplete() flag.
  • Or, narrower: skip createSessionToken()'s keygen when a valid, non-expired token already exists for the uid from a very recent request, reusing it instead of minting a new one every time.

Happy to help test against a real deployment if useful — happy path (bearer, no cookie, repeated same-uid requests) is easy to reproduce.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/private/legacy/OC_User.php and trace loginWithApache() through OC_Util::setupFS(), SetupManager::setupForUser(), and createSessionToken(). Compare this path with SetupManager::setupForPath() and the fs_mount_cache_duration behavior, then reproduce repeated bearer requests for one uid. Done means avoiding redundant full setup or key generation across recent requests without moving the session-user guard or reintroducing the correctness issue from #1452.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
authentication, backend, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.