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
This commit is contained in:
Nilesh Sarupriya 2022-10-26 14:43:21 +05:30 committed by GitHub
parent b9e8f54f1e
commit e572a24f87
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 96 additions and 11 deletions

View File

@ -1,6 +1,7 @@
package com.appsmith.server.services; package com.appsmith.server.services;
import com.appsmith.server.helpers.ResponseUtils; import com.appsmith.server.helpers.ResponseUtils;
import com.appsmith.server.repositories.ActionCollectionRepository;
import com.appsmith.server.services.ce.LayoutCollectionServiceCEImpl; import com.appsmith.server.services.ce.LayoutCollectionServiceCEImpl;
import com.appsmith.server.solutions.RefactoringSolution; import com.appsmith.server.solutions.RefactoringSolution;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
@ -16,9 +17,10 @@ public class LayoutCollectionServiceImpl extends LayoutCollectionServiceCEImpl i
ActionCollectionService actionCollectionService, ActionCollectionService actionCollectionService,
NewActionService newActionService, NewActionService newActionService,
AnalyticsService analyticsService, AnalyticsService analyticsService,
ResponseUtils responseUtils) { ResponseUtils responseUtils,
ActionCollectionRepository actionCollectionRepository) {
super(newPageService, layoutActionService, refactoringSolution, actionCollectionService, newActionService, analyticsService, super(newPageService, layoutActionService, refactoringSolution, actionCollectionService, newActionService, analyticsService,
responseUtils); responseUtils, actionCollectionRepository);
} }
} }

View File

