Skip to content

Commit

Permalink
Switched scheduling of auth file reload to JobScheduler
Browse files Browse the repository at this point in the history
  • Loading branch information
fickludd committed Sep 11, 2016
1 parent 314d800 commit 767643d
Show file tree
Hide file tree
Showing 13 changed files with 58 additions and 50 deletions.
Expand Up @@ -23,6 +23,7 @@
import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.impl.util.JobScheduler;
import org.neo4j.kernel.lifecycle.Lifecycle;
import org.neo4j.logging.LogProvider;

Expand All @@ -40,7 +41,8 @@ public Factory( String key, String... altKeys )
super( key, altKeys );
}

public abstract AuthManager newInstance( Config config, LogProvider logProvider, FileSystemAbstraction fileSystem );
public abstract AuthManager newInstance( Config config, LogProvider logProvider,
FileSystemAbstraction fileSystem, JobScheduler jobScheduler );
}

boolean supports( Map<String, Object> authToken );
Expand Down
Expand Up @@ -102,7 +102,9 @@ public CommunityEditionModule( PlatformModule platformModule )
dependencies.satisfyDependency(
createKernelData( fileSystem, pageCache, storeDir, config, graphDatabaseFacade, life ) );

life.add( dependencies.satisfyDependency( createAuthManager( config, logging, platformModule.fileSystem ) ) );
life.add( dependencies.satisfyDependency( createAuthManager( config, logging, platformModule.fileSystem,
platformModule.jobScheduler )
) );

commitProcessFactory = new CommunityCommitProcessFactory();

Expand Down
Expand Up @@ -45,6 +45,7 @@
import org.neo4j.kernel.impl.store.id.IdReuseEligibility;
import org.neo4j.kernel.impl.store.id.configuration.IdTypeConfigurationProvider;
import org.neo4j.kernel.impl.transaction.TransactionHeaderInformationFactory;
import org.neo4j.kernel.impl.util.JobScheduler;
import org.neo4j.kernel.info.DiagnosticsManager;
import org.neo4j.kernel.internal.KernelDiagnostics;
import org.neo4j.udc.UsageData;
Expand Down Expand Up @@ -107,7 +108,8 @@ protected void publishEditionInfo( UsageData sysInfo, DatabaseInfo databaseInfo,
config.augment( singletonMap( Configuration.editionName.name(), databaseInfo.edition.toString() ) );
}

public static AuthManager createAuthManager( Config config, LogService logging, FileSystemAbstraction fileSystem )
public static AuthManager createAuthManager( Config config, LogService logging,
FileSystemAbstraction fileSystem, JobScheduler jobScheduler )
{
boolean authEnabled = config.get( GraphDatabaseSettings.auth_enabled );
if ( !authEnabled )
Expand All @@ -121,14 +123,14 @@ public static AuthManager createAuthManager( Config config, LogService logging,
String candidateId = candidate.getKeys().iterator().next();
if ( candidateId.equals( key ) )
{
return candidate.newInstance( config, logging.getUserLogProvider(), fileSystem );
return candidate.newInstance( config, logging.getUserLogProvider(), fileSystem, jobScheduler );
}
else if ( key.isEmpty() )
{
// As a default use the available service for the configured build edition
logging.getInternalLog( GraphDatabaseFacadeFactory.class )
.info( "No auth manager implementation specified, defaulting to '" + candidateId + "'" );
return candidate.newInstance( config, logging.getUserLogProvider(), fileSystem );
return candidate.newInstance( config, logging.getUserLogProvider(), fileSystem, jobScheduler );
}
}

Expand Down
Expand Up @@ -166,6 +166,11 @@ class Groups
* Storage maintenance.
*/
public static Group storageMaintenance = new Group( "StorageMaintenance", POOLED );

/**
* Native security.
*/
public static Group nativeSecurity = new Group( "NativeSecurity", POOLED );
}

interface JobHandle
Expand Down
Expand Up @@ -57,7 +57,7 @@ public void shouldFailWhenAuthEnabledAndNoAuthManagerServiceFound()
exception.expectMessage( "Auth enabled but no auth manager found. This is an illegal product configuration." );

// When
EditionModule.createAuthManager( config, logService, null );
EditionModule.createAuthManager( config, logService, null, null );

