From 9c08ff93bc15069ca90acbe279c0c1b316b7362e Mon Sep 17 00:00:00 2001 From: soriaoli Date: Thu, 3 Sep 2026 10:45:03 +0200 Subject: [PATCH] [request-title] - Add catalogItemId to groups validation. --- .../ProvisionerActionsApiController.java | 4 +- .../facade/ProvisionerActionsApiFacade.java | 15 +++- .../ProvisionerActionsApiControllerTest.java | 8 +- .../ProvisionerActionsApiFacadeTest.java | 83 ++++++++++++++++--- 4 files changed, 89 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/opendevstack/component_catalog/server/controllers/ProvisionerActionsApiController.java b/src/main/java/org/opendevstack/component_catalog/server/controllers/ProvisionerActionsApiController.java index 3f3c945..5d0b919 100644 --- a/src/main/java/org/opendevstack/component_catalog/server/controllers/ProvisionerActionsApiController.java +++ b/src/main/java/org/opendevstack/component_catalog/server/controllers/ProvisionerActionsApiController.java @@ -42,7 +42,7 @@ public ResponseEntity notifyProvisioningStatusUpdate(String projectKey, projectKey, provisioningStatusUpdateRequest.toString()); var normalizedProjectKey = projectKey.toUpperCase(); - provisionerActionsApiFacade.validateGroupRestrictions(normalizedProjectKey); + provisionerActionsApiFacade.validateGroupRestrictions(normalizedProjectKey, provisioningStatusUpdateRequest); var normalizedComponentUrl = provisioningStatusUpdateRequest.getComponentUrl().orElse(Strings.EMPTY); var parameters = map(provisioningStatusUpdateRequest); @@ -68,7 +68,7 @@ public ResponseEntity notifyProvisioningStatusUpdatePartially(String proje projectKey, provisioningStatusUpdateRequest.toString()); var normalizedProjectKey = projectKey.toUpperCase(); - provisionerActionsApiFacade.validateGroupRestrictions(normalizedProjectKey); + provisionerActionsApiFacade.validateGroupRestrictions(normalizedProjectKey, provisioningStatusUpdateRequest); var normalizedComponentUrl = provisioningStatusUpdateRequest.getComponentUrl().orElse(Strings.EMPTY); var parameters = map(provisioningStatusUpdateRequest); diff --git a/src/main/java/org/opendevstack/component_catalog/server/facade/ProvisionerActionsApiFacade.java b/src/main/java/org/opendevstack/component_catalog/server/facade/ProvisionerActionsApiFacade.java index 6f64c40..5638e48 100644 --- a/src/main/java/org/opendevstack/component_catalog/server/facade/ProvisionerActionsApiFacade.java +++ b/src/main/java/org/opendevstack/component_catalog/server/facade/ProvisionerActionsApiFacade.java @@ -4,6 +4,7 @@ import org.jspecify.annotations.NonNull; import org.opendevstack.component_catalog.config.ApplicationPropertiesConfiguration; import org.opendevstack.component_catalog.server.controllers.exceptions.ForbiddenException; +import org.opendevstack.component_catalog.server.controllers.exceptions.InvalidRestEntityException; import org.opendevstack.component_catalog.server.model.ProvisioningStatusUpdateRequest; import org.opendevstack.component_catalog.server.services.ProjectsInfoService; import org.opendevstack.component_catalog.server.services.catalog.CatalogItemUserActionGroupsRestriction; @@ -49,8 +50,15 @@ public ProvisionerActionsApiFacade(ProjectsInfoService projectsInfoService, .toList(); } - public void validateGroupRestrictions(String projectKey) { + public void validateGroupRestrictions(String projectKey, ProvisioningStatusUpdateRequest provisioningStatusUpdateRequest) { var accessToken = authenticationFacade.getAccessToken(); + var catalogItemId = provisioningStatusUpdateRequest.getCatalogItemId(); + + if (catalogItemId == null) { + log.error("Catalog item id is null. Cannot validate group restrictions"); + + throw new InvalidRestEntityException("Catalog item id is null. Cannot validate group restrictions"); + } var oid = JwtUtils.extractClaim(accessToken, "oid"); @@ -61,11 +69,11 @@ public void validateGroupRestrictions(String projectKey) { } else { log.debug("Token with oid '{}' is NOT allowed to bypass group restrictions for project {}. Validating group restrictions", oid.orElse("unknown"), projectKey); - validateGroupRestrictions(projectKey, accessToken); + validateGroupRestrictions(projectKey, catalogItemId, accessToken); } } - private void validateGroupRestrictions(String projectKey, String accessToken) { + private void validateGroupRestrictions(String projectKey, String catalogItemId, String accessToken) { var groupRestriction = CatalogItemUserActionGroupsRestriction.builder() .prefix(groupsRestrictionProps.getPrefix()) .suffix(groupsRestrictionProps.getSuffix()) @@ -81,6 +89,7 @@ private void validateGroupRestrictions(String projectKey, String accessToken) { var params = RestrictionsParams.builder() .userGroups(userGroups) .projectKey(projectKey) + .catalogItemId(catalogItemId) .build(); var requestable = Optional.ofNullable(groupsRestrictionsEvaluator.evaluate(evaluationRestrictions, params)) diff --git a/src/test/java/org/opendevstack/component_catalog/server/controllers/ProvisionerActionsApiControllerTest.java b/src/test/java/org/opendevstack/component_catalog/server/controllers/ProvisionerActionsApiControllerTest.java index 8d2f2e2..118a115 100644 --- a/src/test/java/org/opendevstack/component_catalog/server/controllers/ProvisionerActionsApiControllerTest.java +++ b/src/test/java/org/opendevstack/component_catalog/server/controllers/ProvisionerActionsApiControllerTest.java @@ -89,7 +89,7 @@ void givenAProjectKey_whenNotifyProvisioningCompleted_thenServiceIsCalled() thro provisionerActionsApiController.notifyProvisioningStatusUpdate(projectKey, status, request); // then - verify(provisionerActionsApiFacade).validateGroupRestrictions(projectKey.toUpperCase()); + verify(provisionerActionsApiFacade).validateGroupRestrictions(projectKey.toUpperCase(), request); verify(provisionerActionsService).updateComponentProvisioningStatus(projectKey.toUpperCase(), request(componentId, catalogItemId, Status.CREATED, componentUrl, workflowJobId, deletionWorkflowJobId, parameters) ); @@ -129,7 +129,7 @@ void givenAProjectKey_whenNotifyProvisioningStatusUpdatePartially_thenCallsServi provisionerActionsApiController.notifyProvisioningStatusUpdatePartially(projectKey, status, request); // then - verify(provisionerActionsApiFacade).validateGroupRestrictions(projectKey.toUpperCase()); + verify(provisionerActionsApiFacade).validateGroupRestrictions(projectKey.toUpperCase(), request); verify(provisionerActionsService).updatePartiallyComponentProvisioningStatus( projectKey.toUpperCase(), request(componentId, catalogItemId, Status.CREATING, componentUrl, workflowJobId, deletionWorkflowJobId, parameters) @@ -168,7 +168,7 @@ void givenAProjectKeyAndNoComponentUrl_whenNotifyProvisioningStatusUpdatePartial provisionerActionsApiController.notifyProvisioningStatusUpdatePartially(projectKey, status, request); // then - verify(provisionerActionsApiFacade).validateGroupRestrictions(eq(projectKey.toUpperCase())); + verify(provisionerActionsApiFacade).validateGroupRestrictions(eq(projectKey.toUpperCase()), eq(request)); verify(provisionerActionsService).updatePartiallyComponentProvisioningStatus( projectKey.toUpperCase(), request(componentId, catalogItemId, Status.CREATING, "", workflowJobId, deletionWorkflowJobId, parameters) @@ -207,7 +207,7 @@ void givenAProjectKeyAndNoComponentUrl_whenNotifyProvisioningStatusUpdate_thenSe provisionerActionsApiController.notifyProvisioningStatusUpdate(projectKey, status, request); // then - verify(provisionerActionsApiFacade).validateGroupRestrictions(eq(projectKey.toUpperCase())); + verify(provisionerActionsApiFacade).validateGroupRestrictions(eq(projectKey.toUpperCase()), eq(request)); verify(provisionerActionsService).updateComponentProvisioningStatus( projectKey.toUpperCase(), request(componentId, catalogItemId, Status.CREATING, "", workflowJobId, deletionWorkflowJobId, parameters) diff --git a/src/test/java/org/opendevstack/component_catalog/server/facade/ProvisionerActionsApiFacadeTest.java b/src/test/java/org/opendevstack/component_catalog/server/facade/ProvisionerActionsApiFacadeTest.java index 78fe5a2..da658c6 100644 --- a/src/test/java/org/opendevstack/component_catalog/server/facade/ProvisionerActionsApiFacadeTest.java +++ b/src/test/java/org/opendevstack/component_catalog/server/facade/ProvisionerActionsApiFacadeTest.java @@ -7,6 +7,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.opendevstack.component_catalog.config.ApplicationPropertiesConfiguration; import org.opendevstack.component_catalog.server.controllers.exceptions.ForbiddenException; +import org.opendevstack.component_catalog.server.controllers.exceptions.InvalidRestEntityException; import org.opendevstack.component_catalog.server.model.ProvisioningStatusUpdateRequest; import org.opendevstack.component_catalog.server.model.ProvisioningStatusUpdateRequestParametersInner; import org.opendevstack.component_catalog.server.services.ProjectsInfoService; @@ -23,6 +24,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -31,6 +33,9 @@ @ExtendWith(MockitoExtension.class) class ProvisionerActionsApiFacadeTest { + private static final String PROJECT_KEY = "PROJECT"; + private static final String CATALOG_ITEM_ID = "catalog-item-id"; + @Mock private ProjectsInfoService projectsInfoService; @Mock @@ -92,12 +97,16 @@ void map_withEmptyParameters_returnsEmptyList() { assertThat(result).isEmpty(); } + private ProvisioningStatusUpdateRequest requestWithCatalogItemId(String catalogItemId) { + return new ProvisioningStatusUpdateRequest().catalogItemId(catalogItemId); + } + @Test void validateGroupRestrictions_whenUserHasPermissions_doesNotThrow() { // given - var projectKey = "PROJECT"; var accessToken = "accessToken"; var userGroups = List.of("group1"); + var request = requestWithCatalogItemId(CATALOG_ITEM_ID); when(authenticationFacade.getAccessToken()).thenReturn(accessToken); when(groupsRestrictionProps.getPrefix()).thenReturn(List.of("prefix-")); @@ -111,16 +120,23 @@ void validateGroupRestrictions_whenUserHasPermissions_doesNotThrow() { .thenReturn(Optional.empty()); // when / then - provisionerActionsApiFacade.validateGroupRestrictions(projectKey); + provisionerActionsApiFacade.validateGroupRestrictions(PROJECT_KEY, request); + + verify(groupsRestrictionsEvaluator) + .evaluate(any(EvaluationRestrictions.class), eq(RestrictionsParams.builder() + .userGroups(userGroups) + .projectKey(PROJECT_KEY) + .catalogItemId(CATALOG_ITEM_ID) + .build())); } } @Test void validateGroupRestrictions_whenUserHasNoPermissions_throwsForbiddenException() { // given - var projectKey = "PROJECT"; var accessToken = "accessToken"; var userGroups = List.of("group1"); + var request = requestWithCatalogItemId(CATALOG_ITEM_ID); when(authenticationFacade.getAccessToken()).thenReturn(accessToken); when(groupsRestrictionProps.getPrefix()).thenReturn(List.of("prefix-")); @@ -134,7 +150,7 @@ void validateGroupRestrictions_whenUserHasNoPermissions_throwsForbiddenException .thenReturn(Optional.empty()); // when / then - assertThatThrownBy(() -> provisionerActionsApiFacade.validateGroupRestrictions(projectKey)) + assertThatThrownBy(() -> provisionerActionsApiFacade.validateGroupRestrictions(PROJECT_KEY, request)) .isInstanceOf(ForbiddenException.class) .hasMessage("User not allowed to perform this action"); } @@ -143,9 +159,9 @@ void validateGroupRestrictions_whenUserHasNoPermissions_throwsForbiddenException @Test void validateGroupRestrictions_whenEvaluatorReturnsNull_doesNotThrow() { // given - var projectKey = "PROJECT"; var accessToken = "accessToken"; var userGroups = List.of("group1"); + var request = requestWithCatalogItemId(CATALOG_ITEM_ID); when(authenticationFacade.getAccessToken()).thenReturn(accessToken); when(groupsRestrictionProps.getPrefix()).thenReturn(List.of("prefix-")); @@ -159,16 +175,16 @@ void validateGroupRestrictions_whenEvaluatorReturnsNull_doesNotThrow() { .thenReturn(Optional.empty()); // when / then - provisionerActionsApiFacade.validateGroupRestrictions(projectKey); + provisionerActionsApiFacade.validateGroupRestrictions(PROJECT_KEY, request); } } @Test void validateGroupRestrictions_whenPermittedOidsContainsExtractedOid_bypassesGroupRestrictions() { // given - var projectKey = "PROJECT"; var accessToken = "accessToken"; var permittedOid = "oid1"; + var request = requestWithCatalogItemId(CATALOG_ITEM_ID); when(authenticationFacade.getAccessToken()).thenReturn(accessToken); @@ -177,7 +193,7 @@ void validateGroupRestrictions_whenPermittedOidsContainsExtractedOid_bypassesGro .thenReturn(Optional.of(permittedOid)); // when - provisionerActionsApiFacade.validateGroupRestrictions(projectKey); + provisionerActionsApiFacade.validateGroupRestrictions(PROJECT_KEY, request); // then - Verify that the group restrictions evaluator is never called when oid is in permittedOids verify(groupsRestrictionsEvaluator, never()) @@ -189,9 +205,9 @@ void validateGroupRestrictions_whenPermittedOidsContainsExtractedOid_bypassesGro @Test void validateGroupRestrictions_whenPermittedOidsContainsExtractedOid_doesNotThrow() { // given - var projectKey = "PROJECT"; var accessToken = "accessToken"; var permittedOid = "oid2"; + var request = requestWithCatalogItemId(CATALOG_ITEM_ID); when(authenticationFacade.getAccessToken()).thenReturn(accessToken); @@ -200,16 +216,16 @@ void validateGroupRestrictions_whenPermittedOidsContainsExtractedOid_doesNotThro .thenReturn(Optional.of(permittedOid)); // when / then - should not throw any exception - provisionerActionsApiFacade.validateGroupRestrictions(projectKey); + provisionerActionsApiFacade.validateGroupRestrictions(PROJECT_KEY, request); } } @Test void validateGroupRestrictions_whenTokenHasNoOid_callsGroupRestrictionsEvaluation() { // given - var projectKey = "PROJECT"; var accessToken = "accessToken"; var userGroups = List.of("group1"); + var request = requestWithCatalogItemId(CATALOG_ITEM_ID); when(authenticationFacade.getAccessToken()).thenReturn(accessToken); when(groupsRestrictionProps.getPrefix()).thenReturn(List.of("prefix-")); @@ -223,7 +239,7 @@ void validateGroupRestrictions_whenTokenHasNoOid_callsGroupRestrictionsEvaluatio .thenReturn(Optional.empty()); // when - provisionerActionsApiFacade.validateGroupRestrictions(projectKey); + provisionerActionsApiFacade.validateGroupRestrictions(PROJECT_KEY, request); // then - Verify that the group restrictions evaluator is called when oid is not in permittedOids verify(groupsRestrictionsEvaluator) @@ -231,5 +247,48 @@ void validateGroupRestrictions_whenTokenHasNoOid_callsGroupRestrictionsEvaluatio verify(projectsInfoService).getProjectGroups(accessToken); } } + + @Test + void validateGroupRestrictions_whenCatalogItemIdIsNull_throwsInvalidRestEntityException() { + // given + var request = requestWithCatalogItemId(null); + + // when / then + assertThatThrownBy(() -> provisionerActionsApiFacade.validateGroupRestrictions(PROJECT_KEY, request)) + .isInstanceOf(InvalidRestEntityException.class) + .hasMessage("Catalog item id is null. Cannot validate group restrictions"); + + verify(authenticationFacade).getAccessToken(); + verify(groupsRestrictionsEvaluator, never()).evaluate(any(EvaluationRestrictions.class), any(RestrictionsParams.class)); + verify(projectsInfoService, never()).getProjectGroups(any()); + } + + @Test + void validateGroupRestrictions_whenExtractedOidIsNotPermitted_callsGroupRestrictionsEvaluation() { + // given + var accessToken = "accessToken"; + var userGroups = List.of("group1"); + var request = requestWithCatalogItemId(CATALOG_ITEM_ID); + + when(authenticationFacade.getAccessToken()).thenReturn(accessToken); + when(groupsRestrictionProps.getPrefix()).thenReturn(List.of("prefix-")); + when(groupsRestrictionProps.getSuffix()).thenReturn(List.of("-suffix")); + when(projectsInfoService.getProjectGroups(accessToken)).thenReturn(userGroups); + when(groupsRestrictionsEvaluator.evaluate(any(EvaluationRestrictions.class), any(RestrictionsParams.class))) + .thenReturn(RestrictionsEvaluatorResultMother.of(true, "allowed")); + + try (var jwtUtilsMocked = mockStatic(JwtUtils.class)) { + jwtUtilsMocked.when(() -> JwtUtils.extractClaim(accessToken, "oid")) + .thenReturn(Optional.of("non-permitted-oid")); + + // when + provisionerActionsApiFacade.validateGroupRestrictions(PROJECT_KEY, request); + + // then + verify(groupsRestrictionsEvaluator) + .evaluate(any(EvaluationRestrictions.class), any(RestrictionsParams.class)); + verify(projectsInfoService).getProjectGroups(accessToken); + } + } }