Datasource is given permission for a public app during create/update of a public action

app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/PolicyUtils.java

@@ -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;
@@ -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
@@ -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));
     }
@@ -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
@@ -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;
+    }
 }

app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewActionServiceImpl.java

@@ -1,5 +1,8 @@
 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;
@@ -9,9 +12,6 @@
 import com.appsmith.external.models.Policy;
 import com.appsmith.external.models.Property;
 import com.appsmith.external.models.Provider;
-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.acl.PolicyGenerator;
@@ -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;
@@ -90,6 +91,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,
@@ -105,7 +107,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;
@@ -117,6 +120,7 @@ public NewActionServiceImpl(Scheduler scheduler,
         this.newPageService = newPageService;
         this.applicationService = applicationService;
         this.sessionUserService = sessionUserService;
+        this.policyUtils = policyUtils;
     }
 
     private Boolean validateActionName(String name) {
@@ -292,7 +296,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 -> {
@@ -922,12 +928,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
@@ -956,7 +963,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();
 
@@ -1021,7 +1028,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
@@ -1050,16 +1057,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
@@ -1071,4 +1078,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);
+    }
+
 }

app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionServiceTest.java

@@ -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;
@@ -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;
 
@@ -107,6 +113,12 @@ public class ActionServiceTest {
     @Autowired
     ActionCollectionService actionCollectionService;
 
+    @Autowired
+    PluginService pluginService;
+
+    @Autowired
+    ApplicationService applicationService;
+
     Application testApp = null;
 
     PageDTO testPage = null;
@@ -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();
+    }
 }