Skip to content

Commit

Permalink
Don't default auth store location to a path that doesn't make sense i…
Browse files Browse the repository at this point in the history
…n the kernel

The old default assumed the existence of a data directory, which doesn't
make sense in the kernel. This changes it so that there is no default
and a value has to be provided if authentication is turned on.

The server always provides a value (and will override any value that
external config tries to set), giving the desired path of
data/dbms/auth.

This means that the Desktop can no longer set a value for this
explicitly, so we set a location for the data directory instead.
  • Loading branch information
benbc committed Mar 23, 2016
1 parent 66ea28d commit c6fb2af
Show file tree
Hide file tree
Showing 14 changed files with 69 additions and 53 deletions.
Expand Up @@ -451,7 +451,7 @@ private static String defaultPageCacheMemory()
public static final Setting<Boolean> auth_enabled = setting( "dbms.security.auth_enabled", BOOLEAN, "false" );

@Internal
public static final Setting<File> auth_store = setting("dbms.security.auth_store.location", PATH, "data/dbms/auth");
public static final Setting<File> auth_store = setting("dbms.security.auth_store.location", PATH, NO_DEFAULT);

// Bolt Settings

Expand Down
Expand Up @@ -19,6 +19,8 @@
*/
package org.neo4j.kernel.impl.factory;

import java.io.File;

import org.neo4j.graphdb.DependencyResolver;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.io.pagecache.IOLimiter;
Expand Down Expand Up @@ -78,7 +80,7 @@ public abstract class EditionModule

public IOLimiter ioLimiter;

