Skip to content

Commit db06302

Browse files
author
Zhen Li
authored
Merge pull request #315 from zhenlineo/1.1-auth-error
Throw UnauthorizedException if failed to conn due to auth errors
2 parents 5d0403a + 19b45fd commit db06302

File tree

10 files changed

+123
-32
lines changed

10 files changed

+123
-32
lines changed

‎driver/src/main/java/org/neo4j/driver/internal/cluster/Rediscovery.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.neo4j.driver.internal.util.Clock;
2525
import org.neo4j.driver.v1.Logger;
2626
import org.neo4j.driver.v1.exceptions.ServiceUnavailableException;
27+
import org.neo4j.driver.v1.exceptions.SecurityException;
2728

2829
import static java.lang.String.format;
2930

@@ -75,7 +76,11 @@ public ClusterComposition lookupRoutingTable( ConnectionPool connections, Routin
7576
{
7677
response = provider.getClusterComposition( connection );
7778
}
78-
catch ( Throwable e )
79+
catch( SecurityException e )
80+
{
81+
throw e; // terminate the discovery immediately
82+
}
83+
catch ( Exception e )
7984
{
8085
// the connection breaks
8186
logger.error( format( "Failed to connect to routing server '%s'.", address ), e );

‎driver/src/main/java/org/neo4j/driver/internal/net/SocketResponseHandler.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Queue;
2323
import java.util.concurrent.ConcurrentLinkedQueue;
2424

25+
import org.neo4j.driver.v1.exceptions.AuthenticationException;
2526
import org.neo4j.driver.internal.messaging.MessageHandler;
2627
import org.neo4j.driver.internal.spi.Collector;
2728
import org.neo4j.driver.internal.summary.InternalNotification;
@@ -65,7 +66,14 @@ public void handleFailureMessage( String code, String message )
6566
switch ( classification )
6667
{
6768
case "ClientError":
68-
error = new ClientException( code, message );
69+
if( code.equalsIgnoreCase( "Neo.ClientError.Security.Unauthorized" ) )
70+
{
71+
error = new AuthenticationException( code, message );
72+
}
73+
else
74+
{
75+
error = new ClientException( code, message );
76+
}
6977
break;
7078
case "TransientError":
7179
error = new TransientException( code, message );

‎driver/src/main/java/org/neo4j/driver/internal/security/TLSSocketChannel.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.neo4j.driver.internal.util.BytePrinter;
3333
import org.neo4j.driver.v1.exceptions.ClientException;
3434
import org.neo4j.driver.v1.exceptions.ServiceUnavailableException;
35+
import org.neo4j.driver.v1.exceptions.SecurityException;
3536

3637
import static java.lang.String.format;
3738
import static javax.net.ssl.SSLEngineResult.HandshakeStatus.FINISHED;
@@ -81,7 +82,7 @@ public static TLSSocketChannel create( ByteChannel channel, Logger logger, SSLEn
8182
}
8283
catch ( SSLHandshakeException e )
8384
{
84-
throw new ClientException( "Failed to establish secured connection with the server: " + e.getMessage(), e );
85+
throw new SecurityException( "Failed to establish secured connection with the server: " + e.getMessage(), e );
8586
}
8687
return tlsChannel;
8788
}

‎driver/src/main/java/org/neo4j/driver/v1/exceptions/AuthenticationException.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Copyright (c) 2002-2017 "Neo Technology,"
3+
* Network Engine for Objects in Lund AB [http://neotechnology.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
package org.neo4j.driver.v1.exceptions;
20+
21+
/**
22+
* Failed to authenticate the driver to the server due to bad credentials provided.
23+
* When this error happens, the error could be recovered by closing the current driver and restart a new driver with
24+
* the correct credentials.
25+
*
26+
* @since 1.1
27+
*/
28+
public class AuthenticationException extends SecurityException
29+
{
30+
public AuthenticationException( String code, String message )
31+
{
32+
super( code, message );
33+
}
34+
}

‎driver/src/main/java/org/neo4j/driver/v1/exceptions/SecurityException.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright (c) 2002-2017 "Neo Technology,"
3+
* Network Engine for Objects in Lund AB [http://neotechnology.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
20+
package org.neo4j.driver.v1.exceptions;
21+
22+
/**
23+
* Failed to communicate with the server due to security errors.
24+
* When this type of error happens, the security cause of the error should be fixed to ensure the safety of your data.
25+
* Restart of server/driver/cluster might be required to recover from this error.
26+
* @since 1.1
27+
*/
28+
public class SecurityException extends Neo4jException
29+
{
30+
public SecurityException( String code, String message )
31+
{
32+
super( code, message );
33+
}
34+
35+
public SecurityException( String message, Throwable t )
36+
{
37+
super( message, t );
38+
}
39+
}

‎driver/src/test/java/org/neo4j/driver/internal/security/TLSSocketChannelTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@
2626
import javax.net.ssl.SSLHandshakeException;
2727
import javax.net.ssl.SSLSession;
2828

29-
import org.neo4j.driver.v1.exceptions.ClientException;
3029
import org.neo4j.driver.v1.exceptions.ServiceUnavailableException;
30+
import org.neo4j.driver.v1.exceptions.SecurityException;
3131

3232
import static junit.framework.TestCase.fail;
3333
import static org.hamcrest.MatcherAssert.assertThat;
@@ -107,7 +107,7 @@ public void shouldCloseConnectionIfFailedToWrite() throws Throwable
107107
}
108108

109109
@Test
110-
public void shouldThrowClientErrorIfFailedToHandshake() throws Throwable
110+
public void shouldThrowUnauthorizedIfFailedToHandshake() throws Throwable
111111
{
112112
// Given
113113
ByteChannel mockedChannel = mock( ByteChannel.class );
@@ -128,7 +128,7 @@ public void shouldThrowClientErrorIfFailedToHandshake() throws Throwable
128128
}
129129
catch( Exception e )
130130
{
131-
assertThat( e, instanceOf( ClientException.class ) );
131+
assertThat( e, instanceOf( SecurityException.class ) );
132132
assertThat( e.getMessage(), startsWith( "Failed to establish secured connection with the server: Failed handshake!" ) );
133133
}
134134
verify( mockedChannel, never() ).close();

‎driver/src/test/java/org/neo4j/driver/v1/integration/CredentialsIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
import org.neo4j.driver.v1.GraphDatabase;
3131
import org.neo4j.driver.v1.Session;
3232
import org.neo4j.driver.v1.Value;
33-
import org.neo4j.driver.v1.exceptions.ClientException;
33+
import org.neo4j.driver.v1.exceptions.SecurityException;
3434
import org.neo4j.driver.v1.util.Neo4jSettings;
3535
import org.neo4j.driver.v1.util.TestNeo4j;
3636

@@ -79,7 +79,7 @@ public void shouldGetHelpfulErrorOnInvalidCredentials() throws Throwable
7979
}
8080
catch( Throwable e )
8181
{
82-
assertThat( e, instanceOf( ClientException.class ) );
82+
assertThat( e, instanceOf( SecurityException.class ) );
8383
assertThat( e.getMessage(), containsString( "The client is unauthorized due to authentication failure." ) );
8484
}
8585
}

‎driver/src/test/java/org/neo4j/driver/v1/integration/TLSSocketChannelIT.java

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import java.net.InetSocketAddress;
3131
import java.nio.channels.SocketChannel;
3232
import java.security.cert.X509Certificate;
33-
import javax.net.ssl.SSLHandshakeException;
3433

3534
import org.neo4j.driver.internal.net.BoltServerAddress;
3635
import org.neo4j.driver.internal.security.SecurityPlan;
@@ -43,7 +42,7 @@
4342
import org.neo4j.driver.v1.Logging;
4443
import org.neo4j.driver.v1.Session;
4544
import org.neo4j.driver.v1.StatementResult;
46-
import org.neo4j.driver.v1.exceptions.ClientException;
45+
import org.neo4j.driver.v1.exceptions.SecurityException;
4746
import org.neo4j.driver.v1.util.CertificateToolTest;
4847
import org.neo4j.driver.v1.util.Neo4jRunner;
4948
import org.neo4j.driver.v1.util.Neo4jSettings;
@@ -53,6 +52,7 @@
5352
import static org.hamcrest.Matchers.containsString;
5453
import static org.junit.Assert.assertEquals;
5554
import static org.junit.Assert.assertFalse;
55+
import static org.junit.Assert.fail;
5656
import static org.mockito.Matchers.anyString;
5757
import static org.mockito.Mockito.atLeastOnce;
5858
import static org.mockito.Mockito.mock;
@@ -150,18 +150,25 @@ public void shouldNotPerformTLSHandshakeWithNonSystemCert() throws Throwable
150150
SecurityPlan securityPlan = SecurityPlan.forSystemCASignedCertificates();
151151

152152
// When
153+
TLSSocketChannel sslChannel = null;
153154
try
154155
{
155-
TLSSocketChannel sslChannel =
156-
TLSSocketChannel.create(address, securityPlan, channel, logger);
157-
sslChannel.close();
156+
sslChannel = TLSSocketChannel.create(address, securityPlan, channel, logger);
157+
fail( "Should have thrown exception" );
158158
}
159-
catch ( ClientException e )
159+
catch ( SecurityException e )
160160
{
161161
assertThat( e.getMessage(), containsString( "General SSLEngine problem" ) );
162162
assertThat( getRootCause( e ).getMessage(),
163163
containsString( "unable to find valid certification path to requested target" ) );
164164
}
165+
finally
166+
{
167+
if( sslChannel != null )
168+
{
169+
sslChannel.close();
170+
}
171+
}
165172
}
166173
finally
167174
{
@@ -188,12 +195,12 @@ public void shouldFailTLSHandshakeDueToWrongCertInKnownCertsFile() throws Throwa
188195
TLSSocketChannel sslChannel = null;
189196
try
190197
{
191-
sslChannel = TLSSocketChannel.create( address, securityPlan, channel, mock( Logger.class ) );
192-
sslChannel.close();
198+
sslChannel = TLSSocketChannel.create( address, securityPlan, channel, DEV_NULL_LOGGER );
199+
fail( "Should have thrown exception" );
193200
}
194-
catch ( SSLHandshakeException e )
201+
catch ( SecurityException e )
195202
{
196-
assertEquals( "General SSLEngine problem", e.getMessage() );
203+
assertThat( e.getMessage(), containsString( "General SSLEngine problem" ) );
197204
assertThat( getRootCause( e ).getMessage(), containsString(
198205
"If you trust the certificate the server uses now, simply remove the line that starts with" ) );
199206
}
@@ -209,14 +216,13 @@ public void shouldFailTLSHandshakeDueToWrongCertInKnownCertsFile() throws Throwa
209216
private void createFakeServerCertPairInKnownCerts( BoltServerAddress address, File knownCerts )
210217
throws Throwable
211218
{
212-
address = address.resolve(); // localhost -> 127.0.0.1
213219
String serverId = address.toString();
214220

215221
X509Certificate cert = CertificateToolTest.generateSelfSignedCertificate();
216222
String certStr = fingerprint(cert);
217223

218224
BufferedWriter writer = new BufferedWriter( new FileWriter( knownCerts, true ) );
219-
writer.write( serverId + "," + certStr );
225+
writer.write( serverId + " " + certStr );
220226
writer.newLine();
221227
writer.close();
222228
}
@@ -241,9 +247,9 @@ public void shouldFailTLSHandshakeDueToServerCertNotSignedByKnownCA() throws Thr
241247
try
242248
{
243249
sslChannel = TLSSocketChannel.create( neo4j.address(), securityPlan, channel, mock( Logger.class ) );
244-
sslChannel.close();
250+
fail( "Should have thrown exception" );
245251
}
246-
catch ( ClientException e )
252+
catch ( SecurityException e )
247253
{
248254
assertThat( e.getMessage(), containsString( "General SSLEngine problem" ) );
249255
assertThat( getRootCause( e ).getMessage(), containsString( "No trusted certificate found" ) );

‎driver/src/test/java/org/neo4j/driver/v1/stress/CausalClusteringStressIT.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
import org.neo4j.driver.v1.StatementResult;
5252
import org.neo4j.driver.v1.Transaction;
5353
import org.neo4j.driver.v1.exceptions.ClientException;
54-
import org.neo4j.driver.v1.exceptions.ServiceUnavailableException;
54+
import org.neo4j.driver.v1.exceptions.SecurityException;
5555
import org.neo4j.driver.v1.types.Node;
5656
import org.neo4j.driver.v1.util.DaemonThreadFactory;
5757
import org.neo4j.driver.v1.util.cc.LocalOrRemoteClusterRule;
@@ -411,14 +411,12 @@ public void execute()
411411
}
412412
catch ( Exception e )
413413
{
414-
assertThat( e, instanceOf( ServiceUnavailableException.class ) );
414+
assertThat( e, instanceOf( SecurityException.class ) );
415+
assertThat( e.getMessage(), containsString( "authentication failure" ) );
415416

416417
ArgumentCaptor<Throwable> captor = ArgumentCaptor.forClass( Throwable.class );
417-
verify( logger ).error( startsWith( "Failed to connect to routing server" ), captor.capture() );
418-
419-
Throwable loggedThrowable = captor.getValue();
420-
assertThat( loggedThrowable, instanceOf( ClientException.class ) );
421-
assertThat( loggedThrowable.getMessage().toLowerCase(), containsString( "authentication failure" ) );
418+
verify( logger ).debug( startsWith( "~~ [CLOSED SECURE CHANNEL]" ), captor.capture() );
419+
verify( logger ).debug( startsWith( "~~ [DISCONNECT]" ), captor.capture() );
422420
}
423421
}
424422
}

