Completion of suspend / activate user.

* general purpose flags on User class and it's serialization
* cleaned up User builder
* added UserTest on immutability
* improved AuthProcedureTests on suspend / activate user

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

@@ -19,6 +19,10 @@
  */
 package org.neo4j.server.security.auth;
 
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
+
 /**
  * Controls authorization and authentication for an individual user.
  */
@@ -35,23 +39,16 @@ public class User
     /** Authentication credentials used by the built in username/password authentication scheme */
     private final Credential credential;
 
-    /** Whether a password change is needed */
-    private final boolean passwordChangeRequired;
-
-    /** User is suspended */
-    private final boolean isSuspended;
+    /** Set of flags, eg. require_password_change */
+    private final HashSet<String> flags;
 
-    public User( String name, Credential credential, boolean passwordChangeRequired )
-    {
-        this(name, credential, passwordChangeRequired, false);
-    }
+    public static final String PASSWORD_CHANGE_REQUIRED = "password_change_required";
 
-    private User( String name, Credential credential, boolean passwordChangeRequired, boolean isSuspended )
+    private User( String name, Credential credential, HashSet<String> flags )
     {
         this.name = name;
         this.credential = credential;
-        this.passwordChangeRequired = passwordChangeRequired;
-        this.isSuspended = isSuspended;
+        this.flags = flags;
     }
 
     public String name()
@@ -64,8 +61,10 @@ public Credential credentials()
         return credential;
     }
 
-    public boolean passwordChangeRequired() { return passwordChangeRequired; }
-    public boolean isSuspended() { return isSuspended; }
+    public boolean hasFlag(String flag) { return flags.contains(flag); }
+    public Iterator<String> getFlags() { return flags.iterator(); }
+
+    public boolean passwordChangeRequired() { return flags.contains( PASSWORD_CHANGE_REQUIRED ); }
 
     /** Use this user as a base for a new user object */
     public Builder augment() { return new Builder(this); }
@@ -84,11 +83,7 @@ public boolean equals( Object o )
 
         User user = (User) o;
 
-        if ( passwordChangeRequired != user.passwordChangeRequired )
-        {
-            return false;
-        }
-        if ( isSuspended != user.isSuspended )
+        if ( !setsAreEquals( flags, user.flags ) )
         {
             return false;
         }
@@ -109,8 +104,7 @@ public int hashCode()
     {
         int result = name != null ? name.hashCode() : 0;
         result = 31 * result + ( credential != null ? credential.hashCode() : 0);
-        result = 31 * result + (passwordChangeRequired ? 1 : 0);
-        result = 31 * result + (isSuspended ? 1 : 0);
+        result = 31 * result + ( flags.hashCode() );
         return result;
     }
 
@@ -120,36 +114,69 @@ public String toString()
         return "User{" +
                 "name='" + name + '\'' +
                 ", credentials=" + credential +
-                ", passwordChangeRequired=" + passwordChangeRequired +
-                ", isSuspended=" + isSuspended +
+                ", flags=" + flags.toString() +
                 '}';
     }
 
     public static class Builder
     {
         private String name;
         private Credential credential = Credential.INACCESSIBLE;
-        private boolean pwdChangeRequired;
-        private boolean isSuspended;
+        private HashSet<String> flags = new HashSet<>();
 
         public Builder() { }
 
+        public Builder( String name, Credential credential )
+        {
+            this.name = name;
+            this.credential = credential;
+        }
+
         public Builder( User base )
         {
             name = base.name;
             credential = base.credential;
-            pwdChangeRequired = base.passwordChangeRequired;
-            isSuspended = base.isSuspended;
+            flags.addAll( base.flags );
         }
 
         public Builder withName( String name ) { this.name = name; return this; }
         public Builder withCredentials( Credential creds ) { this.credential = creds; return this; }
-        public Builder withRequiredPasswordChange( boolean change ) { this.pwdChangeRequired = change; return this; }
-        public Builder withIsSuspended( boolean suspended ) { this.isSuspended = suspended; return this; }
+        public Builder withFlag( String flag ) { this.flags.add(flag); return this; }
+        public Builder withoutFlag( String flag ) { this.flags.remove(flag); return this; }
+
+        public Builder withRequiredPasswordChange( boolean change )
+        {
+            if ( change )
+            {
+                withFlag( PASSWORD_CHANGE_REQUIRED );
+            }
+            else
+            {
+                withoutFlag( PASSWORD_CHANGE_REQUIRED );
+            }
+            return this;
+        }
 
         public User build()
         {
-            return new User(name, credential, pwdChangeRequired, isSuspended );
+            return new User(name, credential, flags );
+        }
+    }
+
+    private static boolean setsAreEquals( Set<String> set1, Set<String> set2 )
+    {
+        if ( set1.size() != set2.size() )
+        {
+            return false;
+        }
+
+        for ( String element : set1 )
+        {
+            if ( !set2.contains( element ) )
+            {
+                return false;
+            }
         }
+        return true;
     }
 }

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

