Skip to content

Commit

Permalink
User with CredentialsExpired should only be allowed to change password
Browse files Browse the repository at this point in the history
This reverts commit fc5bc6d.
  • Loading branch information
OliviaYtterbrink committed May 4, 2017
1 parent b2084f3 commit f304b1e
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 7 deletions.
Expand Up @@ -19,6 +19,8 @@
*/
package org.neo4j.kernel.api.security;

import static org.neo4j.graphdb.security.AuthorizationViolationException.PERMISSION_DENIED;

/** Controls the capabilities of a KernelTransaction. */
public interface SecurityContext
{
Expand All @@ -29,6 +31,14 @@ public interface SecurityContext
SecurityContext freeze();
SecurityContext withMode( AccessMode mode );

default void assertCredentialsNotExpired()
{
if ( subject().getAuthenticationResult().equals( AuthenticationResult.PASSWORD_CHANGE_REQUIRED ) )
{
throw mode().onViolation( PERMISSION_DENIED );
}
}

default String description()
{
return String.format( "user '%s' with %s", subject().username(), mode().name() );
Expand Down
Expand Up @@ -50,6 +50,7 @@ public class BuiltInDbmsProcedures
@Procedure( name = "dbms.listConfig", mode = DBMS )
public Stream<ConfigResult> listConfig( @Name( value = "searchString", defaultValue = "" ) String searchString )
{
securityContext.assertCredentialsNotExpired();
if ( !securityContext.isAdmin() )
{
throw new AuthorizationViolationException( PERMISSION_DENIED );
Expand All @@ -66,6 +67,7 @@ public Stream<ConfigResult> listConfig( @Name( value = "searchString", defaultVa
@Procedure( name = "dbms.procedures", mode = DBMS )
public Stream<ProcedureResult> listProcedures()
{
securityContext.assertCredentialsNotExpired();
return graph.getDependencyResolver().resolveDependency( Procedures.class ).getAllProcedures().stream()
.sorted( Comparator.comparing( a -> a.name().toString() ) )
.map( ProcedureResult::new );
Expand All @@ -75,6 +77,7 @@ public Stream<ProcedureResult> listProcedures()
@Procedure(name = "dbms.functions", mode = DBMS)
public Stream<FunctionResult> listFunctions()
{
securityContext.assertCredentialsNotExpired();
return graph.getDependencyResolver().resolveDependency( Procedures.class ).getAllFunctions().stream()
.sorted( Comparator.comparing( a -> a.name().toString() ) )
.map( FunctionResult::new );
Expand Down
Expand Up @@ -56,13 +56,15 @@ public void createUser(
@Name( value = "requirePasswordChange", defaultValue = "true" ) boolean requirePasswordChange )
throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
userManager.newUser( username, password, requirePasswordChange );
}

@Description( "Delete the specified user." )
@Procedure( name = "dbms.security.deleteUser", mode = DBMS )
public void deleteUser( @Name( "username" ) String username ) throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
if ( securityContext.subject().hasUsername( username ) )
{
throw new InvalidArgumentsException( "Deleting yourself (user '" + username + "') is not allowed." );
Expand Down Expand Up @@ -109,6 +111,7 @@ public Stream<UserResult> showCurrentUserDeprecated() throws InvalidArgumentsExc
@Procedure( name = "dbms.security.listUsers", mode = DBMS )
public Stream<UserResult> listUsers() throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
Set<String> usernames = userManager.getAllUsernames();

if ( usernames.isEmpty() )
Expand Down
Expand Up @@ -104,6 +104,27 @@ public void shouldNotChangeOwnPasswordIfNewPasswordInvalid() throws Exception
assertFail( admin, "CALL dbms.changePassword( 'neo4j' )", "Old password and new password cannot be the same." );
}

@Test
public void newUserShouldBeAbleToChangePassword() throws Throwable
{
// Given
authManager.newUser( "andres", "banana", true );

// Then
assertEmpty( login("andres", "banana"), "CALL dbms.changePassword('abc')" );
}

@Test
public void newUserShouldNotBeAbleToCallOtherProcedures() throws Throwable
{
// Given
authManager.newUser( "andres", "banana", true );
BasicSecurityContext user = login("andres", "banana");

// Then
assertFail( user, "CALL dbms.procedures", "The credentials you provided were valid, but must be changed before you can use this instance." );
}

//---------- create user -----------

@Test
Expand Down
Expand Up @@ -82,6 +82,7 @@ public class EnterpriseBuiltInDbmsProcedures
@Procedure( name = "dbms.setTXMetaData", mode = DBMS )
public void setTXMetaData( @Name( value = "data" ) Map<String,Object> data )
{
securityContext.assertCredentialsNotExpired();
int totalCharSize = data.entrySet().stream()
.mapToInt( e -> e.getKey().length() + e.getValue().toString().length() )
.sum();
Expand Down Expand Up @@ -164,6 +165,7 @@ public Stream<ConnectionResult> terminateConnectionsForUser( @Name( "username" )
@Procedure(name = "dbms.functions", mode = DBMS)
public Stream<FunctionResult> listFunctions()
{
securityContext.assertCredentialsNotExpired();
return graph.getDependencyResolver().resolveDependency( Procedures.class ).getAllFunctions().stream()
.sorted( ( a, b ) -> a.name().toString().compareTo( b.name().toString() ) )
.map( FunctionResult::new );
Expand All @@ -190,6 +192,7 @@ private FunctionResult( UserFunctionSignature signature )
@Procedure( name = "dbms.procedures", mode = DBMS )
public Stream<ProcedureResult> listProcedures()
{
securityContext.assertCredentialsNotExpired();
Procedures procedures = graph.getDependencyResolver().resolveDependency( Procedures.class );
return procedures.getAllProcedures().stream()
.sorted( ( a, b ) -> a.name().toString().compareTo( b.name().toString() ) )
Expand Down Expand Up @@ -239,6 +242,7 @@ public ProcedureResult( ProcedureSignature signature )
@Procedure( name = "dbms.listQueries", mode = DBMS )
public Stream<QueryStatusResult> listQueries() throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
try
{
return getKernelTransactions().activeTransactions().stream()
Expand All @@ -258,6 +262,7 @@ public Stream<QueryStatusResult> listQueries() throws InvalidArgumentsException,
public Stream<ActiveLocksQueryResult> listActiveLocks( @Name( "queryId" ) String queryId )
throws InvalidArgumentsException
{
securityContext.assertCredentialsNotExpired();
try
{
long id = fromExternalString( queryId ).kernelQueryId();
Expand All @@ -276,6 +281,7 @@ public Stream<ActiveLocksQueryResult> listActiveLocks( @Name( "queryId" ) String
public Stream<QueryTerminationResult> killQuery( @Name( "id" ) String idText )
throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
try
{
long queryId = fromExternalString( idText ).kernelQueryId();
Expand All @@ -295,6 +301,7 @@ public Stream<QueryTerminationResult> killQuery( @Name( "id" ) String idText )
public Stream<QueryTerminationResult> killQueries( @Name( "ids" ) List<String> idTexts )
throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
try
{
Set<Long> queryIds = idTexts
Expand Down
Expand Up @@ -49,6 +49,7 @@ public Stream<UserManagementProcedures.UserResult> showCurrentUser() throws Inva
@Procedure( name = "dbms.security.clearAuthCache", mode = DBMS )
public void clearAuthenticationCache()
{
securityContext.assertCredentialsNotExpired();
if ( !securityContext.isAdmin() )
{
throw new AuthorizationViolationException( PERMISSION_DENIED );
Expand Down
Expand Up @@ -40,6 +40,7 @@ public void createUser( @Name( "username" ) String username, @Name( "password" )
@Name( value = "requirePasswordChange", defaultValue = "true" ) boolean requirePasswordChange )
throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
userManager.newUser( username, password, requirePasswordChange );
}

Expand All @@ -58,7 +59,7 @@ public void changePassword( @Name( "password" ) String password,
@Name( value = "requirePasswordChange", defaultValue = "false" ) boolean requirePasswordChange )
throws InvalidArgumentsException, IOException
{
changeUserPassword( securityContext.subject().username(), password, requirePasswordChange );
setUserPassword( securityContext.subject().username(), password, requirePasswordChange );
}

@Description( "Change the given user's password." )
Expand All @@ -67,18 +68,16 @@ public void changeUserPassword( @Name( "username" ) String username, @Name( "new
@Name( value = "requirePasswordChange", defaultValue = "true" ) boolean requirePasswordChange )
throws InvalidArgumentsException, IOException
{
userManager.setUserPassword( username, newPassword, requirePasswordChange );
if ( securityContext.subject().hasUsername( username ) )
{
securityContext.subject().setPasswordChangeNoLongerRequired();
}
securityContext.assertCredentialsNotExpired();
setUserPassword( username, newPassword, requirePasswordChange );
}

@Description( "Assign a role to the user." )
@Procedure( name = "dbms.security.addRoleToUser", mode = DBMS )
public void addRoleToUser( @Name( "roleName" ) String roleName, @Name( "username" ) String username )
throws IOException, InvalidArgumentsException
{
securityContext.assertCredentialsNotExpired();
userManager.addRoleToUser( roleName, username );
}

Expand All @@ -87,13 +86,15 @@ public void addRoleToUser( @Name( "roleName" ) String roleName, @Name( "username
public void removeRoleFromUser( @Name( "roleName" ) String roleName, @Name( "username" ) String username )
throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
userManager.removeRoleFromUser( roleName, username );
}

@Description( "Delete the specified user." )
@Procedure( name = "dbms.security.deleteUser", mode = DBMS )
public void deleteUser( @Name( "username" ) String username ) throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
if ( userManager.deleteUser( username ) )
{
kickoutUser( username, "deletion" );
Expand All @@ -104,6 +105,7 @@ public void deleteUser( @Name( "username" ) String username ) throws InvalidArgu
@Procedure( name = "dbms.security.suspendUser", mode = DBMS )
public void suspendUser( @Name( "username" ) String username ) throws IOException, InvalidArgumentsException
{
securityContext.assertCredentialsNotExpired();
userManager.suspendUser( username );
kickoutUser( username, "suspension" );
}
Expand All @@ -114,13 +116,15 @@ public void activateUser( @Name( "username" ) String username,
@Name( value = "requirePasswordChange", defaultValue = "true" ) boolean requirePasswordChange )
throws IOException, InvalidArgumentsException
{
securityContext.assertCredentialsNotExpired();
userManager.activateUser( username, requirePasswordChange );
}

@Description( "List all local users." )
@Procedure( name = "dbms.security.listUsers", mode = DBMS )
public Stream<UserResult> listUsers() throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
Set<String> users = userManager.getAllUsernames();
if ( users.isEmpty() )
{
Expand All @@ -136,6 +140,7 @@ public Stream<UserResult> listUsers() throws InvalidArgumentsException, IOExcept
@Procedure( name = "dbms.security.listRoles", mode = DBMS )
public Stream<RoleResult> listRoles() throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
Set<String> roles = userManager.getAllRoleNames();
return roles.stream().map( this::roleResultForName );
}
Expand All @@ -145,6 +150,7 @@ public Stream<RoleResult> listRoles() throws InvalidArgumentsException, IOExcept
public Stream<StringResult> listRolesForUser( @Name( "username" ) String username )
throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
return userManager.getRoleNamesForUser( username ).stream().map( StringResult::new );
}

Expand All @@ -153,20 +159,33 @@ public Stream<StringResult> listRolesForUser( @Name( "username" ) String usernam
public Stream<StringResult> listUsersForRole( @Name( "roleName" ) String roleName )
throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
return userManager.getUsernamesForRole( roleName ).stream().map( StringResult::new );
}

@Description( "Create a new role." )
@Procedure( name = "dbms.security.createRole", mode = DBMS )
public void createRole( @Name( "roleName" ) String roleName ) throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
userManager.newRole( roleName );
}

@Description( "Delete the specified role. Any role assignments will be removed." )
@Procedure( name = "dbms.security.deleteRole", mode = DBMS )
public void deleteRole( @Name( "roleName" ) String roleName ) throws InvalidArgumentsException, IOException
{
securityContext.assertCredentialsNotExpired();
userManager.deleteRole( roleName );
}

private void setUserPassword( String username, String newPassword, boolean requirePasswordChange )
throws IOException, InvalidArgumentsException
{
userManager.setUserPassword( username, newPassword, requirePasswordChange );
if ( securityContext.subject().hasUsername( username ) )
{
securityContext.subject().setPasswordChangeNoLongerRequired();
}
}
}
Expand Up @@ -29,7 +29,6 @@
import org.neo4j.graphdb.security.AuthorizationViolationException;
import org.neo4j.kernel.api.exceptions.InvalidArgumentsException;
import org.neo4j.kernel.api.security.AuthenticationResult;
import org.neo4j.kernel.api.security.SecurityContext;
import org.neo4j.kernel.enterprise.api.security.EnterpriseSecurityContext;
import org.neo4j.server.security.enterprise.log.SecurityLog;
import org.neo4j.kernel.impl.util.JobScheduler;
Expand Down
Expand Up @@ -57,6 +57,14 @@ public void setup() throws Exception
File knownHosts = new File( System.getProperty( "user.home" ) + "/.neo4j/known_hosts" );
FileUtils.deleteFile( knownHosts );
adminDriver = GraphDatabase.driver( db.boltURI(), basic( "neo4j", "neo4j" ) );
try ( Session session = adminDriver.session();
Transaction tx = session.beginTransaction() )
{
tx.run( "CALL dbms.changePassword('abc')" ).consume();
tx.success();
}
adminDriver.close();
adminDriver = GraphDatabase.driver( db.boltURI(), basic( "neo4j", "abc" ) );
}

@Test
Expand Down

0 comments on commit f304b1e

Please sign in to comment.