geschema
geschema

Reputation: 2494

Java Spring Security @PostFilter performance

My Java Spring application, which uses ACLs, has a service method to retrieve all the objects corresponding to a given user:

    @Override
    @PostFilter("hasPermission(filterObject, 'READ') or hasPermission(filterObject, 'ADMINISTRATION')")
    public List<SomeClass> findAll() {
        return SomeClassRepository.findAll();
    }

Unfortunately, where there are many objects in the database, this method takes too much time to complete (over 1 second). Probably because it will first fetch all the objects from the database, and then filter them one by one in memory. How can I optimize this without losing the benefits of Spring ACLs?

Edit: the solution I came up with for now is to create repositories for the acl_sid and acl_entry repositories and to fetch the IDs of the object of interest through those repositories. This gives me a 10x improvement in execution time compared to the method above. The new code looks like this:

    @Override
    @PostFilter("hasPermission(filterObject, 'READ') or hasPermission(filterObject, 'ADMINISTRATION')")
    public List<SomeClass> findAll() {
        List<SomeClass> result = new ArrayList<SomeClass>();
        Long userId = (Long) SecurityContextHolder.getContext().getAuthentication().getPrincipal();
        AclSid sid = aclSidRepository.findBySid(Long.toString(userId));
        List<AclEntry> aclEntries = aclEntryRepository.findBySid(sid);
        for (AclEntry aclEntry : aclEntries) {
            AclObjectIdentity aclObjectIdentity = aclEntry.getAclObjectIdentity();
            AclClass aclClass = aclObjectIdentity.getObjectIdClass();
            if (aclClass.getClassName().equals("com.company.app.entity.SomeClass")) {
                Optional<SomeClass> SomeClass = SomeClassRepository
                        .findById(aclObjectIdentity.getObjectIdIdentity());
                if (SomeClass.isPresent()) {
                    result.add(SomeClass.get());
                }
            }
        }
        return result;
    }

Upvotes: 0

Views: 1510

Answers (1)

jccampanero
jccampanero

Reputation: 53431

As Spring filters the information in memory the performance will depend on the actual number of results: if there is a large amount of them, I am afraid that perhaps only caching your repository results before filtering the information may be a suitable solution.

To deal with the problem, you can filter the results at the database level. Two approaches come to my mind:

  • Either use Specifications, and filter the results at the database level taking into account the information about the principal exposed by Spring Security SecurityContext and including the necessary filter Predicates in order to restrict the information returned.
  • Or, if you are using Hibernate, use entity filters to again, based on the information about the principal exposed by Spring Security, apply the necessary data restrictions. Please, see this related SO question which provides great detail about the solution.

Please, consider for instance the use case of the Spring Data Specifications.

Instead of SomeClass, let's suppose that we are working with bank accounts. Let's create the corresponding entity:

@Entity
public class BankAccount {

  @Id
  private String accountNumber;
  private Float balance;
  private String owner;
  private String accountingDepartment;

  //...

}

And the corresponding repository:

public interface BankAccountRepository extends Repository<BankAccount, String>, JpaSpecificationExecutor<BankAccount, String> {
}

In order to filter the information depending on the user who is performing the query, we can define an utility method that, based on the user permissions, returns a List of Predicates that we can add later to the ones we are using in a certain Specification when filtering the bank accounts:

public static List<Predicate> getPredicatesForRestrictingDataByUser(Root<BankAccount> root, CriteriaQuery<?> query, CriteriaBuilder cb) {
  // I realized in your edit that you are returning the user id instead of the user object.
  // There is nothing wrong with it but you are losing a valuable information: if you provide
  // a convenient UserDetails implementation you can have direct access to the authorities a user has, etc
  User user = SecurityContextHolder.getContext().getAuthentication().getPrincipal();

  // Restrict data based on actual permissions

  // If the user is an admin, we assume that he/she can see everything, and we will no return any predicates
  if (hasAuthority(user, 'ADMINISTRATION')) {
    return Collections.emptyList();
  }

  // Let's introduce the accounting manager role.
  // Suppose that an accounting manager can see all the accounts in his/her department
  if (hasAuthority(user, 'ACCOUNTING_MANAGER')) {
    return Collections.singletonList(cb.equal(root.get(BankAccount_.accountingDeparment), user.getDepartment()))
  }

  // In any other case, a user can only see the bank account if he/she is the account owner
  return Collections.singletonList(cb.equal(root.get(BankAccount_.owner), user.getId()));
}

Where hasAuthority can look like:

public static boolean hasAuthority(User user, String... authorities) {
  if (user instanceof UserDetails) {
    for (String authority : authorities) {
      return authentication.getAuthorities().stream()
        .map(GrantedAuthority::getAuthority)
        .findAny(a -> a.equals(authority))
        .isPresent();
    }
  }

  return false;
}

Now, use these methods when constructing your Specifications. Consider for instance:

public static Specification<BankAccount> getBankAccounts(final BankAccountFilter filterCriteria) {
  return new Specification<BankAccount>() {

    @Override
    public Predicate toPredicate(Root<BankAccount> root, CriteriaQuery<?> query, CriteriaBuilder cb) {
      List<Predicate> predicates = new ArrayList<Predicate>();

      // Build your predicate list according to the user provided filter criteria
      String accountNumber = filterCriteria.getAccountNumber();
      if (accountNumber != null) {
        predicates.add(cb.equal(root.get(BankAccount_.accountNmber), accountNumber);
      }

      //...

      // And now, restrict the information a user can see
      // Ideally, define getPredicatesForRestrictingDataByUser in a generic class more suitable for being reused
      List<Predicate> predicatesForRestrictingDataByUser = getPredicatesForRestrictingDataByUser(root, query, cb);
      predicates.addAll(predicatesForRestrictingDataByUser);

      Predicate predicate = cb.and(predicates.toArray(new Predicate[predicates.size()]));
      return predicate;
    }
  };
}

Please, forgive me for the simple use case, but I hope you get the idea.

The solution proposed in his comment by @OsamaAbdulRehman looks interesting as well, although I honestly never tested it.

Upvotes: 4

Related Questions