Fix: skip logging for keys marked as personal data (#119)

Fixes: https://github.tools.sap/cap/issues/issues/16783

---------

Co-authored-by: sjvans <30337871+sjvans@users.noreply.github.com>
Co-authored-by: D050513 <sebastian.van.syckel@sap.com>

Author: mariayord

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.8.2 - tbd
+
+### Fixed
+
+- Erroneous modification log for non-updated key properties
+
 ## Version 0.8.1 - 2024-09-13
 
 ### Fixed

lib/modification.js

@@ -88,19 +88,19 @@ const addDiffToCtx = async function (req) {
 }
 addDiffToCtx._initial = true
 
-const _getOldAndNew = (action, row, key) => {
+const _getOldAndNew = (action, row, key, entity) => {
   let oldValue = action === 'Create' ? null : row._old && row._old[key]
-  if (oldValue === undefined) oldValue = null
+  if (oldValue === undefined) oldValue = action === 'Update' && key in entity.keys ? row[key] : null
   else if (Array.isArray(oldValue)) oldValue = JSON.stringify(oldValue)
   let newValue = action === 'Delete' ? null : row[key]
   if (newValue === undefined) newValue = null
   else if (Array.isArray(newValue)) newValue = JSON.stringify(newValue)
   return { oldValue, newValue }
 }
 
-const _addAttribute = (log, action, row, key) => {
+const _addAttribute = (log, action, row, key, entity) => {
   if (!log.attributes.find(ele => ele.name === key)) {
-    const { oldValue, newValue } = _getOldAndNew(action, row, key)
+    const { oldValue, newValue } = _getOldAndNew(action, row, key, entity)
     if (oldValue !== newValue) {
       const attr = { name: key }
       if (action !== 'Create') attr.old = oldValue
@@ -139,7 +139,7 @@ const _processorFnModification = (modificationLogs, model, req, beforeWrite) =>
     } else if (category === 'DataSubjectID') {
       addDataSubject(modificationLog, row, key, entity)
     } else if (category === 'IsPotentiallyPersonal' || category === 'IsPotentiallySensitive') {
-      _addAttribute(modificationLog, action, row, key)
+      _addAttribute(modificationLog, action, row, key, entity)
       // do not log the value of a sensitive attribute
       if (element['@PersonalData.IsPotentiallySensitive']) _maskAttribute(modificationLog.attributes, key)
     }

test/personal-data/crud.test.js

@@ -678,6 +678,35 @@ describe('personal data audit logging in CRUD', () => {
       })
     })
 
+    test('update entity with key as personal data', async () => {
+      const idMain = 'daac72b5-5b4a-4831-b559-d0e68baa3b22'
+      const idSub = 'f6407ee1-3af5-423a-9b18-83a004306524'
+      const DATA_SUBJECT_M = {
+        type: 'CRUD_1.MainEntities',
+        role: 'MainEntity',
+        id: { ID: idMain }
+      }
+      const responseMain = await POST(
+        '/crud-1/MainEntities',
+        { ID: idMain, subEntities: [{ ID: idSub, name: 'myName' }] },
+        { auth: ALICE }
+      )
+      expect(responseMain).toMatchObject({ status: 201 })
+
+      const response = await PATCH(`/crud-1/SubEntities(${idSub})`, { name: 'newName' }, { auth: ALICE })
+
+      expect(response).toMatchObject({ status: 200 })
+      expect(_logs).toContainMatchObject({
+        user: 'alice',
+        object: {
+          type: 'CRUD_1.SubEntities',
+          id: { ID: idSub }
+        },
+        data_subject: DATA_SUBJECT_M,
+        attributes: [{ name: 'name', old: 'myName', new: 'newName' }]
+      })
+    })
+
     test('update Pages with integers', async () => {
       const page = {
         sensitive: 999,

test/personal-data/srv/crud-service.cds

@@ -15,6 +15,8 @@ service CRUD_1 {
   entity StatusChange          as projection on db.StatusChange;
   entity LastOne               as projection on db.LastOne;
   entity Notes                 as projection on db.Notes;
+  entity MainEntities          as projection on db.MainEntities;
+  entity SubEntities           as projection on db.SubEntities;
 
   entity AddressAttachment     as
     projection on db.AddressAttachment {
@@ -95,6 +97,12 @@ service CRUD_1 {
     notes  @PersonalData.IsPotentiallySensitive  @PersonalData.IsPotentiallyPersonal;
     skills @PersonalData.IsPotentiallyPersonal;
   }
+
+  annotate SubEntities with @PersonalData  : {EntitySemantics: 'DataSubjectDetails'} {
+    mainEntity @PersonalData.FieldSemantics: 'DataSubjectID';
+    ID         @PersonalData.IsPotentiallyPersonal;
+    name       @PersonalData.IsPotentiallyPersonal;
+  }
 }
 
 @path    : '/crud-2'
@@ -132,7 +140,7 @@ service CRUD_2 {
 @requires: 'admin'
 service CRUD_3 {
 
-  entity R1                    as
+  entity R1  as
     projection on db.RBase {
       key ID           as r1_ID,
           emailAddress as r1_emailAddress,
@@ -146,7 +154,7 @@ service CRUD_3 {
     DataSubjectRole: 'Renamed Customer'
   };
 
-  entity R2                    as
+  entity R2  as
     projection on R1 {
       key r1_ID           as r2_ID,
           r1_emailAddress as r2_emailAddress,
@@ -160,23 +168,23 @@ service CRUD_3 {
     DataSubjectRole: 'Twice Renamed Customer'
   };
 
-  entity C                     as
+  entity C   as
     projection on CRUD_1.Customers {
       key ID           as c_id,
           emailAddress as c_emailAddress,
           addresses    as c_addresses
     };
 
 
-  entity CPA                   as
+  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
+  entity AA  as
     projection on CRUD_1.AddressAttachment {
       key ID      as aa_id,
           todo    as aa_todo,
@@ -188,9 +196,9 @@ service CRUD_3 {
 @requires: 'admin'
 service CRUD_4 {
 
-  entity RenamedMainEntities   as projection on db.MainEntities;
+  entity RenamedMainEntities as projection on db.MainEntities;
 
-  entity RenamedSubEntities    as
+  entity RenamedSubEntities  as
     projection on db.SubEntities {
       key ID as renamedID,
           name,
@@ -203,8 +211,8 @@ service CRUD_4 {
 @requires: 'admin'
 service CRUD_5 {
 
-  entity A                     as projection on db.A;
-  entity B                     as projection on db.B;
-  entity C                     as projection on db.C;
+  entity A as projection on db.A;
+  entity B as projection on db.B;
+  entity C as projection on db.C;
 
 }

test/personal-data/srv/fiori-service.cds

@@ -20,33 +20,27 @@ service Fiori_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;
@@ -63,45 +57,33 @@ service Fiori_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;
   }
@@ -115,14 +97,13 @@ service Fiori_2 {
 
   entity CustomerPostalAddress as projection on db.CustomerPostalAddress;
 
-  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';
   }