From f695af0238e73bd3f967b52396384205a6355d03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hubert=20=C5=81ojek?= Date: Thu, 10 Nov 2022 16:05:06 +0100 Subject: [PATCH] Do not require MongoDB privileges for fetching metadata Send `authorizedDatabases=true` parameter while listing databases/schemas Send `authorizedCollections=true` parameter while listing collections/tables Without those listing databases & collections require `listDatabases` & `listCollections` actions. --- plugin/trino-mongodb/pom.xml | 6 + .../io/trino/plugin/mongodb/MongoSession.java | 38 ++++-- .../mongodb/AuthenticatedMongoServer.java | 114 ++++++++++++++++++ .../mongodb/TestMongoConnectorTest.java | 12 ++ .../plugin/mongodb/TestMongoPrivileges.java | 97 +++++++++++++++ 5 files changed, 260 insertions(+), 7 deletions(-) create mode 100644 plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/AuthenticatedMongoServer.java create mode 100644 plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoPrivileges.java diff --git a/plugin/trino-mongodb/pom.xml b/plugin/trino-mongodb/pom.xml index a4e27229e53b..037bcd89f86d 100644 --- a/plugin/trino-mongodb/pom.xml +++ b/plugin/trino-mongodb/pom.xml @@ -197,6 +197,12 @@ test + + org.testcontainers + testcontainers + test + + org.testng testng diff --git a/plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoSession.java b/plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoSession.java index c36d1cf5e3c3..9a45465f119c 100644 --- a/plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoSession.java +++ b/plugin/trino-mongodb/src/main/java/io/trino/plugin/mongodb/MongoSession.java @@ -29,6 +29,7 @@ import com.mongodb.client.MongoCollection; import com.mongodb.client.MongoCursor; import com.mongodb.client.MongoDatabase; +import com.mongodb.client.MongoIterable; import com.mongodb.client.model.Collation; import com.mongodb.client.model.Filters; import com.mongodb.client.model.IndexOptions; @@ -141,6 +142,11 @@ public class MongoSession // The 'simple' locale is the default collection in MongoDB. The locale doesn't allow specifying other fields (e.g. numericOrdering) // https://www.mongodb.com/docs/manual/reference/collation/ private static final Collation SIMPLE_COLLATION = Collation.builder().locale("simple").build(); + private static final Map AUTHORIZED_LIST_COLLECTIONS_COMMAND = ImmutableMap.builder() + .put("listCollections", 1.0) + .put("nameOnly", true) + .put("authorizedCollections", true) + .buildOrThrow(); private final TypeManager typeManager; private final MongoClient client; @@ -180,9 +186,7 @@ public List getAddresses() public List getAllSchemas() { - return ImmutableList.copyOf(client.listDatabaseNames()).stream() - .map(name -> name.toLowerCase(ENGLISH)) - .collect(toImmutableList()); + return ImmutableList.copyOf(listDatabaseNames().map(name -> name.toLowerCase(ENGLISH))); } public void createSchema(String schemaName) @@ -202,7 +206,7 @@ public Set getAllTables(String schema) String schemaName = toRemoteSchemaName(schema); ImmutableSet.Builder builder = ImmutableSet.builder(); - builder.addAll(ImmutableList.copyOf(client.getDatabase(schemaName).listCollectionNames()).stream() + builder.addAll(ImmutableList.copyOf(listCollectionNames(schemaName)).stream() .filter(name -> !name.equals(schemaCollection)) .filter(name -> !SYSTEM_TABLES.contains(name)) .collect(toSet())); @@ -644,7 +648,7 @@ private Document getTableMetadata(String schemaName, String tableName) public boolean collectionExists(MongoDatabase db, String collectionName) { - for (String name : db.listCollectionNames()) { + for (String name : listCollectionNames(db.getName())) { if (name.equalsIgnoreCase(collectionName)) { return true; } @@ -840,7 +844,7 @@ private String toRemoteSchemaName(String schemaName) if (!caseInsensitiveNameMatching) { return schemaName; } - for (String remoteSchemaName : client.listDatabaseNames()) { + for (String remoteSchemaName : listDatabaseNames()) { if (schemaName.equals(remoteSchemaName.toLowerCase(ENGLISH))) { return remoteSchemaName; } @@ -848,13 +852,21 @@ private String toRemoteSchemaName(String schemaName) return schemaName; } + private MongoIterable listDatabaseNames() + { + return client.listDatabases() + .nameOnly(true) + .authorizedDatabasesOnly(true) + .map(result -> result.getString("name")); + } + private String toRemoteTableName(String schemaName, String tableName) { verify(tableName.equals(tableName.toLowerCase(ENGLISH)), "tableName not in lower-case: %s", tableName); if (!caseInsensitiveNameMatching) { return tableName; } - for (String remoteTableName : client.getDatabase(schemaName).listCollectionNames()) { + for (String remoteTableName : listCollectionNames(schemaName)) { if (tableName.equals(remoteTableName.toLowerCase(ENGLISH))) { return remoteTableName; } @@ -862,12 +874,24 @@ private String toRemoteTableName(String schemaName, String tableName) return tableName; } + private List listCollectionNames(String databaseName) + { + MongoDatabase database = client.getDatabase(databaseName); + Document cursor = database.runCommand(new Document(AUTHORIZED_LIST_COLLECTIONS_COMMAND)).get("cursor", Document.class); + + List firstBatch = cursor.get("firstBatch", List.class); + return firstBatch.stream() + .map(document -> document.getString("name")) + .collect(toImmutableList()); + } + private boolean isView(String schemaName, String tableName) { Document listCollectionsCommand = new Document(ImmutableMap.builder() .put("listCollections", 1.0) .put("filter", documentOf("name", tableName)) .put("nameOnly", true) + .put("authorizedCollections", true) .buildOrThrow()); Document cursor = client.getDatabase(schemaName).runCommand(listCollectionsCommand).get("cursor", Document.class); List firstBatch = cursor.get("firstBatch", List.class); diff --git a/plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/AuthenticatedMongoServer.java b/plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/AuthenticatedMongoServer.java new file mode 100644 index 000000000000..fa12de058609 --- /dev/null +++ b/plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/AuthenticatedMongoServer.java @@ -0,0 +1,114 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.plugin.mongodb; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.mongodb.ConnectionString; +import org.bson.Document; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.wait.strategy.Wait; + +import java.io.Closeable; + +import static java.util.Objects.requireNonNull; + +public class AuthenticatedMongoServer + implements Closeable +{ + private static final int MONGODB_INTERNAL_PORT = 27017; + private static final String ROOT_USER = "root"; + private static final String ROOT_PASSWORD = "password"; + private static final String TEST_USER = "testUser"; + private static final String TEST_PASSWORD = "pass"; + private static final String TEST_ROLE = "testRole"; + public static final String TEST_DATABASE = "test"; + public static final String TEST_COLLECTION = "testCollection"; + private final GenericContainer dockerContainer; + + public AuthenticatedMongoServer(String mongoVersion) + { + dockerContainer = new GenericContainer<>("mongo:" + requireNonNull(mongoVersion, "mongoVersion is null")) + .withStartupAttempts(3) + .waitingFor(Wait.forLogMessage(".*Listening on 0\\.0\\.0\\.0.*", 1)) + .withEnv("MONGO_INITDB_ROOT_USERNAME", ROOT_USER) + .withEnv("MONGO_INITDB_ROOT_PASSWORD", ROOT_PASSWORD) + .withEnv("MONGO_INITDB_DATABASE", "admin") + .withExposedPorts(MONGODB_INTERNAL_PORT) + .withCommand("--auth --bind_ip 0.0.0.0"); + dockerContainer.start(); + } + + public ConnectionString rootUserConnectionString() + { + return new ConnectionString("mongodb://%s:%s@%s:%d".formatted( + ROOT_USER, + ROOT_PASSWORD, + dockerContainer.getHost(), + dockerContainer.getMappedPort(MONGODB_INTERNAL_PORT))); + } + + public ConnectionString testUserConnectionString() + { + return new ConnectionString("mongodb://%s:%s@%s:%d/%s".formatted( + TEST_USER, + TEST_PASSWORD, + dockerContainer.getHost(), + dockerContainer.getMappedPort(MONGODB_INTERNAL_PORT), + TEST_DATABASE)); + } + + public static Document createTestRole() + { + return new Document(ImmutableMap.of( + "createRole", TEST_ROLE, + "privileges", ImmutableList.of(privilege("_schema"), privilege(TEST_COLLECTION)), + "roles", ImmutableList.of())); + } + + private static Document privilege(String collectionName) + { + return new Document(ImmutableMap.of( + "resource", resource(collectionName), + "actions", ImmutableList.of("find"))); + } + + private static Document resource(String collectionName) + { + return new Document(ImmutableMap.of( + "db", TEST_DATABASE, + "collection", collectionName)); + } + + public static Document createTestUser() + { + return new Document(ImmutableMap.of( + "createUser", TEST_USER, + "pwd", TEST_PASSWORD, + "roles", ImmutableList.of(role()))); + } + + private static Document role() + { + return new Document(ImmutableMap.of( + "role", TEST_ROLE, + "db", TEST_DATABASE)); + } + + @Override + public void close() + { + dockerContainer.close(); + } +} diff --git a/plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoConnectorTest.java b/plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoConnectorTest.java index 5789ec44077e..c655cc831c30 100644 --- a/plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoConnectorTest.java +++ b/plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoConnectorTest.java @@ -18,6 +18,7 @@ import com.mongodb.DBRef; import com.mongodb.client.MongoClient; import com.mongodb.client.MongoCollection; +import com.mongodb.client.MongoDatabase; import com.mongodb.client.model.Collation; import com.mongodb.client.model.CreateCollectionOptions; import io.trino.sql.planner.plan.LimitNode; @@ -824,6 +825,17 @@ public void testRenameTableTo120bytesTableName() assertUpdate("DROP TABLE \"" + sourceTableName + "\""); } + @Test + public void testListTablesFromSchemaWithBigAmountOfTables() + { + MongoDatabase database = client.getDatabase("huge_schema"); + for (int i = 0; i < 10_000; i++) { + database.createCollection("table_" + i); + } + + assertThat(getQueryRunner().execute("SHOW TABLES FROM mongodb.huge_schema").getRowCount()).isEqualTo(10_000); + } + @Override protected OptionalInt maxSchemaNameLength() { diff --git a/plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoPrivileges.java b/plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoPrivileges.java new file mode 100644 index 000000000000..ad48dd8f5507 --- /dev/null +++ b/plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoPrivileges.java @@ -0,0 +1,97 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.trino.plugin.mongodb; + +import com.google.common.collect.ImmutableMap; +import com.mongodb.client.MongoClient; +import com.mongodb.client.MongoClients; +import com.mongodb.client.MongoDatabase; +import io.trino.testing.AbstractTestQueryFramework; +import io.trino.testing.DistributedQueryRunner; +import io.trino.testing.QueryRunner; +import org.bson.Document; +import org.testng.annotations.Test; + +import java.util.Locale; +import java.util.Optional; + +import static io.airlift.testing.Closeables.closeAllSuppress; +import static io.trino.plugin.mongodb.AuthenticatedMongoServer.TEST_COLLECTION; +import static io.trino.plugin.mongodb.AuthenticatedMongoServer.TEST_DATABASE; +import static io.trino.plugin.mongodb.AuthenticatedMongoServer.createTestRole; +import static io.trino.plugin.mongodb.AuthenticatedMongoServer.createTestUser; +import static io.trino.testing.TestingSession.testSessionBuilder; +import static org.assertj.core.api.Assertions.assertThat; + +public class TestMongoPrivileges + extends AbstractTestQueryFramework +{ + @Override + protected QueryRunner createQueryRunner() + throws Exception + { + AuthenticatedMongoServer mongoServer = closeAfterClass(setupMongoServer()); + return createMongoQueryRunner(mongoServer.testUserConnectionString().getConnectionString()); + } + + @Test + public void testTablesVisibility() + { + assertQuery("SHOW TABLES FROM mongodb." + TEST_DATABASE, "VALUES '%s'".formatted(TEST_COLLECTION.toLowerCase(Locale.ENGLISH))); + } + + private static AuthenticatedMongoServer setupMongoServer() + { + AuthenticatedMongoServer mongoServer = new AuthenticatedMongoServer("4.2.0"); + try (MongoClient client = MongoClients.create(mongoServer.rootUserConnectionString())) { + MongoDatabase testDatabase = client.getDatabase(TEST_DATABASE); + runCommand(testDatabase, createTestRole()); + runCommand(testDatabase, createTestUser()); + testDatabase.createCollection("_schema"); + testDatabase.createCollection(TEST_COLLECTION); + testDatabase.createCollection("anotherCollection"); // this collection/table should not be visible + client.getDatabase("another").createCollection("_schema"); // this database/schema should not be visible + } + return mongoServer; + } + + private static void runCommand(MongoDatabase database, Document document) + { + Double commandStatus = database.runCommand(document) + .get("ok", Double.class); + assertThat(commandStatus).isEqualTo(1.0); + } + + private static DistributedQueryRunner createMongoQueryRunner(String connectionUrl) + throws Exception + { + DistributedQueryRunner queryRunner = null; + try { + queryRunner = DistributedQueryRunner.builder(testSessionBuilder() + .setCatalog(Optional.empty()) + .setSchema(Optional.empty()) + .build()) + .build(); + queryRunner.installPlugin(new MongoPlugin()); + queryRunner.createCatalog("mongodb", "mongodb", ImmutableMap.of( + "mongodb.case-insensitive-name-matching", "true", + "mongodb.connection-url", connectionUrl)); + return queryRunner; + } + catch (Throwable e) { + closeAllSuppress(e, queryRunner); + throw e; + } + } +}