Add @AuthorizeRequestMapping annotation

[rwinch]

Combined with gh-16249 we could add an annotation (e.g. @AuthorizeRequestMapping) that allows adding authorization rules to Spring Controllers but happens at the same time as authorizeHttpRequests() (to reduce the attack surface) rather than late like method security.

The need for a new annotation is due to the fact that @PreAuthorize allows access to method parameters, but we will not have access to those parameters in a web based authorization model.

We'd need the ability to scan for all annotated controllers and create a mapping of the RequestMapping to authorization rules.

A few examples:

@GetMapping("/users/{id}")
@AuthorizeRequestMapping("hasRole('ADMIN')")
User findById(String id) {

}

@GetMapping("/users/{id}")
@AuthorizeRequestMapping("@authz.canReadUser(authentication, #id)")
User findById(String id) {
  // authz is a bean name
  // canReadUser is a method on the authz bean that returns a boolean and accepts a String that is the id of the user to check
  // authentication is the current Authentication (same as all SpEL based Security)
  // id is the parsed id from the @GetMapping
}

The following adds an authorization rule that only admin can access the routes of /admin/users/{id} and /admin/users/.

@AuthorizeRequestMapping("hasRole('ADMIN')")
class AdminController {

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

  }

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

  }

}

We also need the ability to take into account all of the information that a Spring Controller would take into account (e.g. HTTP Method, Content negotiation, etc).

cc @rstoyanchev

[jzheaux]

At the risk of making the issue too noisy with ideas, something we might consider is annotations that do not allow SpEL. For example:

@HasRole("ADMIN")
@HasScope("message:read")
@AuthorizeRequest(authorizationManagerClass=MyAuthorizationManager.class)

This allows not needing to consider method parameters. It also (wisely, IMO) drives applications towards using beans instead of expressions.

The authorization manager implementations would be of type AuthorizationManager<RequestAuthorizationContext> to allow transmitting path variables.

One possibly risky thing that I'm considering is that some of these could double as method annotations in other places. That is, when @HasRole is used on a @RequestMapping-annotated method, it (also) is honored in the filter chain.

[jgrandja]

The concern I have with adding @AuthorizeRequestMapping is it adds another way of configuring the same authorization rules as users currently do with authorizeHttpRequests(). One of the common feedback we receive for Spring Security is there is more than one way to do the same thing, which leaves users confused if they are configuring things correctly.

We'd need the ability to scan for all annotated controllers and create a mapping of the RequestMapping to authorization rules.

Would it make sense to scan all the @PreAuthorize annotations and leverage the authorization rules configured within and enhance authorizeHttpRequests()? However, this might be confusing as well because now we have method security authorization rules leveraged upfront at the web layer.

[rstoyanchev]

My thoughts went in the same direction as @jgrandja. Could the existing annotation be made use of? In effect enhancing the evaluation of the annotation when used on a web controller to take place earlier when feasible (i.e. no use of method parameters)? It wouldn't require applications to roll out a new annotation for what is an implementation detail, and would just work on existing setups.

The name is also much too long for a highly visible annotation. We shortened @RequestMapping by popular demand, while this goes the other way, and that's in addition to the mapping annotation from spring-web.

As an alternative, if this does not work out, perhaps something more broadly useful that has additional user visible value (vs dealing with an implementation detail) along the lines of @jzheaux's comment.

[rwinch]

I do not think that we can reuse an existing annotation (e.g. @PreAuthorize) because it isn't just a different implementation.

The semantics of @PreAuthorize vs this proposal are different in what the user expects. Users already leverage @PreAuthorize today as method based security. Method security provides a second layer of defense for applications with the ability to leverage method parameters, but at the cost of increasing the attack surface of the application (more code is traversed before method security can be performed). This means that applications currently using @PreAuthorize on controllers would be broken if we reuse the annotation. Even if we ignored this fact, applications would not have the ability to do method security on Controllers if we reuse @PreAuthorize.

As for the name I think that we can iterate on that.

I think that I like the idea of experimenting with Josh's suggestion. The concern around it being used on method security is not necessarily a bad thing because these annotations would behave the same (authority/role/scope) based security doesn't care about the object being secured. We could allow both method security and the web based security to act upon the same annotation in the same way.

[evgeniycheban]

Hi, @rwinch @jzheaux are you currently working on this? I would want to implement it.

[rwinch]

@evgeniycheban I think that we need to start with gh-16249 since @AuthorizeRequestMapping will not have an order to it and thus "Best Match" is required.

[evgeniycheban]

@rwinch sure, can you assign it to me?