Skip to content

Commit

Permalink
Renamed role-user procs to demarcate the user as the primary entity
Browse files Browse the repository at this point in the history
  • Loading branch information
@craigtaverner
Petra Selmer authored and craigtaverner committed Aug 25, 2016
1 parent 0d39b7a commit 11054b4
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 119 deletions.
12 changes: 6 additions & 6 deletions ...rise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java
View file
Expand Up @@ -86,16 +86,16 @@ else if ( !enterpriseSubject.isAdmin() )
}
}

@Procedure( name = "dbms.addUserToRole", mode = DBMS )
public void addUserToRole( @Name( "username" ) String username, @Name( "roleName" ) String roleName )
@Procedure( name = "dbms.addRoleToUser", mode = DBMS )
public void addRoleToUser(@Name( "roleName" ) String roleName, @Name( "username" ) String username )
throws IOException, InvalidArgumentsException
{
EnterpriseAuthSubject adminSubject = ensureAdminAuthSubject();
adminSubject.getUserManager().addUserToRole( username, roleName );
adminSubject.getUserManager().addRoleToUser( roleName, username );
}

@Procedure( name = "dbms.removeUserFromRole", mode = DBMS )
public void removeUserFromRole( @Name( "username" ) String username, @Name( "roleName" ) String roleName )
@Procedure( name = "dbms.removeRoleFromUser", mode = DBMS )
public void removeRoleFromUser( @Name( "roleName" ) String roleName, @Name( "username" ) String username )
throws InvalidArgumentsException, IOException
{
EnterpriseAuthSubject adminSubject = ensureAdminAuthSubject();
Expand All @@ -104,7 +104,7 @@ public void removeUserFromRole( @Name( "username" ) String username, @Name( "rol
throw new InvalidArgumentsException( "Removing yourself (user '" + username +
"') from the admin role is not allowed." );
}
adminSubject.getUserManager().removeUserFromRole( username, roleName );
adminSubject.getUserManager().removeRoleFromUser( roleName, username );
}

@Procedure( name = "dbms.deleteUser", mode = DBMS )
Expand Down
12 changes: 6 additions & 6 deletions ...curity/src/main/java/org/neo4j/server/security/enterprise/auth/EnterpriseUserManager.java
View file
Expand Up @@ -36,24 +36,24 @@ public interface EnterpriseUserManager extends UserManager
RoleRecord getRole( String roleName ) throws InvalidArgumentsException;

/**
* Add a user to a role. The role has to exist.
* Assign a role to a user. The role and the user have to exist.
*
* @param username name of user
* @param roleName name of role
* @param username name of user
* @throws InvalidArgumentsException if the role does not exist
* @throws IOException
*/
void addUserToRole( String username, String roleName ) throws IOException, InvalidArgumentsException;
void addRoleToUser( String roleName, String username ) throws IOException, InvalidArgumentsException;

/**
* Remove a user from a role.
* Unassign a role from a user. The role and the user have to exist.
*
* @param username name of user
* @param roleName name of role
* @param username name of user
* @throws InvalidArgumentsException if the username or the role does not exist
* @throws IOException
*/
void removeUserFromRole( String username, String roleName ) throws IOException, InvalidArgumentsException;
void removeRoleFromUser( String roleName, String username ) throws IOException, InvalidArgumentsException;

Set<String> getAllRoleNames();

Expand Down
22 changes: 9 additions & 13 deletions ...curity/src/main/java/org/neo4j/server/security/enterprise/auth/InternalFlatFileRealm.java
View file
Expand Up @@ -164,7 +164,7 @@ private void ensureDefaultRoles() throws IOException, InvalidArgumentsException
{
if ( getAllUsernames().contains( "neo4j" ) )
{
addUserToRole( "neo4j", PredefinedRolesBuilder.ADMIN );
addRoleToUser( PredefinedRolesBuilder.ADMIN, "neo4j" );
}
}
}
Expand Down Expand Up @@ -346,9 +346,10 @@ public RoleRecord getRole( String roleName ) throws InvalidArgumentsException
}