// Then
verify( userLog ).error( anyString() );
Expand Down
Expand Up @@ -28,6 +28,7 @@
import org.neo4j.kernel.api.security.AuthManager;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.impl.logging.LogService;
import org.neo4j.kernel.impl.util.JobScheduler;
import org.neo4j.logging.LogProvider;
import org.neo4j.time.Clocks;

Expand Down Expand Up @@ -67,7 +68,8 @@ public BasicAuthManagerFactory()
}

@Override
public AuthManager newInstance( Config config, LogProvider logProvider, FileSystemAbstraction fileSystem )
public AuthManager newInstance( Config config, LogProvider logProvider,
FileSystemAbstraction fileSystem, JobScheduler jobScheduler )
{
if ( !config.get( GraphDatabaseSettings.auth_enabled ) )
{
Expand Down
Expand Up @@ -215,7 +215,8 @@ private void editionInvariants( PlatformModule platformModule, Dependencies depe
createKernelData( platformModule.fileSystem, platformModule.pageCache, platformModule.storeDir,
config, platformModule.graphDatabaseFacade, life ) );

life.add( dependencies.satisfyDependency( createAuthManager( config, logging, platformModule.fileSystem ) ) );
life.add( dependencies.satisfyDependency( createAuthManager( config, logging,
platformModule.fileSystem, platformModule.jobScheduler ) ) );

ioLimiter = new ConfigurableIOLimiter( platformModule.config );

Expand Down
Expand Up @@ -151,7 +151,8 @@ public void registerProcedures( Procedures procedures )
life.add( dependencies.satisfyDependency(
new DefaultKernelData( fileSystem, pageCache, storeDir, config, graphDatabaseFacade ) ) );

life.add( dependencies.satisfyDependency( createAuthManager( config, logging, platformModule.fileSystem ) ) );
life.add( dependencies.satisfyDependency( createAuthManager( config, logging,
platformModule.fileSystem, platformModule.jobScheduler ) ) );

headerInformationFactory = TransactionHeaderInformationFactory.DEFAULT;

Expand Down
Expand Up @@ -494,7 +494,8 @@ public void elected( String role, InstanceId instanceId, URI electedMember )
createKernelData( config, platformModule.graphDatabaseFacade, members, fs, platformModule.pageCache,
storeDir, lastUpdateTime, lastTxIdGetter, life ) );

life.add( dependencies.satisfyDependency( createAuthManager( config, logging, platformModule.fileSystem ) ) );
life.add( dependencies.satisfyDependency( createAuthManager( config, logging,
platformModule.fileSystem, platformModule.jobScheduler ) ) );

commitProcessFactory = createCommitProcessFactory( dependencies, logging, monitors, config, paxosLife,
clusterClient, members, platformModule.jobScheduler, master, requestContextFactory,
Expand Down
Expand Up @@ -32,6 +32,7 @@
import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.kernel.api.security.AuthManager;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.impl.util.JobScheduler;
import org.neo4j.logging.LogProvider;
import org.neo4j.server.security.auth.AuthenticationStrategy;
import org.neo4j.server.security.auth.BasicPasswordPolicy;
Expand All @@ -56,14 +57,15 @@ public EnterpriseAuthManagerFactory()
}

@Override
public AuthManager newInstance( Config config, LogProvider logProvider, FileSystemAbstraction fileSystem )
public AuthManager newInstance( Config config, LogProvider logProvider,
FileSystemAbstraction fileSystem, JobScheduler jobScheduler )
{
StaticLoggerBinder.setNeo4jLogProvider( logProvider );

List<Realm> realms = new ArrayList<>( 2 );

// We always create the internal realm as it is our only UserManager implementation
InternalFlatFileRealm internalRealm = createInternalRealm( config, logProvider, fileSystem );
InternalFlatFileRealm internalRealm = createInternalRealm( config, logProvider, fileSystem, jobScheduler );

if ( config.get( SecuritySettings.internal_authentication_enabled ) ||
config.get( SecuritySettings.internal_authorization_enabled ) )
Expand Down Expand Up @@ -91,7 +93,7 @@ public AuthManager newInstance( Config config, LogProvider logProvider, FileSyst
}

private static InternalFlatFileRealm createInternalRealm( Config config, LogProvider logProvider,
FileSystemAbstraction fileSystem )
FileSystemAbstraction fileSystem, JobScheduler jobScheduler )
{
// Resolve auth store and roles file names
File authStoreDir = config.get( DatabaseManagementSystemSettings.auth_store_directory );
Expand All @@ -104,6 +106,6 @@ private static InternalFlatFileRealm createInternalRealm( Config config, LogProv

return new InternalFlatFileRealm( userRepository, roleRepository, passwordPolicy, authenticationStrategy,
config.get( SecuritySettings.internal_authentication_enabled ),
config.get( SecuritySettings.internal_authorization_enabled ) );
config.get( SecuritySettings.internal_authorization_enabled ), jobScheduler );
}
}
Expand Up @@ -46,14 +46,13 @@
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

