Removed identifier from auth failure output

The unique identifier was meant as a mechanism of handling multiple instances of neo4j. However it is not used as such and only confuses users.
neo4j · Jul 1, 2016 · 07b4332 · 07b4332
1 parent c7f639a
commit 07b4332
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 111 deletions.
diff --git a/community/bolt/src/main/java/org/neo4j/bolt/security/auth/AuthUtils.java b/community/bolt/src/main/java/org/neo4j/bolt/security/auth/AuthUtils.java
diff --git a/community/bolt/src/main/java/org/neo4j/bolt/security/auth/AuthenticationException.java b/community/bolt/src/main/java/org/neo4j/bolt/security/auth/AuthenticationException.java
@@ -27,19 +27,19 @@ public class AuthenticationException extends IOException implements Status.HasSt
 {
     private final Status status;
 
-    public AuthenticationException( Status status, String identifier )
+    public AuthenticationException( Status status )
     {
-        this(status, identifier, status.code().description(), null);
+        this( status, status.code().description(), null );
     }
 
-    public AuthenticationException( Status status, String identifier, String message )
+    public AuthenticationException( Status status, String message )
     {
-        this(status, identifier, message, null);
+        this( status, message, null );
     }
 
-    public AuthenticationException( Status status, String identifier, String message, Throwable e )
+    public AuthenticationException( Status status, String message, Throwable e )
     {
-        super(message + " (ID:" + identifier + ")"  , e);
+        super( message, e );
         this.status = status;
     }
 

diff --git a/community/bolt/src/main/java/org/neo4j/bolt/security/auth/BasicAuthentication.java b/community/bolt/src/main/java/org/neo4j/bolt/security/auth/BasicAuthentication.java
@@ -21,7 +21,6 @@
 
 import java.io.IOException;
 import java.util.Map;
-import java.util.function.Supplier;
 
 import org.neo4j.graphdb.security.AuthorizationViolationException;
 import org.neo4j.kernel.api.exceptions.Status;
@@ -39,23 +38,20 @@ public class BasicAuthentication implements Authentication
     private final BasicAuthManager authManager;
     private final static String SCHEME = "basic";
     private final Log log;
-    private final Supplier<String> identifier;
     private AuthSubject authSubject;
 
-
+    public BasicAuthentication( BasicAuthManager authManager, LogProvider logProvider)
-    public BasicAuthentication( BasicAuthManager authManager, LogProvider logProvider, Supplier<String> identifier )
     {
         this.authManager = authManager;
         this.log = logProvider.getLog( getClass() );
-        this.identifier = identifier;
     }
 
     @Override
     public AuthenticationResult authenticate( Map<String,Object> authToken ) throws AuthenticationException
     {
         if ( !SCHEME.equals( authToken.get( SCHEME_KEY ) ) )
         {
-            throw new AuthenticationException( Status.Security.Unauthorized, identifier.get(),
+            throw new AuthenticationException( Status.Security.Unauthorized,
                     "Authentication token must contain: '" + SCHEME_KEY + " : " + SCHEME + "'" );
         }
 
@@ -83,10 +79,10 @@ private AuthenticationResult authenticate( String user, String password ) throws
             credentialsExpired = true;
             break;
         case TOO_MANY_ATTEMPTS:
-            throw new AuthenticationException( Status.Security.AuthenticationRateLimit, identifier.get() );
+            throw new AuthenticationException( Status.Security.AuthenticationRateLimit);
         default:
             log.warn( "Failed authentication attempt for '%s'", user);
-            throw new AuthenticationException( Status.Security.Unauthorized, identifier.get() );
+            throw new AuthenticationException( Status.Security.Unauthorized);
         }
         return new BasicAuthenticationResult( authSubject, credentialsExpired );
     }
@@ -106,19 +102,19 @@ private AuthenticationResult update( String user, String password, String newPas
             }
             catch ( AuthorizationViolationException e )
             {
-                throw new AuthenticationException( Status.Security.Forbidden, identifier.get(), e.getMessage(), e );
+                throw new AuthenticationException( Status.Security.Forbidden, e.getMessage(), e );
             }
             catch ( IOException e )
             {
-                throw new AuthenticationException( Status.Security.Unauthorized, identifier.get(), e.getMessage(), e );
+                throw new AuthenticationException( Status.Security.Unauthorized, e.getMessage(), e );
             }
             catch ( IllegalCredentialsException e )
             {
-                throw new AuthenticationException(e.status(), identifier.get(), e.getMessage(), e );
+                throw new AuthenticationException(e.status(), e.getMessage(), e );
             }
             break;
         default:
-            throw new AuthenticationException( Status.Security.Unauthorized, identifier.get() );
+            throw new AuthenticationException( Status.Security.Unauthorized );
         }
         return new BasicAuthenticationResult( authSubject, false );
     }
