Sakai 23 - LDAP Authentication being stale due to using m_callCache

[Sebastian Riemer]

Dear community,

we're locking out inactive i.e. disabled users. We use LDAP for authentication. We use Sakai 23.

This code section is what fails the login successfully:

[UserAuthnComponent.java]
...

// Check to see if the user account is disabled
String disabled = user.getProperties().getProperty("disabled");
if (disabled != null && "true".equals(disabled))
{
throw new AuthenticationException("Account Disabled: The user's authentication has been disabled");
}

However, if we then change the LDAP-criteria which marks if the user is disabled or not, this change has no effect due to caching:

BaseUserDirectoryService.java (line 

public User getUserByEid(String eid) throws UserNotDefinedException

...
String id = m_storage.checkMapForId(eid);
if (id != null)
{
user = getCachedUser(userReference(id));

...

protected UserEdit getCachedUser(String ref)
{
// KNL-1241 removed caching in threadlocal
UserEdit userEdit = null;
if (m_callCache != null)
{
Object cachedRef = m_callCache.get(ref);

This cache is currently configured to keep rows in between 8 to 12 hours.

Clearing (all) caches via "Memory > Reset all caches" solves the issue.

I've tried to sketch the involved classes in a diagram. See appendix. 

In there, I've diagnosed, that the authCache is not involved in this situation, but the m_storage and m_callCache is returning the User-object; Note, that the authentication itself is being done (successfully and would be mapping the LDAP-attribute to the User's disabled-property correctly), using that cached User-object. Our LDAP itself does allow binding, yet returns the LDAP-attribute responsible for calculating the "disabled"-property. - which is never used, since the old cached User-object is being returned instead.

I think it is wrong, that during login, any cached value inside BaseUserDirectoryService is being considered; Instead, each new login-attempt should always lead to consulting:

UnboundidDirectoryProvider.getUserByEid

What is your opinion on this?

Best regards,

Sebastian Riemer

[Sebastian Riemer]

Ok, so I found an interesting comment on UnboundidDirectoryProvider.authenticateUser :

/**
* Authenticates the specified user login by recursively searching for
* and binding to a DN below the configured base DN. Search results are
* subsequently added to the cache.
*
* <p>Caching search results departs from
* behavior in &lt;= 2.3.0 versions, which removed cache entries following
* authentication. If the intention is to ensure fresh user data at each
* login, the most natural approach is probably to clear the cache before
* executing the authentication process. At this writing, though, the
* default {@link org.sakaiproject.user.api.UserDirectoryService} impl
* will invoke {@link #getUser(UserEdit)} prior to
* {{@link #authenticateUser(String, UserEdit, String)}} if the Sakai's
* local db does not recognize the specified EID. Therefore, clearing the
* cache at in {{@link #authenticateUser(String, UserEdit, String)}}
* at best leads to confusing mid-session attribute changes. In the future
* we may want to consider strategizing this behavior, or adding an eid
* parameter to {@link #destroyAuthentication()} so cache records can
* be invalidated on logout without ugly dependencies on the
* {@link org.sakaiproject.tool.api.SessionManager}
*
* @see #lookupUserBindDn(String, LDAPConnection)
*/
public boolean authenticateUser(final String userLogin, final UserEdit edit, final String password)

By testing I discovered, that the method BaseUserDirectoryService.putCachedUser is called each time when a Login-Attempt is made, and it keeps putting the previously loaded cache entry (!) into the cache (again) - this will renew the cache entry which effectively will keep the stale data alive. Practically speaking, if that cache has a TTL of 8 hours (I've seen others have usually a TTL of 14400 secons = 4 hours), users would have to not make a login attempt for that timespan, to ever succeed in logging in, after we've changed the LDAP-flag in order to reactivate their user account.

I thus don't think an easy solution would be to drastically lower that TTL of the cache, given that it's purpose is to provide SakaiProxyImpl which is heavily used in templates etc.

So I will now investigate the options for evicting the cache entry before each authentication attempt.

Best regards,

Sebastian

[Sebastian Riemer]

Ok, I've resolved that issue for us by removing the user from the cache, prior to authenticating:

BaseUserDirectoryService > getProviderAuthenticatedUser

...

try
{

String calculatedUserreference = userReference(m_storage.checkMapForId(loginId));
removeCachedUser(calculatedUserreference, loginId);

user = (UserEdit)getUserByAid(loginId);
} catch (UserNotDefinedException e)
{
return null;
}
boolean authenticated = m_provider.authenticateUser(loginId, user, password);
if (!authenticated) user = null;

Hope this might help somebody else in the future!

Best regards,

Sebastian