Skip to content

fix: unreachable LDAP backend must not block local/DB account login#62220

Open
mvanhorn wants to merge 1 commit into
nextcloud:masterfrom
mvanhorn:fix/36281-ldap-outage-blocks-local-login
Open

fix: unreachable LDAP backend must not block local/DB account login#62220
mvanhorn wants to merge 1 commit into
nextcloud:masterfrom
mvanhorn:fix/36281-ldap-outage-blocks-local-login

Conversation

@mvanhorn

Copy link
Copy Markdown

Summary

In OC\User\Manager::checkPasswordNoLogging() the two foreach ($backends ...) loops call $backend->checkPassword() directly; when the LDAP backend (OCA\User_LDAP\User_Proxy) cannot reach the server it throws OC\ServerNotAvailableException, which propagates uncaught out of Session::loginWithPassword() (the login command chain catches only Session/Token exceptions), surfacing as the reported 500 and preventing any later backend (the local OC\User\Database backend that would authenticate the local admin) from being tried. Wrap each per-backend checkPassword() call in a try/catch (\OC\ServerNotAvailableException $e) that logs a warning via the injected logger and continues to the next backend, so an unavailable backend degrades gracefully instead of blocking login for users of other backends. The exception is logged (not silently swallowed) to preserve diagnosability, and credential validation for every backend is unchanged, so this is a purely defensive hardening with no authentication-bypass surface. Add use OC\ServerNotAvailableException; (or fully-qualify) since Manager lives in the OC\User namespace.

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests are included
  • Screenshots before/after for front-end changes
  • Documentation has been updated or is not required
  • Backports requested where applicable
  • Labels added where applicable
  • Milestone added for target branch/version

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>

@CarlSchwan CarlSchwan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$this->logger->warning('Unable to check password against user backend:' . $backend instanceof IUserBackend ? $backend->getBackendName() : get_class($backend), ['exception' => $e]);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$this->logger->warning('Unable to check password against user backend:' . $backend instanceof IUserBackend ? $backend->getBackendName() : get_class($backend), ['exception' => $e]);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to be adapted too afterward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Lost connection to LDAP server, login not possible, neither for locale or ldap accounts.

2 participants