https://bz.apache.org/bugzilla/show_bug.cgi?id=63336

            Bug ID: 63336
           Summary: Currently there is no way to know in form error page
                    that the user was not authenticated because it was
                    locked out
           Product: Tomcat 8
           Version: 8.5.x-trunk
          Hardware: PC
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Catalina
          Assignee: dev@tomcat.apache.org
          Reporter: jchobanto...@yahoo.com
  Target Milestone: ----

If a user is locked out from LockOutRealm or if there are some specific
exceptions in backend user realms like user is locked in the backend, user is
required to change it's password first etc (see JAAS exception like
AccountExpiredException, AccountLockedException, AccountNotFoundException or
CredentialExpiredException - in case the password is valid but because it was
requested that the user should change it's password because forgot password has
been requested)

So we need some way to inform the user of the web app that the account has been
locked up in the login error page instead of just saying the username/password
is invalid as it is confusing and users are going to request forgot password
flow which will change their password and they are going to still not be able
to login if LockOutRealm has triggered lockout for 5 min.

What I'm suggesting is to provide custom configurable HttpServletRequest
attribute for example "login.error.message" of type String that describes why
the user was not able to login along with the exception itself so that we could
pass additional information into the exception itself in an attribute
"login.error.exception" (again configurable request attribute name in
server.xml as it is not standard - please do not use standard servlet error
message and error attributes as some frameworks will clear those attributes and
the login error page will not be able to get the correct message/exception)

Because LockOutRealm do not have the HttpServletRequest passed into the user
realms we need to have a Valve that will put the HttpServletRequest/Response
into thread local variable so that user realms/JAAS modules could obtain the
HttpServletRequest and inject the user attribute to be used by the login error
page

Note that currently basic authenticator will report 401 error but it will not
put into the body the reason why it was rejected so it could be a good thing to
refactor that as well and if request have the attribute to pull the value and
when sending 401 Http error from basic authentication to also put the error
message in the response body.

Here is an example that I'm using for LockOutRealm in order to report to the
user that the account is locked up and not that the username/password is
incorrect and having the user wonder what's wrong:

import java.security.Principal;
import java.security.cert.X509Certificate;

import javax.security.jacc.PolicyContext;
import javax.security.jacc.PolicyContextException;
import javax.servlet.http.HttpServletRequest;

import org.apache.catalina.realm.LockOutRealm;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.ietf.jgss.GSSContext;
import org.ietf.jgss.GSSException;
import org.ietf.jgss.GSSName;

public class ExtendedLockOutRealm extends LockOutRealm {

    public static final String REQUEST_ATTRIBUTE_LOGIN_ERROR_MESSAGE =
"login.error.message"; 

    public static final String REQUEST_ATTRIBUTE_LOCKOUT_MESSAGE =
ExtendedLockOutRealm.class.getName() + ".REQUEST_ATTRIBUTE_LOCKOUT_MESSAGE"; 
    public static final String REQUEST_ATTRIBUTE_LOCKOUT_USERNAME =
ExtendedLockOutRealm.class.getName() + ".REQUEST_ATTRIBUTE_LOCKOUT_USERNAME"; 
    public static final String REQUEST_ATTRIBUTE_LOCKOUT_TIME =
ExtendedLockOutRealm.class.getName() + ".REQUEST_ATTRIBUTE_LOCKOUT_TIME"; 

    private static final Log log = LogFactory.getLog(LockOutRealm.class);

    @Override
    public Principal authenticate(String username, String clientDigest,
            String nonce, String nc, String cnonce, String qop,
            String realmName, String md5a2) {
        if (isLocked(username)) {
            processLockOutUser(username, null);
            return null;
        }
        Principal principal = super.authenticate(username, clientDigest, nonce,
nc, cnonce, qop, realmName, md5a2);
        processLockOutUser(username, principal);
        return principal;
    }

    @Override
    public Principal authenticate(String username, String credentials) {
        if (isLocked(username)) {
            processLockOutUser(username, null);
            return null;
        }
        Principal principal = super.authenticate(username, credentials);
        processLockOutUser(username, principal);
        return principal;
    }

    @Override
    public Principal authenticate(X509Certificate[] certs) {
        String username = null;
        if (certs != null && certs.length >0) {
            username = certs[0].getSubjectDN().getName();
        }
        if (isLocked(username)) {
            processLockOutUser(username, null);
            return null;
        }

        Principal principal = super.authenticate(certs);
        processLockOutUser(username, principal);
        return principal;
    }

    @Override
    public Principal authenticate(GSSContext gssContext, boolean storeCreds) {
        String username = null;
        if (gssContext.isEstablished()) {
            GSSName name = null;
            try {
                name = gssContext.getSrcName();
            } catch (GSSException e) {
                log.warn(sm.getString("realmBase.gssNameFail"), e);
                return null;
            }

            username = name.toString();

            if (isLocked(username)) {
                processLockOutUser(username, null);
                return null;
            }
        }
        Principal principal = super.authenticate(gssContext, storeCreds);
        processLockOutUser(username, principal);
        return principal;
    }

