Merge pull request #8574 from fickludd/3.1-ldap-authr-only-bug

Augment showCurrentUser and LDAP bugfix

community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/Neo4jWithSocket.java

@@ -110,7 +110,10 @@ public void shutdownDatabase()
     {
         try
         {
-            gdb.shutdown();
+            if ( gdb != null)
+            {
+                gdb.shutdown();
+            }
         }
         finally
         {

enterprise/kernel/src/main/java/org/neo4j/kernel/enterprise/api/security/EnterpriseSecurityContext.java

@@ -19,6 +19,9 @@
  */
 package org.neo4j.kernel.enterprise.api.security;
 
+import java.util.Collections;
+import java.util.Set;
+
 import org.neo4j.kernel.api.security.AccessMode;
 import org.neo4j.kernel.api.security.AuthSubject;
 import org.neo4j.kernel.api.security.SecurityContext;
@@ -34,6 +37,8 @@ public interface EnterpriseSecurityContext extends SecurityContext
     @Override
     EnterpriseSecurityContext withMode( AccessMode mode );
 
+    Set<String> roles();
+
     EnterpriseSecurityContext AUTH_DISABLED = new AuthDisabled( AccessMode.Static.FULL );
 
     /** Allows all operations. */
@@ -58,6 +63,12 @@ public EnterpriseSecurityContext withMode( AccessMode mode )
             return new EnterpriseSecurityContext.AuthDisabled( mode );
         }
 
+        @Override
+        public Set<String> roles()
+        {
+            return Collections.emptySet();
+        }
+
         @Override
         public AuthSubject subject()
         {
@@ -93,12 +104,14 @@ final class Frozen implements EnterpriseSecurityContext
     {
         private final AuthSubject subject;
         private final AccessMode mode;
+        private final Set<String> roles;
         private final boolean isAdmin;
 
-        public Frozen( AuthSubject subject, AccessMode mode, boolean isAdmin )
+        public Frozen( AuthSubject subject, AccessMode mode, Set<String> roles, boolean isAdmin )
         {
             this.subject = subject;
             this.mode = mode;
+            this.roles = roles;
             this.isAdmin = isAdmin;
         }
 
@@ -129,7 +142,13 @@ public EnterpriseSecurityContext freeze()
         @Override
         public EnterpriseSecurityContext withMode( AccessMode mode )
         {
-            return new EnterpriseSecurityContext.Frozen( subject, mode, isAdmin );
+            return new EnterpriseSecurityContext.Frozen( subject, mode, roles, isAdmin );
+        }
+
+        @Override
+        public Set<String> roles()
+        {
+            return roles;
         }
     }
 }

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

@@ -112,12 +112,27 @@ public static class StringResult
         }
     }
 
-    protected UserResult userResultForName( String username )
+    protected UserResult userResultForSubject()
     {
+        String username = securityContext.subject().username();
         User user = userManager.silentlyGetUser( username );
         Iterable<String> flags = user == null ? emptyList() : user.getFlags();
-        Set<String> roles = userManager.silentlyGetRoleNamesForUser( username );
-        return new UserResult( username, roles, flags );
+        return new UserResult( username, securityContext.roles(), flags );
+    }
+
+    protected UserResult userResultForName( String username )
+    {
+        if ( username.equals( securityContext.subject().username() ) )
+        {
+            return userResultForSubject();
+        }
+        else
+        {
+            User user = userManager.silentlyGetUser( username );
+            Iterable<String> flags = user == null ? emptyList() : user.getFlags();
+            Set<String> roles = userManager.silentlyGetRoleNamesForUser( username );
+            return new UserResult( username, roles, flags );
+        }
     }
 
     public static class UserResult

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

