From ec68aac33141eef88806c94676c299394dfeafb7 Mon Sep 17 00:00:00 2001 From: fickludd Date: Fri, 14 Oct 2016 11:47:38 +0200 Subject: [PATCH] Change error message for PERMISSION_DENIED We augment the error message on permission denied to detail both the AuthSubject (when available) and the full AccessMode, including roles in enterprise. --- .../cypher/internal/ExecutionEngine.scala | 2 +- .../kernel/api/security/AnonymousContext.java | 2 +- .../kernel/api/security/SecurityContext.java | 38 +++-- .../kernel/impl/api/KernelStatement.java | 4 +- .../kernel/impl/api/OperationsFacade.java | 4 +- .../security/auth/BasicSecurityContext.java | 2 +- .../auth/SecurityContextDescriptionTest.java | 115 ++++++++++++++++ .../security/EnterpriseSecurityContext.java | 32 +++-- .../StandardEnterpriseSecurityContext.java | 33 +++-- ...eddedBuiltInProceduresInteractionTest.java | 2 +- ...erpriseSecurityContextDescriptionTest.java | 130 ++++++++++++++++++ .../EnterpriseAuthenticationTestBase.java | 34 +++-- .../bolt/LdapAuthenticationIT.java | 79 ++--------- .../bolt/PluginAuthenticationIT.java | 12 +- .../java/org/neo4j/procedure/ProcedureIT.java | 9 +- 15 files changed, 370 insertions(+), 128 deletions(-) create mode 100644 community/security/src/test/java/org/neo4j/server/security/auth/SecurityContextDescriptionTest.java create mode 100644 enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityContextDescriptionTest.java diff --git a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/ExecutionEngine.scala b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/ExecutionEngine.scala index c32216c0c8a5a..0174443599f88 100644 --- a/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/ExecutionEngine.scala +++ b/community/cypher/cypher/src/main/scala/org/neo4j/cypher/internal/ExecutionEngine.scala @@ -151,7 +151,7 @@ class ExecutionEngine(val queryService: GraphDatabaseQueryService, logProvider: // Temporarily change access mode during query planning // NOTE: This will force read allowance if the current transaction did not have it - val revertable = tc.restrictCurrentTransaction(tc.securityContext.freeze(AccessMode.Static.READ)) + val revertable = tc.restrictCurrentTransaction(tc.securityContext.withMode(AccessMode.Static.READ)) val ((plan: ExecutionPlan, extractedParameters), touched) = try { // fetch plan cache diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/security/AnonymousContext.java b/community/kernel/src/main/java/org/neo4j/kernel/api/security/AnonymousContext.java index 3be6d07264c94..4f5962eb4ba8a 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/security/AnonymousContext.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/security/AnonymousContext.java @@ -62,7 +62,7 @@ public SecurityContext freeze() } @Override - public SecurityContext freeze( AccessMode mode ) + public SecurityContext withMode( AccessMode mode ) { return new Frozen( subject(), mode ); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/security/SecurityContext.java b/community/kernel/src/main/java/org/neo4j/kernel/api/security/SecurityContext.java index 6793ab80b7010..1e40112309010 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/security/SecurityContext.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/security/SecurityContext.java @@ -26,7 +26,12 @@ public interface SecurityContext AuthSubject subject(); SecurityContext freeze(); - SecurityContext freeze( AccessMode mode ); + SecurityContext withMode( AccessMode mode ); + + default String description() + { + return String.format( "user '%s' with %s", subject().username(), mode().name() ); + } default String defaultString( String name ) { @@ -34,12 +39,27 @@ default String defaultString( String name ) } /** Allows all operations. */ - SecurityContext AUTH_DISABLED = new SecurityContext() + SecurityContext AUTH_DISABLED = new AuthDisabled(AccessMode.Static.FULL); + + final class AuthDisabled implements SecurityContext { + private final AccessMode mode; + + private AuthDisabled( AccessMode mode ) + { + this.mode = mode; + } + @Override public AccessMode mode() { - return AccessMode.Static.FULL; + return mode; + } + + @Override + public AuthSubject subject() + { + return AuthSubject.AUTH_DISABLED; } @Override @@ -49,9 +69,9 @@ public String toString() } @Override - public AuthSubject subject() + public String description() { - return AuthSubject.AUTH_DISABLED; + return "AUTH_DISABLED with " + mode.name(); } @Override @@ -61,13 +81,13 @@ public SecurityContext freeze() } @Override - public SecurityContext freeze( AccessMode mode ) + public SecurityContext withMode( AccessMode mode ) { - return new Frozen( subject(), mode ); + return new AuthDisabled( mode ); } }; - class Frozen implements SecurityContext + final class Frozen implements SecurityContext { private final AuthSubject subject; private final AccessMode mode; @@ -97,7 +117,7 @@ public SecurityContext freeze() } @Override - public SecurityContext freeze( AccessMode mode ) + public SecurityContext withMode( AccessMode mode ) { return new Frozen( subject, mode ); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelStatement.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelStatement.java index 46ba84e892f7d..196ba3d34269a 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelStatement.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelStatement.java @@ -255,8 +255,8 @@ private void assertAllows( Function allows, String mode ) if ( !allows.apply( accessMode ) ) { throw accessMode.onViolation( - String.format( "%s operations are not allowed for '%s'.", mode, transaction.securityContext() - .subject().username() ) ); + String.format( "%s operations are not allowed for %s.", mode, + transaction.securityContext().description() ) ); } } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java index e4f332f78f63b..c3f53e6ec0692 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java @@ -1553,7 +1553,7 @@ private RawIterator callProcedure( { statement.assertOpen(); - final SecurityContext procedureSecurityContext = tx.securityContext().freeze( override ); + final SecurityContext procedureSecurityContext = tx.securityContext().withMode( override ); final RawIterator procedureCall; try ( KernelTransaction.Revertable ignore = tx.overrideWith( procedureSecurityContext ) ) { @@ -1609,7 +1609,7 @@ private Object callFunction( { statement.assertOpen(); - try ( KernelTransaction.Revertable ignore = tx.overrideWith( tx.securityContext().freeze( mode ) ) ) + try ( KernelTransaction.Revertable ignore = tx.overrideWith( tx.securityContext().withMode( mode ) ) ) { BasicContext ctx = new BasicContext(); ctx.put( Context.KERNEL_TRANSACTION, tx ); diff --git a/community/security/src/main/java/org/neo4j/server/security/auth/BasicSecurityContext.java b/community/security/src/main/java/org/neo4j/server/security/auth/BasicSecurityContext.java index 0896abdb86668..2f75a28957133 100644 --- a/community/security/src/main/java/org/neo4j/server/security/auth/BasicSecurityContext.java +++ b/community/security/src/main/java/org/neo4j/server/security/auth/BasicSecurityContext.java @@ -125,7 +125,7 @@ public SecurityContext freeze() } @Override - public SecurityContext freeze( AccessMode mode ) + public SecurityContext withMode( AccessMode mode ) { return new Frozen( authSubject, mode ); } diff --git a/community/security/src/test/java/org/neo4j/server/security/auth/SecurityContextDescriptionTest.java b/community/security/src/test/java/org/neo4j/server/security/auth/SecurityContextDescriptionTest.java new file mode 100644 index 0000000000000..aa56cef0a1ff4 --- /dev/null +++ b/community/security/src/test/java/org/neo4j/server/security/auth/SecurityContextDescriptionTest.java @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2002-2016 "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 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 General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.server.security.auth; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import org.neo4j.kernel.api.security.AccessMode; +import org.neo4j.kernel.api.security.SecurityContext; +import org.neo4j.kernel.impl.api.security.OverriddenAccessMode; +import org.neo4j.kernel.impl.api.security.RestrictedAccessMode; +import org.neo4j.time.Clocks; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.neo4j.server.security.auth.SecurityTestUtils.authToken; + +public class SecurityContextDescriptionTest +{ + private BasicAuthManager manager; + private SecurityContext context; + + @Before + public void setup() throws Throwable + { + manager = + new BasicAuthManager( + new InMemoryUserRepository(), + new BasicPasswordPolicy(), + Clocks.systemClock(), + new InMemoryUserRepository() ); + manager.init(); + manager.start(); + manager.newUser( "johan", "bar", false ); + context = manager.login( authToken( "johan", "bar" ) ); + } + + @After + public void teardown() throws Throwable + { + manager.stop(); + manager.shutdown(); + } + + @Test + public void shouldMakeNiceDescription() throws Throwable + { + assertThat( context.description(), equalTo( "user 'johan' with FULL" ) ); + } + + @Test + public void shouldMakeNiceDescriptionFrozen() throws Throwable + { + SecurityContext frozen = context.freeze(); + assertThat( frozen.description(), equalTo( "user 'johan' with FULL" ) ); + } + + @Test + public void shouldMakeNiceDescriptionWithMode() throws Throwable + { + SecurityContext modified = context.withMode( AccessMode.Static.WRITE ); + assertThat( modified.description(), equalTo( "user 'johan' with WRITE" ) ); + } + + @Test + public void shouldMakeNiceDescriptionRestricted() throws Throwable + { + SecurityContext restricted = + context.withMode( new RestrictedAccessMode( context.mode(), AccessMode.Static.READ ) ); + assertThat( restricted.description(), equalTo( "user 'johan' with FULL restricted to READ" ) ); + } + + @Test + public void shouldMakeNiceDescriptionOverridden() throws Throwable + { + SecurityContext overridden = + context.withMode( new OverriddenAccessMode( context.mode(), AccessMode.Static.READ ) ); + assertThat( overridden.description(), equalTo( "user 'johan' with FULL overridden by READ" ) ); + } + + @Test + public void shouldMakeNiceDescriptionAuthDisabled() throws Throwable + { + SecurityContext disabled = SecurityContext.AUTH_DISABLED; + assertThat( disabled.description(), equalTo( "AUTH_DISABLED with FULL" ) ); + } + + @Test + public void shouldMakeNiceDescriptionAuthDisabledAndRestricted() throws Throwable + { + SecurityContext disabled = SecurityContext.AUTH_DISABLED; + SecurityContext restricted = + disabled.withMode( new RestrictedAccessMode( disabled.mode(), AccessMode.Static.READ ) ); + assertThat( restricted.description(), equalTo( "AUTH_DISABLED with FULL restricted to READ" ) ); + } + +} diff --git a/enterprise/kernel/src/main/java/org/neo4j/kernel/enterprise/api/security/EnterpriseSecurityContext.java b/enterprise/kernel/src/main/java/org/neo4j/kernel/enterprise/api/security/EnterpriseSecurityContext.java index d51d2d406c85d..7c49d75f6ee02 100644 --- a/enterprise/kernel/src/main/java/org/neo4j/kernel/enterprise/api/security/EnterpriseSecurityContext.java +++ b/enterprise/kernel/src/main/java/org/neo4j/kernel/enterprise/api/security/EnterpriseSecurityContext.java @@ -32,10 +32,20 @@ public interface EnterpriseSecurityContext extends SecurityContext, CouldBeAdmin EnterpriseSecurityContext freeze(); @Override - EnterpriseSecurityContext freeze( AccessMode mode ); + EnterpriseSecurityContext withMode( AccessMode mode ); - EnterpriseSecurityContext AUTH_DISABLED = new EnterpriseSecurityContext() + EnterpriseSecurityContext AUTH_DISABLED = new AuthDisabled( AccessMode.Static.FULL ); + + /** Allows all operations. */ + final class AuthDisabled implements EnterpriseSecurityContext { + private final AccessMode mode; + + private AuthDisabled( AccessMode mode ) + { + this.mode = mode; + } + @Override public EnterpriseSecurityContext freeze() { @@ -43,9 +53,9 @@ public EnterpriseSecurityContext freeze() } @Override - public EnterpriseSecurityContext freeze( AccessMode mode ) + public EnterpriseSecurityContext withMode( AccessMode mode ) { - return new Frozen( subject(), mode, isAdmin() ); + return new EnterpriseSecurityContext.AuthDisabled( mode ); } @Override @@ -57,7 +67,13 @@ public AuthSubject subject() @Override public AccessMode mode() { - return AccessMode.Static.FULL; + return mode; + } + + @Override + public String description() + { + return "AUTH_DISABLED with " + mode().name(); } @Override @@ -71,9 +87,9 @@ public boolean isAdmin() { return true; } - }; + } - class Frozen implements EnterpriseSecurityContext + final class Frozen implements EnterpriseSecurityContext { private final AuthSubject subject; private final AccessMode mode; @@ -111,7 +127,7 @@ public EnterpriseSecurityContext freeze() } @Override - public EnterpriseSecurityContext freeze( AccessMode mode ) + public EnterpriseSecurityContext withMode( AccessMode mode ) { return new EnterpriseSecurityContext.Frozen( subject, mode, isAdmin ); } diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/StandardEnterpriseSecurityContext.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/StandardEnterpriseSecurityContext.java index 565014656659e..e614f978c1630 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/StandardEnterpriseSecurityContext.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/StandardEnterpriseSecurityContext.java @@ -23,6 +23,10 @@ import java.io.IOException; import java.util.Collection; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.neo4j.graphdb.security.AuthorizationViolationException; import org.neo4j.kernel.api.exceptions.InvalidArgumentsException; @@ -90,7 +94,7 @@ public EnterpriseSecurityContext freeze() } @Override - public EnterpriseSecurityContext freeze( AccessMode mode ) + public EnterpriseSecurityContext withMode( AccessMode mode ) { return new Frozen( neoShiroSubject, mode, isAdmin() ); } @@ -134,18 +138,12 @@ public boolean allowsSchemaWrites() @Override public boolean allowsProcedureWith( String[] roleNames ) throws InvalidArgumentsException { - for ( AuthorizationInfo info : authorizationInfoSnapshot ) + Set roles = roleNames(); + for ( int i = 0; i < roleNames.length; i++ ) { - Collection roles = info.getRoles(); - if ( roles != null ) + if ( roles.contains( roleNames[i] ) ) { - for ( int i = 0; i < roleNames.length; i++ ) - { - if ( roles.contains( roleNames[i] ) ) - { - return true; - } - } + return true; } } return false; @@ -167,7 +165,18 @@ public AuthorizationViolationException onViolation( String msg ) @Override public String name() { - return ""; + Set roles = new TreeSet<>( roleNames() ); + return roles.isEmpty() ? "no roles" : "roles [" + String.join( ",", roles ) + "]"; + } + + private Set roleNames() + { + return authorizationInfoSnapshot.stream() + .flatMap( authInfo -> { + Collection roles = authInfo.getRoles(); + return roles == null ? Stream.empty() : roles.stream(); + } ) + .collect( Collectors.toSet() ); } } diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EmbeddedBuiltInProceduresInteractionTest.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EmbeddedBuiltInProceduresInteractionTest.java index 8d48b7ffa2f4b..1dc7dbfe1e9fd 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EmbeddedBuiltInProceduresInteractionTest.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EmbeddedBuiltInProceduresInteractionTest.java @@ -103,7 +103,7 @@ public EnterpriseSecurityContext freeze() } @Override - public EnterpriseSecurityContext freeze( AccessMode mode ) + public EnterpriseSecurityContext withMode( AccessMode mode ) { return new EnterpriseSecurityContext.Frozen( subject(), mode, isAdmin() ); } diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityContextDescriptionTest.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityContextDescriptionTest.java new file mode 100644 index 0000000000000..6e0da5704d1b0 --- /dev/null +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/EnterpriseSecurityContextDescriptionTest.java @@ -0,0 +1,130 @@ +/* + * Copyright (c) 2002-2016 "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 . + */ +package org.neo4j.server.security.enterprise.auth; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +import java.time.Clock; + +import org.neo4j.kernel.api.security.AccessMode; +import org.neo4j.kernel.enterprise.api.security.EnterpriseSecurityContext; +import org.neo4j.kernel.impl.api.security.OverriddenAccessMode; +import org.neo4j.kernel.impl.api.security.RestrictedAccessMode; +import org.neo4j.server.security.auth.InMemoryUserRepository; +import org.neo4j.server.security.auth.RateLimitedAuthenticationStrategy; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.neo4j.server.security.auth.SecurityTestUtils.authToken; +import static org.neo4j.server.security.enterprise.auth.plugin.api.PredefinedRoles.PUBLISHER; + +public class EnterpriseSecurityContextDescriptionTest +{ + @Rule + public MultiRealmAuthManagerRule authManagerRule = new MultiRealmAuthManagerRule( new InMemoryUserRepository(), + new RateLimitedAuthenticationStrategy( Clock.systemUTC(), 3 ) ); + + private EnterpriseSecurityContext context; + private EnterpriseUserManager manager; + + @Before + public void setUp() throws Throwable + { + authManagerRule.getManager().start(); + manager = authManagerRule.getManager().getUserManager(); + manager.newUser( "mats", "foo", false ); + context = authManagerRule.getManager().login( authToken( "mats", "foo" ) ); + } + + @Test + public void shouldMakeNiceDescriptionWithoutRoles() throws Throwable + { + assertThat( context.description(), equalTo( "user 'mats' with no roles" ) ); + } + + @Test + public void shouldMakeNiceDescriptionWithRoles() throws Throwable + { + manager.newRole( "role1", "mats" ); + manager.addRoleToUser( PUBLISHER, "mats" ); + + assertThat( context.description(), equalTo( "user 'mats' with roles [publisher,role1]" ) ); + } + + @Test + public void shouldMakeNiceDescriptionFrozen() throws Throwable + { + manager.newRole( "role1", "mats" ); + manager.addRoleToUser( PUBLISHER, "mats" ); + + EnterpriseSecurityContext frozen = context.freeze(); + assertThat( frozen.description(), equalTo( "user 'mats' with roles [publisher,role1]" ) ); + } + + @Test + public void shouldMakeNiceDescriptionWithMode() throws Throwable + { + manager.newRole( "role1", "mats" ); + manager.addRoleToUser( PUBLISHER, "mats" ); + + EnterpriseSecurityContext modified = context.withMode( AccessMode.Static.CREDENTIALS_EXPIRED ); + assertThat( modified.description(), equalTo( "user 'mats' with CREDENTIALS_EXPIRED" ) ); + } + + @Test + public void shouldMakeNiceDescriptionRestricted() throws Throwable + { + manager.newRole( "role1", "mats" ); + manager.addRoleToUser( PUBLISHER, "mats" ); + + EnterpriseSecurityContext restricted = + context.withMode( new RestrictedAccessMode( context.mode(), AccessMode.Static.READ ) ); + assertThat( restricted.description(), equalTo( "user 'mats' with roles [publisher,role1] restricted to READ" ) ); + } + + @Test + public void shouldMakeNiceDescriptionOverridden() throws Throwable + { + manager.newRole( "role1", "mats" ); + manager.addRoleToUser( PUBLISHER, "mats" ); + + EnterpriseSecurityContext overridden = + context.withMode( new OverriddenAccessMode( context.mode(), AccessMode.Static.READ ) ); + assertThat( overridden.description(), equalTo( "user 'mats' with roles [publisher,role1] overridden by READ" ) ); + } + + @Test + public void shouldMakeNiceDescriptionAuthDisabled() throws Throwable + { + EnterpriseSecurityContext disabled = EnterpriseSecurityContext.AUTH_DISABLED; + assertThat( disabled.description(), equalTo( "AUTH_DISABLED with FULL" ) ); + } + + @Test + public void shouldMakeNiceDescriptionAuthDisabledAndRestricted() throws Throwable + { + EnterpriseSecurityContext disabled = EnterpriseSecurityContext.AUTH_DISABLED; + EnterpriseSecurityContext restricted = + disabled.withMode( new RestrictedAccessMode( disabled.mode(), AccessMode.Static.READ ) ); + assertThat( restricted.description(), equalTo( "AUTH_DISABLED with FULL restricted to READ" ) ); + } +} diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/EnterpriseAuthenticationTestBase.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/EnterpriseAuthenticationTestBase.java index f2d69d9921130..6bf53d1c031c0 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/EnterpriseAuthenticationTestBase.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/EnterpriseAuthenticationTestBase.java @@ -23,6 +23,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Rule; +import org.parboiled.common.StringUtils; import java.io.IOException; import java.util.LinkedHashMap; @@ -45,6 +46,7 @@ import static java.lang.String.format; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.neo4j.bolt.v1.messaging.message.InitMessage.init; import static org.neo4j.bolt.v1.messaging.message.PullAllMessage.pullAll; import static org.neo4j.bolt.v1.messaging.message.RunMessage.run; @@ -158,7 +160,7 @@ protected void testAuthWithReaderUser( String username, String realm ) throws Ex { assertAuth( username, "abc123", realm ); assertReadSucceeds(); - assertWriteFails( username ); + assertWriteFails( username, "reader" ); } protected void testAuthWithPublisherUser( String username, String realm ) throws Exception @@ -170,7 +172,7 @@ protected void testAuthWithPublisherUser( String username, String realm ) throws protected void testAuthWithNoPermissionUser( String username ) throws Exception { assertAuth( username, "abc123" ); - assertReadFails( username ); + assertReadFails( username, "" ); } protected void assertAuth( String username, String password ) throws Exception @@ -224,25 +226,29 @@ protected void assertConnectionFails( Map authToken ) throws Exce protected void assertReadSucceeds() throws Exception { // When - client.send( chunk( - run( "MATCH (n) RETURN n" ), + client.send( TransportTestUtil.chunk( run( "MATCH (n) RETURN count(n)" ), pullAll() ) ); // Then - assertThat( client, eventuallyReceives( msgSuccess(), msgSuccess() ) ); + assertThat( client, eventuallyReceives( + msgSuccess(), + msgRecord( eqRecord( greaterThanOrEqualTo( 0L ) ) ), + msgSuccess() ) ); } - protected void assertReadFails( String username ) throws Exception + protected void assertReadFails( String username, String roles ) throws Exception { // When client.send( chunk( run( "MATCH (n) RETURN n" ), pullAll() ) ); + String roleString = StringUtils.isEmpty( roles ) ? "no roles" : "roles [" + roles +"]"; + // Then assertThat( client, eventuallyReceives( msgFailure( Status.Security.Forbidden, - format( "Read operations are not allowed for '%s'.", username ) ) ) ); + format( "Read operations are not allowed for user '%s' with %s.", username, roleString ) ) ) ); } protected void assertWriteSucceeds() throws Exception @@ -256,28 +262,30 @@ protected void assertWriteSucceeds() throws Exception assertThat( client, eventuallyReceives( msgSuccess(), msgSuccess() ) ); } - protected void assertWriteFails( String username ) throws Exception + protected void assertWriteFails( String username, String roles ) throws Exception { // When client.send( chunk( run( "CREATE ()" ), pullAll() ) ); + String roleString = StringUtils.isEmpty( roles ) ? "no roles" : "roles [" + roles +"]"; + // Then assertThat( client, eventuallyReceives( msgFailure( Status.Security.Forbidden, - format( "Write operations are not allowed for '%s'.", username ) ) ) ); + format( "Write operations are not allowed for user '%s' with %s.", username, roleString ) ) ) ); } - protected void assertBeginTransaction() throws Exception + protected void assertBeginTransactionSucceeds() throws Exception { // When - client.send( chunk( - run( "BEGIN" ), + client.send( TransportTestUtil.chunk( run( "BEGIN" ), pullAll() ) ); // Then - assertThat( client, eventuallyReceives( msgSuccess(), msgSuccess() ) ); + assertThat( client, eventuallyReceives( + msgSuccess(), msgSuccess() ) ); } protected void assertCommitTransaction() throws Exception diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/LdapAuthenticationIT.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/LdapAuthenticationIT.java index 72944272c3a46..1a1228e68e281 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/LdapAuthenticationIT.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/LdapAuthenticationIT.java @@ -621,7 +621,7 @@ public void shouldBeAbleToLoginAndAuthorizeReaderWithUserLdapContextOnEC2() thro assertAuth( "neo", "abc123ABC123" ); assertReadSucceeds(); - assertWriteFails( "neo" ); + assertWriteFails( "neo", "" ); } //@Test @@ -631,7 +631,7 @@ public void shouldBeAbleToLoginAndAuthorizeReaderOnEC2() throws Throwable assertAuth( "neo", "abc123ABC123" ); assertReadSucceeds(); - assertWriteFails( "neo" ); + assertWriteFails( "neo", "" ); } //@Test @@ -658,7 +658,7 @@ public void shouldBeAbleToLoginAndAuthorizeNoPermissionUserWithUserLdapContextOn restartNeo4jServerWithOverriddenSettings( activeDirectoryOnEc2NotUsingSystemAccountSettings ); assertAuth( "smith", "abc123ABC123" ); - assertReadFails( "smith" ); + assertReadFails( "smith", "" ); } //@Test @@ -667,7 +667,7 @@ public void shouldBeAbleToLoginAndAuthorizeNoPermissionUserOnEC2() throws Throwa restartNeo4jServerWithOverriddenSettings( activeDirectoryOnEc2UsingSystemAccountSettings ); assertAuth( "smith", "abc123ABC123" ); - assertReadFails( "smith" ); + assertReadFails( "smith", "" ); } //------------------------------------------------------------------ @@ -685,7 +685,7 @@ public void shouldBeAbleToLoginAndAuthorizeReaderUsingLdapsOnEC2() throws Throwa assertAuth( "neo", "abc123ABC123" ); assertReadSucceeds(); - assertWriteFails( "neo" ); + assertWriteFails( "neo", "" ); } //@Test @@ -697,7 +697,7 @@ public void shouldBeAbleToLoginAndAuthorizeReaderWithUserLdapContextUsingLDAPSOn assertAuth( "neo", "abc123ABC123" ); assertReadSucceeds(); - assertWriteFails( "neo" ); + assertWriteFails( "neo", "" ); } //@Test @@ -709,7 +709,7 @@ public void shouldBeAbleToLoginAndAuthorizeReaderUsingStartTlsOnEC2() throws Thr assertAuth( "neo", "abc123ABC123" ); assertReadSucceeds(); - assertWriteFails( "neo" ); + assertWriteFails( "neo", "" ); } //@Test @@ -721,7 +721,7 @@ public void shouldBeAbleToLoginAndAuthorizeReaderWithUserLdapContextUsingStartTl assertAuth( "neo", "abc123ABC123" ); assertReadSucceeds(); - assertWriteFails( "neo" ); + assertWriteFails( "neo", "" ); } //------------------------------------------------------------------------- @@ -825,7 +825,7 @@ public void shouldClearAuthorizationCache() throws Throwable // Then assertAuth( "tank", "abc123", "ldap" ); assertReadSucceeds(); - assertWriteFails( "tank" ); + assertWriteFails( "tank", "reader" ); } } @@ -847,67 +847,6 @@ private void clearAuthCacheFromDifferentConnection() throws Exception assertThat( adminClient, eventuallyReceives( msgSuccess(), msgSuccess() ) ); } - protected void assertReadSucceeds() throws Exception - { - // When - client.send( TransportTestUtil.chunk( run( "MATCH (n) RETURN count(n)" ), - pullAll() ) ); - - // Then - assertThat( client, eventuallyReceives( - msgSuccess(), - msgRecord( eqRecord( greaterThanOrEqualTo( 0L ) ) ), - msgSuccess() ) ); - } - - protected void assertReadFails( String username ) throws Exception - { - // When - client.send( TransportTestUtil.chunk( - run( "MATCH (n) RETURN n" ), - pullAll() ) ); - - // Then - assertThat( client, eventuallyReceives( - msgFailure( Status.Security.Forbidden, - String.format( "Read operations are not allowed for '%s'.", username ) ) ) ); - } - - protected void assertWriteSucceeds() throws Exception - { - // When - client.send( TransportTestUtil.chunk( - run( "CREATE ()" ), - pullAll() ) ); - - // Then - assertThat( client, eventuallyReceives( msgSuccess(), msgSuccess() ) ); - } - - protected void assertWriteFails( String username ) throws Exception - { - // When - client.send( TransportTestUtil.chunk( - run( "CREATE ()" ), - pullAll() ) ); - - // Then - assertThat( client, eventuallyReceives( - msgFailure( Status.Security.Forbidden, - String.format( "Write operations are not allowed for '%s'.", username ) ) ) ); - } - - protected void assertBeginTransactionSucceeds() throws Exception - { - // When - client.send( TransportTestUtil.chunk( run( "BEGIN" ), - pullAll() ) ); - - // Then - assertThat( client, eventuallyReceives( - msgSuccess(), msgSuccess() ) ); - } - private void testClearAuthCache() throws Exception { assertAuth( "neo4j", "abc123" ); diff --git a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/PluginAuthenticationIT.java b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/PluginAuthenticationIT.java index 7a14cd7e598dd..fbf82682b17b1 100644 --- a/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/PluginAuthenticationIT.java +++ b/enterprise/security/src/test/java/org/neo4j/server/security/enterprise/auth/integration/bolt/PluginAuthenticationIT.java @@ -140,7 +140,7 @@ public void shouldAuthenticateAndAuthorizeWithTestAuthPlugin() throws Throwable { assertConnectionSucceeds( authToken( "neo4j", "neo4j", "plugin-TestAuthPlugin" ) ); assertReadSucceeds(); - assertWriteFails( "neo4j" ); + assertWriteFails( "neo4j", "reader" ); } @Test @@ -148,7 +148,7 @@ public void shouldAuthenticateAndAuthorizeWithCacheableTestAuthPlugin() throws T { assertConnectionSucceeds( authToken( "neo4j", "neo4j", "plugin-TestCacheableAuthPlugin" ) ); assertReadSucceeds(); - assertWriteFails( "neo4j" ); + assertWriteFails( "neo4j", "reader" ); } @Test @@ -167,14 +167,14 @@ public void shouldAuthenticateWithTestCacheableAuthPlugin() throws Throwable assertConnectionSucceeds( authToken ); assertThat( TestCacheableAuthPlugin.getAuthInfoCallCount.get(), equalTo( 1 ) ); assertReadSucceeds(); - assertWriteFails( "neo4j" ); + assertWriteFails( "neo4j", "reader" ); // When we log in the second time our plugin should _not_ get a call since auth info should be cached reconnect(); assertConnectionSucceeds( authToken ); assertThat( TestCacheableAuthPlugin.getAuthInfoCallCount.get(), equalTo( 1 ) ); assertReadSucceeds(); - assertWriteFails( "neo4j" ); + assertWriteFails( "neo4j", "reader" ); // When we log in the with the wrong credentials it should fail and // our plugin should _not_ get a call since auth info should be cached @@ -193,7 +193,7 @@ public void shouldAuthenticateAndAuthorizeWithTestCombinedAuthPlugin() throws Th assertConnectionSucceeds( authToken( "neo4j", "neo4j", "plugin-TestCombinedAuthPlugin" ) ); assertReadSucceeds(); - assertWriteFails( "neo4j" ); + assertWriteFails( "neo4j", "reader" ); } @Test @@ -206,7 +206,7 @@ public void shouldAuthenticateAndAuthorizeWithTwoSeparateTestPlugins() throws Th assertConnectionSucceeds( authToken( "neo4j", "neo4j", null ) ); assertReadSucceeds(); - assertWriteFails( "neo4j" ); + assertWriteFails( "neo4j", "reader" ); } @Test diff --git a/integrationtests/src/test/java/org/neo4j/procedure/ProcedureIT.java b/integrationtests/src/test/java/org/neo4j/procedure/ProcedureIT.java index 3a5b8f1d42c36..b0448e92795d1 100644 --- a/integrationtests/src/test/java/org/neo4j/procedure/ProcedureIT.java +++ b/integrationtests/src/test/java/org/neo4j/procedure/ProcedureIT.java @@ -612,7 +612,8 @@ public void shouldDenyReadWriteProcedureToPerformSchema() throws Throwable { // Expect exception.expect( QueryExecutionException.class ); - exception.expectMessage( "Schema operations are not allowed for 'AUTH_DISABLED restricted to WRITE'." ); + exception.expectMessage( + "Schema operations are not allowed for AUTH_DISABLED with FULL restricted to WRITE." ); // Give try ( Transaction ignore = db.beginTx() ) @@ -822,7 +823,10 @@ public void shouldGracefullyFailWhenSpawningThreadsCreatingTransactionInProcedur // given Runnable doIt = () -> { Result result = db.execute( "CALL org.neo4j.procedure.unsupportedProcedure()" ); - result.resultAsString(); + while ( result.hasNext() ) + { + result.next(); + } result.close(); }; @@ -1248,6 +1252,7 @@ public NodeListRecord( List nodes ) } } + @SuppressWarnings( "unused" ) public static class ClassWithProcedures { @Context