protected void doAfterRecoveryAndStartup( DatabaseInfo databaseInfo, DependencyResolver dependencyResolver)
protected void doAfterRecoveryAndStartup( DatabaseInfo databaseInfo, DependencyResolver dependencyResolver )
{
DiagnosticsManager diagnosticsManager = dependencyResolver.resolveDependency( DiagnosticsManager.class );
NeoStoreDataSource neoStoreDataSource = dependencyResolver.resolveDependency( NeoStoreDataSource.class );
Expand All @@ -96,14 +98,20 @@ protected void publishEditionInfo( UsageData sysInfo, DatabaseInfo databaseInfo,
config.augment( singletonMap( Configuration.editionName.name(), databaseInfo.edition.toString() ) );
}

protected AuthManager createAuthManager(Config config, LifeSupport life, LogProvider logProvider)
protected AuthManager createAuthManager( Config config, LifeSupport life, LogProvider logProvider )
{

boolean authEnabled = config.get( GraphDatabaseSettings.auth_enabled );
if ( authEnabled)
if ( authEnabled )
{
FileUserRepository users = life.add( new FileUserRepository( config.get( GraphDatabaseSettings.auth_store ).toPath(), logProvider ) );
return life.add(new BasicAuthManager( users, systemUTC(), true ));
File storePath = config.get( GraphDatabaseSettings.auth_store );
if ( storePath == null )
{
logProvider.getLog( EditionModule.class ).warn( "Authentication not enabled because %s is not set.",
GraphDatabaseSettings.auth_store.name() );
return AuthManager.NO_AUTH;
}
FileUserRepository users = life.add( new FileUserRepository( storePath.toPath(), logProvider ) );
return life.add( new BasicAuthManager( users, systemUTC(), true ) );
}
else
{
Expand Down
Expand Up @@ -30,7 +30,6 @@

import org.apache.commons.io.FileUtils;

import org.neo4j.dbms.DatabaseManagementSystemSettings;
import org.neo4j.graphdb.GraphDatabaseService;
import org.neo4j.graphdb.config.Setting;
import org.neo4j.harness.ServerControls;
Expand All @@ -51,8 +50,8 @@
import org.neo4j.server.configuration.ServerSettings;
import org.neo4j.server.configuration.ThirdPartyJaxRsPackage;

import static org.neo4j.dbms.DatabaseManagementSystemSettings.data_directory;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.auth_enabled;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.auth_store;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.boltConnector;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.pagecache_memory;
import static org.neo4j.helpers.collection.Iterables.append;
Expand Down Expand Up @@ -86,7 +85,6 @@ private void init( File workingDir )
{
setDirectory( workingDir );
withConfig( auth_enabled, "false" );
withConfig( auth_store, getRelativePath( workingDir, auth_store ) );
withConfig( pagecache_memory, "8m" );
withConfig( httpConnector( "1" ).type, "HTTP" );
withConfig( httpConnector( "1" ).enabled, "true" );
Expand Down Expand Up @@ -211,7 +209,7 @@ public TestServerBuilder withProcedure( Class<?> procedureClass )
private TestServerBuilder setDirectory( File dir )
{
this.serverFolder = dir;
config.put( DatabaseManagementSystemSettings.data_directory.name(), serverFolder.getAbsolutePath() );
config.put( data_directory.name(), serverFolder.getAbsolutePath() );
return this;
}

Expand Down
Expand Up @@ -19,15 +19,14 @@
*/
package org.neo4j.harness;

import org.junit.Rule;
import org.junit.Test;

import java.io.File;
import java.io.IOException;

import org.junit.Rule;
import org.junit.Test;

import org.neo4j.graphdb.Label;
import org.neo4j.graphdb.Transaction;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.io.fs.FileUtils;
import org.neo4j.server.ServerTestUtils;
import org.neo4j.server.configuration.ServerSettings;
Expand All @@ -38,6 +37,7 @@
import static junit.framework.TestCase.fail;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;

import static org.neo4j.harness.TestServerBuilders.newInProcessBuilder;
import static org.neo4j.test.server.HTTP.RawPayload.quotedJson;

Expand Down Expand Up @@ -224,9 +224,7 @@ private TestServerBuilder getServerBuilder( File targetFolder ) throws IOExcepti
.withConfig( ServerSettings.tls_key_file.name(),
ServerTestUtils.getRelativePath( testDir.directory(), ServerSettings.tls_key_file ) )
.withConfig( ServerSettings.tls_certificate_file.name(),
ServerTestUtils.getRelativePath( testDir.directory(), ServerSettings.tls_certificate_file ) )
.withConfig( GraphDatabaseSettings.auth_store.name(),
ServerTestUtils.getRelativePath( testDir.directory(), GraphDatabaseSettings.auth_store ) );
ServerTestUtils.getRelativePath( testDir.directory(), ServerSettings.tls_certificate_file ) );
return serverBuilder;
}

Expand Down
Expand Up @@ -104,8 +104,7 @@ private TestServerBuilder getTestServerBuilder( File workDir )
.withConfig( ServerSettings.tls_key_file.name(),
ServerTestUtils.getRelativePath( testDir.directory(), ServerSettings.tls_key_file ) )
.withConfig( ServerSettings.tls_certificate_file.name(),
ServerTestUtils.getRelativePath( testDir.directory(), ServerSettings.tls_certificate_file ) )
.withConfig( GraphDatabaseSettings.auth_store.name(), new File(workDir, "auth").getAbsolutePath() );
ServerTestUtils.getRelativePath( testDir.directory(), ServerSettings.tls_certificate_file ) );
}

@Test
Expand Down
Expand Up @@ -19,22 +19,20 @@
*/
package org.neo4j.harness;

import java.util.List;

import org.junit.Rule;
import org.junit.Test;
import org.junit.runners.model.Statement;

import java.util.List;

import org.neo4j.graphdb.GraphDatabaseService;
import org.neo4j.graphdb.Label;
import org.neo4j.graphdb.Result;
import org.neo4j.graphdb.Transaction;
import org.neo4j.graphdb.factory.GraphDatabaseFactory;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.harness.extensionpackage.MyUnmanagedExtension;
import org.neo4j.harness.junit.Neo4jRule;
import org.neo4j.helpers.collection.Iterators;
import org.neo4j.server.ServerTestUtils;
import org.neo4j.server.configuration.ServerSettings;
import org.neo4j.test.SuppressOutput;
import org.neo4j.test.TargetDirectory;
Expand All @@ -43,6 +41,7 @@
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;

