Thursday, September 23, 2010

Dumbassing things up

In a previous entry, Some management servlets, in the section You do know that anyone in the world can delete your datastore, right?, I goofed.

In my defense, the first section of that entry WAS entitled I have no idea what I'm talking about. But it turns out my ignorance knows few bounds.

web.xml, access control, and Guice


I stated that in moving my BuildDB servlet under the control of Guice, I lost the ability to use a security-constraint in the web.xml file to stop the hoi polloi from accessing our servlet. Not so.

Simply have Guice respond to a URL pattern (such as "/*") that includes the security-constraint constrained URL below it, say /admin/*. All of your servlets now are Guicified, and the ones you want protected are protected.

Yeah, shoulda figured that out the first time.

war/WEB-INF/web.xml
<web-app>
 <!-- Servlets -->
 <security-constraint>
  <web-resource-collection>
   <url-pattern>/admin/*</url-pattern>
  </web-resource-collection>
  <auth-constraint>
   <role-name>admin</role-name>
  </auth-constraint>
 </security-constraint>

 <filter>
  <filter-name>guiceFilter</filter-name>
  <filter-class>com.google.inject.servlet.GuiceFilter</filter-class>
 </filter>

 <filter-mapping>
  <filter-name>guiceFilter</filter-name>
  <url-pattern>/*</url-pattern>
 </filter-mapping>

 <listener>
  <listener-class>com.lisedex.voluntickler.server.guice.VolunticklerServletContextListener</listener-class>
 </listener>

 <!-- Default page to serve -->
 <welcome-file-list>
  <welcome-file>index.html</welcome-file>
 </welcome-file-list>
</web-app>

The observant among you will notice that our package is now com.lisedex.voluntickler instead of com.lisedex.volinfoman. It's a sexy name change, I know, and I realize that you're a bit jealous, but the domains are already registered. Sorry.

Saturday, September 4, 2010

Authenticating in GWT

Now that we have some basic ground work done for creating accounts and logging in, we can integrate this into our GWT application. We always need to assume that the client is hostile, that the JavaScript code has been changed, or someone is calling our RPC methods directly. None of our authentication checking should be done in the GWT client, except for stuff that decides which pages should be shown. Any method that then provides data to these pages must still do a proper permissions and authentication check before returning anything.

The code is available on github, and the tag for these revisions is add-gwt-authentication.

No more client passwords


Since we're no longer using GWT to enter the user's login information, we no longer need any of the methods we had that passed entered passwords through RPC, namely checkUserPassword() and authenticatedUser(). The password can be sent once through HTTPS on our login form, and a session cookie is set on the client. That cookie maps to a session stored in both the App Engine datastore, as well as memcache. We can use the session under App Engine to store information about the session, such as whether the user is authenticated, and no client program can change the value without going through an RPC call.

So our UserService RemoteService is stripped down to an isAuthenticated() method.

src/com/lisedex/volinfoman/server/UserServiceImpl.java
        @Override
        public boolean isAuthenticated() {
                return SessionHandler.isAuthenticated(getSession());
        }

        private HttpSession getSession() {
                return this.getThreadLocalRequest().getSession();
        }

When we call the isAuthenticated() method, if our session cookie is set, it will be passed to the server in the Cookie header.

Oops. I just made a security hole.


Writing this rattled something loose in the back of my brain, which is apparently the Login Security FAQ for GWT.

The server should not rely on the session ID that comes in the Cookie header, as doing so opens up the cross-site request forgery (XSRF or CSRF) vulnerability.

In short, if the server relies on the Cookie header for the session, and the user's session has not expired, then another site could have a link to something like http://www.vulnerablesite.com/user?deleteMe, which could be even hidden as an <img src> tag. The user's session cookie would automatically be sent with the deleteMe request, and the vulnerable server would go ahead and process the request. If we require the session to be submitted in the RPC payload, the link would have to be something like http://www.vulnerablesite.com/user?deleteMe?sessionId=787asdfasdf. Since the evil site has no way to access the user's session cookie, it can't generate a usable link. Hole closed.

So let's rewind, patch the hole, and do this again. The new code is available under the fix-csrf-hole tag on github.

Fixing the CSRF vulnerability


In looking for a pretty solution to this issue, I came across references to CSRF being addressed in GWT 2.1. The current RpcRequestBuilder in 2.0.4 adds a custom header, X-GWT-Permutation, and it looks like the code in 2.1 at least checks if this header is set. But it doesn't do anything beyond an existence test, on the idea that CSRF attacks can't add custom HTTP headers. However, apparently by using Adobe Flash, you can add custom headers. It would appear that this simple test is not enough. But since 2.1 is not yet out, I'm not sure if this is the extent of what will be checked. Assuming that Google will probably handle it correctly in the end, I decided not to spend a ton of time architecting an elegant solution.

src/com/lisedex/volinfoman/client/data/UserService.java
       boolean isAuthenticated(final String sessionId);

All of my UserService RPC methods will also pass a copy of the JSESSIONID cookie in its parameters. The servlet will then compare the passed copy with the header cookie, and if they match, we assume its a valid RPC call. There's still a possibility that someone could sniff the cookie, but since this software is not exactly financial software, I'm pretty comfortable with the level of security this adds. More elaborate security could be added by using a nonce, or maybe a hash across the session and the command name.

src/com/lisedex/volinfoman/server/UserServiceImpl.java
        public boolean isAuthenticated(final String sessionId) {
                if (validateSessionId(sessionId)) {
                        return SessionHandler.isAuthenticated(getSession());
                } else {
                        LOG.severe("Possible CSRF.  JSESSIONID cookie does " +
                           "not match included session id");
                        return false;
                }
        }

        private HttpSession getSession() {
                return this.getThreadLocalRequest().getSession();
        }

        private boolean validateSessionId(String sessionId) {
                String headerSessionId = (String) getSession().getId();
                if (sessionId != null && headerSessionId != null
                                && headerSessionId.equals(sessionId)) {
                        return true;
                }
                return false;
        }

In each RPC method, we call validateSessionId, which gets the session id from the HttpServletRequest, and compares it with the passed session id. Assuming they match, the RPC method will continue.

src/com/lisedex/volinfoman/server/SessionHandler.java
        public static final String AUTHENTICATED = "authenticated";

        public static void setAuthenticated(HttpSession session,
                        boolean authenticated) {
                session.setAttribute(AUTHENTICATED, authenticated);
        }

        public static boolean isAuthenticated(HttpSession session) {
                if (session == null)
                        return false;

                Boolean auth = (Boolean) session.getAttribute(AUTHENTICATED);
                if (auth == null) {
                        return false;
                } else {
                        return auth.booleanValue();
                }
        }

In the case of isAuthenticated, we call SessionHandler.isAuthenticated, a static method that operates on the HttpServletRequest session, and just gets the value of the authenticated attribute stored in the session table in the datastore.

In our Login servlet, we call the setAuthenticated() method to mark the session as authenticated if the login is successful, and before redirecting the user to the GWT application.

The new home page, MainPageView, is just a SimplePanel, which should allow us to do a very small application load before checking whether the session is authenticated. If it is, we can put a new widget which contains the actual layout in the panel.

src/com/lisedex/volinfoman/client/MainPagePresenter.java
    private static final String HOME_PAGE_URL = "/index.html";

    private UserServiceAsync userService = null;

    @Override
    public void bind() {
        view.setContent(new HTMLPanel(LOADING_MESSAGE));
        userService.isAuthenticated(
                Cookies.getCookie(Volinfoman.SESSION_COOKIE),
                    new AsyncCallback<Boolean>() {
                        @Override
                        public void onSuccess(Boolean result) {
                                if (result.booleanValue() == true) {
                                        view.setContent(new HTMLPanel(
                                            "<h1>AUTHENTICATED</h1>"));
                                } else {
                                        view.setContent(new HTMLPanel(
                                                NOT_AUTHENTICATED_MESSAGE));
                                        Window.Location.replace(HOME_PAGE_URL);
                                }
                        }

                        @Override
                        public void onFailure(Throwable caught) {
                                view.setContent(new HTMLPanel(RPC_FAILURE_MESSAGE));
                        }
                });
    }

We pop a loading message in as the widget in our SimplePanel, and call the RPC isAuthenticated method. Cookies.getCookie is a static method which gives us access to the browser's cookies for our site. If we get a successful response for our RPC method, and it turns out that our session is marked as authenticated, we put up another message just informing us that we authenticated successfully. In the future, we'd set the SimplePanel's widget to some sort of layout panel, and set up our application. If our session is not marked as authenticated, we redirect the browser back to the login page. The Window class gives us access to some of the user's browser's properties and events. In this case, Window.Location allows us to change the current URL.

Paying attention


This definitely shows me how, if I'm not paying attention, security lapses can easily slip right by. Writing the blog entries actually helps me reparse the stuff that I've written, and catch some of my mistakes before I move on, but even then I almost blew right past this one.

The Live HTTP Headers plugin for Firefox made testing and playing with this vulnerability really easy. I could simply catch a remote call, and then replay it with whatever modifications I wanted to make: header modifications or deletions, payload changes. It's pretty smooth. I also made some packet captures, but this plugin is far easier.

I think it's time to start making some sort of user configuration wizard, where people can set up their volunteering preferences, or, if they happen to be a volunteer coordinator, the volunteer opportunities for their organization.

Some management servlets

So I got StarCraft II, and let's just say I've spent a lot more time with that than I have with this project, what with playing, watching Day9 Daily archives, reading strategy...

And I'm still in the bronze league. You may now commence laughing.

But a couple of days ago, I picked this stuff back up. Leaving it for so long is a mistake, as it always takes me a while to remember what's what, where I was, and where I was going. I messed around with the interface, trying to get a very basic layout with a working login page.

I learned a few important lessons, like wrapping widgets with SimplePanels so they could be swapped out easily with just a setWidget() as the interface required, and my previous understanding, as explained in my last entry, of bind() from mvp4g was off. But I learned a much more important lesson.

I have no idea what I'm talking about


OK, that's maybe a bit harsh, but really, I think I was coming at this with a fundamental misconception about how it'll work. I was trying to build an application where you could login, or register for the site, or request a reset password, or actually use the site for its intended purpose basically without leaving the layout and original landing page. Lunacy. Trying to fit the project to the tool, while not understanding the tool all that well, either.

At first, I realized things should work more like Gmail, where your initial login and registration could be handled on some regular pages, and then you move in to the application itself. Flipping this switch in my brain alone allowed me to start making much faster progress (obviously helped along because I had moved back to a more familiar paradigm). Taking that lesson more generally, I shouldn't force this thing into a single page where widgets are swapped in and out. If it makes more sense, I should be willing to just move to another page, or sprinkle parts of GWT throughout pages, unless there's too much of a startup penalty.

In short, I don't need to write this as if it were some monolithic desktop application.

On that note, I'm going to focus in this post on the account registration, email confirmation, App Engine cron jobs. At this time, these all use GAE, but not GWT. You'll just have to ignore the GWT client code for now, as it's in a screwy state that compiles and renders a page, but doesn't do anything remotely useful.

As always, the code is available at github under the add-static-login tag.

Moving the starting line


The first step was moving the home page, and having it basically static with a login form and a link to the registration page.

war/WEB-INF/web.xml
<!-- Default page to serve -->
        <welcome-file-list>
               <welcome-file>index.html</welcome-file>
        </welcome-file-list>

I can keep the original Volinfoman.html for later, and this automatically solves one of the issues I had been thinking about, but had put off for later: auto-complete not working in GWT login pages, and using HTTPS to submit the user's password.

The link to the registration page leads to another simple, static form, register.html. This submits to a servlet which is written using App Engine, Register.

The making of a servlet


We need to register this servlet with Guice so that injection works.

src/com/lisedex/volinfoman/server/guice/VolinfomanGuiceModule.java
bind(Register.class).in(Singleton.class);

src/com/lisedex/volinfoman/server/guice/VolinfomanServletModule.java
serve("/volinfoman/register").with(Register.class);

In our Guice module, we configure Register as a Singleton, and in Guice's servlet configuration, we let it know that the URL /volinfoman/register should be sent to Register for handling. This is, of course, the action we use for the form in register.html.

src/com/lisedex/volinfoman/server/authenticate/Register.java
public class Register extends HttpServlet {
        @Inject
        private Dao dao;

        private static final Logger log = Logger.getLogger(Register.class.getName());

        @Override
        public void doGet(HttpServletRequest req, HttpServletResponse resp)
                        throws IOException {

                PrintWriter output = resp.getWriter();

                // build HTML response page
                resp.setContentType("text/html");
                resp.setCharacterEncoding("utf-8");
                output.println("<head><title>Add initial datastore information</title></head>");
                output.println("<body>");

                String username = req.getParameter("username");
                String firstName = req.getParameter("firstName");
                String lastName = req.getParameter("lastName");
                String password = req.getParameter("password");
                String email = req.getParameter("email");

                if (!StringSafety.isSafe(username)) {
                        output.println("<span style=\"color: #ff0000;\">Username bad, please go back and enter it again</span>");
                        output.println("</body>");
                        return;
                }

                if (!StringSafety.isSafe(firstName)) {
                        output.println("<span style=\"color: #ff0000;\">First name bad, please go back and enter it again</span>");
                        output.println("</body>");
                        return;
                }

                // ... etc, etc. for input safety ...

                if (dao.getUser(username) != null) {
                        output.println("<span style=\"color: #ff0000;\">Username already exists, please go back and enter it again</span>");
                        output.println("</body>");
                        return;
                }

This first pass at the servlet is pretty ugly, with plenty of string literals, but what we're doing is laying the groundwork for the registration by collecting the user's input.

We extend javax.servlet.http.HttpServlet and override its doGet() method, since at this point we have not declared a method for our HTML form to use and it defaults to GET. Later, we'll change this to doPost() since the user is entering their password in the form, and we don't really need that showing up in the address bar of their browser. Or our server logs.

We start setting up the page that gets sent back to the user when the servlet completes, by filling out the HttpServletResponse object passed in as resp. After setting what type of page we'll be returning, we print to the object line-by-line in the order we would want to build a standard HTML page.

We need to get the user's input, so we use the getParameter method of HttpServletRequest, which parses the form data for us, and hides the logic needed to handle both GET and POST forms. After we get the input, we want to protect ourselves from things like injection attacks, so we have to check each piece of input that comes from outside of the servlet. The static StringSafety.isSafe() method right now is terribly simple, just looking for ; and & characters. Later, we'd want to do better verification, like making sure the username and password meet whatever requirements we have and that the email address looks basically valid.

If any of the input doesn't meet our standards, we spit out an error message and abort the servlet. I check each separately, so I can inform the user of which field doesn't meet our standards. Ideally, instead of forcing the user to go back, we'd re-render the form, with the error message integrated into it, but right now we're doing it the simple way. I removed some of the repetitious statements for brevity; see the code for the full version.

Finally, we make sure that the username requested doesn't already exist. This seems like an expensive way to look this up, and is also open to another account with the same username getting created between that test and the next lines of code.

User user = new User(null, username, 
                    User.STATUS_UNCONFIRMED, firstName, lastName, 
                    email, password);
                dao.putUser(user);

We put the user in the datastore, with a status of User.STATUS_UNCONFIRMED. This status indicates that the account has been created, but the user has not yet clicked the link included in the email we're about to send them. This code has a bug, in that the password is not passed through BCrypt before storage. We'll fix this as we come back and fix this class up.

Properties props = new Properties();
                Session session = Session.getDefaultInstance(props, null);

                String msgBody = "Thank you for registering a VolunteerIM "
                   + "account!  Please follow the link below to confirm "
                   + "your account:\n\n";
                Random r = new Random();
                msgBody += "http://lisedexvolinfomantest/volinfoman/emailConfirm?code="
                    + Long.toString(Math.abs(r.nextLong()), 36)
                    + "\n\n";
                msgBody += "Note: Please do not reply to this address, as "
                    + "email is thrown away.  If you did not set up a "
                    + "VolunteerIM account, please ignore this email, as the "
                    + "account will be removed automatically in a week.\n";

                try {
                    Message msg = new MimeMessage(session);
                    msg.setFrom(new InternetAddress("admin@lisedex.com", 
                        "VolunteerIM Confirmation"));
                    msg.addRecipient(Message.RecipientType.TO,
                             new InternetAddress(email, firstName + 
                                 " " + lastName));
                    msg.setSubject("VolunteerIM account confirmation");
                    msg.setText(msgBody);
                    Transport.send(msg);
                } catch (AddressException e) {
                    output.println("Bad email address.  Please try again. " +
                        e.toString() + "</body>");
                    log.info("AddressException sending confirmation email: " +
                        e.toString());
                    return;
                } catch (MessagingException e) {
                    output.println("Error sending confirmation email.  Please try again. "
                        + e.toString() + "</body>");
                    log.info("MessagingException sending confirmation email: "
                        + e.toString());
                    return;
                }

        output.println("We have sent a confirmation email to " 
             + email + ".  It should arrive shortly.  As soon as you receive " 
             + "it, please <a href=\"/\">return to the front "
             + "page to log in.</a>");
        }
}

Google's App Engine has an API for sending emails which uses the JavaMail API, but we just use a tiny subset.

We need a reference to a mail Session, which we use to build a MIME compatible email. First, we build a quick message body, which contains a link with a random string that we'll use as a confirmation code.

When we set the address that the message will come from, it has to be set as an email address that is listed in the App Engine dashboard as a collaborator on the project. I found this the hard way, after a MessagingException kept getting thrown during testing. The code worked fine on the local development server, but when deployed it choked.

Then we specify the recipient and subject, and attach the message body we already generated.

We can still get an exception when sending the email, if we use a To address App Engine doesn't like, or, as I found out, a From address that's not registered. If one of these pop up, we generate an error for the user, and exit the servlet. At this stage of the code, this will leave an UNCONFIRMED account in the datastore, with no confirmation email sent. Whooops.

If everything's gone according to plan, we let the user know that they should keep an eye out for the confirmation email.

Of course, right now, if the user were to click on the link in the email, we'd have to return a 404, since there's no servlet registered at /volinfoman/emailConfirm.

OK, do you at least have an email account you can read?


Before that, let's change the registration form to use the POST method. The GET method puts all of the user's input into the URL, which include the user's password in plaintext. So not only does it show in their address bar, it will live on forever in the server logs. Fortunately, HttpServlet makes this easy: just add method="post" to the form declaration in register.html, and change the doGet() method in Register to doPost(). The class library handles the details.

Now, we want to make sure that the email address the user entered is linked to an account they can read, like every other website in existence with individual accounts. We're sending a link with a code embedded to the user after they register, so we'll need a servlet to manage confirming the accounts. We could put the confirmation code for each user into the User class, and have the datastore index it so we could run a query, but we also want to expire accounts that haven't been confirmed after some length of time. So then we'd need to add an indexed expiration date field to the User class, and it starts to look cleaner to create a datastore table just for this purpose.

src/com/lisedex/volinfoman/shared/ConfirmationCode.java
public class ConfirmationCode implements Serializable {
        @Id
        private Long id;

        @Indexed
        private String username;

        @Indexed
        private String code;

        @Indexed
        private long expires;

        public ConfirmationCode() {
        }

        /**
         * Constructor
         * @param id Datastore primary key
         * @param username username associated with code
         * @param code confirmation code associated with username
         * @param expires expiration date for code in milliseconds
         */
        public ConfirmationCode(Long id, String username, String code, long expires) {
                setId(id);
                setUsername(username);
                setCode(code);
                setExpires(expires);
        }

        public Long getId() {
                return id;
        }

        public void setId(Long id) {
                this.id = id;
        }

        public String getUsername() {
                return username;
        }

        // ... etc, etc ... it's getters and setters all the way down
}