@@ -128,7 +124,7 @@ private String safeCast( String key, Map<String,Object> authToken ) throws Authe
         Object value = authToken.get( key );
         if ( value == null || !(value instanceof String) )
         {
-            throw new AuthenticationException( Status.Security.Unauthorized, identifier.get(),
+            throw new AuthenticationException( Status.Security.Unauthorized,
                     "The value associated with the key `" + key + "` must be a String but was: " +
                     (value == null ? "null" : value.getClass().getSimpleName()));
         }

diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/internal/StandardSessions.java b/community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/internal/StandardSessions.java
@@ -19,22 +19,17 @@
  */
 package org.neo4j.bolt.v1.runtime.internal;
 
-import java.util.function.Supplier;
-
-import org.neo4j.bolt.security.auth.AuthUtils;
 import org.neo4j.bolt.security.auth.Authentication;
 import org.neo4j.bolt.security.auth.BasicAuthentication;
 import org.neo4j.bolt.v1.runtime.Session;
 import org.neo4j.bolt.v1.runtime.Sessions;
 import org.neo4j.graphdb.DependencyResolver;
 import org.neo4j.graphdb.factory.GraphDatabaseSettings;
-import org.neo4j.kernel.NeoStoreDataSource;
 import org.neo4j.kernel.configuration.Config;
 import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge;
 import org.neo4j.kernel.impl.factory.GraphDatabaseFacade;
 import org.neo4j.kernel.impl.logging.LogService;
 import org.neo4j.kernel.impl.query.QueryExecutionEngine;
-import org.neo4j.kernel.impl.store.StoreId;
 import org.neo4j.kernel.lifecycle.LifeSupport;
 import org.neo4j.kernel.lifecycle.LifecycleAdapter;
 import org.neo4j.server.security.auth.BasicAuthManager;
@@ -106,11 +101,7 @@ private Authentication authentication( DependencyResolver dependencyResolver )
 
         if ( config.get( GraphDatabaseSettings.auth_enabled ) )
         {
-            Supplier<String> identifier = () -> {
+            return new BasicAuthentication( dependencyResolver.resolveDependency( BasicAuthManager.class ), logging.getUserLogProvider());
-                StoreId storeId = dependencyResolver.resolveDependency( NeoStoreDataSource.class ).getStoreId();
-                return AuthUtils.uniqueIdentifier( storeId );
-            };
-            return new BasicAuthentication( dependencyResolver.resolveDependency( BasicAuthManager.class ), logging.getUserLogProvider(), identifier );
         }
         else
         {

diff --git a/community/bolt/src/test/java/org/neo4j/bolt/security/auth/BasicAuthenticationTest.java b/community/bolt/src/test/java/org/neo4j/bolt/security/auth/BasicAuthenticationTest.java
@@ -26,8 +26,6 @@
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
-import java.util.function.Supplier;
-
 import org.neo4j.kernel.api.exceptions.Status;
 import org.neo4j.logging.Log;
 import org.neo4j.logging.LogProvider;
@@ -47,15 +45,14 @@ public class BasicAuthenticationTest
     @Rule
     public ExpectedException exception = ExpectedException.none();
 
-    private final Supplier<String> identifier = () -> "UNIQUE";
 
     @Test
     public void shouldNotDoAnythingOnSuccess() throws AuthenticationException
     {
         // Given
         BasicAuthManager manager = mock( BasicAuthManager.class );
         BasicAuthSubject authSubject = mock( BasicAuthSubject.class );
-        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ), identifier  );
+        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class )  );
         when( manager.login( anyString(), anyString() ) ).thenReturn( authSubject );
         when( authSubject.getAuthenticationResult() ).thenReturn( AuthenticationResult.SUCCESS );
 
