fix(sso): stop swallowing Entra ID login failures at DEBUG level - #3218
Merged
Conversation
getLoginCredential() caught every exception from processAuthenticationData(), logged it only when DEBUG was enabled, and returned null. SsoAction already has a handler that logs SsoLoginException at WARN and shows the SSO error message -- which is what the OpenID Connect authenticator relies on -- but the inner catch meant it never ran for Entra ID. The result was that a state mismatch, a nonce mismatch, a failed token acquisition and a Microsoft Graph outage all produced the same silent redirect to the login page, with nothing above DEBUG to tell them apart. Rethrow SsoLoginException as-is and wrap anything else in one, so both reach SsoAction. The user-visible outcome is unchanged: the SSO error message and a redirect to the login page. Wrapping rather than propagating the raw exception keeps a RuntimeException from the Graph client, such as CurlException, from escaping as a 500 error page.
marevol
force-pushed
the
entraid-surface-login-errors
branch
from
August 10, 2026 01:19
c53bb39 to
f3a3e84
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
EntraIdAuthenticator.getLoginCredential()caught every exception, logged it only at DEBUG,and returned
null:SsoAction.index()already has the right handler one frame up:but the inner catch meant it never ran for Entra ID.
OpenIdConnectAuthenticatordoes notswallow, so it gets the WARN — Entra ID was the outlier.
The practical effect: a state mismatch, a nonce mismatch, a failed token acquisition and a
Microsoft Graph outage all produce the same silent redirect to the login page, with nothing above
DEBUG to tell them apart.
Fix
Rethrow
SsoLoginExceptionunchanged, and wrap anything else in one, so both reachSsoAction.The user-visible outcome does not change — the SSO error message, then the login page. Wrapping
rather than letting the raw exception propagate matters because the Graph client throws
CurlException, aRuntimeException, whichSsoActiondoes not catch and which would otherwisesurface as a 500 error page.
Tests
EntraIdAuthenticator's unit test class, 2 new tests, both failing before the change:test_getLoginCredential_surfacesUnexpectedFailures— anIllegalStateExceptionfromprocessAuthenticationData()comes back as anSsoLoginExceptionwrapping ittest_getLoginCredential_propagatesSsoLoginExceptionUnwrapped— anSsoLoginExceptionisrethrown as the same instance, not double-wrapped
(the whole
org.codelibs.fess.ssopackage plusSsoActionTest)