@Override
public void addUserToRole( String username, String roleName ) throws IOException, InvalidArgumentsException
public void addRoleToUser( String roleName, String username ) throws IOException, InvalidArgumentsException
{
checkValidityOfUsernameAndRoleName( username, roleName );
assertValidRoleName( roleName );
assertValidUsername( username );

synchronized ( this )
{
Expand All @@ -362,16 +363,17 @@ public void addUserToRole( String username, String roleName ) throws IOException
catch ( ConcurrentModificationException e )
{
// Try again
addUserToRole( username, roleName );
addRoleToUser( roleName, username );
}
}
clearCachedAuthorizationInfoForUser( username );
}

@Override
public void removeUserFromRole( String username, String roleName ) throws IOException, InvalidArgumentsException
public void removeRoleFromUser( String roleName, String username ) throws IOException, InvalidArgumentsException
{
checkValidityOfUsernameAndRoleName( username, roleName );
assertValidRoleName( roleName );
assertValidUsername( username );

synchronized ( this )
{
Expand All @@ -386,7 +388,7 @@ public void removeUserFromRole( String username, String roleName ) throws IOExce
catch ( ConcurrentModificationException e )
{
// Try again
removeUserFromRole( username, roleName );
removeRoleFromUser( roleName, username );
}
}
clearCachedAuthorizationInfoForUser( username );
Expand Down Expand Up @@ -536,12 +538,6 @@ private void removeUserFromAllRoles( String username ) throws IOException
}
}

private void checkValidityOfUsernameAndRoleName( String username, String roleName ) throws InvalidArgumentsException
{
assertValidUsername( username );
assertValidRoleName( roleName );
}

private void assertValidUsername( String name ) throws InvalidArgumentsException
{
if ( name.isEmpty() )
Expand Down
92 changes: 47 additions & 45 deletions ...rity/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTestLogic.java
View file
Expand Up @@ -80,8 +80,8 @@ public void shouldChangeOwnPasswordEvenIfHasNoAuthorization() throws Throwable
@Test
public void shouldNotChangeOwnPasswordIfNewPasswordInvalid() throws Exception
{
assertFail( readSubject, "CALL dbms.changePassword( '' )", "A password cannot be empty." );
assertFail( readSubject, "CALL dbms.changePassword( '123' )",
assertFail( readSubject, "CALL dbms.security.changePassword( '' )", "A password cannot be empty." );
assertFail( readSubject, "CALL dbms.security.changePassword( '123' )",
"Old password and new password cannot be the same." );
}

Expand Down Expand Up @@ -315,7 +315,8 @@ public void shouldNotChangeUserPasswordIfNonExistentUser() throws Exception
@Test
public void shouldNotChangeUserPasswordIfEmptyPassword() throws Exception
{
assertFail( adminSubject, "CALL dbms.changeUserPassword( 'readSubject', '' )", "A password cannot be empty." );
assertFail( adminSubject, "CALL dbms.security.changeUserPassword( 'readSubject', '' )",
"A password cannot be empty." );
}

// Should fail to change password for admin subject and same password
Expand Down Expand Up @@ -406,7 +407,7 @@ public void shouldDeleteUser() throws Exception
e.getMessage().contains( "User 'noneSubject' does not exist." ) );
}

