Merge pull request #9301 from OliviaYtterbrink/3.2-revert-authorizati…

…on-fix

Revert "User with CredentialsExpired should only be allowed to change password"

community/kernel/src/main/java/org/neo4j/kernel/api/security/SecurityContext.java

@@ -19,8 +19,6 @@
  */
 package org.neo4j.kernel.api.security;
 
-import static org.neo4j.graphdb.security.AuthorizationViolationException.PERMISSION_DENIED;
-
 /** Controls the capabilities of a KernelTransaction. */
 public interface SecurityContext
 {
@@ -31,14 +29,6 @@ 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() );

community/kernel/src/main/java/org/neo4j/kernel/builtinprocs/BuiltInDbmsProcedures.java

@@ -50,7 +50,6 @@ 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 );
@@ -67,7 +66,6 @@ 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 );
@@ -77,7 +75,6 @@ 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 );

community/security/src/main/java/org/neo4j/server/security/auth/AuthProcedures.java

@@ -56,15 +56,13 @@ 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." );
@@ -111,7 +109,6 @@ 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() )

community/security/src/test/java/org/neo4j/server/security/auth/AuthProceduresIT.java

@@ -104,27 +104,6 @@ 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

enterprise/kernel/src/main/java/org/neo4j/kernel/enterprise/builtinprocs/EnterpriseBuiltInDbmsProcedures.java

@@ -82,7 +82,6 @@ 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();
@@ -165,7 +164,6 @@ 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 );
@@ -192,7 +190,6 @@ 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() ) )
@@ -242,7 +239,6 @@ public ProcedureResult( ProcedureSignature signature )
     @Procedure( name = "dbms.listQueries", mode = DBMS )
     public Stream<QueryStatusResult> listQueries() throws InvalidArgumentsException, IOException
     {
-        securityContext.assertCredentialsNotExpired();
         try
         {
             return getKernelTransactions().activeTransactions().stream()
@@ -262,7 +258,6 @@ public Stream<QueryStatusResult> listQueries() throws InvalidArgumentsException,
     public Stream<ActiveLocksQueryResult> listActiveLocks( @Name( "queryId" ) String queryId )
             throws InvalidArgumentsException
     {
-        securityContext.assertCredentialsNotExpired();
         try
         {
             long id = fromExternalString( queryId ).kernelQueryId();
@@ -281,7 +276,6 @@ 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();
@@ -301,7 +295,6 @@ 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

enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/SecurityProcedures.java

@@ -49,7 +49,6 @@ 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 );

enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/UserManagementProcedures.java

@@ -40,7 +40,6 @@ 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 );
     }
 
@@ -59,7 +58,7 @@ public void changePassword( @Name( "password" ) String password,
             @Name( value = "requirePasswordChange", defaultValue = "false" ) boolean requirePasswordChange )
             throws InvalidArgumentsException, IOException
     {
-        setUserPassword( securityContext.subject().username(), password, requirePasswordChange );
+        changeUserPassword( securityContext.subject().username(), password, requirePasswordChange );
     }
 
     @Description( "Change the given user's password." )
@@ -68,16 +67,18 @@ public void changeUserPassword( @Name( "username" ) String username, @Name( "new
             @Name( value = "requirePasswordChange", defaultValue = "true" ) boolean requirePasswordChange )
             throws InvalidArgumentsException, IOException
     {
-        securityContext.assertCredentialsNotExpired();
-        setUserPassword( username, newPassword, requirePasswordChange );
+        userManager.setUserPassword( username, newPassword, requirePasswordChange );
+        if ( securityContext.subject().hasUsername( username ) )
+        {
+            securityContext.subject().setPasswordChangeNoLongerRequired();
+        }
     }
 
     @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 );
     }
 
@@ -86,15 +87,13 @@ 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" );
@@ -105,7 +104,6 @@ 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" );
     }
@@ -116,15 +114,13 @@ 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() )
         {
@@ -140,7 +136,6 @@ 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 );
     }
@@ -150,7 +145,6 @@ 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 );
     }
 
@@ -159,33 +153,20 @@ 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();
-        }
-    }
 }

enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/UserManagementProceduresLoggingTest.java

@@ -29,6 +29,7 @@
 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;