Skip to content

Commit

Permalink
Handle query by token and caching inside UserRepository
Browse files Browse the repository at this point in the history
  • Loading branch information
@cleishm
cleishm committed Jan 15, 2015
1 parent 4a06ca0 commit 5ce452a
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 56 deletions.
8 changes: 4 additions & 4 deletions community/server/src/main/java/org/neo4j/server/security/auth/Authentication.java
View file
Expand Up @@ -92,7 +92,7 @@ private boolean tooManyAuthAttemtps()

protected boolean isCorrectPassword( String password )
{
User user = users.get( name );
User user = users.findByName( name );
if(user != null)
{
String hash = hash( user.credentials().salt(), password, user.credentials().digestAlgorithm() );
Expand Down Expand Up @@ -145,7 +145,7 @@ public boolean authenticate( String name, String password ) throws TooManyAuthen

public void setPassword( String name, String password ) throws IOException
{
User user = users.get( name );
User user = users.findByName( name );
if(user != null)
{
try
Expand All @@ -170,7 +170,7 @@ public void setPassword( String name, String password ) throws IOException
/** Mark the user with the specified name as requiring a password change. All API access will be blocked until the password is changed. */
public void requirePasswordChange( String name ) throws IOException
{
User user = users.get( name );
User user = users.findByName( name );
if(user != null)
{
try
Expand Down Expand Up @@ -198,7 +198,7 @@ private AuthenticationMetadata authMetadataFor( String name )
AuthenticationMetadata authMeta = authenticationData.get( name );
if(authMeta == null)
{
User user = users.get( name );
User user = users.findByName( name );
if ( user != null )
{
authMeta = new AuthenticationMetadata( name, maxFailedAttempts, failedAuthCooldownPeriod, clock );
Expand Down
32 changes: 19 additions & 13 deletions community/server/src/main/java/org/neo4j/server/security/auth/FileUserRepository.java
View file
Expand Up @@ -24,7 +24,6 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -52,6 +51,9 @@ public class FileUserRepository extends LifecycleAdapter implements UserReposito
/** Quick lookup of users by name */
private final Map<String, User> usersByName = new ConcurrentHashMap<>();

/** Quick lookup of users by token */
private final Map<String, User> usersByToken = new ConcurrentHashMap<>();

/** Master list of users */
private volatile List<User> users = new ArrayList<>();

Expand All @@ -65,11 +67,17 @@ public FileUserRepository( FileSystemAbstraction fs, File file )
}

@Override
public User get( String name )
public User findByName( String name )
{
return usersByName.get( name );
}

@Override
public User findByToken( String name )
{
return usersByToken.get( name );
}

@Override
public void start() throws Throwable
{
Expand Down Expand Up @@ -100,22 +108,22 @@ public synchronized void save( User user ) throws IllegalTokenException, IOExcep

// Copy-on-write for the users list
List<User> newUsers = new ArrayList<>(users);
boolean replacedExisting = false;
User existingUser = null;
for ( int i = 0; i < newUsers.size(); i++ )
{
User other = newUsers.get( i );
if( other.name().equals( user.name() ))
{
existingUser = other;
newUsers.set( i, user );
replacedExisting = true;
}
else if ( user.token() != User.NO_TOKEN && other.tokenEquals( user.token() ) )
{
throw new IllegalTokenException( "The specified token is already in use." );
}
}

if(!replacedExisting)
if ( existingUser == null )
{
newUsers.add( user );
}
Expand All @@ -125,6 +133,11 @@ else if ( user.token() != User.NO_TOKEN && other.tokenEquals( user.token() ) )
commitToDisk();

usersByName.put( user.name(), user );
if ( existingUser != null )
{
usersByToken.remove( existingUser.token() );
}
usersByToken.put( user.token(), user );
}

@Override
Expand Down Expand Up @@ -197,15 +210,8 @@ private void loadUsersFromFile( File fileToLoadFrom ) throws IOException
for ( User user : users )
{
usersByName.put( user.name(), user );
usersByToken.put( user.token(), user );
}
}
}

@Override
public Iterator<User> iterator()
{
return users.iterator();
}


}
27 changes: 3 additions & 24 deletions community/server/src/main/java/org/neo4j/server/security/auth/SecurityCentral.java
View file
Expand Up @@ -21,8 +21,6 @@

import java.io.IOException;
import java.security.SecureRandom;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

import org.neo4j.helpers.Clock;
import org.neo4j.helpers.ThisShouldNotHappenError;
Expand All @@ -42,9 +40,6 @@ public class SecurityCentral extends LifecycleAdapter
{
public static final User UNAUTHENTICATED = new UnauthenticatedUser();

// Cache to speed up token lookups
private final ConcurrentMap<String, User> usersByToken = new ConcurrentHashMap<>();

private final Authentication authentication;
private final UserRepository users;
private final SecureRandom rand = new SecureRandom();
Expand Down Expand Up @@ -100,21 +95,12 @@ public User userForToken( String token )
return UNAUTHENTICATED;
}

User user = usersByToken.get( token );
User user = users.findByToken( token );
if( user != null)
{
return user;
}

for ( User candidate : users )
{
if( candidate.tokenEquals( token ) )
{
usersByToken.putIfAbsent( candidate.token(), candidate );
return candidate;
}
}

return UNAUTHENTICATED;
}

Expand All @@ -129,7 +115,7 @@ public User userForName( String name )
return UNAUTHENTICATED;
}

User user = users.get( name );
User user = users.findByName( name );
if( user != null)
{
return user;
Expand Down Expand Up @@ -159,10 +145,9 @@ public String regenerateToken( String name ) throws IOException
public synchronized void setToken( String name, String token ) throws IllegalTokenException, IOException
{
assertValidToken( token );
User user = users.get( name );
User user = users.findByName( name );
if(user != null)
{
String oldToken = user.token();
user = user.augment().withToken( token ).build();
try
{
Expand All @@ -172,12 +157,6 @@ public synchronized void setToken( String name, String token ) throws IllegalTok
{
throw new ThisShouldNotHappenError( "Jake", "Username has already been accepted, we are modifying the token only." );
}
usersByToken.put( token, user );

if(oldToken != User.NO_TOKEN)
{
usersByToken.remove( oldToken );
}
}
}

Expand Down
6 changes: 4 additions & 2 deletions community/server/src/main/java/org/neo4j/server/security/auth/UserRepository.java
View file
Expand Up @@ -27,9 +27,11 @@
/**
* A component that can store and retrieve users. Implementations must be thread safe.
*/
public interface UserRepository extends Iterable<User>
public interface UserRepository
{
public User get( String name );
public User findByName( String name );

public User findByToken( String token );

/** Saves a user, given that the users token is unique. */
public void save( User user ) throws IllegalTokenException, IOException, IllegalUsernameException;
Expand Down
45 changes: 40 additions & 5 deletions community/server/src/test/java/org/neo4j/server/security/auth/FileUserRepositoryTest.java
View file
Expand Up @@ -27,6 +27,7 @@
import org.neo4j.test.EphemeralFileSystemRule;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
Expand All @@ -36,20 +37,35 @@ public class FileUserRepositoryTest
public @Rule EphemeralFileSystemRule fsRule = new EphemeralFileSystemRule();

@Test
public void shouldStoreAndRetriveUsers() throws Exception
public void shouldStoreAndRetriveUsersByName() throws Exception
{
// Given
FileUserRepository users = new FileUserRepository( fsRule.get(), new File( "dbms/auth.db" ) );
User user = new User( "jake", "af123", Privileges.ADMIN, Credentials.INACCESSIBLE, true );
users.save( user );

// When
User result = users.get( user.name() );
User result = users.findByName( user.name() );

// Then
assertThat(result, equalTo(user));
}

@Test
public void shouldStoreAndRetriveUsersByToken() throws Exception
{
// Given
FileUserRepository users = new FileUserRepository( fsRule.get(), new File( "dbms/auth.db" ) );
User user = new User( "jake", "af123", Privileges.ADMIN, Credentials.INACCESSIBLE, true );
users.save( user );

// When
User result = users.findByToken( user.token() );

// Then
assertThat( result, equalTo( user ) );
}

@Test
public void shouldPersistUsers() throws Throwable
{
Expand All @@ -62,10 +78,29 @@ public void shouldPersistUsers() throws Throwable
users.start();

// When
User result = users.get( user.name() );
User resultByName = users.findByName( user.name() );
User resultByToken = users.findByToken( user.token() );

// Then
assertThat(result, equalTo(user));
assertThat( resultByName, equalTo( user ) );
assertThat( resultByToken, equalTo( user ) );
}

@Test
public void shouldNotFindUserByTokenAfterChangingToken() throws Throwable
{
// Given
FileUserRepository users = new FileUserRepository( fsRule.get(), new File( "dbms/auth.db" ) );
User user = new User( "jake", "af123", Privileges.ADMIN, Credentials.INACCESSIBLE, true );
users.save( user );

// When
User updatedUser = new User( "jake", "321fa", Privileges.ADMIN, Credentials.INACCESSIBLE, true );
users.save( updatedUser );

// Then
assertThat( users.findByToken( updatedUser.token() ), equalTo( updatedUser ) );
assertThat( users.findByToken( user.token() ), nullValue() );
}

@Test
Expand Down Expand Up @@ -105,6 +140,6 @@ public void shouldRecoverIfCrashedDuringWrite() throws Throwable
// Then
assertFalse(fsRule.get().fileExists( tempFile ));
assertTrue(fsRule.get().fileExists( dbFile ));
assertThat( users.get( user.name() ), equalTo(user));
assertThat( users.findByName( user.name() ), equalTo(user));
}
}
23 changes: 15 additions & 8 deletions community/server/src/test/java/org/neo4j/server/security/auth/InMemoryUserRepository.java
View file
Expand Up @@ -19,7 +19,6 @@
*/
package org.neo4j.server.security.auth;

import java.util.Iterator;
import java.util.concurrent.ConcurrentHashMap;

import org.neo4j.server.security.auth.exception.IllegalTokenException;
Expand All @@ -29,11 +28,25 @@ public class InMemoryUserRepository implements UserRepository
{
private final ConcurrentHashMap<String, User> users = new ConcurrentHashMap<>();

public User get( String name )
@Override
public User findByName( String name )
{
return users.get( name );
}

@Override
public User findByToken( String token )
{
for ( User user : users.values() )
{
if ( user.token().equals( token ) )
{
return user;
}
}
return null;
}

/** This is synchronized to ensure we can't have users with duplicate tokens. */
public synchronized void save( User user ) throws IllegalTokenException
{
Expand All @@ -51,12 +64,6 @@ public synchronized void save( User user ) throws IllegalTokenException
users.put( user.name(), user );
}

@Override
public Iterator<User> iterator()
{
return users.values().iterator();
}

@Override
public int numberOfUsers()
{
Expand Down

0 comments on commit 5ce452a

Please sign in to comment.