From c822a5b60315fdb213276795b2d9acd2afa59421 Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Thu, 18 Jun 2026 19:04:15 +0530 Subject: [PATCH 1/8] Use Client Credentials auth instead JWTBearer client credentials --- lib/sdm.js | 225 ++++++++++++++++++++++++++++++++---- lib/util/index.js | 45 +++++++- lib/util/messageConsts.js | 1 + test/lib/sdm.test.js | 178 +++++++++++++++++++++++++--- test/lib/util/index.test.js | 121 +++++++++++++++++++ 5 files changed, 532 insertions(+), 38 deletions(-) diff --git a/lib/sdm.js b/lib/sdm.js index 875404e3..16a86a02 100644 --- a/lib/sdm.js +++ b/lib/sdm.js @@ -24,8 +24,10 @@ const { getSecondaryTypeProperties, getUpdatedSecondaryProperties, getSdmInstanceName, + getSdmClientId, transformSDMServiceBindingToJWTBearerCredentialsDestination, - transformSDMServiceBindingToClientCredentialsDestination + transformSDMServiceBindingToClientCredentialsDestination, + isClientCredentialForced } = require("./util/index"); const { getDraftAttachments, @@ -77,7 +79,7 @@ const { const { getDestinationFromServiceBinding,retrieveJwt} = require('@sap-cloud-sdk/connectivity'); module.exports = class SDMAttachmentsService extends ( - require("@cap-js/attachments/srv/attachments/basic") + require("@cap-js/attachments/srv/basic") ) { async init() { this.creds = this.options.credentials; @@ -97,15 +99,18 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; }); return technicalUserDestn; } - async getDestination(req) { - // Cache destination on request object to avoid multiple calls - if (req._sdmDestination) { - return req._sdmDestination; + async getDestination(req, attachmentsEntity) { + // Cache destinations per-composition on the request: a parent with two + // attachment compositions (one annotated, one not) needs both flavors, + // and the non-rename callsites still hit '_default' for back-compat. + const cacheKey = attachmentsEntity?.name || '_default'; + if (req._sdmDestinations?.[cacheKey]) { + return req._sdmDestinations[cacheKey]; } const userJwt = retrieveJwt(req); let destination; - if (!cds.context?.user?.authInfo?.token?.payload?.origin) { + if (isClientCredentialForced(req, attachmentsEntity) || !cds.context?.user?.authInfo?.token?.payload?.origin) { let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; destination = await getDestinationFromServiceBinding({ destinationName: getSdmInstanceName(), @@ -121,7 +126,8 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; // Cache for subsequent calls in the same request - req._sdmDestination = destination; + if (!req._sdmDestinations) req._sdmDestinations = {}; + req._sdmDestinations[cacheKey] = destination; return destination; } getSDMCredentials() { @@ -155,7 +161,7 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; if (!req) { throw new Error('HTTP request context not available'); } - const destination = await this.getDestination(req); + const destination = await this.getDestination(req, attachments); const content = await readAttachment(Key, destination, this.creds); if(content && typeof content === 'object' && content.status && content.status == 200){ return content.data; @@ -250,7 +256,7 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; } async updateDraftAttachments(req, attachment, attachmentsEntity, secondaryPropertiesWithInvalidDefinitions, secondaryTypeProperties, compositionName) { - const attachmentData = await this.getAttachementDataInSDM(this.creds.uri, attachment.url, req); + const attachmentData = await this.getAttachementDataInSDM(this.creds.uri, attachment.url, req, attachmentsEntity); const filenameInSDM = attachmentData.filename; const context = { @@ -306,7 +312,7 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; if (Object.keys(updatedSecondaryProperties).length > 0) { const updateResult = await this.performAttachmentUpdate( - req, attachment, updatedSecondaryProperties, invalidDefinitions, filenameInRequest + req, attachment, updatedSecondaryProperties, invalidDefinitions, filenameInRequest, attachmentsEntity ); if (updateResult.error) { @@ -328,9 +334,9 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; return null; } - async performAttachmentUpdate(req, attachment, updatedSecondaryProperties, invalidDefinitions, filenameInRequest) { + async performAttachmentUpdate(req, attachment, updatedSecondaryProperties, invalidDefinitions, filenameInRequest, attachmentsEntity) { try { - const destination = await this.getDestination(req); + const destination = await this.getDestination(req, attachmentsEntity); const responseCode = await updateAttachment(req, attachment, this.creds, destination, updatedSecondaryProperties, invalidDefinitions); switch (responseCode) { @@ -489,8 +495,8 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; return errorResponse; } - async getAttachementDataInSDM(uri, objectId, req) { - const destination = await this.getDestination(req); + async getAttachementDataInSDM(uri, objectId, req, attachmentsEntity) { + const destination = await this.getDestination(req, attachmentsEntity); const response = await getAttachment(uri, destination, objectId); const responseData = { filename: response?.data?.succinctProperties["cmis:name"], folderId: response?.data?.succinctProperties["sap:parentIds"][0] }; return responseData; @@ -577,7 +583,7 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; } } if (!parentId) { - const destination = await this.getDestination(req); + const destination = await this.getDestination(req, attachments); const folderId = await getFolderIdByPath( req, this.creds, @@ -1100,6 +1106,16 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; linkUrl: req.data?.url, DraftAdministrativeData_DraftUUID: draftUUID[0].DraftAdministrativeData_DraftUUID, }; + + // Custom action bypasses srv.before hooks, so stamp the SDM technical + // user directly into the draft row when the annotation is in effect. + // Without this, CAP's managed aspect would persist req.user.id and the + // UI would show the human name even though DI shows the client_id. + if (isClientCredentialForced(req)) { + const clientId = getSdmClientId(); + if (clientId) this._setManagedUser(updatedFields, 'CREATE', clientId); + } + console.info(`[createLink] updating link in draft`, { updatedFields }); await updateLinkInDraft(req, updatedFields); @@ -1132,7 +1148,7 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; const newLinkUrl = req.data.url; const filenameToUpdate = existingAttachment.filename.replace(/\.url$/, ''); const objectIdToUpdate = existingAttachment.url; - const destination = await this.getDestination(req); + const destination = await this.getDestination(req, attachmentsEntity); const response = await editLink(objectIdToUpdate, filenameToUpdate, newLinkUrl, this.creds, destination); const status = response?.status || response?.code; @@ -1150,6 +1166,14 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; linkUrl: newLinkUrl, note: `__BASELINE_URL__:${baselineUrl}` }; + + // Custom action bypasses srv.before hooks, so stamp the SDM technical + // user directly. UPDATE event => modifiedBy only. + if (isClientCredentialForced(req)) { + const clientId = getSdmClientId(); + if (clientId) this._setManagedUser(updatedFields, 'UPDATE', clientId); + } + await editLinkInDraft(req, updatedFields); return { success: true, @@ -1168,6 +1192,7 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; const baseEntityName = req.target.name; const attachmentCompositions = this.getAttachmentCompositions({ name: baseEntityName }); const parentId = req.data?.ID; + const clientId = getSdmClientId(); for (const compositionName of attachmentCompositions) { const attachmentsEntityName = `${baseEntityName}.${compositionName}`; @@ -1187,10 +1212,96 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; }); } + // After draft activation, fix up createdBy/modifiedBy on the active + // table: + // - Freshly activated rows (new uploads / new links): set BOTH + // createdBy and modifiedBy to client_id. createdBy is set here once + // and never touched again. + // - Pre-existing rows whose draft entry was edited under client + // credentials in this session: set ONLY modifiedBy to client_id. + // createdBy is intentionally left as-is. + // Untouched rows (in draft only because the draft was opened, not + // edited) are filtered out by the draft.modifiedBy === client_id check + // captured in the snapshot. + if ( + clientId && + parentId && + attachmentsEntity["@SDM.useClientCredential"] === true && + req._sdmSaveSnapshot?.[compositionName] + ) { + const { preExistingActiveIds, draftClientStampedIds } = + req._sdmSaveSnapshot[compositionName]; + const draftIds = [...draftClientStampedIds]; + const freshlyActivated = draftIds.filter((id) => !preExistingActiveIds.has(id)); + const modifiedExisting = draftIds.filter((id) => preExistingActiveIds.has(id)); + + if (freshlyActivated.length > 0) { + await global.UPDATE(attachmentsEntityName) + .set({ createdBy: clientId, modifiedBy: clientId }) + .where({ ID: { in: freshlyActivated } }); + } + if (modifiedExisting.length > 0) { + await global.UPDATE(attachmentsEntityName) + .set({ modifiedBy: clientId }) + .where({ ID: { in: modifiedExisting } }); + } + } + await this.updateBaselinesForEntity(attachmentsEntityName); } } + /** + * Snapshot two ID sets per annotated attachment composition before draft + * activation runs: + * 1. preExistingActiveIds — rows already on the active table for this + * parent. Used to distinguish freshly activated rows from edits to + * pre-existing rows. + * 2. draftClientStampedIds — rows in the draft table whose modifiedBy is + * client_id, i.e. the user actually touched them in this draft session + * under @SDM.useClientCredential. Untouched draft rows (the draft was + * opened but the row wasn't edited) are excluded so we don't bump + * modifiedBy on rows the user only looked at. + * Stored on req._sdmSaveSnapshot[compositionName]. Only populated for + * compositions whose target carries @SDM.useClientCredential: true. + */ + async captureSaveSnapshot(req) { + const clientId = getSdmClientId(); + if (!clientId) return; + const baseEntityName = req.target.name; + const parentId = req.data?.ID; + if (!parentId) return; + + const attachmentCompositions = this.getAttachmentCompositions({ name: baseEntityName }); + if (attachmentCompositions.length === 0) return; + + req._sdmSaveSnapshot = {}; + for (const compositionName of attachmentCompositions) { + const attachmentsEntityName = `${baseEntityName}.${compositionName}`; + const attachmentsEntity = cds.model.definitions[attachmentsEntityName]; + if (!attachmentsEntity) continue; + if (attachmentsEntity["@SDM.useClientCredential"] !== true) continue; + + const upKey = attachmentsEntity.keys?.up_?.keys?.[0]?.$generatedFieldName || 'up__ID'; + const draftEntityName = `${attachmentsEntityName}.drafts`; + + const activeRows = await global.SELECT.from(attachmentsEntityName) + .columns('ID') + .where({ [upKey]: parentId }); + + const draftRows = cds.model.definitions[draftEntityName] + ? await global.SELECT.from(draftEntityName) + .columns('ID') + .where({ [upKey]: parentId, modifiedBy: clientId }) + : []; + + req._sdmSaveSnapshot[compositionName] = { + preExistingActiveIds: new Set(activeRows.map((r) => r.ID)), + draftClientStampedIds: new Set(draftRows.map((r) => r.ID)) + }; + } + } + async updateBaselinesForEntity(attachmentsEntityName) { for (const [attachmentKey] of this.originalUrlMap.entries()) { const attachment = await global.SELECT.one.from(attachmentsEntityName) @@ -1241,7 +1352,7 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; const baselineUrl = attachment.note.substring("__BASELINE_URL__:".length); if (baselineUrl && attachment.linkUrl !== baselineUrl) { - await this.revertLinkInSDM(attachment, baselineUrl, req); + await this.revertLinkInSDM(attachment, baselineUrl, req, attachmentsEntity); const attachmentKey = `${attachment.ID}`; this.originalUrlMap.delete(attachmentKey); } @@ -1250,11 +1361,11 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; } } - async revertLinkInSDM(draftAttachment, originalLinkUrl, req) { + async revertLinkInSDM(draftAttachment, originalLinkUrl, req, attachmentsEntity) { try { const filenameToUpdate = draftAttachment.filename.replace(/\.url$/, ''); const objectIdToUpdate = draftAttachment.url; - const destination = await this.getDestination(req); + const destination = await this.getDestination(req, attachmentsEntity); await editLink( objectIdToUpdate, @@ -1590,7 +1701,8 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; attachment, updatedSecondaryProperties, validationContext.invalidDefinitions, - newFilename + newFilename, + attachmentsEntity ); } @@ -1612,9 +1724,9 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; * Updates attachment in SDM and maps response to error object * @private */ - async _updateAttachmentInSDM(req, currentAttachment, attachment, updatedProperties, invalidDefinitions, filename) { + async _updateAttachmentInSDM(req, currentAttachment, attachment, updatedProperties, invalidDefinitions, filename, attachmentsEntity) { try { - const destination = await this.getDestination(req); + const destination = await this.getDestination(req, attachmentsEntity); const responseCode = await updateAttachment( req, { ...currentAttachment, ...attachment }, @@ -1687,6 +1799,51 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; } } + /** + * When @SDM.useClientCredential is set on the attachment target, the file is + * uploaded to SDM under the technical user (UAA clientid). To keep the plugin + * DB consistent with what DMS/DI shows, we override createdBy/modifiedBy on + * the request data so CAP's managed aspect doesn't fall back to the human + * user. Runs as a before-hook for CREATE/UPDATE/PUT on the attachment entity + * (and its drafts), and for SAVE on the parent entity (where we have to walk + * the attachment compositions, since the annotation lives on the target). + */ + applyClientCredentialUser(req) { + const clientId = getSdmClientId(); + if (!clientId) return; + + // Direct attachment-target operations: req.target carries the annotation. + if (isClientCredentialForced(req)) { + this._setManagedUser(req.data, req.event, clientId); + return; + } + + // Parent SAVE: annotation lives on the composition target, not on req.target. + if (req.event !== 'SAVE' && req.event !== 'CREATE' && req.event !== 'UPDATE') return; + if (!req.target?.elements) return; + + for (const [elementName, element] of Object.entries(req.target.elements)) { + if (element.type !== 'cds.Composition' || !element.target) continue; + const targetDef = cds.model.definitions[element.target]; + if (!targetDef?.["@SDM.useClientCredential"]) continue; + const rows = req.data?.[elementName]; + if (!Array.isArray(rows)) continue; + for (const row of rows) { + // _op === 'delete' rows shouldn't have managed fields touched. + if (row?._op === 'delete') continue; + this._setManagedUser(row, row?._op === 'update' ? 'UPDATE' : 'CREATE', clientId); + } + } + } + + _setManagedUser(data, event, clientId) { + if (!data || typeof data !== 'object') return; + if (event === 'CREATE' || event === 'PUT') { + data.createdBy = clientId; + } + data.modifiedBy = clientId; + } + registerHandlers(srv) { // First call the parent registerHandlers to ensure base functionality is registered if (super.registerHandlers) { @@ -1709,6 +1866,23 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; registerSDMHandlers(srv, entity, target) { + // When @SDM.useClientCredential is set, override createdBy/modifiedBy with + // the SDM technical user clientid so the plugin DB matches DMS/DI. Run + // these first so they win against later managed-aspect defaults. + const applyClientCred = this.applyClientCredentialUser.bind(this); + srv.before(["CREATE", "UPDATE", "PUT"], target, applyClientCred); + if (target.drafts) { + srv.before(["CREATE", "UPDATE", "PUT"], target.drafts, applyClientCred); + } + if (entity.drafts) { + // Draft activation: annotation lives on the composition target, so we + // hook on the parent SAVE and walk compositions inside the handler. + srv.before("SAVE", entity, applyClientCred); + } else { + // Non-draft parent UPDATE/CREATE may carry composition rows too. + srv.before(["CREATE", "UPDATE"], entity, applyClientCred); + } + // Handle DELETE and UPDATE for both draft and non-draft entities srv.before( ["DELETE","UPDATE"], @@ -1739,6 +1913,11 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; // Draft-specific handlers if (entity.drafts) { srv.before("DELETE", entity.drafts, this.handleDraftDiscardForLinks.bind(this)); + // Snapshot existing attachment IDs BEFORE activation so the after-SAVE + // stamp can target only freshly activated rows. Registered before the + // rename handler so req._sdmSaveSnapshot is populated for everything + // downstream that runs in the SAVE flow. + srv.before("SAVE", entity, this.captureSaveSnapshot.bind(this)); srv.after("SAVE", entity, this.handleDraftSaveForLinks.bind(this)); srv.before("SAVE", entity, this.draftEntityRenameHandler.bind(this)); } else { diff --git a/lib/util/index.js b/lib/util/index.js index 4ba5867e..907fbb38 100644 --- a/lib/util/index.js +++ b/lib/util/index.js @@ -1,6 +1,6 @@ const cds = require("@sap/cds"); const NodeCache = require("node-cache"); -const { sdmAnnotationAdditionalpropertyName, sdmAnnotationAdditionalproperty } = require("./messageConsts"); +const { sdmAnnotationAdditionalpropertyName, sdmAnnotationAdditionalproperty, sdmAnnotationUseClientCredential } = require("./messageConsts"); const cache = new NodeCache(); const { jwtBearerToken, serviceToken, decodeJwt } = require('@sap-cloud-sdk/connectivity'); @@ -314,6 +314,45 @@ function getSdmInstanceName() { return sdmInstanceName; } +// Returns the SDM service binding's UAA clientid — i.e. the technical user that +// DMS/DI sees when @SDM.useClientCredential is in effect. Used to keep the +// plugin DB's createdBy/modifiedBy aligned with the actual SDM principal. +function getSdmClientId() { + const data = process.env.VCAP_SERVICES; + if (!data) return null; + try { + const jsonData = JSON.parse(data); + if (jsonData.sdm && jsonData.sdm.length > 0) { + return jsonData.sdm[0].credentials?.uaa?.clientid || null; + } + } catch (e) { + return null; + } + return null; +} + +function isClientCredentialForced(req, attachmentsEntity) { + // Explicit entity wins — caller knows which composition it's working with, + // so per-composition selection is honored even when the parent has multiple + // attachment compositions with different annotations. + if (attachmentsEntity) + return attachmentsEntity[sdmAnnotationUseClientCredential] === true; + // Direct attachment-target call (e.g. CREATE on Incidents.references). + if (req.target?.[sdmAnnotationUseClientCredential] === true) return true; + // Parent-entity fallback (rename / SAVE handlers that haven't been threaded + // with the explicit entity): scan attachment compositions on req.target. + const elements = req.target?.elements; + if (elements) { + for (const element of Object.values(elements)) { + if (element?.type === 'cds.Composition' && element.target) { + const targetDef = cds.model.definitions[element.target]; + if (targetDef?.[sdmAnnotationUseClientCredential] === true) return true; + } + } + } + return false; +} + module.exports = { getConfigurations, isRepositoryVersioned, @@ -330,5 +369,7 @@ module.exports = { transformSDMServiceBindingToJWTBearerCredentialsDestination, transformSDMServiceBindingToClientCredentialsDestination, buildClientCredentialsDestination, - getSdmInstanceName + getSdmInstanceName, + getSdmClientId, + isClientCredentialForced }; diff --git a/lib/util/messageConsts.js b/lib/util/messageConsts.js index f3ef9f5c..cad12f3a 100644 --- a/lib/util/messageConsts.js +++ b/lib/util/messageConsts.js @@ -58,6 +58,7 @@ module.exports.linkNameConstraintMessage = (fileNameWithRestrictedCharacters, op }; module.exports.sdmAnnotationAdditionalpropertyName = "@SDM.Attachments.AdditionalProperty.name"; module.exports.sdmAnnotationAdditionalproperty = "@SDM.Attachments.AdditionalProperty"; +module.exports.sdmAnnotationUseClientCredential = "@SDM.useClientCredential"; module.exports.updateAttachmentError = "Could not update the attachment"; module.exports.sdmRolesErrorMessage = "Unable to rename the file due to an error at the server"; module.exports.unsupportedProperties = "Unsupported properties"; diff --git a/test/lib/sdm.test.js b/test/lib/sdm.test.js index b7d12f52..de6ffdc1 100644 --- a/test/lib/sdm.test.js +++ b/test/lib/sdm.test.js @@ -5,6 +5,8 @@ const { getConfigurations, isRepositoryVersioned, getSdmInstanceName, + getSdmClientId, + isClientCredentialForced, isRestrictedCharactersInName, getStatusCondition, getPropertyTitles, @@ -116,7 +118,10 @@ jest.mock("../../lib/util", () => ({ getConfigurations: jest.fn(), isRepositoryVersioned: jest.fn(), getSdmInstanceName: jest.fn(), + getSdmClientId: jest.fn(), transformSDMServiceBindingToJWTBearerCredentialsDestination: jest.fn(), + transformSDMServiceBindingToClientCredentialsDestination: jest.fn(), + isClientCredentialForced: jest.fn(), isRestrictedCharactersInName: jest.fn(), getStatusCondition: jest.fn(), getPropertyTitles: jest.fn(), @@ -438,9 +443,7 @@ describe("SDMAttachmentsService", () => { describe("getDestination", () => { it("should get destination and cache it on request object", async () => { const service = new SDMAttachmentsService(); - const mockReq = { - _sdmDestination: undefined - }; + const mockReq = {}; cds.context = { user: { authInfo: { @@ -470,7 +473,7 @@ describe("SDMAttachmentsService", () => { serviceBindingTransformFn: expect.any(Function) }); expect(result).toEqual(mockDestination); - expect(mockReq._sdmDestination).toEqual(mockDestination); + expect(mockReq._sdmDestinations._default).toEqual(mockDestination); }); it("should return cached destination when available", async () => { @@ -478,7 +481,7 @@ describe("SDMAttachmentsService", () => { const service = new SDMAttachmentsService(); const cachedDestination = { url: "http://cached.com" }; const mockReq = { - _sdmDestination: cachedDestination + _sdmDestinations: { _default: cachedDestination } }; const result = await service.getDestination(mockReq); @@ -486,13 +489,59 @@ describe("SDMAttachmentsService", () => { expect(getDestinationFromServiceBinding).not.toHaveBeenCalled(); expect(result).toEqual(cachedDestination); }); + + it("should cache per-attachment-entity to support parents with multiple compositions", async () => { + jest.clearAllMocks(); + const service = new SDMAttachmentsService(); + const mockReq = {}; + cds.context = { + user: { + authInfo: { token: { payload: { origin: "sap.custom", ext_attr: { zdn: "sub" } } } } + } + }; + retrieveJwt.mockReturnValue("user-jwt"); + getSdmInstanceName.mockReturnValue("sdm-instance"); + const dest1 = { url: "http://a.com" }; + const dest2 = { url: "http://b.com" }; + getDestinationFromServiceBinding + .mockResolvedValueOnce(dest1) + .mockResolvedValueOnce(dest2); + + const annotated = { name: 'Refs', '@SDM.useClientCredential': true }; + const unannotated = { name: 'Plain' }; + + // The same request needs both flavors; per-entity cache makes this safe. + isClientCredentialForced + .mockReturnValueOnce(true) // for annotated entity + .mockReturnValueOnce(false); // for unannotated entity + + const result1 = await service.getDestination(mockReq, annotated); + const result2 = await service.getDestination(mockReq, unannotated); + + expect(result1).toEqual(dest1); + expect(result2).toEqual(dest2); + expect(mockReq._sdmDestinations.Refs).toEqual(dest1); + expect(mockReq._sdmDestinations.Plain).toEqual(dest2); + expect(getDestinationFromServiceBinding).toHaveBeenCalledTimes(2); + }); + + it("should reuse cached per-entity destination across calls", async () => { + jest.clearAllMocks(); + const service = new SDMAttachmentsService(); + const cached = { url: "http://cached.com" }; + const mockReq = { _sdmDestinations: { Refs: cached } }; + const entity = { name: 'Refs' }; + + const result = await service.getDestination(mockReq, entity); + + expect(getDestinationFromServiceBinding).not.toHaveBeenCalled(); + expect(result).toEqual(cached); + }); }); it("should use Client Credentials when origin is not present in token", async () => { jest.clearAllMocks(); const service = new SDMAttachmentsService(); - const mockReq = { - _sdmDestination: undefined - }; + const mockReq = {}; // Mock cds.context without origin in token payload cds.context = { @@ -524,15 +573,13 @@ describe("SDMAttachmentsService", () => { serviceBindingTransformFn: expect.any(Function) }); expect(result).toEqual(mockDestination); - expect(mockReq._sdmDestination).toEqual(mockDestination); + expect(mockReq._sdmDestinations._default).toEqual(mockDestination); }); it("should use JWT Bearer when origin is present in token", async () => { jest.clearAllMocks(); const service = new SDMAttachmentsService(); - const mockReq = { - _sdmDestination: undefined - }; + const mockReq = {}; // Mock cds.context with origin in token payload cds.context = { @@ -565,7 +612,7 @@ describe("SDMAttachmentsService", () => { serviceBindingTransformFn: expect.any(Function) }); expect(result).toEqual(mockDestination); - expect(mockReq._sdmDestination).toEqual(mockDestination); + expect(mockReq._sdmDestinations._default).toEqual(mockDestination); }); describe("getSDMCredentials", () => { it("should return credentials", () => { @@ -7240,4 +7287,109 @@ describe("SDMAttachmentsService", () => { expect(result.length).toBe(2); }); }); + + describe('applyClientCredentialUser', () => { + let service; + + beforeEach(() => { + service = new SDMAttachmentsService(); + getSdmClientId.mockReset(); + isClientCredentialForced.mockReset(); + }); + + it('does nothing when getSdmClientId returns null', () => { + getSdmClientId.mockReturnValue(null); + const req = { event: 'CREATE', target: { '@SDM.useClientCredential': true }, data: {} }; + service.applyClientCredentialUser(req); + expect(req.data.createdBy).toBeUndefined(); + expect(req.data.modifiedBy).toBeUndefined(); + }); + + it('sets createdBy and modifiedBy on direct CREATE when annotation is set', () => { + getSdmClientId.mockReturnValue('sb-clientid-xyz'); + isClientCredentialForced.mockReturnValue(true); + const req = { event: 'CREATE', target: { '@SDM.useClientCredential': true }, data: { filename: 'a.pdf' } }; + service.applyClientCredentialUser(req); + expect(req.data.createdBy).toBe('sb-clientid-xyz'); + expect(req.data.modifiedBy).toBe('sb-clientid-xyz'); + }); + + it('sets only modifiedBy on UPDATE', () => { + getSdmClientId.mockReturnValue('sb-clientid-xyz'); + isClientCredentialForced.mockReturnValue(true); + const req = { event: 'UPDATE', target: { '@SDM.useClientCredential': true }, data: { filename: 'a.pdf' } }; + service.applyClientCredentialUser(req); + expect(req.data.createdBy).toBeUndefined(); + expect(req.data.modifiedBy).toBe('sb-clientid-xyz'); + }); + + it('sets createdBy and modifiedBy on PUT', () => { + getSdmClientId.mockReturnValue('sb-clientid-xyz'); + isClientCredentialForced.mockReturnValue(true); + const req = { event: 'PUT', target: { '@SDM.useClientCredential': true }, data: {} }; + service.applyClientCredentialUser(req); + expect(req.data.createdBy).toBe('sb-clientid-xyz'); + expect(req.data.modifiedBy).toBe('sb-clientid-xyz'); + }); + + it('does nothing when annotation is absent on direct attachment target', () => { + getSdmClientId.mockReturnValue('sb-clientid-xyz'); + isClientCredentialForced.mockReturnValue(false); + const req = { event: 'CREATE', target: { elements: {} }, data: {} }; + service.applyClientCredentialUser(req); + expect(req.data.createdBy).toBeUndefined(); + expect(req.data.modifiedBy).toBeUndefined(); + }); + + it('walks composition rows on parent SAVE when composition target has annotation', () => { + getSdmClientId.mockReturnValue('sb-clientid-xyz'); + isClientCredentialForced.mockReturnValue(false); + cds.model.definitions['Test.Refs'] = { '@SDM.useClientCredential': true }; + cds.model.definitions['Test.OtherRefs'] = { '@SDM.useClientCredential': false }; + const req = { + event: 'SAVE', + target: { + elements: { + references: { type: 'cds.Composition', target: 'Test.Refs' }, + other: { type: 'cds.Composition', target: 'Test.OtherRefs' } + } + }, + data: { + references: [ + { ID: '1', filename: 'a.pdf' }, + { ID: '2', filename: 'b.pdf', _op: 'update' }, + { ID: '3', _op: 'delete' } + ], + other: [{ ID: '4', filename: 'c.pdf' }] + } + }; + service.applyClientCredentialUser(req); + // annotated composition rows get the override (delete row skipped) + expect(req.data.references[0].createdBy).toBe('sb-clientid-xyz'); + expect(req.data.references[0].modifiedBy).toBe('sb-clientid-xyz'); + expect(req.data.references[1].createdBy).toBeUndefined(); + expect(req.data.references[1].modifiedBy).toBe('sb-clientid-xyz'); + expect(req.data.references[2].createdBy).toBeUndefined(); + expect(req.data.references[2].modifiedBy).toBeUndefined(); + // unannotated composition rows untouched + expect(req.data.other[0].createdBy).toBeUndefined(); + expect(req.data.other[0].modifiedBy).toBeUndefined(); + }); + + it('does nothing on SAVE when no composition has the annotation', () => { + getSdmClientId.mockReturnValue('sb-clientid-xyz'); + isClientCredentialForced.mockReturnValue(false); + cds.model.definitions['Test.Plain'] = { '@SDM.useClientCredential': false }; + const req = { + event: 'SAVE', + target: { + elements: { plain: { type: 'cds.Composition', target: 'Test.Plain' } } + }, + data: { plain: [{ ID: '1' }] } + }; + service.applyClientCredentialUser(req); + expect(req.data.plain[0].createdBy).toBeUndefined(); + expect(req.data.plain[0].modifiedBy).toBeUndefined(); + }); + }); }); \ No newline at end of file diff --git a/test/lib/util/index.test.js b/test/lib/util/index.test.js index bd326b90..2d95eae4 100644 --- a/test/lib/util/index.test.js +++ b/test/lib/util/index.test.js @@ -1217,6 +1217,127 @@ describe("util", () => { expect(result).toBeNull(); }); }); + + describe("getSdmClientId", () => { + const originalEnv = process.env.VCAP_SERVICES; + + afterEach(() => { + if (originalEnv !== undefined) { + process.env.VCAP_SERVICES = originalEnv; + } else { + delete process.env.VCAP_SERVICES; + } + }); + + it("should extract clientid from SDM service binding's UAA credentials", () => { + process.env.VCAP_SERVICES = JSON.stringify({ + sdm: [ + { + name: "my-sdm", + credentials: { uaa: { clientid: "sb-clientid-xyz" } } + } + ] + }); + + const { getSdmClientId } = require("../../../lib/util/index"); + expect(getSdmClientId()).toBe("sb-clientid-xyz"); + }); + + it("should return null when sdm service is missing", () => { + process.env.VCAP_SERVICES = JSON.stringify({ other: [] }); + const { getSdmClientId } = require("../../../lib/util/index"); + expect(getSdmClientId()).toBeNull(); + }); + + it("should return null when uaa.clientid is missing", () => { + process.env.VCAP_SERVICES = JSON.stringify({ + sdm: [{ name: "my-sdm", credentials: { uaa: {} } }] + }); + const { getSdmClientId } = require("../../../lib/util/index"); + expect(getSdmClientId()).toBeNull(); + }); + + it("should return null when VCAP_SERVICES is unset", () => { + delete process.env.VCAP_SERVICES; + const { getSdmClientId } = require("../../../lib/util/index"); + expect(getSdmClientId()).toBeNull(); + }); + + it("should return null on malformed VCAP_SERVICES", () => { + process.env.VCAP_SERVICES = "not-json"; + const { getSdmClientId } = require("../../../lib/util/index"); + expect(getSdmClientId()).toBeNull(); + }); + }); + + describe("isClientCredentialForced", () => { + const ANNOT = '@SDM.useClientCredential'; + + beforeEach(() => { + cds.model = { definitions: {} }; + }); + + it("should return true when explicit attachmentsEntity has the annotation", () => { + const { isClientCredentialForced } = require("../../../lib/util/index"); + const entity = { [ANNOT]: true }; + expect(isClientCredentialForced({}, entity)).toBe(true); + }); + + it("should return false when explicit attachmentsEntity lacks the annotation, even if a sibling has it", () => { + // Explicit entity wins — per-composition selection. + cds.model.definitions['Test.AnnotatedSibling'] = { [ANNOT]: true }; + const req = { + target: { + elements: { + annotated: { type: 'cds.Composition', target: 'Test.AnnotatedSibling' } + } + } + }; + const { isClientCredentialForced } = require("../../../lib/util/index"); + const unannotatedEntity = {}; + expect(isClientCredentialForced(req, unannotatedEntity)).toBe(false); + }); + + it("should return true when annotation is on req.target directly", () => { + const { isClientCredentialForced } = require("../../../lib/util/index"); + const req = { target: { [ANNOT]: true } }; + expect(isClientCredentialForced(req)).toBe(true); + }); + + it("should fall back to scanning composition targets on the parent", () => { + cds.model.definitions['Test.AnnotatedComp'] = { [ANNOT]: true }; + cds.model.definitions['Test.PlainComp'] = {}; + const req = { + target: { + elements: { + annotated: { type: 'cds.Composition', target: 'Test.AnnotatedComp' }, + other: { type: 'cds.Composition', target: 'Test.PlainComp' } + } + } + }; + const { isClientCredentialForced } = require("../../../lib/util/index"); + expect(isClientCredentialForced(req)).toBe(true); + }); + + it("should return false when no annotation anywhere", () => { + cds.model.definitions['Test.PlainOnly'] = {}; + const req = { + target: { + elements: { + other: { type: 'cds.Composition', target: 'Test.PlainOnly' } + } + } + }; + const { isClientCredentialForced } = require("../../../lib/util/index"); + expect(isClientCredentialForced(req)).toBe(false); + }); + + it("should return false when req.target has no elements", () => { + const { isClientCredentialForced } = require("../../../lib/util/index"); + expect(isClientCredentialForced({ target: {} })).toBe(false); + expect(isClientCredentialForced({})).toBe(false); + }); + }); }); describe("getPropertyTitles edge cases", () => { From a82389fed1b55a2750bd51740533a26596b9c0e8 Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Thu, 18 Jun 2026 19:08:09 +0530 Subject: [PATCH 2/8] lint fix --- lib/util/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/util/index.js b/lib/util/index.js index 907fbb38..9f3fa63a 100644 --- a/lib/util/index.js +++ b/lib/util/index.js @@ -325,7 +325,7 @@ function getSdmClientId() { if (jsonData.sdm && jsonData.sdm.length > 0) { return jsonData.sdm[0].credentials?.uaa?.clientid || null; } - } catch (e) { + } catch { return null; } return null; From 0b7eda31e2fb6a432094867b25c0f98497dc2db8 Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Thu, 18 Jun 2026 19:47:08 +0530 Subject: [PATCH 3/8] coverage and failure fix --- test/lib/sdm.test.js | 201 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 198 insertions(+), 3 deletions(-) diff --git a/test/lib/sdm.test.js b/test/lib/sdm.test.js index de6ffdc1..063418fe 100644 --- a/test/lib/sdm.test.js +++ b/test/lib/sdm.test.js @@ -69,7 +69,7 @@ let { mimeTypeInvalidError } = require("../../lib/util/messageConsts"); -jest.mock("@cap-js/attachments/srv/attachments/basic", () => class { +jest.mock("@cap-js/attachments/srv/basic", () => class { async init() { return Promise.resolve(); } @@ -4129,7 +4129,201 @@ describe("SDMAttachmentsService", () => { expect(service.updateBaselinesForEntity).not.toHaveBeenCalled(); }); }); - + + describe('captureSaveSnapshot', () => { + const { getSdmClientId } = require("../../lib/util"); + let service; + let req; + + beforeEach(() => { + jest.clearAllMocks(); + service = new SDMAttachmentsService(); + // Wire SELECT chain to chainable from -> columns -> where + global.SELECT.from = jest.fn().mockReturnThis(); + global.SELECT.columns = jest.fn().mockReturnThis(); + global.SELECT.where = jest.fn(); + req = { + target: { name: 'ProcessorService.Incidents' }, + data: { ID: 'parent-1' } + }; + }); + + it('returns early when no clientId', async () => { + getSdmClientId.mockReturnValue(null); + await service.captureSaveSnapshot(req); + expect(req._sdmSaveSnapshot).toBeUndefined(); + }); + + it('returns early when no parentId', async () => { + getSdmClientId.mockReturnValue('client-X'); + req.data = {}; + await service.captureSaveSnapshot(req); + expect(req._sdmSaveSnapshot).toBeUndefined(); + }); + + it('skips compositions without the annotation', async () => { + getSdmClientId.mockReturnValue('client-X'); + cds.model.definitions['ProcessorService.Incidents'] = { + elements: { + refs: { type: 'cds.Composition', target: 'ProcessorService.Incidents.refs' } + } + }; + cds.model.definitions['ProcessorService.Incidents.refs'] = { + includes: ['sap.attachments.Attachments'], + keys: { up_: { keys: [{ $generatedFieldName: 'up__ID' }] } } + // no @SDM.useClientCredential + }; + await service.captureSaveSnapshot(req); + expect(req._sdmSaveSnapshot).toEqual({}); + }); + + it('captures preExistingActiveIds and draftClientStampedIds when annotated', async () => { + getSdmClientId.mockReturnValue('client-X'); + cds.model.definitions['ProcessorService.Incidents'] = { + elements: { + references: { type: 'cds.Composition', target: 'ProcessorService.Incidents.references' } + } + }; + cds.model.definitions['ProcessorService.Incidents.references'] = { + includes: ['sap.attachments.Attachments'], + keys: { up_: { keys: [{ $generatedFieldName: 'up__ID' }] } }, + '@SDM.useClientCredential': true + }; + cds.model.definitions['ProcessorService.Incidents.references.drafts'] = { name: 'drafts' }; + + // Two SELECT calls: active first, draft second + global.SELECT.where + .mockResolvedValueOnce([{ ID: 'a1' }, { ID: 'a2' }]) + .mockResolvedValueOnce([{ ID: 'a2' }]); + + await service.captureSaveSnapshot(req); + + const snap = req._sdmSaveSnapshot.references; + expect(snap.preExistingActiveIds).toEqual(new Set(['a1', 'a2'])); + expect(snap.draftClientStampedIds).toEqual(new Set(['a2'])); + }); + }); + + describe('handleDraftSaveForLinks - client credential stamping', () => { + const { getSdmClientId } = require("../../lib/util"); + let service; + let req; + + beforeEach(() => { + jest.clearAllMocks(); + service = new SDMAttachmentsService(); + service.originalUrlMap = new Map(); + service.updateBaselinesForEntity = jest.fn(); + global.UPDATE.mockClear().mockImplementation(() => ({ + set: jest.fn().mockReturnThis(), + where: jest.fn().mockResolvedValue() + })); + req = { + target: { name: 'ProcessorService.Incidents' }, + data: { ID: 'parent-1' } + }; + cds.model.definitions['ProcessorService.Incidents'] = { + elements: { + references: { type: 'cds.Composition', target: 'ProcessorService.Incidents.references' } + } + }; + cds.model.definitions['ProcessorService.Incidents.references'] = { + includes: ['sap.attachments.Attachments'], + keys: { up_: { keys: [{ $generatedFieldName: 'up__ID' }] } }, + '@SDM.useClientCredential': true + }; + }); + + it('stamps both createdBy and modifiedBy on freshly activated rows; only modifiedBy on edited existing', async () => { + getSdmClientId.mockReturnValue('client-X'); + req._sdmSaveSnapshot = { + references: { + preExistingActiveIds: new Set(['old-1']), + draftClientStampedIds: new Set(['old-1', 'new-1']) + } + }; + + const updateCalls = []; + global.UPDATE.mockImplementation((entityName) => { + const obj = { + set: jest.fn(function (data) { updateCalls.push({ entityName, data, where: null }); return obj; }), + where: jest.fn(function (where) { updateCalls[updateCalls.length - 1].where = where; return Promise.resolve(); }) + }; + return obj; + }); + + await service.handleDraftSaveForLinks({}, req); + + // Filter out the mimeType fix; pick the createdBy/modifiedBy stamping calls + const stampingCalls = updateCalls.filter((c) => c.data.createdBy || c.data.modifiedBy); + expect(stampingCalls).toHaveLength(2); + + const fresh = stampingCalls.find((c) => c.data.createdBy); + expect(fresh.data).toEqual({ createdBy: 'client-X', modifiedBy: 'client-X' }); + expect(fresh.where).toEqual({ ID: { in: ['new-1'] } }); + + const modified = stampingCalls.find((c) => !c.data.createdBy); + expect(modified.data).toEqual({ modifiedBy: 'client-X' }); + expect(modified.where).toEqual({ ID: { in: ['old-1'] } }); + }); + + it('does nothing when annotation absent', async () => { + getSdmClientId.mockReturnValue('client-X'); + cds.model.definitions['ProcessorService.Incidents.references']['@SDM.useClientCredential'] = false; + req._sdmSaveSnapshot = { + references: { preExistingActiveIds: new Set(), draftClientStampedIds: new Set(['x']) } + }; + + const updateCalls = []; + global.UPDATE.mockImplementation((entityName) => { + const obj = { + set: jest.fn(function (data) { updateCalls.push({ entityName, data }); return obj; }), + where: jest.fn().mockResolvedValue() + }; + return obj; + }); + + await service.handleDraftSaveForLinks({}, req); + expect(updateCalls.filter((c) => c.data.createdBy || c.data.modifiedBy)).toHaveLength(0); + }); + + it('does nothing when no clientId', async () => { + getSdmClientId.mockReturnValue(null); + req._sdmSaveSnapshot = { + references: { preExistingActiveIds: new Set(), draftClientStampedIds: new Set(['x']) } + }; + + const updateCalls = []; + global.UPDATE.mockImplementation((entityName) => { + const obj = { + set: jest.fn(function (data) { updateCalls.push({ entityName, data }); return obj; }), + where: jest.fn().mockResolvedValue() + }; + return obj; + }); + + await service.handleDraftSaveForLinks({}, req); + expect(updateCalls.filter((c) => c.data.createdBy || c.data.modifiedBy)).toHaveLength(0); + }); + + it('does nothing when snapshot missing for composition', async () => { + getSdmClientId.mockReturnValue('client-X'); + req._sdmSaveSnapshot = {}; + + const updateCalls = []; + global.UPDATE.mockImplementation((entityName) => { + const obj = { + set: jest.fn(function (data) { updateCalls.push({ entityName, data }); return obj; }), + where: jest.fn().mockResolvedValue() + }; + return obj; + }); + + await service.handleDraftSaveForLinks({}, req); + expect(updateCalls.filter((c) => c.data.createdBy || c.data.modifiedBy)).toHaveLength(0); + }); + }); + describe('updateBaselinesForEntity', () => { let service; @@ -4258,7 +4452,8 @@ describe("SDMAttachmentsService", () => { expect(service.revertLinkInSDM).toHaveBeenCalledWith( draftAttachments[0], 'http://original.com', - req + req, + expect.any(Object) ); expect(service.originalUrlMap.has('attach1')).toBe(false); }); From c5b4d911d3718f595876944f6cea2159e70b9c61 Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Thu, 18 Jun 2026 20:05:12 +0530 Subject: [PATCH 4/8] test case fix --- lib/sdm.js | 15 ++++++++++++--- test/lib/sdm.test.js | 11 +++++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/lib/sdm.js b/lib/sdm.js index 16a86a02..15609046 100644 --- a/lib/sdm.js +++ b/lib/sdm.js @@ -78,9 +78,18 @@ const { } = require("./util/messageConsts"); const { getDestinationFromServiceBinding,retrieveJwt} = require('@sap-cloud-sdk/connectivity'); -module.exports = class SDMAttachmentsService extends ( - require("@cap-js/attachments/srv/basic") -) { +// Resolve the base class across @cap-js/attachments layouts: 3.8 ships it at +// "srv/basic", 3.12+ ships it at "srv/attachments/basic". Try both so the +// plugin works regardless of which version actually got installed. +function resolveAttachmentsBasic() { + try { + return require("@cap-js/attachments/srv/attachments/basic"); + } catch { + return require("@cap-js/attachments/srv/basic"); + } +} + +module.exports = class SDMAttachmentsService extends resolveAttachmentsBasic() { async init() { this.creds = this.options.credentials; // Temporary storage for original URLs during draft editing diff --git a/test/lib/sdm.test.js b/test/lib/sdm.test.js index 063418fe..2e72e1ef 100644 --- a/test/lib/sdm.test.js +++ b/test/lib/sdm.test.js @@ -69,7 +69,12 @@ let { mimeTypeInvalidError } = require("../../lib/util/messageConsts"); -jest.mock("@cap-js/attachments/srv/basic", () => class { +// @cap-js/attachments switched the base-class path between versions: +// 3.8 ships it at "srv/basic", 3.12+ at "srv/attachments/basic". virtual:true +// lets jest.mock register a name that doesn't physically exist in the +// installed copy, so whichever layout sdm.js resolves at runtime, our mock +// wins. +const _attachmentsBasicMock = class { async init() { return Promise.resolve(); } @@ -84,7 +89,9 @@ jest.mock("@cap-js/attachments/srv/basic", () => class { registerHandlers(_srv) { // Mock parent registerHandlers } -}); +}; +jest.mock("@cap-js/attachments/srv/basic", () => _attachmentsBasicMock, { virtual: true }); +jest.mock("@cap-js/attachments/srv/attachments/basic", () => _attachmentsBasicMock, { virtual: true }); jest.mock("@sap-cloud-sdk/connectivity", () => ({ getDestinationFromServiceBinding: jest.fn(), retrieveJwt: jest.fn() From 47177e7dbdc036413e7cdf35faf358714f441b96 Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Thu, 25 Jun 2026 12:57:19 +0530 Subject: [PATCH 5/8] Update service.cds --- app/single-tenant/personal-space/incidents-app/srv/service.cds | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/single-tenant/personal-space/incidents-app/srv/service.cds b/app/single-tenant/personal-space/incidents-app/srv/service.cds index b17fc484..c4d397b2 100644 --- a/app/single-tenant/personal-space/incidents-app/srv/service.cds +++ b/app/single-tenant/personal-space/incidents-app/srv/service.cds @@ -29,6 +29,8 @@ service ProcessorService { extend my.Incidents with { references: Composition of many Attachments } extend my.Projects with { references: Composition of many Attachments } +annotate my.Incidents.references with @SDM.useClientCredential: false; + extend Attachments with { customProperty1 : Association to WDIRSCodeList @SDM.Attachments.AdditionalProperty: { From 813f72fd8e5e3b76f461a4e9990618f22e36d4d2 Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Fri, 26 Jun 2026 16:51:21 +0530 Subject: [PATCH 6/8] coverage fix --- test/lib/sdm.test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/lib/sdm.test.js b/test/lib/sdm.test.js index 8bb1cfdc..f25e8d62 100644 --- a/test/lib/sdm.test.js +++ b/test/lib/sdm.test.js @@ -7505,6 +7505,18 @@ describe("SDMAttachmentsService", () => { }); }); + describe('getDestination — per-composition cache', () => { + it('uses attachmentsEntity.name as cache key when provided', async () => { + const service = new SDMAttachmentsService(); + const mockDest = { url: 'http://cached-with-key/' }; + const req = { _sdmDestinations: { 'MyEntity.references': mockDest } }; + const attachmentsEntity = { name: 'MyEntity.references' }; + + const result = await service.getDestination(req, attachmentsEntity); + expect(result).toBe(mockDest); + }); + }); + describe('applyClientCredentialUser', () => { let service; From cc27eaa48d4c7bbedd332c6167884f3dfdfc4d1d Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Fri, 26 Jun 2026 16:58:32 +0530 Subject: [PATCH 7/8] coverage fix --- test/lib/sdm.test.js | 95 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/test/lib/sdm.test.js b/test/lib/sdm.test.js index f25e8d62..8abfee93 100644 --- a/test/lib/sdm.test.js +++ b/test/lib/sdm.test.js @@ -7517,6 +7517,84 @@ describe("SDMAttachmentsService", () => { }); }); + describe('_setManagedUser — branch coverage', () => { + let service; + beforeEach(() => { + service = new SDMAttachmentsService(); + }); + + it('returns silently when data is null', () => { + expect(() => service._setManagedUser(null, 'CREATE', 'client-id')).not.toThrow(); + }); + + it('returns silently when data is not an object (string)', () => { + expect(() => service._setManagedUser('not-an-object', 'CREATE', 'client-id')).not.toThrow(); + }); + + it('sets createdBy and modifiedBy on CREATE event', () => { + const data = {}; + service._setManagedUser(data, 'CREATE', 'client-id'); + expect(data.createdBy).toBe('client-id'); + expect(data.modifiedBy).toBe('client-id'); + }); + + it('sets createdBy and modifiedBy on PUT event', () => { + const data = {}; + service._setManagedUser(data, 'PUT', 'client-id'); + expect(data.createdBy).toBe('client-id'); + expect(data.modifiedBy).toBe('client-id'); + }); + + it('sets only modifiedBy on UPDATE event', () => { + const data = {}; + service._setManagedUser(data, 'UPDATE', 'client-id'); + expect(data.createdBy).toBeUndefined(); + expect(data.modifiedBy).toBe('client-id'); + }); + }); + + describe('_rejectIfVirusScanLargeFile — file size threshold', () => { + let service; + let origEnv; + + beforeEach(() => { + service = new SDMAttachmentsService(); + origEnv = cds.env; + cds.env = { requires: { sdm: { settings: {} } } }; + }); + + afterEach(() => { + cds.env = origEnv; + }); + + it('returns silently when file size is below the threshold', () => { + const req = { reject: jest.fn() }; + service._rejectIfVirusScanLargeFile(req, 1024); + expect(req.reject).not.toHaveBeenCalled(); + }); + + it('rejects with 409 when file exceeds threshold and virus scan is enabled (string "true")', () => { + cds.env.requires.sdm.settings.isVirusScanEnabled = 'true'; + const req = { reject: jest.fn() }; + service._rejectIfVirusScanLargeFile(req, 500 * 1024 * 1024); // 500MB + expect(req.reject).toHaveBeenCalledWith(409, expect.any(String)); + }); + + it('rejects with 409 when file exceeds threshold and virus scan is enabled (boolean true)', () => { + cds.env.requires.sdm.settings.isVirusScanEnabled = true; + const req = { reject: jest.fn() }; + service._rejectIfVirusScanLargeFile(req, 500 * 1024 * 1024); + expect(req.reject).toHaveBeenCalledWith(409, expect.any(String)); + }); + + it('does NOT reject when file exceeds threshold but virus scan is not enabled', () => { + cds.env.requires.sdm.settings.isVirusScanEnabled = 'false'; + const req = { reject: jest.fn() }; + service._rejectIfVirusScanLargeFile(req, 500 * 1024 * 1024); + expect(req.reject).not.toHaveBeenCalled(); + }); + }); + describe('applyClientCredentialUser', () => { let service; @@ -7620,6 +7698,23 @@ describe("SDMAttachmentsService", () => { expect(req.data.plain[0].createdBy).toBeUndefined(); expect(req.data.plain[0].modifiedBy).toBeUndefined(); }); + + it('returns early when req.event is not SAVE/CREATE/UPDATE and annotation absent', () => { + getSdmClientId.mockReturnValue('sb-clientid-xyz'); + isClientCredentialForced.mockReturnValue(false); + const req = { event: 'DELETE', target: { elements: {} }, data: { foo: 'bar' } }; + service.applyClientCredentialUser(req); + // No managed fields stamped — DELETE is not a write event + expect(req.data.createdBy).toBeUndefined(); + expect(req.data.modifiedBy).toBeUndefined(); + }); + + it('returns early when req.target.elements is missing', () => { + getSdmClientId.mockReturnValue('sb-clientid-xyz'); + isClientCredentialForced.mockReturnValue(false); + const req = { event: 'SAVE', target: {}, data: {} }; + expect(() => service.applyClientCredentialUser(req)).not.toThrow(); + }); }); // --------------------------------------------------------------------------- From 8ebb8cf4490ec34ab5dc5e77b643913c4924a6d3 Mon Sep 17 00:00:00 2001 From: Ankush Kumar Garg Date: Mon, 29 Jun 2026 13:49:35 +0530 Subject: [PATCH 8/8] fix issues --- lib/sdm.js | 7 ++++-- lib/util/index.js | 43 ++++++++++++++++++++++++------------- test/lib/sdm.test.js | 31 ++++++++++++++++++++++++++ test/lib/util/index.test.js | 17 +++++++++++++++ 4 files changed, 81 insertions(+), 17 deletions(-) diff --git a/lib/sdm.js b/lib/sdm.js index 51fa4d72..5a6691d5 100644 --- a/lib/sdm.js +++ b/lib/sdm.js @@ -1870,8 +1870,11 @@ let subdomain = cds.context?.user?.authInfo?.token?.payload?.ext_attr?.zdn; const rows = req.data?.[elementName]; if (!Array.isArray(rows)) continue; for (const row of rows) { - // _op === 'delete' rows shouldn't have managed fields touched. - if (row?._op === 'delete') continue; + // Skip rows the user did not touch: + // - 'delete' rows shouldn't have managed fields written + // - 'read' rows are included by CAP for reference / unchanged children; + // stamping modifiedBy here would falsely mark them as edited + if (row?._op === 'delete' || row?._op === 'read') continue; this._setManagedUser(row, row?._op === 'update' ? 'UPDATE' : 'CREATE', clientId); } } diff --git a/lib/util/index.js b/lib/util/index.js index 1a7785ce..74479835 100644 --- a/lib/util/index.js +++ b/lib/util/index.js @@ -324,29 +324,42 @@ function getContentLength(content) { return -1; } +// Cache for parsed VCAP_SERVICES. VCAP_SERVICES is set once at process start and +// never changes at runtime, but the two getters below are called multiple times +// per request — without caching, each call re-runs JSON.parse on a multi-KB +// blob. We key the cache on the raw env-var string so any (test-time) reassignment +// invalidates it automatically. +let _vcapCache = { raw: undefined, parsed: undefined }; +function _getVcapServices() { + const data = process.env.VCAP_SERVICES; + if (_vcapCache.raw === data) return _vcapCache.parsed; + let parsed = null; + if (data) { + try { + parsed = JSON.parse(data); + } catch { + parsed = null; + } + } + _vcapCache = { raw: data, parsed }; + return parsed; +} + function getSdmInstanceName() { - let data = process.env.VCAP_SERVICES; - let sdmInstanceName = null; - let jsonData = JSON.parse(data); - if (jsonData.sdm && jsonData.sdm.length > 0) { - sdmInstanceName = jsonData.sdm[0].name; + const jsonData = _getVcapServices(); + if (jsonData?.sdm && jsonData.sdm.length > 0) { + return jsonData.sdm[0].name; } - return sdmInstanceName; + return null; } // Returns the SDM service binding's UAA clientid — i.e. the technical user that // DMS/DI sees when @SDM.useClientCredential is in effect. Used to keep the // plugin DB's createdBy/modifiedBy aligned with the actual SDM principal. function getSdmClientId() { - const data = process.env.VCAP_SERVICES; - if (!data) return null; - try { - const jsonData = JSON.parse(data); - if (jsonData.sdm && jsonData.sdm.length > 0) { - return jsonData.sdm[0].credentials?.uaa?.clientid || null; - } - } catch { - return null; + const jsonData = _getVcapServices(); + if (jsonData?.sdm && jsonData.sdm.length > 0) { + return jsonData.sdm[0].credentials?.uaa?.clientid || null; } return null; } diff --git a/test/lib/sdm.test.js b/test/lib/sdm.test.js index 8abfee93..b5f92d4c 100644 --- a/test/lib/sdm.test.js +++ b/test/lib/sdm.test.js @@ -7683,6 +7683,37 @@ describe("SDMAttachmentsService", () => { expect(req.data.other[0].modifiedBy).toBeUndefined(); }); + it('does NOT stamp rows with _op === "read" (PR review fix: untouched rows must not be marked as modified)', () => { + getSdmClientId.mockReturnValue('sb-clientid-xyz'); + isClientCredentialForced.mockReturnValue(false); + cds.model.definitions['Test.ReadRefs'] = { '@SDM.useClientCredential': true }; + const req = { + event: 'SAVE', + target: { + elements: { + references: { type: 'cds.Composition', target: 'Test.ReadRefs' } + } + }, + data: { + references: [ + { ID: '1', filename: 'a.pdf', _op: 'create' }, + { ID: '2', filename: 'b.pdf', _op: 'read' }, // untouched — should NOT be stamped + { ID: '3', filename: 'c.pdf', _op: 'update' } + ] + } + }; + service.applyClientCredentialUser(req); + // create row → both fields stamped + expect(req.data.references[0].createdBy).toBe('sb-clientid-xyz'); + expect(req.data.references[0].modifiedBy).toBe('sb-clientid-xyz'); + // read row → completely untouched (the regression this guards against) + expect(req.data.references[1].createdBy).toBeUndefined(); + expect(req.data.references[1].modifiedBy).toBeUndefined(); + // update row → only modifiedBy stamped + expect(req.data.references[2].createdBy).toBeUndefined(); + expect(req.data.references[2].modifiedBy).toBe('sb-clientid-xyz'); + }); + it('does nothing on SAVE when no composition has the annotation', () => { getSdmClientId.mockReturnValue('sb-clientid-xyz'); isClientCredentialForced.mockReturnValue(false); diff --git a/test/lib/util/index.test.js b/test/lib/util/index.test.js index 77dd99e0..e5614310 100644 --- a/test/lib/util/index.test.js +++ b/test/lib/util/index.test.js @@ -1269,6 +1269,23 @@ describe("util", () => { const { getSdmClientId } = require("../../../lib/util/index"); expect(getSdmClientId()).toBeNull(); }); + + it("should re-parse VCAP_SERVICES when the env var changes (cache invalidation)", () => { + // Verifies that the VCAP cache keyed on the raw string invalidates + // correctly when the env var is reassigned — important for tests + // that swap VCAP_SERVICES between scenarios. + process.env.VCAP_SERVICES = JSON.stringify({ + sdm: [{ name: "first", credentials: { uaa: { clientid: "first-id" } } }] + }); + const { getSdmClientId } = require("../../../lib/util/index"); + expect(getSdmClientId()).toBe("first-id"); + + // Reassign — the cache MUST invalidate + process.env.VCAP_SERVICES = JSON.stringify({ + sdm: [{ name: "second", credentials: { uaa: { clientid: "second-id" } } }] + }); + expect(getSdmClientId()).toBe("second-id"); + }); }); describe("isClientCredentialForced", () => {