cap-js / audit-logging

CDS plugin providing integration to the SAP Audit Log service as well as out-of-the-box personal data-related audit logging based on annotations.
https://cap.cloud.sap/docs/guides/data-privacy
Apache License 2.0
4 stars 3 forks source link

Provide Simple Interface to Create Audit Log for Provider Tenant #84

Closed Kkoile closed 4 months ago

Kkoile commented 7 months ago

When running in a tenant context we need to provide an audit log in the context of the provider tenant.
The audit logging implementation already has the logic of using the provider tenant id if no tenant id is provided. https://github.com/cap-js/audit-logging/blob/main/srv/log2restv2.js#L95 So I thought we could use:

await cds.unboxed(auditLogService).send("securityLog", {
  action: "some action",
  data: "some message",
  user: "SYSTEM",
  tenant: undefined // which is of course also undefined if not included in the object
});

But the audit logging service has a before handler to set the tenant id from the context, if not provided: https://github.com/cap-js/audit-logging/blob/main/srv/service.js#L11

So our current solution is to modify the tenant context before sending an audit log:

const originalTenant = cds.context.tenant;
cds.context.tenant = undefined;
await cds.unboxed(auditLogService).send("securityLog", {
  action: "some action",
  data: "some message",
  user: "SYSTEM"
});
cds.context.tenant = originalTenant;

It would be great if you could add an API to make it easier to provide a log in the context of the provider tenant. Something like:

await cds.unboxed(auditLogService).send("securityLog", {
  action: "some action",
  data: "some message",
  user: "SYSTEM",
  tenant: "$PROVIDER"
});

I know that I could also find out the provider tenant id by myself and provide it when sending the audit log, but to keep the code clean and don't reimplement what you already do, this addition would be great.

If you agree that this is a valid feature request, I could also provide a pull request, if you like.

David-Kunz commented 5 months ago

Hi @Kkoile ,

We already have an API to set the tenant, cds.tx:

const srv = cds.unboxed(auditLogService)
await srv.tx({ tenant: 'abc' }, async srv => {
  // here, the tenant is set.
  await srv.send(...)
})

Does that help?

Thanks and best regards, David

Kkoile commented 5 months ago

Hi @David-Kunz ,

unfortunately this doesn't work, because I need to unset the tenant and using {tenant: undefined} or {tenant: null} will still keep the original tenant id.

sjvans commented 5 months ago

hi @Kkoile

setting a falsy value is gap, we'll look into it. in the meantime, you should be able to use { tenant: cds.env.requires['audit-log'].credentials.uaa.tenantid }.

best, sebastian

sjvans commented 4 months ago

hi @Kkoile

with the next release, you will be able to do tenant: undefined as part of the log payload.

best, sebastian