Skip to content

Commit

Permalink
Pass ldap timeout error to client on authentication
Browse files Browse the repository at this point in the history 
If an ldap timeout error occurs on authentication we now pass the specific
error to the client instead of the generic authentication failed error.
  • Loading branch information
@henriknyman
henriknyman committed Oct 26, 2016
1 parent d6fc850 commit 8fbac03
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 10 deletions.
3 changes: 2 additions & 1 deletion community/bolt/src/main/java/org/neo4j/bolt/v1/runtime/BoltStateMachine.java
View file
Expand Up @@ -33,6 +33,7 @@
import org.neo4j.bolt.v1.runtime.spi.BoltResult;
import org.neo4j.function.ThrowingConsumer;
import org.neo4j.graphdb.security.AuthExpirationException;
import org.neo4j.graphdb.security.AuthTimeoutException;
import org.neo4j.kernel.api.bolt.ManagedBoltStateMachine;
import org.neo4j.kernel.api.exceptions.KernelException;
import org.neo4j.kernel.api.exceptions.Status;
Expand Down Expand Up @@ -350,7 +351,7 @@ public State init( BoltStateMachine machine, String userAgent,
}
return READY;
}
catch ( AuthenticationException e )
catch ( AuthenticationException | AuthTimeoutException e )
{
fail( machine, Neo4jError.fatalFrom( e.status(), e.getMessage() ) );
throw new BoltConnectionAuthFatality( e.getMessage() );
Expand Down
2 changes: 1 addition & 1 deletion community/common/src/main/java/org/neo4j/kernel/api/exceptions/Status.java
View file
Expand Up @@ -413,7 +413,7 @@ enum Security implements Status
EncryptionRequired( ClientError, "A TLS encrypted connection is required." ),
Forbidden( ClientError, "An attempt was made to perform an unauthorized action." ),
AuthorizationExpired( ClientError, "The stored authorization info has expired. Please reconnect." ),
Timeout( TransientError, "An authorization request timed out." );
Timeout( TransientError, "An auth provider request timed out." );

private final Code code;

Expand Down
20 changes: 17 additions & 3 deletions enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/LdapRealm.java
View file
Expand Up @@ -81,6 +81,9 @@ public class LdapRealm extends JndiLdapRealm implements RealmLifecycle, ShiroAut

private static final String JNDI_LDAP_CONNECT_TIMEOUT = "com.sun.jndi.ldap.connect.timeout";
private static final String JNDI_LDAP_READ_TIMEOUT = "com.sun.jndi.ldap.read.timeout";
private static final String JNDI_LDAP_READ_TIMEOUT_MESSAGE_PART = "LDAP response read timed out";
public static final String LDAP_CONNECTION_TIMEOUT_CLIENT_MESSAGE = "LDAP connection timed out.";
public static final String LDAP_READ_TIMEOUT_CLIENT_MESSAGE = "LDAP response timed out.";

private Boolean authenticationEnabled;
private Boolean authorizationEnabled;
Expand Down Expand Up @@ -148,6 +151,15 @@ protected AuthenticationInfo queryForAuthenticationInfo( AuthenticationToken tok
{
securityLog.error( withRealm( "Failed to authenticate user '%s' against %s: %s",
token.getPrincipal(), serverString, e.getMessage() ) );
if ( e instanceof CommunicationException )
{
throw new AuthTimeoutException( LDAP_CONNECTION_TIMEOUT_CLIENT_MESSAGE, e );
}
else if (e instanceof NamingException &&
e.getMessage().contains( JNDI_LDAP_READ_TIMEOUT_MESSAGE_PART ) )
{
throw new AuthTimeoutException( LDAP_READ_TIMEOUT_CLIENT_MESSAGE, e );
}
throw e;
}
}
Expand Down Expand Up @@ -340,11 +352,13 @@ protected AuthorizationInfo doGetAuthorizationInfo( PrincipalCollection principa
{
securityLog.error( withRealm( "Failed to get authorization info: '%s' caused by '%s'",
e.getMessage(), e.getCause().getMessage() ) );
if ( e.getCause().getMessage().contains( "LDAP response read timed out" ) )
// Shiro's doGetAuthorizationInfo() wraps a NamingException in an AuthorizationException
if ( e.getCause() != null && e.getCause() instanceof NamingException &&
e.getCause().getMessage().contains( JNDI_LDAP_READ_TIMEOUT_MESSAGE_PART ) )
{
throw new AuthTimeoutException( e.getCause().getMessage(), e );
throw new AuthTimeoutException( LDAP_READ_TIMEOUT_CLIENT_MESSAGE, e );
}
// TODO: We should define a Neo4j exception with a status for this so it doesn't become Unknown Error
// TODO: Should we define a Neo4j exception with a status for this so it doesn't become Unknown Error?
throw e;
}
}
Expand Down
7 changes: 7 additions & 0 deletions ...curity/src/main/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManager.java
View file
Expand Up @@ -43,6 +43,7 @@
import java.util.List;
import java.util.Map;

import org.neo4j.graphdb.security.AuthTimeoutException;
import org.neo4j.kernel.api.security.AuthToken;
import org.neo4j.kernel.api.security.AuthenticationResult;
import org.neo4j.kernel.api.security.SecurityContext;
Expand Down Expand Up @@ -119,6 +120,12 @@ public EnterpriseSecurityContext login( Map<String,Object> authToken ) throws In
}
catch ( AuthenticationException e )
{
if ( e.getCause() != null && e.getCause() instanceof AuthTimeoutException )
{
securityLog.error( "[%s]: failed to log in: auth server timeout",
escape( token.getPrincipal().toString() ) );
throw new AuthTimeoutException( e.getCause().getMessage(), e.getCause() );
}
securityContext = new StandardEnterpriseSecurityContext( this,
new ShiroSubject( securityManager, AuthenticationResult.FAILURE ) );
securityLog.error( "[%s]: failed to log in: invalid principal or credentials",
Expand Down
35 changes: 30 additions & 5 deletions ...java/org/neo4j/server/security/enterprise/auth/integration/bolt/LdapAuthenticationIT.java
View file
Expand Up @@ -28,7 +28,6 @@
import org.apache.directory.server.core.annotations.CreateDS;
import org.apache.directory.server.core.annotations.CreatePartition;
import org.apache.directory.server.core.annotations.LoadSchema;
import org.apache.directory.server.core.api.DirectoryService;
import org.apache.directory.server.core.api.filtering.EntryFilteringCursor;
import org.apache.directory.server.core.api.interceptor.BaseInterceptor;
import org.apache.directory.server.core.api.interceptor.Interceptor;
Expand All @@ -38,6 +37,7 @@
import org.apache.shiro.realm.ldap.JndiLdapContextFactory;
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.runner.RunWith;

import java.io.File;
Expand Down Expand Up @@ -72,6 +72,10 @@
import static org.neo4j.bolt.v1.messaging.util.MessageMatchers.msgSuccess;
import static org.neo4j.bolt.v1.runtime.spi.StreamMatchers.eqRecord;
import static org.neo4j.bolt.v1.transport.integration.TransportTestUtil.eventuallyReceives;
import static org.neo4j.server.security.enterprise.auth.LdapRealm.LDAP_CONNECTION_TIMEOUT_CLIENT_MESSAGE;
import static org.neo4j.server.security.enterprise.auth.LdapRealm.LDAP_READ_TIMEOUT_CLIENT_MESSAGE;

interface TimeoutTests { /* Category marker */ };

@RunWith( FrameworkRunner.class )
@CreateDS(
Expand Down Expand Up @@ -521,6 +525,7 @@ public void shouldBeAbleToUseProcedureAllowedAnnotationWithLdapGroupToRoleMappin
}

@Test
@Category( TimeoutTests.class )
public void shouldTimeoutIfInvalidLdapServer() throws Throwable
{
// When
Expand All @@ -529,10 +534,12 @@ public void shouldTimeoutIfInvalidLdapServer() throws Throwable
settings.put( SecuritySettings.ldap_connection_timeout, "1s" );
} ) );

assertAuthFail( "neo", "abc123" );
assertConnectionTimeout( authToken( "neo", "abc123", null ),
LDAP_CONNECTION_TIMEOUT_CLIENT_MESSAGE );
}

@Test
@Category( TimeoutTests.class )
public void shouldTimeoutIfLdapServerDoesNotRespond() throws Throwable
{
try ( DirectoryServiceWaitOnSearch ignore = new DirectoryServiceWaitOnSearch( 5000 ) )
Expand All @@ -547,6 +554,7 @@ public void shouldTimeoutIfLdapServerDoesNotRespond() throws Throwable
}

@Test
@Category( TimeoutTests.class )
public void shouldTimeoutIfLdapServerDoesNotRespondWithLdapUserContext() throws Throwable
{
try ( DirectoryServiceWaitOnSearch ignore = new DirectoryServiceWaitOnSearch( 5000 ) )
Expand All @@ -557,7 +565,8 @@ public void shouldTimeoutIfLdapServerDoesNotRespondWithLdapUserContext() throws
settings.put( SecuritySettings.ldap_read_timeout, "1s" );
} ) );

assertAuthFail( "neo", "abc123" );
assertConnectionTimeout( authToken( "neo", "abc123", null ),
LDAP_READ_TIMEOUT_CLIENT_MESSAGE );
}
}

Expand Down Expand Up @@ -902,8 +911,18 @@ private void assertLdapAuthorizationTimeout() throws IOException

// Then
assertThat( client, eventuallyReceives(
msgFailure( Status.Security.Timeout,
String.format( "LDAP response read timed out" ) ) ) );
msgFailure( Status.Security.Timeout, LDAP_READ_TIMEOUT_CLIENT_MESSAGE ) ) );
}

private void assertConnectionTimeout( Map<String,Object> authToken, String message ) throws Exception
{
client.connect( address )
.send( TransportTestUtil.acceptedVersions( 1, 0, 0, 0 ) )
.send( TransportTestUtil.chunk(
init( "TestClient/1.1", authToken ) ) );

assertThat( client, eventuallyReceives( new byte[]{0, 0, 0, 1} ) );
assertThat( client, eventuallyReceives( msgFailure( Status.Security.Timeout, message ) ) );
}

private void testClearAuthCache() throws Exception
Expand Down Expand Up @@ -972,6 +991,12 @@ public DirectoryServiceWaitOnSearch( long waitingTimeMillis )
{
waitOnSearchInterceptor = new BaseInterceptor()
{
@Override
public String getName()
{
return getClass().getName();
}

@Override
public EntryFilteringCursor search( SearchOperationContext searchContext ) throws LdapException
{
Expand Down

0 comments on commit 8fbac03

Please sign in to comment.