Bug 63333 - JAASRealm needs to override isAvailable method to prevent LockOutRealm to lock the user in case JAAS login modules are unavailable
Summary: JAASRealm needs to override isAvailable method to prevent LockOutRealm to loc...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.5.x-trunk
Hardware: PC All
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-10 15:28 UTC by jchobantonov
Modified: 2019-04-30 10:12 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description jchobantonov 2019-04-10 15:28:16 UTC
JAASRealm needs to override isAvailable method to prevent LockOutRealm to lock the user in case JAAS login modules are unavailable

If JAAS login module fails to authenticate because of network communication issues it could throw RuntimeException (unstead of checked LoginException) in that case and if the configuration of JAAS is invalid

In following method:
protected Principal authenticate(String username, CallbackHandler callbackHandler)

where 
ExceptionUtils.handleThrowable(e); is invoked set the available flag to false so that it indicates that JAASRealm is not available to authenticate the user instead of LockOutRealm to think that the user is not authenticated because of Principal is null.

In CombinedRealm it's isAvailable() method will check if the realm !realm.isAvailable() and will not lock the user out
Comment 1 Mark Thomas 2019-04-10 17:57:00 UTC
Care to provide a patch (in diff -u format) or a pull request?

It looks like there are multiple places where available should be set to false and you'll also need to identify where it should be set to true.
Comment 2 jchobantonov 2019-04-10 19:45:35 UTC
Pull request: https://github.com/apache/tomcat/pull/157
Comment 3 Mark Thomas 2019-04-30 10:12:21 UTC
Fixed in:
- master for 9.0.20 onwards
- 8.5.x for 8.5.41 onwards

Thanks for the PR