Spring Security SEC-3136: Allow for array Http methods and varargs patterns for Ant matcher

Abhijit Sarkar (Migrated from SEC-3136) said:

Often times, the same security requirement applies to multiple Http methods. For example, "any POST, DELETE, PUT or PATCH must only be accessible to the admins, regardless of the URL pattern". The Ant matchers have 2 methods, one that takes a URL pattern and applies to all Http methods and another that takes a Http method and applies to a varargs URL patterns. None of these methods could be used in the above use case requiring redundant code like the following. It'd be nice to have an ant matcher that could take a array Http method and varargs patterns as arguments.

protected void configure(HttpSecurity http) throws Exception {
    http
    .authorizeRequests()
    .antMatchers(GET, SECURE_PATTERNS).authenticated()
    .antMatchers(POST).hasAnyRole("ADMIN")
    .antMatchers(DELETE).hasAnyRole("ADMIN")
    .antMatchers(PUT).hasAnyRole("ADMIN")
    .antMatchers(PATCH).hasAnyRole("ADMIN").and()
    .httpBasic()
    .and().csrf().disable() 
    .userDetailsService(userDetailsService); //
}

Comment From: spring-projects-issues

Jean de Klerk said:

Abhijit, the signature for the antMatcher that takes HttpMethod is

public C antMatchers(HttpMethod method, String... antPatterns)

(see AbstractRequestMatcherRegistry.java). Only the last argument to a java function can be vararg - not both.

Comment From: spring-projects-issues

Abhijit Sarkar said:

Yes, but the first one can be an array. I've updated the description.

Comment From: khooz

Something I felt I can add, is to use bitwise enumerations, eg. HttpMethod.any(HttpMethod.GET, HttpMethod.POST, ...) and any would do something like value = Arrays.stream(args).reduce(0, (prev, cur) -> prev | cur.value); and would have some method like contains(HttpMethod arg) like return (this.value & arg.value) == arg.value.

This approach is easy to integrate (because you wouldn't need to change current code if nobody used ordinal values instead of labels, which is enumeration heresy), performant because bitwise operations are fast and simple, because it's that simple!

Comment From: jzheaux

Hi, @khooz, thanks for the suggestion. However, since HttpMethod is an enum, I think it would be odd to have something else in its enumerated values that is not an HTTP method.

Comment From: jzheaux

Also, I'm curious about the original request since the sample doesn't match the description. The description calls for an antMatchers method that sends both paths and methods, but the sample only needs methods. Would adding antMatchers(HttpMethod... methods) be more appropriate, then?

Comment From: khooz

Thanks @jzheaux! It is in fact common practice in other languages to enumerate values base on non-overlapping binary integers, i.e. flags; specially more low-level languages and OS API calls, like accessing sockets/files. I actually like Java enums because they are more flexible and have separate data and representation layers. Here is what I would add to HttpMethod:

public enum HttpMethod
{

    NONE(0x0) ,GET(0x1), HEAD(0x2), POST(0x4), PUT(0x8), PATCH(0x10), DELETE(0x20), OPTIONS(0x40), TRACE(0x80);

    /// ... HttpMethod's default implementation above

    private Integer value;

    private HttpMethod(Integer httpMethodValue)
    {
        this.value = httpMethodValue;
    }

    public boolean any(HttpMethod ...args)
    {
        for(HttpMethod httpMethod : args)
        {
            if ((this.value & httpMethod.value) == httpMethod.value)
            {
                return true;
            }
        }

        return false;
    }

    public boolean all(HttpMethod ...args)
    {
        for (HttpMethod httpMethod : args)
        {
            if ((this.value & httpMethod.value) != httpMethod.value)
            {
                return false;
            }
        }

        return true;
    }

    public boolean matches(String ...methods)
    {
        HttpMethod[] httpMethods = (HttpMethod[]) Arrays.stream(methods)
            .map(HttpMethod::resolve)
            .toArray();

        return this.all(httpMethods);
    }

    public boolean matchesAll(String ...methods)
    {
        return this.matches(methods);
    }

    public boolean matchesAny(String ...methods)
    {
        HttpMethod[] httpMethods = (HttpMethod[]) Arrays.stream(methods)
            .map(HttpMethod::resolve)
            .toArray();

        return this.any(httpMethods);
    }

    public static HttpMethod aggregate(HttpMethod ...args)
    {
        Integer result = 0x0;
        for (HttpMethod httpMethod : args)
        {
            result = result | httpMethod.value;
        }
        HttpMethod httpMethodResult = HttpMethod.NONE;
        httpMethodResult.value = result; 
        return httpMethodResult;
    }

}

Comment From: jzheaux

It is in fact common practice in other languages to enumerate values base on non-overlapping binary integers

Agreed, though I haven't seen this exposed as a special enum value. For the level of flexibility you are talking about, I believe you'd need it to be a class instead of an enum.

Here is what I would add to HttpMethod

Is aggregate thread-safe? Since HttpMethod.NONE is a singleton, I believe setting one of its fields would set it for all threads.

Either way, since HttpMethod is part of Spring Framework, I'd direct you to that project for any changes you'd like to propose.