It's a pretty bare bones data structure, though we should probably add some input verification later to the setters. We also index all of the elements of the table. At different times we'll need to search by the code, and we'll also need to query for expiration times that are older than a certain time. Currently, the username does not need to be indexed, but we'll leave it for now.

In the earlier blog entry about Guice, I talked some about a BuildDB class which could delete all Users in the datastore, and just leave one admin account. Well, we also want the option of wiping out our confirmation code table while developing, which requires a new method in our DAO. We declare it in the Dao interface, and implement it in DaoGaeDatastore.

src/com/lisedex/volinfoman/server/DaoGaeDatastore.java
@Override
        public void deleteUser(Long id) {
                ofy().delete(User.class, id.toString());
        }

        @Override
        public void putConfirmationCode(ConfirmationCode code) {
                ofy().put(code);
        }

        @Override
        public void deleteAllConfirmationCodes() {
                ofy().delete(ofy().query(ConfirmationCode.class).fetchKeys());
        }

While we're in there, we added a couple of other methods that we'll use to work with the confirmation codes right now. We'll need a way to delete the users that we put into the datastore, if we get an exception during sending them their confirmation email. When the exception is caught, we want to throw away the new User object we put in the datastore, so the user can reuse it when they resubmit their registration. When calling this, we'll already have the User object, which includes the Id field, so we won't need to run a query; we can just straight up delete using an Objectify convenience method that builds a datastore Key from the provided class and its Id.

