diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ProcedureAllowedConfig.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ProcedureAllowedConfig.java index ee21ba7ef3f05..1f9a965ba1d1d 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ProcedureAllowedConfig.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ProcedureAllowedConfig.java @@ -28,11 +28,14 @@ import org.neo4j.kernel.configuration.Config; +import static java.util.Arrays.stream; + public class ProcedureAllowedConfig { public static final String PROC_ALLOWED_SETTING_DEFAULT_NAME = "dbms.security.procedures.default_allowed"; public static final String PROC_ALLOWED_SETTING_ROLES = "dbms.security.procedures.roles"; + private static final String ROLES_DELIMITER = ","; private static final String SETTING_DELIMITER = ";"; private static final String MAPPING_DELIMITER = ":"; @@ -52,9 +55,12 @@ public ProcedureAllowedConfig( Config config ) if ( params.containsKey( PROC_ALLOWED_SETTING_ROLES ) ) { this.matchers = Stream.of( params.get( PROC_ALLOWED_SETTING_ROLES ).split( SETTING_DELIMITER ) ) - .map( procToRoleSpec -> { + .map( procToRoleSpec -> + { String[] spec = procToRoleSpec.split( MAPPING_DELIMITER ); - return new ProcMatcher( spec[0].trim(), spec[1].trim() ); + String[] roles = stream( spec[1].split( ROLES_DELIMITER ) ) + .map( String::trim ).toArray( String[]::new ); + return new ProcMatcher( spec[0].trim(), roles ); } ).collect( Collectors.toList() ); } else @@ -65,9 +71,9 @@ public ProcedureAllowedConfig( Config config ) String[] rolesFor( String procedureName ) { - String[] wildCardRoles = - matchers.stream().filter( matcher -> matcher.matches( procedureName ) ).map( ProcMatcher::role ) - .toArray( String[]::new ); + String[] wildCardRoles = matchers.stream().filter( matcher -> matcher.matches( procedureName ) ) + .map( ProcMatcher::roles ).reduce( new String[0], + ( acc, next ) -> Stream.concat( stream( acc ), stream( next ) ).toArray( String[]::new ) ); if ( wildCardRoles.length > 0 ) { return wildCardRoles; @@ -80,7 +86,7 @@ String[] rolesFor( String procedureName ) private String[] getDefaultValue() { - return defaultValue == null || defaultValue.isEmpty() ? new String[0]: new String[]{defaultValue}; + return defaultValue == null || defaultValue.isEmpty() ? new String[0] : new String[]{defaultValue}; } static final ProcedureAllowedConfig DEFAULT = new ProcedureAllowedConfig(); @@ -88,12 +94,12 @@ private String[] getDefaultValue() private static class ProcMatcher { private final Pattern pattern; - private final String role; + private final String[] roles; - private ProcMatcher(String procedurePattern, String role) + private ProcMatcher( String procedurePattern, String[] roles ) { this.pattern = Pattern.compile( procedurePattern.replaceAll( "\\.", "\\\\." ).replaceAll( "\\*", ".*" ) ); - this.role = role; + this.roles = roles; } boolean matches( String procedureName ) @@ -101,9 +107,9 @@ boolean matches( String procedureName ) return pattern.matcher( procedureName ).matches(); } - String role() + String[] roles() { - return role; + return roles; } } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/proc/ProcedureAllowedConfigTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/proc/ProcedureAllowedConfigTest.java index c2e0b88f34401..6924154026063 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/proc/ProcedureAllowedConfigTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/proc/ProcedureAllowedConfigTest.java @@ -26,6 +26,8 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; import static org.neo4j.helpers.collection.MapUtil.genericMap; +import static org.neo4j.kernel.impl.proc.ProcedureAllowedConfig.PROC_ALLOWED_SETTING_DEFAULT_NAME; +import static org.neo4j.kernel.impl.proc.ProcedureAllowedConfig.PROC_ALLOWED_SETTING_ROLES; public class ProcedureAllowedConfigTest { @@ -48,7 +50,7 @@ public void shouldHaveEmptyDefaultConfigs() public void shouldHaveConfigsWithDefaultProcedureAllowed() { Config config = Config.defaults() - .with( genericMap( ProcedureAllowedConfig.PROC_ALLOWED_SETTING_DEFAULT_NAME, "role1" ) ); + .with( genericMap( PROC_ALLOWED_SETTING_DEFAULT_NAME, "role1" ) ); ProcedureAllowedConfig procConfig = new ProcedureAllowedConfig( config ); assertThat( procConfig.rolesFor( "x" ), equalTo( arrayOf( "role1" ) ) ); } @@ -57,8 +59,8 @@ public void shouldHaveConfigsWithDefaultProcedureAllowed() public void shouldHaveConfigsWithExactMatchProcedureAllowed() { Config config = Config.defaults() - .with( genericMap( ProcedureAllowedConfig.PROC_ALLOWED_SETTING_DEFAULT_NAME, "role1", - ProcedureAllowedConfig.PROC_ALLOWED_SETTING_ROLES, "xyz:anotherRole" ) ); + .with( genericMap( PROC_ALLOWED_SETTING_DEFAULT_NAME, "role1", + PROC_ALLOWED_SETTING_ROLES, "xyz:anotherRole" ) ); ProcedureAllowedConfig procConfig = new ProcedureAllowedConfig( config ); assertThat( procConfig.rolesFor( "xyz" ), equalTo( arrayOf( "anotherRole" ) ) ); assertThat( procConfig.rolesFor( "abc" ), equalTo( arrayOf( "role1" ) ) ); @@ -68,8 +70,8 @@ public void shouldHaveConfigsWithExactMatchProcedureAllowed() public void shouldHaveConfigsWithWildcardProcedureAllowed() { Config config = Config.defaults() - .with( genericMap( ProcedureAllowedConfig.PROC_ALLOWED_SETTING_DEFAULT_NAME, "role1", - ProcedureAllowedConfig.PROC_ALLOWED_SETTING_ROLES, "xyz*:anotherRole" ) ); + .with( genericMap( PROC_ALLOWED_SETTING_DEFAULT_NAME, "role1", + PROC_ALLOWED_SETTING_ROLES, "xyz*:anotherRole" ) ); ProcedureAllowedConfig procConfig = new ProcedureAllowedConfig( config ); assertThat( procConfig.rolesFor( "xyzabc" ), equalTo( arrayOf( "anotherRole" ) ) ); assertThat( procConfig.rolesFor( "abcxyz" ), equalTo( arrayOf( "role1" ) ) ); @@ -79,7 +81,7 @@ public void shouldHaveConfigsWithWildcardProcedureAllowed() public void shouldHaveConfigsWithWildcardProcedureAllowedAndNoDefault() { Config config = Config.defaults() - .with( genericMap( ProcedureAllowedConfig.PROC_ALLOWED_SETTING_ROLES, "xyz*:anotherRole" ) ); + .with( genericMap( PROC_ALLOWED_SETTING_ROLES, "xyz*:anotherRole" ) ); ProcedureAllowedConfig procConfig = new ProcedureAllowedConfig( config ); assertThat( procConfig.rolesFor( "xyzabc" ), equalTo( arrayOf( "anotherRole" ) ) ); assertThat( procConfig.rolesFor( "abcxyz" ), equalTo( EMPTY ) ); @@ -90,7 +92,7 @@ public void shouldHaveConfigsWithMultipleWildcardProcedureAllowedAndNoDefault() { Config config = Config.defaults() .with( genericMap( - ProcedureAllowedConfig.PROC_ALLOWED_SETTING_ROLES, + PROC_ALLOWED_SETTING_ROLES, "apoc.convert.*:apoc_reader;apoc.load.json:apoc_writer;apoc.trigger.add:TriggerHappy" ) ); ProcedureAllowedConfig procConfig = new ProcedureAllowedConfig( config ); @@ -111,8 +113,8 @@ public void shouldHaveConfigsWithOverlappingMatchingWildcards() { Config config = Config.defaults() .with( genericMap( - ProcedureAllowedConfig.PROC_ALLOWED_SETTING_DEFAULT_NAME, "default", - ProcedureAllowedConfig.PROC_ALLOWED_SETTING_ROLES, + PROC_ALLOWED_SETTING_DEFAULT_NAME, "default", + PROC_ALLOWED_SETTING_ROLES, "apoc.*:apoc;apoc.load.*:loader;apoc.trigger.*:trigger;apoc.trigger.add:TriggerHappy" ) ); ProcedureAllowedConfig procConfig = new ProcedureAllowedConfig( config ); @@ -123,4 +125,16 @@ public void shouldHaveConfigsWithOverlappingMatchingWildcards() assertThat( procConfig.rolesFor( "apoc.trigger.remove" ), equalTo( arrayOf( "apoc", "trigger" ) ) ); assertThat( procConfig.rolesFor( "apoc.load-xml" ), equalTo( arrayOf( "apoc" ) ) ); } + + @Test + public void shouldSupportSeveralRolesPerPattern() + { + Config config = Config.defaults().with( genericMap( PROC_ALLOWED_SETTING_ROLES, + "xyz*:role1,role2, role3 , role4 ; abc: role3 ,role1" ) ); + ProcedureAllowedConfig procConfig = new ProcedureAllowedConfig( config ); + + assertThat( procConfig.rolesFor( "xyzabc" ), equalTo( arrayOf( "role1", "role2", "role3", "role4" ) ) ); + assertThat( procConfig.rolesFor( "abc" ), equalTo( arrayOf( "role3", "role1" ) ) ); + assertThat( procConfig.rolesFor( "abcxyz" ), equalTo( EMPTY ) ); + } } diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/ConfiguredProceduresTestBase.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/ConfiguredProceduresTestBase.java index 097f8550e521f..9d85f25e4517b 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/ConfiguredProceduresTestBase.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/ConfiguredProceduresTestBase.java @@ -205,6 +205,21 @@ public void shouldHandleWriteAfterAllowedReadProcedureWithAuthDisabled() throws .registerProcedure( ClassWithProcedures.class ); S subject = neo.login( "no_auth", "" ); - assertEmpty( subject, "CALL test.allowedReadProcedure() YIELD value CREATE (:NEWNODE {name:value})" ); + assertEmpty( subject, "CALL test.allowedReadProcedure() YIELD value CREATE (:NewNode {name: value})" ); + } + + @Test + public void shouldHandleMultipleRolesSpecifiedForMapping() throws Throwable + { + // Given + configuredSetup( stringMap( SecuritySettings.procedure_roles.name(), "test.*:tester, other" ) ); + + // When + userManager.newRole( "tester", "noneSubject" ); + userManager.newRole( "other", "readSubject" ); + + // Then + assertSuccess( readSubject, "CALL test.createNode", ResourceIterator::close ); + assertSuccess( noneSubject, "CALL test.numNodes", itr -> assertKeyIs( itr, "count", "4" ) ); } }