spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.83k stars 5.91k forks source link

Add path variables for RegexRequestMatcher #3855

Closed ksokol closed 7 years ago

ksokol commented 8 years ago

We know that we can use an AntPathRequestMatcher for Path Variables Web Security Expressions like:

<http>
    <intercept-url pattern="/user/{userId}/**"
        access="@webSecurity.checkUserId(authentication,userId)"/>
    ...
</http>

but how we do achieve the same with a RegexRequestMatcher?

So far, I tried the following without success:

<http request-matcher="regex">
    <intercept-url pattern="/user/(?&lt;userId>.*)/.*" 
        access="@webSecurity.checkUserId(authentication,userId)"/>
    ...
</http>
rwinch commented 8 years ago

@ksokol Thank you for the question. At this time regex is not supported.

rwinch commented 8 years ago

PS: A pull request for this feature would be most welcome

ksokol commented 8 years ago

Thank you for your answer. I will come up with a PR in the next days.

ksokol commented 8 years ago

Due to the fact that Spring Security supports Java 5 and 6 I can not rely on Matcher.group(String) that was introduced in Java 7. Matcher.group(String) would make implementation a whole lot easier.

In respect to Java 5 and 6, I think the best option to support path variables in RegexRequestMatcher is to encapsulate regex group name parsing in a dedicated parser class. Due to the fact that

Capturing groups are indexed from left to right

one can assume that the groups in a regular expression like /(?<foo>)/(?<bar>)/.* are indexed as 1 => foo and 2 => bar.

Therefore, the dedicated parser class maps group name to corresponding group index that in turn retrieves the regex group via Matcher.group(int).

Please take a look at my experimental branch for details.

I think introducing named-regexp as a dependency for regex group name parsing is not a good option. I would go for a hand crafted implementation to detect group names.

@rwinch What do you think?

rwinch commented 8 years ago

In the next release we will be updating to Spring 5 which is going to have a minimum version of JDK8. This means we could use the Matcher group.

ksokol commented 8 years ago

It is worth to wait until Spring 5 before this feature request gets handled.

rwinch commented 8 years ago

You can get it all ready and we will merge once we get started on 5.x work. Thanks! On May 7, 2016 12:24 PM, "Kamill Sokol" notifications@github.com wrote:

It is worth to wait until Spring 5 before this feature request gets handled.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/spring-projects/spring-security/issues/3855#issuecomment-217653343

ksokol commented 7 years ago

Since the introduction of named groups in Java 7 we do not have an api to request all names of all named groups in a regular expression. This is still true to this day.

There is already an open issue in the OpenJDK bug tracker. But it has a low priority and it is not clear when it will be added to the JDK. Even if it will be included in JDK 10, it will take quite some time until Spring Security moves to JDK 10 or higher.

Even though we can not have native support for named groups in RegexRequestMatcher, we can use named groups in Spring Security by writing a custom RequestMatcher .e.g. with named-regexp library.

A custom RequestMatcher that is not backed into Spring Security is the best option we have at the moment.

Example of a RequestMatcher that supports named groups:

import com.google.code.regexp.Matcher;
import com.google.code.regexp.Pattern;
import org.springframework.security.web.util.matcher.RegexRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.security.web.util.matcher.RequestVariablesExtractor;

import javax.servlet.http.HttpServletRequest;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

public final class NamedGroupRegexRequestMatcher implements RequestMatcher, RequestVariablesExtractor {

    private final RegexRequestMatcher regexRequestMatcher;
    private final Pattern pattern;

    public NamedGroupRegexRequestMatcher(String pattern) {
        this(pattern, null, false);
    }

    public NamedGroupRegexRequestMatcher(String pattern, String httpMethod) {
        this(pattern, httpMethod, false);
    }

    public NamedGroupRegexRequestMatcher(String pattern, String httpMethod, boolean caseInsensitive) {
        this.regexRequestMatcher = new RegexRequestMatcher(pattern, httpMethod, caseInsensitive);
        this.pattern = Pattern.compile(pattern);
    }

    public static RequestMatcher regexp(String pattern) {
        return new NamedGroupRegexRequestMatcher(pattern);
    }

    @Override
    public boolean matches(HttpServletRequest request) {
        return this.regexRequestMatcher.matches(request);
    }

    @Override
    public Map<String, String> extractUriTemplateVariables(HttpServletRequest request) {
        String url = getRequestPath(request);
        Matcher matcher = pattern.matcher(url);

        if (!matcher.matches()) {
            return Collections.emptyMap();
        }

        Map<String, String> variables = new HashMap<>();

        for (String namedGroup : pattern.groupNames()) {
            String group = matcher.group(namedGroup);
            variables.put(namedGroup, group);
        }

        return variables;
    }

    private String getRequestPath(HttpServletRequest request) {
        String url = request.getServletPath();

        if (request.getPathInfo() != null) {
            url += request.getPathInfo();
        }

        return url;
    }
}

Security configuration:

@Override
protected void configure(HttpSecurity http) throws Exception {
        http
        .authorizeRequests().requestMatchers(regexp("(?<userId>.*)/.*")).access("@webSecurity.checkUserId(authentication, #userId)")
        ...
}