Refactor password policy

- Set password policy for ShiroAuthManager - Simplify UserAccount to just use SimpleAuthorizationInfo
neo4j · May 25, 2016 · 8eb0cd8 · 8eb0cd8
1 parent 2f89598
commit 8eb0cd8
Show file tree

Hide file tree

Showing 10 changed files with 49 additions and 24 deletions.
diff --git a/community/security/src/main/java/org/neo4j/server/security/auth/BasicAuthManager.java b/community/security/src/main/java/org/neo4j/server/security/auth/BasicAuthManager.java
@@ -151,7 +151,7 @@ public void setPassword( AuthSubject authSubject, String username, String passwo
             throw new AuthorizationViolationException( "Invalid attempt to change the password for user " + username );
         }
 
-        passwordPolicy.validatePassword( authSubject, password );
+        passwordPolicy.validatePassword( authSubject, password, basicAuthSubject::credentialsMatchesPassword );
 
         setUserPassword( username, password );
     }
@@ -169,7 +169,7 @@ public void setUserPassword( String username, String password ) throws IOExcepti
 
         if ( existingUser.credentials().matchesPassword( password ) )
         {
-            return;
+            throw new IllegalCredentialsException( "Old password and new password cannot be the same." );
         }
 
         try

diff --git a/community/security/src/main/java/org/neo4j/server/security/auth/BasicPasswordPolicy.java b/community/security/src/main/java/org/neo4j/server/security/auth/BasicPasswordPolicy.java
@@ -25,16 +25,15 @@
 public class BasicPasswordPolicy implements PasswordPolicy
 {
     @Override
-    public void validatePassword( AuthSubject authSubject, String password ) throws IllegalCredentialsException
+    public void validatePassword( AuthSubject authSubject, String password, CredentialsMatcher credentialsMatcher )
+            throws IllegalCredentialsException
     {
         if ( password == null || password.isEmpty() )
         {
             throw new IllegalCredentialsException( "Password cannot be empty." );
         }
 
-        BasicAuthSubject basicAuthSubject = BasicAuthSubject.castOrFail( authSubject );
-
-        if ( basicAuthSubject.credentialsMatchesPassword( password ) )
+        if ( credentialsMatcher.credentialsMatchesPassword( password ) )
         {
             throw new IllegalCredentialsException( "Old password and new password cannot be the same." );
         }

diff --git a/community/security/src/main/java/org/neo4j/server/security/auth/PasswordPolicy.java b/community/security/src/main/java/org/neo4j/server/security/auth/PasswordPolicy.java
@@ -24,5 +24,11 @@
 
 public interface PasswordPolicy
 {
-    void validatePassword( AuthSubject authSubject, String password ) throws IllegalCredentialsException;
+    interface CredentialsMatcher
+    {
+        boolean credentialsMatchesPassword( String password );
+    }
+
+    void validatePassword( AuthSubject authSubject, String password, CredentialsMatcher credentialsMatcher )
+            throws IllegalCredentialsException;
 }
diff --git a/...rise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AccountBuilder.java b/...rise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AccountBuilder.java
@@ -23,5 +23,5 @@
 
 public interface AccountBuilder
 {
-    UserAccount buildAccount(User user, String realmName);
+    UserAccount buildAccount(User user);
 }
diff --git a/...src/main/java/org/neo4j/server/security/enterprise/auth/EnterpriseAuthManagerFactory.java b/...src/main/java/org/neo4j/server/security/enterprise/auth/EnterpriseAuthManagerFactory.java
@@ -24,6 +24,7 @@
 import org.neo4j.kernel.api.security.AuthManager;
 import org.neo4j.kernel.configuration.Config;
 import org.neo4j.logging.LogProvider;
+import org.neo4j.server.security.auth.BasicPasswordPolicy;
 import org.neo4j.server.security.auth.FileUserRepository;
 import org.neo4j.server.security.auth.PasswordPolicy;
 import org.neo4j.server.security.auth.UserRepository;
@@ -47,8 +48,7 @@ public AuthManager newInstance( Config config, LogProvider logProvider )
         final UserRepository userRepository =
                 new FileUserRepository( config.get( GraphDatabaseSettings.auth_store ).toPath(), logProvider );
 
-        // TODO
-        final PasswordPolicy passwordPolicy = ( authSubject, password ) -> {};
+        final PasswordPolicy passwordPolicy = new BasicPasswordPolicy();
 
         return new EnterpriseAuthManager( userRepository, passwordPolicy, systemUTC(),
                 config.get( GraphDatabaseSettings.auth_enabled ) );

diff --git a/...prise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java b/...prise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java
@@ -74,7 +74,7 @@ protected AuthorizationInfo doGetAuthorizationInfo( PrincipalCollection principa
     {
         User user = userRepository.findByName( (String) principals.getPrimaryPrincipal() );
 
-        return accountBuilder.buildAccount(user, getName());
+        return accountBuilder.buildAccount( user );
     }
 
     @Override
@@ -84,12 +84,17 @@ protected AuthenticationInfo doGetAuthenticationInfo( AuthenticationToken token
 
         User user = userRepository.findByName( usernamePasswordToken.getUsername() );
 
+        if ( user == null )
+        {
+            throw new AuthenticationException( "User " + usernamePasswordToken.getUsername() + " does not exist" );
+        }
+
         // TODO: This will not work if AuthenticationInfo is cached,
         // unless you always do SecurityManager.logout properly (which will invalidate the cache)
         // For REST we may need to connect HttpSessionListener.sessionDestroyed with logout
-        if (user.passwordChangeRequired())
+        if ( user.passwordChangeRequired() )
         {
-            throw new ExpiredCredentialsException("Password change required");
+            throw new ExpiredCredentialsException( "Password change required" );
         }
 
         return new SimpleAuthenticationInfo( user.name(), user.credentials(), getName() );
@@ -124,4 +129,16 @@ private void assertValidName( String name )
                     "User name contains illegal characters. Please use simple ascii characters and numbers." );
         }
     }
+
+    boolean credentialsMatchesPassword( String username, String password )
+    {
+        User user = userRepository.findByName( username );
+
+        if ( user != null )
+        {
+            return user.credentials().matchesPassword( password );
+        }
+
+        return false;
+    }
 }
