From 5993230043b3b08934c8d56b641779422c6f7fbc Mon Sep 17 00:00:00 2001 From: fickludd Date: Tue, 1 Nov 2016 14:42:45 +0100 Subject: [PATCH] More sanity checks on auth config loading --- .../auth/EnterpriseSecurityModule.java | 143 ++++++++++++------ .../auth/EnterpriseSecurityModuleTest.java | 139 +++++++++++------ 2 files changed, 188 insertions(+), 94 deletions(-) diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityModule.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityModule.java index b9df934446d2..7592172a0064 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityModule.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityModule.java @@ -128,39 +128,35 @@ private EnterpriseSecurityContext asEnterprise( SecurityContext securityContext public EnterpriseAuthAndUserManager newAuthManager( Config config, LogProvider logProvider, SecurityLog securityLog, FileSystemAbstraction fileSystem, JobScheduler jobScheduler ) { - List configuredRealms = config.get( SecuritySettings.auth_providers ); - List realms = new ArrayList<>( configuredRealms.size() + 1 ); + SecurityConfig securityConfig = new SecurityConfig( config ); + securityConfig.validate(); + List realms = new ArrayList<>( securityConfig.authProviders.size() + 1 ); SecureHasher secureHasher = new SecureHasher(); InternalFlatFileRealm internalRealm = null; - - if ( config.get( SecuritySettings.native_authentication_enabled ) || - config.get( SecuritySettings.native_authorization_enabled ) ) + if ( securityConfig.hasNativeProvider ) { internalRealm = createInternalRealm( config, logProvider, fileSystem, jobScheduler ); realms.add( internalRealm ); } - if ( ( config.get( SecuritySettings.ldap_authentication_enabled ) || - config.get( SecuritySettings.ldap_authorization_enabled ) ) - && configuredRealms.contains( SecuritySettings.LDAP_REALM_NAME ) ) + if ( securityConfig.hasLdapProvider ) { realms.add( new LdapRealm( config, securityLog, secureHasher ) ); } - // Load plugin realms if a plugin provider in configured - if ( configuredRealms.stream().anyMatch( ( r ) -> r.startsWith( SecuritySettings.PLUGIN_REALM_NAME_PREFIX ) ) ) + if ( securityConfig.hasPluginProvider ) { - realms.addAll( createPluginRealms( config, securityLog, secureHasher, configuredRealms ) ); + realms.addAll( createPluginRealms( config, securityLog, secureHasher, securityConfig ) ); } // Select the active realms in the order they are configured - List orderedActiveRealms = selectOrderedActiveRealms( configuredRealms, realms ); + List orderedActiveRealms = selectOrderedActiveRealms( securityConfig.authProviders, realms ); if ( orderedActiveRealms.isEmpty() ) { - throw new IllegalArgumentException( "Illegal configuration: No valid auth provider is active." ); + throw illegalConf( "No valid auth provider is active." ); } return new MultiRealmAuthManager( internalRealm, orderedActiveRealms, createCacheManager( config ), @@ -208,20 +204,14 @@ private static CacheManager createCacheManager( Config config ) } private static List createPluginRealms( - Config config, SecurityLog securityLog, SecureHasher secureHasher, List configuredRealms ) + Config config, SecurityLog securityLog, SecureHasher secureHasher, SecurityConfig securityConfig ) { List availablePluginRealms = new ArrayList<>(); Set excludedClasses = new HashSet<>(); - Boolean pluginAuthenticationEnabled = config.get( SecuritySettings.plugin_authentication_enabled ); - Boolean pluginAuthorizationEnabled = config.get( SecuritySettings.plugin_authorization_enabled ); - - if ( pluginAuthenticationEnabled && pluginAuthorizationEnabled ) + if ( securityConfig.pluginAuthentication && securityConfig.pluginAuthorization ) { - // Combined authentication and authorization plugins - Iterable authPlugins = Service.load( AuthPlugin.class ); - - for ( AuthPlugin plugin : authPlugins ) + for ( AuthPlugin plugin : Service.load( AuthPlugin.class ) ) { PluginRealm pluginRealm = new PluginRealm( plugin, config, securityLog, Clocks.systemClock(), secureHasher ); @@ -229,16 +219,13 @@ private static List createPluginRealms( } } - if ( pluginAuthenticationEnabled ) + if ( securityConfig.pluginAuthentication ) { - // Authentication only plugins - Iterable authenticationPlugins = Service.load( AuthenticationPlugin.class ); - - for ( AuthenticationPlugin plugin : authenticationPlugins ) + for ( AuthenticationPlugin plugin : Service.load( AuthenticationPlugin.class ) ) { PluginRealm pluginRealm; - if ( pluginAuthorizationEnabled && plugin instanceof AuthorizationPlugin ) + if ( securityConfig.pluginAuthorization && plugin instanceof AuthorizationPlugin ) { // This plugin implements both interfaces, create a combined plugin pluginRealm = new PluginRealm( plugin, (AuthorizationPlugin) plugin, config, securityLog, @@ -257,39 +244,37 @@ private static List createPluginRealms( } } - if ( pluginAuthorizationEnabled ) + if ( securityConfig.pluginAuthorization ) { - // Authorization only plugins - Iterable authorizationPlugins = Service.load( AuthorizationPlugin.class ); - - for ( AuthorizationPlugin plugin : authorizationPlugins ) + for ( AuthorizationPlugin plugin : Service.load( AuthorizationPlugin.class ) ) { if ( !excludedClasses.contains( plugin.getClass() ) ) { - PluginRealm pluginRealm = - new PluginRealm( null, plugin, config, securityLog, Clocks.systemClock(), secureHasher ); - availablePluginRealms.add( pluginRealm ); + availablePluginRealms.add( + new PluginRealm( null, plugin, config, securityLog, Clocks.systemClock(), secureHasher ) + ); } } } List realms = availablePluginRealms.stream() - .filter( realm -> configuredRealms.contains( realm.getName() ) ) + .filter( realm -> securityConfig.authProviders.contains( realm.getName() ) ) .collect( Collectors.toList() ); - boolean missingAuthenticationRealm = pluginAuthenticationEnabled && - !realms.stream().anyMatch( PluginRealm::canAuthenticate ); - boolean missingAuthorizingRealm = pluginAuthorizationEnabled && - !realms.stream().anyMatch( PluginRealm::canAuthorize ); - if ( missingAuthenticationRealm || missingAuthorizingRealm ) + boolean missingAuthenticatingRealm = + securityConfig.pluginAuthentication && !realms.stream().anyMatch( PluginRealm::canAuthenticate ); + boolean missingAuthorizingRealm = + securityConfig.pluginAuthorization && !realms.stream().anyMatch( PluginRealm::canAuthorize ); + + if ( missingAuthenticatingRealm || missingAuthorizingRealm ) { String missingProvider = - ( missingAuthenticationRealm && missingAuthorizingRealm ) ? "authentication or authorization" : - ( missingAuthenticationRealm ) ? "authentication" : "authorization"; + ( missingAuthenticatingRealm && missingAuthorizingRealm ) ? "authentication or authorization" : + ( missingAuthenticatingRealm ) ? "authentication" : "authorization"; - throw new IllegalArgumentException( format( "Illegal configuration: No plugin %s provider loaded even " + - "though required by configuration.", missingProvider ) ); + throw illegalConf( format( + "No plugin %s provider loaded even though required by configuration.", missingProvider ) ); } return realms; @@ -317,4 +302,70 @@ public static File getDefaultAdminRepositoryFile( Config config ) return new File( config.get( DatabaseManagementSystemSettings.auth_store_directory ), DEFAULT_ADMIN_STORE_FILENAME ); } + + private static IllegalArgumentException illegalConf( String message ) + { + return new IllegalArgumentException( "Illegal configuration: " + message ); + } + + class SecurityConfig + { + final List authProviders; + final boolean hasNativeProvider; + final boolean hasLdapProvider; + final boolean hasPluginProvider; + final boolean nativeAuthentication; + final boolean nativeAuthorization; + final boolean ldapAuthentication; + final boolean ldapAuthorization; + final boolean pluginAuthentication; + final boolean pluginAuthorization; + + SecurityConfig( Config config ) + { + authProviders = config.get( SecuritySettings.auth_providers ); + hasNativeProvider = authProviders.contains( SecuritySettings.NATIVE_REALM_NAME ); + hasLdapProvider = authProviders.contains( SecuritySettings.LDAP_REALM_NAME ); + hasPluginProvider = authProviders.stream() + .anyMatch( ( r ) -> r.startsWith( SecuritySettings.PLUGIN_REALM_NAME_PREFIX ) ); + + nativeAuthentication = config.get( SecuritySettings.native_authentication_enabled ); + nativeAuthorization = config.get( SecuritySettings.native_authorization_enabled ); + ldapAuthentication = config.get( SecuritySettings.ldap_authentication_enabled ); + ldapAuthorization = config.get( SecuritySettings.ldap_authorization_enabled ); + pluginAuthentication = config.get( SecuritySettings.plugin_authentication_enabled ); + pluginAuthorization = config.get( SecuritySettings.plugin_authorization_enabled ); + } + + void validate() + { + if ( !nativeAuthentication && !ldapAuthentication && !pluginAuthentication ) + { + throw illegalConf( "All authentication providers are disabled." ); + } + + if ( !nativeAuthorization && !ldapAuthorization && !pluginAuthorization ) + { + throw illegalConf( "All authorization providers are disabled." ); + } + + if ( hasNativeProvider && !nativeAuthentication && !nativeAuthorization ) + { + throw illegalConf( + "Native auth provider configured, but both authentication and authorization are disabled." ); + } + + if ( hasLdapProvider && !ldapAuthentication && !ldapAuthorization ) + { + throw illegalConf( + "LDAP auth provider configured, but both authentication and authorization are disabled." ); + } + + if ( hasPluginProvider && !pluginAuthentication && !pluginAuthorization ) + { + throw illegalConf( + "Plugin auth provider configured, but both authentication and authorization are disabled." ); + } + } + } } diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityModuleTest.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityModuleTest.java index 39f2882c62ec..91186ba608dc 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityModuleTest.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityModuleTest.java @@ -19,6 +19,7 @@ */ package org.neo4j.server.security.enterprise.auth; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -39,23 +40,17 @@ public class EnterpriseSecurityModuleTest { @Rule public ExpectedException thrown = ExpectedException.none(); + private Config config; + private LogProvider mockLogProvider; @Test public void shouldFailOnIllegalRealmNameConfiguration() { // Given - Config config = mock( Config.class ); - LogProvider mockLogProvider = mock( LogProvider.class ); - Log mockLog = mock( Log.class ); - when( mockLogProvider.getLog( anyString() ) ).thenReturn( mockLog ); - when( mockLog.isDebugEnabled() ).thenReturn( true ); - when( config.get( SecuritySettings.native_authentication_enabled ) ).thenReturn( true ); - when( config.get( SecuritySettings.native_authorization_enabled ) ).thenReturn( true ); - when( config.get( SecuritySettings.ldap_authentication_enabled ) ).thenReturn( true ); - when( config.get( SecuritySettings.ldap_authorization_enabled ) ).thenReturn( true ); - when( config.get( SecuritySettings.plugin_authentication_enabled ) ).thenReturn( false ); - when( config.get( SecuritySettings.plugin_authorization_enabled ) ).thenReturn( false ); - when( config.get( SecuritySettings.auth_providers ) ).thenReturn( Arrays.asList( "this-realm-does-not-exist" ) ); + nativeAuth( true, true ); + ldapAuth( true, true ); + pluginAuth( false, false ); + authProviders( "this-realm-does-not-exist" ); // Then thrown.expect( IllegalArgumentException.class ); @@ -65,30 +60,53 @@ public void shouldFailOnIllegalRealmNameConfiguration() new EnterpriseSecurityModule().newAuthManager( config, mockLogProvider, mock( SecurityLog.class), null, null ); } + @Test + public void shouldFailOnNoAuthenticationMechanism() + { + // Given + nativeAuth( false, true ); + ldapAuth( false, false ); + pluginAuth( false, false ); + authProviders( SecuritySettings.NATIVE_REALM_NAME ); + + // Then + thrown.expect( IllegalArgumentException.class ); + thrown.expectMessage( "Illegal configuration: All authentication providers are disabled." ); + + // When + new EnterpriseSecurityModule().newAuthManager( config, mockLogProvider, mock( SecurityLog.class), null, null ); + } + + @Test + public void shouldFailOnNoAuthorizationMechanism() + { + // Given + nativeAuth( true, false ); + ldapAuth( false, false ); + pluginAuth( false, false ); + authProviders( SecuritySettings.NATIVE_REALM_NAME ); + + // Then + thrown.expect( IllegalArgumentException.class ); + thrown.expectMessage( "Illegal configuration: All authorization providers are disabled." ); + + // When + new EnterpriseSecurityModule().newAuthManager( config, mockLogProvider, mock( SecurityLog.class), null, null ); + } + @Test public void shouldFailOnIllegalAdvancedRealmConfiguration() { // Given - Config config = mock( Config.class ); - LogProvider mockLogProvider = mock( LogProvider.class ); - Log mockLog = mock( Log.class ); - when( mockLogProvider.getLog( anyString() ) ).thenReturn( mockLog ); - when( mockLog.isDebugEnabled() ).thenReturn( true ); - when( config.get( SecuritySettings.native_authentication_enabled ) ).thenReturn( false ); - when( config.get( SecuritySettings.native_authorization_enabled ) ).thenReturn( false ); - when( config.get( SecuritySettings.ldap_authentication_enabled ) ).thenReturn( false ); - when( config.get( SecuritySettings.ldap_authorization_enabled ) ).thenReturn( false ); - when( config.get( SecuritySettings.plugin_authentication_enabled ) ).thenReturn( true ); - when( config.get( SecuritySettings.plugin_authorization_enabled ) ).thenReturn( true ); - when( config.get( SecuritySettings.auth_providers ) ).thenReturn( - Arrays.asList( - SecuritySettings.NATIVE_REALM_NAME, - SecuritySettings.LDAP_REALM_NAME ) - ); + nativeAuth( false, false ); + ldapAuth( false, false ); + pluginAuth( true, true ); + authProviders( SecuritySettings.NATIVE_REALM_NAME, SecuritySettings.LDAP_REALM_NAME ); // Then thrown.expect( IllegalArgumentException.class ); - thrown.expectMessage( "Illegal configuration: No valid auth provider is active." ); + thrown.expectMessage( "Illegal configuration: Native auth provider configured, " + + "but both authentication and authorization are disabled." ); // When new EnterpriseSecurityModule().newAuthManager( config, mockLogProvider, mock( SecurityLog.class), null, null ); @@ -98,33 +116,58 @@ public void shouldFailOnIllegalAdvancedRealmConfiguration() public void shouldFailOnNotLoadedPluginAuthProvider() { // Given - Config config = mock( Config.class ); - LogProvider mockLogProvider = mock( LogProvider.class ); + nativeAuth( false, false ); + ldapAuth( false, false ); + pluginAuth( true, true ); + authProviders( + SecuritySettings.PLUGIN_REALM_NAME_PREFIX + "TestAuthenticationPlugin", + SecuritySettings.PLUGIN_REALM_NAME_PREFIX + "IllConfiguredAuthorizationPlugin" + ); + + // Then + thrown.expect( IllegalArgumentException.class ); + thrown.expectMessage( "Illegal configuration: No plugin authorization provider loaded even though required by " + + "configuration." ); + + // When + new EnterpriseSecurityModule().newAuthManager( config, mockLogProvider, mock( SecurityLog.class), null, null ); + } + + // --------- HELPERS ---------- + + @Before + public void setup() + { + config = mock( Config.class ); + mockLogProvider = mock( LogProvider.class ); Log mockLog = mock( Log.class ); when( mockLogProvider.getLog( anyString() ) ).thenReturn( mockLog ); when( mockLog.isDebugEnabled() ).thenReturn( true ); when( config.get( SecuritySettings.auth_cache_ttl ) ).thenReturn( 0L ); when( config.get( SecuritySettings.auth_cache_max_capacity ) ).thenReturn( 10 ); when( config.get( SecuritySettings.security_log_successful_authentication ) ).thenReturn( false ); + } - when( config.get( SecuritySettings.native_authentication_enabled ) ).thenReturn( false ); - when( config.get( SecuritySettings.native_authorization_enabled ) ).thenReturn( false ); - when( config.get( SecuritySettings.ldap_authentication_enabled ) ).thenReturn( false ); - when( config.get( SecuritySettings.ldap_authorization_enabled ) ).thenReturn( false ); - when( config.get( SecuritySettings.plugin_authentication_enabled ) ).thenReturn( true ); - when( config.get( SecuritySettings.plugin_authorization_enabled ) ).thenReturn( true ); - when( config.get( SecuritySettings.auth_providers ) ).thenReturn( - Arrays.asList( - SecuritySettings.PLUGIN_REALM_NAME_PREFIX + "TestAuthenticationPlugin", - SecuritySettings.PLUGIN_REALM_NAME_PREFIX + "IllConfiguredAuthorizationPlugin" - ) ); + private void nativeAuth( boolean authn, boolean authr ) + { + when( config.get( SecuritySettings.native_authentication_enabled ) ).thenReturn( authn ); + when( config.get( SecuritySettings.native_authorization_enabled ) ).thenReturn( authr ); + } - // Then - thrown.expect( IllegalArgumentException.class ); - thrown.expectMessage( "Illegal configuration: No plugin authorization provider loaded even though required by " + - "configuration." ); + private void ldapAuth( boolean authn, boolean authr ) + { + when( config.get( SecuritySettings.ldap_authentication_enabled ) ).thenReturn( authn ); + when( config.get( SecuritySettings.ldap_authorization_enabled ) ).thenReturn( authr ); + } - // When - new EnterpriseSecurityModule().newAuthManager( config, mockLogProvider, mock( SecurityLog.class), null, null ); + private void pluginAuth( boolean authn, boolean authr ) + { + when( config.get( SecuritySettings.plugin_authentication_enabled ) ).thenReturn( authn ); + when( config.get( SecuritySettings.plugin_authorization_enabled ) ).thenReturn( authr ); + } + + private void authProviders( String... authProviders ) + { + when( config.get( SecuritySettings.auth_providers ) ).thenReturn( Arrays.asList( authProviders ) ); } }