import org.neo4j.kernel.api.security.AuthToken;
import org.neo4j.kernel.api.security.AuthenticationResult;
import org.neo4j.kernel.api.security.exception.InvalidArgumentsException;
import org.neo4j.kernel.api.security.exception.InvalidAuthTokenException;
import org.neo4j.kernel.impl.util.JobScheduler;
import org.neo4j.server.security.auth.AuthenticationStrategy;
import org.neo4j.server.security.auth.Credential;
import org.neo4j.server.security.auth.PasswordPolicy;
Expand Down Expand Up @@ -102,8 +101,14 @@ public Collection<Permission> resolvePermissionsInRole( String roleString )
private final Map<String,SimpleRole> roles;

public InternalFlatFileRealm( UserRepository userRepository, RoleRepository roleRepository,
PasswordPolicy passwordPolicy, AuthenticationStrategy authenticationStrategy, boolean authenticationEnabled,
boolean authorizationEnabled, ScheduledExecutorService executorService )
PasswordPolicy passwordPolicy, AuthenticationStrategy authenticationStrategy, JobScheduler jobScheduler )
{
this( userRepository, roleRepository, passwordPolicy, authenticationStrategy, true, true, jobScheduler );
}

public InternalFlatFileRealm( UserRepository userRepository, RoleRepository roleRepository,
PasswordPolicy passwordPolicy, AuthenticationStrategy authenticationStrategy,
boolean authenticationEnabled, boolean authorizationEnabled, JobScheduler jobScheduler )
{
super();

Expand All @@ -120,16 +125,10 @@ public InternalFlatFileRealm( UserRepository userRepository, RoleRepository role

roles = new PredefinedRolesBuilder().buildRoles();

executorService.scheduleAtFixedRate( () -> tryReload( RELOAD_ATTEMPTS, new LinkedList<>() ),
5, 5, TimeUnit.SECONDS );
}

public InternalFlatFileRealm( UserRepository userRepository, RoleRepository roleRepository,
PasswordPolicy passwordPolicy, AuthenticationStrategy authenticationStrategy, boolean authenticationEnabled,
boolean authorizationEnabled )
{
this( userRepository, roleRepository, passwordPolicy, authenticationStrategy, authenticationEnabled,
authorizationEnabled, new ScheduledThreadPoolExecutor( 1 ) );
jobScheduler.scheduleRecurring(
JobScheduler.Groups.nativeSecurity,
() -> tryReload( RELOAD_ATTEMPTS, new LinkedList<>() ),
5, TimeUnit.SECONDS );
}

private void tryReload( int attemptLeft, java.util.List<String> failures )
Expand Down Expand Up @@ -158,12 +157,6 @@ private void tryReload( int attemptLeft, java.util.List<String> failures )
}
}

public InternalFlatFileRealm( UserRepository userRepository, RoleRepository roleRepository,
PasswordPolicy passwordPolicy, AuthenticationStrategy authenticationStrategy )
{
this( userRepository, roleRepository, passwordPolicy, authenticationStrategy, true, true );
}