@@ -74,7 +71,7 @@ public void shouldThrowAndLogOnFailure() throws AuthenticationException
         Log log = mock( Log.class );
         LogProvider logProvider = mock( LogProvider.class );
         when( logProvider.getLog( BasicAuthentication.class ) ).thenReturn( log );
-        BasicAuthentication authentication = new BasicAuthentication( manager, logProvider, identifier );
+        BasicAuthentication authentication = new BasicAuthentication( manager, logProvider );
         when( manager.login( anyString(), anyString() ) ).thenReturn( authSubject );
         when( authSubject.getAuthenticationResult() ).thenReturn( AuthenticationResult.FAILURE );
 
@@ -96,7 +93,7 @@ public void shouldIndicateThatCredentialsExpired() throws AuthenticationExceptio
         // Given
         BasicAuthManager manager = mock( BasicAuthManager.class );
         BasicAuthSubject authSubject = mock( BasicAuthSubject.class );
-        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ), identifier );
+        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
         when( manager.login( anyString(), anyString() ) ).thenReturn( authSubject );
         when( authSubject.getAuthenticationResult() ).thenReturn( AuthenticationResult.PASSWORD_CHANGE_REQUIRED );
 
@@ -116,7 +113,7 @@ public void shouldFailWhenTooManyAttempts() throws AuthenticationException
         // Given
         BasicAuthManager manager = mock( BasicAuthManager.class );
         BasicAuthSubject authSubject = mock( BasicAuthSubject.class );
-        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ), identifier );
+        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
         when( manager.login( anyString(), anyString() ) ).thenReturn( authSubject );
         when( authSubject.getAuthenticationResult() ).thenReturn( AuthenticationResult.TOO_MANY_ATTEMPTS );
 
@@ -135,7 +132,7 @@ public void shouldBeAbleToUpdateCredentials() throws AuthenticationException
         // Given
         BasicAuthManager manager = mock( BasicAuthManager.class );
         BasicAuthSubject authSubject = mock( BasicAuthSubject.class );
-        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ), identifier );
+        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
         when( manager.login( anyString(), anyString() ) ).thenReturn( authSubject );
         when( authSubject.getAuthenticationResult() ).thenReturn( AuthenticationResult.SUCCESS );
 
@@ -152,7 +149,7 @@ public void shouldBeAbleToUpdateExpiredCredentials() throws AuthenticationExcept
         // Given
         BasicAuthManager manager = mock( BasicAuthManager.class );
         BasicAuthSubject authSubject = mock( BasicAuthSubject.class );
-        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ), identifier );
+        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
         when( manager.login( anyString(), anyString() ) ).thenReturn( authSubject );
         when( authSubject.getAuthenticationResult() ).thenReturn( AuthenticationResult.PASSWORD_CHANGE_REQUIRED );
 
@@ -169,7 +166,7 @@ public void shouldNotBeAbleToUpdateCredentialsIfOldCredentialsAreInvalid() throw
         // Given
         BasicAuthManager manager = mock( BasicAuthManager.class );
         BasicAuthSubject authSubject = mock( BasicAuthSubject.class );
-        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ), identifier );
+        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
         when( manager.login( anyString(), anyString() ) ).thenReturn( authSubject );
         when( authSubject.getAuthenticationResult() ).thenReturn( AuthenticationResult.FAILURE );
 
