From b7435e546fe737b424107c4eef00bdb3f00353f1 Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Fri, 8 May 2020 15:39:36 +0000 Subject: [PATCH] Added action inheritance for permissions during create. No update permissions for action added. Fixed the existing test cases to run with the new code. TODO : Add test cases for action permissions. --- .../server/services/ActionServiceImpl.java | 46 +++++++++-- .../server/configurations/SeedMongoData.java | 7 +- .../server/services/ActionServiceTest.java | 78 ++++++++++++++++++- .../services/ApplicationServiceTest.java | 7 -- .../services/CurlImporterServiceTest.java | 18 ++++- 5 files changed, 138 insertions(+), 18 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java index 84d46ea797..b0c31d5821 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java @@ -6,16 +6,20 @@ import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.PaginationField; import com.appsmith.external.models.PaginationType; import com.appsmith.external.models.Param; +import com.appsmith.external.models.Policy; import com.appsmith.external.models.Provider; import com.appsmith.external.plugins.PluginExecutor; import com.appsmith.server.acl.AclPermission; +import com.appsmith.server.acl.PolicyGenerator; import com.appsmith.server.constants.AnalyticsEvents; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.Action; import com.appsmith.server.domains.ActionProvider; import com.appsmith.server.domains.Datasource; +import com.appsmith.server.domains.Page; import com.appsmith.server.domains.Plugin; import com.appsmith.server.domains.PluginType; +import com.appsmith.server.domains.User; import com.appsmith.server.dtos.ExecuteActionDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; @@ -56,6 +60,8 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import static com.appsmith.server.acl.AclPermission.MANAGE_PAGES; +import static com.appsmith.server.acl.AclPermission.READ_PAGES; import static com.appsmith.server.helpers.MustacheHelper.extractMustacheKeysFromJson; @Slf4j @@ -71,6 +77,7 @@ public class ActionServiceImpl extends BaseService user.getCurrentOrganizationId()) - .map(orgId -> { + + if (action.getPageId() == null || action.getPageId().isBlank()) { + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.PAGE_ID)); + } + + Mono userMono = sessionUserService + .getCurrentUser() + .cache(); + + return pageService + .findById(action.getPageId(), READ_PAGES) + .zipWith(userMono) + .map(tuple -> { + Page page = tuple.getT1(); + User user = tuple.getT2(); + + // Inherit the action policies from the page. + generateAndSetActionPolicies(page, user, action); + Datasource datasource = action.getDatasource(); if (datasource != null) { - datasource.setOrganizationId(orgId); + datasource.setOrganizationId(user.getCurrentOrganizationId()); action.setDatasource(datasource); } - action.setOrganizationId(orgId); + action.setOrganizationId(user.getCurrentOrganizationId()); return action; }) .flatMap(this::validateAndSaveActionToRepository); @@ -600,4 +625,13 @@ public class ActionServiceImpl extends BaseService policySet = page.getPolicies().stream() + .filter(policy -> policy.getPermission().equals(MANAGE_PAGES.getValue()) + || policy.getPermission().equals(READ_PAGES.getValue())) + .collect(Collectors.toSet()); + Set documentPolicies = policyGenerator.getAllChildPolicies(user, policySet, Page.class); + action.setPolicies(documentPolicies); + } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java index af0bc3414f..893da986a6 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java @@ -33,6 +33,7 @@ import static com.appsmith.server.acl.AclPermission.MANAGE_PAGES; import static com.appsmith.server.acl.AclPermission.ORGANIZATION_MANAGE_APPLICATIONS; import static com.appsmith.server.acl.AclPermission.READ_APPLICATIONS; import static com.appsmith.server.acl.AclPermission.READ_ORGANIZATIONS; +import static com.appsmith.server.acl.AclPermission.READ_PAGES; import static com.appsmith.server.acl.AclPermission.READ_USERS; import static com.appsmith.server.acl.AclPermission.USER_MANAGE_ORGANIZATIONS; import static org.springframework.data.mongodb.core.query.Criteria.where; @@ -73,6 +74,10 @@ public class SeedMongoData { .users(Set.of(API_USER_EMAIL)) .build(); + Policy readPagePolicy = Policy.builder().permission(READ_PAGES.getValue()) + .users(Set.of(API_USER_EMAIL)) + .build(); + Policy readOrgPolicy = Policy.builder().permission(READ_ORGANIZATIONS.getValue()) .users(Set.of(API_USER_EMAIL)) .build(); @@ -102,7 +107,7 @@ public class SeedMongoData { {"Another TestApplications", Set.of(manageAppPolicy, readAppPolicy)} }; Object[][] pageData = { - {"validPageName", Set.of(managePagePolicy)} + {"validPageName", Set.of(managePagePolicy, readPagePolicy)} }; Object[][] pluginData = { {"Installed Plugin Name", PluginType.API, "installed-plugin"}, diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionServiceTest.java index ed65a23289..91537720b9 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionServiceTest.java @@ -1,11 +1,16 @@ package com.appsmith.server.services; import com.appsmith.external.models.ActionConfiguration; +import com.appsmith.external.models.Policy; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.Action; +import com.appsmith.server.domains.Application; +import com.appsmith.server.domains.Page; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import lombok.extern.slf4j.Slf4j; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -18,7 +23,10 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; import java.util.Map; +import java.util.Set; +import static com.appsmith.server.acl.AclPermission.MANAGE_ACTIONS; +import static com.appsmith.server.acl.AclPermission.READ_ACTIONS; import static org.assertj.core.api.Assertions.assertThat; @RunWith(SpringRunner.class) @@ -29,12 +37,76 @@ public class ActionServiceTest { @Autowired ActionService actionService; + @Autowired + ApplicationPageService applicationPageService; + + @Autowired + PageService pageService; + + Application testApp = null; + + Page testPage = null; + + int i = 0; + + @Before + @WithUserDetails(value = "api_user") + public void setup() { + if (testApp == null && testPage == null) { + //Create application and page which will be used by the tests to create actions for. + Application application = new Application(); + application.setName("ActionServiceTest-App-" + String.valueOf(i)); + i++; + testApp = applicationPageService.createApplication(application).block(); + testPage = pageService.getById(testApp.getPages().get(0).getId()).block(); + } + } + + @After + @WithUserDetails(value = "api_user") + public void cleanup() { + applicationPageService.deleteApplication(testApp.getId()).block(); + testApp = null; + testPage = null; + + } + @Test @WithUserDetails(value = "api_user") + public void createValidActionAndCheckPermissions() { + Policy manageActionPolicy = Policy.builder().permission(MANAGE_ACTIONS.getValue()) + .users(Set.of("api_user")) + .build(); + Policy readActionPolicy = Policy.builder().permission(READ_ACTIONS.getValue()) + .users(Set.of("api_user")) + .build(); + + Action action = new Action(); + action.setName("validAction"); + action.setPageId(testPage.getId()); + ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + action.setActionConfiguration(actionConfiguration); + + Mono actionMono = actionService.create(action); + + StepVerifier + .create(actionMono) + .assertNext(createdAction -> { + assertThat(createdAction.getId()).isNotEmpty(); + assertThat(createdAction.getName()).isEqualTo(action.getName()); + assertThat(createdAction.getPolicies()).containsAll(Set.of(manageActionPolicy, readActionPolicy)); + }) + .verifyComplete(); + } + + + @Test + @WithUserDetails(value = "api_user") public void createValidActionWithJustName() { Action action = new Action(); action.setName("randomActionName"); - action.setPageId("randomPageId"); + action.setPageId(testPage.getId()); ActionConfiguration actionConfiguration = new ActionConfiguration(); actionConfiguration.setHttpMethod(HttpMethod.GET); action.setActionConfiguration(actionConfiguration); @@ -56,7 +128,7 @@ public class ActionServiceTest { public void createValidActionNullActionConfiguration() { Action action = new Action(); action.setName("randomActionName2"); - action.setPageId("randomPageId"); + action.setPageId(testPage.getId()); Mono actionMono = Mono.just(action) .flatMap(actionService::create); StepVerifier @@ -74,7 +146,7 @@ public class ActionServiceTest { @WithUserDetails(value = "api_user") public void invalidCreateActionNullName() { Action action = new Action(); - action.setPageId("randomPageId"); + action.setPageId(testPage.getId()); ActionConfiguration actionConfiguration = new ActionConfiguration(); actionConfiguration.setHttpMethod(HttpMethod.GET); action.setActionConfiguration(actionConfiguration); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java index ade7e6413a..b17759b857 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java @@ -72,13 +72,6 @@ public class ApplicationServiceTest { .users(Set.of("api_user")) .build(); - Policy managePagePolicy = Policy.builder().permission(MANAGE_PAGES.getValue()) - .users(Set.of("api_user")) - .build(); - Policy readPagePolicy = Policy.builder().permission(READ_PAGES.getValue()) - .users(Set.of("api_user")) - .build(); - StepVerifier .create(applicationMono) .assertNext(application -> { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/CurlImporterServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/CurlImporterServiceTest.java index de81241f5d..9a6c28d756 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/CurlImporterServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/CurlImporterServiceTest.java @@ -3,7 +3,10 @@ package com.appsmith.server.services; import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.Property; import com.appsmith.external.plugins.PluginExecutor; +import com.appsmith.server.acl.AclPermission; import com.appsmith.server.domains.Action; +import com.appsmith.server.domains.Application; +import com.appsmith.server.domains.Page; import lombok.extern.slf4j.Slf4j; import org.junit.Before; import org.junit.Test; @@ -38,6 +41,12 @@ public class CurlImporterServiceTest { @MockBean PluginExecutor pluginExecutor; + @Autowired + ApplicationPageService applicationPageService; + + @Autowired + PageService pageService; + @Before public void setup() { Mockito.when(this.pluginManager.getExtensions(Mockito.any(), Mockito.anyString())) @@ -82,8 +91,15 @@ public class CurlImporterServiceTest { @Test @WithUserDetails(value = "api_user") public void importValidCurlCommand() { + // Set up the application & page for which this import curl action would be added + Application app = new Application(); + app.setName("curlTest App"); + + Application application = applicationPageService.createApplication(app).block(); + Page page = pageService.findById(application.getPages().get(0).getId(), AclPermission.MANAGE_PAGES).block(); + String command = "curl -X GET http://localhost:8080/api/v1/actions?name=something -H 'Accept: */*' -H 'Accept-Encoding: gzip, deflate' -H 'Authorization: Basic YXBpX3VzZXI6OHVBQDsmbUI6Y252Tn57Iw==' -H 'Cache-Control: no-cache' -H 'Connection: keep-alive' -H 'Content-Type: application/json' -H 'Cookie: SESSION=97c5def4-4f72-45aa-96fe-e8a9f5ade0b5,SESSION=97c5def4-4f72-45aa-96fe-e8a9f5ade0b5; SESSION=' -H 'Host: localhost:8080' -H 'Postman-Token: 16e4b6bc-2c7a-4ab1-a127-bca382dfc0f0,a6655daa-db07-4c5e-aca3-3fd505bd230d' -H 'User-Agent: PostmanRuntime/7.20.1' -H 'cache-control: no-cache' -d '{someJson}'"; - Mono action = curlImporterService.importAction(command, "pageId", "actionName"); + Mono action = curlImporterService.importAction(command, page.getId(), "actionName"); StepVerifier .create(action) .assertNext(action1 -> {