@Override
public void initialize() throws Throwable
{
Expand Down
Expand Up @@ -30,12 +30,11 @@
import java.io.InputStream;
import java.util.LinkedList;
import java.util.Queue;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

import org.neo4j.graphdb.mockfs.DelegatingFileSystemAbstraction;
import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.kernel.impl.util.Neo4jJobScheduler;
import org.neo4j.logging.LogProvider;
import org.neo4j.logging.NullLogProvider;
import org.neo4j.server.security.auth.AuthenticationStrategy;
Expand All @@ -60,7 +59,7 @@ public class InternalFlatFileRealmIT
@Rule
public EphemeralFileSystemRule fsRule = new EphemeralFileSystemRule();

TestScheduledExecutorService executor = new TestScheduledExecutorService( 1 );
TestJobScheduler jobScheduler = new TestJobScheduler();
LogProvider logProvider = NullLogProvider.getInstance();
InternalFlatFileRealm realm;
EvilFileSystem fs;
Expand All @@ -79,7 +78,7 @@ public void setup() throws Throwable
AuthenticationStrategy authenticationStrategy = new RateLimitedAuthenticationStrategy( Clocks.systemClock(), 3 );

realm = new InternalFlatFileRealm( userRepository, roleRepository, passwordPolicy, authenticationStrategy,
true, true, executor );
true, true, jobScheduler );
realm.init();
realm.start();
}
Expand All @@ -101,7 +100,7 @@ public void shouldReloadAuthFiles() throws Exception
"admin:Mia\n" +
"publisher:Hanna,Carol\n" );

executor.scheduledRunnable.run();
jobScheduler.scheduledRunnable.run();

assertThat( realm.getAllUsernames(), containsInAnyOrder( "Hanna", "Carol", "Mia" ) );
assertThat( realm.getUsernamesForRole( "admin" ), containsInAnyOrder( "Mia" ) );
Expand Down Expand Up @@ -138,7 +137,7 @@ public void shouldReloadAuthFilesUntilValid() throws Exception
"admin:Mia\n" +
"publisher:Hanna,Carol\n" );

executor.scheduledRunnable.run();
jobScheduler.scheduledRunnable.run();

assertThat( realm.getAllUsernames(), containsInAnyOrder( "Hanna", "Carol", "Mia" ) );
assertThat( realm.getUsernamesForRole( "admin" ), containsInAnyOrder( "Mia" ) );
Expand Down Expand Up @@ -171,7 +170,7 @@ public void shouldEventuallyFailReloadAttempts() throws Exception

try
{
executor.scheduledRunnable.run();
jobScheduler.scheduledRunnable.run();
fail( "Expected exception due to invalid auth file combo." );
}
catch ( Exception e )
Expand All @@ -183,17 +182,12 @@ public void shouldEventuallyFailReloadAttempts() throws Exception
}
}

class TestScheduledExecutorService extends ScheduledThreadPoolExecutor
class TestJobScheduler extends Neo4jJobScheduler
{
Runnable scheduledRunnable;

public TestScheduledExecutorService( int corePoolSize )
{
super( corePoolSize );
}

@Override
public ScheduledFuture<?> scheduleAtFixedRate( Runnable r, long initialDelay, long delay, TimeUnit timeUnit )
public JobHandle scheduleRecurring( Group group, Runnable r, long delay, TimeUnit timeUnit )
{
this.scheduledRunnable = r;
return null;
Expand Down
Expand Up @@ -29,6 +29,7 @@
import org.neo4j.kernel.api.security.AuthSubject;
import org.neo4j.kernel.api.security.AuthenticationResult;
import org.neo4j.kernel.api.security.exception.InvalidArgumentsException;
import org.neo4j.kernel.impl.util.JobScheduler;
import org.neo4j.server.security.auth.AuthenticationStrategy;
import org.neo4j.server.security.auth.Credential;
import org.neo4j.server.security.auth.InMemoryUserRepository;
Expand Down Expand Up @@ -61,8 +62,10 @@ public void setUp() throws Throwable
authStrategy = mock( AuthenticationStrategy.class );

InternalFlatFileRealm internalFlatFileRealm =
new InternalFlatFileRealm( users, new InMemoryRoleRepository(), mock( PasswordPolicy.class ), authStrategy );
manager = new MultiRealmAuthManager( internalFlatFileRealm, Collections.singleton( internalFlatFileRealm ), new MemoryConstrainedCacheManager() );
new InternalFlatFileRealm( users, new InMemoryRoleRepository(), mock( PasswordPolicy.class ),
authStrategy, mock( JobScheduler.class ) );
manager = new MultiRealmAuthManager( internalFlatFileRealm, Collections.singleton( internalFlatFileRealm ),
new MemoryConstrainedCacheManager() );
manager.init();
userManager = manager.getUserManager();
}
Expand Down

0 comments on commit 767643d

Please sign in to comment.