@Filter enabled on all Entity types instead of only on instances of TenantEntity

[bernardgut]

Hello again

Here is the scenario

1. I have entity A and B with their respective repository interfaces. A extends TenantEntity, B doesn't.
2. I have AController that is implemented the same as in your example code.
3. I have BController that is a public endpoint(non Authenticated). BController has a function to return some general information from the Database when you run a GET against it.

Unfortunately this setup fails because apparently the TenantServiceAspect enables the @filter automatically on all Data repositories regardless of if the Repository Entity is an instance of TenantEntity.

In my case it crashes even before, when you run TenantAssistance.resolveCurrentTenantIdentifier() because there is no token in the request to BController:

  @Around("execution(public * *(..)) && enableMultiTenancy()")
  public Object aroundExecution(final ProceedingJoinPoint pjp) throws Throwable {
    final Filter filter =
        this.entityManager
            .unwrap(Session.class) // requires transaction
            .enableFilter(TENANT_FILTER_NAME)
            .setParameter(
                TENANT_FILTER_ARGUMENT_NAME, TenantAssistance.resolveCurrentTenantIdentifier());
    filter.validate();
    return pjp.proceed();

but regardless, I think that in general usage the above method should not enable the filter if the Entity of the Repository is not an instance of TenantEntity. Otherwise it is not correct.

I've been trying to add this condition in the above code for a certain number of hours (And maybe even submit a PR) but I cant manage to find a clear enough documentation on how to get the entity information in the above method using the Aspect programming.

Any ideas?

Thanks and have a good day

• Bernard

[M-Devloo]

Hi @bernardgut,

Your comment made me smile, thank you for that 😄
Last night have recreated your issue and already implemented some counter measurements locally (branch to be pushed later on)

• A new annotation will be added to determine if a @Entity is not multi tenancy compliant
• In the TenantInterceptor this annotation is checked to determine if a tenant_id value needs to be added or not. Default it will now throw a UnknownTenantException as it expects the non multi tenancy annotation or that the entity extends from TenantEntity. If you add both, it will also throw an exception.

This is added in case you forget to extend the entity with TenantEntity, it will throw an exception (to keep the default closed statement in mind). If you don't want Multi tenancy, the annotation can be added on top of your entity and this will by pass the TenantInterceptor addTenantIdIfObjectIsTenantEntity() method. And it should indeed also bypass the Hibernate filter.

Everything is done and tested so far except the Hibernate filter which is for now always auto applied.
I have been playing around with reflection and specific Spring JPA hooks to dynamically enable / disable the filter in case the annotation is present on a @Entity but for now, the solutions that i have done is always breaching one of my statements.
But i haven't been through all the roads yet, so i will continue to check on how this can be properly be implemented.

Edit: Already have something brewing on that could possible work -> See comment below for solution

I will also publish the ways that i have already investigated in case you would like to work on it as well 👍

I will push the branch very soon and link it to this ticket.

Best regards,

Michael

[bernardgut]

With my limited understanding of this aspect thing I did this : in TenantServiceAspect I added :

  @Pointcut("execution(public * com.myexample.multitenancy.MultiTenant+.*(..))")
  void isMultitenantRepository(){
    /* aspect */
  }

  @Pointcut(value = "isMultitenantRepository()")
  void enableMultiTenancy2() {
    /* aspect */
  }


  @Pointcut("execution(public * org.springframework.data.repository.Repository+.*(..))")
  void isRepository() {
    /* aspect */
  }

  @Pointcut(value = "isRepository()")
  void enableMultiTenancy() {
    /* aspect */
  }

  @Around("execution(public * *(..)) && enableMultiTenancy() && enableMultiTenancy2()")
   ...

Then I added an empty MultiTenant interface :

public interface MultiTenant {
    /* Switch.
    To enable multi-tenancy on a repository, extends this from the repository interface
    */
}

that I use as a switch to enable multi-tenancy in selected repositories:

public interface UserRepository extends JpaRepository<User, Long>, MultiTenant {

	...
}

Simple enough and seems to work so far.

What do you think?

[M-Devloo]

Hey @bernardgut, your solution works, but now you are explicitly enabling Multi tenancy on a repository and if you do not define that annotation, no multi tenancy is applied.

The framework is created so that developers could worry-free implement Multi tenancy.
Thanks to Spring AOP, the hibernate filter & interceptor this is currently being achieved.

I spent some few hours on a working draft solution which is pushed on branch: NoMultiTenancySupport.
Didn't really have much time last couple of days, so this is what i came up with.

Some key changes:

• Multi tenancy is still enabled on all Repositories (all Highlights are still valid!)
• Spring AOP is not modified!
• Annotations are not inherited on class implementations (which makes sense), so using a third party Reflections library to perform a classpath scan on startup of the application which returns a list of Class<?> objects.
  The pointcut returns a ProceedingJoinPoint which contains the target class which we compare it to the classpath scanned classes. When the annotation is present, the hibernate filter gets disabled. If not, the filter gets enabled.

(The getEnabledFilter() check verifies if the entity has a column named tenant_id. If this is not present, an exception is thrown)

• Already added 2 tests that validate this, but still need to add a bunch of tests to make sure that this implementation is 100% water tight.
  -- TODO: targetClass.getClass().getInterfaces() should also support targetClass.getClass() as the annotation should also support implementations.

But for now, this draft should resolve the issue that you have logged.

Best regards,

Michael

Edit: I will continue working on this feature and merge it in the master branch when it is completely tested

[M-Devloo]

Still haven't found the necessary time to finish up the NoMultiTenancySupport branch, but the core design is already present.
Still some // TODO's that need to be resolved (like additional tests).
When this is completely tested and validated, NoMultiTenancy support will be added to the master branch

edit: 1/11/2021 -> Still need to spend a evening or two to clean up the branch and merge this feature into master.