Enforcing Spring role based security with a custom PMD rule

Spring makes it easy to have role based security with its @PreAuthorize annotation. But how do you make sure that developers remember to add a security annotation? I like using PMD to do static analysis checks on our code and one of the great things about static analysis tools like PMD is that as well as using the built in rules, you can write your own rules tailored to your software. In this case, I’d like a rule that does the following:

• Checks if a Java class is known to be an http endpoint.
• If so, checks if it is on a whitelist of non-secure endpoints.
• If not, creates a PMD rule violation if there is no @PreAuthorize on the appropriate method.

In our case, we use GWT (Google Web Toolkit) for most of our UI. The requests actually go to a single endpoint, but we use the command pattern to send the command to an appropriate handler. Thus it is the execute method on the handler that needs to be secured. I came up with the following code:

import net.sourceforge.pmd.RuleContext;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTPackageDeclaration;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.HashSet;
import java.util.Set;

/** Looks for http endpoints which do not have an @PreAuthorize annotation and are not listed in
 * a whitelist file of unsecured endpoints.
 */
public class UnsecuredHttpEndpointRule extends AbstractJavaRule {

    private static final String UNSECURED_HTTP_ENDPOINT_MESSAGE =
            "HTTP endpoint class missing @PreAuthorize annotation with role and not in unsecured endpoints whitelist";

    private static Set<String> UNSECURED_ENDPOINT_WHITELIST = new HashSet<>();
    private String packageName;

    static {
        try (BufferedReader fileReader = new BufferedReader(
                new InputStreamReader(UnsecuredHttpEndpointRule.class.getClassLoader()
                        .getResourceAsStream("unsecured-http-endpoints-whitelist.txt")));) {
            String line = fileReader.readLine();
            while (line != null) {
                if (!(line.startsWith("//") || line.startsWith("#"))) {
                    UNSECURED_ENDPOINT_WHITELIST.add(line);
                }
                line = fileReader.readLine();
            }
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    @Override
    public Object visit(ASTPackageDeclaration node, Object data) {
        packageName = node.getName();
        return super.visit(node, data);
    }

    @Override
    public Object visit(ASTClassOrInterfaceDeclaration node, Object data) {
        String className = node.getQualifiedName().getClassSimpleName();
        String fullyQualifiedClassName = packageName != null ? packageName + "." + className : className;
        if (className.endsWith("Handler")
                && !UNSECURED_ENDPOINT_WHITELIST.contains(fullyQualifiedClassName)) {
            return super.visit(node, data);
        }
        // if the class is not an http endpoint, or is on the whitelist, there is no need
        // to carry on down the abstract syntax tree, so just return null
        return null;
    }

    @Override
    public Object visit(ASTMethodDeclaration node, Object data) {
        if (node.getName().equals("execute") && !node.isAnnotationPresent("PreAuthorize")) {
            addViolationWithMessage(data, node, UNSECURED_HTTP_ENDPOINT_MESSAGE);
        }
        return super.visit(node, data);
    }

    @Override
    public void end(RuleContext ctx) {
        packageName = null;
    }


}

The code loads a whitelist of unsecured classes. You can see I’ve done this in a static block as there may be multiple instances of a PMD rule instantiated as PMD could be using multiple threads (see the PMD docs on Java rules at Writing Java Rules).