Bug in multitenancy queries ? tenant_id = "no_tenant" is applied incorrectly

Support
#1

Hi,
encountered this when testing the no_tenant value in scope of New to Cuba / Issue with Multitenancy - #10 от пользователя tom.monnier - CUBA.Platform

I never get any results for an entity with a “no_tenant” value It seems the generated query is incorrect.

in org/eclipse/persistence/internal/databaseaccess/DatabaseCall.java:832

        List parameters = getParameters();
        int size = parameters.size();
        for (int index = 0; index < size; index++) {
            session.getPlatform().setParameterValueInDatabaseCall(parameters.get(index), (PreparedStatement)statement, index+1, session);
        }

the sql → 

SQLCall(SELECT t1.ID AS a1, t1.DATE_ AS a2, t1.DELETE_TS AS a3, t1.DELETED_BY AS a4, t1.LOGO_URL AS a5, t1.MANUFACTURER AS a6, t1.TENANT_ID AS a7, t1.VERSION AS a8, t1.MANUFACTURER_URL_ID AS a9, t0.ID AS a10, t0.DELETE_TS AS a11, t0.DELETED_BY AS a12, t0.DUTCH AS a13, t0.ENGLISH AS a14, t0.FRENCH AS a15, t0.GERMAN AS a16, t0.ITALIAN AS a17, t0.RUSSIAN AS a18, t0.TENANT_ID AS a19, t0.VERSION AS a20 FROM PMC_CATALOG t1 LEFT OUTER JOIN PMC_LOCALIZED_STRING t0 ON (t0.ID = t1.MANUFACTURER_URL_ID) WHERE ((t1.DELETE_TS IS NULL) AND ((? = ?) OR (t1.TENANT_ID = ?))) LIMIT ?, ?)

the vital part here is (? = ?)
this is applied because the column names for normal tenant entities have a TENANT_ID column where Cuba system entities have a SYS_TENANT_ID column.

the parameters:
image

it seems that first parameter should be the column name instead of the value

New to Cuba / Issue with Multitenancy
#2

@belyaev

I wanted to create a pull request for this, but fixing this one creates a new issue.

For the fix, this in HasTenantAdditionalCriteriaProvider

    @Override
    public String getAdditionalCriteria(Class entityClass) {
        MetaProperty metaProperty = tenantEntityOperation.getTenantMetaProperty(entityClass);
        return String.format("(:tenantId = '%s' or this.%s = :tenantId)", TenantProvider.NO_TENANT, metaProperty.getName());
    }

should be replaced by

    @Override
    public String getAdditionalCriteria(Class entityClass) {
        MetaProperty metaProperty = tenantEntityOperation.getTenantMetaProperty(entityClass);
        return String.format("(this.%s = '%s' or this.%s = :tenantId)", metaProperty.getName(), TenantProvider.NO_TENANT, metaProperty.getName());
    }

However, during startup an anonymous login is mandatory. In the as is this results in a condition on the query

'no_tenant' = 'no_tenant'

after applying the fix we get

t1.TENANT_ID = 'no_tenant'

As the anonymous user has a null value in the tenant_id the condition returns false. no anymous user is found and the server fails to start.

So it seems a bit more work will be necessary and I’m not sure what is the right way to go here :sob: :sob:

ps. The sec_user table has 3 columns regarding tenants (TENANT_ID, SYS_TENANT_ID & SYS_TENANT_ID_NN)
Something tells me to stay away from that stuff :scream:

#4

@belyaev

Is there a ticket for this issue? Or can you advice what to do for the anonymous user so I can create a pull request?

#5

Hi,

You can just create an issue in the corresponding GitHub project: GitHub - cuba-platform/multitenancy-addon: Implementation of a single database multi-tenancy support for CUBA applications.

#7

@belyaev
I created a ticket in the github project you mentioned on february 1, but there is still no comment or action taken.

This actually blocks me, the whole 'no_tenant" thing is not working.

#9

Hello Tom.

I’m investigating your problem. Can you tell me all steps to reproduce for 'no_tenant' = 'no_tenant' situation?

#11

basically by doing what is the expected behaviour…

  1. implement a multitenant entity
  2. login and create a tenant
  3. logout and login again as that tenant
  4. create an instance of the entity
  5. change the tenant_id for that record in the database to “no_tenant”
  6. refresh the browse screen -> should contain 1 row, but is empty
#12

Hi,
NO_TENANT is associated with users without a tenant. Users without tenant are global users and have access to all data in a system.
But tenant users should only see the data belonging to their tenant. It’s by design.

So, the described case is not valid.

#13

@subbotin

Ok, now I understand why the query is like that.
It seems the meaning of the NO_TENANT is different from what I expected. I actually came this conclusion because of another topic I opened with a replay from @belyaev