@@ -246,43 +246,44 @@ protected AuthorizationInfo queryForAuthorizationInfo( PrincipalCollection princ
     {
         if ( authorizationEnabled )
         {
-            Collection realmPrincipals = principals.fromRealm( getName() );
-            if (!CollectionUtils.isEmpty(realmPrincipals))
+            String username = (String) getAvailablePrincipal( principals );
+            if ( username == null )
             {
-                String username = (String) realmPrincipals.iterator().next();
-                if ( useSystemAccountForAuthorization )
-                {
-                    // Perform context search using the system context
-                    LdapContext ldapContext = useStartTls ? getSystemLdapContextUsingStartTls( ldapContextFactory ) :
-                                              ldapContextFactory.getSystemLdapContext();
+                return null;
+            }
 
-                    Set<String> roleNames;
-                    try
-                    {
-                        roleNames = findRoleNamesForUser( username, ldapContext );
-                    }
-                    finally
-                    {
-                        LdapUtils.closeContext( ldapContext );
-                    }
+            if ( useSystemAccountForAuthorization )
+            {
+                // Perform context search using the system context
+                LdapContext ldapContext = useStartTls ? getSystemLdapContextUsingStartTls( ldapContextFactory ) :
+                                          ldapContextFactory.getSystemLdapContext();
 
-                    return new SimpleAuthorizationInfo( roleNames );
+                Set<String> roleNames;
+                try
+                {
+                    roleNames = findRoleNamesForUser( username, ldapContext );
                 }
-                else
+                finally
                 {
-                    // Authorization info is cached during authentication
-                    Cache<Object,AuthorizationInfo> authorizationCache = getAuthorizationCache();
-                    AuthorizationInfo authorizationInfo = authorizationCache.get( username );
-                    if ( authorizationInfo == null )
-                    {
-                        // The cached authorization info has expired.
-                        // Since we do not have the subject's credentials we cannot perform a new LDAP search
-                        // for authorization info. Instead we need to fail with a special status,
-                        // so that the client can react by re-authenticating.
-                        throw new AuthorizationExpiredException( "LDAP authorization info expired." );
-                    }
-                    return authorizationInfo;
+                    LdapUtils.closeContext( ldapContext );
+                }
+
+                return new SimpleAuthorizationInfo( roleNames );
+            }
+            else
+            {
+                // Authorization info is cached during authentication
+                Cache<Object,AuthorizationInfo> authorizationCache = getAuthorizationCache();
+                AuthorizationInfo authorizationInfo = authorizationCache.get( username );
+                if ( authorizationInfo == null )
+                {
+                    // The cached authorization info has expired.
+                    // Since we do not have the subject's credentials we cannot perform a new LDAP search
+                    // for authorization info. Instead we need to fail with a special status,
+                    // so that the client can react by re-authenticating.
+                    throw new AuthorizationExpiredException( "LDAP authorization info expired." );
                 }
+                return authorizationInfo;
             }
         }
         return null;

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

@@ -42,7 +42,7 @@ public class SecurityProcedures extends AuthProceduresBase
     @Procedure( name = "dbms.security.showCurrentUser", mode = DBMS )
     public Stream<UserManagementProcedures.UserResult> showCurrentUser() throws InvalidArgumentsException, IOException
     {
-        return Stream.of( userResultForName( securityContext.subject().username() ) );
+        return Stream.of( userResultForSubject() );
     }
 
     @Description( "Clears authentication and authorization cache." )

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

@@ -70,15 +70,15 @@ public AuthSubject subject()
     }
 
     @Override
-    public AccessMode mode()
+    public StandardAccessMode mode()
     {
         boolean isAuthenticated = shiroSubject.isAuthenticated();
         return new StandardAccessMode(
                 isAuthenticated && shiroSubject.isPermitted( READ ),
                 isAuthenticated && shiroSubject.isPermitted( READ_WRITE ),
                 isAuthenticated && shiroSubject.isPermitted( SCHEMA_READ_WRITE ),
                 shiroSubject.getAuthenticationResult() == AuthenticationResult.PASSWORD_CHANGE_REQUIRED,
-                authManager.getAuthorizationInfo( shiroSubject.getPrincipals() )
+                queryForRoleNames()
             );
     }
 
@@ -91,13 +91,32 @@ public String toString()
     @Override
     public EnterpriseSecurityContext freeze()
     {
-        return new Frozen( neoShiroSubject, mode(), isAdmin() );
+        StandardAccessMode mode = mode();
+        return new Frozen( neoShiroSubject, mode, mode.roles, isAdmin() );
     }
 
     @Override
     public EnterpriseSecurityContext withMode( AccessMode mode )
     {
-        return new Frozen( neoShiroSubject, mode, isAdmin() );
+        return new Frozen( neoShiroSubject, mode, queryForRoleNames(), isAdmin() );
+    }
+
+    @Override
+    public Set<String> roles()
+    {
+        return queryForRoleNames();
+    }
+
+    private Set<String> queryForRoleNames()
+    {
+        Collection<AuthorizationInfo> authorizationInfo =
+                authManager.getAuthorizationInfo( shiroSubject.getPrincipals() );
+        return authorizationInfo.stream()
+                .flatMap( authInfo -> {
+                    Collection<String> roles = authInfo.getRoles();
+                    return roles == null ? Stream.empty() : roles.stream();
+                } )
+                .collect( Collectors.toSet() );
     }
 
     private static class StandardAccessMode implements AccessMode
@@ -106,16 +125,16 @@ private static class StandardAccessMode implements AccessMode
         private final boolean allowsWrites;
         private final boolean allowsSchemaWrites;
         private final boolean passwordChangeRequired;
-        private Collection<AuthorizationInfo> authorizationInfoSnapshot;
+        private final Set<String> roles;
 
         StandardAccessMode( boolean allowsReads, boolean allowsWrites, boolean allowsSchemaWrites,
-                boolean passwordChangeRequired, Collection<AuthorizationInfo> authorizationInfo )
+                boolean passwordChangeRequired, Set<String> roles )
         {
             this.allowsReads = allowsReads;
             this.allowsWrites = allowsWrites;
             this.allowsSchemaWrites = allowsSchemaWrites;
             this.passwordChangeRequired = passwordChangeRequired;
-            authorizationInfoSnapshot = authorizationInfo;
+            this.roles = roles;
         }
 
         @Override
@@ -139,7 +158,6 @@ public boolean allowsSchemaWrites()
         @Override
         public boolean allowsProcedureWith( String[] roleNames ) throws InvalidArgumentsException
         {
-            Set<String> roles = roleNames();
             for ( int i = 0; i < roleNames.length; i++ )
             {
                 if ( roles.contains( roleNames[i] ) )
@@ -166,18 +184,8 @@ public AuthorizationViolationException onViolation( String msg )
         @Override
         public String name()
         {
-            Set<String> roles = new TreeSet<>( roleNames() );
-            return roles.isEmpty() ? "no roles" : "roles [" + String.join( ",", roles ) + "]";
-        }
-
-        private Set<String> roleNames()
-        {
-            return authorizationInfoSnapshot.stream()
-                    .flatMap( authInfo -> {
-                        Collection<String> roles = authInfo.getRoles();
-                        return roles == null ? Stream.empty() : roles.stream();
-                    } )
-                    .collect( Collectors.toSet() );
+            Set<String> sortedRoles = new TreeSet<>( roles );
+            return roles.isEmpty() ? "no roles" : "roles [" + String.join( ",", sortedRoles ) + "]";
         }
     }
 

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

@@ -124,7 +124,7 @@ public Stream<UserResult> listUsers() throws InvalidArgumentsException, IOExcept
         Set<String> users = userManager.getAllUsernames();
         if ( users.isEmpty() )
         {
-            return Stream.of( userResultForName( securityContext.subject().username() ) );
+            return Stream.of( userResultForSubject() );
         }
         else
         {

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

@@ -23,6 +23,7 @@
 
 import java.util.Collections;
 import java.util.Map;
+import java.util.Set;
 
 import org.neo4j.graphdb.QueryExecutionException;
 import org.neo4j.graphdb.Result;
@@ -104,7 +105,13 @@ public EnterpriseSecurityContext freeze()
             @Override
             public EnterpriseSecurityContext withMode( AccessMode mode )
             {
-                return new EnterpriseSecurityContext.Frozen( subject(), mode, isAdmin() );
+                return new EnterpriseSecurityContext.Frozen( subject(), mode, roles(), isAdmin() );
+            }
+
+            @Override
+            public Set<String> roles()
+            {
+                return Collections.emptySet();
             }
 
             AnonymousContext inner = AnonymousContext.none();

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

@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2002-2017 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.server.security.enterprise.auth;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.neo4j.kernel.api.exceptions.InvalidArgumentsException;
+import org.neo4j.kernel.api.security.AuthSubject;
+import org.neo4j.kernel.enterprise.api.security.EnterpriseSecurityContext;
+import org.neo4j.server.security.enterprise.auth.AuthProceduresBase.UserResult;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.equalTo;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class SecurityProceduresTest
+{
+
+    private SecurityProcedures procedures;
+
+    @Before
+    public void setup()
+    {
+        AuthSubject subject = mock( AuthSubject.class );
+        when( subject.username() ).thenReturn( "pearl" );
+
+        EnterpriseSecurityContext ctx = mock( EnterpriseSecurityContext.class );
+        when( ctx.subject() ).thenReturn( subject );
+        when( ctx.roles() ).thenReturn( Collections.singleton( "jammer" ) );
+
+        procedures = new SecurityProcedures();
+        procedures.securityContext = ctx;
+        procedures.userManager = mock( EnterpriseUserManager.class );
+    }
+
+    @Test
+    public void shouldReturnSecurityContextRoles() throws IOException, InvalidArgumentsException
+    {
+        List<UserResult> infoList = procedures.showCurrentUser().collect( Collectors.toList() );
+        assertThat( infoList.size(), equalTo(1) );
+
+        UserResult row = infoList.get( 0 );
+        assertThat( row.username, equalTo( "pearl" ) );
+        assertThat( row.roles, containsInAnyOrder( "jammer" ) );
+        assertThat( row.flags, empty() );
+    }
+}