Servlet and JSP for user registration, with CAPTCHA and error handling

Question

I have a form that submits to a registration Servlet, which inserts a new user to the database when they sign up to the web application. Depending on the erroneous user input, the Servlet will sendRedirect() to a result page, telling the user what went wrong.

Servlet: doPost()

protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
    String userId = request.getParameter("username");
    String userEmail1 = request.getParameter("email1");
    String userEmail2 = request.getParameter("email2");
    String userPassword1 = request.getParameter("pass1");
    String userPassword2 = request.getParameter("pass2");
    String captchaAnswer = request.getParameter("answer");

    String[] userDetails = {
        userId,
        userEmail1,
        userEmail2,
        userPassword1,
        userPassword2,
        captchaAnswer,
    };

    UserManager um = new UserManager();
    boolean isElementNull = um.isElementNull(userDetails);

    if(isElementNull) {
        response.sendRedirect("index.jsp");
        return;
    }

    // simple captcha
    HttpSession session = request.getSession(true);
    Captcha captcha = (Captcha) session.getAttribute(Captcha.NAME);

    if(captcha == null) {
        response.sendRedirect("register.jsp");
        return;
    }

    request.setCharacterEncoding("UTF-8");  

    boolean isCaptchaCorrect = captcha.isCorrect(captchaAnswer);
    session.setAttribute("isCaptchaCorrect", isCaptchaCorrect);

    // for retaining form value
    session.setAttribute("userId", userId);
    session.setAttribute("userEmail1", userEmail1);
    session.setAttribute("userEmail2", userEmail2);

    if(isCaptchaCorrect) {
        // validate inputs
        boolean isUsernameBlank = um.isFieldBlank(userId);
        boolean isEmailBlank = um.isFieldBlank(userEmail1);
        boolean isPasswordBlank = um.isFieldBlank(userPassword1);
        boolean isEmailValid = um.isEmailValid(userEmail1, userEmail2);
        boolean isPasswordLengthValid = um.isPasswordLengthValid(userPassword1, userPassword2);         
        boolean isExistingUsername = um.isExisting("user_id", userId);
        boolean isExistingEmail = um.isExisting("user_email", userEmail1);
        boolean isEmailMatch = um.isMatch(userEmail1, userEmail2);
        boolean isPasswordMatch = um.isMatch(userPassword1, userPassword2);

        // bind objects to session
        session.setAttribute("isUsernameBlank", isUsernameBlank);
        session.setAttribute("isEmailBlank", isEmailBlank);
        session.setAttribute("isPasswordBlank", isPasswordBlank);
        session.setAttribute("isEmailValid", isEmailValid);
        session.setAttribute("isPasswordLengthValid", isPasswordLengthValid);           
        session.setAttribute("isExistingUsername", isExistingUsername);
        session.setAttribute("isExistingEmail", isExistingEmail);
        session.setAttribute("isEmailMatch", isEmailMatch);
        session.setAttribute("isPasswordMatch", isPasswordMatch);       

        if(isUsernameBlank || isEmailBlank || isPasswordBlank || !isEmailValid || !isPasswordLengthValid
            || isExistingUsername || isExistingEmail || !isEmailMatch || !isPasswordMatch) {

            response.sendRedirect("register-result.jsp");

        // register success
        } else {
            String userPassword = userPassword1;
            String userEmail = userEmail1;

            // get current system time
            java.util.Date utilDate = new java.util.Date();

            // convert to sql date
            java.sql.Date registeredDate = new java.sql.Date(utilDate.getTime());

            // assemble user bean object
            User user = UserAssembler.getInstance(
                userId,
                userPassword,
                userEmail,
                2, // 2 = User
                registeredDate
            );

            // insert user into database
            um.registerUser(user);  

            response.sendRedirect("register-result.jsp");
        }

    // wrong captcha answer
    } else {
        response.sendRedirect("register-result.jsp");               
    }
}

JSP: (register-result.jsp)

<body>
    <strong>REGISTER</strong>

    <%
    Boolean isCaptchaCorrect = (Boolean) session.getAttribute("isCaptchaCorrect");

    if(isCaptchaCorrect == null) {
        response.sendRedirect("index.jsp");
        return;
    }

    if(isCaptchaCorrect) {
        Boolean isUsernameBlank = (Boolean) session.getAttribute("isUsernameBlank");
        Boolean isEmailBlank = (Boolean) session.getAttribute("isEmailBlank");
        Boolean isPasswordBlank = (Boolean) session.getAttribute("isPasswordBlank");
        Boolean isEmailValid = (Boolean) session.getAttribute("isEmailValid");
        Boolean isPasswordLengthValid = (Boolean) session.getAttribute("isPasswordLengthValid");                
        Boolean isExistingUsername = (Boolean) session.getAttribute("isExistingUsername");
        Boolean isExistingEmail = (Boolean) session.getAttribute("isExistingEmail");
        Boolean isEmailMatch = (Boolean) session.getAttribute("isEmailMatch");
        Boolean isPasswordMatch = (Boolean) session.getAttribute("isPasswordMatch");

    if(isUsernameBlank) {
    %>

    <form action="register.jsp">
    <table>
        <tr><td style="color: #FF0000">Username cannot be blank!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Try Again"/>
    </form>

    <% } else if(isEmailBlank) { %>

    <form action="register.jsp">
    <table>
        <tr><td style="color: #FF0000">Email cannot be blank!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Try Again"/>
    </form>

    <% } else if(isPasswordBlank) { %>

    <form action="register.jsp">
    <table>
        <tr><td style="color: #FF0000">Password cannot be blank!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Try Again"/>
    </form>

    <% } else if(!isEmailValid) { %>

    <form action="register.jsp">
    <table>
        <tr><td style="color: #FF0000">Email is not in a valid format!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Try Again"/>
    </form>

    <% } else if(!isPasswordLengthValid) { %>

    <form action="register.jsp">
    <table>
        <tr><td style="color: #FF0000">Password must be at least 8 characters!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Try Again"/>
    </form>

    <% } else if(isExistingUsername) { %>

    <form action="register.jsp">
    <table>
        <tr><td style="color: #FF0000">Username already exists!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Try Again"/>
    </form>

    <% } else if(isExistingEmail) { %>

    <form action="register.jsp">
    <table>
        <tr><td style="color: #FF0000">Email already exists!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Try Again"/>
    </form>

    <% } else if(!isEmailMatch) { %>

    <form action="register.jsp">
    <table>
        <tr><td style="color: #FF0000">Emails entered do not match!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Try Again"/>
    </form>

    <% } else if(!isPasswordMatch) { %>

    <form action="register.jsp">
    <table>
        <tr><td style="color: #FF0000">Passwords entered do not match!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Try Again"/>
    </form>

    <% } else { %>

    <!-- register success -->
    <form action="login.jsp">
    <table>
        <tr><td>Your registration was successful!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Login"/>
    </form>

    <% }

    } else { %>             

    <!-- wrong captcha answer -->
    <form action="register.jsp">
    <table>
        <tr><td style="color: #FF0000">Your CAPTCHA answer is incorrect!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Try Again"/>
    </form>

    <% } %>
