From 53f1c09f16de10421f2df9fd79d91ec41ff4f6b2 Mon Sep 17 00:00:00 2001 From: Olivia Ytterbrink Date: Thu, 9 Jun 2016 15:11:22 +0200 Subject: [PATCH] Implemented the listRolesForUser procedure --- .../auth/AbstractRoleRepository.java | 6 +- .../enterprise/auth/AuthProcedures.java | 13 +++ .../enterprise/auth/FileUserRealm.java | 2 +- .../security/enterprise/auth/RoleManager.java | 2 + .../enterprise/auth/RoleRepository.java | 2 +- .../enterprise/auth/ShiroAuthManager.java | 11 +++ .../enterprise/auth/AuthProceduresTest.java | 89 +++++++++++++++++++ 7 files changed, 121 insertions(+), 4 deletions(-) diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AbstractRoleRepository.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AbstractRoleRepository.java index 7dc8d95db4a71..b5ff3dd85720f 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AbstractRoleRepository.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AbstractRoleRepository.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; @@ -50,9 +51,10 @@ public RoleRecord findByName( String name ) } @Override - public Set findByUsername( String username ) + public Set findRoleNamesByUsername( String username ) { - return rolesByUsername.get( username ); + Set roleNames = rolesByUsername.get( username ); + return roleNames != null ? roleNames : Collections.emptySet(); } @Override diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java index c6fafd5982f1d..d7680e8c60be8 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java @@ -137,6 +137,19 @@ public Stream listRoles() throws IllegalCredentialsException, IOEx return shiroSubject.getRoleManager().getAllRoleNames().stream().map( StringResult::new ); } + @PerformsDBMS + @Procedure( "dbms.listRolesForUser" ) + public Stream listRolesForUser( @Name( "username" ) String username ) + throws IllegalCredentialsException, IOException + { + ShiroAuthSubject shiroSubject = ShiroAuthSubject.castOrFail( authSubject ); + if ( !shiroSubject.isAdmin() ) + { + throw new AuthorizationViolationException( PERMISSION_DENIED ); + } + return shiroSubject.getRoleManager().getRoleNamesForUser( username ).stream().map( StringResult::new ); + } + public class StringResult { public final String value; diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java index e6aafb3409a6d..31166c6f6adbb 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java @@ -125,7 +125,7 @@ protected AuthorizationInfo doGetAuthorizationInfo( PrincipalCollection principa } else { - Set roles = roleRepository.findByUsername( user.name() ); + Set roles = roleRepository.findRoleNamesByUsername( user.name() ); return new SimpleAuthorizationInfo( roles ); } } diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/RoleManager.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/RoleManager.java index f577360f8f7e8..cc8ca807b2288 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/RoleManager.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/RoleManager.java @@ -45,4 +45,6 @@ public interface RoleManager void removeUserFromRole( String username, String roleName ) throws IOException; Set getAllRoleNames(); + + Set getRoleNamesForUser( String username ); } diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/RoleRepository.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/RoleRepository.java index 2f0604836120e..37f229cf47af4 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/RoleRepository.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/RoleRepository.java @@ -32,7 +32,7 @@ public interface RoleRepository extends Lifecycle { RoleRecord findByName( String name ); - Set findByUsername( String username ); + Set findRoleNamesByUsername( String username ); /** * Create a role, given that the roles token is unique. diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManager.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManager.java index 2437660bbc0cb..6c1534a8ca93d 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManager.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManager.java @@ -238,6 +238,17 @@ public Set getAllRoleNames() return roleRepository.getAllRoleNames(); } + @Override + public Set getRoleNamesForUser( String username ) + { + assertAuthEnabled(); + if (users.findByName( username ) == null) + { + throw new IllegalArgumentException( "User " + username + " does not exist." ); + } + return roleRepository.findRoleNamesByUsername( username ); + } + public Set getAllUsernames() { assertAuthEnabled(); diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java index 615ad1ba4eab2..29e4cac4caf79 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java @@ -757,6 +757,76 @@ public void rolesListing() throws Exception testSuccessfulListRolesAction( subject ); } + @Test + public void shouldListRolesForUser() throws Exception + { + testResult( db, adminSubject, "CALL dbms.listRolesForUser('adminSubject') YIELD value as roles RETURN roles", + r -> { + List roles = getObjectsAsList( r, "roles" ); + Assert.assertThat( roles, containsInAnyOrder( PredefinedRolesBuilder.ADMIN ) ); + assertEquals( 1, roles.size() ); + } ); + } + + @Test + public void shouldNotAllowNonAdminListUserRoles() throws Exception + { + testFailListUserRoles( noneSubject, "adminSubject" ); + testFailListUserRoles( readSubject, "adminSubject" ); + testFailListUserRoles( writeSubject, "adminSubject" ); + testFailListUserRoles( schemaSubject, "adminSubject" ); + } + + @Test + public void shouldFailToListRolesForUnknownUser() throws Exception + { + try + { + testResult( db, adminSubject, "CALL dbms.listRolesForUser('Henrik') YIELD value as roles RETURN roles", + r -> assertFalse( r.hasNext() ) ); + fail( "Expected exception to be thrown" ); + } + catch ( QueryExecutionException e ) + { + assertTrue( "Exception should contain 'User Henrik does not exist.' but was " + e.getMessage(), + e.getMessage().contains( "User Henrik does not exist." ) ); + } + } + + @Test + public void shouldListNoRolesForUserWithNoRoles() throws Exception + { + testCallEmpty( db, adminSubject, "CALL dbms.createUser('Henrik', 'bar', false)", null ); + testResult( db, adminSubject, "CALL dbms.listRolesForUser('Henrik') YIELD value as roles RETURN roles", + r -> assertFalse( r.hasNext() ) ); + } + + /* + Admin creates user Henrik with password bar + Admin creates user Craig with password foo + Admin adds user Craig to role Publisher + Henrik logs in with correct password → ok + Henrik lists all roles for user Craig → permission denied + Admin lists all roles for user Craig → ok + */ + @Test + public void listingUserRoles() throws Exception + { + testCallEmpty( db, adminSubject, "CALL dbms.createUser('Henrik', 'bar', false)", null ); + testCallEmpty( db, adminSubject, "CALL dbms.createUser('Craig', 'foo', false)", null ); + testCallEmpty( db, adminSubject, "CALL dbms.addUserToRole('Craig', '" + PredefinedRolesBuilder.PUBLISHER + "')", + null ); + AuthSubject subject = manager.login( authToken( "Henrik", "bar" ) ); + assertEquals( AuthenticationResult.SUCCESS, subject.getAuthenticationResult() ); + testFailListUserRoles( subject, "Craig" ); + testResult( db, adminSubject, "CALL dbms.listRolesForUser('Craig') YIELD value as roles RETURN roles", + r -> { + List roles = getObjectsAsList( r, "roles" ); + Assert.assertThat( roles, containsInAnyOrder( PredefinedRolesBuilder.PUBLISHER ) ); + assertEquals( 1, roles.size() ); + } ); + } + //-------------Helper functions--------------- private void testSuccessfulReadAction( AuthSubject subject, Long count ) @@ -915,6 +985,25 @@ private void testFailListRoles( AuthSubject subject ) } } + private void testSuccessfulListUserRolesAction( AuthSubject subject, String username ) + { + testCall( db, subject, + "CALL dbms.listRolesForUser('" + username + "') YIELD value AS roles RETURN count(roles)", Map::clear ); + } + + private void testFailListUserRoles( AuthSubject subject, String username ) + { + try + { + testSuccessfulListUserRolesAction( subject, username ); + } + catch ( QueryExecutionException e ) + { + assertTrue( "Exception should contain '" + AuthProcedures.PERMISSION_DENIED + "'", + e.getMessage().contains( AuthProcedures.PERMISSION_DENIED ) ); + } + } + private List getObjectsAsList( Result r, String key ) { return r.stream().map( s -> s.get( key ) ).collect( Collectors.toList() );