We'll also need to store new ConfirmationCode objects, and the deleteAllConfirmationCodes() method is pretty much ripped from the deleteAllUsers() method verbatim (just changing the class).

We also need to register the ConfirmationCode class with Objectify, so it knows how to handle it. We do this in the same place we register User: a static constructor in our DAO implementation.

ObjectifyService.register(ConfirmationCode.class);

Now, in our BuildDB servlet, we can simply call dao.deleteAllConfirmationCodes when we want, and clean out the table. "Dangerous Database Deletions" are my middle names.

Now, in our Register servlet, we want to add the confirmation code to the datastore.

src/com/lisedex/volinfoman/server/authenticate/Register.java
public static final int EXPIRATION_FIELD = Calendar.DATE;
    public static final int EXPIRATION_INCREMENT = 7;

We'll use the Java Calendar class to calculate our expiration date. This gives us flexibility to easily change the delay without having to recalculate how many milliseconds it is. The EXPIRATION_FIELD says which field we'll be incrementing, where DATE is days, but we could also use MONTH or HOUR or even YEAR. Calendar will handle the rollover and carrying the ones and the leap years and leap seconds and all of that. The EXPIRATION_INCREMENT, surprisingly enough, is how much that field will be incremented by.

Random r = new Random();
                String code = Long.toString(Math.abs(r.nextLong()), 36);
                Calendar expirationTime = Calendar.getInstance();
                expirationTime.add(EXPIRATION_FIELD, EXPIRATION_INCREMENT);

                ConfirmationCode confCode = new ConfirmationCode(null, username, 
                    code, expirationTime.getTimeInMillis());
                dao.putConfirmationCode(confCode);

