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 647e806ebba16..c32216c0c8a5a 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(SecurityContext.frozen(tc.securityContext, AccessMode.Static.READ)) + val revertable = tc.restrictCurrentTransaction(tc.securityContext.freeze(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 623b77283b12e..3be6d07264c94 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 @@ -55,6 +55,18 @@ public AuthSubject subject() return AuthSubject.ANONYMOUS; } + @Override + public SecurityContext freeze() + { + return this; + } + + @Override + public SecurityContext freeze( AccessMode mode ) + { + return new Frozen( subject(), mode ); + } + @Override public AccessMode 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 23f4f4b83d8fe..759e23b24e70e 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 @@ -25,6 +25,9 @@ public interface SecurityContext AccessMode mode(); AuthSubject subject(); + SecurityContext freeze(); + SecurityContext freeze( AccessMode mode ); + default String defaultString( String name ) { return String.format( "%s{ securityContext=%s, allowance=%s }", name, subject().username(), mode() ); @@ -50,34 +53,53 @@ public AuthSubject subject() { return AuthSubject.AUTH_DISABLED; } + + @Override + public SecurityContext freeze() + { + return this; + } + + @Override + public SecurityContext freeze( AccessMode mode ) + { + return new Frozen( subject(), mode ); + } }; - static SecurityContext frozen( SecurityContext context, AccessMode accessMode ) + class Frozen implements SecurityContext { - return frozen( context.subject(), accessMode ); - } + private final AuthSubject subject; + private final AccessMode mode; - static SecurityContext frozen( AuthSubject subject, AccessMode accessMode ) - { - return new SecurityContext() + public Frozen( AuthSubject subject, AccessMode mode ) + { + this.subject = subject; + this.mode = mode; + } + + @Override + public AccessMode mode() + { + return mode; + } + + @Override + public AuthSubject subject() { - @Override - public AccessMode mode() - { - return accessMode; - } + return subject; + } - @Override - public AuthSubject subject() - { - return subject; - } + @Override + public SecurityContext freeze() + { + return this; + } - @Override - public String toString() - { - return defaultString( "frozen-security-context" ); - } - }; + @Override + public SecurityContext freeze( AccessMode mode ) + { + return new Frozen( subject, mode ); + } } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java index 1ae40a5579d62..c2e0bb8f06337 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java @@ -229,7 +229,7 @@ public KernelTransactionImplementation initialize( this.lastTransactionTimestampWhenStarted = lastTimeStamp; this.transactionEvent = tracer.beginTransaction(); assert transactionEvent != null : "transactionEvent was null!"; - this.securityContext = securityContext; + this.securityContext = securityContext.freeze(); this.transactionId = NOT_COMMITTED_TRANSACTION_ID; this.commitTime = NOT_COMMITTED_TRANSACTION_COMMIT_TIME; this.currentTransactionOperations = timeoutMillis > 0 ? operationContainer.guardedParts() : operationContainer.nonGuarderParts(); 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 a1d6964f862fb..e4f332f78f63b 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 = SecurityContext.frozen( tx.securityContext(), override ); + final SecurityContext procedureSecurityContext = tx.securityContext().freeze( 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( SecurityContext.frozen( tx.securityContext(), mode )) ) + try ( KernelTransaction.Revertable ignore = tx.overrideWith( tx.securityContext().freeze( 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 ab48e8269af15..0896abdb86668 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 @@ -118,6 +118,18 @@ public AuthSubject subject() return authSubject; } + @Override + public SecurityContext freeze() + { + return this; + } + + @Override + public SecurityContext freeze( AccessMode mode ) + { + return new Frozen( authSubject, mode ); + } + @Override public AccessMode mode() { 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 13209481ef047..d51d2d406c85d 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 @@ -28,8 +28,26 @@ */ public interface EnterpriseSecurityContext extends SecurityContext, CouldBeAdmin { + @Override + EnterpriseSecurityContext freeze(); + + @Override + EnterpriseSecurityContext freeze( AccessMode mode ); + EnterpriseSecurityContext AUTH_DISABLED = new EnterpriseSecurityContext() { + @Override + public EnterpriseSecurityContext freeze() + { + return this; + } + + @Override + public EnterpriseSecurityContext freeze( AccessMode mode ) + { + return new Frozen( subject(), mode, isAdmin() ); + } + @Override public AuthSubject subject() { @@ -54,4 +72,48 @@ public boolean isAdmin() return true; } }; + + class Frozen implements EnterpriseSecurityContext + { + private final AuthSubject subject; + private final AccessMode mode; + private final boolean isAdmin; + + public Frozen( AuthSubject subject, AccessMode mode, boolean isAdmin ) + { + this.subject = subject; + this.mode = mode; + this.isAdmin = isAdmin; + } + + @Override + public boolean isAdmin() + { + return isAdmin; + } + + @Override + public AccessMode mode() + { + return mode; + } + + @Override + public AuthSubject subject() + { + return subject; + } + + @Override + public EnterpriseSecurityContext freeze() + { + return this; + } + + @Override + public EnterpriseSecurityContext freeze( AccessMode mode ) + { + return new EnterpriseSecurityContext.Frozen( subject, mode, isAdmin ); + } + } } diff --git a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProceduresBase.java b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProceduresBase.java index 67fe70a6d0905..5aa6128cec943 100644 --- a/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProceduresBase.java +++ b/enterprise/security/src/main/java/org/neo4j/server/security/enterprise/auth/AuthProceduresBase.java @@ -23,7 +23,6 @@ import java.util.List; import java.util.Set; -import org.neo4j.graphdb.security.AuthorizationViolationException; import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.KernelTransactionHandle; import org.neo4j.kernel.api.bolt.BoltConnectionTracker; @@ -38,7 +37,6 @@ import org.neo4j.server.security.enterprise.log.SecurityLog; import static java.util.Collections.emptyList; -import static org.neo4j.graphdb.security.AuthorizationViolationException.PERMISSION_DENIED; @SuppressWarnings( "WeakerAccess" ) public class AuthProceduresBase 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 29c494c1e462e..565014656659e 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 @@ -83,6 +83,18 @@ public String toString() return defaultString( "enterprise-security-context" ); } + @Override + public EnterpriseSecurityContext freeze() + { + return new Frozen( neoShiroSubject, mode(), isAdmin() ); + } + + @Override + public EnterpriseSecurityContext freeze( AccessMode mode ) + { + return new Frozen( neoShiroSubject, mode, isAdmin() ); + } + private static class StandardAccessMode implements AccessMode { private final boolean allowsReads; 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 5c66501624657..8d48b7ffa2f4b 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 @@ -96,6 +96,18 @@ private EnterpriseSecurityContext createFakeAnonymousEnterpriseSecurityContext() { return new EnterpriseSecurityContext() { + @Override + public EnterpriseSecurityContext freeze() + { + return this; + } + + @Override + public EnterpriseSecurityContext freeze( AccessMode mode ) + { + return new EnterpriseSecurityContext.Frozen( subject(), mode, isAdmin() ); + } + AnonymousContext inner = AnonymousContext.none(); @Override 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 f0e646d33e795..f2d69d9921130 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 @@ -31,8 +31,8 @@ import org.neo4j.bolt.v1.transport.integration.Neo4jWithSocket; import org.neo4j.bolt.v1.transport.integration.TransportTestUtil; -import org.neo4j.bolt.v1.transport.socket.client.TransportConnection; import org.neo4j.bolt.v1.transport.socket.client.SecureSocketConnection; +import org.neo4j.bolt.v1.transport.socket.client.TransportConnection; import org.neo4j.function.Factory; import org.neo4j.graphdb.config.Setting; import org.neo4j.graphdb.factory.GraphDatabaseSettings; @@ -42,12 +42,17 @@ import org.neo4j.test.TestEnterpriseGraphDatabaseFactory; import org.neo4j.test.TestGraphDatabaseFactory; +import static java.lang.String.format; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; 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; import static org.neo4j.bolt.v1.messaging.util.MessageMatchers.msgFailure; +import static org.neo4j.bolt.v1.messaging.util.MessageMatchers.msgRecord; import static org.neo4j.bolt.v1.messaging.util.MessageMatchers.msgSuccess; +import static org.neo4j.bolt.v1.runtime.spi.StreamMatchers.eqRecord; +import static org.neo4j.bolt.v1.transport.integration.TransportTestUtil.chunk; import static org.neo4j.bolt.v1.transport.integration.TransportTestUtil.eventuallyReceives; import static org.neo4j.helpers.collection.MapUtil.map; @@ -138,16 +143,15 @@ protected void testAuthWithPublisherUser() throws Exception protected void testCreateReaderUser( String username ) throws Exception { - assertAuth( "neo4j", "abc123" ); - // NOTE: The default user 'neo4j' has password change required, so we have to first change it - client.send( TransportTestUtil.chunk( - run( "CALL dbms.security.changeUserPassword('neo4j', '123', false) " + - "CALL dbms.security.createUser( '" + username + "', 'abc123', false ) " + + assertAuthAndChangePassword( "neo4j", "abc123", "123" ); + + client.send( chunk( + run( "CALL dbms.security.createUser( '" + username + "', 'abc123', false ) " + "CALL dbms.security.addRoleToUser( 'reader', '" + username + "' ) RETURN 0" ), pullAll() ) ); - assertThat( client, eventuallyReceives( msgSuccess() ) ); + assertThat( client, eventuallyReceives( msgSuccess(), msgRecord( eqRecord( equalTo( 0L ) ) ) ) ); } protected void testAuthWithReaderUser( String username, String realm ) throws Exception @@ -174,6 +178,14 @@ protected void assertAuth( String username, String password ) throws Exception assertConnectionSucceeds( authToken( username, password, null ) ); } + protected void assertAuthAndChangePassword( String username, String password, String newPassword ) throws Exception + { + assertAuth( username, password ); + String query = format( "CALL dbms.security.changeUserPassword('%s', '%s', false)", username, newPassword ); + client.send( chunk( run( query ), pullAll() ) ); + assertThat( client, eventuallyReceives( msgSuccess(), msgSuccess() ) ); + } + protected void assertAuth( String username, String password, String realm ) throws Exception { assertConnectionSucceeds( authToken( username, password, realm ) ); @@ -189,7 +201,7 @@ protected void assertConnectionSucceeds( Map authToken ) throws E // When client.connect( address ) .send( TransportTestUtil.acceptedVersions( 1, 0, 0, 0 ) ) - .send( TransportTestUtil.chunk( + .send( chunk( init( "TestClient/1.1", authToken ) ) ); // Then @@ -201,7 +213,7 @@ protected void assertConnectionFails( Map authToken ) throws Exce { client.connect( address ) .send( TransportTestUtil.acceptedVersions( 1, 0, 0, 0 ) ) - .send( TransportTestUtil.chunk( + .send( chunk( init( "TestClient/1.1", authToken ) ) ); assertThat( client, eventuallyReceives( new byte[]{0, 0, 0, 1} ) ); @@ -212,7 +224,7 @@ protected void assertConnectionFails( Map authToken ) throws Exce protected void assertReadSucceeds() throws Exception { // When - client.send( TransportTestUtil.chunk( + client.send( chunk( run( "MATCH (n) RETURN n" ), pullAll() ) ); @@ -223,20 +235,20 @@ protected void assertReadSucceeds() throws Exception protected void assertReadFails( String username ) throws Exception { // When - client.send( TransportTestUtil.chunk( + client.send( 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 ) ) ) ); + format( "Read operations are not allowed for '%s'.", username ) ) ) ); } protected void assertWriteSucceeds() throws Exception { // When - client.send( TransportTestUtil.chunk( + client.send( chunk( run( "CREATE ()" ), pullAll() ) ); @@ -247,20 +259,20 @@ protected void assertWriteSucceeds() throws Exception protected void assertWriteFails( String username ) throws Exception { // When - client.send( TransportTestUtil.chunk( + client.send( chunk( run( "CREATE ()" ), pullAll() ) ); // Then assertThat( client, eventuallyReceives( msgFailure( Status.Security.Forbidden, - String.format( "Write operations are not allowed for '%s'.", username ) ) ) ); + format( "Write operations are not allowed for '%s'.", username ) ) ) ); } protected void assertBeginTransaction() throws Exception { // When - client.send( TransportTestUtil.chunk( + client.send( chunk( run( "BEGIN" ), pullAll() ) ); @@ -271,7 +283,7 @@ protected void assertBeginTransaction() throws Exception protected void assertCommitTransaction() throws Exception { // When - client.send( TransportTestUtil.chunk( + client.send( chunk( run( "COMMIT" ), pullAll() ) ); @@ -282,7 +294,7 @@ protected void assertCommitTransaction() throws Exception protected void assertQuerySucceeds( String query ) throws Exception { // When - client.send( TransportTestUtil.chunk( + client.send( chunk( run( query ), pullAll() ) );