Skip to content

feat: display friendly error messages for LDAP authentication failures#995

Merged
JohnVillalovos merged 1 commit intodevelopfrom
jlvillal/ldap_error
Feb 12, 2026
Merged

feat: display friendly error messages for LDAP authentication failures#995
JohnVillalovos merged 1 commit intodevelopfrom
jlvillal/ldap_error

Conversation

@JohnVillalovos
Copy link
Collaborator

Wrap authentication validation in try/catch to handle LDAP-related exceptions (missing pear/net_ldap2 library, unreachable LDAP server) and display (hopefully) user-friendly error messages on the login page instead of a generic error or unhandled exception.

This doesn't change the message seen if the issue is a wrong username/password.

Wrap authentication validation in try/catch to handle LDAP-related
exceptions (missing pear/net_ldap2 library, unreachable LDAP server) and
display (hopefully) user-friendly error messages on the login page
instead of a generic error or unhandled exception.

This doesn't change the message seen if the issue is a wrong
username/password.
Copilot AI review requested due to automatic review settings February 11, 2026 23:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the login experience by catching LDAP-related authentication exceptions and showing a more specific, user-facing error message on the login page instead of an unhandled exception/generic failure.

Changes:

  • Wrap LoginPresenter::Login() credential validation in try/catch and map known LDAP failure messages to localized UI strings.
  • Add support in LoginPage + template to display an optional login error message override.
  • Extend fakes/tests to cover exception-driven authentication failures and the new messaging behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
Presenters/LoginPresenter.php Catches auth exceptions, logs them, and selects an LDAP-specific message when applicable.
Pages/LoginPage.php Adds SetLoginErrorMessage() and initializes a LoginErrorMessage template variable.
tpl/login.tpl Displays LoginErrorMessage when present, otherwise falls back to the existing LoginError translation.
tests/fakes/FakeWebAuthentication.php Allows the fake Validate() to throw exceptions for presenter tests.
tests/Presenters/LoginPresenterTest.php Adds test cases for LDAP dependency missing / connection errors / generic exception fallback; updates fake page with new setter.
lang/en_us.php Adds new localized strings for LDAP dependency missing and connection errors.
lang/de_de.php Adds the same LDAP error strings in German.
lang/es.php Adds the same LDAP error strings in Spanish.
lang/ja_jp.php Adds the same LDAP error strings in Japanese.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JohnVillalovos JohnVillalovos merged commit cc320fa into develop Feb 12, 2026
17 checks passed
@JohnVillalovos JohnVillalovos deleted the jlvillal/ldap_error branch February 12, 2026 00:04
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.

1 participant