@@ -190,7 +187,7 @@ public void shouldFailOnUnknownScheme() throws AuthenticationException
         // Given
         BasicAuthManager manager = mock( BasicAuthManager.class );
         BasicAuthSubject authSubject = mock( BasicAuthSubject.class );
-        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ), identifier );
+        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
         when( manager.login( anyString(), anyString() ) ).thenReturn( authSubject );
         when( authSubject.getAuthenticationResult() ).thenReturn( AuthenticationResult.SUCCESS );
 
@@ -209,7 +206,7 @@ public void shouldFailOnMalformedToken() throws AuthenticationException
         // Given
         BasicAuthManager manager = mock( BasicAuthManager.class );
         BasicAuthSubject authSubject = mock( BasicAuthSubject.class );
-        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ), identifier );
+        BasicAuthentication authentication = new BasicAuthentication( manager, mock( LogProvider.class ) );
         when( manager.login( anyString(), anyString() ) ).thenReturn( authSubject );
         when( authSubject.getAuthenticationResult() ).thenReturn( AuthenticationResult.SUCCESS );
 

diff --git a/community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/AuthenticationIT.java b/community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/AuthenticationIT.java
@@ -117,7 +117,7 @@ public void shouldFailIfWrongCredentials() throws Throwable
         // Then
         assertThat( client, eventuallyRecieves( new byte[]{0, 0, 0, 1} ) );
         assertThat( client, eventuallyRecieves( msgFailure( Status.Security.Unauthorized,
-                String.format( "The client is unauthorized due to authentication failure. (ID:%s)", server.uniqueIdentier()) ) ) );
+                "The client is unauthorized due to authentication failure." ) ) );
     }
 
     @Test
@@ -143,7 +143,7 @@ public void shouldBeAbleToUpdateCredentials() throws Throwable
                                 map( "principal", "neo4j", "credentials", "neo4j", "scheme", "basic" ) ) ) );
         assertThat( client, eventuallyRecieves( new byte[]{0, 0, 0, 1} ) );
         assertThat( client, eventuallyRecieves( msgFailure( Status.Security.Unauthorized,
-                String.format( "The client is unauthorized due to authentication failure. (ID:%s)", server.uniqueIdentier()) ) ) );
+                "The client is unauthorized due to authentication failure." ) ) );
 
         // But the new password works fine
         reconnect();
@@ -188,7 +188,7 @@ public void shouldBeAbleToChangePasswordUsingBuiltInProcedure() throws Throwable
                                 map( "principal", "neo4j", "credentials", "neo4j", "scheme", "basic" ) ) ) );
         assertThat( client, eventuallyRecieves( new byte[]{0, 0, 0, 1} ) );
         assertThat( client, eventuallyRecieves( msgFailure( Status.Security.Unauthorized,
-                String.format( "The client is unauthorized due to authentication failure. (ID:%s)", server.uniqueIdentier()) ) ) );
+                "The client is unauthorized due to authentication failure." ) ) );
 
         // But the new password works fine
         reconnect();

diff --git a/community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/Neo4jWithSocket.java b/community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/Neo4jWithSocket.java
@@ -31,7 +31,6 @@
 import java.util.function.Consumer;
 
 import org.neo4j.bolt.BoltKernelExtension;
-import org.neo4j.bolt.security.auth.AuthUtils;
 import org.neo4j.graphdb.GraphDatabaseService;
 import org.neo4j.graphdb.config.Setting;
 import org.neo4j.kernel.impl.factory.GraphDatabaseFacade;
@@ -56,12 +55,6 @@ public Neo4jWithSocket( Consumer<Map<Setting<?>, String>> configure )
         this.configure = configure;
     }
 
-    public String uniqueIdentier()
-    {
-        return AuthUtils.uniqueIdentifier( storeId );
-    }
-
-
     @Override
     public Statement apply( final Statement statement, Description description )
     {