Skip to content

Commit

Permalink
Improved error messages for unsupported auth tokens
Browse files Browse the repository at this point in the history
  • Loading branch information
@fickludd
fickludd committed Jan 4, 2017
1 parent 8ab0685 commit 5380344
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 19 deletions.
23 changes: 19 additions & 4 deletions community/kernel/src/main/java/org/neo4j/kernel/api/security/AuthToken.java
View file
Expand Up @@ -19,11 +19,14 @@
*/ */
package org.neo4j.kernel.api.security; package org.neo4j.kernel.api.security;


import org.apache.commons.lang3.StringUtils;

import java.util.Collections; import java.util.Collections;
import java.util.Map; import java.util.Map;


import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException; import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException;


import static java.lang.String.format;
import static org.neo4j.helpers.collection.MapUtil.map; import static org.neo4j.helpers.collection.MapUtil.map;


public interface AuthToken public interface AuthToken
Expand All @@ -40,11 +43,14 @@ public interface AuthToken
static String safeCast( String key, Map<String,Object> authToken ) throws InvalidAuthTokenException static String safeCast( String key, Map<String,Object> authToken ) throws InvalidAuthTokenException
{ {
Object value = authToken.get( key ); Object value = authToken.get( key );
if ( value == null || !(value instanceof String) ) if ( value == null )
{ {
throw new InvalidAuthTokenException( throw invalidToken( "missing key `" + key + "`" );
"The value associated with the key `" + key + "` must be a String but was: " + }
(value == null ? "null" : value.getClass().getSimpleName()) ); else if ( !(value instanceof String) )
{
throw invalidToken( "the value associated with the key `" + key + "` must be a String but was: "
+ value.getClass().getSimpleName() );
} }
return (String) value; return (String) value;
} }
Expand All @@ -69,6 +75,15 @@ else if ( value instanceof Map )
} }
} }


static InvalidAuthTokenException invalidToken( String explanation )
{
if ( StringUtils.isNotEmpty( explanation ) && !explanation.matches( "^[,.:;].*" ) )
{
explanation = ", " + explanation;
}
return new InvalidAuthTokenException( format( "Unsupported authentication token%s", explanation ) );
}