    private void processLockOutUser(String username, Principal principal) {
        if (principal == null && username != null && isLocked(username)) {

            log.warn(sm.getString("lockOutRealm.authLockedUser", username));

            if
(PolicyContext.getHandlerKeys().contains("javax.servlet.http.HttpServletRequest"))
{
                try {
                    Object object =
PolicyContext.getContext("javax.servlet.http.HttpServletRequest");
                    if (object instanceof HttpServletRequest) {
                        HttpServletRequest httpServletRequest =
(HttpServletRequest) object;
                        int lockoutMin = lockOutTime / 60;
                        String message = "Account has been locked for " +
(lockoutMin > 0 ? (lockoutMin + (lockoutMin > 1 ? " minutes" : " minute")) :
(lockOutTime + (lockOutTime > 1 ? " seconds" : " second")));

                       
httpServletRequest.setAttribute(REQUEST_ATTRIBUTE_LOGIN_ERROR_MESSAGE,
message);

                       
httpServletRequest.setAttribute(REQUEST_ATTRIBUTE_LOCKOUT_MESSAGE, message);
                       
httpServletRequest.setAttribute(REQUEST_ATTRIBUTE_LOCKOUT_USERNAME, username);
                       
httpServletRequest.setAttribute(REQUEST_ATTRIBUTE_LOCKOUT_TIME, lockOutTime);
                    }
                } catch (PolicyContextException ignored) {
                }
            }
        }
    }
}


the override authenticate methods are because I want if the user is locked out
to return immediately and not invoke the inner user realms 
basically adding the check:
if (isLocked(username)) {
}
before the actual super.authenticate method

I'm using javax.security.jacc.PolicyContext in order to store http
request/response so that security classes be able to obtain them


Here is the valve to register the request and reponse with PolicyContext:

import java.io.IOException;

import javax.security.jacc.PolicyContext;
import javax.security.jacc.PolicyContextException;
import javax.servlet.ServletException;

import org.apache.catalina.LifecycleException;
import org.apache.catalina.connector.Request;
import org.apache.catalina.connector.Response;
import org.apache.catalina.valves.ValveBase;

public class JACCPolicyContextValve extends ValveBase {

    private static final String ALREADY_FILTERED_ATTRIBUTE =
JACCPolicyContextValve.class.getName() + ".FILTERED";

    private boolean debug = false;

    public void setDebug(final Boolean debug) {
        if (debug != null) {
            this.debug = debug;
        }
    }

    @Override
    protected void initInternal() throws LifecycleException {
        if (debug) {
            System.out.println("[JACCPolicyContextValve] initInternal
invoked.");
        }
        super.initInternal();
        if
(!PolicyContext.getHandlerKeys().contains(HttpServletRequestPolicyContextHandler.KEY))
{
            try {
               
PolicyContext.registerHandler(HttpServletRequestPolicyContextHandler.KEY, new
HttpServletRequestPolicyContextHandler(), false);
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Register " +
HttpServletRequestPolicyContextHandler.class.getName() + " handler for key " +
HttpServletRequestPolicyContextHandler.KEY);
                }
            } catch (PolicyContextException ex) {
                if (debug) {
                    ex.printStackTrace();
                }
            }
        } else {
            if (debug) {
                System.out.println("[JACCPolicyContextValve] PolicyContext
already have " + HttpServletRequestPolicyContextHandler.KEY + " handler key.");
            }
        }
        if
(!PolicyContext.getHandlerKeys().contains(HttpServletResponsePolicyContextHandler.KEY))
{
            try {
               
PolicyContext.registerHandler(HttpServletResponsePolicyContextHandler.KEY, new
HttpServletResponsePolicyContextHandler(), false);
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Register " +
HttpServletResponsePolicyContextHandler.class.getName() + " handler for key " +
HttpServletResponsePolicyContextHandler.KEY);
                }
            } catch (PolicyContextException ex) {
                if (debug) {
                    ex.printStackTrace();
                }
            }
        } else {
            if (debug) {
                System.out.println("[JACCPolicyContextValve] PolicyContext
already have " + HttpServletResponsePolicyContextHandler.KEY + " handler
key.");
            }
        }
    }

    @Override
    public void invoke(Request request, Response response) throws IOException,
ServletException {
        if (request.getAttribute(ALREADY_FILTERED_ATTRIBUTE) == null) {
            request.setAttribute(ALREADY_FILTERED_ATTRIBUTE, Boolean.TRUE);
            try {
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Set request of
class: " + request.getClass().getName());
                }
                SecurityContextAssociationHandler.activeRequest.set(request);
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Set response
of class: " + response.getClass().getName());
                }
                SecurityContextAssociationHandler.activeResponse.set(response);
                getNext().invoke(request, response);
            } finally {
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Unset
request.");
                }
                SecurityContextAssociationHandler.activeRequest.set(null);
                if (debug) {
                    System.out.println("[JACCPolicyContextValve] Unset
response.");
                }
                SecurityContextAssociationHandler.activeResponse.set(null);
            }
        } else {
            getNext().invoke(request, response);
        }
    }
}


Now having request obtained from thread local will allow JAAS modules to put
information why it did not login so that even when the JAASRealm catches the
exceptions thrown from JAAS modules and just return null for the Principal to
still know why it failed.

Having catalina's standard way to obtain the request in security related
structure that do not have http request passed as parameter will allow security
providers to put security related information that is needed to inform the user
as to why he/she can't login

This issue is related to 
https://bz.apache.org/bugzilla/show_bug.cgi?id=63334

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to