DAV Basic Auth: empty username/password reaches getByEmail and scans oc_preferences (heavy on NC 31/32)

[joaoathaide]

DAV Basic Auth: empty username/password reaches getByEmail and scans oc_preferences (heavy on NC 31/32)

How to reproduce

1. Run a WebDAV request against a private calendar/addressbook URL with Basic authentication header where both username and password are empty (e.g. Thunderbird or other clients sending Authorization: Basic Og==).

Example:

curl -X PROPFIND \
  -H 'Authorization: Basic Og==' \
  -H 'Content-Type: text/xml; charset=utf-8' \
  -H 'User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:140.0) Gecko/20100101 Thunderbird/140.9.0' \
  -d '<?xml version="1.0" encoding="UTF-8"?><D:propfind xmlns:D="DAV:"><D:prop><D:resourcetype/></D:prop></D:propfind>' \
  'https://<host>/remote.php/dav/calendars/<user>/personal/'

Expected behaviour

The request should fail fast as unauthenticated without running a broad database query over user preferences (equivalent to “no credentials provided”).

Actual behaviour

The Sabre OCA\DAV\Connector\Sabre\Auth backend still proceeds far enough that Session::logClientIn() runs with an empty login name. That path calls isTwoFactorEnforced($user) with an empty/null-like username. Inside isTwoFactorEnforced, IUserManager::get() fails and the code falls through to getByEmail($username) with an empty string.

User\Manager::getByEmail() resolves users via getUsersForUserValueCaseInsensitive('settings', 'email', $email), which builds a query on preferences. On instances where UserConfig::isUpgradedTo31() is true (Nextcloud 31.x and 32.x), searchUsersByTypedValue() adds the indexed/configvalue OR logic, producing SQL similar to:

SELECT `userid` FROM `oc_preferences`
WHERE (`appid` = 'settings')
  AND (`configkey` = 'email')
  AND ((`indexed` = '') OR (((`flags` & 2) <> 2) AND (`configvalue` = '')))

That effectively scans users matching “empty email” storage, which is very expensive on large instances and is triggered by a client sending empty Basic credentials.

On 33+, isUpgradedTo31() is false (only 31.* and 32.*), so the query shape differs and this specific heavy path may not apply — but the logical bug (treating empty credentials like a resolvable email lookup) likely remains worth fixing centrally.

Analysis / call chain (summary)

1. apps/dav/lib/Connector/Sabre/Auth.php — validateUserPass($username, $password) can call logClientIn even when both are empty.
2. lib/private/User/Session.php — logClientIn → isTwoFactorEnforced($user) before password validation.
3. isTwoFactorEnforced → getByEmail($username) when user is null.
4. lib/private/User/Manager.php::getByEmail → preference search.
5. lib/private/Config/UserConfig.php::searchUsersByTypedValue — expensive branch when isUpgradedTo31() is true.

Proposed fix (one option)

Early-return in DAV Basic auth when both username and password are empty, e.g. in Auth::validateUserPass: close session and return false without calling logClientIn. This matches the idea that Basic auth with no credentials should not enter the full login/2FA resolution stack.

Alternatively (or additionally), guard in Session::isTwoFactorEnforced / getByEmail against null/empty email so preference search is never run for an empty lookup value.

Server configuration

• Affected: Nextcloud 31.x, 32.x (confirmed heavy query path via isUpgradedTo31()).
• Client example: Thunderbird 140.x sending Basic Og== on PROPFIND to CalDAV.

Additional context

We validated locally that rejecting empty user+password at the DAV auth layer returns a normal authentication failure to the client without triggering the preference scan.

[icarta-l]

Hi!

I was able to reproduce the error locally and confirm that the empty Basic Auth curl request does fire the Session::logClientIn() method on master.

If that's okay, I would like to implement the fix to make the Auth::validateUserPass() return false when both the username and password are empty, unless @joaoathaide wants to fix it himself.

[icarta-l]

I have added a small test to AuthTest.php and a condition to Auth.php.

I hope it helps.

[joaoathaide]

Thanks @icarta-l, this looks aligned with the issue.

The important part for me is that empty Basic Auth credentials fail fast and do not reach logClientIn() / getByEmail(''), which is what triggers the expensive oc_preferences query. Your PR seems to address that path, and the added unit test is helpful.

Please continue with this approach.