static Map<String,Object> newBasicAuthToken( String username, String password ) static Map<String,Object> newBasicAuthToken( String username, String password )
{ {
return map( AuthToken.SCHEME_KEY, BASIC_SCHEME, AuthToken.PRINCIPAL, username, AuthToken.CREDENTIALS, return map( AuthToken.SCHEME_KEY, BASIC_SCHEME, AuthToken.PRINCIPAL, username, AuthToken.CREDENTIALS,
Expand Down
21 changes: 16 additions & 5 deletions community/security/src/main/java/org/neo4j/server/security/auth/BasicAuthManager.java
View file
Expand Up @@ -34,6 +34,8 @@
import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException; import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException;
import org.neo4j.server.security.auth.exception.ConcurrentModificationException; import org.neo4j.server.security.auth.exception.ConcurrentModificationException;


import static org.neo4j.kernel.api.security.AuthToken.invalidToken;

/** /**
* Manages server authentication and authorization. * Manages server authentication and authorization.
* <p> * <p>
Expand Down Expand Up @@ -109,11 +111,7 @@ public void shutdown() throws Throwable
@Override @Override
public BasicSecurityContext login( Map<String,Object> authToken ) throws InvalidAuthTokenException public BasicSecurityContext login( Map<String,Object> authToken ) throws InvalidAuthTokenException
{ {
String scheme = AuthToken.safeCast( AuthToken.SCHEME_KEY, authToken ); assertValidScheme( authToken );
if ( !scheme.equals( AuthToken.BASIC_SCHEME ) )
{
throw new InvalidAuthTokenException( "Unsupported authentication scheme '" + scheme + "'." );
}


String username = AuthToken.safeCast( AuthToken.PRINCIPAL, authToken ); String username = AuthToken.safeCast( AuthToken.PRINCIPAL, authToken );
String password = AuthToken.safeCast( AuthToken.CREDENTIALS, authToken ); String password = AuthToken.safeCast( AuthToken.CREDENTIALS, authToken );
Expand Down Expand Up @@ -228,4 +226,17 @@ public UserManager getUserManager()
{ {
return this; return this;
} }

private void assertValidScheme( Map<String,Object> token ) throws InvalidAuthTokenException
{
String scheme = AuthToken.safeCast( AuthToken.SCHEME_KEY, token );
if ( scheme.equals( "none" ) )
{
throw invalidToken( ", scheme 'none' is only allowed when auth is disabled." );
}
if ( !scheme.equals( AuthToken.BASIC_SCHEME ) )
{
throw invalidToken( ", scheme '" + scheme + "' is not supported." );
}
}
} }
35 changes: 34 additions & 1 deletion community/security/src/test/java/org/neo4j/server/security/auth/BasicAuthManagerTest.java
View file
Expand Up @@ -27,7 +27,7 @@


import org.neo4j.kernel.api.exceptions.InvalidArgumentsException; import org.neo4j.kernel.api.exceptions.InvalidArgumentsException;
import org.neo4j.kernel.api.security.AuthManager; import org.neo4j.kernel.api.security.AuthManager;
import org.neo4j.kernel.api.security.AuthSubject; import org.neo4j.kernel.api.security.AuthToken;
import org.neo4j.kernel.api.security.AuthenticationResult; import org.neo4j.kernel.api.security.AuthenticationResult;
import org.neo4j.kernel.api.security.SecurityContext; import org.neo4j.kernel.api.security.SecurityContext;
import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException; import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException;
Expand All @@ -43,11 +43,13 @@
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
import static org.neo4j.helpers.collection.MapUtil.map;
import static org.neo4j.kernel.api.security.AuthenticationResult.FAILURE; import static org.neo4j.kernel.api.security.AuthenticationResult.FAILURE;
import static org.neo4j.kernel.api.security.AuthenticationResult.PASSWORD_CHANGE_REQUIRED; import static org.neo4j.kernel.api.security.AuthenticationResult.PASSWORD_CHANGE_REQUIRED;
import static org.neo4j.kernel.api.security.AuthenticationResult.SUCCESS; import static org.neo4j.kernel.api.security.AuthenticationResult.SUCCESS;
import static org.neo4j.kernel.api.security.AuthenticationResult.TOO_MANY_ATTEMPTS; import static org.neo4j.kernel.api.security.AuthenticationResult.TOO_MANY_ATTEMPTS;
import static org.neo4j.server.security.auth.SecurityTestUtils.authToken; import static org.neo4j.server.security.auth.SecurityTestUtils.authToken;
import static org.neo4j.test.assertion.Assert.assertException;


public class BasicAuthManagerTest extends InitialUserTests public class BasicAuthManagerTest extends InitialUserTests
{ {
Expand Down Expand Up @@ -221,6 +223,37 @@ public void shouldReturnNullWhenSettingPasswordForUnknownUser() throws Throwable
} }
} }


@Test
public void shouldFailWhenAuthTokenIsInvalid() throws Throwable
{
manager.start();

assertException(
() -> manager.login( map( AuthToken.SCHEME_KEY, "supercool", AuthToken.PRINCIPAL, "neo4j" ) ),
InvalidAuthTokenException.class,
"Unsupported authentication token, scheme 'supercool' is not supported." );

assertException(
() -> manager.login( map( AuthToken.SCHEME_KEY, "none" ) ),
InvalidAuthTokenException.class,
"Unsupported authentication token, scheme 'none' is only allowed when auth is disabled" );

assertException(
() -> manager.login( map( "key", "value" ) ),
InvalidAuthTokenException.class,
"Unsupported authentication token, missing key `scheme`" );

assertException(
() -> manager.login( map( AuthToken.SCHEME_KEY, "basic", AuthToken.PRINCIPAL, "neo4j" ) ),
InvalidAuthTokenException.class,
"Unsupported authentication token, missing key `credentials`" );

assertException(
() -> manager.login( map( AuthToken.SCHEME_KEY, "basic", AuthToken.CREDENTIALS, "very-secret" ) ),
InvalidAuthTokenException.class,
"Unsupported authentication token, missing key `principal`" );
}

private void assertLoginGivesResult( String username, String password, AuthenticationResult expectedResult ) private void assertLoginGivesResult( String username, String password, AuthenticationResult expectedResult )
throws InvalidAuthTokenException throws InvalidAuthTokenException
{ {
Expand Down
24 changes: 21 additions & 3 deletions ...curity/src/main/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManager.java
View file
Expand Up @@ -44,14 +44,14 @@
import java.util.Map; import java.util.Map;


import org.neo4j.graphdb.security.AuthProviderTimeoutException; import org.neo4j.graphdb.security.AuthProviderTimeoutException;
import org.neo4j.kernel.api.security.AuthToken;
import org.neo4j.kernel.api.security.AuthenticationResult; import org.neo4j.kernel.api.security.AuthenticationResult;
import org.neo4j.kernel.api.security.SecurityContext; import org.neo4j.kernel.api.security.SecurityContext;
import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException; import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException;
import org.neo4j.kernel.enterprise.api.security.EnterpriseSecurityContext; import org.neo4j.kernel.enterprise.api.security.EnterpriseSecurityContext;
import org.neo4j.server.security.enterprise.log.SecurityLog; import org.neo4j.server.security.enterprise.log.SecurityLog;


import static org.neo4j.helpers.Strings.escape; import static org.neo4j.helpers.Strings.escape;
import static org.neo4j.kernel.api.security.AuthToken.invalidToken;


class MultiRealmAuthManager implements EnterpriseAuthAndUserManager class MultiRealmAuthManager implements EnterpriseAuthAndUserManager
{ {
Expand Down Expand Up @@ -93,8 +93,8 @@ public EnterpriseSecurityContext login( Map<String,Object> authToken ) throws In
{ {
EnterpriseSecurityContext securityContext; EnterpriseSecurityContext securityContext;


AuthToken.safeCast( AuthToken.SCHEME_KEY, authToken ); // Scheme must be set
ShiroAuthToken token = new ShiroAuthToken( authToken ); ShiroAuthToken token = new ShiroAuthToken( authToken );
assertValidScheme( token );


try try
{ {
Expand All @@ -108,7 +108,12 @@ public EnterpriseSecurityContext login( Map<String,Object> authToken ) throws In
catch ( UnsupportedTokenException e ) catch ( UnsupportedTokenException e )
{ {
securityLog.error( "Unknown user failed to log in: %s", e.getMessage() ); securityLog.error( "Unknown user failed to log in: %s", e.getMessage() );
throw new InvalidAuthTokenException( e.getMessage() ); Throwable cause = e.getCause();
if ( cause != null && cause instanceof InvalidAuthTokenException )
{
throw new InvalidAuthTokenException( cause.getMessage() + ": " + token );
}
throw invalidToken( ": " + token );
} }
catch ( ExcessiveAttemptsException e ) catch ( ExcessiveAttemptsException e )
{ {
Expand All @@ -135,6 +140,19 @@ public EnterpriseSecurityContext login( Map<String,Object> authToken ) throws In
return securityContext; return securityContext;
} }


private void assertValidScheme( ShiroAuthToken token ) throws InvalidAuthTokenException
{
String scheme = token.getSchemeSilently();
if ( scheme == null )
{
throw invalidToken( "missing key `scheme`: " + token );
}
else if ( scheme.equals( "none" ) )
{
throw invalidToken( "scheme='none' only allowed when auth is disabled: " + token );
}
}

@Override @Override
public void init() throws Throwable public void init() throws Throwable
{ {
Expand Down
43 changes: 43 additions & 0 deletions ...rise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthToken.java
View file
Expand Up @@ -21,13 +21,20 @@


import org.apache.shiro.authc.AuthenticationToken; import org.apache.shiro.authc.AuthenticationToken;


import java.util.ArrayList;
import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.stream.Collectors;


import org.neo4j.kernel.api.security.AuthToken; import org.neo4j.kernel.api.security.AuthToken;
import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException; import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException;


public class ShiroAuthToken implements AuthenticationToken public class ShiroAuthToken implements AuthenticationToken
{ {
private static final String VALUE_QOUTE = "'";
private static final String PAIR_DELIMITER = ", ";
private static final String KEY_VALUE_DELIMITER = "=";

private final Map<String,Object> authToken; private final Map<String,Object> authToken;


public ShiroAuthToken( Map<String,Object> authToken ) public ShiroAuthToken( Map<String,Object> authToken )
Expand All @@ -52,6 +59,12 @@ public String getScheme() throws InvalidAuthTokenException
return AuthToken.safeCast( AuthToken.SCHEME_KEY, authToken ); return AuthToken.safeCast( AuthToken.SCHEME_KEY, authToken );
} }


public String getSchemeSilently()
{
Object scheme = authToken.get( AuthToken.SCHEME_KEY );
return scheme == null ? null : scheme.toString();
}

public Map<String,Object> getAuthTokenMap() public Map<String,Object> getAuthTokenMap()
{ {
return authToken; return authToken;
Expand All @@ -65,4 +78,34 @@ public boolean supportsRealm( String realm )
authToken.get( AuthToken.REALM_KEY ).equals( "*" ) || authToken.get( AuthToken.REALM_KEY ).equals( "*" ) ||
authToken.get( AuthToken.REALM_KEY ).equals( realm ); authToken.get( AuthToken.REALM_KEY ).equals( realm );
} }

@Override
public String toString()
{
if ( authToken.isEmpty() )
{
return "{}";
}

List<String> keys = new ArrayList<>(authToken.keySet());
int schemeIndex = keys.indexOf( AuthToken.SCHEME_KEY );
if ( schemeIndex > 0 )
{
keys.set( schemeIndex, keys.get( 0 ) );
keys.set( 0, AuthToken.SCHEME_KEY );
}

Iterable<String> keyValuePairs =
keys.stream()
.map( this::keyValueString )
.collect( Collectors.toList() );

return "{ " + String.join( PAIR_DELIMITER, keyValuePairs ) + " }";
}

private String keyValueString( String key )
{
String valueString = ( key.equals( AuthToken.CREDENTIALS ) ? "******" : authToken.get( key ).toString() );
return key + KEY_VALUE_DELIMITER + VALUE_QOUTE + valueString + VALUE_QOUTE;
}
} }
30 changes: 24 additions & 6 deletions ...ty/src/test/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManagerTest.java
View file
Expand Up @@ -262,12 +262,30 @@ public void shouldFailWhenAuthTokenIsInvalid() throws Throwable
{ {
manager.start(); manager.start();


assertException( () -> manager.login( map( AuthToken.SCHEME_KEY, "supercool", AuthToken.PRINCIPAL, "neo4j" ) ), assertException(
InvalidAuthTokenException.class, "does not support authentication token" ); () -> manager.login( map( AuthToken.SCHEME_KEY, "supercool", AuthToken.PRINCIPAL, "neo4j" ) ),
assertException( () -> manager.login( map( "key", "value" ) ), InvalidAuthTokenException.class,
InvalidAuthTokenException.class, "The value associated with the key `scheme` must be a String but was: null" ); "Unsupported authentication token: { scheme='supercool', principal='neo4j' }" );
assertException( () -> manager.login( map( AuthToken.SCHEME_KEY, "basic", AuthToken.PRINCIPAL, "neo4j" ) ),
InvalidAuthTokenException.class, "The value associated with the key `credentials` must be a String but was: null" ); assertException(
() -> manager.login( map( AuthToken.SCHEME_KEY, "none" ) ),
InvalidAuthTokenException.class,
"Unsupported authentication token, scheme='none' only allowed when auth is disabled: { scheme='none' }" );

assertException(
() -> manager.login( map( "key", "value" ) ),
InvalidAuthTokenException.class,
"Unsupported authentication token, missing key `scheme`: { key='value' }" );

assertException(
() -> manager.login( map( AuthToken.SCHEME_KEY, "basic", AuthToken.PRINCIPAL, "neo4j" ) ),
InvalidAuthTokenException.class,
"Unsupported authentication token, missing key `credentials`: { scheme='basic', principal='neo4j' }" );

assertException(
() -> manager.login( map( AuthToken.SCHEME_KEY, "basic", AuthToken.CREDENTIALS, "very-secret" ) ),
InvalidAuthTokenException.class,
"Unsupported authentication token, missing key `principal`: { scheme='basic', credentials='******' }" );
} }


@Test @Test
Expand Down

0 comments on commit 5380344

Please sign in to comment.