Dear Shiro Team:
In my opnion,
org.apache.shiro.authc.AbstractAuthenticator.authenticate(AuthenticationToken
token) handles exception inappropriately.Below is the code indicates the
problem with my note in red:
//
org.apache.shiro.authc.AbstractAuthenticator.authenticate(AuthenticationToken
token)
public final AuthenticationInfo authenticate(AuthenticationToken token)
throws AuthenticationException {
if (token == null) {
throw new IllegalArgumentException("Method argumet (authentication
token) cannot be null.");
}
log.trace("Authentication attempt received for token [{}]", token);
AuthenticationInfo info;
try {
info = doAuthenticate(token);
if (info == null) {
String msg = "No account information found for authentication
token [" + token + "] by this " +
"Authenticator instance. Please check that it is
configured correctly.";
throw new AuthenticationException(msg);
}
} catch (Throwable t) {
AuthenticationException ae = null;
if (t instanceof AuthenticationException) {
ae = (AuthenticationException) t;
}
// Indeed, the Throwable t which is not caused by auhtentication
problem but other such as a database problem is wrapped to be a
AuthenticationException instance, but details including exception message and
stacktrace are missing and the follow-up handling makes the raw //Throwable t
ignored.
if (ae == null) {
//Exception thrown was not an expected AuthenticationException.
Therefore it is probably a little more
//severe or unexpected. So, wrap in an
AuthenticationException, log to warn, and propagate:
String msg = "Authentication failed for token submission [" +
token + "]. Possible unexpected " +
"error? (Typical or expected login exceptions should
extend from AuthenticationException).";
ae = new AuthenticationException(msg, t);
}
try {
notifyFailure(token, ae);
} catch (Throwable t2) {
if (log.isWarnEnabled()) {
String msg = "Unable to send notification for failed
authentication attempt - listener error?. " +
"Please check your AuthenticationListener
implementation(s). Logging sending exception " +
"and propagating original AuthenticationException
instead...";
log.warn(msg, t2);
}
}
throw ae;
}
log.debug("Authentication successful for token [{}]. Returned account
[{}]", token, info);
notifySuccess(token, info);
return info;
}
And then, the Authencication instance that wraps the orignal Thowable
instance is thrown all the way, and get dumped below:
// org.apache.shiro.web.filter.authc.AuthenticatingFilter
protected boolean executeLogin(ServletRequest request, ServletResponse
response) throws Exception {
AuthenticationToken token = createToken(request, response);
if (token == null) {
String msg = "createToken method implementation returned null. A
valid non-null AuthenticationToken " +
"must be created in order to execute a login attempt.";
throw new IllegalStateException(msg);
}
try {
Subject subject = getSubject(request, response);
subject.login(token);
return onLoginSuccess(token, subject, request, response);
} catch (AuthenticationException e) {
// ignore the AuthenticationException instance, only return a
boolean, make the original Throwable ignored and it is hard to find out the
mistake in the authentication progress.
return onLoginFailure(token, e, request, response);
}
}
Above shows the problem, that makes the origianl Throwable instance
disappeared. In my situation, a Hibernate Exception was thrown, but no message
showed, which made me take a long time to find out the mistake .
English is not my native laguage. I know I express not well in English,
hope it does not influence you to understand my opnion.
Yours respectfully,
Liang Weiwei
