Add support for multitenant applications about table-api-generator HOT 12 CLOSED

Hi Paolo,

I find this a nice enhancement. And it matches also a new features we implemented for the next release. Namely the support for audit columns, which you can also generate with QuickSQL. We have in the upcoming version a parameter p_audit_column_mappings and p_audit_user_expression to configure the audit columns for the generator. This can help you to avoid triggers only for audit columns. But I would suggest to separate the data schema from the user interface schema to prevent direct access to the tables and to force the usage of the APIs. Do you use such an approach?

Should the security group id handled by the API internally to prevent providing other values? I mean, in your current implementation the user interface can provide a different security group id when the API is used - if it is handled inside the API, then the user interface schema has no chance to manipulate the parameter value.

Example: you give the generator the following parameters

...
p_security_group_mapping => '#PREFIX#_SECURITY_GROUP_ID=TO_NUMBER(SYS_CONTEXT('MY_SEC_CTX','MY_SECURITY_GROUP_ID'))',
p_enable_secure_view => true,
...

and the generator hides this column (as it is with the audit columns) and generates the internal calls like this:

  FUNCTION create_row (
    p_au_first_name IN "APP_USERS"."AU_FIRST_NAME"%TYPE,
    p_au_last_name  IN "APP_USERS"."AU_LAST_NAME"%TYPE,
    p_au_email      IN "APP_USERS"."AU_EMAIL"%TYPE /*UK*/,
    p_au_active_yn  IN "APP_USERS"."AU_ACTIVE_YN"%TYPE )
  RETURN "APP_USERS"."AU_ID"%TYPE
  IS
    v_return "APP_USERS"."AU_ID"%TYPE;
  BEGIN
    INSERT INTO "APP_USERS" (
      "AU_FIRST_NAME",
      "AU_LAST_NAME",
      "AU_EMAIL" /*UK*/,
      "AU_ACTIVE_YN",
      "AU_CREATED_ON",
      "AU_CREATED_BY",
      "AU_UPDATED_ON",
      "AU_UPDATED_BY",
      "AU_SECURITY_GROUP_ID" )
    VALUES (
      p_au_first_name,
      p_au_last_name,
      p_au_email,
      p_au_active_yn,
      systimestamp,
      coalesce(sys_context('apex$session','app_user'), sys_context('userenv','os_user'), sys_context('userenv','session_user')),
      systimestamp,
      coalesce(sys_context('apex$session','app_user'), sys_context('userenv','os_user'), sys_context('userenv','session_user')),
      TO_NUMBER(SYS_CONTEXT('MY_SEC_CTX','MY_SECURITY_GROUP_ID')) )
    RETURN
      "AU_ID"
    INTO
      v_return;
    RETURN v_return;
  END create_row;

What do you think?

Best regards
Ottmar

from table-api-generator.

Hi Ottmar,
thanks for your feedback.

Indeed I believe the solution you propose is way better from the security point of view, because it encapsulates the logic handling the security_group_id in the API code, which normally should not be reachable when separating the data schema from the user interface schema. I am aware of the benefits of this approach and I'll try to introduce it also in our project.

One remark though: I believe that the security_group_id should be assigned only once at insert time and it should not be allowed to change it via an update API call, because we assume that the data belongs to one tenant and it should never be possible to switch it to another tenant, especially when you are handling sensitive data.

Additionally the extended support for the audit columns of the upcoming version would be very welcome for us, so that the logic in the triggers generated by QuickSQL can be completely replaced by the functionality you described above..

In summary, I am looking forward to the next release and I'll try to do some testing on the DEV version (free time permitting...).

Thanks again,
Paolo

from table-api-generator.

Hi Paolo,

you are right. For updates and deletions the security_group_id have to be part of the where clause as it is in the security view. One more reason to hide it inside the API.

I think that the DML view and the 1:1 view (new in the upcoming version) should also have the security_group_id in the where clause. With this it would be consistent and we have no more reason for a special secure view. That means also we need only one new parameter p_security_group_mapping. What dou you think? Is this ok for you?

Best regards
Ottmar

from table-api-generator.

when I am thinking further - the security_group_id should be hidden from all generated views and APIs and only handled inside the API - with this it acts a bit like the VPD and is transparent to the user (not visible at all)...

from table-api-generator.

Hello Ottmar,
indeed, I think that should be the correct approach, anyway it will require us to do some changes in our current code to remove any references to the secure views and to remove the security group id from the API call parameters, so it may take some time before we migrate to the new TAPI version...