@ -133,7 +133,8 @@ public class DatasourceServiceCEImpl extends BaseService<DatasourceRepository, D
.flatMap(savedDatasource -> .flatMap(savedDatasource ->
analyticsService.sendCreateEvent(savedDatasource, getAnalyticsProperties(savedDatasource)) 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<Datasource> generateAndSetDatasourcePolicies(Mono<User> userMono, Datasource datasource) { private Mono<Datasource> generateAndSetDatasourcePolicies(Mono<User> userMono, Datasource datasource) {

View File

@ -19,6 +19,7 @@ import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.CollectionUtils; import com.appsmith.server.helpers.CollectionUtils;
import com.appsmith.server.helpers.DefaultResourcesUtils; import com.appsmith.server.helpers.DefaultResourcesUtils;
import com.appsmith.server.helpers.ResponseUtils; import com.appsmith.server.helpers.ResponseUtils;
import com.appsmith.server.repositories.ActionCollectionRepository;
import com.appsmith.server.services.ActionCollectionService; import com.appsmith.server.services.ActionCollectionService;
import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.AnalyticsService;
import com.appsmith.server.services.LayoutActionService; import com.appsmith.server.services.LayoutActionService;
@ -59,6 +60,7 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE
private final NewActionService newActionService; private final NewActionService newActionService;
private final AnalyticsService analyticsService; private final AnalyticsService analyticsService;
private final ResponseUtils responseUtils; private final ResponseUtils responseUtils;
private final ActionCollectionRepository actionCollectionRepository;
/** /**
* Called by ActionCollection controller to create ActionCollection * Called by ActionCollection controller to create ActionCollection
@ -190,6 +192,7 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE
} }
return actionCollectionService.save(savedActionCollection); return actionCollectionService.save(savedActionCollection);
}) })
.flatMap(actionCollectionRepository::setUserPermissionsInObject)
.cache(); .cache();
return actionCollectionMono return actionCollectionMono

View File

@ -419,6 +419,7 @@ public class NewActionServiceCEImpl extends BaseService<NewActionRepository, New
} }
return Mono.just(savedAction); return Mono.just(savedAction);
}) })
.flatMap(repository::setUserPermissionsInObject)
.switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.REPOSITORY_SAVE_FAILED))) .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.REPOSITORY_SAVE_FAILED)))
.flatMap(this::setTransientFieldsInUnpublishedAction); .flatMap(this::setTransientFieldsInUnpublishedAction);
} }

View File

@ -173,6 +173,7 @@ public class NewPageServiceCEImpl extends BaseService<NewPageRepository, NewPage
} }
return Mono.just(savedPage); return Mono.just(savedPage);
}) })
.flatMap(repository::setUserPermissionsInObject)
.flatMap(page -> getPageByViewMode(page, false)); .flatMap(page -> getPageByViewMode(page, false));
} }

View File

@ -53,6 +53,7 @@ import java.util.stream.Collectors;
import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.assertj.core.api.Assertions.assertThat;
@ExtendWith(SpringExtension.class) @ExtendWith(SpringExtension.class)
@Slf4j @Slf4j
@ -115,7 +116,8 @@ public class ActionCollectionServiceImplTest {
actionCollectionService, actionCollectionService,
newActionService, newActionService,
analyticsService, analyticsService,
responseUtils responseUtils,
actionCollectionRepository
); );
Mockito Mockito
@ -266,12 +268,21 @@ public class ActionCollectionServiceImplTest {
argument.setId("testActionCollectionId"); argument.setId("testActionCollectionId");
return Mono.just(argument); 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<ActionCollectionDTO> actionCollectionDTOMono = layoutCollectionService.createCollection(actionCollectionDTO); final Mono<ActionCollectionDTO> actionCollectionDTOMono = layoutCollectionService.createCollection(actionCollectionDTO);
StepVerifier.create(actionCollectionDTOMono) StepVerifier.create(actionCollectionDTOMono)
.assertNext(actionCollectionDTO1 -> { .assertNext(actionCollectionDTO1 -> {
assertTrue(actionCollectionDTO1.getActions().isEmpty()); assertTrue(actionCollectionDTO1.getActions().isEmpty());
assertThat(actionCollectionDTO1.getUserPermissions()).hasSize(2);
}) })
.verifyComplete(); .verifyComplete();
} }
@ -336,11 +347,21 @@ public class ActionCollectionServiceImplTest {
return Mono.just(argument); 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<ActionCollectionDTO> actionCollectionDTOMono = layoutCollectionService.createCollection(actionCollectionDTO); final Mono<ActionCollectionDTO> actionCollectionDTOMono = layoutCollectionService.createCollection(actionCollectionDTO);
StepVerifier.create(actionCollectionDTOMono) StepVerifier.create(actionCollectionDTOMono)
.assertNext(actionCollectionDTO1 -> { .assertNext(actionCollectionDTO1 -> {
assertEquals(1, actionCollectionDTO1.getActions().size()); assertEquals(1, actionCollectionDTO1.getActions().size());
assertThat(actionCollectionDTO1.getUserPermissions()).hasSize(2);
final ActionDTO actionDTO = actionCollectionDTO1.getActions().get(0); final ActionDTO actionDTO = actionCollectionDTO1.getActions().get(0);
assertEquals("testAction", actionDTO.getName()); assertEquals("testAction", actionDTO.getName());
assertEquals("testActionId", actionDTO.getId()); assertEquals("testActionId", actionDTO.getId());

View File

@ -195,6 +195,28 @@ public class ActionCollectionServiceTest {
testPage = null; 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 @Test
@WithUserDetails(value = "api_user") @WithUserDetails(value = "api_user")
public void createValidActionCollectionAndCheckPermissions() { public void createValidActionCollectionAndCheckPermissions() {

View File

@ -149,8 +149,10 @@ public class DatasourceServiceTest {
.assertNext(datasource -> { .assertNext(datasource -> {
assertThat(datasource.getT1().getName()).isEqualTo("Untitled Datasource"); assertThat(datasource.getT1().getName()).isEqualTo("Untitled Datasource");
assertThat(datasource.getT1().getWorkspaceId()).isEqualTo("random-org-id-1"); assertThat(datasource.getT1().getWorkspaceId()).isEqualTo("random-org-id-1");
assertThat(datasource.getT1().getUserPermissions()).isNotEmpty();
assertThat(datasource.getT2().getName()).isEqualTo("Untitled Datasource"); assertThat(datasource.getT2().getName()).isEqualTo("Untitled Datasource");
assertThat(datasource.getT2().getWorkspaceId()).isEqualTo("random-org-id-2"); assertThat(datasource.getT2().getWorkspaceId()).isEqualTo("random-org-id-2");
assertThat(datasource.getT2().getUserPermissions()).isNotEmpty();
}) })
.verifyComplete(); .verifyComplete();
} }
@ -176,6 +178,7 @@ public class DatasourceServiceTest {
.assertNext(createdDatasource -> { .assertNext(createdDatasource -> {
assertThat(createdDatasource.getId()).isNotEmpty(); assertThat(createdDatasource.getId()).isNotEmpty();
assertThat(createdDatasource.getName()).isEqualTo(datasource.getName()); assertThat(createdDatasource.getName()).isEqualTo(datasource.getName());
assertThat(createdDatasource.getUserPermissions()).isNotEmpty();
assertThat(createdDatasource.getIsValid()).isFalse(); assertThat(createdDatasource.getIsValid()).isFalse();
assertThat(createdDatasource.getInvalids()).containsExactlyInAnyOrder("Missing plugin id. Please enter one."); assertThat(createdDatasource.getInvalids()).containsExactlyInAnyOrder("Missing plugin id. Please enter one.");
}) })
@ -244,6 +247,7 @@ public class DatasourceServiceTest {
assertThat(createdDatasource.getId()).isNotEmpty(); assertThat(createdDatasource.getId()).isNotEmpty();
assertThat(createdDatasource.getPluginId()).isEqualTo(datasource.getPluginId()); assertThat(createdDatasource.getPluginId()).isEqualTo(datasource.getPluginId());
assertThat(createdDatasource.getName()).isEqualTo(datasource.getName()); assertThat(createdDatasource.getName()).isEqualTo(datasource.getName());
assertThat(createdDatasource.getUserPermissions()).isNotEmpty();
assertThat(createdDatasource.getIsValid()).isFalse(); assertThat(createdDatasource.getIsValid()).isFalse();
assertThat(createdDatasource.getInvalids()).contains("Plugin " + datasource.getPluginId() + " not installed"); assertThat(createdDatasource.getInvalids()).contains("Plugin " + datasource.getPluginId() + " not installed");
}) })
@ -303,6 +307,7 @@ public class DatasourceServiceTest {
.create(datasourceMono) .create(datasourceMono)
.assertNext(createdDatasource -> { .assertNext(createdDatasource -> {
assertThat(createdDatasource.getId()).isNotEmpty(); assertThat(createdDatasource.getId()).isNotEmpty();
assertThat(createdDatasource.getUserPermissions()).isNotEmpty();
assertThat(createdDatasource.getPluginId()).isEqualTo(datasource.getPluginId()); assertThat(createdDatasource.getPluginId()).isEqualTo(datasource.getPluginId());
assertThat(createdDatasource.getName()).isEqualTo(datasource.getName()); assertThat(createdDatasource.getName()).isEqualTo(datasource.getName());
Policy manageDatasourcePolicy = Policy.builder().permission(MANAGE_DATASOURCES.getValue()) Policy manageDatasourcePolicy = Policy.builder().permission(MANAGE_DATASOURCES.getValue())

View File

@ -1,11 +1,16 @@
package com.appsmith.server.services; 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.Application;
import com.appsmith.server.domains.ApplicationMode; import com.appsmith.server.domains.ApplicationMode;
import com.appsmith.server.domains.PermissionGroup;
import com.appsmith.server.domains.Workspace; import com.appsmith.server.domains.Workspace;
import com.appsmith.server.dtos.ApplicationPagesDTO; import com.appsmith.server.dtos.ApplicationPagesDTO;
import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.dtos.PageDTO;
import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.repositories.PermissionGroupRepository;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired; 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.core.publisher.Mono;
import reactor.test.StepVerifier; import reactor.test.StepVerifier;
import java.util.Set;
import java.util.UUID; import java.util.UUID;
import java.util.stream.Collectors;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
@ -32,6 +39,30 @@ public class NewPageServiceTest {
@Autowired @Autowired
WorkspaceService workspaceService; WorkspaceService workspaceService;
@Autowired
PermissionGroupRepository permissionGroupRepository;
@Test
@WithUserDetails("api_user")
public void testCreateDefault() {
Set<String> 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 @Test
@WithUserDetails("api_user") @WithUserDetails("api_user")
public void findApplicationPages_WhenApplicationIdAndPageIdNotPresent_ThrowsException() { public void findApplicationPages_WhenApplicationIdAndPageIdNotPresent_ThrowsException() {

View File

@ -318,9 +318,7 @@ public class ActionServiceCE_Test {
action.setActionConfiguration(actionConfiguration); action.setActionConfiguration(actionConfiguration);
action.setDatasource(datasource); action.setDatasource(datasource);
Mono<ActionDTO> actionMono = layoutActionService.createSingleAction(action) Mono<ActionDTO> actionMono = layoutActionService.createSingleAction(action);
.flatMap(createdAction -> newActionService.findById(createdAction.getId(), READ_ACTIONS))
.flatMap(newAction -> newActionService.generateActionByViewMode(newAction, false));
StepVerifier StepVerifier
.create(Mono.zip(actionMono, defaultPermissionGroupsMono)) .create(Mono.zip(actionMono, defaultPermissionGroupsMono))
@ -329,6 +327,7 @@ public class ActionServiceCE_Test {
assertThat(createdAction.getId()).isNotEmpty(); assertThat(createdAction.getId()).isNotEmpty();
assertThat(createdAction.getName()).isEqualTo(action.getName()); assertThat(createdAction.getName()).isEqualTo(action.getName());
assertThat(createdAction.getExecuteOnLoad()).isFalse(); assertThat(createdAction.getExecuteOnLoad()).isFalse();
assertThat(createdAction.getUserPermissions()).isNotEmpty();
List<PermissionGroup> permissionGroups = tuple.getT2(); List<PermissionGroup> permissionGroups = tuple.getT2();
PermissionGroup adminPermissionGroup = permissionGroups.stream() PermissionGroup adminPermissionGroup = permissionGroups.stream()
@ -381,17 +380,15 @@ public class ActionServiceCE_Test {
action.setActionConfiguration(actionConfiguration); action.setActionConfiguration(actionConfiguration);
action.setDatasource(datasource); action.setDatasource(datasource);
Mono<ActionDTO> actionMono = layoutActionService.createSingleActionWithBranch(action, branchName) Mono<ActionDTO> actionMono = layoutActionService.createSingleActionWithBranch(action, branchName);
.flatMap(createdAction -> newActionService.findByBranchNameAndDefaultActionId(branchName, createdAction.getId(), READ_ACTIONS))
.flatMap(newAction -> newActionService.generateActionByViewMode(newAction, false));
StepVerifier StepVerifier
.create(Mono.zip(actionMono, defaultPermissionGroupsMono)) .create(Mono.zip(actionMono, defaultPermissionGroupsMono))
.assertNext(tuple -> { .assertNext(tuple -> {
ActionDTO createdAction = tuple.getT1(); ActionDTO createdAction = tuple.getT1();
assertThat(createdAction.getExecuteOnLoad()).isFalse(); assertThat(createdAction.getExecuteOnLoad()).isFalse();
assertThat(createdAction.getDefaultResources()).isNotNull(); assertThat(createdAction.getDefaultResources()).isNotNull();
assertThat(createdAction.getUserPermissions()).isNotEmpty();
assertThat(createdAction.getDefaultResources().getActionId()).isEqualTo(createdAction.getId()); assertThat(createdAction.getDefaultResources().getActionId()).isEqualTo(createdAction.getId());
assertThat(createdAction.getDefaultResources().getPageId()).isEqualTo(gitConnectedPage.getId()); assertThat(createdAction.getDefaultResources().getPageId()).isEqualTo(gitConnectedPage.getId());
assertThat(createdAction.getDefaultResources().getApplicationId()).isEqualTo(gitConnectedPage.getApplicationId()); assertThat(createdAction.getDefaultResources().getApplicationId()).isEqualTo(gitConnectedPage.getApplicationId());
@ -449,6 +446,7 @@ public class ActionServiceCE_Test {
ActionMoveDTO actionMoveDTO = new ActionMoveDTO(); ActionMoveDTO actionMoveDTO = new ActionMoveDTO();
actionMoveDTO.setAction(savedAction); actionMoveDTO.setAction(savedAction);
actionMoveDTO.setDestinationPageId(destinationPage.getId()); actionMoveDTO.setDestinationPageId(destinationPage.getId());
assertThat(savedAction.getUserPermissions()).isNotEmpty();
return layoutActionService.moveAction(actionMoveDTO); return layoutActionService.moveAction(actionMoveDTO);
}); });