From 497dd1a21bdfdc148716958e84642a6443634868 Mon Sep 17 00:00:00 2001 From: Mojtaba Date: Wed, 16 Feb 2022 12:03:13 +0330 Subject: [PATCH] fix: showing invalid credentials message on MongoDB test datasource if there is an unauthorized error (#11124) * fix: if test datasource gets Unauthorized accessing admin database It will try to access the default database. * fix: fix broken test * fix: cleanup unused imports * test: make test more readable with better comments --- .../com/external/plugins/MongoPlugin.java | 52 ++++++++++++------- .../com/external/plugins/MongoPluginTest.java | 14 +++-- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java b/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java index 43175cb5a2..8dc0a50e32 100644 --- a/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java +++ b/app/server/appsmith-plugins/mongoPlugin/src/main/java/com/external/plugins/MongoPlugin.java @@ -40,12 +40,14 @@ import org.json.JSONArray; import org.json.JSONObject; import org.pf4j.Extension; import org.pf4j.PluginWrapper; +import org.reactivestreams.Publisher; import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; import reactor.core.scheduler.Schedulers; +import reactor.util.function.Tuple2; import java.math.BigDecimal; import java.math.BigInteger; @@ -61,6 +63,8 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; +import java.util.function.Function; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -870,29 +874,25 @@ public class MongoPlugin extends BasePlugin { @Override public Mono testDatasource(DatasourceConfiguration datasourceConfiguration) { + final Consumer> closeClient = tuple -> tuple.getT1().close(); + + Function timeoutExceptionThrowableFunction = error -> new AppsmithPluginException( + AppsmithPluginError.PLUGIN_DATASOURCE_TIMEOUT_ERROR, + "Connection timed out. Please check if the datasource configuration fields have " + + "been filled correctly." + ); return datasourceCreate(datasourceConfiguration) .flatMap(mongoClient -> { - return Mono.zip(Mono.just(mongoClient), - Mono.from(mongoClient.getDatabase("admin").runCommand(new Document( - "listDatabases", 1)))); - }) - .doOnSuccess(tuple -> { - MongoClient mongoClient = tuple.getT1(); + Publisher result = mongoClient.getDatabase("admin").runCommand(new Document( + "listDatabases", 1)); - if (mongoClient != null) { - mongoClient.close(); - } + + return Mono.zip(Mono.just(mongoClient), Mono.from(result)); }) + .doOnSuccess(closeClient) .then(Mono.just(new DatasourceTestResult())) .timeout(Duration.ofSeconds(TEST_DATASOURCE_TIMEOUT_SECONDS)) - .onErrorMap( - TimeoutException.class, - error -> new AppsmithPluginException( - AppsmithPluginError.PLUGIN_DATASOURCE_TIMEOUT_ERROR, - "Connection timed out. Please check if the datasource configuration fields have " + - "been filled correctly." - ) - ) + .onErrorMap(TimeoutException.class, timeoutExceptionThrowableFunction) .onErrorResume(error -> { /** * 1. Return OK response on "Unauthorized" exception. @@ -902,7 +902,23 @@ public class MongoPlugin extends BasePlugin { */ if (error instanceof MongoCommandException && ((MongoCommandException) error).getErrorCodeName().equals("Unauthorized")) { - return Mono.just(new DatasourceTestResult()); + return datasourceCreate(datasourceConfiguration) + .flatMap(mongoClient -> { + if (datasourceConfiguration.getConnection() != null && datasourceConfiguration.getConnection().getDefaultDatabaseName() != null) { + final String defaultDatabaseName = datasourceConfiguration.getConnection().getDefaultDatabaseName(); + Publisher result = mongoClient.getDatabase(defaultDatabaseName).runCommand(new Document( + "listCollections", 1).append("nameOnly", TRUE)); + return Mono.zip(Mono.just(mongoClient), Mono.from(result)); + } + return Mono.error(new AppsmithPluginException( + AppsmithPluginError.PLUGIN_DATASOURCE_TEST_GENERIC_ERROR, + "Not enough information to test the Datasource. " + + "Please provide the default database name.")); + + }).doOnSuccess(closeClient).then(Mono.just(new DatasourceTestResult())) + .timeout(Duration.ofSeconds(TEST_DATASOURCE_TIMEOUT_SECONDS)) + .onErrorMap(TimeoutException.class, timeoutExceptionThrowableFunction) + .onErrorResume(err -> Mono.just(new DatasourceTestResult(mongoErrorUtils.getReadableError(err)))); } return Mono.just(new DatasourceTestResult(mongoErrorUtils.getReadableError(error))); diff --git a/app/server/appsmith-plugins/mongoPlugin/src/test/java/com/external/plugins/MongoPluginTest.java b/app/server/appsmith-plugins/mongoPlugin/src/test/java/com/external/plugins/MongoPluginTest.java index 73d6a29c43..93f7e7b002 100644 --- a/app/server/appsmith-plugins/mongoPlugin/src/test/java/com/external/plugins/MongoPluginTest.java +++ b/app/server/appsmith-plugins/mongoPlugin/src/test/java/com/external/plugins/MongoPluginTest.java @@ -31,7 +31,6 @@ import org.junit.Assert; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; -import org.springframework.data.mongodb.core.query.Query; import org.testcontainers.containers.GenericContainer; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -79,8 +78,6 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; -import static org.springframework.data.mongodb.core.query.Criteria.where; -import static org.springframework.data.mongodb.core.query.Query.query; /** * Unit tests for MongoPlugin @@ -190,11 +187,10 @@ public class MongoPluginTest { } /* - * 1. Test that when a query is attempted to run on mongodb but refused because of lack of authorization, then - * also, it indicates a successful connection establishment. + * 1. Test that when a query is attempted to run on mongodb but refused because of lack of authorization. */ @Test - public void testDatasourceWithUnauthorizedException() throws NoSuchFieldException { + public void testDatasourceWithUnauthorizedException() { /* * 1. Create mock exception of type: MongoCommandException. * - mock method getErrorCodeName() to return String "Unauthorized". @@ -202,6 +198,8 @@ public class MongoPluginTest { MongoCommandException mockMongoCommandException = mock(MongoCommandException.class); when(mockMongoCommandException.getErrorCodeName()).thenReturn("Unauthorized"); when(mockMongoCommandException.getMessage()).thenReturn("Mock Unauthorized Exception"); + when(mockMongoCommandException.getErrorMessage()).thenReturn("Mock error : Expected 'something' , but got something else.\n" + + "Doc = [{find mockAction} {filter mockFilter} {limit 10} {$db mockDB} ...]"); /* * 1. Spy MongoPluginExecutor class. @@ -216,13 +214,13 @@ public class MongoPluginTest { doReturn(Mono.error(mockMongoCommandException)).when(spyMongoPluginExecutor).datasourceCreate(any()); /* - * 1. Test that MongoCommandException with error code "Unauthorized" is caught and no error is reported. + * 1. Test that MongoCommandException with error code "Unauthorized" is not successful because of invalid credentials. */ DatasourceConfiguration dsConfig = createDatasourceConfiguration(); StepVerifier .create(spyMongoPluginExecutor.testDatasource(dsConfig)) .assertNext(datasourceTestResult -> { - assertTrue(datasourceTestResult.isSuccess()); + assertFalse(datasourceTestResult.isSuccess()); }) .verifyComplete(); }