Skip to content

Add "Best Match" based Web Authorization Rules #16249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rwinch opened this issue Dec 9, 2024 · 1 comment
Open

Add "Best Match" based Web Authorization Rules #16249

rwinch opened this issue Dec 9, 2024 · 1 comment
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Milestone

Comments

@rwinch
Copy link
Member

rwinch commented Dec 9, 2024

Currently the web based authorization rules are specified in a specific order and the first rule that matches the request is used. For example with the following rules:

.requestMatchers("/users/**").authenticated()
.requestMatchers("/user/{id}").hasRole("USERS")

A request to /users/123 would match on /users/** first, so the authenticaticated() rule is applied even though the request also (and more precisely matches /users/{id}.

This is in contrast to Spring MVC and WebFlux routing where the @RequestMapping are not specified in any particular order but the best match is found and used.

For example, in the example below the URL /users/123 will still route to findUserById method:

@GetMapping("/users/**")
List<User> users() {

}

@GetMapping("/users/{id}")
User findUserById(String id) {

}

It would be nice if Spring Security could support a "Best Match" based algorithm. Things to consider:

  • Performance: Is this going to perform well?
  • Caching: Spring MVC / WebFlux will likely have to replicate the same logic over the same @RequestMapping
  • Make it clear that order does not matter
  • Make it clear that the algorithm being used is Spring's since Spring Security is used on Spring applications but also used on standard servlet applications which may determine "Best Match" differently

cc @wilkinsona @rstoyanchev

@rwinch rwinch added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement labels Dec 9, 2024
@rwinch rwinch added this to the 6.5.x milestone Dec 9, 2024
@rwinch rwinch self-assigned this Dec 9, 2024
@rwinch rwinch modified the milestones: 6.5.x, 7.0.x Apr 4, 2025
@evgeniycheban
Copy link
Contributor

Hi, @rwinch I've been investigating how this could be implemented, given that PathPatternRequestMatcher is going to be a default choice, we could probably try to achieve this by sorting mapped RequestMatchers using PathPattern.SPECIFICITY_COMPARATOR, this way we would have more specific matchers placed at the beginning of the mapped authorization rules:

/**
 * Creates a {@link RequestMatcherDelegatingAuthorizationManager} instance.
 * @return the {@link RequestMatcherDelegatingAuthorizationManager} instance
 */
public RequestMatcherDelegatingAuthorizationManager build() {
    this.mappings.sort((m1, m2) -> {
        if (m1.getRequestMatcher() instanceof PathPatternRequestMatcher o1
                && m2.getRequestMatcher() instanceof PathPatternRequestMatcher o2) {
            return PathPattern.SPECIFICITY_COMPARATOR.compare(o1.getPattern(), o2.getPattern());
        }
        return 0;
    });
    return new RequestMatcherDelegatingAuthorizationManager(this.mappings);
}
@Test
void authorizeWhenMultipleMatchersMatchRequestThenMoreSpecificOneTakesPrecedence() {
    RequestMatcherDelegatingAuthorizationManager manager = RequestMatcherDelegatingAuthorizationManager.builder()
        .requestMatchers(PathPatternRequestMatcher.withDefaults().matcher("/users/**"))
        .authenticated()
        .requestMatchers(PathPatternRequestMatcher.withDefaults().matcher("/users/{id}"))
        .hasRole("ADMIN")
        .build();
    AuthorizationResult result = manager.authorize(TestAuthentication::authenticatedUser,
            new MockHttpServletRequest(null, "/users/1"));
    assertThat(result).isNotNull();
    assertThat(result.isGranted()).isFalse();
}

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants