Improve internal connection pool key defaults

docs/using-the-jdbc-driver/using-plugins/UsingTheReadWriteSplittingPlugin.md

@@ -39,7 +39,7 @@ The wrapper driver currently uses [Hikari](https://github.com/brettwooldridge/Hi
 - username
 - password
 
-You can optionally pass in a `HikariPoolMapping` function as a second parameter to the `HikariPooledConnectionProvider`. Internally, the connection pools used by the plugin are maintained as a map from instance URLs to connection pools. If you would like to define a different key system, you should pass in a `HikariPoolMapping` function defining this logic. This is helpful, for example, when you would like to create multiple Connection objects to the same instance with different users. In this scenario, you should pass in a `HikariPoolMapping` that incorporates the instance URL and the username from the `Properties` object into the map key.
+You can optionally pass in a `HikariPoolMapping` function as a second parameter to the `HikariPooledConnectionProvider`. This allows you to decide when new connection pools should be created by defining what is included in the pool map key. A new pool will be created each time a new connection is requested with a unique key. By default, a new pool will be created for each unique instance-user combination. If you would like to define a different key system, you should pass in a `HikariPoolMapping` function defining this logic. Note that the user and password will always be included in the key for security reasons.
 
 2. Call `ConnectionProviderManager.setConnectionProvider`, passing in the `HikariPooledConnectionProvider` you created in step 1.
 

wrapper/src/main/java/software/amazon/jdbc/HikariPooledConnectionProvider.java

@@ -120,7 +120,7 @@ public Connection connect(
       @NonNull Properties props)
       throws SQLException {
     final HikariDataSource ds = databasePools.computeIfAbsent(
-        poolMapping.getKey(hostSpec, props),
+        getPoolKey(hostSpec, props),
         url -> createHikariDataSource(protocol, hostSpec, props)
     );
 
@@ -140,6 +140,14 @@ public Connection connect(
     return null;
   }
 
+  // The pool key should always be retrieved using this method, because the username/password
+  // must always be included to avoid sharing privileged connections with other users.
+  private String getPoolKey(HostSpec hostSpec, Properties props) {
+    return poolMapping.getKey(hostSpec, props)
+        + props.getProperty(PropertyDefinition.USER.name)
+        + props.getProperty(PropertyDefinition.PASSWORD.name);
+  }
+
   @Override
   public void releaseResources() {
     databasePools.forEach((String url, HikariDataSource ds) -> ds.close());

wrapper/src/test/java/integration/refactored/container/tests/ReadWriteSplittingTests.java

@@ -27,10 +27,12 @@
 import static org.junit.jupiter.api.Assertions.fail;
 
 import com.zaxxer.hikari.HikariConfig;
+import com.zaxxer.hikari.pool.HikariPool;
 import integration.refactored.DatabaseEngine;
 import integration.refactored.DatabaseEngineDeployment;
 import integration.refactored.DriverHelper;
 import integration.refactored.TestEnvironmentFeatures;
+import integration.refactored.TestEnvironmentInfo;
 import integration.refactored.TestInstanceInfo;
 import integration.refactored.container.ConnectionStringHelper;
 import integration.refactored.container.ProxyHelper;
@@ -597,7 +599,7 @@ public void test_pooledConnection_reuseCachedConnection() throws SQLException {
   protected static HikariConfig getHikariConfig(HostSpec hostSpec, Properties props) {
     final HikariConfig config = new HikariConfig();
     config.setMaximumPoolSize(1);
-    config.setInitializationFailTimeout(75000);
+    config.setInitializationFailTimeout(20000);
     return config;
   }
 
@@ -758,4 +760,74 @@ public void test_pooledConnection_failoverInTransaction()
       ConnectionProviderManager.resetProvider();
     }
   }
+
+  @TestTemplate
+  public void test_pooledConnection_differentUsers() throws SQLException {
+    Properties privilegedUserProps = getProps();
+
+    Properties privilegedUserWithWrongPasswordProps = getProps();
+    privilegedUserWithWrongPasswordProps.setProperty(PropertyDefinition.PASSWORD.name, "bogus_password");
+
+    Properties limitedUserProps = getProps();
+    String limitedUserName = "limited_user";
+    String limitedUserPassword = "limited_user";
+    String limitedUserNewDb = "limited_user_db";
+    limitedUserProps.setProperty(PropertyDefinition.USER.name, limitedUserName);
+    limitedUserProps.setProperty(PropertyDefinition.PASSWORD.name, limitedUserPassword);
+
+    Properties wrongUserRightPasswordProps = getProps();
+    wrongUserRightPasswordProps.setProperty(PropertyDefinition.USER.name, "bogus_user");
+
+    final HikariPooledConnectionProvider provider =
+        new HikariPooledConnectionProvider(ReadWriteSplittingTests::getHikariConfig);
+    ConnectionProviderManager.setConnectionProvider(provider);
+
+    try {
+      try (Connection conn = DriverManager.getConnection(ConnectionStringHelper.getWrapperUrl(),
+          privilegedUserProps); Statement stmt = conn.createStatement()) {
+        stmt.execute("DROP USER IF EXISTS " + limitedUserName);
+        auroraUtil.createUser(conn, limitedUserName, limitedUserPassword);
+        TestEnvironmentInfo info = TestEnvironment.getCurrent().getInfo();
+        DatabaseEngine engine = info.getRequest().getDatabaseEngine();
+        if (DatabaseEngine.MYSQL.equals(engine)) {
+          String db = info.getDatabaseInfo().getDefaultDbName();
+          // MySQL needs this extra command to allow the limited user access to the database
+          stmt.execute("GRANT ALL PRIVILEGES ON " + db + ".* to " + limitedUserName);
+        }
+      }
+
+      try (final Connection conn = DriverManager.getConnection(
+          ConnectionStringHelper.getWrapperUrl(), limitedUserProps);
+           Statement stmt = conn.createStatement()) {
+        assertThrows(SQLException.class,
+            () -> stmt.execute("CREATE DATABASE " + limitedUserNewDb));
+      }
+
+      assertThrows(
+          HikariPool.PoolInitializationException.class, () -> {
+            try (final Connection conn = DriverManager.getConnection(
+                ConnectionStringHelper.getWrapperUrl(), privilegedUserWithWrongPasswordProps)) {
+              // Do nothing (close connection automatically)
+            }
+          });
+
+      assertThrows(
+          HikariPool.PoolInitializationException.class, () -> {
+            try (final Connection conn = DriverManager.getConnection(
+                ConnectionStringHelper.getWrapperUrl(), wrongUserRightPasswordProps)) {
+              // Do nothing (close connection automatically)
+            }
+          });
+    } finally {
+      try (Connection conn = DriverManager.getConnection(ConnectionStringHelper.getWrapperUrl(),
+          privilegedUserProps);
+           Statement stmt = conn.createStatement()) {
+        stmt.execute("DROP DATABASE IF EXISTS " + limitedUserNewDb);
+        stmt.execute("DROP USER IF EXISTS " + limitedUserName);
+      }
+
+      ConnectionProviderManager.releaseResources();
+      ConnectionProviderManager.resetProvider();
+    }
+  }
 }

wrapper/src/test/java/integration/util/AuroraTestUtility.java

@@ -73,7 +73,6 @@
 import software.amazon.awssdk.services.rds.model.Filter;
 import software.amazon.awssdk.services.rds.model.Tag;
 import software.amazon.awssdk.services.rds.waiters.RdsWaiter;
-import software.amazon.jdbc.plugin.failover.FailoverSuccessSQLException;
 import software.amazon.jdbc.util.StringUtils;
 
 /**
@@ -762,6 +761,25 @@ public String executeInstanceIdQuery(DatabaseEngine databaseEngine, Statement st
     return null;
   }
 
+  public void createUser(Connection conn, String username, String password) throws SQLException {
+    DatabaseEngine engine = TestEnvironment.getCurrent().getInfo().getRequest().getDatabaseEngine();
+    String dropUserSql = getCreateUserSql(engine, username, password);
+    try (Statement stmt = conn.createStatement()) {
+      stmt.execute(dropUserSql);
+    }
+  }
+
+  protected String getCreateUserSql(DatabaseEngine engine, String username, String password) {
+    switch (engine) {
+      case MYSQL:
+        return "CREATE USER " + username + " identified by '" + password + "'";
+      case PG:
+        return "CREATE USER " + username + " with password '" + password + "'";
+      default:
+        throw new UnsupportedOperationException(engine.toString());
+    }
+  }
+
   public void addAuroraAwsIamUser(
       DatabaseEngine databaseEngine,
       String connectionUrl,

wrapper/src/test/java/software/amazon/jdbc/HikariPooledConnectionProviderTest.java

@@ -62,15 +62,18 @@ void tearDown() throws Exception {
   @Test
   void testConnectWithDefaultMapping() throws SQLException {
     when(mockHostSpec.getUrl()).thenReturn("url");
-    final Set<String> expected = new HashSet<>(Collections.singletonList("url"));
+    final Set<String> expected = new HashSet<>(Collections.singletonList("urlusernamepassword"));
 
     final HikariPooledConnectionProvider provider =
         spy(new HikariPooledConnectionProvider((hostSpec, properties) -> mockConfig));
 
     doReturn(mockDataSource).when(provider).createHikariDataSource(any(), any(), any());
 
+    Properties props = new Properties();
+    props.setProperty(PropertyDefinition.USER.name, "username");
+    props.setProperty(PropertyDefinition.PASSWORD.name, "password");
     try (Connection conn = provider.connect(
-        "protocol", mockDialect, mockHostSpec, emptyProperties)) {
+        "protocol", mockDialect, mockHostSpec, props)) {
       assertEquals(mockConnection, conn);
       assertEquals(1, provider.getHostCount());
       final Set<String> hosts = provider.getHosts();
@@ -83,16 +86,19 @@ void testConnectWithDefaultMapping() throws SQLException {
   @Test
   void testConnectWithCustomMapping() throws SQLException {
     when(mockHostSpec.getUrl()).thenReturn("url");
-    final Set<String> expected = new HashSet<>(Collections.singletonList("url+someUniqueKey"));
+    final Set<String> expected = new HashSet<>(Collections.singletonList("url+someUniqueKeyusernamepassword"));
 
     final HikariPooledConnectionProvider provider = spy(new HikariPooledConnectionProvider(
         (hostSpec, properties) -> mockConfig,
         (hostSpec, properties) -> hostSpec.getUrl() + "+someUniqueKey"));
 
     doReturn(mockDataSource).when(provider).createHikariDataSource(any(), any(), any());
 
+    Properties props = new Properties();
+    props.setProperty(PropertyDefinition.USER.name, "username");
+    props.setProperty(PropertyDefinition.PASSWORD.name, "password");
     try (Connection conn = provider.connect(
-        "protocol", mockDialect, mockHostSpec, emptyProperties)) {
+        "protocol", mockDialect, mockHostSpec, props)) {
       assertEquals(mockConnection, conn);
       assertEquals(1, provider.getHostCount());
       final Set<String> hosts = provider.getHosts();