diff --git a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/constants/ErrorMessages.java b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/constants/ErrorMessages.java index 466b205d2e07..53937354c813 100644 --- a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/constants/ErrorMessages.java +++ b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/constants/ErrorMessages.java @@ -39,6 +39,9 @@ public class ErrorMessages extends BasePluginErrorMessages { public static final String MISSING_ROW_INDEX_ERROR_MSG = "Missing required field 'Row index'"; + public static final String MISSING_SPREADSHEET_URL_SELECTED_SHEETS_ERROR_MSG = + "Missing required field 'Spreadsheet Url'. Please check if your datasource is authorized to use this spreadsheet."; + public static final String UNABLE_TO_CREATE_URI_ERROR_MSG = "Unable to create URI"; public static final String MISSING_VALID_RESPONSE_ERROR_MSG = "Missing a valid response object."; diff --git a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java index e99f97a9fe55..f336d54b8fc2 100644 --- a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java +++ b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/plugins/GoogleSheetsPlugin.java @@ -51,7 +51,7 @@ import static com.appsmith.external.helpers.PluginUtils.getDataValueSafelyFromFormData; import static com.appsmith.external.helpers.PluginUtils.setDataValueSafelyInFormData; import static com.appsmith.external.helpers.PluginUtils.validConfigurationPresentInFormData; -import static com.external.utils.SheetsUtil.getUserAuthorizedSheetIds; +import static com.external.utils.SheetsUtil.validateAndGetUserAuthorizedSheetIds; import static java.lang.Boolean.TRUE; @Slf4j @@ -175,7 +175,8 @@ public Mono executeCommon( // This will get list of authorised sheet ids from datasource config, and transform execution response to // contain only authorised files - final Set userAuthorizedSheetIds = getUserAuthorizedSheetIds(datasourceConfiguration); + final Set userAuthorizedSheetIds = + validateAndGetUserAuthorizedSheetIds(datasourceConfiguration, methodConfig); // Triggering the actual REST API call return executionMethod @@ -338,7 +339,8 @@ public Mono trigger( // This will get list of authorised sheet ids from datasource config, and transform trigger response to // contain only authorised files - Set userAuthorizedSheetIds = getUserAuthorizedSheetIds(datasourceConfiguration); + Set userAuthorizedSheetIds = + validateAndGetUserAuthorizedSheetIds(datasourceConfiguration, methodConfig); return triggerMethod .getTriggerClient(client, methodConfig) diff --git a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/utils/SheetsUtil.java b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/utils/SheetsUtil.java index 90b1e17698db..ee8506e3dfea 100644 --- a/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/utils/SheetsUtil.java +++ b/app/server/appsmith-plugins/googleSheetsPlugin/src/main/java/com/external/utils/SheetsUtil.java @@ -1,7 +1,11 @@ package com.external.utils; +import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; +import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.OAuth2; +import com.external.config.MethodConfig; +import com.external.constants.ErrorMessages; import com.external.enums.GoogleSheetMethodEnum; import com.fasterxml.jackson.databind.JsonNode; @@ -17,8 +21,10 @@ public class SheetsUtil { private static final String FILE_SPECIFIC_DRIVE_SCOPE = "https://www.googleapis.com/auth/drive.file"; private static final int USER_AUTHORIZED_SHEET_IDS_INDEX = 1; - public static Set getUserAuthorizedSheetIds(DatasourceConfiguration datasourceConfiguration) { + public static Set validateAndGetUserAuthorizedSheetIds( + DatasourceConfiguration datasourceConfiguration, MethodConfig methodConfig) { OAuth2 oAuth2 = (OAuth2) datasourceConfiguration.getAuthentication(); + Set userAuthorisedSheetIds = null; if (!isEmpty(datasourceConfiguration.getProperties()) && datasourceConfiguration.getProperties().size() > 1 && datasourceConfiguration.getProperties().get(USER_AUTHORIZED_SHEET_IDS_INDEX) != null @@ -33,9 +39,23 @@ public static Set getUserAuthorizedSheetIds(DatasourceConfiguration data .getProperties() .get(USER_AUTHORIZED_SHEET_IDS_INDEX) .getValue(); - return new HashSet(temp); + userAuthorisedSheetIds = new HashSet(temp); + + // This is added specifically for selected gsheets, so that whenever authorisation changes from one sheet to + // another + // We throw an error, this is done because when we use drive.file scope which is for selected sheets through + // file picker + // The access token for this scope grants access to all selected sheets across datasources + // we want to constraint the access for datasource to the sheet which was selected during ds authorisation + if (methodConfig != null + && methodConfig.getSpreadsheetId() != null + && !userAuthorisedSheetIds.contains(methodConfig.getSpreadsheetId())) { + throw new AppsmithPluginException( + AppsmithPluginError.PLUGIN_EXECUTE_ARGUMENT_ERROR, + ErrorMessages.MISSING_SPREADSHEET_URL_SELECTED_SHEETS_ERROR_MSG); + } } - return null; + return userAuthorisedSheetIds; } public static Map getSpreadsheetData( diff --git a/app/server/appsmith-plugins/googleSheetsPlugin/src/test/java/com/external/config/SheetsUtilTest.java b/app/server/appsmith-plugins/googleSheetsPlugin/src/test/java/com/external/config/SheetsUtilTest.java index 1b5d7856880a..c99f3f2ecd6f 100644 --- a/app/server/appsmith-plugins/googleSheetsPlugin/src/test/java/com/external/config/SheetsUtilTest.java +++ b/app/server/appsmith-plugins/googleSheetsPlugin/src/test/java/com/external/config/SheetsUtilTest.java @@ -1,22 +1,22 @@ package com.external.config; +import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; import com.appsmith.external.models.DatasourceConfiguration; import com.appsmith.external.models.OAuth2; import com.appsmith.external.models.Property; +import com.external.constants.ErrorMessages; import com.external.utils.SheetsUtil; import com.fasterxml.jackson.core.JsonProcessingException; import org.junit.jupiter.api.Test; -import java.util.ArrayList; -import java.util.List; -import java.util.Set; +import java.util.*; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.*; public class SheetsUtilTest { @Test - public void testGetUserAuthorizedSheetIds_allsheets_returnsNull() throws JsonProcessingException { + public void testValidateAndGetUserAuthorizedSheetIds_allsheets_returnsNull() throws JsonProcessingException { DatasourceConfiguration dsConfig = new DatasourceConfiguration(); List propList = new ArrayList(); Property prop = new Property(); @@ -24,12 +24,13 @@ public void testGetUserAuthorizedSheetIds_allsheets_returnsNull() throws JsonPro prop.setValue("test_email"); propList.add(prop); dsConfig.setProperties(propList); - Set result = SheetsUtil.getUserAuthorizedSheetIds(dsConfig); - assertEquals(result, null); + Set result = SheetsUtil.validateAndGetUserAuthorizedSheetIds(dsConfig, null); + assertEquals(null, result); } @Test - public void testGetUserAuthorizedSheetIds_specificSheets_returnsSetOfFileIds() throws JsonProcessingException { + public void testValidateAndGetUserAuthorizedSheetIds_specificSheets_returnsSetOfFileIds() + throws JsonProcessingException { DatasourceConfiguration dsConfig = new DatasourceConfiguration(); List propList = new ArrayList(); OAuth2 oAuth2 = new OAuth2(); @@ -47,7 +48,92 @@ public void testGetUserAuthorizedSheetIds_specificSheets_returnsSetOfFileIds() t propList.add(prop2); dsConfig.setProperties(propList); - Set result = SheetsUtil.getUserAuthorizedSheetIds(dsConfig); - assertEquals(result.size(), 1); + Set result = SheetsUtil.validateAndGetUserAuthorizedSheetIds(dsConfig, null); + assertEquals(1, result.size()); + } + + @Test + public void testValidateAndGetUserAuthorizedSheetIds_invalidSpecificSheets_throwsException() { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + List propList = new ArrayList<>(); + OAuth2 oAuth2 = new OAuth2(); + + oAuth2.setScopeString("https://www.googleapis.com/auth/drive.file"); + dsConfig.setAuthentication(oAuth2); + + Property prop1 = new Property("emailAddress", "test_email"); + propList.add(prop1); + + List ids = new ArrayList<>(); + ids.add("id1"); + + Property prop2 = new Property("userAuthorizedSheetIds", ids); + propList.add(prop2); + + dsConfig.setProperties(propList); + + // Create formData map with only the spreadsheetUrl + Map formData = new HashMap<>(); + Map spreadsheetUrlData = new HashMap<>(); + spreadsheetUrlData.put("data", "https://docs.google.com/spreadsheets/d/id2"); + formData.put("sheetUrl", spreadsheetUrlData); + + MethodConfig methodConfig = new MethodConfig(formData); + + AppsmithPluginException exception = assertThrows(AppsmithPluginException.class, () -> { + SheetsUtil.validateAndGetUserAuthorizedSheetIds(dsConfig, methodConfig); + }); + + String expectedErrorMessage = ErrorMessages.MISSING_SPREADSHEET_URL_SELECTED_SHEETS_ERROR_MSG; + assertEquals(expectedErrorMessage, exception.getMessage()); + } + + @Test + public void testValidateAndGetUserAuthorizedSheetIds_validSpecificSheets_returnsAuthorisedSheetIds() { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + List propList = new ArrayList<>(); + OAuth2 oAuth2 = new OAuth2(); + + oAuth2.setScopeString("https://www.googleapis.com/auth/drive.file"); + dsConfig.setAuthentication(oAuth2); + + Property prop1 = new Property("emailAddress", "test_email"); + propList.add(prop1); + + List ids = new ArrayList<>(); + ids.add("id1"); + + Property prop2 = new Property("userAuthorizedSheetIds", ids); + propList.add(prop2); + + dsConfig.setProperties(propList); + + // Create formData map with the spreadsheetUrl + Map formData = new HashMap<>(); + Map spreadsheetUrlData = new HashMap<>(); + spreadsheetUrlData.put("data", "https://docs.google.com/spreadsheets/d/id1"); + formData.put("sheetUrl", spreadsheetUrlData); + + MethodConfig methodConfig = new MethodConfig(formData); + + Set result = SheetsUtil.validateAndGetUserAuthorizedSheetIds(dsConfig, methodConfig); + + assertEquals(1, result.size()); + assertTrue(result.contains("id1")); + } + + @Test + public void testValidateAndGetUserAuthorizedSheetIds_allSheets_returnsNull() { + DatasourceConfiguration dsConfig = new DatasourceConfiguration(); + OAuth2 oAuth2 = new OAuth2(); + + oAuth2.setScopeString("https://www.googleapis.com/auth/drive,https://www.googleapis.com/auth/spreadsheets"); + dsConfig.setAuthentication(oAuth2); + + // Not adding any properties to dsConfig + + Set result = SheetsUtil.validateAndGetUserAuthorizedSheetIds(dsConfig, null); + + assertNull(result); } }