Made driver throw SQLSTATE 28000 for invalid username or password (#7)

* Made driver throw SQLSTATE 28000 for invalid username or password Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Made SQLSTATE string a constant and added a separate catch block for HTTPException Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Changed constant name to follow naming convention Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Added 08001 SQLSTATE to catch connection errors Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Reverted 08001 change Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> --------- Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>
opensearch-project · Jun 29, 2023 · 1d1c491 · 1d1c491
1 parent 5aa8c12
commit 1d1c491
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 1 deletion.
diff --git a/src/main/java/org/opensearch/jdbc/ConnectionImpl.java b/src/main/java/org/opensearch/jdbc/ConnectionImpl.java
@@ -17,6 +17,7 @@
 import org.opensearch.jdbc.protocol.Protocol;
 import org.opensearch.jdbc.protocol.ProtocolFactory;
 import org.opensearch.jdbc.protocol.exceptions.ResponseException;
+import org.opensearch.jdbc.protocol.http.HttpException;
 import org.opensearch.jdbc.protocol.http.JsonHttpProtocolFactory;
 import org.opensearch.jdbc.transport.Transport;
 import org.opensearch.jdbc.transport.TransportException;
@@ -55,6 +56,9 @@ public class ConnectionImpl implements OpenSearchConnection, JdbcWrapper, Loggin
     private Transport transport;
     private Protocol protocol;
     private ClusterMetadata clusterMetadata;
+    // https://docs.oracle.com/cd/E15817_01/appdev.111/b31228/appd.htm
+    // 28000 is the SQLSTATE for invalid authorization specification
+    private final String INCORRECT_CREDENTIALS_SQLSTATE = "28000";
 
     public ConnectionImpl(ConnectionConfig connectionConfig, Logger log) throws SQLException {
         this(connectionConfig, ApacheHttpTransportFactory.INSTANCE, JsonHttpProtocolFactory.INSTANCE, log);
@@ -83,8 +87,15 @@ log, new SQLNonTransientException("Could not initialize transport for the connec
             ConnectionResponse connectionResponse = this.protocol.connect(connectionConfig.getLoginTimeout() * 1000);
             this.clusterMetadata = connectionResponse.getClusterMetadata();
             this.open = true;
+        } catch (HttpException ex) {
+            if (ex.getStatusCode() == 401) {
+                logAndThrowSQLException(log, new SQLException("Connection error " + ex.getMessage(),
+                    INCORRECT_CREDENTIALS_SQLSTATE, ex));
+            } else {
+                logAndThrowSQLException(log, new SQLException("Connection error " + ex.getMessage(), ex));
+            }
         } catch (ResponseException | IOException ex) {
-            logAndThrowSQLException(log, new SQLException("Connection error "+ex.getMessage(), ex));
+            logAndThrowSQLException(log, new SQLException("Connection error " + ex.getMessage(), ex));
         }
 
     }

diff --git a/src/test/java/org/opensearch/jdbc/ConnectionTests.java b/src/test/java/org/opensearch/jdbc/ConnectionTests.java
@@ -7,11 +7,17 @@
 package org.opensearch.jdbc;
 
 import org.opensearch.jdbc.config.AuthConnectionProperty;
+import org.opensearch.jdbc.config.ConnectionConfig;
 import org.opensearch.jdbc.config.ConnectionPropertyException;
 import org.opensearch.jdbc.config.PasswordConnectionProperty;
 import org.opensearch.jdbc.config.RegionConnectionProperty;
 import org.opensearch.jdbc.config.RequestCompressionConnectionProperty;
 import org.opensearch.jdbc.config.UserConnectionProperty;
+import org.opensearch.jdbc.logging.NoOpLogger;
+import org.opensearch.jdbc.protocol.Protocol;
+import org.opensearch.jdbc.protocol.ProtocolFactory;
+import org.opensearch.jdbc.protocol.exceptions.ResponseException;
+import org.opensearch.jdbc.protocol.http.HttpException;
 import org.opensearch.jdbc.protocol.http.JsonHttpProtocol;
 import org.opensearch.jdbc.test.PerTestWireMockServerExtension;
 import org.opensearch.jdbc.test.WireMockServerHelpers;
@@ -25,6 +31,8 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
+import org.opensearch.jdbc.transport.Transport;
+import org.opensearch.jdbc.transport.TransportFactory;
 
 import java.io.IOException;
 import java.sql.Connection;
@@ -34,7 +42,12 @@
 
 import static com.github.tomakehurst.wiremock.client.WireMock.*;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 @ExtendWith(PerTestWireMockServerExtension.class)
 class ConnectionTests implements WireMockServerHelpers {
@@ -117,6 +130,28 @@ void testConnectDefaultAuthWithUsername(final WireMockServer mockServer) throws
         con.close();
     }
 
+    @Test
+    void testConnectInvalidUsernameOrPassword(final WireMockServer mockServer) throws ResponseException, IOException {
+        TransportFactory mockTransportFactory = mock(TransportFactory.class);
+        when(mockTransportFactory.getTransport(any(), any(), any()))
+                .thenReturn(mock(Transport.class));
+        ProtocolFactory mockProtocolFactory = mock(ProtocolFactory.class);
+        Protocol mockProtocol = mock(Protocol.class);
+
+        when(mockProtocolFactory.getProtocol(any(ConnectionConfig.class), any(Transport.class)))
+                .thenReturn(mockProtocol);
+        when(mockProtocol.connect(anyInt())).thenThrow(new HttpException(401, "Unauthorized"));
+
+        SQLException sqlException = Assertions.assertThrows(SQLException.class,
+                () -> new ConnectionImpl(mock(ConnectionConfig.class),
+                mockTransportFactory, mockProtocolFactory, NoOpLogger.INSTANCE));
+
+        // 28000 is the SQLSTATE for invalid authorization specification
+        // https://docs.oracle.com/cd/E15817_01/appdev.111/b31228/appd.htm
+        assertEquals(sqlException.getSQLState(), "28000");
+        assertEquals(sqlException.getMessage(), "Connection error Unauthorized");
+    }
+
     @Test
     void testConnectWithRequestCompression(final WireMockServer mockServer) throws SQLException {
         // Respond only if request mentions it accepts gzip