‎driver/src/test/java/org/neo4j/driver/v1/tck/DriverSecurityComplianceSteps.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
import org.neo4j.driver.v1.GraphDatabase;
3434
import org.neo4j.driver.v1.Session;
3535
import org.neo4j.driver.v1.StatementResult;
36-
import org.neo4j.driver.v1.exceptions.ClientException;
36+
import org.neo4j.driver.v1.exceptions.SecurityException;
3737
import org.neo4j.driver.v1.util.CertificateToolTest.CertificateSigningRequestGenerator;
3838
import org.neo4j.driver.v1.util.CertificateToolTest.SelfSignedCertificateGenerator;
3939
import org.neo4j.driver.v1.util.Neo4jRunner;
@@ -134,7 +134,7 @@ public void creatingSessionsShouldFail() throws Throwable
134134
public void iShouldGetAHelpfulErrorExplainingThatCertificateChanged( String str ) throws Throwable
135135
{
136136
assertThat( exception, notNullValue() );
137-
assertThat( exception, instanceOf( ClientException.class ) );
137+
assertThat( exception, instanceOf( SecurityException.class ) );
138138
Throwable rootCause = getRootCause( exception );
139139
assertThat( rootCause.toString(), containsString(
140140
"Unable to connect to neo4j at `localhost:7687`, because the certificate the server uses has changed. " +
@@ -244,7 +244,7 @@ public void aRunningNeo4jDatabaseUsingACertNotSignedByTheTrustedCertificates() t
244244
public void iShouldGetAHelpfulErrorExplainingThatCertificatedNotSigned() throws Throwable
245245
{
246246
assertThat( exception, notNullValue() );
247-
assertThat( exception, instanceOf( ClientException.class ) );
247+
assertThat( exception, instanceOf( SecurityException.class ) );
248248
Throwable rootCause = getRootCause( exception );
249249
assertThat( rootCause.toString(), containsString( "Signature does not match.") );
250250
}

0 commit comments

Comments
 (0)