import static org.neo4j.server.ServerTestUtils.getRelativePath;
import static org.neo4j.server.ServerTestUtils.getSharedTestTemporaryFolder;
import static org.neo4j.test.TargetDirectory.testDirForTest;
Expand Down Expand Up @@ -116,8 +115,6 @@ public void shouldRuleWorkWithExsitingDirectory()

GraphDatabaseService db = new GraphDatabaseFactory()
.newEmbeddedDatabaseBuilder( testDirectory.directory() )
.setConfig( GraphDatabaseSettings.auth_store, ServerTestUtils
.getRelativePath( testDirectory.directory(), GraphDatabaseSettings.auth_store ) )
.newGraphDatabase();
try {
db.execute( "create ()" );
Expand Down
Expand Up @@ -26,13 +26,15 @@
import java.util.Map;
import java.util.Optional;

import org.neo4j.dbms.DatabaseManagementSystemSettings;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.helpers.collection.MapUtil;
import org.neo4j.helpers.collection.Pair;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.logging.Log;
import org.neo4j.shell.ShellSettings;

import static org.neo4j.helpers.collection.MapUtil.stringMap;
import static org.neo4j.kernel.configuration.Settings.TRUE;

public class ConfigLoader
Expand Down Expand Up @@ -61,9 +63,17 @@ public Config loadConfig( Optional<File> configFile, Log log, Pair<String, Strin
HashMap<String, String> settings = calculateSettings( configFile, log, configOverrides );
Config config = new Config( settings, settingsClasses.calculate( settings ) );
config.setLogger( log );
setServerSettings( config );
return config;
}

private void setServerSettings( Config config )
{
String authStore = new File( config.get( DatabaseManagementSystemSettings.data_directory ), "dbms/auth" )
.toString();
config.augment( stringMap( GraphDatabaseSettings.auth_store.name(), authStore ) );
}

private HashMap<String, String> calculateSettings( Optional<File> config, Log log,
Pair<String, String>[] configOverrides )
{
Expand Down
Expand Up @@ -36,7 +36,6 @@

import org.neo4j.dbms.DatabaseManagementSystemSettings;
import org.neo4j.graphdb.config.Setting;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.server.configuration.ServerSettings;

public class ServerTestUtils
Expand Down Expand Up @@ -102,7 +101,6 @@ public static Map<String,String> getDefaultRelativeProperties() throws IOExcepti
public static void addDefaultRelativeProperties( Map<String,String> properties, File temporaryFolder )
{
addRelativeProperty( temporaryFolder, properties, DatabaseManagementSystemSettings.data_directory );
addRelativeProperty( temporaryFolder, properties, GraphDatabaseSettings.auth_store );
addRelativeProperty( temporaryFolder, properties, ServerSettings.tls_certificate_file );
addRelativeProperty( temporaryFolder, properties, ServerSettings.tls_key_file );
}
Expand Down
Expand Up @@ -26,6 +26,7 @@
import java.util.Optional;

import org.neo4j.dbms.DatabaseManagementSystemSettings;
import org.neo4j.graphdb.config.Setting;
import org.neo4j.helpers.collection.MapUtil;
import org.neo4j.server.ServerTestUtils;

Expand Down Expand Up @@ -74,4 +75,10 @@ public ConfigFileBuilder withNameValue( String name, String value )
nameValuePairs.add( new Tuple( name, value ) );
return this;
}

public ConfigFileBuilder withSetting( Setting setting, String value )
{
nameValuePairs.add( new Tuple( setting.name(), value ) );
return this;
}
}
Expand Up @@ -30,15 +30,19 @@
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import org.neo4j.dbms.DatabaseManagementSystemSettings;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.logging.Log;
import org.neo4j.logging.NullLog;
import org.neo4j.server.CommunityBootstrapper;
import org.neo4j.server.ServerTestUtils;
import org.neo4j.test.SuppressOutput;

import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;

