diff --git a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java index 8ff266c2..4a47c36d 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java +++ b/sdm/src/main/java/com/sap/cds/sdm/constants/SDMErrorMessages.java @@ -137,6 +137,8 @@ private SDMErrorMessages() { "\nPlease contact your administrator for assistance with any necessary adjustments."; public static final String MAX_COUNT_ERROR_MESSAGE = "Cannot upload more than %s attachments."; public static final String FETCH_CHANGELOG_ERROR = "Could not fetch the changelog"; + public static final String CHANGELOG_NOT_AUTHORISED_ERROR = + "You do not have the required permissions to view the changelog. Please contact your administrator for access."; public static final String FAILED_TO_MOVE_ATTACHMENT = "Failed to move attachment"; public static final String INVALID_SECONDARY_PROPERTIES_FOR_MOVE_PREFIX = "Invalid secondary properties detected: "; diff --git a/sdm/src/main/java/com/sap/cds/sdm/persistence/DBQuery.java b/sdm/src/main/java/com/sap/cds/sdm/persistence/DBQuery.java index 219c4f2e..546c46be 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/persistence/DBQuery.java +++ b/sdm/src/main/java/com/sap/cds/sdm/persistence/DBQuery.java @@ -105,6 +105,49 @@ public CmisDocument getObjectIdForAttachmentID( return cmisDocument; } + public CmisDocument getObjectIdForAttachmentID( + CdsEntity attachmentEntity, + PersistenceService persistenceService, + String id, + String upIdKey, + String upId) { + logger.debug( + "Fetching objectId for attachment ID: {} upId: {} from entity: {}", + id, + upId, + attachmentEntity.getQualifiedName()); + CqnSelect q = + Select.from(attachmentEntity) + .columns( + "objectId", + "folderId", + "fileName", + "mimeType", + "contentId", + "linkUrl", + "uploadStatus") + .where(doc -> doc.get("ID").eq(id).and(doc.get(upIdKey).eq(upId))); + Result result = persistenceService.run(q); + Optional res = result.first(); + CmisDocument cmisDocument = new CmisDocument(); + if (res.isPresent()) { + logger.debug("Attachment found for ID: {} within parent scope", id); + Row row = res.get(); + cmisDocument.setObjectId(row.get("objectId").toString()); + cmisDocument.setFileName(row.get("fileName").toString()); + cmisDocument.setFolderId(row.get("folderId").toString()); + cmisDocument.setMimeType(row.get("mimeType").toString()); + cmisDocument.setContentId( + row.get("contentId") != null ? row.get("contentId").toString() : null); + cmisDocument.setUrl(row.get("linkUrl") != null ? row.get("linkUrl").toString() : null); + cmisDocument.setUploadStatus( + row.get("uploadStatus") != null ? row.get("uploadStatus").toString() : null); + } else { + logger.debug("No attachment found for ID: {} within parent scope (upId: {})", id, upId); + } + return cmisDocument; + } + public CmisDocument getAttachmentForObjectID( PersistenceService persistenceService, String id, AttachmentCopyEventContext context) { logger.debug("Fetching attachment for objectId: {}", id); diff --git a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMServiceGenericHandler.java b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMServiceGenericHandler.java index 2ae1af56..bfee568b 100644 --- a/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMServiceGenericHandler.java +++ b/sdm/src/main/java/com/sap/cds/sdm/service/handler/SDMServiceGenericHandler.java @@ -79,30 +79,71 @@ public void changelog(AttachmentLogContext context) throws IOException { CqnAnalyzer cqnAnalyzer = CqnAnalyzer.create(cdsModel); - Optional attachmentEntity = + Optional attachmentDraftEntity = cdsModel.findEntity(context.getTarget().getQualifiedName() + "_drafts"); Map targetKeys = cqnAnalyzer.analyze((CqnSelect) context.get("cqn")).targetKeyValues(); - // get the objectId against the Id String id = targetKeys.get("ID").toString(); logger.debug("Fetching changelog for attachment ID: {}", id); - CmisDocument cmisDocument = - dbQuery.getObjectIdForAttachmentID(attachmentEntity.get(), persistenceService, id); + // Derive parent entity name and upIdKey to scope the DB lookup by parent record + String targetQualifiedName = context.getTarget().getQualifiedName(); + int lastDot = targetQualifiedName.lastIndexOf('.'); + String parentEntityName = lastDot > 0 ? targetQualifiedName.substring(0, lastDot) : null; + Optional parentEntityOpt = + parentEntityName != null ? cdsModel.findEntity(parentEntityName) : Optional.empty(); + String upIdKey = + attachmentDraftEntity.isPresent() + ? SDMUtils.getUpIdKey(attachmentDraftEntity.get()) + : "up__ID"; + + // fetchUPIDFromCQN extracts the parent record key from the navigation path in the CQN + String upId = + parentEntityOpt.isPresent() + ? SDMUtils.fetchUPIDFromCQN((CqnSelect) context.get("cqn"), parentEntityOpt.get()) + : null; + + if (upId == null || upId.isEmpty()) { + logger.warn("Cannot determine parent scope for changelog request on attachment ID: {}", id); + throw new ServiceException(SDMUtils.getErrorMessage("FILE_NOT_FOUND_ERROR")); + } + + CmisDocument cmisDocument = lookupAttachmentScoped(attachmentDraftEntity, id, upIdKey, upId); if (cmisDocument.getFileName() == null || cmisDocument.getFileName().isEmpty()) { - // open attachment is triggered on non-draft entity logger.debug("Draft entity returned empty fileName, fetching from active entity"); - attachmentEntity = cdsModel.findEntity(context.getTarget().getQualifiedName()); + Optional attachmentActiveEntity = + cdsModel.findEntity(context.getTarget().getQualifiedName()); + cmisDocument = lookupAttachmentScoped(attachmentActiveEntity, id, upIdKey, upId); + } - cmisDocument = - dbQuery.getObjectIdForAttachmentID(attachmentEntity.get(), persistenceService, id); + if (cmisDocument.getObjectId() == null || cmisDocument.getObjectId().isEmpty()) { + logger.warn("Attachment ID {} not found within authorized parent scope (upId: {})", id, upId); + throw new ServiceException(SDMUtils.getErrorMessage("FILE_NOT_FOUND_ERROR")); } SDMCredentials sdmCredentials = tokenHandler.getSDMCredentials(); + try { + JSONObject objectResponse = + sdmService.getObject( + cmisDocument.getObjectId(), sdmCredentials, context.getUserInfo().isSystemUser()); + if (objectResponse == null) { + logger.warn("File not found in SDM for objectId: {}", cmisDocument.getObjectId()); + throw new ServiceException(SDMUtils.getErrorMessage("FILE_NOT_FOUND_ERROR")); + } + } catch (ServiceException e) { + if (e.getMessage() != null + && e.getMessage().equals(SDMUtils.getErrorMessage("USER_NOT_AUTHORISED_ERROR"))) { + logger.warn( + "User not authorized to view changelog for objectId: {}", cmisDocument.getObjectId()); + throw new ServiceException(SDMUtils.getErrorMessage("CHANGELOG_NOT_AUTHORISED_ERROR")); + } + throw e; + } + JSONObject jsonObject = sdmService.getChangeLog( cmisDocument.getObjectId(), sdmCredentials, context.getUserInfo().isSystemUser()); @@ -114,6 +155,15 @@ public void changelog(AttachmentLogContext context) throws IOException { logger.debug("END: Changelog event"); } + private CmisDocument lookupAttachmentScoped( + Optional entityOpt, String id, String upIdKey, String upId) { + if (entityOpt.isEmpty()) { + return new CmisDocument(); + } + return dbQuery.getObjectIdForAttachmentID( + entityOpt.get(), persistenceService, id, upIdKey, upId); + } + @On(event = "copyAttachments") public void copyAttachments(EventContext context) throws IOException { logger.debug("START: Copy attachments event"); @@ -195,11 +245,12 @@ public void handleDraftDiscardForLinks(DraftCancelEventContext context) throws I revertLinksForComposition(context, parentKeys, entry.getKey()); } } - revertNestedEntityLinks(context); + revertNestedEntityLinks(context, parentKeys); logger.debug("END: Handle draft discard for links"); } - private void revertNestedEntityLinks(DraftCancelEventContext context) throws IOException { + private void revertNestedEntityLinks( + DraftCancelEventContext context, Map parentKeys) throws IOException { logger.debug("START: Revert nested entity links"); CdsEntity parentDraftEntity = context.getTarget(); @@ -214,7 +265,7 @@ private void revertNestedEntityLinks(DraftCancelEventContext context) throws IOE .forEach( composition -> { try { - processNestedEntityComposition(context, composition); + processNestedEntityComposition(context, composition, parentKeys); } catch (IOException e) { throw new RuntimeException(e); } @@ -224,7 +275,8 @@ private void revertNestedEntityLinks(DraftCancelEventContext context) throws IOE } private void processNestedEntityComposition( - DraftCancelEventContext context, CdsElement composition) throws IOException { + DraftCancelEventContext context, CdsElement composition, Map parentKeys) + throws IOException { logger.debug("Processing nested entity composition: {}", composition.getName()); CdsAssociationType associationType = (CdsAssociationType) composition.getType(); @@ -243,12 +295,26 @@ private void processNestedEntityComposition( return; } - // Get the actual key field names from the entity instead of hardcoding "ID" + String upIdKey = SDMUtils.getUpIdKey(nestedDraftEntity.get()); + if (upIdKey == null || upIdKey.isEmpty()) { + logger.debug("No upIdKey found for nested entity: {}, skipping", targetEntityName); + return; + } + String parentKeyName = upIdKey.replaceFirst("^up__", ""); + Object parentId = parentKeys.get(parentKeyName); + if (parentId == null) { + logger.warn( + "parentId is null for upIdKey '{}' in entity '{}', skipping revert", + upIdKey, + targetEntityName); + return; + } List keyElementNames = getKeyElementNames(nestedDraftEntity.get()); Result nestedRecords = persistenceService.run( - Select.from(nestedDraftEntity.get()).where(e -> e.get("IsActiveEntity").eq(false))); + Select.from(nestedDraftEntity.get()) + .where(e -> e.get(upIdKey).eq(parentId).and(e.get("IsActiveEntity").eq(false)))); logger.debug("Found {} nested records to process", nestedRecords.rowCount()); for (Row nestedRecord : nestedRecords) { @@ -294,6 +360,13 @@ private void revertLinksForComposition( } String parentKeyName = upIdKey.replaceFirst("^up__", ""); Object parentId = parentKeys.get(parentKeyName); + if (parentId == null) { + logger.warn( + "parentId is null for upIdKey '{}', skipping link revert for: {}", + upIdKey, + attachmentCompositionDefinition); + return; + } CqnSelect selectDraftLinks = Select.from(draftEntity) diff --git a/sdm/src/test/java/integration/com/sap/cds/sdm/Api.java b/sdm/src/test/java/integration/com/sap/cds/sdm/Api.java index 212f67e4..502805d4 100644 --- a/sdm/src/test/java/integration/com/sap/cds/sdm/Api.java +++ b/sdm/src/test/java/integration/com/sap/cds/sdm/Api.java @@ -1354,6 +1354,41 @@ public Map fetchChangelog( } } + @Override + public String fetchChangelogRaw( + String appUrl, String entityName, String facetName, String entityID, String ID) + throws IOException { + String url = + "https://" + + appUrl + + "/odata/v4/" + + serviceName + + "/" + + entityName + + "(ID=" + + entityID + + ",IsActiveEntity=false)/" + + facetName + + "(up__ID=" + + entityID + + ",ID=" + + ID + + ",IsActiveEntity=false)/" + + serviceName + + ".changelog"; + + RequestBody body = RequestBody.create("{}", MediaType.parse("application/json")); + Request request = + new Request.Builder().url(url).addHeader("Authorization", token).post(body).build(); + + try (Response response = executeWithRetry(request)) { + String responseBody = response.body() != null ? response.body().string() : ""; + return response.code() + "|" + responseBody; + } catch (IOException e) { + throw new IOException("Error fetching changelog raw: " + e.getMessage(), e); + } + } + @Override public String createAttachmentInActive( String appUrl, String entityName, String facetName, String entityID) throws IOException { diff --git a/sdm/src/test/java/integration/com/sap/cds/sdm/ApiInterface.java b/sdm/src/test/java/integration/com/sap/cds/sdm/ApiInterface.java index 0cbe54f1..49512bc2 100644 --- a/sdm/src/test/java/integration/com/sap/cds/sdm/ApiInterface.java +++ b/sdm/src/test/java/integration/com/sap/cds/sdm/ApiInterface.java @@ -139,6 +139,10 @@ public Map fetchChangelog( String appUrl, String entityName, String facetName, String entityID, String ID) throws IOException; + public String fetchChangelogRaw( + String appUrl, String entityName, String facetName, String entityID, String ID) + throws IOException; + public String downloadSelectedAttachments( String appUrl, String entityName, String facetName, String entityID, List ids) throws IOException; diff --git a/sdm/src/test/java/integration/com/sap/cds/sdm/ApiMT.java b/sdm/src/test/java/integration/com/sap/cds/sdm/ApiMT.java index e0805f29..46804e11 100644 --- a/sdm/src/test/java/integration/com/sap/cds/sdm/ApiMT.java +++ b/sdm/src/test/java/integration/com/sap/cds/sdm/ApiMT.java @@ -1280,6 +1280,37 @@ public Map fetchChangelog( } } + @Override + public String fetchChangelogRaw( + String appUrl, String entityName, String facetName, String entityID, String ID) + throws IOException { + String url = + "https://" + + appUrl + + "/api/admin/" + + entityName + + "(ID=" + + entityID + + ",IsActiveEntity=false)/" + + facetName + + "(up__ID=" + + entityID + + ",ID=" + + ID + + ",IsActiveEntity=false)/AdminService.changelog"; + + RequestBody body = RequestBody.create("{}", MediaType.parse("application/json")); + Request request = + new Request.Builder().url(url).addHeader("Authorization", token).post(body).build(); + + try (Response response = executeWithRetry(request)) { + String responseBody = response.body() != null ? response.body().string() : ""; + return response.code() + "|" + responseBody; + } catch (IOException e) { + throw new IOException("Error fetching changelog raw: " + e.getMessage(), e); + } + } + @Override public String createAttachmentInActive( String appUrl, String entityName, String facetName, String entityID) throws IOException { diff --git a/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java b/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java index 67286c04..0eb1729c 100644 --- a/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java +++ b/sdm/src/test/java/integration/com/sap/cds/sdm/IntegrationTest_SingleFacet.java @@ -8705,4 +8705,109 @@ void testMoveAttachments_FromSdmFolder_WithSecondaryProperties_Preserved() throw api.deleteEntity(appUrl, entityName, targetEntity); api.deleteEntity(appUrl, entityName, sourceEntity); } + + @Test + @Order(104) + void testChangelogWithoutSDMRole() throws IOException { + System.out.println("Test (104): Changelog action denied for user without SDM role"); + + // Create entity and upload attachment using the privileged user + String entityId = api.createEntityDraft(appUrl, entityName, entityName2, srvpath); + assertNotNull(entityId, "Failed to create entity for changelog role test"); + + ClassLoader classLoader = getClass().getClassLoader(); + File file = new File(classLoader.getResource("sample.txt").getFile()); + assertTrue(file.exists(), "sample.txt should exist"); + + Map postData = new HashMap<>(); + postData.put("up__ID", entityId); + postData.put("mimeType", "text/plain"); + postData.put("createdAt", new Date().toString()); + postData.put("createdBy", "test@test.com"); + postData.put("modifiedBy", "test@test.com"); + + List createResponse = + api.createAttachment(appUrl, entityName, facetName, entityId, srvpath, postData, file); + assertEquals(2, createResponse.size()); + assertEquals("Attachment created", createResponse.get(0), "Attachment should be created"); + String attachmentId = createResponse.get(1); + + // Attempt changelog fetch using the no-role user + String raw = + apiNoRoles.fetchChangelogRaw(appUrl, entityName, facetName, entityId, attachmentId); + String[] parts = raw.split("\\|", 2); + int statusCode; + try { + statusCode = Integer.parseInt(parts[0]); + } catch (NumberFormatException e) { + fail("fetchChangelogRaw returned unexpected format (expected '|'): " + raw); + return; + } + String body = parts.length > 1 ? parts[1] : ""; + + assertTrue( + statusCode == 403 || statusCode == 500, + "Expected 403 or 500 for unauthorized changelog, got: " + statusCode); + assertTrue( + body.contains("view the changelog"), + "Error message should mention changelog access, got: " + body); + + // Clean up + api.deleteEntityDraft(appUrl, entityName, entityId); + } + + @Test + @Order(105) + void testChangelogForAttachmentOfDifferentEntity() throws IOException { + System.out.println( + "Test (105): Changelog action returns FILE_NOT_FOUND for attachment belonging to a different entity (IDOR guard)"); + + // Create two separate entities + String entityIdA = api.createEntityDraft(appUrl, entityName, entityName2, srvpath); + String entityIdB = api.createEntityDraft(appUrl, entityName, entityName2, srvpath); + assertNotNull(entityIdA, "Failed to create entity A"); + assertNotNull(entityIdB, "Failed to create entity B"); + + ClassLoader classLoader = getClass().getClassLoader(); + File file = new File(classLoader.getResource("sample.txt").getFile()); + assertTrue(file.exists(), "sample.txt should exist"); + + // Upload attachment to entity A + Map postData = new HashMap<>(); + postData.put("up__ID", entityIdA); + postData.put("mimeType", "text/plain"); + postData.put("createdAt", new Date().toString()); + postData.put("createdBy", "test@test.com"); + postData.put("modifiedBy", "test@test.com"); + + List createResponse = + api.createAttachment(appUrl, entityName, facetName, entityIdA, srvpath, postData, file); + assertEquals(2, createResponse.size()); + assertEquals("Attachment created", createResponse.get(0)); + String attachmentIdOfA = createResponse.get(1); + + // Request changelog for attachment of entity A, but scoped under entity B's navigation path + String raw = api.fetchChangelogRaw(appUrl, entityName, facetName, entityIdB, attachmentIdOfA); + String[] parts = raw.split("\\|", 2); + int statusCode; + try { + statusCode = Integer.parseInt(parts[0]); + } catch (NumberFormatException e) { + fail("fetchChangelogRaw returned unexpected format (expected '|'): " + raw); + return; + } + String body = parts.length > 1 ? parts[1] : ""; + + assertTrue( + statusCode == 404 || statusCode == 500, + "Expected 404 or 500 when attachment does not belong to the requested entity, got: " + + statusCode); + assertTrue( + body.contains("not found") || body.contains("Object not found"), + "Error message should indicate the attachment was not found, got: " + body); + + // Clean up + api.deleteEntityDraft(appUrl, entityName, entityIdA); + api.deleteEntityDraft(appUrl, entityName, entityIdB); + } } diff --git a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMServiceGenericHandlerTest.java b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMServiceGenericHandlerTest.java index 4a0aa163..cc36bfd5 100644 --- a/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMServiceGenericHandlerTest.java +++ b/sdm/src/test/java/unit/com/sap/cds/sdm/service/handler/SDMServiceGenericHandlerTest.java @@ -136,21 +136,33 @@ void testChangelogSuccess() throws IOException { when(mockUserInfo.isSystemUser()).thenReturn(false); // Mock the model and entity + CdsEntity mockParentEntity = mock(CdsEntity.class); when(cdsModel.findEntity("MyService.MyEntity.attachments_drafts")) .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("MyService.MyEntity")).thenReturn(Optional.of(mockParentEntity)); // Mock CqnAnalyzer cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); - // Mock DB query - when(dbQuery.getObjectIdForAttachmentID(draftEntity, persistenceService, "test-id-123")) + // Mock parent scope resolution + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("parent-id-123"); + + // Mock DB query (scoped by parent) + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "test-id-123", "up__ID", "parent-id-123")) .thenReturn(cmisDocument); // Mock token handler when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + // Mock SDM ACL pre-check + when(sdmService.getObject("object-123", sdmCredentials, false)).thenReturn(new JSONObject()); + // Mock SDM service when(sdmService.getChangeLog("object-123", sdmCredentials, false)) .thenReturn(mockChangeLogResult); @@ -170,8 +182,10 @@ void testChangelogSuccess() throws IOException { && jsonResult.has("version"); })); - verify(dbQuery).getObjectIdForAttachmentID(draftEntity, persistenceService, "test-id-123"); - verify(tokenHandler).getSDMCredentials(); + verify(dbQuery) + .getObjectIdForAttachmentID( + draftEntity, persistenceService, "test-id-123", "up__ID", "parent-id-123"); + verify(sdmService).getObject("object-123", sdmCredentials, false); verify(sdmService).getChangeLog("object-123", sdmCredentials, false); } @@ -202,21 +216,35 @@ void testChangelogWithSystemUser() throws IOException { when(mockUserInfo.isSystemUser()).thenReturn(true); // Mock the model and entity + CdsEntity mockParentEntity = mock(CdsEntity.class); when(cdsModel.findEntity("SystemService.SystemEntity.attachments_drafts")) .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("SystemService.SystemEntity")) + .thenReturn(Optional.of(mockParentEntity)); // Mock CqnAnalyzer cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); - // Mock DB query - when(dbQuery.getObjectIdForAttachmentID(draftEntity, persistenceService, "system-id-456")) + // Mock parent scope resolution + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("parent-id-456"); + + // Mock DB query (scoped by parent) + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "system-id-456", "up__ID", "parent-id-456")) .thenReturn(cmisDocument); // Mock token handler when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + // Mock SDM ACL pre-check + when(sdmService.getObject("system-object-456", sdmCredentials, true)) + .thenReturn(new JSONObject()); + // Mock SDM service when(sdmService.getChangeLog("system-object-456", sdmCredentials, true)) .thenReturn(mockChangeLogResult); @@ -283,21 +311,34 @@ void testChangelogServiceException() throws IOException { when(mockUserInfo.isSystemUser()).thenReturn(false); // Mock the model and entity + CdsEntity mockParentEntity = mock(CdsEntity.class); when(cdsModel.findEntity("ErrorService.ErrorEntity.attachments_drafts")) .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("ErrorService.ErrorEntity")).thenReturn(Optional.of(mockParentEntity)); // Mock CqnAnalyzer cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); - // Mock DB query - when(dbQuery.getObjectIdForAttachmentID(draftEntity, persistenceService, "error-id-789")) + // Mock parent scope resolution + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("parent-id-789"); + + // Mock DB query (scoped by parent) + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "error-id-789", "up__ID", "parent-id-789")) .thenReturn(cmisDocument); // Mock token handler when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + // Mock SDM ACL pre-check + when(sdmService.getObject("error-object-789", sdmCredentials, false)) + .thenReturn(new JSONObject()); + // Mock SDM service to throw ServiceException (runtime exception) when(sdmService.getChangeLog("error-object-789", sdmCredentials, false)) .thenThrow(new ServiceException("Network error")); @@ -312,6 +353,113 @@ void testChangelogServiceException() throws IOException { verify(sdmService).getChangeLog("error-object-789", sdmCredentials, false); } + @Test + void testChangelogSDMAccessDenied() throws IOException { + AttachmentLogContext mockLogContext = mock(AttachmentLogContext.class); + CqnAnalyzer mockCqnAnalyzer = mock(CqnAnalyzer.class); + AnalysisResult mockAnalysisResult = mock(AnalysisResult.class); + UserInfo mockUserInfo = mock(UserInfo.class); + CdsEntity mockTarget = mock(CdsEntity.class); + + Map targetKeys = new HashMap<>(); + targetKeys.put("ID", "denied-id-001"); + + cmisDocument.setFileName("denied-doc.pdf"); + cmisDocument.setObjectId("denied-object-001"); + + when(mockLogContext.getModel()).thenReturn(cdsModel); + when(mockLogContext.getTarget()).thenReturn(mockTarget); + when(mockTarget.getQualifiedName()).thenReturn("DeniedService.DeniedEntity.attachments"); + when(mockLogContext.get("cqn")).thenReturn(cqnSelect); + when(mockLogContext.getUserInfo()).thenReturn(mockUserInfo); + when(mockUserInfo.isSystemUser()).thenReturn(false); + + CdsEntity mockParentEntity = mock(CdsEntity.class); + when(cdsModel.findEntity("DeniedService.DeniedEntity.attachments_drafts")) + .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("DeniedService.DeniedEntity")) + .thenReturn(Optional.of(mockParentEntity)); + + cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); + when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); + when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); + + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("parent-id-001"); + + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "denied-id-001", "up__ID", "parent-id-001")) + .thenReturn(cmisDocument); + + when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + + // getObject throws USER_NOT_AUTHORISED_ERROR — same as SDMServiceImpl does on HTTP 403 + String unauthorisedMsg = SDMErrorMessages.USER_NOT_AUTHORISED_ERROR; + when(sdmService.getObject("denied-object-001", sdmCredentials, false)) + .thenThrow(new ServiceException(unauthorisedMsg)); + + ServiceException ex = + assertThrows( + ServiceException.class, () -> sdmServiceGenericHandler.changelog(mockLogContext)); + + assertTrue(ex.getMessage().contains("view the changelog")); + verify(sdmService).getObject("denied-object-001", sdmCredentials, false); + verify(sdmService, never()).getChangeLog(any(), any(), anyBoolean()); + } + + @Test + void testChangelogSDMObjectNotFound() throws IOException { + AttachmentLogContext mockLogContext = mock(AttachmentLogContext.class); + CqnAnalyzer mockCqnAnalyzer = mock(CqnAnalyzer.class); + AnalysisResult mockAnalysisResult = mock(AnalysisResult.class); + UserInfo mockUserInfo = mock(UserInfo.class); + CdsEntity mockTarget = mock(CdsEntity.class); + + Map targetKeys = new HashMap<>(); + targetKeys.put("ID", "missing-id-002"); + + cmisDocument.setFileName("missing-doc.pdf"); + cmisDocument.setObjectId("missing-object-002"); + + when(mockLogContext.getModel()).thenReturn(cdsModel); + when(mockLogContext.getTarget()).thenReturn(mockTarget); + when(mockTarget.getQualifiedName()).thenReturn("MissingService.MissingEntity.attachments"); + when(mockLogContext.get("cqn")).thenReturn(cqnSelect); + when(mockLogContext.getUserInfo()).thenReturn(mockUserInfo); + when(mockUserInfo.isSystemUser()).thenReturn(false); + + CdsEntity mockParentEntity = mock(CdsEntity.class); + when(cdsModel.findEntity("MissingService.MissingEntity.attachments_drafts")) + .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("MissingService.MissingEntity")) + .thenReturn(Optional.of(mockParentEntity)); + + cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); + when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); + when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); + + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("parent-id-002"); + + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "missing-id-002", "up__ID", "parent-id-002")) + .thenReturn(cmisDocument); + + when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + + // getObject returns null — object does not exist in SDM + when(sdmService.getObject("missing-object-002", sdmCredentials, false)).thenReturn(null); + + assertThrows(ServiceException.class, () -> sdmServiceGenericHandler.changelog(mockLogContext)); + + verify(sdmService).getObject("missing-object-002", sdmCredentials, false); + verify(sdmService, never()).getChangeLog(any(), any(), anyBoolean()); + } + @Test void testChangelogWithNullObjectId() throws IOException { // Arrange @@ -337,31 +485,35 @@ void testChangelogWithNullObjectId() throws IOException { when(mockUserInfo.isSystemUser()).thenReturn(false); // Mock the model and entity + CdsEntity mockParentEntity = mock(CdsEntity.class); when(cdsModel.findEntity("NullService.NullEntity.attachments_drafts")) .thenReturn(Optional.of(draftEntity)); + when(cdsModel.findEntity("NullService.NullEntity")).thenReturn(Optional.of(mockParentEntity)); // Mock CqnAnalyzer cqnAnalyzerMock.when(() -> CqnAnalyzer.create(cdsModel)).thenReturn(mockCqnAnalyzer); when(mockCqnAnalyzer.analyze(cqnSelect)).thenReturn(mockAnalysisResult); when(mockAnalysisResult.targetKeyValues()).thenReturn(targetKeys); - // Mock DB query to return document with null objectId - when(dbQuery.getObjectIdForAttachmentID(draftEntity, persistenceService, "null-object-id")) - .thenReturn(nullObjectIdDocument); - - // Mock token handler - when(tokenHandler.getSDMCredentials()).thenReturn(sdmCredentials); + // Mock parent scope resolution + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(draftEntity)).thenReturn("up__ID"); + sdmUtilsMock + .when(() -> SDMUtils.fetchUPIDFromCQN(cqnSelect, mockParentEntity)) + .thenReturn("null-parent-id"); - // Mock SDM service - when(sdmService.getChangeLog(null, sdmCredentials, false)) - .thenThrow(new IllegalArgumentException("ObjectId cannot be null")); + // Mock DB query (scoped) to return document with null objectId + when(dbQuery.getObjectIdForAttachmentID( + draftEntity, persistenceService, "null-object-id", "up__ID", "null-parent-id")) + .thenReturn(nullObjectIdDocument); - // Act & Assert + // Act & Assert — handler throws ServiceException (FILE_NOT_FOUND) when objectId is null assertThrows( - IllegalArgumentException.class, + ServiceException.class, () -> { sdmServiceGenericHandler.changelog(mockLogContext); }); + + verify(sdmService, never()).getChangeLog(any(), any(), anyBoolean()); } @Test @@ -2375,13 +2527,13 @@ void testRevertNestedEntityLinks_MainFlow() throws Exception { Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "revertNestedEntityLinks", DraftCancelEventContext.class); + "revertNestedEntityLinks", DraftCancelEventContext.class, Map.class); method.setAccessible(true); assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context); + method.invoke(sdmServiceGenericHandler, context, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -2419,13 +2571,13 @@ void testRevertNestedEntityLinks_MissingActiveEntity() throws Exception { Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "revertNestedEntityLinks", DraftCancelEventContext.class); + "revertNestedEntityLinks", DraftCancelEventContext.class, Map.class); method.setAccessible(true); assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context); + method.invoke(sdmServiceGenericHandler, context, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -2453,13 +2605,13 @@ void testRevertNestedEntityLinks_EmptyCompositions() throws Exception { Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "revertNestedEntityLinks", DraftCancelEventContext.class); + "revertNestedEntityLinks", DraftCancelEventContext.class, Map.class); method.setAccessible(true); assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context); + method.invoke(sdmServiceGenericHandler, context, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -2503,6 +2655,12 @@ void testRevertNestedEntityLinks_ComplexAttachments() throws Exception { when(nestedRecordsResult.iterator()).thenReturn(Arrays.asList(nestedRecord).iterator()); when(nestedRecord.get("ID")).thenReturn("chapter1"); + // Mock nestedDraftEntity key element so nestedEntityKeys gets "ID" populated + CdsElement nestedKeyElement = mock(CdsElement.class); + when(nestedKeyElement.isKey()).thenReturn(true); + when(nestedKeyElement.getName()).thenReturn("ID"); + when(nestedDraftEntity.elements()).thenReturn(Stream.of(nestedKeyElement)); + Map attachmentMapping = new HashMap<>(); attachmentMapping.put("AdminService.Attachments", "path1"); @@ -2536,6 +2694,7 @@ void testRevertNestedEntityLinks_ComplexAttachments() throws Exception { // Mock SDMUtils.getUpIdKey to return non-null value sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(attachmentDraftEntity)).thenReturn("up__ID"); + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(eq(nestedDraftEntity))).thenReturn("up__ID"); try (var attachmentUtilsMock = mockStatic( @@ -2554,13 +2713,15 @@ void testRevertNestedEntityLinks_ComplexAttachments() throws Exception { Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "revertNestedEntityLinks", DraftCancelEventContext.class); + "revertNestedEntityLinks", DraftCancelEventContext.class, Map.class); method.setAccessible(true); assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context); + Map parentKeysArg = new HashMap<>(); + parentKeysArg.put("ID", "test-book-id"); + method.invoke(sdmServiceGenericHandler, context, parentKeysArg); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -2603,7 +2764,7 @@ void testRevertNestedEntityLinks_ExceptionInProcessing() throws Exception { // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "revertNestedEntityLinks", DraftCancelEventContext.class); + "revertNestedEntityLinks", DraftCancelEventContext.class, Map.class); method.setAccessible(true); // Execute the test and expect RuntimeException to be thrown @@ -2611,7 +2772,7 @@ void testRevertNestedEntityLinks_ExceptionInProcessing() throws Exception { RuntimeException.class, () -> { try { - method.invoke(sdmServiceGenericHandler, context); + method.invoke(sdmServiceGenericHandler, context, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3284,6 +3445,9 @@ void testProcessNestedEntityComposition() throws Exception { when(context.getUserInfo()).thenReturn(userInfo); when(userInfo.isSystemUser()).thenReturn(false); + // Mock upIdKey for nested draft entity + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(eq(nestedDraftEntity))).thenReturn("up__ID"); + // Mock the static method call try (var attachmentUtilsMock = mockStatic( @@ -3316,14 +3480,19 @@ void testProcessNestedEntityComposition() throws Exception { // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + Map parentKeysArg = new HashMap<>(); + parentKeysArg.put("ID", "test-entity-id"); + method.invoke(sdmServiceGenericHandler, context, composition, parentKeysArg); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3361,14 +3530,17 @@ void testProcessNestedEntityComposition_NoDraftEntity() throws Exception { // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3416,14 +3588,17 @@ void testProcessNestedEntityComposition_EmptyAttachmentMapping() throws Exceptio // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3469,6 +3644,9 @@ void testProcessNestedEntityComposition_NoNestedRecords() throws Exception { Map attachmentMapping = new HashMap<>(); attachmentMapping.put("AdminService.Attachments", "path1"); + // Mock upIdKey for nested draft entity + sdmUtilsMock.when(() -> SDMUtils.getUpIdKey(eq(nestedDraftEntity))).thenReturn("up__ID"); + // Mock the static method call try (var attachmentUtilsMock = mockStatic( @@ -3485,14 +3663,19 @@ void testProcessNestedEntityComposition_NoNestedRecords() throws Exception { // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + Map parentKeysArg = new HashMap<>(); + parentKeysArg.put("ID", "test-entity-id"); + method.invoke(sdmServiceGenericHandler, context, composition, parentKeysArg); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3532,7 +3715,10 @@ void testProcessNestedEntityComposition_ExceptionHandling() throws Exception { // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test and expect exception @@ -3540,7 +3726,7 @@ void testProcessNestedEntityComposition_ExceptionHandling() throws Exception { RuntimeException.class, () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + method.invoke(sdmServiceGenericHandler, context, composition, new HashMap<>()); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause(); @@ -3574,6 +3760,12 @@ void testProcessNestedEntityComposition_MultipleAttachmentPaths() throws Excepti when(nestedRecordsResult.iterator()).thenReturn(Arrays.asList(nestedRecord).iterator()); when(nestedRecord.get("ID")).thenReturn("chapter1"); + // Mock nestedDraftEntity key element so nestedEntityKeys gets "ID" populated + CdsElement nestedKeyElement = mock(CdsElement.class); + when(nestedKeyElement.isKey()).thenReturn(true); + when(nestedKeyElement.getName()).thenReturn("ID"); + when(nestedDraftEntity.elements()).thenReturn(Stream.of(nestedKeyElement)); + // Mock multiple attachment paths Map attachmentMapping = new HashMap<>(); attachmentMapping.put("AdminService.ChapterAttachments", "path1"); @@ -3682,14 +3874,19 @@ void testProcessNestedEntityComposition_MultipleAttachmentPaths() throws Excepti // Use reflection to invoke the private method Method method = SDMServiceGenericHandler.class.getDeclaredMethod( - "processNestedEntityComposition", DraftCancelEventContext.class, CdsElement.class); + "processNestedEntityComposition", + DraftCancelEventContext.class, + CdsElement.class, + Map.class); method.setAccessible(true); // Execute the test assertDoesNotThrow( () -> { try { - method.invoke(sdmServiceGenericHandler, context, composition); + Map parentKeysArg = new HashMap<>(); + parentKeysArg.put("ID", "test-entity-id"); + method.invoke(sdmServiceGenericHandler, context, composition, parentKeysArg); } catch (Exception e) { if (e.getCause() instanceof RuntimeException) { throw (RuntimeException) e.getCause();