From 51bbf865e5ac7adc68d3bd395023fee4fe91f637 Mon Sep 17 00:00:00 2001 From: sjvans <30337871+sjvans@users.noreply.github.com> Date: Thu, 7 Dec 2023 13:13:28 +0100 Subject: [PATCH] fix: mod logs for deep with renamings (#71) closes https://github.com/cap-js/audit-logging/issues/70 \+ tests run with old and new db --- CHANGELOG.md | 6 ++ jest.config.js | 3 - lib/modification.js | 38 +++++--- package.json | 9 +- test/personal-data/crud.test.js | 58 +++++++++++ test/personal-data/srv/crud-service.cds | 122 ++++++++++++------------ 6 files changed, 158 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 814d735..5b77fde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). The format is based on [Keep a Changelog](http://keepachangelog.com/). +## Version 0.5.2 - 2023-12-07 + +### Fixed + +- Automatic personal data modification logging for deep data structures with renamings + ## Version 0.5.1 - 2023-11-30 ### Fixed diff --git a/jest.config.js b/jest.config.js index ac2cafd..c5f698f 100644 --- a/jest.config.js +++ b/jest.config.js @@ -1,6 +1,3 @@ -// FIXME: should not be necessary -process.env.CDS_ENV = 'better-sqlite' - const config = { testTimeout: 42222, testMatch: ['**/*.test.js'], diff --git a/lib/modification.js b/lib/modification.js index 36db3f6..b1146bb 100644 --- a/lib/modification.js +++ b/lib/modification.js @@ -17,27 +17,39 @@ const { let audit -const _applyTransitionRecursively = (transition, data, d) => { +const _applyTransitionRecursively = (transition, data, d, old = {}) => { for (const [k, v] of transition.mapping) { if (v.transition) { - if (Array.isArray(data[k])) { + const ok = v.ref?.[0] || k + if (!data[ok]) continue + if (Array.isArray(data[ok])) { d[k] = [] - for (let i = 0; i < data[k].length; i++) { - d[k].push({ _op: data[k][i]._op }) - if (data[k][i]._old) d[k][i]._old = {} - _applyTransitionRecursively(v.transition, data[k][i], d[k][i]) + for (let i = 0; i < data[ok].length; i++) { + const _op = data[ok][i]._op || d._op + d[k].push(_op ? { _op } : {}) + const _old = old[ok]?.[i] || data[ok][i]._old + if (_old) d[k][i]._old = _old + _applyTransitionRecursively(v.transition, data[ok][i], d[k][i], _old) + } + if (old[ok] && data[ok].length !== old[ok].length) { + for (const each of Object.values(old[ok]).filter(ele => !ele.__visited)) { + const i = d[k].push({ _op: 'delete' }) + d[k][i - 1]._old = each + _applyTransitionRecursively(v.transition, each, d[k][i - 1], each) + } } } else { - d[k] = { _op: data[k]._op } - if (data[k]._old) d[k]._old = {} - _applyTransitionRecursively(v.transition, data[k], d[k]) + d[k] = { _op: data[ok]._op || d._op } + const _old = old[ok] || data[ok]._old + if (_old) d[k]._old = _old + _applyTransitionRecursively(v.transition, data[ok], d[k], _old) } - continue } else if (v.ref) { if (v.ref[0] in data) d[k] = data[v.ref[0]] - if (data._old && v.ref[0] in data._old) d._old[k] = data._old[v.ref[0]] + if (v.ref[0] in old) d._old[k] = old[v.ref[0]] } } + if (Object.keys(old).length) old.__visited = true } // REVISIT: remove once old database impl is removed @@ -47,9 +59,9 @@ const _getDataWithAppliedTransitions = (data, req) => { // NOTE: there will only be transitions if old database impl is used const transition = query._transitions?.find(t => t.queryTarget.name === req.target.name) if (transition) { - d = { _op: data._op } + d = data._op ? { _op: data._op } : {} if (data._old) d._old = {} - _applyTransitionRecursively(transition, data, d) + _applyTransitionRecursively(transition, data, d, data._old) } return d || data } diff --git a/package.json b/package.json index 288e252..2c4f5c3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@cap-js/audit-logging", - "version": "0.5.1", + "version": "0.5.2", "description": "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.", "repository": "cap-js/audit-logging", "author": "SAP SE (https://www.sap.com)", @@ -14,7 +14,9 @@ ], "scripts": { "lint": "npx eslint .", - "test": "npx jest --silent" + "test": "npm run test-new-db && npm run test-old-db", + "test-new-db": "CDS_ENV='better-sqlite' npx jest --silent", + "test-old-db": "npx jest --silent" }, "peerDependencies": { "@sap/cds": "^7" @@ -25,7 +27,8 @@ "axios": "^1", "eslint": "^8", "express": "^4", - "jest": "^29" + "jest": "^29", + "sqlite3": "^5.1.6" }, "cds": { "requires": { diff --git a/test/personal-data/crud.test.js b/test/personal-data/crud.test.js index 3673b70..02e7e9f 100644 --- a/test/personal-data/crud.test.js +++ b/test/personal-data/crud.test.js @@ -2046,5 +2046,63 @@ describe('personal data audit logging in CRUD', () => { ] }) }) + + test('deep', async () => { + const c1 = { + c_emailAddress: 'foo@bar.baz', + c_addresses: [ + { + cpa_town: 'moo', + cpa_attachments: [ + { + aa_todo: 'boo' + }, + { + aa_todo: 'who' + } + ] + }, + { + cpa_town: 'shu' + } + ] + } + const { data: r1 } = await POST('/crud-3/C', c1, { auth: ALICE }) + Object.assign(c1, r1) + expect(_logs.length).toBe(5) + expect(_logs).toContainMatchObject({ attributes: [{ name: 'c_emailAddress', new: 'foo@bar.baz' }] }) + expect(_logs).toContainMatchObject({ attributes: [{ name: 'cpa_town', new: 'moo' }] }) + expect(_logs).toContainMatchObject({ attributes: [{ name: 'cpa_town', new: 'shu' }] }) + expect(_logs).toContainMatchObject({ attributes: [{ name: 'aa_todo', new: 'boo' }] }) + expect(_logs).toContainMatchObject({ attributes: [{ name: 'aa_todo', new: 'who' }] }) + + // reset logs + _logs = [] + + const c2 = { + c_emailAddress: 'foo@bar.bas', + c_addresses: [ + { + cpa_id: c1.c_addresses[0].cpa_id, + cpa_town: 'voo', + cpa_attachments: [ + { + aa_id: c1.c_addresses[0].cpa_attachments[0].aa_id, + aa_todo: 'doo' + } + ] + } + ] + } + await PATCH(`/crud-3/C/${c1.c_id}`, c2, { auth: ALICE }) + expect(_logs.length).toBe(5) + expect(_logs).toContainMatchObject({ + attributes: [{ name: 'c_emailAddress', old: 'foo@bar.baz', new: 'foo@bar.bas' }] + }) + expect(_logs).toContainMatchObject({ attributes: [{ name: 'cpa_town', old: 'moo', new: 'voo' }] }) + expect(_logs).toContainMatchObject({ attributes: [{ name: 'cpa_town', old: 'shu' }] }) + expect(_logs).toContainMatchObject({ attributes: [{ name: 'aa_todo', old: 'boo', new: 'doo' }] }) + expect(_logs).toContainMatchObject({ attributes: [{ name: 'aa_todo', old: 'who' }] }) + }) }) }) diff --git a/test/personal-data/srv/crud-service.cds b/test/personal-data/srv/crud-service.cds index 9865d3d..53eba55 100644 --- a/test/personal-data/srv/crud-service.cds +++ b/test/personal-data/srv/crud-service.cds @@ -16,33 +16,27 @@ service CRUD_1 { entity LastOne as projection on db.LastOne; entity Notes as projection on db.Notes; - entity AddressAttachment as projection on db.AddressAttachment { - *, - address.customer as customer - } + entity AddressAttachment as + projection on db.AddressAttachment { + *, + address.customer as customer + } - annotate Orders with @PersonalData: { - EntitySemantics: 'Other' - } { + annotate Orders with @PersonalData: {EntitySemantics: 'Other'} { misc @PersonalData.IsPotentiallySensitive; } - annotate OrderHeader with @PersonalData: { - EntitySemantics: 'Other' - } { + annotate OrderHeader with @PersonalData: {EntitySemantics: 'Other'} { description @PersonalData.IsPotentiallySensitive; } - annotate OrderHeader.sensitiveData with @PersonalData: { - EntitySemantics: 'Other' - } { + annotate OrderHeader.sensitiveData with @PersonalData: {EntitySemantics: 'Other'} { note @PersonalData.IsPotentiallySensitive; } - annotate Pages with @PersonalData : { - EntitySemantics: 'DataSubject' - // no DataSubjectRole for testing purposes - } { + annotate Pages with @PersonalData : {EntitySemantics: 'DataSubject' + // no DataSubjectRole for testing purposes + } { ID @PersonalData.FieldSemantics: 'DataSubjectID'; sensitive @PersonalData.IsPotentiallySensitive; personal @PersonalData.IsPotentiallyPersonal; @@ -59,45 +53,33 @@ service CRUD_1 { creditCardNo @PersonalData.IsPotentiallySensitive; } - annotate CustomerPostalAddress with @PersonalData: { - EntitySemantics: 'DataSubjectDetails' - } { + annotate CustomerPostalAddress with @PersonalData: {EntitySemantics: 'DataSubjectDetails'} { customer @PersonalData.FieldSemantics : 'DataSubjectID'; street @PersonalData.IsPotentiallySensitive; town @PersonalData.IsPotentiallyPersonal; } - annotate CustomerStatus with @PersonalData: { - EntitySemantics: 'DataSubjectDetails' - } { + annotate CustomerStatus with @PersonalData: {EntitySemantics: 'DataSubjectDetails'} { description @PersonalData.IsPotentiallySensitive; todo @PersonalData.IsPotentiallyPersonal; } - annotate StatusChange with @PersonalData: { - EntitySemantics: 'DataSubjectDetails' - } { + annotate StatusChange with @PersonalData: {EntitySemantics: 'DataSubjectDetails'} { description @PersonalData.IsPotentiallySensitive; secondKey @PersonalData.IsPotentiallyPersonal; } - annotate LastOne with @PersonalData: { - EntitySemantics: 'DataSubjectDetails' - } { + annotate LastOne with @PersonalData: {EntitySemantics: 'DataSubjectDetails'} { lastOneField @PersonalData.IsPotentiallySensitive; } - annotate AddressAttachment with @PersonalData: { - EntitySemantics: 'DataSubjectDetails' - } { + annotate AddressAttachment with @PersonalData: {EntitySemantics: 'DataSubjectDetails'} { customer @PersonalData.FieldSemantics : 'DataSubjectID'; description @PersonalData.IsPotentiallySensitive; todo @PersonalData.IsPotentiallyPersonal; } - annotate Notes with @PersonalData: { - EntitySemantics: 'Other' - } { + annotate Notes with @PersonalData: {EntitySemantics: 'Other'} { note @PersonalData.IsPotentiallySensitive; dummyArray @PersonalData.IsPotentiallyPersonal; } @@ -122,14 +104,13 @@ service CRUD_2 { entity CustomerPostalAddress as projection on db.CustomerPostalAddress; entity CustomerStatus as projection on db.CustomerStatus; - entity AddressAttachment as projection on db.AddressAttachment { - *, - address.customer as customer - } + entity AddressAttachment as + projection on db.AddressAttachment { + *, + address.customer as customer + } - annotate Customers with @PersonalData : { - EntitySemantics: 'Other' - } { + annotate Customers with @PersonalData : {EntitySemantics: 'Other'} { addresses @PersonalData.FieldSemantics: 'DataSubjectID'; } @@ -144,38 +125,61 @@ service CRUD_2 { } // invalid modeling (nothing personal/ sensitive), must have no effect - annotate CustomerStatus with @PersonalData: { - EntitySemantics: 'DataSubjectDetails' - }; + annotate CustomerStatus with @PersonalData: {EntitySemantics: 'DataSubjectDetails'}; } @path : '/crud-3' @requires: 'admin' service CRUD_3 { - entity R1 as projection on db.RBase { - key ID as r1_ID, - emailAddress as r1_emailAddress, - firstName as r1_firstName, - lastName as r1_lastName, - creditCardNo as r1_creditCardNo - } + entity R1 as + projection on db.RBase { + key ID as r1_ID, + emailAddress as r1_emailAddress, + firstName as r1_firstName, + lastName as r1_lastName, + creditCardNo as r1_creditCardNo + } annotate R1 with @PersonalData: { EntitySemantics: 'DataSubject', DataSubjectRole: 'Renamed Customer' }; - entity R2 as projection on R1 { - key r1_ID as r2_ID, - r1_emailAddress as r2_emailAddress, - r1_firstName as r2_firstName, - r1_lastName as r2_lastName, - r1_creditCardNo as r2_creditCardNo - } + entity R2 as + projection on R1 { + key r1_ID as r2_ID, + r1_emailAddress as r2_emailAddress, + r1_firstName as r2_firstName, + r1_lastName as r2_lastName, + r1_creditCardNo as r2_creditCardNo + } annotate R2 with @PersonalData: { EntitySemantics: 'DataSubject', DataSubjectRole: 'Twice Renamed Customer' }; + + entity C as + projection on CRUD_1.Customers { + key ID as c_id, + emailAddress as c_emailAddress, + addresses as c_addresses + }; + + + entity CPA as + projection on CRUD_1.CustomerPostalAddress { + key ID as cpa_id, + town as cpa_town, + customer as cpa_customer, + attachments as cpa_attachments + }; + + entity AA as + projection on CRUD_1.AddressAttachment { + key ID as aa_id, + todo as aa_todo, + address as aa_address + }; }