From a81b2389b96b2af5ebe6629d0209ff9f672e4352 Mon Sep 17 00:00:00 2001 From: lutovich Date: Thu, 18 May 2017 12:26:57 +0200 Subject: [PATCH] Fail early on auth errors in direct driver This commit makes direct driver test connectivity on creation. It's done by opening and closing a connection. Previously driver did not try to connect during creation and auth errors (incorrect credentials, etc.) popped up only when first query was executed or first transaction was started. Such delayed errors were confusing. Also routing driver behaved differently because it fetches routing table at creation and auth errors come out of constructor. --- .../internal/DirectConnectionProvider.java | 11 +++ .../DirectConnectionProviderTest.java | 46 ++++++++- .../driver/internal/DirectDriverTest.java | 25 ++++- .../driver/internal/DriverFactoryTest.java | 20 +++- .../neo4j/driver/v1/GraphDatabaseTest.java | 8 +- .../v1/integration/ConnectionHandlingIT.java | 23 +++-- .../driver/v1/integration/CredentialsIT.java | 77 ++++++++++----- .../neo4j/driver/v1/integration/ErrorIT.java | 38 +++---- .../driver/v1/integration/LoggingIT.java | 12 +-- .../driver/v1/integration/SessionIT.java | 17 ++-- .../neo4j/driver/v1/tck/DriverAuthSteps.java | 51 +++++++--- .../v1/tck/DriverSecurityComplianceSteps.java | 98 +++++++++++-------- .../org/neo4j/driver/v1/util/StubServer.java | 9 +- .../test/resources/dummy_connection.script | 2 + .../neo4j/docs/driver/ConfigTrustExample.java | 2 +- 15 files changed, 293 insertions(+), 146 deletions(-) create mode 100644 driver/src/test/resources/dummy_connection.script diff --git a/driver/src/main/java/org/neo4j/driver/internal/DirectConnectionProvider.java b/driver/src/main/java/org/neo4j/driver/internal/DirectConnectionProvider.java index fd6b171126..3b80a75eac 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/DirectConnectionProvider.java +++ b/driver/src/main/java/org/neo4j/driver/internal/DirectConnectionProvider.java @@ -37,6 +37,8 @@ public class DirectConnectionProvider implements ConnectionProvider { this.address = address; this.pool = pool; + + verifyConnectivity(); } @Override @@ -55,4 +57,13 @@ public BoltServerAddress getAddress() { return address; } + + /** + * Acquires and releases a connection to verify connectivity so this connection provider fails fast. This is + * especially valuable when driver was created with incorrect credentials. + */ + private void verifyConnectivity() + { + acquireConnection( AccessMode.READ ).close(); + } } diff --git a/driver/src/test/java/org/neo4j/driver/internal/DirectConnectionProviderTest.java b/driver/src/test/java/org/neo4j/driver/internal/DirectConnectionProviderTest.java index ca479dfc87..406f07f08b 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/DirectConnectionProviderTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/DirectConnectionProviderTest.java @@ -25,10 +25,13 @@ import org.neo4j.driver.internal.spi.PooledConnection; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; +import static org.junit.Assert.fail; import static org.mockito.Matchers.any; +import static org.mockito.Mockito.RETURNS_MOCKS; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.only; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.neo4j.driver.v1.AccessMode.READ; @@ -42,7 +45,7 @@ public void acquiresConnectionsFromThePool() ConnectionPool pool = mock( ConnectionPool.class ); PooledConnection connection1 = mock( PooledConnection.class ); PooledConnection connection2 = mock( PooledConnection.class ); - when( pool.acquire( any( BoltServerAddress.class ) ) ).thenReturn( connection1 ).thenReturn( connection2 ); + when( pool.acquire( any( BoltServerAddress.class ) ) ).thenReturn( connection1, connection1, connection2 ); DirectConnectionProvider provider = newConnectionProvider( pool ); @@ -53,12 +56,12 @@ public void acquiresConnectionsFromThePool() @Test public void closesPool() throws Exception { - ConnectionPool pool = mock( ConnectionPool.class ); + ConnectionPool pool = mock( ConnectionPool.class, RETURNS_MOCKS ); DirectConnectionProvider provider = newConnectionProvider( pool ); provider.close(); - verify( pool, only() ).close(); + verify( pool ).close(); } @Test @@ -71,9 +74,42 @@ public void returnsCorrectAddress() assertEquals( address, provider.getAddress() ); } + @Test + public void testsConnectivityOnCreation() + { + ConnectionPool pool = mock( ConnectionPool.class ); + PooledConnection connection = mock( PooledConnection.class ); + when( pool.acquire( any( BoltServerAddress.class ) ) ).thenReturn( connection ); + + assertNotNull( newConnectionProvider( pool ) ); + + verify( pool ).acquire( BoltServerAddress.LOCAL_DEFAULT ); + verify( connection ).close(); + } + + @Test + public void throwsWhenTestConnectionThrows() + { + ConnectionPool pool = mock( ConnectionPool.class ); + PooledConnection connection = mock( PooledConnection.class ); + RuntimeException error = new RuntimeException(); + doThrow( error ).when( connection ).close(); + when( pool.acquire( any( BoltServerAddress.class ) ) ).thenReturn( connection ); + + try + { + newConnectionProvider( pool ); + fail( "Exception expected" ); + } + catch ( Exception e ) + { + assertSame( error, e ); + } + } + private static DirectConnectionProvider newConnectionProvider( BoltServerAddress address ) { - return new DirectConnectionProvider( address, mock( ConnectionPool.class ) ); + return new DirectConnectionProvider( address, mock( ConnectionPool.class, RETURNS_MOCKS ) ); } private static DirectConnectionProvider newConnectionProvider( ConnectionPool pool ) diff --git a/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java b/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java index 3df69e3bbd..531f73c91e 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java @@ -18,6 +18,8 @@ */ package org.neo4j.driver.internal; +import org.junit.After; +import org.junit.ClassRule; import org.junit.Test; import java.net.URI; @@ -28,6 +30,7 @@ import org.neo4j.driver.v1.Record; import org.neo4j.driver.v1.Session; import org.neo4j.driver.v1.util.StubServer; +import org.neo4j.driver.v1.util.TestNeo4j; import static org.hamcrest.Matchers.is; import static org.hamcrest.core.IsEqual.equalTo; @@ -40,6 +43,20 @@ public class DirectDriverTest { + @ClassRule + public static final TestNeo4j neo4j = new TestNeo4j(); + + private Driver driver; + + @After + public void closeDriver() throws Exception + { + if ( driver != null ) + { + driver.close(); + } + } + @Test public void shouldUseDefaultPortIfMissing() { @@ -47,7 +64,7 @@ public void shouldUseDefaultPortIfMissing() URI uri = URI.create( "bolt://localhost" ); // When - Driver driver = GraphDatabase.driver( uri ); + driver = GraphDatabase.driver( uri, neo4j.authToken() ); // Then assertThat( driver, is( directDriverWithAddress( LOCAL_DEFAULT ) ) ); @@ -61,7 +78,7 @@ public void shouldAllowIPv6Address() BoltServerAddress address = BoltServerAddress.from( uri ); // When - Driver driver = GraphDatabase.driver( uri ); + driver = GraphDatabase.driver( uri, neo4j.authToken() ); // Then assertThat( driver, is( directDriverWithAddress( address ) ) ); @@ -76,7 +93,7 @@ public void shouldRejectInvalidAddress() // When & Then try { - Driver driver = GraphDatabase.driver( uri ); + driver = GraphDatabase.driver( uri, neo4j.authToken() ); fail("Expecting error for wrong uri"); } catch( IllegalArgumentException e ) @@ -93,7 +110,7 @@ public void shouldRegisterSingleServer() BoltServerAddress address = BoltServerAddress.from( uri ); // When - Driver driver = GraphDatabase.driver( uri ); + driver = GraphDatabase.driver( uri, neo4j.authToken() ); // Then assertThat( driver, is( directDriverWithAddress( address ) ) ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/DriverFactoryTest.java b/driver/src/test/java/org/neo4j/driver/internal/DriverFactoryTest.java index a86beb69f6..c4c8c5fd93 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/DriverFactoryTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/DriverFactoryTest.java @@ -36,6 +36,7 @@ import org.neo4j.driver.internal.security.SecurityPlan; import org.neo4j.driver.internal.spi.ConnectionPool; import org.neo4j.driver.internal.spi.ConnectionProvider; +import org.neo4j.driver.internal.spi.PooledConnection; import org.neo4j.driver.v1.AuthToken; import org.neo4j.driver.v1.AuthTokens; import org.neo4j.driver.v1.Config; @@ -45,9 +46,11 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; +import static org.mockito.Mockito.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.neo4j.driver.v1.AccessMode.READ; import static org.neo4j.driver.v1.Config.defaultConfig; @@ -69,7 +72,7 @@ public static List uris() @Test public void connectionPoolClosedWhenDriverCreationFails() throws Exception { - ConnectionPool connectionPool = mock( ConnectionPool.class ); + ConnectionPool connectionPool = connectionPoolMock(); DriverFactory factory = new ThrowingDriverFactory( connectionPool ); try @@ -87,7 +90,7 @@ public void connectionPoolClosedWhenDriverCreationFails() throws Exception @Test public void connectionPoolCloseExceptionIsSupressedWhenDriverCreationFails() throws Exception { - ConnectionPool connectionPool = mock( ConnectionPool.class ); + ConnectionPool connectionPool = connectionPoolMock(); RuntimeException poolCloseError = new RuntimeException( "Pool close error" ); doThrow( poolCloseError ).when( connectionPool ).close(); @@ -142,6 +145,13 @@ private Driver createDriver( DriverFactory driverFactory, Config config ) return driverFactory.newInstance( uri, auth, routingSettings, RetrySettings.DEFAULT, config ); } + private static ConnectionPool connectionPoolMock() + { + ConnectionPool pool = mock( ConnectionPool.class ); + when( pool.acquire( any( BoltServerAddress.class ) ) ).thenReturn( mock( PooledConnection.class ) ); + return pool; + } + private static class ThrowingDriverFactory extends DriverFactory { final ConnectionPool connectionPool; @@ -196,5 +206,11 @@ protected SessionFactory createSessionFactory( ConnectionProvider connectionProv capturedSessionFactory = sessionFactory; return sessionFactory; } + + @Override + protected ConnectionPool createConnectionPool( AuthToken authToken, SecurityPlan securityPlan, Config config ) + { + return connectionPoolMock(); + } } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/GraphDatabaseTest.java b/driver/src/test/java/org/neo4j/driver/v1/GraphDatabaseTest.java index 791c7595cf..17175fc605 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/GraphDatabaseTest.java +++ b/driver/src/test/java/org/neo4j/driver/v1/GraphDatabaseTest.java @@ -39,16 +39,18 @@ public class GraphDatabaseTest { @Test - public void boltSchemeShouldInstantiateDirectDriver() + public void boltSchemeShouldInstantiateDirectDriver() throws Exception { // Given - URI uri = URI.create( "bolt://localhost:7687" ); + StubServer server = StubServer.start( "dummy_connection.script", 9001 ); + URI uri = URI.create( "bolt://localhost:9001" ); // When - Driver driver = GraphDatabase.driver( uri ); + Driver driver = GraphDatabase.driver( uri, INSECURE_CONFIG ); // Then assertThat( driver, is( directDriver() ) ); + server.exit(); } @Test diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/ConnectionHandlingIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/ConnectionHandlingIT.java index cad65bd856..0929c8cc9b 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/ConnectionHandlingIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/ConnectionHandlingIT.java @@ -40,7 +40,6 @@ import org.neo4j.driver.internal.spi.PooledConnection; import org.neo4j.driver.internal.util.Clock; import org.neo4j.driver.v1.AuthToken; -import org.neo4j.driver.v1.AuthTokens; import org.neo4j.driver.v1.Config; import org.neo4j.driver.v1.Driver; import org.neo4j.driver.v1.Logging; @@ -85,6 +84,7 @@ public void createDriver() RetrySettings retrySettings = RetrySettings.DEFAULT; driver = driverFactory.newInstance( neo4j.uri(), auth, routingSettings, retrySettings, defaultConfig() ); connectionPool = driverFactory.connectionPool; + connectionPool.startMemorizing(); // start memorizing connections after driver creation } @After @@ -370,23 +370,34 @@ protected ConnectionPool createConnectionPool( AuthToken authToken, SecurityPlan private static class MemorizingConnectionPool extends SocketConnectionPool { PooledConnection lastAcquiredConnectionSpy; + boolean memorize; MemorizingConnectionPool( PoolSettings poolSettings, Connector connector, Clock clock, Logging logging ) { super( poolSettings, connector, clock, logging ); } + void startMemorizing() + { + memorize = true; + } + @Override public PooledConnection acquire( BoltServerAddress address ) { PooledConnection connection = super.acquire( address ); - // this connection pool returns spies so spies will be returned to the pool - // prevent spying on spies... - if ( !Mockito.mockingDetails( connection ).isSpy() ) + + if ( memorize ) { - connection = spy( connection ); + // this connection pool returns spies so spies will be returned to the pool + // prevent spying on spies... + if ( !Mockito.mockingDetails( connection ).isSpy() ) + { + connection = spy( connection ); + } + lastAcquiredConnectionSpy = connection; } - lastAcquiredConnectionSpy = connection; + return connection; } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/CredentialsIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/CredentialsIT.java index f3bb5590b7..49dbaa04b5 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/CredentialsIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/CredentialsIT.java @@ -26,19 +26,24 @@ import java.util.HashMap; import org.neo4j.driver.internal.security.InternalAuthToken; +import org.neo4j.driver.v1.AuthToken; +import org.neo4j.driver.v1.AuthTokens; +import org.neo4j.driver.v1.Config; import org.neo4j.driver.v1.Driver; import org.neo4j.driver.v1.GraphDatabase; import org.neo4j.driver.v1.Session; import org.neo4j.driver.v1.Value; +import org.neo4j.driver.v1.exceptions.AuthenticationException; import org.neo4j.driver.v1.exceptions.SecurityException; import org.neo4j.driver.v1.util.Neo4jSettings; import org.neo4j.driver.v1.util.TestNeo4j; -import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; +import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; +import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING; import static org.neo4j.driver.v1.AuthTokens.basic; import static org.neo4j.driver.v1.AuthTokens.custom; import static org.neo4j.driver.v1.Values.ofValue; @@ -52,14 +57,34 @@ public class CredentialsIT @ClassRule public static TestNeo4j neo4j = new TestNeo4j(); - private static String password = "secret"; + private static final String PASSWORD = "secret"; + + @BeforeClass + public static void enableAuth() throws Exception + { + neo4j.restart( Neo4jSettings.TEST_SETTINGS + .updateWith( Neo4jSettings.AUTH_ENABLED, "true" ) + .updateWith( Neo4jSettings.DATA_DIR, tempDir.getRoot().getAbsolutePath().replace( "\\", "/" ) ) ); + + InternalAuthToken authToken = new InternalAuthToken( parameters( + "scheme", "basic", + "principal", "neo4j", + "credentials", "neo4j", + "new_credentials", PASSWORD ).asMap( ofValue() ) ); + + try ( Driver driver = GraphDatabase.driver( neo4j.uri(), authToken ); + Session session = driver.session() ) + { + session.run( "RETURN 1" ).consume(); + } + } @Test public void basicCredentialsShouldWork() throws Throwable { // When & Then try( Driver driver = GraphDatabase.driver( neo4j.uri(), - basic("neo4j", password ) ); + basic( "neo4j", PASSWORD ) ); Session session = driver.session() ) { Value single = session.run( "RETURN 1" ).single().get( 0 ); @@ -71,7 +96,7 @@ public void basicCredentialsShouldWork() throws Throwable public void shouldGetHelpfulErrorOnInvalidCredentials() throws Throwable { // When - try ( Driver driver = GraphDatabase.driver( neo4j.uri(), basic( "thisisnotthepassword", password ) ); + try ( Driver driver = GraphDatabase.driver( neo4j.uri(), basic( "thisisnotthepassword", PASSWORD ) ); Session session = driver.session() ) { session.run( "RETURN 1" ); @@ -89,7 +114,7 @@ public void shouldBeAbleToProvideRealmWithBasicAuth() throws Throwable { // When & Then try( Driver driver = GraphDatabase.driver( neo4j.uri(), - basic("neo4j", password, "native") ); + basic( "neo4j", PASSWORD, "native" ) ); Session session = driver.session() ) { Value single = session.run( "CREATE () RETURN 1" ).single().get( 0 ); @@ -102,7 +127,7 @@ public void shouldBeAbleToConnectWithCustomToken() throws Throwable { // When & Then try( Driver driver = GraphDatabase.driver( neo4j.uri(), - custom("neo4j", password, "native", "basic" ) ); + custom( "neo4j", PASSWORD, "native", "basic" ) ); Session session = driver.session() ) { Value single = session.run( "CREATE () RETURN 1" ).single().get( 0 ); @@ -118,7 +143,7 @@ public void shouldBeAbleToConnectWithCustomTokenWithAdditionalParameters() throw // When & Then try( Driver driver = GraphDatabase.driver( neo4j.uri(), - custom("neo4j", password, "native", "basic", parameters ) ); + custom( "neo4j", PASSWORD, "native", "basic", parameters ) ); Session session = driver.session() ) { Value single = session.run( "CREATE () RETURN 1" ).single().get( 0 ); @@ -126,23 +151,31 @@ public void shouldBeAbleToConnectWithCustomTokenWithAdditionalParameters() throw } } - @BeforeClass - public static void enableAuth() throws Exception + @Test + public void directDriverShouldFailEarlyOnWrongCredentials() { - neo4j.restart( Neo4jSettings.TEST_SETTINGS - .updateWith( Neo4jSettings.AUTH_ENABLED, "true" ) - .updateWith( Neo4jSettings.DATA_DIR, tempDir.getRoot().getAbsolutePath().replace("\\", "/") )); - - try ( Driver setPassword = - GraphDatabase.driver( neo4j.uri(), new InternalAuthToken( - parameters( - "scheme", "basic", - "principal", "neo4j", - "credentials", "neo4j", - "new_credentials", password ).asMap( ofValue()) ) ); - Session sess = setPassword.session() ) + testDriverFailureOnWrongCredentials( "bolt://localhost" ); + } + + @Test + public void routingDriverShouldFailEarlyOnWrongCredentials() + { + testDriverFailureOnWrongCredentials( "bolt+routing://localhost" ); + } + + private void testDriverFailureOnWrongCredentials( String uri ) + { + Config config = Config.build().withLogging( DEV_NULL_LOGGING ).toConfig(); + AuthToken authToken = AuthTokens.basic( "neo4j", "wrongSecret" ); + + try + { + GraphDatabase.driver( uri, authToken, config ); + fail( "Exception expected" ); + } + catch ( Exception e ) { - sess.run( "RETURN 1" ).consume(); + assertThat( e, instanceOf( AuthenticationException.class ) ); } } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/ErrorIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/ErrorIT.java index dbcd57db9d..4032a10ab8 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/ErrorIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/ErrorIT.java @@ -25,9 +25,7 @@ import java.util.UUID; import org.neo4j.driver.v1.Config; -import org.neo4j.driver.v1.Driver; import org.neo4j.driver.v1.GraphDatabase; -import org.neo4j.driver.v1.Session; import org.neo4j.driver.v1.StatementResult; import org.neo4j.driver.v1.Transaction; import org.neo4j.driver.v1.exceptions.ClientException; @@ -114,18 +112,11 @@ public void shouldAllowNewTransactionAfterRecoverableError() throws Throwable @Test public void shouldExplainConnectionError() throws Throwable { - // Given - try ( Driver driver = GraphDatabase.driver( "bolt://localhost:7777" ); - Session session = driver.session() ) - { - // Expect - exception.expect( ServiceUnavailableException.class ); - exception.expectMessage( "Unable to connect to localhost:7777, ensure the database is running " + - "and that there is a working network connection to it." ); + exception.expect( ServiceUnavailableException.class ); + exception.expectMessage( "Unable to connect to localhost:7777, ensure the database is running " + + "and that there is a working network connection to it." ); - // When - session.run( "RETURN 1" ); - } + GraphDatabase.driver( "bolt://localhost:7777" ); } @Test @@ -157,22 +148,17 @@ public void shouldHandleFailureAtCommitTime() throws Throwable @Test public void shouldGetHelpfulErrorWhenTryingToConnectToHttpPort() throws Throwable { - // Given //the http server needs some time to start up Thread.sleep( 2000 ); + Config config = Config.build().withoutEncryption().toConfig(); - try ( Driver driver = GraphDatabase.driver( "bolt://localhost:7474", config ); - Session session = driver.session() ) - { - // Expect - exception.expect( ClientException.class ); - exception.expectMessage( - "Server responded HTTP. Make sure you are not trying to connect to the http endpoint " + - "(HTTP defaults to port 7474 whereas BOLT defaults to port 7687)" ); - - // When - session.run( "RETURN 1" ); - } + + exception.expect( ClientException.class ); + exception.expectMessage( + "Server responded HTTP. Make sure you are not trying to connect to the http endpoint " + + "(HTTP defaults to port 7474 whereas BOLT defaults to port 7687)" ); + + GraphDatabase.driver( "bolt://localhost:7474", config ); } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/LoggingIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/LoggingIT.java index 8417623807..d94cc9fa39 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/LoggingIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/LoggingIT.java @@ -47,15 +47,15 @@ public void logShouldRecordDebugAndTraceInfo() throws Exception Logging logging = mock( Logging.class ); Logger logger = mock( Logger.class ); - try( Driver driver = GraphDatabase.driver( neo4j.uri(), neo4j.authToken(), + when( logging.getLog( anyString() ) ).thenReturn( logger ); + when( logger.isDebugEnabled() ).thenReturn( true ); + when( logger.isTraceEnabled() ).thenReturn( true ); + + try ( Driver driver = GraphDatabase.driver( neo4j.uri(), neo4j.authToken(), Config.build().withLogging( logging ).toConfig() ) ) { // When - when( logging.getLog( anyString() ) ).thenReturn( logger ); - when( logger.isDebugEnabled() ).thenReturn( true ); - when( logger.isTraceEnabled() ).thenReturn( true ); - - try( Session session = driver.session() ) + try ( Session session = driver.session() ) { session.run( "CREATE (a {name:'Cat'})" ); } diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/SessionIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/SessionIT.java index 5ae19869d6..8b2aa495e3 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/SessionIT.java +++ b/driver/src/test/java/org/neo4j/driver/v1/integration/SessionIT.java @@ -44,7 +44,6 @@ import org.neo4j.driver.internal.util.ServerVersion; import org.neo4j.driver.v1.AccessMode; import org.neo4j.driver.v1.AuthToken; -import org.neo4j.driver.v1.AuthTokens; import org.neo4j.driver.v1.Config; import org.neo4j.driver.v1.Driver; import org.neo4j.driver.v1.GraphDatabase; @@ -54,6 +53,7 @@ import org.neo4j.driver.v1.StatementRunner; import org.neo4j.driver.v1.Transaction; import org.neo4j.driver.v1.TransactionWork; +import org.neo4j.driver.v1.exceptions.AuthenticationException; import org.neo4j.driver.v1.exceptions.ClientException; import org.neo4j.driver.v1.exceptions.Neo4jException; import org.neo4j.driver.v1.exceptions.ServiceUnavailableException; @@ -116,7 +116,7 @@ public void shouldKnowSessionIsClosed() throws Throwable public void shouldHandleNullConfig() throws Throwable { // Given - try ( Driver driver = GraphDatabase.driver( neo4j.uri(), AuthTokens.none(), null ) ) + try ( Driver driver = GraphDatabase.driver( neo4j.uri(), neo4j.authToken(), null ) ) { Session session = driver.session(); @@ -132,18 +132,13 @@ public void shouldHandleNullConfig() throws Throwable @Test public void shouldHandleNullAuthToken() throws Throwable { - // Given AuthToken token = null; - try ( Driver driver = GraphDatabase.driver( neo4j.uri(), token ) ) - { - Session session = driver.session(); - // When - session.close(); + exception.expect( AuthenticationException.class ); - // Then - assertFalse( session.isOpen() ); - } + // null auth token should be interpreted as AuthTokens.none() and fail driver creation + // because server expects basic auth + GraphDatabase.driver( neo4j.uri(), token ); } @Test diff --git a/driver/src/test/java/org/neo4j/driver/v1/tck/DriverAuthSteps.java b/driver/src/test/java/org/neo4j/driver/v1/tck/DriverAuthSteps.java index eb32269693..cb4a07890c 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/tck/DriverAuthSteps.java +++ b/driver/src/test/java/org/neo4j/driver/v1/tck/DriverAuthSteps.java @@ -35,16 +35,18 @@ import org.neo4j.driver.v1.Session; import org.neo4j.driver.v1.util.Neo4jSettings; -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.CoreMatchers.startsWith; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; import static org.neo4j.driver.v1.Values.ofValue; import static org.neo4j.driver.v1.Values.parameters; import static org.neo4j.driver.v1.tck.DriverComplianceIT.neo4j; public class DriverAuthSteps { - Driver driver = null; - File tempDir = null; + private Driver driver; + private File tempDir; + private Exception driverCreationError; @Before( "@auth" ) public void setUp() throws IOException @@ -92,27 +94,44 @@ public void aDriverIsConfiguredWithAuthEnabledAndTheWrongPasswordIsProvided() th { driver = configureCredentials( "neo4j", "neo4j", "password" ); driver.close(); - driver = GraphDatabase.driver( neo4j.uri(), new InternalAuthToken( - parameters( - "scheme", "basic", - "principal", "neo4j", - "credentials", "wrong" ).asMap( ofValue() ) ) ); + + try + { + driver = GraphDatabase.driver( neo4j.uri(), new InternalAuthToken( + parameters( + "scheme", "basic", + "principal", "neo4j", + "credentials", "wrong" ).asMap( ofValue() ) ) ); + } + catch ( Exception e ) + { + driver = null; + driverCreationError = e; + } } @Then( "^reading and writing to the database should not be possible$" ) public void readingAndWritingToTheDatabaseShouldNotBePossible() throws Throwable { - try(Session session = driver.session()) + Exception error = null; + if ( driver != null ) { - session.run( "CREATE (:label1)" ).consume(); + try ( Session session = driver.session() ) + { + session.run( "CREATE (:label1)" ).consume(); + } + catch ( Exception e ) + { + error = e; + } } - catch ( Exception e ) + else { - assertThat(e.getMessage().startsWith( "The client is unauthorized due to authentication failure" ), - equalTo(true)); - return; + error = driverCreationError; } - throw new RuntimeException( "Exception should have been thrown" ); + + assertNotNull( "Exception should have been thrown", error ); + assertThat( error.getMessage(), startsWith( "The client is unauthorized due to authentication failure" ) ); } @And( "^a `Protocol Error` is raised$" ) diff --git a/driver/src/test/java/org/neo4j/driver/v1/tck/DriverSecurityComplianceSteps.java b/driver/src/test/java/org/neo4j/driver/v1/tck/DriverSecurityComplianceSteps.java index 1f83a6a618..89cc6b5fbc 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/tck/DriverSecurityComplianceSteps.java +++ b/driver/src/test/java/org/neo4j/driver/v1/tck/DriverSecurityComplianceSteps.java @@ -43,6 +43,7 @@ import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.notNullValue; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.neo4j.driver.internal.util.CertificateTool.saveX509Cert; import static org.neo4j.driver.v1.Config.TrustStrategy.trustCustomCertificateSignedBy; @@ -57,7 +58,9 @@ public class DriverSecurityComplianceSteps private Driver driver; private File knownHostsFile; private Throwable exception; + private Config driverKittenConfig; private Driver driverKitten; // well, just a reference to another driver + private File certificate; // first use @Given( "^a running Neo4j Database$" ) @@ -81,6 +84,7 @@ public void firstUseConnect() throws Throwable @Then( "sessions should simply work$" ) public void sessionsShouldSimplyWork() throws Throwable { + assertNull( exception ); try ( Session session = driver.session() ) { StatementResult statementResult = session.run( "RETURN 1" ); @@ -100,11 +104,19 @@ public void aRunningNeoJDatabaseThatIHaveConnectedTo() throws Throwable @When( "^I connect via a TLS-enabled transport again$" ) public void iConnectViaATlsEnabledTransportAgain() throws Throwable { - driver = GraphDatabase.driver( - Neo4jRunner.DEFAULT_URI, - Neo4jRunner.DEFAULT_AUTH_TOKEN, - Config.build().withEncryptionLevel( EncryptionLevel.REQUIRED ) - .withTrustStrategy( trustOnFirstUse( knownHostsFile ) ).toConfig() ); + try + { + driver = GraphDatabase.driver( + Neo4jRunner.DEFAULT_URI, + Neo4jRunner.DEFAULT_AUTH_TOKEN, + Config.build().withEncryptionLevel( EncryptionLevel.REQUIRED ) + .withTrustStrategy( trustOnFirstUse( knownHostsFile ) ).toConfig() ); + } + catch ( Exception e ) + { + driver = null; + exception = e; + } } // man in the middle attack @@ -128,13 +140,16 @@ public void theDatabaseHasChangedWhichCertificateItUses() throws Throwable @Then( "^creating sessions should fail$" ) public void creatingSessionsShouldFail() throws Throwable { - try ( Session session = driver.session() ) + if ( driver != null ) { - session.run( "RETURN 1" ); - } - catch ( Exception e ) - { - exception = e; + try ( Session session = driver.session() ) + { + session.run( "RETURN 1" ); + } + catch ( Exception e ) + { + exception = e; + } } } @@ -169,11 +184,10 @@ public void twoDriversWithDifferentKnownHostsFiles() throws Throwable sessionsShouldSimplyWork(); File tempFile = tempFile( "known_hosts", ".tmp" ); - driverKitten = GraphDatabase.driver( - Neo4jRunner.DEFAULT_URI, - Neo4jRunner.DEFAULT_AUTH_TOKEN, - Config.build().withEncryptionLevel( EncryptionLevel.REQUIRED ) - .withTrustStrategy( trustOnFirstUse( tempFile ) ).toConfig() ); + driverKittenConfig = Config.build() + .withEncryptionLevel( EncryptionLevel.REQUIRED ) + .withTrustStrategy( trustOnFirstUse( tempFile ) ) + .toConfig(); } @Then( "^the two drivers should not interfere with one another's known hosts files$" ) @@ -186,6 +200,9 @@ public void twoDriversShouldNotInterfereWithEachOther() throws Throwable iShouldGetAHelpfulErrorExplainingThatCertificateChanged( "nah" ); // However as driverKitten has not connected to the server, so driverKitten should just simply connect! + driverKitten = GraphDatabase.driver( Neo4jRunner.DEFAULT_URI, Neo4jRunner.DEFAULT_AUTH_TOKEN, + driverKittenConfig ); + try ( Session session = driverKitten.session() ) { StatementResult statementResult = session.run( "RETURN 1" ); @@ -203,20 +220,13 @@ public void aDriverConfiguredToUseATrustedCertificate() throws Throwable public void aRunningNeo4jDatabaseUsingACertificateSignedByTheSameTrustedCertificate() throws Throwable { // create new root certificate - File rootCert = tempFile( "temp_root_cert", ".cert" ); + certificate = tempFile( "temp_root_cert", ".cert" ); File rootKey = tempFile( "temp_root_key", ".key" ); SelfSignedCertificateGenerator certGenerator = new SelfSignedCertificateGenerator(); - certGenerator.saveSelfSignedCertificate( rootCert ); + certGenerator.saveSelfSignedCertificate( certificate ); certGenerator.savePrivateKey( rootKey ); - // give root certificate to driver - driver = GraphDatabase.driver( - Neo4jRunner.DEFAULT_URI, - Neo4jRunner.DEFAULT_AUTH_TOKEN, - Config.build().withEncryption() - .withTrustStrategy( trustCustomCertificateSignedBy( rootCert ) ).toConfig() ); - // generate certificate signing request and get a certificate signed by the root private key File cert = tempFile( "temp_cert", ".cert" ); File key = tempFile( "temp_key", ".key" ); @@ -232,34 +242,35 @@ public void aRunningNeo4jDatabaseUsingACertificateSignedByTheSameTrustedCertific @When( "^I connect via a TLS-enabled transport$" ) public void iConnectViaATlsEnabledTransport() { + try + { + // give root certificate to driver + driver = GraphDatabase.driver( + Neo4jRunner.DEFAULT_URI, + Neo4jRunner.DEFAULT_AUTH_TOKEN, + Config.build().withEncryption() + .withTrustStrategy( trustCustomCertificateSignedBy( certificate ) ).toConfig() ); + } + catch ( Exception e ) + { + driver = null; + exception = e; + } } // same certificate @And( "^a running Neo4j Database using that exact trusted certificate$" ) public void aRunningNeo4jDatabaseUsingThatExactTrustedCertificate() { - driver = GraphDatabase.driver( - Neo4jRunner.DEFAULT_URI, - Neo4jRunner.DEFAULT_AUTH_TOKEN, - Config.build().withEncryption() - .withTrustStrategy( trustCustomCertificateSignedBy( - new File( HOME_DIR, DEFAULT_TLS_CERT_PATH ) ) ) - .toConfig() ); + certificate = new File( HOME_DIR, DEFAULT_TLS_CERT_PATH ); } // invalid cert @And( "^a running Neo4j Database using a certificate not signed by the trusted certificate$" ) public void aRunningNeo4jDatabaseUsingACertNotSignedByTheTrustedCertificates() throws Throwable { - File cert = tempFile( "temp_cert", ".cert" ); - saveX509Cert( generateSelfSignedCertificate(), cert ); - - // give root certificate to driver - driver = GraphDatabase.driver( - Neo4jRunner.DEFAULT_URI, - Neo4jRunner.DEFAULT_AUTH_TOKEN, - Config.build().withEncryption() - .withTrustStrategy( trustCustomCertificateSignedBy( cert ) ).toConfig() ); + certificate = tempFile( "temp_cert", ".cert" ); + saveX509Cert( generateSelfSignedCertificate(), certificate ); } @And( "^I should get a helpful error explaining that no trusted certificate found$" ) @@ -274,7 +285,10 @@ public void iShouldGetAHelpfulErrorExplainingThatCertificatedNotSigned() throws @After( "@tls" ) public void clearAfterEachScenario() throws Throwable { - driver.close(); + if ( driver != null ) + { + driver.close(); + } driver = null; knownHostsFile = null; diff --git a/driver/src/test/java/org/neo4j/driver/v1/util/StubServer.java b/driver/src/test/java/org/neo4j/driver/v1/util/StubServer.java index 17d3ee0fb6..cae3e4aa30 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/util/StubServer.java +++ b/driver/src/test/java/org/neo4j/driver/v1/util/StubServer.java @@ -74,12 +74,17 @@ public int exitStatus() throws InterruptedException, ForceKilled catch ( IllegalThreadStateException ex ) { // not exited yet - process.destroy(); - process.waitFor(); + exit(); throw new ForceKilled(); } } + public void exit() throws InterruptedException + { + process.destroy(); + process.waitFor(); + } + private static String resource( String fileName ) { File resource = new File( TestNeo4j.TEST_RESOURCE_FOLDER_PATH, fileName ); diff --git a/driver/src/test/resources/dummy_connection.script b/driver/src/test/resources/dummy_connection.script new file mode 100644 index 0000000000..8cb5e929e3 --- /dev/null +++ b/driver/src/test/resources/dummy_connection.script @@ -0,0 +1,2 @@ +!: AUTO INIT +!: AUTO RESET diff --git a/examples/src/main/java/org/neo4j/docs/driver/ConfigTrustExample.java b/examples/src/main/java/org/neo4j/docs/driver/ConfigTrustExample.java index 4306b650ce..44531432ec 100644 --- a/examples/src/main/java/org/neo4j/docs/driver/ConfigTrustExample.java +++ b/examples/src/main/java/org/neo4j/docs/driver/ConfigTrustExample.java @@ -34,7 +34,7 @@ public class ConfigTrustExample implements AutoCloseable public ConfigTrustExample( String uri, String user, String password ) { driver = GraphDatabase.driver( uri, AuthTokens.basic( user, password ), - Config.build().withTrustStrategy( Config.TrustStrategy.trustSystemCertificates() ).toConfig() ); + Config.build().withTrustStrategy( Config.TrustStrategy.trustAllCertificates() ).toConfig() ); } // end::config-trust[]