Skip to content

Commit

Permalink
Datasource is given permission for a public app during create/update …
Browse files Browse the repository at this point in the history 
…of a public action (#3086)

* Added test case to assert that new datasources and actions created post making an application public have the correct permissions for public execution.
  • Loading branch information
@trishaanand
trishaanand committed Feb 18, 2021
1 parent 2c395b8 commit b565301
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 32 deletions.
52 changes: 39 additions & 13 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/PolicyUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -106,6 +107,7 @@ public <T extends BaseDomain> T removePoliciesFromExistingObject(Map<String, Pol

/**
* Given a set of AclPermissions, generate all policies (including policies from lateral permissions) for the user.
*
* @param permissions
* @param user
* @return
Expand Down Expand Up @@ -162,19 +164,19 @@ public Flux<Datasource> updateWithNewPoliciesToDatasourcesByOrgId(String orgId,
public Flux<Datasource> updateWithNewPoliciesToDatasourcesByDatasourceIds(Set<String> ids, Map<String, Policy> datasourcePolicyMap, boolean addPolicyToObject) {

return datasourceRepository
.findAllByIds(ids, MANAGE_DATASOURCES)
// In case we have come across a datasource the current user is not allowed to manage, move on.
.switchIfEmpty(Mono.empty())
.flatMap(datasource -> {
Datasource updatedDatasource;
if (addPolicyToObject) {
updatedDatasource = addPoliciesToExistingObject(datasourcePolicyMap, datasource);
} else {
updatedDatasource = removePoliciesFromExistingObject(datasourcePolicyMap, datasource);
}

return Mono.just(updatedDatasource);
})
.findAllByIds(ids, MANAGE_DATASOURCES)
// In case we have come across a datasource the current user is not allowed to manage, move on.
.switchIfEmpty(Mono.empty())
.flatMap(datasource -> {
Datasource updatedDatasource;
if (addPolicyToObject) {
updatedDatasource = addPoliciesToExistingObject(datasourcePolicyMap, datasource);
} else {
updatedDatasource = removePoliciesFromExistingObject(datasourcePolicyMap, datasource);
}

return Mono.just(updatedDatasource);
})
.collectList()
.flatMapMany(datasources -> datasourceRepository.saveAll(datasources));
}
Expand Down Expand Up @@ -223,6 +225,7 @@ public Flux<NewPage> updateWithApplicationPermissionsToAllItsPages(String applic
* 1. Instead of bulk updating actions page wise, we do bulk update of actions in one go for the entire application.
* 2. If the action is associated with different pages (in published/unpublished page due to movement of action), fetching
* actions by applicationId ensures that we update ALL the actions and don't have to do special handling for the same.
*
* @param applicationId
* @param newActionPoliciesMap
* @param addPolicyToObject
Expand Down Expand Up @@ -253,4 +256,27 @@ public Map<String, Policy> generateInheritedPoliciesFromSourcePolicies(Map<Strin
.stream()
.collect(Collectors.toMap(Policy::getPermission, Function.identity()));
}

public Boolean isPermissionPresentForUser(Set<Policy> policies, String permission, String username) {

if (policies == null || policies.isEmpty()) {
return false;
}

Optional<Policy> requestedPermissionPolicyOptional = policies.stream().filter(policy -> {
if (policy.getPermission().equals(permission)) {
Set<String> users = policy.getUsers();
if (users.contains(username)) {
return true;
}
}
return false;
}).findFirst();

if (requestedPermissionPolicyOptional.isPresent()) {
return true;
}

return false;
}
}
70 changes: 54 additions & 16 deletions ...rver/appsmith-server/src/main/java/com/appsmith/server/services/NewActionServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.MustacheHelper;
import com.appsmith.server.helpers.PluginExecutorHelper;
import com.appsmith.server.helpers.PolicyUtils;
import com.appsmith.server.repositories.NewActionRepository;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang.ArrayUtils;
Expand Down Expand Up @@ -93,6 +94,7 @@ public class NewActionServiceImpl extends BaseService<NewActionRepository, NewAc
private final NewPageService newPageService;
private final ApplicationService applicationService;
private final SessionUserService sessionUserService;
private final PolicyUtils policyUtils;

public NewActionServiceImpl(Scheduler scheduler,
Validator validator,
Expand All @@ -108,7 +110,8 @@ public NewActionServiceImpl(Scheduler scheduler,
PolicyGenerator policyGenerator,
NewPageService newPageService,
ApplicationService applicationService,
SessionUserService sessionUserService) {
SessionUserService sessionUserService,
PolicyUtils policyUtils) {
super(scheduler, validator, mongoConverter, reactiveMongoTemplate, repository, analyticsService);
this.repository = repository;
this.datasourceService = datasourceService;
Expand All @@ -120,6 +123,7 @@ public NewActionServiceImpl(Scheduler scheduler,
this.newPageService = newPageService;
this.applicationService = applicationService;
this.sessionUserService = sessionUserService;
this.policyUtils = policyUtils;
}

private Boolean validateActionName(String name) {
Expand Down Expand Up @@ -295,7 +299,9 @@ private Mono<ActionDTO> validateAndSaveActionToRepository(NewAction newAction) {
// datasource is found. Update the action.
newAction.setOrganizationId(datasource.getOrganizationId());
return datasource;
});
})
// If the action is publicly executable, update the datasource policy
.flatMap(datasource -> updateDatasourcePolicyForPublicAction(newAction.getPolicies(), datasource));
}

Mono<Plugin> pluginMono = datasourceMono.flatMap(datasource -> {
Expand Down Expand Up @@ -972,12 +978,13 @@ public Flux<NewAction> findByPageId(String pageId) {
/**
* !!!WARNING!!! This function edits the parameters actionUpdates and messages which are eventually returned back to
* the caller with the updates values.
*
* @param onLoadActions : All the actions which have been found to be on page load
* @param pageId
* @param actionUpdates : Empty array list which would be set in this function with all the page actions whose
* execute on load setting has changed (whether flipped from true to false, or vice versa)
* @param messages : Empty array list which would be set in this function with all the messages that should be
* displayed to the developer user communicating the action executeOnLoad changes.
* @param messages : Empty array list which would be set in this function with all the messages that should be
* displayed to the developer user communicating the action executeOnLoad changes.
* @return
*/
@Override
Expand Down Expand Up @@ -1006,7 +1013,7 @@ public Mono<Boolean> updateActionsExecuteOnLoad(List<ActionDTO> onLoadActions,

return existingOnPageLoadActionsMono
.zipWith(pageActionsFlux.collectList())
.flatMap( tuple -> {
.flatMap(tuple -> {
List<ActionDTO> existingOnPageLoadActions = tuple.getT1();
List<ActionDTO> pageActions = tuple.getT2();

Expand Down Expand Up @@ -1071,7 +1078,7 @@ public Mono<Boolean> updateActionsExecuteOnLoad(List<ActionDTO> onLoadActions,

// Add newly turned on page actions to report back to the caller
actionUpdates.addAll(
addActionUpdatesForActionNames(pageActions, turnedOnActionNames)
addActionUpdatesForActionNames(pageActions, turnedOnActionNames)
);

// Add newly turned off page actions to report back to the caller
Expand Down Expand Up @@ -1100,16 +1107,16 @@ private List<LayoutActionUpdateDTO> addActionUpdatesForActionNames(List<ActionDT
Set<String> actionNames) {

return pageActions
.stream()
.filter(pageAction -> actionNames.contains(pageAction.getName()))
.map(pageAction -> {
LayoutActionUpdateDTO layoutActionUpdateDTO = new LayoutActionUpdateDTO();
layoutActionUpdateDTO.setId(pageAction.getId());
layoutActionUpdateDTO.setName(pageAction.getName());
layoutActionUpdateDTO.setExecuteOnLoad(pageAction.getExecuteOnLoad());
return layoutActionUpdateDTO;
})
.collect(Collectors.toList());
.stream()
.filter(pageAction -> actionNames.contains(pageAction.getName()))
.map(pageAction -> {
LayoutActionUpdateDTO layoutActionUpdateDTO = new LayoutActionUpdateDTO();
layoutActionUpdateDTO.setId(pageAction.getId());
layoutActionUpdateDTO.setName(pageAction.getName());
layoutActionUpdateDTO.setExecuteOnLoad(pageAction.getExecuteOnLoad());
return layoutActionUpdateDTO;
})
.collect(Collectors.toList());
}

@Override
Expand All @@ -1121,4 +1128,35 @@ public Mono<NewAction> delete(String id) {
.flatMap(analyticsService::sendDeleteEvent);
}

private Mono<Datasource> updateDatasourcePolicyForPublicAction(Set<Policy> actionPolicies, Datasource datasource) {
if (datasource.getId() == null) {
// This seems to be a nested datasource. Return as is.
return Mono.just(datasource);
}

// If action has EXECUTE permission for anonymous, check and assign the same to the datasource.
if (policyUtils.isPermissionPresentForUser(actionPolicies, EXECUTE_ACTIONS.getValue(), FieldName.ANONYMOUS_USER)) {
// Check if datasource has execute permission
if (policyUtils.isPermissionPresentForUser(datasource.getPolicies(), EXECUTE_DATASOURCES.getValue(), FieldName.ANONYMOUS_USER)) {
// Datasource has correct permission. Return as is
return Mono.just(datasource);
}
// Add the permission to datasource
AclPermission datasourcePermission = EXECUTE_DATASOURCES;

User user = new User();
user.setName(FieldName.ANONYMOUS_USER);
user.setEmail(FieldName.ANONYMOUS_USER);
user.setIsAnonymous(true);

Map<String, Policy> datasourcePolicyMap = policyUtils.generatePolicyFromPermission(Set.of(datasourcePermission), user);

Datasource updatedDatasource = policyUtils.addPoliciesToExistingObject(datasourcePolicyMap, datasource);

return datasourceService.save(updatedDatasource);
}

return Mono.just(datasource);
}

}
101 changes: 98 additions & 3 deletions app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionServiceTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,27 +1,29 @@
package com.appsmith.server.services;

import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
import com.appsmith.external.exceptions.pluginExceptions.StaleConnectionException;
import com.appsmith.external.models.ActionConfiguration;
import com.appsmith.external.models.ActionExecutionResult;
import com.appsmith.external.models.DatasourceConfiguration;
import com.appsmith.external.models.PaginationField;
import com.appsmith.external.models.PaginationType;
import com.appsmith.external.models.Policy;
import com.appsmith.external.models.Property;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError;
import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException;
import com.appsmith.external.exceptions.pluginExceptions.StaleConnectionException;
import com.appsmith.external.plugins.PluginExecutor;
import com.appsmith.server.acl.AclPermission;
import com.appsmith.server.constants.FieldName;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.Datasource;
import com.appsmith.server.domains.Layout;
import com.appsmith.server.domains.NewAction;
import com.appsmith.server.domains.Organization;
import com.appsmith.server.domains.Plugin;
import com.appsmith.server.domains.User;
import com.appsmith.server.dtos.ActionDTO;
import com.appsmith.server.dtos.ActionMoveDTO;
import com.appsmith.server.dtos.ActionViewDTO;
import com.appsmith.server.dtos.ApplicationAccessDTO;
import com.appsmith.server.dtos.ExecuteActionDTO;
import com.appsmith.server.dtos.PageDTO;
import com.appsmith.server.exceptions.AppsmithError;
Expand Down Expand Up @@ -55,8 +57,12 @@
import java.util.UUID;

import static com.appsmith.external.constants.ActionConstants.DEFAULT_ACTION_EXECUTION_TIMEOUT_MS;
import static com.appsmith.server.acl.AclPermission.EXECUTE_ACTIONS;
import static com.appsmith.server.acl.AclPermission.EXECUTE_DATASOURCES;
import static com.appsmith.server.acl.AclPermission.MANAGE_ACTIONS;
import static com.appsmith.server.acl.AclPermission.MANAGE_DATASOURCES;
import static com.appsmith.server.acl.AclPermission.READ_ACTIONS;
import static com.appsmith.server.acl.AclPermission.READ_DATASOURCES;
import static com.appsmith.server.acl.AclPermission.READ_PAGES;
import static org.assertj.core.api.Assertions.assertThat;

Expand Down Expand Up @@ -107,6 +113,12 @@ public class ActionServiceTest {
@Autowired
ActionCollectionService actionCollectionService;

@Autowired
PluginService pluginService;

@Autowired
ApplicationService applicationService;

Application testApp = null;

PageDTO testPage = null;
Expand Down Expand Up @@ -883,4 +895,87 @@ public void updateShouldNotResetUserSetOnLoad() {
})
.verifyComplete();
}

@Test
@WithUserDetails(value = "api_user")
public void checkNewActionAndNewDatasourceAnonymousPermissionInPublicApp() {
Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(new MockPluginExecutor()));

Application application = new Application();
application.setName("validApplicationPublic-ExplicitDatasource-Test");

Policy manageDatasourcePolicy = Policy.builder().permission(MANAGE_DATASOURCES.getValue())
.users(Set.of("api_user"))
.build();
Policy readDatasourcePolicy = Policy.builder().permission(READ_DATASOURCES.getValue())
.users(Set.of("api_user"))
.build();
Policy executeDatasourcePolicy = Policy.builder().permission(EXECUTE_DATASOURCES.getValue())
.users(Set.of("api_user", FieldName.ANONYMOUS_USER))
.build();

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();
Policy executeActionPolicy = Policy.builder().permission(EXECUTE_ACTIONS.getValue())
.users(Set.of("api_user", FieldName.ANONYMOUS_USER))
.build();

Application createdApplication = applicationPageService.createApplication(application, orgId).block();

String pageId = createdApplication.getPages().get(0).getId();

Plugin plugin = pluginService.findByName("Installed Plugin Name").block();

ApplicationAccessDTO applicationAccessDTO = new ApplicationAccessDTO();
applicationAccessDTO.setPublicAccess(true);

Mono<Application> publicAppMono = applicationService
.changeViewAccess(createdApplication.getId(), applicationAccessDTO)
.cache();

Datasource datasource = new Datasource();
datasource.setName("After Public Datasource");
datasource.setPluginId(plugin.getId());
DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration();
datasourceConfiguration.setUrl("http://test.com");
datasource.setDatasourceConfiguration(datasourceConfiguration);
datasource.setOrganizationId(orgId);

Datasource savedDatasource = datasourceService.create(datasource).block();

ActionDTO action = new ActionDTO();
action.setName("After Public action");
action.setPageId(pageId);
action.setDatasource(savedDatasource);
ActionConfiguration actionConfiguration1 = new ActionConfiguration();
actionConfiguration1.setHttpMethod(HttpMethod.GET);
action.setActionConfiguration(actionConfiguration1);

ActionDTO savedAction = newActionService.createAction(action).block();

Mono<Datasource> datasourceMono = publicAppMono
.then(datasourceService.findById(savedDatasource.getId()));

Mono<NewAction> actionMono = publicAppMono
.then(newActionService.findById(savedAction.getId()));

StepVerifier
.create(Mono.zip(datasourceMono, actionMono))
.assertNext(tuple -> {
Datasource datasourceFromDb = tuple.getT1();
NewAction actionFromDb = tuple.getT2();

// Check that the datasource used in the app contains public execute permission
assertThat(datasourceFromDb.getPolicies()).containsAll(Set.of(manageDatasourcePolicy, readDatasourcePolicy, executeDatasourcePolicy));

// Check that the action used in the app contains public execute permission
assertThat(actionFromDb.getPolicies()).containsAll(Set.of(manageActionPolicy, readActionPolicy, executeActionPolicy));

})
.verifyComplete();
}
}

0 comments on commit b565301

Please sign in to comment.