SecurityExpressionRoot to be made resolvable as an argument

[ememisya]

I have a controller that does some further fine grained authorization in the body of the method rather than pre or post like so. I wouldn't want to iterate a large collection more than once.

  @RequestMapping(value = "/batch-mail-request/distribution-lists", method = RequestMethod.GET,
          produces = MediaType.APPLICATION_JSON_VALUE)
  @PreAuthorize("hasAuthority('gs/batchmail/batch-mail-request/distribution-lists?get') && " +
          "hasPermission(#id, T(edu.vt.graduateschool.batchmail.domain.BatchMailRequest)" +
          ".ENTITLEMENT_ENTITY_PATH_SEGMENT_NAME, 'get')")
  @HandleAuthorizationDenied(handlerClass = EmptySetAuthorizationDeniedHandler.class)
  public ResponseEntity<Set<DistributionListModel>> batchMailRequestDistributionLists(
          @RequestParam final Long id,
          final Authentication authentication,
          @AutowireSecurityExpressionRoot final SecurityExpressionRoot root)
  {
    if (root == null) {
      throw new IllegalArgumentException("SecurityExpressionRoot could not be found");
    }
   //... some retrieval
    return new ResponseEntity<>(distributionLists.stream()
            .filter(p -> root.hasPermission(p.getId(), DistributionList.ENTITLEMENT_ENTITY_PATH_SEGMENT_NAME, "get"))
            .map(e -> conversionService.convert(e, DistributionListModel.class))
            .collect(Collectors.toSet()), HttpStatus.OK);
  }

An extremely hacky way to get the SecurityExpressionRoot efficiently is as follows:

public class SecurityExpressionRootArgumentResolver implements HandlerMethodArgumentResolver
{

  /**
   * {@link MethodSecurityExpressionHandler} default handler.
   */
  @Autowired
  private MethodSecurityExpressionHandler expressionHandler;

  //....

  /**
   * Albeit an extremely hacky way to get the security root object it appears to be the only way to get an instance
   * autowired. {@link SimpleMethodInvocation} is only used for containing the actual bean instance which is the
   * only thing accessible to the root object via JoinPoint interface's getThis method.
   *
   * I will most likely rethink this in the future.
   *
   * @param parameter parameter
   * @param mavContainer mavContainer
   * @param webRequest webRequest
   * @param binderFactory binderFactory
   * @return Resolved argument
   *
   * @throws NoSuchFieldException NoSuchFieldException
   * @throws IllegalArgumentException IllegalArgumentException
   * @throws IllegalAccessException IllegalAccessException
   */
  @Override
  public Object resolveArgument(final MethodParameter parameter, final ModelAndViewContainer mavContainer,
          final NativeWebRequest webRequest, final WebDataBinderFactory binderFactory)
          throws NoSuchFieldException, IllegalArgumentException, IllegalAccessException
  {
    final Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
    if (authentication == null) {
      return null;
    }
    final Field outerClass = parameter.getClass().getDeclaredField("this$0");
    outerClass.setAccessible(true);
    final Object declaringBean = ((HandlerMethod) outerClass.get(parameter))
            .createWithResolvedBean().getBean();
    return expressionHandler.createEvaluationContext(authentication,
            new SimpleMethodInvocation(declaringBean instanceof String ?
                    parameter.getContainingClass() : declaringBean,
                    parameter.getMethod()))
            .getRootObject().getValue();
  }
 // ...
}

Current Behavior

Currently there doesn't appear to be a way to hand the SecurityExpressionRoot to the body of a method other than using a String to be parsed by SpEL framework pre or post.

Context

Trying to have an efficient way to apply security root to a stream in the body of a method. I have found a way, but I'm afraid it's through unorthodox means. I have considered modifying the SecurityExpressionRoot to be made available but thought it best not to touch that according to the documentation.

I have created an interceptor that does the same thing but with a real method invocation like so:

public class MethodSecurityEvaluationContextInterceptor implements MethodInterceptor
{

  /**
   * {@link MethodSecurityExpressionHandler} default handler.
   */
  @Autowired
  private MethodSecurityExpressionHandler expressionHandler;

  @Override
  public Object invoke(final MethodInvocation invocation) throws Throwable
  {
    if (invocation != null) {
      final Annotation[][] parameterAnnotations =
              invocation.getMethod().getParameterAnnotations();
      if (parameterAnnotations.length > 0) {
        for (int i = 0; i < parameterAnnotations.length; i++) {
          final Annotation[] annotations = parameterAnnotations[i];
          for (final Annotation annotation : annotations) {
            if (annotation instanceof AutowireSecurityExpressionRoot) {
              invocation.getArguments()[i] = expressionHandler.createEvaluationContext(
                      SecurityContextHolder.getContext().getAuthentication(), invocation)
                      .getRootObject().getValue();
            }
          }
        }
      }
      return invocation.proceed();
    }
    throw new RuntimeException("Invocation was null");
  }

}