userManager.addUserToRole( "readSubject", PUBLISHER );
userManager.addRoleToUser( PUBLISHER, "readSubject" );
assertEmpty( adminSubject, "CALL dbms.deleteUser('readSubject')" );
try
{
Expand Down Expand Up @@ -571,99 +572,100 @@ public void shouldFailToActivateYourself() throws Exception
//---------- add user to role -----------

@Test
public void shouldAddUserToRole() throws Exception
public void shouldAddRoleToUser() throws Exception
{
assertFalse( "Should not have role publisher", userHasRole( "readSubject", PUBLISHER ) );
assertEmpty( adminSubject, "CALL dbms.addUserToRole('readSubject', '" + PUBLISHER + "')" );
assertEmpty( adminSubject, "CALL dbms.addRoleToUser('" + PUBLISHER + "', 'readSubject' )" );
assertTrue( "Should have role publisher", userHasRole( "readSubject", PUBLISHER ) );
}

@Test
public void shouldAddRetainUserInRole() throws Exception
{
assertTrue( "Should have role reader", userHasRole( "readSubject", READER ) );
assertEmpty( adminSubject, "CALL dbms.addUserToRole('readSubject', '" + READER + "')" );
assertEmpty( adminSubject, "CALL dbms.addRoleToUser('" + READER + "', 'readSubject')" );
assertTrue( "Should have still have role reader", userHasRole( "readSubject", READER ) );
}

@Test
public void shouldFailToAddNonExistentUserToRole() throws Exception
{
testFailAddUserToRole( adminSubject, "Olivia", PUBLISHER, "User 'Olivia' does not exist." );
testFailAddUserToRole( adminSubject, "Olivia", "thisRoleDoesNotExist", "User 'Olivia' does not exist." );
testFailAddUserToRole( adminSubject, "Olivia", "", "The provided role name is empty." );
testFailAddRoleToUser( adminSubject, PUBLISHER, "Olivia", "User 'Olivia' does not exist." );
testFailAddRoleToUser( adminSubject, "thisRoleDoesNotExist", "Olivia", "User 'Olivia' does not exist." );
testFailAddRoleToUser( adminSubject, "", "Olivia", "The provided role name is empty." );
}

@Test
public void shouldFailToAddUserToNonExistentRole() throws Exception
{
testFailAddUserToRole( adminSubject, "readSubject", "thisRoleDoesNotExist",
testFailAddRoleToUser( adminSubject, "thisRoleDoesNotExist", "readSubject",
"Role 'thisRoleDoesNotExist' does not exist." );
testFailAddUserToRole( adminSubject, "readSubject", "", "The provided role name is empty." );
testFailAddRoleToUser( adminSubject, "", "readSubject", "The provided role name is empty." );
}

@Test
public void shouldFailToAddUserToRoleIfNotAdmin() throws Exception
public void shouldFailToAddRoleToUserIfNotAdmin() throws Exception
{
testFailAddUserToRole( pwdSubject, "readSubject", PUBLISHER, CHANGE_PWD_ERR_MSG );
testFailAddUserToRole( readSubject, "readSubject", PUBLISHER, PERMISSION_DENIED );
testFailAddUserToRole( writeSubject, "readSubject", PUBLISHER, PERMISSION_DENIED );
testFailAddRoleToUser( pwdSubject, PUBLISHER, "readSubject", CHANGE_PWD_ERR_MSG );
testFailAddRoleToUser( readSubject, PUBLISHER, "readSubject", PERMISSION_DENIED );
testFailAddRoleToUser( writeSubject, PUBLISHER, "readSubject", PERMISSION_DENIED );

testFailAddUserToRole( schemaSubject, "readSubject", PUBLISHER, PERMISSION_DENIED );
testFailAddUserToRole( schemaSubject, "Olivia", PUBLISHER, PERMISSION_DENIED );
testFailAddUserToRole( schemaSubject, "Olivia", "thisRoleDoesNotExist", PERMISSION_DENIED );
testFailAddRoleToUser( schemaSubject, PUBLISHER, "readSubject", PERMISSION_DENIED );
testFailAddRoleToUser( schemaSubject, PUBLISHER, "Olivia", PERMISSION_DENIED );
testFailAddRoleToUser( schemaSubject, "thisRoleDoesNotExist", "Olivia", PERMISSION_DENIED );
}

//---------- remove user from role -----------

@Test
public void shouldRemoveUserFromRole() throws Exception
public void shouldRemoveRoleFromUser() throws Exception
{
assertEmpty( adminSubject, "CALL dbms.removeUserFromRole('readSubject', '" + READER + "')" );
assertEmpty( adminSubject, "CALL dbms.removeRoleFromUser('" + READER + "', 'readSubject')" );
assertFalse( "Should not have role reader", userHasRole( "readSubject", READER ) );
}

@Test
public void shouldKeepUserOutOfRole() throws Exception
{
assertFalse( "Should not have role publisher", userHasRole( "readSubject", PUBLISHER ) );
assertEmpty( adminSubject, "CALL dbms.removeUserFromRole('readSubject', '" + PUBLISHER + "')" );
assertEmpty( adminSubject, "CALL dbms.removeRoleFromUser('" + PUBLISHER + "', 'readSubject')" );
assertFalse( "Should not have role publisher", userHasRole( "readSubject", PUBLISHER ) );
}

@Test
public void shouldFailToRemoveNonExistentUserFromRole() throws Exception
{
testFailRemoveUserFromRole( adminSubject, "Olivia", PUBLISHER, "User 'Olivia' does not exist." );
testFailRemoveUserFromRole( adminSubject, "Olivia", "thisRoleDoesNotExist", "User 'Olivia' does not exist." );
testFailRemoveUserFromRole( adminSubject, "Olivia", "", "The provided role name is empty." );
testFailRemoveUserFromRole( adminSubject, "", "", "The provided user name is empty." );
testFailRemoveRoleFromUser( adminSubject, PUBLISHER, "Olivia", "User 'Olivia' does not exist." );
testFailRemoveRoleFromUser( adminSubject, "thisRoleDoesNotExist", "Olivia", "User 'Olivia' does not exist." );
testFailRemoveRoleFromUser( adminSubject, "", "Olivia", "The provided role name is empty." );
testFailRemoveRoleFromUser( adminSubject, "", "", "The provided role name is empty." );
testFailRemoveRoleFromUser( adminSubject, PUBLISHER, "", "The provided user name is empty." );
}

@Test
public void shouldFailToRemoveUserFromNonExistentRole() throws Exception
{
testFailRemoveUserFromRole( adminSubject, "readSubject", "thisRoleDoesNotExist",
testFailRemoveRoleFromUser( adminSubject, "thisRoleDoesNotExist", "readSubject",
"Role 'thisRoleDoesNotExist' does not exist." );
testFailRemoveUserFromRole( adminSubject, "readSubject", "", "The provided role name is empty." );
testFailRemoveRoleFromUser( adminSubject, "", "readSubject", "The provided role name is empty." );
}

@Test
public void shouldFailToRemoveUserFromRoleIfNotAdmin() throws Exception
public void shouldFailToRemoveRoleFromUserIfNotAdmin() throws Exception
{
testFailRemoveUserFromRole( pwdSubject, "readSubject", PUBLISHER,CHANGE_PWD_ERR_MSG );
testFailRemoveUserFromRole( readSubject, "readSubject", PUBLISHER, PERMISSION_DENIED );
testFailRemoveUserFromRole( writeSubject, "readSubject", PUBLISHER, PERMISSION_DENIED );
testFailRemoveRoleFromUser( pwdSubject, PUBLISHER, "readSubject",CHANGE_PWD_ERR_MSG );
testFailRemoveRoleFromUser( readSubject, PUBLISHER, "readSubject", PERMISSION_DENIED );
testFailRemoveRoleFromUser( writeSubject, PUBLISHER, "readSubject", PERMISSION_DENIED );

testFailRemoveUserFromRole( schemaSubject, "readSubject", READER, PERMISSION_DENIED );
testFailRemoveUserFromRole( schemaSubject, "Olivia", READER, PERMISSION_DENIED );
testFailRemoveUserFromRole( schemaSubject, "Olivia", "thisRoleDoesNotExist", PERMISSION_DENIED );
testFailRemoveRoleFromUser( schemaSubject, READER, "readSubject", PERMISSION_DENIED );
testFailRemoveRoleFromUser( schemaSubject, READER, "Olivia", PERMISSION_DENIED );
testFailRemoveRoleFromUser( schemaSubject, "thisRoleDoesNotExist", "Olivia", PERMISSION_DENIED );
}

@Test
public void shouldFailToRemoveYourselfFromAdminRole() throws Exception
{
assertFail( adminSubject, "CALL dbms.removeUserFromRole('adminSubject', '" + ADMIN + "')",
assertFail( adminSubject, "CALL dbms.removeRoleFromUser('" + ADMIN + "', 'adminSubject')",
"Removing yourself (user 'adminSubject') from the admin role is not allowed." );
}

Expand All @@ -674,12 +676,12 @@ public void shouldAllowAddingAndRemovingUserFromMultipleRoles() throws Exception
{
assertFalse( "Should not have role publisher", userHasRole( "readSubject", PUBLISHER ) );
assertFalse( "Should not have role architect", userHasRole( "readSubject", ARCHITECT ) );
assertEmpty( adminSubject, "CALL dbms.addUserToRole('readSubject', '" + PUBLISHER + "')" );
assertEmpty( adminSubject, "CALL dbms.addUserToRole('readSubject', '" + ARCHITECT + "')" );
assertEmpty( adminSubject, "CALL dbms.addRoleToUser('" + PUBLISHER + "', 'readSubject')" );
assertEmpty( adminSubject, "CALL dbms.addRoleToUser('" + ARCHITECT + "', 'readSubject')" );
assertTrue( "Should have role publisher", userHasRole( "readSubject", PUBLISHER ) );
assertTrue( "Should have role architect", userHasRole( "readSubject", ARCHITECT ) );
assertEmpty( adminSubject, "CALL dbms.removeUserFromRole('readSubject', '" + PUBLISHER + "')" );
assertEmpty( adminSubject, "CALL dbms.removeUserFromRole('readSubject', '" + ARCHITECT + "')" );
assertEmpty( adminSubject, "CALL dbms.removeRoleFromUser('" + PUBLISHER + "', 'readSubject')" );
assertEmpty( adminSubject, "CALL dbms.removeRoleFromUser('" + ARCHITECT + "', 'readSubject')" );
assertFalse( "Should not have role publisher", userHasRole( "readSubject", PUBLISHER ) );
assertFalse( "Should not have role architect", userHasRole( "readSubject", ARCHITECT ) );
}
Expand All @@ -705,7 +707,7 @@ public void shouldReturnUsersWithRoles() throws Exception
"noneSubject", listOf( ),
"neo4j", listOf( ADMIN )
);
userManager.addUserToRole( "writeSubject", READER );
userManager.addRoleToUser( READER, "writeSubject" );
assertSuccess( adminSubject, "CALL dbms.listUsers()",
r -> assertKeyIsMap( r, "username", "roles", expected ) );
}
Expand All @@ -731,7 +733,7 @@ public void shouldReturnUsersWithFlags() throws Exception
@Test
public void shouldShowCurrentUser() throws Exception
{
userManager.addUserToRole( "writeSubject", READER );
userManager.addRoleToUser( READER, "writeSubject" );
assertSuccess( adminSubject, "CALL dbms.showCurrentUser()",
r -> assertKeyIsMap( r, "username", "roles", map( "adminSubject", listOf( ADMIN ) ) ) );
assertSuccess( readSubject, "CALL dbms.showCurrentUser()",
Expand Down Expand Up @@ -894,7 +896,7 @@ public void shouldSetCorrectPasswordChangeRequiredPermissions() throws Throwable
assertPasswordChangeWhenPasswordChangeRequired( pwdSubject, "321" );

assertEmpty( adminSubject, "CALL dbms.createUser('Henrik', 'bar', true)" );
assertEmpty( adminSubject, "CALL dbms.addUserToRole('Henrik', '" + ARCHITECT + "')" );
assertEmpty( adminSubject, "CALL dbms.addRoleToUser('" + ARCHITECT + "', 'Henrik')" );
S henrik = neo.login( "Henrik", "bar" );
neo.assertPasswordChangeRequired( henrik );
testFailRead( henrik, 3, pwdReqErrMsg( READ_OPS_NOT_ALLOWED ) );
Expand All @@ -903,7 +905,7 @@ public void shouldSetCorrectPasswordChangeRequiredPermissions() throws Throwable
assertPasswordChangeWhenPasswordChangeRequired( henrik, "321" );

assertEmpty( adminSubject, "CALL dbms.createUser('Olivia', 'bar', true)" );
assertEmpty( adminSubject, "CALL dbms.addUserToRole('Olivia', '" + ADMIN + "')" );
assertEmpty( adminSubject, "CALL dbms.addRoleToUser('" + ADMIN + "', 'Olivia')" );
S olivia = neo.login( "Olivia", "bar" );
neo.assertPasswordChangeRequired( olivia );
testFailRead( olivia, 3, pwdReqErrMsg( READ_OPS_NOT_ALLOWED ) );
Expand Down Expand Up @@ -966,7 +968,7 @@ public void shouldSetCorrectAdminPermissions() throws Exception
@Test
public void shouldSetCorrectMultiRolePermissions() throws Exception
{
assertEmpty( adminSubject, "CALL dbms.addUserToRole('schemaSubject', '" + READER + "')" );
assertEmpty( adminSubject, "CALL dbms.addRoleToUser('" + READER + "', 'schemaSubject')" );

testSuccessfulRead( schemaSubject, 3 );
testSuccessfulWrite( schemaSubject );
Expand Down

0 comments on commit 11054b4

Please sign in to comment.