The security_group_id is not present in the secure view I generate and I think it should be correct like that. When I started thinking about adding support for the security_group_id to the TAPI generator, I opted to slip a secure view under the DML view to keep my changes as limited as possible and avoiding touching the core of the TAPI code generation. This is also why the security group id is still present in the generated API parameters in our modified version.

Finally what is it this new 1:1 view? Would that be a view which replicates the table structure 1:1? Can we change the name of this view using the parameters?

Thanks,
Paolo

from table-api-generator.

Hi Paolo,

yes, the 1:1 view replicates the structure and can be generated for convenience. When you separate the data from the ui in at least two different schemas, than you need some sort of views for the ui schema to read the data. As not every project needs a dml view there might be a need for generating such an read only view. Together with the security group id it makes even more sense to have that option in the generator.

And yes, you are right with the question regarding a naming option for the generated objects. In your case by naming the 1:1 view to table_name_sv you are almost done and need no changes here. Except for the changed API signatures with the missing security group id... sorry for that, but I think it is the cleaner way...

I think, I will add a naming option for the two generated views. I will need some time for the implementation - I am a bit busy at the moment...

Best regards
Ottmar

from table-api-generator.

Ottmar,
no worries, I believe that is the right way to go. Having already the possibility to keep the SV views is already a great thing. Also don't worry too much about the timing, as I am a bit busy as well...

Thanks for your efforts,
Paolo

from table-api-generator.

Ottmar,
just one more (maybe rhetorical) question:
are you planning to add the security group id also in the setters and getters?

I have just found that my own custom solution is not very good in that regard and it will happily return data from other tenants if called with the wrong ids...

Thanks,
Paolo

from table-api-generator.

Hi Paolo,

sure, the getters and setters will support this also. The same is true for the audit columns and for the row version column - here one of our test tables:

create table users (
  u_id          integer            generated always as identity,
  u_first_name  varchar2(15 char)                         ,
  u_last_name   varchar2(15 char)                         ,
  u_email       varchar2(30 char)               not null  ,
  u_active_yn   varchar2(1 char)   default 'Y'  not null  ,
  u_version_id  integer                         not null  ,
  u_created_on  date                            not null  , -- This is only for demo purposes.
  u_created_by  char(15 char)                   not null  , -- In reality we expect more
  u_updated_at  timestamp                       not null  , -- unified names and types
  u_updated_by  varchar2(15 char)               not null  , -- for audit columns.
  --
  primary key (u_id),
  unique (u_email),
  check (u_active_yn in ('Y', 'N'))
)

Here the generator call:

om_tapigen.compile_api(
  p_table_name                 => 'USERS',
  p_double_quote_names         => false,
  p_row_version_column_mapping => '#PREFIX#_VERSION_ID=global_version_sequence.nextval',
  p_audit_column_mappings      => 'created=#PREFIX#_CREATED_ON, created_by=#PREFIX#_CREATED_BY, updated=#PREFIX#_UPDATED_AT, updated_by=#PREFIX#_UPDATED_BY'
);

And here one of the generated setters in the package body:

PROCEDURE set_u_email (
  p_u_id         IN TAG_USERS.U_ID%TYPE /*PK*/,
  p_u_email      IN TAG_USERS.U_EMAIL%TYPE )
IS
BEGIN
  UPDATE
    USERS
  SET
    U_EMAIL      = p_u_email /*UK*/,
    U_VERSION_ID = global_version_sequence.nextval,
    U_UPDATED_AT = systimestamp,
    U_UPDATED_BY = coalesce(sys_context('apex$session','app_user'), sys_context('userenv','os_user'), sys_context('userenv','session_user'))
  WHERE
    U_ID = p_u_id;
END set_u_email;

The security group will be part of the where clause (also in the getters).

Best regards
Ottmar

from table-api-generator.

Hi Paolo,

the new version is out including the tenant support. Hopefully, it fulfills your requirements. If not, please let us know.

Best regards
Ottmar

from table-api-generator.

Hi Ottmar,
sorry for the late answer.

We are currently looking into the new version of TAPIGEN for a new project of ours, but it seems there are some constraints with the available database versions (11g is still around at few customer sites... ). :(

I hope I will have some time to test it in the coming weeks and try to discover all the new features you have added (really like the bulk processing one!) and I will open a new ticket in case of issues.

Thanks again for your hard work!
Paolo

from table-api-generator.

Hi Paolo,

you are welcome - and thank YOU for all the good ideas and great discussions. This has helped a lot to make the generator better as before :-)

Best regards
Ottmar

from table-api-generator.