Which does achieve what's needed however I have to proxy the bean and argument resolvers are already a processing cost applied without the additional overhead so I preferred the argument resolver route albeit worse in hackiness, it's more efficient.

I was wondering if it could be a feature where if method security is enabled, much like Authentication being autowired as a parameter, SecurityExpressionRoot could also be auto wired similarly?

[jzheaux]

Interesting idea, @ememisya. SecurityExpressionRoot probably isn't the best since it has public fields needed for expression context.

It would be better to expose SecurityExpressionOperations, though I'm not a huge fan of handing something with the name Expressions into a place that has nothing to do with SpEL.

Let's explore a few other ideas first.

First, have you considered @PostFilter? I'm not sure what is happening when you do // ... some retrieval, but it could possibly do something like this:

@PostFilter("hasPermission(filterObject.id, yourEntitlementExpression)")
public Set<DistributionList> findDistributionList(...) {
  ...
}

In this way, you wouldn't need to filter in the controller method.

Or second, you can inject your PermissionEvaluator @Bean into your controller and do:

return new ResponseEntity<>(distributionLists.stream()
    .filter(p -> this.permissionEvaluator.hasPermission(p.getId(), DistributionList.ENTITLEMENT_ENTITY_PATH_SEGMENT_NAME, "get"))
    // ...

Does one of those suit your needs?

[ememisya]

@PostFilter will work actually and I did not know that existed (but the same sort of performance hit of reiterating a collection). I see what you are saying with methods that only return booleans statically for the expression context. The methods of interest within the controller method are ones that start with has and is but not denyAll() for example. I wonder if just the has. and is. portion of SecurityExpressionOperations can be wired as an argument?

[jzheaux]

Maybe, though I'd want to wait and see a concrete use case where that's desirable over the alternatives. If folks want to use the expressions programmatically, then can already do the following:

@Component("authz")
public final class AuthorizationExpressions {
    public boolean hasEntitiement(MethodSecurityExpressionOperations operations, String entitlementExpression) {
        DistributionList list = (DistributionList) operations.getFilterObject();
        return operations.hasPermission(list.getId(), entitlemtnExpression);
    }
}

And then do:

@PostFilter("@authz.hasEntitlement(#root, yourEntitlementExpression)")
public Set<DistributionList> findDistributionList(...) {
  ...
}

Or even simpler:

@Retention(RetentionPolicy.RUNTIME)
@PostFilter("@authz.hasEntitlement(#root, '{value}')")
public @interface FilterByEntitlement {
    String value();
}

And then:

@FilterByEntitlement("yourEntitlementExpression")
public Set<DistributionList> findDistributionList(...) {
  ...
}

This is nice because it encourages folks to extract the authorization logic as an aspect of the behavior instead of embedding it into the business logic.

The methods of interest within the controller method are ones that start with has and is but not denyAll() for example. I wonder if just the has. and is. portion of SecurityExpressionOperations can be wired as an argument?

It's found in a few different places at this point:

• For hasPermission, wire a PermissionEvaluator as a dependency
• For isXXX, wire an AuthenticationTrustResolver as a dependency
• For the other has methods, you can use AuthorityAuthorizationDecision as a member variable:

public class ... {

    private final AuthoritiesAuthorizationManager authorization = new AuthoritiesAuthorizationManager();

    @GetMapping
    public String method(Authentication authentication) {
        if (this.authorization.check(() -> authentication, List.of("ROLE_ADMIN", "SCOPE_read")).isGranted()) {
            // ... equivalent of hasAnyAuthority
        }
        // ...
    }
}

Though again, I'd advise if you want to get an instance of SecurityExpressionOperations and do things programmatically, you are likely better off publishing a bean, as demonstrated with findDistributionList and AuthorizationExpressions above.

I appreciate the idea, @ememisya, it made me think. I'm going to close this for now. I'm glad to hear that @PostFilter will work for you. We can always revisit this if injecting authorization logic as an MVC method parameter resurfaces.

[ememisya]

I'm glad to hear that @PostFilter will work for you.

While it does work, it increases processing overhead. An interpreted language has to be used upon another interpreted language (SpEL over Java). For collections of sufficient size this is noticeable. I can always wire the beans within SecurityExpressionRoot and recreate the methods mentioned for authorization however I'd like to use security related code that I am not maintaining for ease of mind and future updates. It would be nice given most of the SecurityContext related operations are static and the beans involved singletons, if I could easily get access to it within the method without an additional performance hit (or as a part of one that is already accepted during parameter binding).

Thank you for looking into this.