Feature #9218
Changes in internal tenant management
0%
Related issues
History
#1 Updated by Ovidiu Maxiniuc almost 2 years ago
After initial iteration implementation some changes in the management of FWD internal management of tenant feature.
- replacement of tenant identifier/key by the (integer) ID instead of its name (a
String). Currently, the tenant name is used in lots of tenant events as identifier. However, from my experiments, 4GL seems to use the id instead. It makes sense to do the same in FWD. The lookup and compare operations are much faster inintdata-type rather thanString. Aside, that, it is easier to identify the tenant type, directly:0for default tenant,- incrementing positive values for regular tenants
- decrementing negative values for super-tenants (yet, no support at all in FWD);
The switch is probably a good stop in the normalization oflandlorddatabase, too;
- also related to tenant ID. In 9057, it is computed at server startup/tenant creation, but there is no persistent storage for it. As such, the value might not be consistent among server execution. The internal H2 database which stores the tenant data must be altered to accommodate it;
- a third thing: the current tenant table in landlord database already features a UID (as a UUID value). However, this does not have a 4GL counterpart. I do not know the reason for its existence, but I can speculate: extension of value space, non-linearity of the values (same reason the youtube does not assign incrementing numbers to videos, for example) which increases security. The problem is that UUIDs do not map well with integer values used in 4GL, and using then requires additional CPU power. My question here is whether this is intended or can we drop them in favour of the simpler
int? - refactoring and normalization of
landlorddatabase.
Currently, the physical databases are 'added' directly to a tenant. This should not be the case. Although the physical databases are physically different in FWD, once a tenant is configured for a logical database, it must be able to access all its physical components. The current landlord database schema and admin flow allow incomplete logical database definitions.
#2 Updated by Ovidiu Maxiniuc almost 2 years ago
- Related to Feature #9057: enable intermixed access to default tenant (shared) data and tenant-specific data added
#3 Updated by Greg Shah almost 2 years ago
0for default tenant,- incrementing positive values for regular tenants
- decrementing negative values for super-tenants (yet, no support at all in FWD);
...
- a third thing: the current tenant table in landlord database already features a UID (as a UUID value). However, this does not have a 4GL counterpart. I do not know the reason for its existence, but I can speculate: extension of value space, non-linearity of the values (same reason the youtube does not assign incrementing numbers to videos, for example) which increases security. The problem is that UUIDs do not map well with integer values used in 4GL, and using then requires additional CPU power. My question here is whether this is intended or can we drop them in favour of the simpler
int?
The thing I don't like about a simple incrementing int is the security aspect. You should not be able to predict tenant ids. I am not apposed to getting rid of the UUIDs but we should assign random integer values instead of using a simple increment.
If you want to keep the negative values for super tenants, that is OK, but they should also be randomly generated.
#4 Updated by Ovidiu Maxiniuc almost 2 years ago
I did additional research and found this it gives more insights.
So it seems this is the way 4GL does it: assign the ids at the time the server starts. But, during the authentication, the id is not needed/required. Only internally, afterwards. From security PoV, I do not see a concern, at least accessing strictly from 4GL code.
However, we can generate it pseudo-randomly, but keeping the constraint I described above (regular with positive values, super-tenants - negative).
#5 Updated by Greg Shah almost 2 years ago
My concern is for any interface (now or introduced later) which takes that tenant id. It is just more secure if the id cannot be predicted.
we can generate it pseudo-randomly, but keeping the constraint I described above (regular with positive values, super-tenants - negative)
Good.
#6 Updated by Ovidiu Maxiniuc almost 2 years ago
I adjusted the implementation of the tenant ID to match the one from 4GL. As result, at each server restart, the unique ID will be assigned the tenants in an unspecified order. At the moment, the ids are incremented incremented by some value for each tenant, making them semi-random.
Regarding the last bullet in #9218-1. Using the Tenant Admin API from Multi-Tenancy does not impose a fully defined for all databases. My initial thoughts were to add some changes in the API to enforce that, at least by the addition of a new request for validation of tenant configuration. However, after a in-depth research I realized that this is not required. The FWD runtime should be able to handle these cases; if the tenant was not (correctly) defined a condition error will be raised either at authentication time, or when the database is accessed.
#7 Updated by Eric Faulhaber almost 2 years ago
Ovidiu, is this work in a task branch yet? I'd like to look it over to get a better understanding of the changes.
#8 Updated by Ovidiu Maxiniuc almost 2 years ago
I have cherry-picked the changes and put them in 9057a. Revision 15463. I hope the branch is not broken (compilable at least).
#9 Updated by Greg Shah almost 2 years ago
I adjusted the implementation of the tenant ID to match the one from 4GL. As result, at each server restart, the unique ID will be assigned the tenants in an unspecified order. At the moment, the ids are incremented incremented by some value for each tenant, making them semi-random.
The problem is that once one knows one id, they can check for other ids easily (above and below). Why not just use a random number and exclude the ones that exist already? The value can be turned negative of positive as needed.