@@ -20,7 +20,9 @@
 package org.neo4j.server.security.auth;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
 
 import org.neo4j.string.HexString;
@@ -73,36 +75,33 @@ private String serialize( User user )
     {
         return join( userSeparator, user.name(),
                 serialize( user.credentials() ),
-                user.passwordChangeRequired() ? "password_change_required" : "",
-                user.isSuspended() ? "is_suspended" : "" );
+                join( ",", user.getFlags() )
+            );
     }
 
     private User deserializeUser( String line, int lineNumber ) throws FormatException
     {
         String[] parts = line.split( userSeparator, -1 );
-        boolean isSuspended;
-        if ( parts.length == 3 ) // Before suspension impl. assume non-suspended user
+        if ( parts.length != 3 )
         {
-            isSuspended = false;
-        }
-        else if ( parts.length == 4 )
-        {
-            isSuspended = parts[3].equals( "is_suspended" );
-        }
-        else
-        {
-            throw new FormatException( format( "wrong number of line fields, expected 3 or 4, got %d [line %d]",
+            throw new FormatException( format( "wrong number of line fields, expected 3, got %d [line %d]",
                     parts.length,
                     lineNumber
             ) );
         }
 
-        return new User.Builder()
+        User.Builder b = new User.Builder()
                 .withName( parts[0] )
-                .withCredentials( deserializeCredentials( parts[1], lineNumber ) )
-                .withRequiredPasswordChange( parts[2].equals( "password_change_required" ) )
-                .withIsSuspended( isSuspended )
-                .build();
+                .withCredentials( deserializeCredentials( parts[1], lineNumber ) );
+
+        for ( String flag : parts[2].split( ",", -1 ))
+        {
+            String trimmed = flag.trim();
+            if (!trimmed.isEmpty())
+                b = b.withFlag( trimmed );
+        }
+
+        return  b.build();
     }
 
     private String serialize( Credential cred )
@@ -129,12 +128,19 @@ private Credential deserializeCredentials( String part, int lineNumber ) throws
     }
 
     private String join( String separator, String... segments )
+    {
+        return join(separator, Arrays.asList(segments).iterator() );
+    }
+
+    private String join( String separator, Iterator<String> segments )
     {
         StringBuilder sb = new StringBuilder();
-        for ( int i = 0; i < segments.length; i++ )
+        boolean afterFirst = false;
+        while (segments.hasNext())
         {
-            if(i > 0) { sb.append( separator ); }
-            sb.append( segments[i] == null ? "" : segments[i] );
+            if ( afterFirst ) { sb.append( separator ); }
+            else              { afterFirst = true;      }
+            sb.append( segments.next() );
         }
         return sb.toString();
     }

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

@@ -59,7 +59,7 @@ public void shouldFindAndAuthenticateUserSuccessfully() throws Throwable
     {
         // Given
         final InMemoryUserRepository users = new InMemoryUserRepository();
-        final User user = new User( "jake", Credential.forPassword( "abc123" ), false );
+        final User user = new User.Builder( "jake", Credential.forPassword( "abc123" )).build();
         users.create( user );
         final AuthenticationStrategy authStrategy = mock( AuthenticationStrategy.class );
         final BasicAuthManager manager = new BasicAuthManager( users, mock( PasswordPolicy.class ), authStrategy );
@@ -79,7 +79,7 @@ public void shouldFindAndAuthenticateUserAndReturnAuthStrategyResult() throws Th
     {
         // Given
         final InMemoryUserRepository users = new InMemoryUserRepository();
-        final User user = new User( "jake", Credential.forPassword( "abc123" ), true );
+        final User user = new User.Builder( "jake", Credential.forPassword( "abc123" )).withRequiredPasswordChange( true ).build();
         users.create( user );
         final AuthenticationStrategy authStrategy = mock( AuthenticationStrategy.class );
         final BasicAuthManager manager = new BasicAuthManager( users, mock( PasswordPolicy.class ), authStrategy );
@@ -99,7 +99,7 @@ public void shouldFindAndAuthenticateUserAndReturnPasswordChangeIfRequired() thr
     {
         // Given
         final InMemoryUserRepository users = new InMemoryUserRepository();
-        final User user = new User( "jake", Credential.forPassword( "abc123" ), true );
+        final User user = new User.Builder( "jake", Credential.forPassword( "abc123" )).withRequiredPasswordChange( true ).build();
         users.create( user );
         final AuthenticationStrategy authStrategy = mock( AuthenticationStrategy.class );
         final BasicAuthManager manager = new BasicAuthManager( users, mock( PasswordPolicy.class ), authStrategy );
@@ -119,7 +119,7 @@ public void shouldFailAuthenticationIfUserIsNotFound() throws Throwable
     {
         // Given
         final InMemoryUserRepository users = new InMemoryUserRepository();
-        final User user = new User( "jake", Credential.forPassword( "abc123" ), true );
+        final User user = new User.Builder( "jake", Credential.forPassword( "abc123" )).withRequiredPasswordChange( true ).build();
         users.create( user );
         final AuthenticationStrategy authStrategy = mock( AuthenticationStrategy.class );
         final BasicAuthManager manager = new BasicAuthManager( users, mock( PasswordPolicy.class ), authStrategy );
@@ -157,7 +157,7 @@ public void shouldDeleteUser() throws Throwable
     {
         // Given
         final InMemoryUserRepository users = new InMemoryUserRepository();
-        final User user = new User( "jake", Credential.forPassword( "abc123" ), true );
+        final User user = new User.Builder( "jake", Credential.forPassword( "abc123" )).withRequiredPasswordChange( true ).build();
         users.create( user );
         final BasicAuthManager manager =
                 new BasicAuthManager( users, mock( PasswordPolicy.class ), mock( AuthenticationStrategy.class ) );
@@ -175,7 +175,7 @@ public void shouldDeleteUnknownUser() throws Throwable
     {
         // Given
         final InMemoryUserRepository users = new InMemoryUserRepository();
-        final User user = new User( "jake", Credential.forPassword( "abc123" ), true );
+        final User user = new User.Builder( "jake", Credential.forPassword( "abc123" )).withRequiredPasswordChange( true ).build();
         users.create( user );
         final BasicAuthManager manager =
                 new BasicAuthManager( users, mock( PasswordPolicy.class ), mock( AuthenticationStrategy.class ) );
@@ -193,7 +193,7 @@ public void shouldSetPassword() throws Throwable
     {
         // Given
         final InMemoryUserRepository users = new InMemoryUserRepository();
-        users.create( new User( "jake", Credential.forPassword( "abc123" ), true ) );
+        users.create( new User.Builder( "jake", Credential.forPassword( "abc123" )).withRequiredPasswordChange( true ).build() );
         final BasicAuthManager manager =
                 new BasicAuthManager( users, mock( PasswordPolicy.class ), mock( AuthenticationStrategy.class ) );
         manager.start();

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

@@ -83,7 +83,7 @@ public void shouldStoreAndRetriveUsersByName() throws Exception
     {
         // Given
         FileUserRepository users = new FileUserRepository( authFile, NullLogProvider.getInstance() );
-        User user = new User( "jake", Credential.INACCESSIBLE, true );
+        User user = new User.Builder( "jake", Credential.INACCESSIBLE ).withRequiredPasswordChange( true ).build();
         users.create( user );
 
         // When
@@ -98,7 +98,7 @@ public void shouldPersistUsers() throws Throwable
     {
         // Given
         FileUserRepository users = new FileUserRepository( authFile, NullLogProvider.getInstance() );
-        User user = new User( "jake", Credential.INACCESSIBLE, true );
+        User user = new User.Builder( "jake", Credential.INACCESSIBLE ).withRequiredPasswordChange( true ).build();
         users.create( user );
 
         users = new FileUserRepository( authFile, NullLogProvider.getInstance() );
@@ -116,7 +116,7 @@ public void shouldNotFindUserAfterDelete() throws Throwable
     {
         // Given
         FileUserRepository users = new FileUserRepository( authFile, NullLogProvider.getInstance() );
-        User user = new User( "jake", Credential.INACCESSIBLE, true );
+        User user = new User.Builder( "jake", Credential.INACCESSIBLE ).withRequiredPasswordChange( true ).build();
         users.create( user );
 
         // When
@@ -172,7 +172,7 @@ public void move( Path source, Path target, CopyOption... options ) throws IOExc
 
         FileUserRepository users = new FileUserRepository( authFile, NullLogProvider.getInstance() );
         users.start();
-        User user = new User( "jake", Credential.INACCESSIBLE, true );
+        User user = new User.Builder( "jake", Credential.INACCESSIBLE ).withRequiredPasswordChange( true ).build();
 
         // When
         try
@@ -194,11 +194,12 @@ public void shouldThrowIfUpdateChangesName() throws Throwable
     {
         // Given
         FileUserRepository users = new FileUserRepository( authFile, NullLogProvider.getInstance() );
-        User user = new User( "jake", Credential.INACCESSIBLE, true );
+        User user = new User.Builder( "jake", Credential.INACCESSIBLE ).withRequiredPasswordChange( true ).build();
         users.create( user );
 
         // When
-        User updatedUser = new User( "john", Credential.INACCESSIBLE, true );
+        User updatedUser = new User.Builder( "john", Credential.INACCESSIBLE ).withRequiredPasswordChange( true )
+                .build();
         try
         {
             users.update( user, updatedUser );
@@ -216,12 +217,12 @@ public void shouldThrowIfExistingUserDoesNotMatch() throws Throwable
     {
         // Given
         FileUserRepository users = new FileUserRepository( authFile, NullLogProvider.getInstance() );
-        User user = new User( "jake", Credential.INACCESSIBLE, true );
+        User user = new User.Builder( "jake", Credential.INACCESSIBLE ).withRequiredPasswordChange( true ).build();
         users.create( user );
-        User modifiedUser = new User( "jake", Credential.forPassword( "foo" ), false );
+        User modifiedUser = user.augment().withCredentials( Credential.forPassword( "foo" ) ).build();
 
         // When
-        User updatedUser = new User( "jake", Credential.forPassword( "bar" ), false );
+        User updatedUser = user.augment().withCredentials( Credential.forPassword( "bar" ) ).build();
         try
         {
             users.update( modifiedUser, updatedUser );
@@ -261,7 +262,7 @@ public void shouldFailOnReadingInvalidEntries() throws Throwable
             logProvider.assertExactly(
                     AssertableLogProvider.inLog( FileUserRepository.class ).error(
                             "Failed to read authentication file \"%s\" (%s)", authFile.toAbsolutePath(),
-                            "wrong number of line fields [line 2]"
+                            "wrong number of line fields, expected 3, got 4 [line 2]"
                     )
             );
             throw e;