From e572a24f8721ec56faa18583ce40fc00f48bdeee Mon Sep 17 00:00:00 2001 From: Nilesh Sarupriya Date: Wed, 26 Oct 2022 14:43:21 +0530 Subject: [PATCH] feat: set user permissions to objects when creating (#17871) * set user permissions to objects when creating * add/edit test cases * add action collection test case * update failing test cases --- .../services/LayoutCollectionServiceImpl.java | 6 ++-- .../services/ce/DatasourceServiceCEImpl.java | 3 +- .../ce/LayoutCollectionServiceCEImpl.java | 3 ++ .../services/ce/NewActionServiceCEImpl.java | 1 + .../services/ce/NewPageServiceCEImpl.java | 1 + .../ActionCollectionServiceImplTest.java | 23 +++++++++++++- .../services/ActionCollectionServiceTest.java | 22 +++++++++++++ .../services/DatasourceServiceTest.java | 5 +++ .../server/services/NewPageServiceTest.java | 31 +++++++++++++++++++ .../services/ce/ActionServiceCE_Test.java | 12 +++---- 10 files changed, 96 insertions(+), 11 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutCollectionServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutCollectionServiceImpl.java index f2876f3e27..9cded5a007 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutCollectionServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutCollectionServiceImpl.java @@ -1,6 +1,7 @@ package com.appsmith.server.services; import com.appsmith.server.helpers.ResponseUtils; +import com.appsmith.server.repositories.ActionCollectionRepository; import com.appsmith.server.services.ce.LayoutCollectionServiceCEImpl; import com.appsmith.server.solutions.RefactoringSolution; import lombok.extern.slf4j.Slf4j; @@ -16,9 +17,10 @@ public class LayoutCollectionServiceImpl extends LayoutCollectionServiceCEImpl i ActionCollectionService actionCollectionService, NewActionService newActionService, AnalyticsService analyticsService, - ResponseUtils responseUtils) { + ResponseUtils responseUtils, + ActionCollectionRepository actionCollectionRepository) { super(newPageService, layoutActionService, refactoringSolution, actionCollectionService, newActionService, analyticsService, - responseUtils); + responseUtils, actionCollectionRepository); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceServiceCEImpl.java index d74a026e99..d8ab09a6ea 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceServiceCEImpl.java @@ -133,7 +133,8 @@ public class DatasourceServiceCEImpl extends BaseService analyticsService.sendCreateEvent(savedDatasource, getAnalyticsProperties(savedDatasource)) ) - .flatMap(this::populateHintMessages); // For REST API datasource create flow. + .flatMap(this::populateHintMessages) // For REST API datasource create flow. + .flatMap(repository::setUserPermissionsInObject); } private Mono generateAndSetDatasourcePolicies(Mono userMono, Datasource datasource) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java index bc3c6ed1c0..b59f6028e5 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java @@ -19,6 +19,7 @@ import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.CollectionUtils; import com.appsmith.server.helpers.DefaultResourcesUtils; import com.appsmith.server.helpers.ResponseUtils; +import com.appsmith.server.repositories.ActionCollectionRepository; import com.appsmith.server.services.ActionCollectionService; import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.LayoutActionService; @@ -59,6 +60,7 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE private final NewActionService newActionService; private final AnalyticsService analyticsService; private final ResponseUtils responseUtils; + private final ActionCollectionRepository actionCollectionRepository; /** * Called by ActionCollection controller to create ActionCollection @@ -190,6 +192,7 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE } return actionCollectionService.save(savedActionCollection); }) + .flatMap(actionCollectionRepository::setUserPermissionsInObject) .cache(); return actionCollectionMono diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java index 28cd8b5fd1..efdd8c77ec 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/NewActionServiceCEImpl.java @@ -419,6 +419,7 @@ public class NewActionServiceCEImpl extends BaseService getPageByViewMode(page, false)); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java index 174c394468..a089e8ab8f 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java @@ -53,6 +53,7 @@ import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; @ExtendWith(SpringExtension.class) @Slf4j @@ -115,7 +116,8 @@ public class ActionCollectionServiceImplTest { actionCollectionService, newActionService, analyticsService, - responseUtils + responseUtils, + actionCollectionRepository ); Mockito @@ -266,12 +268,21 @@ public class ActionCollectionServiceImplTest { argument.setId("testActionCollectionId"); return Mono.just(argument); }); + Mockito + .when(actionCollectionRepository.setUserPermissionsInObject(Mockito.any())) + .thenAnswer(invocation -> { + final ActionCollection argument = (ActionCollection) invocation.getArguments()[0]; + argument.setId("testActionCollectionId"); + argument.setUserPermissions(Set.of("test-user-permission1", "test-user-permission2")); + return Mono.just(argument); + }); final Mono actionCollectionDTOMono = layoutCollectionService.createCollection(actionCollectionDTO); StepVerifier.create(actionCollectionDTOMono) .assertNext(actionCollectionDTO1 -> { assertTrue(actionCollectionDTO1.getActions().isEmpty()); + assertThat(actionCollectionDTO1.getUserPermissions()).hasSize(2); }) .verifyComplete(); } @@ -336,11 +347,21 @@ public class ActionCollectionServiceImplTest { return Mono.just(argument); }); + Mockito + .when(actionCollectionRepository.setUserPermissionsInObject(Mockito.any())) + .thenAnswer(invocation -> { + final ActionCollection argument = (ActionCollection) invocation.getArguments()[0]; + argument.setId("testActionCollectionId"); + argument.setUserPermissions(Set.of("test-user-permission1", "test-user-permission2")); + return Mono.just(argument); + }); + final Mono actionCollectionDTOMono = layoutCollectionService.createCollection(actionCollectionDTO); StepVerifier.create(actionCollectionDTOMono) .assertNext(actionCollectionDTO1 -> { assertEquals(1, actionCollectionDTO1.getActions().size()); + assertThat(actionCollectionDTO1.getUserPermissions()).hasSize(2); final ActionDTO actionDTO = actionCollectionDTO1.getActions().get(0); assertEquals("testAction", actionDTO.getName()); assertEquals("testActionId", actionDTO.getId()); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java index ef62898079..15892804e4 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java @@ -195,6 +195,28 @@ public class ActionCollectionServiceTest { testPage = null; } + @Test + @WithUserDetails(value = "api_user") + public void testCreateActionCollection() { + ActionCollectionDTO actionCollectionDTO = new ActionCollectionDTO(); + actionCollectionDTO.setName("testActionCollection"); + actionCollectionDTO.setApplicationId(testApp.getId()); + actionCollectionDTO.setWorkspaceId(testApp.getWorkspaceId()); + actionCollectionDTO.setPageId(testPage.getId()); + actionCollectionDTO.setPluginId(datasource.getPluginId()); + actionCollectionDTO.setPluginType(PluginType.JS); + + StepVerifier.create(layoutCollectionService.createCollection(actionCollectionDTO)) + .assertNext(actionCollectionDTO1 -> { + assertThat(actionCollectionDTO1.getApplicationId()).isEqualTo(testApp.getId()); + assertThat(actionCollectionDTO1.getWorkspaceId()).isEqualTo(testApp.getWorkspaceId()); + assertThat(actionCollectionDTO1.getPageId()).isEqualTo(testPage.getId()); + assertThat(actionCollectionDTO1.getPluginId()).isEqualTo(datasource.getPluginId()); + assertThat(actionCollectionDTO1.getUserPermissions()).isNotEmpty(); + }) + .verifyComplete(); + } + @Test @WithUserDetails(value = "api_user") public void createValidActionCollectionAndCheckPermissions() { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java index 0195aa043f..5d0794acab 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java @@ -149,8 +149,10 @@ public class DatasourceServiceTest { .assertNext(datasource -> { assertThat(datasource.getT1().getName()).isEqualTo("Untitled Datasource"); assertThat(datasource.getT1().getWorkspaceId()).isEqualTo("random-org-id-1"); + assertThat(datasource.getT1().getUserPermissions()).isNotEmpty(); assertThat(datasource.getT2().getName()).isEqualTo("Untitled Datasource"); assertThat(datasource.getT2().getWorkspaceId()).isEqualTo("random-org-id-2"); + assertThat(datasource.getT2().getUserPermissions()).isNotEmpty(); }) .verifyComplete(); } @@ -176,6 +178,7 @@ public class DatasourceServiceTest { .assertNext(createdDatasource -> { assertThat(createdDatasource.getId()).isNotEmpty(); assertThat(createdDatasource.getName()).isEqualTo(datasource.getName()); + assertThat(createdDatasource.getUserPermissions()).isNotEmpty(); assertThat(createdDatasource.getIsValid()).isFalse(); assertThat(createdDatasource.getInvalids()).containsExactlyInAnyOrder("Missing plugin id. Please enter one."); }) @@ -244,6 +247,7 @@ public class DatasourceServiceTest { assertThat(createdDatasource.getId()).isNotEmpty(); assertThat(createdDatasource.getPluginId()).isEqualTo(datasource.getPluginId()); assertThat(createdDatasource.getName()).isEqualTo(datasource.getName()); + assertThat(createdDatasource.getUserPermissions()).isNotEmpty(); assertThat(createdDatasource.getIsValid()).isFalse(); assertThat(createdDatasource.getInvalids()).contains("Plugin " + datasource.getPluginId() + " not installed"); }) @@ -303,6 +307,7 @@ public class DatasourceServiceTest { .create(datasourceMono) .assertNext(createdDatasource -> { assertThat(createdDatasource.getId()).isNotEmpty(); + assertThat(createdDatasource.getUserPermissions()).isNotEmpty(); assertThat(createdDatasource.getPluginId()).isEqualTo(datasource.getPluginId()); assertThat(createdDatasource.getName()).isEqualTo(datasource.getName()); Policy manageDatasourcePolicy = Policy.builder().permission(MANAGE_DATASOURCES.getValue()) diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/NewPageServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/NewPageServiceTest.java index 0630e94340..41197e0ff6 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/NewPageServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/NewPageServiceTest.java @@ -1,11 +1,16 @@ package com.appsmith.server.services; +import com.appsmith.external.models.DefaultResources; +import com.appsmith.external.models.Policy; +import com.appsmith.server.acl.AclPermission; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.ApplicationMode; +import com.appsmith.server.domains.PermissionGroup; import com.appsmith.server.domains.Workspace; import com.appsmith.server.dtos.ApplicationPagesDTO; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.repositories.PermissionGroupRepository; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.beans.factory.annotation.Autowired; @@ -15,7 +20,9 @@ import org.springframework.test.context.junit.jupiter.SpringExtension; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; +import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; @@ -32,6 +39,30 @@ public class NewPageServiceTest { @Autowired WorkspaceService workspaceService; + @Autowired + PermissionGroupRepository permissionGroupRepository; + + @Test + @WithUserDetails("api_user") + public void testCreateDefault() { + Set permissionGroupIds = permissionGroupRepository.findAll().collectList().block().stream() + .map(PermissionGroup::getId).collect(Collectors.toSet()); + PageDTO pageDTO = new PageDTO(); + pageDTO.setApplicationId("test-application-id"); + DefaultResources testDefaultResources = new DefaultResources(); + pageDTO.setDefaultResources(testDefaultResources); + Policy testPolicy = Policy.builder() + .permissionGroups(permissionGroupIds) + .build(); + pageDTO.setPolicies(Set.of(testPolicy)); + StepVerifier.create(newPageService.createDefault(pageDTO)) + .assertNext(pageDTO1 -> { + assertThat(pageDTO1.getId()).isNotNull(); + assertThat(pageDTO1.getUserPermissions()).isNotEmpty(); + }) + .verifyComplete(); + } + @Test @WithUserDetails("api_user") public void findApplicationPages_WhenApplicationIdAndPageIdNotPresent_ThrowsException() { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ActionServiceCE_Test.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ActionServiceCE_Test.java index c93ac23134..becc460be8 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ActionServiceCE_Test.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ActionServiceCE_Test.java @@ -318,9 +318,7 @@ public class ActionServiceCE_Test { action.setActionConfiguration(actionConfiguration); action.setDatasource(datasource); - Mono actionMono = layoutActionService.createSingleAction(action) - .flatMap(createdAction -> newActionService.findById(createdAction.getId(), READ_ACTIONS)) - .flatMap(newAction -> newActionService.generateActionByViewMode(newAction, false)); + Mono actionMono = layoutActionService.createSingleAction(action); StepVerifier .create(Mono.zip(actionMono, defaultPermissionGroupsMono)) @@ -329,6 +327,7 @@ public class ActionServiceCE_Test { assertThat(createdAction.getId()).isNotEmpty(); assertThat(createdAction.getName()).isEqualTo(action.getName()); assertThat(createdAction.getExecuteOnLoad()).isFalse(); + assertThat(createdAction.getUserPermissions()).isNotEmpty(); List permissionGroups = tuple.getT2(); PermissionGroup adminPermissionGroup = permissionGroups.stream() @@ -381,17 +380,15 @@ public class ActionServiceCE_Test { action.setActionConfiguration(actionConfiguration); action.setDatasource(datasource); - Mono actionMono = layoutActionService.createSingleActionWithBranch(action, branchName) - .flatMap(createdAction -> newActionService.findByBranchNameAndDefaultActionId(branchName, createdAction.getId(), READ_ACTIONS)) - .flatMap(newAction -> newActionService.generateActionByViewMode(newAction, false)); + Mono actionMono = layoutActionService.createSingleActionWithBranch(action, branchName); StepVerifier .create(Mono.zip(actionMono, defaultPermissionGroupsMono)) .assertNext(tuple -> { ActionDTO createdAction = tuple.getT1(); assertThat(createdAction.getExecuteOnLoad()).isFalse(); - assertThat(createdAction.getDefaultResources()).isNotNull(); + assertThat(createdAction.getUserPermissions()).isNotEmpty(); assertThat(createdAction.getDefaultResources().getActionId()).isEqualTo(createdAction.getId()); assertThat(createdAction.getDefaultResources().getPageId()).isEqualTo(gitConnectedPage.getId()); assertThat(createdAction.getDefaultResources().getApplicationId()).isEqualTo(gitConnectedPage.getApplicationId()); @@ -449,6 +446,7 @@ public class ActionServiceCE_Test { ActionMoveDTO actionMoveDTO = new ActionMoveDTO(); actionMoveDTO.setAction(savedAction); actionMoveDTO.setDestinationPageId(destinationPage.getId()); + assertThat(savedAction.getUserPermissions()).isNotEmpty(); return layoutActionService.moveAction(actionMoveDTO); });