From e392a0deb1e51a89c3a893deb4de07c76fa59c22 Mon Sep 17 00:00:00 2001 From: fickludd Date: Thu, 2 Jun 2016 16:29:44 +0200 Subject: [PATCH] Started working on suspending users. Boolean impl on User. --- .../org/neo4j/server/security/auth/User.java | 23 +++++++++++++-- .../security/auth/UserSerialization.java | 21 ++++++++++++-- .../enterprise/auth/AuthProcedures.java | 25 ++++++++++++++++ .../enterprise/auth/FileUserRealm.java | 20 +++++++++++++ .../enterprise/auth/ShiroAuthManager.java | 13 +++++++++ .../enterprise/auth/ShiroAuthSubject.java | 2 +- .../enterprise/auth/AuthProceduresTest.java | 29 +++++++++++++++++++ 7 files changed, 127 insertions(+), 6 deletions(-) diff --git a/community/security/src/main/java/org/neo4j/server/security/auth/User.java b/community/security/src/main/java/org/neo4j/server/security/auth/User.java index be25a2789ae6..cfc0664de3c7 100644 --- a/community/security/src/main/java/org/neo4j/server/security/auth/User.java +++ b/community/security/src/main/java/org/neo4j/server/security/auth/User.java @@ -38,11 +38,20 @@ public class User /** Whether a password change is needed */ private final boolean passwordChangeRequired; - public User(String name, Credential credential, boolean passwordChangeRequired) + /** User is suspended */ + private final boolean isSuspended; + + public User( String name, Credential credential, boolean passwordChangeRequired ) + { + this(name, credential, passwordChangeRequired, false); + } + + private User( String name, Credential credential, boolean passwordChangeRequired, boolean isSuspended ) { this.name = name; this.credential = credential; this.passwordChangeRequired = passwordChangeRequired; + this.isSuspended = isSuspended; } public String name() @@ -56,6 +65,7 @@ public Credential credentials() } public boolean passwordChangeRequired() { return passwordChangeRequired; } + public boolean isSuspended() { return isSuspended; } /** Use this user as a base for a new user object */ public Builder augment() { return new Builder(this); } @@ -78,6 +88,10 @@ public boolean equals( Object o ) { return false; } + if ( isSuspended != user.isSuspended ) + { + return false; + } if ( credential != null ? !credential.equals( user.credential ) : user.credential != null ) { return false; @@ -96,6 +110,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); return result; } @@ -106,6 +121,7 @@ public String toString() "name='" + name + '\'' + ", credentials=" + credential + ", passwordChangeRequired=" + passwordChangeRequired + + ", isSuspended=" + isSuspended + '}'; } @@ -114,6 +130,7 @@ public static class Builder private String name; private Credential credential = Credential.INACCESSIBLE; private boolean pwdChangeRequired; + private boolean isSuspended; public Builder() { } @@ -122,15 +139,17 @@ public Builder( User base ) name = base.name; credential = base.credential; pwdChangeRequired = base.passwordChangeRequired; + isSuspended = base.isSuspended; } 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 User build() { - return new User(name, credential, pwdChangeRequired ); + return new User(name, credential, pwdChangeRequired, isSuspended ); } } } diff --git a/community/security/src/main/java/org/neo4j/server/security/auth/UserSerialization.java b/community/security/src/main/java/org/neo4j/server/security/auth/UserSerialization.java index 833cf0a16ac1..6f5eaa2ab93b 100644 --- a/community/security/src/main/java/org/neo4j/server/security/auth/UserSerialization.java +++ b/community/security/src/main/java/org/neo4j/server/security/auth/UserSerialization.java @@ -73,20 +73,35 @@ private String serialize( User user ) { return join( userSeparator, user.name(), serialize( user.credentials() ), - user.passwordChangeRequired() ? "password_change_required" : "" ); + user.passwordChangeRequired() ? "password_change_required" : "", + user.isSuspended() ? "is_suspended" : "" ); } private User deserializeUser( String line, int lineNumber ) throws FormatException { String[] parts = line.split( userSeparator, -1 ); - if ( parts.length != 3 ) + boolean isSuspended; + if ( parts.length == 3 ) // Before suspension impl. assume non-suspended user { - throw new FormatException( format( "wrong number of line fields [line %d]", lineNumber ) ); + 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]", + parts.length, + lineNumber + ) ); + } + return new User.Builder() .withName( parts[0] ) .withCredentials( deserializeCredentials( parts[1], lineNumber ) ) .withRequiredPasswordChange( parts[2].equals( "password_change_required" ) ) + .withIsSuspended( isSuspended ) .build(); } diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java index be6863153145..72bbc0a237fd 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProcedures.java @@ -28,6 +28,7 @@ import org.neo4j.procedure.Name; import org.neo4j.procedure.PerformsDBMS; import org.neo4j.procedure.Procedure; +import org.neo4j.server.security.auth.exception.ConcurrentModificationException; public class AuthProcedures { @@ -86,4 +87,28 @@ public void deleteUser( @Name( "username" ) String username ) throws IllegalCred } shiroSubject.getUserManager().deleteUser( username ); } + + @PerformsDBMS + @Procedure( "dbms.suspendUser" ) + public void suspendUser( @Name( "username" ) String username ) throws IOException, ConcurrentModificationException + { + ShiroAuthSubject shiroSubject = ShiroAuthSubject.castOrFail( authSubject ); + if ( !shiroSubject.isAdmin() ) + { + throw new AuthorizationViolationException( PERMISSION_DENIED ); + } + shiroSubject.getUserManager().suspendUser( username ); + } + + @PerformsDBMS + @Procedure( "dbms.activateUser" ) + public void activateUser( @Name( "username" ) String username ) throws IOException, ConcurrentModificationException + { + ShiroAuthSubject shiroSubject = ShiroAuthSubject.castOrFail( authSubject ); + if ( !shiroSubject.isAdmin() ) + { + throw new AuthorizationViolationException( PERMISSION_DENIED ); + } + shiroSubject.getUserManager().activateUser( username ); + } } diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java index dba4bff8b64b..5af819739b8a 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/FileUserRealm.java @@ -270,6 +270,26 @@ boolean deleteUser( String username ) throws IOException return result; } + void suspendUser( String username ) throws IOException, ConcurrentModificationException + { + User user = userRepository.findByName( username ); + if ( user != null && !user.isSuspended() ) + { + User suspendedUser = user.augment().withIsSuspended( true ).build(); + userRepository.update( user, suspendedUser ); + } + } + + void activateUser( String username ) throws IOException, ConcurrentModificationException + { + User user = userRepository.findByName( username ); + if ( user != null && user.isSuspended() ) + { + User activatedUser = user.augment().withIsSuspended( false ).build(); + userRepository.update( user, activatedUser ); + } + } + private void removeUserFromAllRoles( String username ) throws IOException { try diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManager.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManager.java index 2dd13eec9389..9c13bd625478 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManager.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthManager.java @@ -42,6 +42,7 @@ import org.neo4j.server.security.auth.RateLimitedAuthenticationStrategy; import org.neo4j.server.security.auth.User; import org.neo4j.server.security.auth.UserRepository; +import org.neo4j.server.security.auth.exception.ConcurrentModificationException; public class ShiroAuthManager extends BasicAuthManager implements RoleManager { @@ -216,6 +217,18 @@ public boolean deleteUser( String username ) throws IOException return realm.deleteUser( username ); } + void suspendUser( String username ) throws IOException, ConcurrentModificationException + { + assertAuthEnabled(); + realm.suspendUser( username ); + } + + void activateUser( String username ) throws IOException, ConcurrentModificationException + { + assertAuthEnabled(); + realm.activateUser( username ); + } + private Subject buildSubject( String username ) { Subject.Builder subjectBuilder = new Subject.Builder( securityManager ); diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthSubject.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthSubject.java index 24e833d13820..0f6598efe89f 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthSubject.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/ShiroAuthSubject.java @@ -78,7 +78,7 @@ public RoleManager getRoleManager() return authManager; } - public UserManager getUserManager() + public ShiroAuthManager getUserManager() { return authManager; } diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java index bfd478d4871a..89024c960717 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/AuthProceduresTest.java @@ -543,6 +543,35 @@ public void userDeletion4() throws Exception } } + //----------User suspension/activation scenarios----------- + + /* + Admin suspends user Henrik + User Henrik logs in with correct password → fail + */ + @Test + public void userSuspension1() throws Exception + { + testCallEmpty( db, adminSubject, "CALL dbms.suspendUser('Henrik')", null ); + AuthSubject subject = manager.login( "Henrik", "bar" ); + assertEquals( AuthenticationResult.FAILURE, subject.getAuthenticationResult() ); + } + + /* + Admin suspends user Henrik + User Henrik logs in with correct password → fail + */ + @Test + public void userActivation1() throws Exception + { + testCallEmpty( db, adminSubject, "CALL dbms.suspendUser('Henrik')", null ); + AuthSubject subject = manager.login( "Henrik", "bar" ); + assertEquals( AuthenticationResult.FAILURE, subject.getAuthenticationResult() ); + testCallEmpty( db, adminSubject, "CALL dbms.activateUser('Henrik')", null ); + subject = manager.login( "Henrik", "bar" ); + assertEquals( AuthenticationResult.SUCCESS, subject.getAuthenticationResult() ); + } + //-------------Helper functions--------------- private void testSuccessfulReadAction( AuthSubject subject, Long count )