diff --git a/...c/main/java/org/neo4j/server/security/enterprise/auth/PredefinedGroupsAccountBuilder.java b/...c/main/java/org/neo4j/server/security/enterprise/auth/PredefinedGroupsAccountBuilder.java
@@ -28,10 +28,10 @@
 public class PredefinedGroupsAccountBuilder implements AccountBuilder
 {
     @Override
-    public UserAccount buildAccount( User user, String realmName )
+    public UserAccount buildAccount( User user )
     {
         Set<String> roleNames = getRolesForGroup( user.group() );
-        return new UserAccount( user.name(), user.credentials(), realmName, roleNames );
+        return new UserAccount( roleNames );
     }
 
     private Set<String> getRolesForGroup( String group )

diff --git a/...se/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManager.java b/...se/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManager.java
@@ -157,7 +157,8 @@ public void setPassword( AuthSubject authSubject, String username, String passwo
             throw new AuthorizationViolationException( "Invalid attempt to change the password for user " + username );
         }
 
-        passwordPolicy.validatePassword( authSubject, password );
+        passwordPolicy.validatePassword( authSubject, password,
+                pass -> realm.credentialsMatchesPassword( username, pass ) );
 
         setUserPassword( username, password );
     }

diff --git a/...se/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthSubject.java b/...se/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthSubject.java
@@ -66,7 +66,7 @@ public void logout()
     @Override
     public void setPassword( String password ) throws IOException, IllegalCredentialsException
     {
-        authManager.setPassword( this, (String) subject.getPrincipals().getPrimaryPrincipal(), password );
+        authManager.setPassword( this, (String) subject.getPrincipal(), password );
     }
 
     public boolean doesUsernameMatch( String username )
@@ -98,6 +98,11 @@ public String name()
         return "AUTH";
     }
 
+    Subject getSubject()
+    {
+        return subject;
+    }
+
     private AccessMode.Static getAccesMode()
     {
         if ( subject.isAuthenticated() )

diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/UserAccount.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/UserAccount.java
@@ -19,17 +19,14 @@
  */
 package org.neo4j.server.security.enterprise.auth;
 
-import org.apache.shiro.authc.SimpleAccount;
+import org.apache.shiro.authz.SimpleAuthorizationInfo;
 
-import java.util.Collections;
 import java.util.Set;
 
-import org.neo4j.server.security.auth.Credential;
-
-class UserAccount extends SimpleAccount
+class UserAccount extends SimpleAuthorizationInfo
 {
-    public UserAccount( String name, Credential credentials, String realm, Set<String> roleNames )
+    public UserAccount( Set<String> roleNames )
     {
-        super( Collections.singletonList( name ), credentials, realm, roleNames, null );
+        super( roleNames );
     }
 }