</body>

The code appears unwieldy with all the booleans being setAttribute() in the Servlet and then getAttribute() in the JSP. The error messages are also hardcoded in JSP as seen above.

Is there an elegant way to handle user input errors and error messages, where they do not have to be hardcoded in the JSP itself?

(Please bear with me as we are limited to using scriptlets for now.)

Answer 1

Duplication:

<% } else if(!isEmailMatch) { %>

<form action="register.jsp">
<table>
    <tr><td style="color: #FF0000">Emails entered do not match!</td></tr>
</table>
<br/>
<input type="submit" value="Try Again"/>
</form>

<% } else if(!isPasswordMatch) { %>

<form action="register.jsp">
<table>
    <tr><td style="color: #FF0000">Passwords entered do not match!</td></tr>
</table>
<br/>
<input type="submit" value="Try Again"/>
</form>

Stop. Doing. This.

You shouldn't duplicate such a tremendous amount of hardcoded html! The only time the construct around this changes, is a success... You could shorthand this, if you stored the error-messages somewhere and fetched them depending on what part of validation failed. That would shorthand that whole block of <% } else if (!...Match) { %> <HTML-STUFF> repetitions to:

<% if (!Validaton.wasSuccessful()) { %>
    <form action="register.jsp">
    <table>
        <tr><td style="color: #FF0000"><% Validation.getMessage() %></td></tr>
    </table>
    <br />
    <input type="submit" value="Try Again" />
    </form>
<% } else { %>
    <form action="login.jsp">
    <table>
        <tr><td>Your registration was successful!</td></tr>
    </table>
    <br/>
    <input type="submit" value="Login"/>
    </form>
<% } %>

Extract your validation into a proper class or just return a little very much less from your servlet. Something like (in JSON):

{ success: "false", messsage: "Emails entered do not match!" }

Storage:

Don't abuse the session. You are terribly abusing the session to store request-scoped results. Instead you may want to strive to make your registration in a Web-Service-ish manner. What you want to do is post your form's data using AJAX, and handle the response on client side.

This takes strain from your server, allows easier modulation and brings me directly to my next point:

Security:

You're sending a password over the network, only "secured" by https... TWICE!

Thats horrible!

Server and Network-load

Having everything run against the server strains the server and the network.
A better way to do something like this is client-side validation: Javascript comes to the rescue.

Here you can check simple constraints like:

• Passwords must match
• Emails must match
• Fields may not be empty

Because this does not help when your new user disables javascript in his browser, you should revalidate these constraints in your service or enforce usage of javascsript by using <noscript>, but back to the main problem:

</interjection>

What you want to do is send the form's data except the passwords to your server, have the server check the constraints and if, and only if the server has no problems, then return success, alongside a user-specific salt.

You take that salt, hash the password with it on client side and send the encrypted password without the salt back to the server.

This is also how you might want to do your login.

Nitpicks:

Some small stuff I found, after now tearing your design apart (sorry for that):

• Don't use inline stylings:

  <td style="color: #FF0000">
  

  should be something like:

  <td class="error-msg">
  

• Properly indent the html you're using:
  After the <form> you should add a level of indentation.

• A <table> is overkill: Instead you could just as well use a <p>

• Make comments count: Some of your comments are superfluous:
  You shouldn't have to comment <!-- registration success -->. If you structure your code clearly comments like this will feel useless. If you need such comments (also in you java-code) you're probably doing something wrong.

That concludes my review.

Answer 2

Comments:

1. You using arrays to replace classes, String[] userDetails doesn't make any sense, you can have a class that represents those details

   class UserDetails{
      String userId;
      String userEmail1;
      String userEmail2;
      String userPassword1;
      String userPassword2;
      String captchaAnswer;
   }
   

   And then you can have a method that constructs objects from request

   private UserDetails fromRequest(HttpServletRequest request){
      ...
   }
   

2. Never write code in your JSP files, it is a horrible idea. Use beans instead. It's much easier to validate your model in the code and not within scriptlets.

3. Use Javascript/jQuery for client side validation