import static org.neo4j.kernel.configuration.Settings.NO_DEFAULT;
import static org.neo4j.kernel.configuration.Settings.STRING;
Expand Down Expand Up @@ -109,7 +113,7 @@ public void shouldFindThirdPartyJaxRsPackages() throws IOException
// given
File file = ServerTestUtils.createTempConfigFile( folder.getRoot() );

try(BufferedWriter out = new BufferedWriter( new FileWriter( file, true ) ))
try ( BufferedWriter out = new BufferedWriter( new FileWriter( file, true ) ) )
{
out.write( ServerSettings.third_party_packages.name() );
out.write( "=" );
Expand Down Expand Up @@ -163,4 +167,16 @@ public void shouldWorkFineWhenSpecifiedConfigFileDoesNotExist()
// Then
assertNotNull( config );
}

@Test
public void shouldSetAValueForAuthStoreLocation() throws IOException
{
Optional<File> configFile = ConfigFileBuilder.builder( folder.getRoot() )
.withSetting( DatabaseManagementSystemSettings.data_directory, "the-data-dir" )
.build();
Config config = configLoader.loadConfig( configFile, log );

assertThat( config.get( GraphDatabaseSettings.auth_store ),
is( new File( "the-data-dir/dbms/auth" ).getAbsoluteFile() ) );
}
}
Expand Up @@ -19,6 +19,9 @@
*/
package org.neo4j.server.rest.security;

import java.io.IOException;
import javax.ws.rs.core.HttpHeaders;

import com.sun.jersey.core.util.Base64;
import org.codehaus.jackson.JsonNode;
import org.junit.After;
Expand All @@ -27,15 +30,9 @@
import org.junit.Rule;
import org.junit.Test;

import java.io.File;
import java.io.IOException;
import javax.ws.rs.core.HttpHeaders;

import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.io.fs.FileUtils;
import org.neo4j.kernel.impl.annotations.Documented;
import org.neo4j.server.CommunityNeoServer;
import org.neo4j.server.ServerTestUtils;
import org.neo4j.server.helpers.CommunityServerBuilder;
import org.neo4j.server.rest.RESTDocsGenerator;
import org.neo4j.server.rest.domain.JsonHelper;
Expand Down Expand Up @@ -327,11 +324,8 @@ public void startServerWithConfiguredUser() throws IOException

public void startServer( boolean authEnabled ) throws IOException
{
File authStore = ServerTestUtils.getRelativeFile( GraphDatabaseSettings.auth_store );
FileUtils.deleteFile( authStore);
server = CommunityServerBuilder.server()
.withProperty( GraphDatabaseSettings.auth_enabled.name(), Boolean.toString( authEnabled ) )
.withProperty( GraphDatabaseSettings.auth_store.name(), authStore.getAbsolutePath() )
.build();
server.start();
}
Expand Down
Expand Up @@ -19,22 +19,19 @@
*/
package org.neo4j.server.rest.security;

import java.io.IOException;
import javax.ws.rs.core.HttpHeaders;

import com.sun.jersey.core.util.Base64;
import org.codehaus.jackson.JsonNode;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;

import java.io.File;
import java.io.IOException;
import javax.ws.rs.core.HttpHeaders;

import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.io.fs.FileUtils;
import org.neo4j.kernel.impl.annotations.Documented;
import org.neo4j.server.CommunityNeoServer;
import org.neo4j.server.ServerTestUtils;
import org.neo4j.server.helpers.CommunityServerBuilder;
import org.neo4j.server.rest.RESTDocsGenerator;
import org.neo4j.server.rest.domain.JsonHelper;
Expand Down Expand Up @@ -153,11 +150,8 @@ public void cleanup()

public void startServer(boolean authEnabled) throws IOException
{
File file = ServerTestUtils.getRelativeFile( GraphDatabaseSettings.auth_store );
FileUtils.deleteFile( file );
server = CommunityServerBuilder.server()
.withProperty( GraphDatabaseSettings.auth_enabled.name(), Boolean.toString( authEnabled ) )
.withProperty( GraphDatabaseSettings.auth_store.name(), file.getAbsolutePath() )
.build();
server.start();
}
Expand Down

0 comments on commit c6fb2af

Please sign in to comment.