We generate the confirmation code before the message body so we can insert it into the ConfirmationCode table in the datastore. Calendar.getInstance() gets a Calendar object representing the current time, and we add however much time we'd like to make our expiration time in milliseconds.

In both of the exception handlers for email transmission errors, we add a line to delete the users we just added to the datastore.

dao.deleteUser(user.getId());

You do know that anyone in the world can delete your datastore, right?


In the Guice blog entry, we moved BuildDB under control of Guice so we could inject our DAO implementation. In doing so, we lost the ability to use the App Engine user authentication to control access to the servlet. As we get further along, this becomes unacceptable, but I'm still not ready to build my own authentication mechanism into the servlet. We'll also need the authentication for protecting our cron jobs, so fixing this is not a waste.

In BuildDB, we no longer inject the Dao, we directly instantiate a DaoGaeDatastore. In the VolinfomanGuiceModule and VolinfomanServletModule, we pull out our definitions for BuildDB (and CacheStats, another servlet that I don't believe works at the moment).

war/WEB-INF/web.xml
<filter-mapping>
            <filter-name>guiceFilter</filter-name>
            <url-pattern>/volinfoman/*</url-pattern>
    </filter-mapping>


    <security-constraint>
        <web-resource-collection>
            <url-pattern>/admin/*</url-pattern>
        </web-resource-collection>
        <auth-constraint>
            <role-name>admin</role-name>
        </auth-constraint>
    </security-constraint>

    <servlet>
        <servlet-name>BuildDB</servlet-name>
        <servlet-class>com.lisedex.volinfoman.server.admin.BuildDB</servlet-class>
    </servlet>

        <servlet-mapping>
                <servlet-name>BuildDB</servlet-name>
                <url-pattern>/admin/builddb</url-pattern>
        </servlet-mapping>

In web.xml, we will change the Guice url-pattern from /* to /volinfoman/*, to allow us some granularity in defining which servlets get processed through Guice. To get BuildDB out of Guice's control, we'll need to change its URL from /volinfoman/admin/builddb to /admin/builddb.

We also set a security-constraint on the url-pattern /admin/* which requires the user have an administrator role for the application. If the user is not logged in, they will be presented with a login form from Google that allows them to log in with whatever account they've used as a collaborator for the project. If their Google account is not registered as a collaborator on the project, they will get an error and the servlet will not run. This is also used to protect cron jobs, as they will run as if run by an authenticated admin user.

Finally, we set the url-pattern that configures what URLs BuildDB processes. Since /admin/builddb matches the /admin/* security-constraint definition, it's protected by Google's authentication.

Get rid of expired confirmation codes


We want to run a cron job that will find any expired confirmation codes, and remove them. It will also need to look at the User associated with the code, and if it's still in the User.UNCONFIRMED state, the User needs to be deleted as well. The reason we double check this, is that I can imagine a time when someone emails support, and support confirms their account, but forgets to remove the associated confirmation code. If we didn't check the User's status, we could delete a confirmed account.

@Override
        public void expireCodesBefore(long now) {
                Query<ConfirmationCode> oldCodes = 
                    ofy().query(ConfirmationCode.class).
                        filter("expires <", now);
                for (ConfirmationCode code: oldCodes) {
                        User user = getUser(code.getUsername());
                        if (user != null) {
                                // make sure user is still in unconfirmed state
                                if (user.getStatus() == User.STATUS_UNCONFIRMED) {
                                        deleteUser(user.getId());
                                }
                        }

                        deleteConfirmationCode(code);
                }
        }

        @Override
        public ConfirmationCode getConfirmationCode(String code) {
                ConfirmationCode fetched = 
                    ofy().query(ConfirmationCode.class).
                        filter("code", code).get();
                return fetched;
        }

        @Override
        public void deleteConfirmationCode(ConfirmationCode code) {
                if (code != null) {
                        ofy().delete(code);
                }
        }


The expireCodesBefore() method takes a time, in milliseconds, and deletes all confirmation codes with an expiration time that comes before that time. This can be tested with a relational operator: if the expiration time is less than the time provided, it's expired.

The query does just that, and the query itself is Iterable, so we walk through it. For each expired code, we get the username associated with it, and pull in the User by that name. If the user exists, and the user's status is still unconfirmed, we delete the user. The expired confirmation code is deleted regardless of the user's state.

getConfirmationCode() will retrieve a confirmation code by the code field, instead of by Id. To do this, we need to run a query. Since it should only return one hit, we only return the first one found. If there were multiple entries with the same username, we'd only work with the first one.

Lastly, we add a method to delete confirmation codes.

In the Register servlet, we move most of the string literals scatted throughout the code into static Strings at the top of the class. This includes parameter names that are submitted by the registration form, as well as information used to build the email.

src/com/lisedex/volinfoman/server/authenticate/Register.java
User user = new User(null, username, User.STATUS_UNCONFIRMED, 
                firstName, lastName, email, null);
            dao.changeUserPassword(user, password);

We now build the User with a null password, and insert the User into the datastore using the password change method. This allows us to hash the password before storing it, fixing the bug we introduced in the early version of the code.

src/com/lisedex/volinfoman/server/cron/ExpireConfirmationCodes.java
public class ExpireConfirmationCodes extends HttpServlet {
        private Dao dao = new DaoGaeDatastore();

    private static final Logger log = Logger.getLogger(ExpireConfirmationCodes.class.getName());

        @Override
        public void doGet(HttpServletRequest req, HttpServletResponse resp)
                throws IOException {

                Calendar now = Calendar.getInstance();
                log.info("Expiring expiration codes at " + Long.toString(now.getTimeInMillis()));

                dao.expireCodesBefore(now.getTimeInMillis());
        }
}

The ExpireConfirmationCodes cron job is just a stripped down servlet based on BuildDB. Since it accepts no parameters, we simply get the current time, and pass its value in milliseconds to the DAO method we wrote above for this purpose. Note that we're directly instantiating the DaoGaeDatastore again, as we need to bypass Guice since we need to put our cron jobs behind a security-constraint.

war/WEB-INF/web.xml
<!-- Cron jobs -->
        <security-constraint>
        <web-resource-collection>
            <url-pattern>/cron/*</url-pattern>
        </web-resource-collection>
        <auth-constraint>
            <role-name>admin</role-name>
        </auth-constraint>
        </security-constraint>
  
        <servlet>
                <servlet-name>expireConfirmationCodes</servlet-name>
                <servlet-class>com.lisedex.volinfoman.server.cron.ExpireConfirmationCodes</servlet-class>
        </servlet>

        <servlet-mapping>
                <servlet-name>expireConfirmationCodes</servlet-name>
                <url-pattern>/cron/expireConfirmationCodes</url-pattern>
        </servlet-mapping>

We add /cron/* URLs to what we hide behind Google's authentication, and define the URL /cron/expireConfirmationCodes as the one which invokes our expiration servlet.

war/WEB-INF/cron.xml
<cronentries>
        <cron>
                <url>/cron/expireConfirmationCodes</url>
                <description>Expire confirmation codes every day</description>
                <schedule>every day 02:00</schedule>
        </cron>
</cronentries>

The cron job definition specifies what URL gets called at what frequency. If you use a frequency like "every 5 minutes", the job is started 5 minutes after the last job completed. For example, if the job took 1 minute to run, the first job would start at 01:00, the next run at 01:06, the next at 01:12, and so on. If you wanted the jobs to start at 01:00, 01:05, and 01:10, you would need to add the keyword synchronized to the schedule.

Lastly (aka FINALLY), we can process the confirmation link and login form


This has stretched a bit longer than I expected, but it's almost over.

src/com/lisedex/volinfoman/server/authenticate/ConfirmationCodeChecker.java
public class ConfirmationCodeChecker extends HttpServlet {
        private static final String ACCOUNT_NEW = "Sorry, the account you're trying" +
           " to confirmed has not reached the point where it can be confirmed.  " +
           "Unfortunately, the only way to resolve this is to contact our support" +
           " department, or <a href=\"/register.html\">apply for a new account</a>." +
           "  We apologize for the inconvenience.";

        private static final String ACCOUNT_INVALID = ""; // another message
        private static final String ALREADY_CONFIRMED = ""; // another message
        private static final String ACCOUNT_CLOSED = ""; // another message
        private static final String BAD_CONFIRMATION_CODE = "";
        private static final String CONFIRMATION_SUCCESS = "";

        @Inject
        private Dao dao;

        private static final Logger log = Logger
                        .getLogger(ConfirmationCodeChecker.class.getName());

        private static final String UNSAFE_ERROR = "<span style=\"color: #ff0000;\">" +
            "There is a problem with the link used to confirm your user account." +
            "  Please try clicking the link again, and if the problem continues, " +
            "return to the home page and request that the email is sent again.  " +
            "Or you can contact support.  Sorry for the inconvenience!</span>";

        @Override
        public void doGet(HttpServletRequest req, HttpServletResponse resp)
                        throws IOException {
                // standard stuff
                output.println("<head><title>VolunteerIM account confirmation</title></head>");
                output.println("<body>");

                output.println("<h2>VolunteerIM account confirmation</h2><p>");

                String username = req.getParameter("username");
                String code = req.getParameter("code");

                log.info("Confirmation request with username " + username
                                + " and code " + code);

                if (!StringSafety.isSafe(username)) {
                        output.println(UNSAFE_ERROR + "</body>");
                        return;
                }

                if (!StringSafety.isSafe(code)) {
                        output.println(UNSAFE_ERROR + "</body>");
                        return;
                }

                ConfirmationCode testCode = dao.getConfirmationCode(code);
                if ((testCode == null) || (testCode.getUsername() == null)
                                || (testCode.getCode() == null)) {
                        output.println(BAD_CONFIRMATION_CODE + "</body>");
                        return;
                }

                User fetched = dao.getUser(testCode.getUsername());

                if (fetched.getStatus() == User.STATUS_UNCONFIRMED) {
                        fetched.setStatus(User.STATUS_CONFIRMED);
                        dao.putUser(fetched);

                        // eliminate confirmation code from datastore, as we're
                        // done with it
                        dao.deleteConfirmationCode(testCode);

                        output.println(CONFIRMATION_SUCCESS + "</body>");
                        log.info("Confirmed username " + fetched.getUsername());
                        return;
                }

                if (fetched.getStatus() == User.STATUS_CLOSED) {
                        output.println(ACCOUNT_CLOSED + "</body>");
                        return;
                }

                if (fetched.getStatus() == User.STATUS_CONFIRMED) {
                        output.println(ALREADY_CONFIRMED + "</body>");
                        return;
                }

                if (fetched.getStatus() == User.STATUS_INVALID) {
                        output.println(ACCOUNT_INVALID + "</body>");
                        return;
                }

                if (fetched.getStatus() == User.STATUS_NEW) {
                        output.println(ACCOUNT_NEW + "</body>");
                        return;
                }
        }
}

This servlet is under Guice's control, so its URL is defined in VolinfomanGuiceModule and VolinfomanServletModule. In this case it's /volinfoman/emailConfirm, as specified in the email sent to the user.

We define a bunch of messages that may get sent to the user, for different states of the User status, or malformed URL parameters. We get both a username and code from the URL, which we check for bad data, and also use to cross check whether the code matches the specified username. If it does, we pull up the User, and based on its status, we return different messages.

If the User was previously unconfirmed, we tell them it's now confirmed, we mark it as such in the datastore, and they can now log in from the home page. If it was closed, we let them know they can register for a new account, or contact support to find out why it was closed. Already confirmed Users get told they can go ahead and log in, and invalid and new users are told to contact support or register new accounts. In the code above, I've stripped the messages since they're pretty long, but they're available in the code in the repository at github.

src/com/lisedex/volinfoman/server/authenticate/Login.java
public class Login extends HttpServlet {
        @Inject
        Dao dao;

        private static final Logger log = Logger.getLogger(Login.class.getName());

        @Override
        public void doPost(HttpServletRequest req, HttpServletResponse resp)
                        throws IOException {

                // standard stuff

                String username = req.getParameter("username");
                String password = req.getParameter("password");

                if ((username == null) || (password == null)) {
                        output.println("<head><title>VolunteerIM login</title></head>");
                        output.println("<body>Please fill out both username and " +
                            "password fields.</body>");
                        return;
                }

                if (!StringSafety.isSafe(username)) {
                        output.println("<head><title>VolunteerIM login</title></head>");
                        output.println("<body>Username invalid, please go back and " +
                            "try again.</body>");
                        return;
                }

                if (!StringSafety.isSafe(password)) {
                        output.println("<head><title>VolunteerIM login</title></head>");
                        output.println("<body>Password invalid, please go back and try" +
                            " again.</body>");
                        return;
                }

                if (dao.checkUserPassword(username, password)) {
                        if (dao.getUser(username).getStatus() == User.STATUS_CONFIRMED) {
                                HttpSession session = req.getSession();
                                resp.sendRedirect(resp.encodeRedirectURL("/Volinfoman.html"));
                                session.setAttribute(Session.AUTHENTICATEDUSER, username);
                                return;
                        } else {
                                output.println("<head><title>VolunteerIM login</title></head>");
                                output.println("<body>INSERT CONFIRMATION MESSAGE HERE</body>");
                                return;
                        }
                } else {
                        output.println("<head><title>VolunteerIM login</title></head>");
                        output.println("<body>Username and password do not match." +
                            "  Please try again.</body>");
                        return;
                }
        }
}

For the login form, we have a Login servlet. I've stripped out some messages and some of the boilerplate code I've been using in all of these servlets. We check the submitted username and password for safety, and then pass them to our previously (way back in the day) developed password checking code in the DAO. If it's a match, we also check the User's status to make sure it's been confirmed. If it has, we set a cookie with the HTTP session identifier, mark it in App Engine's session table as an authorized user, and redirect them to /Volinfoman.html, which is the entry point to our GWT application. Currently, there is no testing whether the browser visiting /Volinfoman.html is authenticated, so anyone can visit it directly, but this will be rectified in the next bit of code.

Whew


I realize this is way too long, and maybe showing the revisions as I was figuring this stuff out is not helpful, but I wanted to give a decent introduction to some of the code that will run the site behind the scenes. I've been focusing almost exclusively on GWT code, and as I described above, I'm realizing something that I should have realized a long time ago (except I was blinded by GWT's novelty). I should have realized that GWT's not everything.

Friday, July 23, 2010

Unable to install breakpoint due to missing line number attributes

I was happily coding along, and didn't change anything in the Eclipse configuration, just a few lines of code since the last time, and hit F11 to start debugging. I had set a breakpoint to see the output of the BCrypt hashing implementation I put in.

An error dialog popped up with the message:

Unable to install breakpoint in 
com.lisedex.volinfoman.server.admin.BuildDB$$FastClassByGuice$$22710124
due to missing line number attributes.  Modify compiler
options to generate line number attributes.

Reason:
Absent Line Number Information

Crap. Well, I didn't change anything, but I thought maybe I hit some key combination that turned off line numbers? I checked Window -> Preferences -> Java -> Compiler, but sure enough, the line number checkbox was ticked. So I unticked it, applied the settings, came back and re-ticked it, but no luck. Did some quick searches, and people recommended checking that option, or adding -g to the compiler options, but that doesn't really seem to be an option here.

Added a bunch of logging to look at what I wanted to see, and went ahead and ran the program anyways, OKing through the dialog.

It stopped at my breakpoint.

I tried this in several other places in my code, and they all work. I still get the error, but it doesn't appear that the problem actually exists.

So if you see this error, my suggestion is, before wasting a bunch of time trying to fix it, see if it's actually true. You may just be able to check the box for it to not notify you, and go on with your life.

Thursday, July 22, 2010

Converting to the MVP architecture

If you see one thing referenced again and again in writings about GWT development, it's Ray Ryan's Google IO 2009 talk on GWT Architecture Best Practices. In the talk he describes using the MVP architecture to develop GWT applications that allow easier unit testing and clean logic separation. There's also a couple of articles about large scale application development and MVP also at Google Code. And there's a million other things you can read with a quick search.

I've used the MVC architecture for other projects, and it's treated me pretty well, but you can still end up with some code that's difficult to follow from beginning to end. This is generally because of the View cutting the Controller out of the loop in going to the Model. MVP looks like it could help resolve that, so I wanted to give it a shot while learning GWT.

Finding a framework


I also didn't want to do all of the work involved in developing my own framework to support MVP, so after looking around to see what open source projects are available, I came across gwt-presenter. Several blogs and examples talk about this framework, and early on I had decided that I'd use it for this project. But now that I'm at the stage where I want to implement it, I looked a little closer.

It doesn't seem to be in active development. The last release was in August 2009, and GWT 2.0 was not yet released. There are two branches, and the GAE/GWT development blog talks about using the "replace" branch, but I'm not sure what stage it's at.

Though this may be due to my own prejudices, I preferred something that's currently being hacked, and I found that in mvp4g. It also looks to have some more features without being bogged down, have some more complete documentation, uses JUnit with pretty high coverage, and a discussion group where the developer regularly responds to questions. So I'm giving it a shot, at least the 1.2.0 snapshot (found in the examples zip), since it supports Gin.

Moving to mvp4g


The code for this stage is available under the "add-mvp4g" tag on github, or in a zip download.

I'm going to convert the login page that we currently have working (well, working is probably a bit too strong) to using this framework. The mvp4g FAQ has some good, quick explanations of their architecture, including a class diagram that I didn't find terribly useful until I got into implementing it.

The first step, as usual, is to include the jar in your classpath, as well as commons-lang and commons-configuration. We also need to inherit it in our .gwt.xml module file.

com/lisedex/volinfoman/Volinfoman.gwt.xml
<!-- Mvp4g configuration -->
       <inherits name='com.mvp4g.Mvp4gModule'/>

We fire it up in our entry point class.

com/lisedex/volinfoman/client/Volinfoman.java
// code to initialize mvp4g, which handles setting up our
      // Ginjection
      Mvp4gModule module = (Mvp4gModule)GWT.create( Mvp4gModule.class );
      module.createAndStartModule();
      RootPanel.get().add( (Widget)module.getStartView() );

If you prefer to use a layout panel for your application, the last line can use RootLayoutPanel instead of the RootPanel.

Event bus


Most of the initial configuration is done through annotations in our event bus class, so we'll set that up next; apparently you can also set mvp4g up through an XML file, but I didn't play with that.

com/lisedex/volinfoman/client/VolinfomanEventBus.java
@Events(startView = LoginView.class, historyOnStart = true,
        ginModule=VolinfomanModule.class)
@Debug( logLevel = LogLevel.DETAILED, logger = 
        Mvp4gLoggerToGwtLogAdapter.class )
public interface VolinfomanEventBus extends EventBus {
        @Event(handlers = LoginPresenter.class)
        public void login();
}

The @Events annotation declares what View our application will start in, whether it will parse any history information embedded in the URL that starts the application (we currently don't use this, but I have it enabled anyways), and what Gin module, if any, we want it to use to configure a Ginjector. Previously we defined the Ginjector in the onModuleLoad2() method in our entry point, but for mvp4g we pull it out and let the framework handle it.

I also turn on debugging using the @Debug annotation, and set the logger to one that I wrote that forwards mvp4g's logging calls to gwt-log instead of GWT.log. I wanted all of the logging in one place. It's a very simple class, so I'm going to skip it here.

Finally, we declare an interface that extends EventBus which defines all of the events that can be raised on the bus. The events should not return anything, and can have either one or zero parameters. We don't need a parameter for the login event, since the presenter that handles it will have access to the fields the user has filled out. The @Event annotation declares which classes, separated by a comma, are registered to be notified of the event.

Presenter


com/lisedex/volinfoman/client/LoginPresenter.java
@Presenter(view = LoginView.class)
public class LoginPresenter extends BasePresenter
        <LoginPresenter.LoginViewInterface, VolinfomanEventBus> {

The Presenter will be matched with a View, so we declare that association through the @Presenter annotation, which also lets the mvp4g compiler know that this class defines a Presenter. Our presenter extends BasePresenter, which is a generic that takes the class defining our View interactions (LoginViewInterface), and our Event Bus class (VolinfomanEventBus) as parameters.

The Presenter class handles the logic that will make the interface displayed to the user actually do something. In this case, we're working with a login page, so we need to define what we need for interactions with the View. I followed the mvp4g documentation, which defines it using an inner interface. I don't see why you couldn't break it out to a separate file, but it's a bit more clear having it bound to the Presenter class. Plus it's right there to reference when writing your Presenter.

public interface LoginViewInterface {
  public String getUsername();
  public String getPassword();
  public void setMessage(String msg);
  public HasClickHandlers getLoginButton();
  public void setLoginButtonEnabled(boolean enabled);
 }

We need a few things from the user, and we don't really care how the View gets them. All we care about is that it can provide the information we need to process the login. In this case, we want to be able to get the username and password they've entered, and we also want to be able to send them messages, in case of an error. We also need to find out when the user's told the interface they've completed their data entry, so we want to bind to something that throws Click events; we don't really care if it's a button or not. I also need to be able to control whether the user can request to log in; for instance, if they've just clicked, until the server responds, I don't want them to be able to keep clicking. If the View is not implementing a button, they can use this information in whatever way makes sense. In fact, writing this, it really shouldn't be called setLoginButtonEnabled, but something like setLoginEnabled instead.

The Presenter is going to perform the logic we had in our previous DefaultHomepage class, so we need our UserService.

private UserServiceAsync userService = null;

 @InjectService
 public void setService(UserServiceAsync service) {
  this.userService = service;
 }

mvp4g supports injecting these services (both RPC and non-RPC) with the @InjectService annotation. You define them as you normally would, with both Service and the ServiceAsync version. I didn't have to change the code at all.

The bind() method is called by the framework for us to do further set up of our interactions with the View. The BasePresenter we're extending provides both view and eventBus references so we don't need to handle that.

@Override
 public void bind() {
  super.bind();
  view.getLoginButton().addClickHandler(new ClickHandler() {
   public void onClick(ClickEvent event) {
    view.setLoginButtonEnabled(false);
    eventBus.login();
   }
  });
 }

All I want right now is to know when the login button (or whatever the View is using) is clicked, and when it is, I turn off the login button and fire a login event onto the Event Bus. This will allow some flexibility later, if I need to have another class know about logins.

Finally we define the method to handle the login event. All events are handled by methods that start with "on" followed by the event name. In this case, it will be onLogin.

public void onLogin() {
 if (userService == null) {
  Log.fatal("userService is null, not injected", 
    new NullPointerException("LoginPresenter.userService"));
  view.setMessage("Fatal application error.  \"userService not injected.\"");
  view.setLoginButtonEnabled(true);
  return;
 }
  
 userService.getUser(view.getUsername(), new AsyncCallback<User>() {
  @Override
  public void onFailure(Throwable caught) {
   view.setMessage("RPC FAILED");
   view.setLoginButtonEnabled(true);
  }

  @Override
  public void onSuccess(User result) {
   if (result == null) {
    view.setMessage("NO SUCH USER");
   } else {
    view.setMessage("SUCCESS: " + result.toString());
   }
   view.setLoginButtonEnabled(true);
  }
 });
}

This is pretty much the click handler that was in DefaultHomepage. We first do a quick sanity check for our UserService, and if it's not there, there's something really wrong. Then we call our service's getUser method, and when it returns we either tell the user the user doesn't exist, or spit out the user's information. Then we turn the login button back on.

View


The View just sets up visual aspect of the interface the user sees. In this case, we're still using a UiBinder, copied from the DefaultHomepage, and then adding the methods to flesh out the View interface we built in the Presenter.

com/lisedex/volinfoman/client/LoginView.java
public class LoginView extends Composite 
 implements LoginPresenter.LoginViewInterface {

 private static LoginViewUiBinder uiBinder = GWT
  .create(LoginViewUiBinder.class);

 interface LoginViewUiBinder extends UiBinder<Widget, LoginView> {
 }

The View is going to be a Composite widget, and it will implement the View interface. We also bind the View with its associated LoginView.ui.xml file, so we can use the fields declared there to provide information to the Presenter.

@UiField
 TextBox username;
 
 @UiField
 TextBox password;
 
 @UiField
 Button sendButton;
 
 @UiField
 HTML sendStatus;
 
 public LoginView() {
  initWidget(uiBinder.createAndBindUi(this));
  
  sendButton.setText("Login");
  
  sendStatus.setStyleName("serverResponseLabelError");
  
  DeferredCommand.addCommand(new Command() {
   public void execute() {
    username.setFocus(true);
   }
  });
  username.selectAll();
 }

This code is basically the same as what was in DefaultHomepage. We bind the XML defined user interface, then we set the text for our login button. We also set the style for the place we'll send messages to the user, so text shows up red, and then we set the focus on the username field. Setting focus did not work in previous versions of our code, so after a quick search I found this workaround described in GWT issue 1849, where they discuss making this the default implementation of setFocus().

The rest of the class is just implementing the LoginViewInterface.

@Override
 public String getUsername() {
  return username.getText();
 }

 @Override
 public String getPassword() {
  return password.getText();
 }

        .... etc, etc.

They're all just basic getters and setters, so I won't include them here, but you can take a peek at the complete code if you want to see the whole thing.

Some notes


I like the way mvp4g seems to work, and reading past posts on its discussion group was useful in figuring some of it out. There's other functionality I haven't even touched yet: code-splitting through using different modules for different parts of the interface, lazy loading, and, most importantly, history. I'll be adding history as soon as I have a second page or something where history makes sense. The others seem to be optimizations which we don't need yet.

I also may want to implement the command pattern later, to have more complete bits of information passed with their event on the event bus.

I've been avoiding it long enough, but now it's time to figure out a good authentication/session design, since I don't want to require the volunteers using this to have a Google account. If I did, it would pretty much be implemented for me in the App Engine Users API.