Fix review comments

- Add tests for AccessMode.NONE - Add unauthorized access violation error for REST in AuthorizationFilter
neo4j · Feb 26, 2016 · 72b028b · 72b028b
1 parent 818d7d0
commit 72b028b
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 2 deletions.
diff --git a/...unity/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java b/...unity/kernel/src/main/java/org/neo4j/kernel/impl/api/KernelTransactionImplementation.java
@@ -108,7 +108,7 @@ TransactionType enableReadTransaction()
                     }
                 };
 
-        TransactionType enableReadTransaction() throws IllegalStateException
+        TransactionType enableReadTransaction()
         {
             return READ_ONLY;
         }

diff --git a/...unity/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionAccessModeTest.java b/...unity/kernel/src/test/java/org/neo4j/kernel/impl/api/KernelTransactionAccessModeTest.java
@@ -36,6 +36,45 @@ public class KernelTransactionAccessModeTest extends KernelTransactionTestBase
 {
     @Rule public ExpectedException exception = ExpectedException.none();
 
+    @Test
+    public void shouldNotAllowReadsInNoneMode() throws Throwable
+    {
+        // Given
+        KernelTransactionImplementation tx = newTransaction( AccessMode.NONE );
+
+        // Expect
+        exception.expect( AuthorizationViolationException.class );
+
+        // When
+        tx.acquireStatement().readOperations();
+    }
+
+    @Test
+    public void shouldNotAllowWritesInNoneMode() throws Throwable
+    {
+        // Given
+        KernelTransactionImplementation tx = newTransaction( AccessMode.NONE );
+
+        // Expect
+        exception.expect( AuthorizationViolationException.class );
+
+        // When
+        tx.acquireStatement().dataWriteOperations();
+    }
+
+    @Test
+    public void shouldNotAllowSchemaWritesInNoneMode() throws Throwable
+    {
+        // Given
+        KernelTransactionImplementation tx = newTransaction( AccessMode.NONE );
+
+        // Expect
+        exception.expect( AuthorizationViolationException.class );
+
+        // When
+        tx.acquireStatement().schemaWriteOperations();
+    }
+
     @Test
     public void shouldAllowReadsInReadMode() throws Throwable
     {

diff --git a/community/server/src/main/java/org/neo4j/server/rest/dbms/AuthorizationFilter.java b/community/server/src/main/java/org/neo4j/server/rest/dbms/AuthorizationFilter.java
@@ -36,6 +36,7 @@
 import javax.ws.rs.core.UriBuilder;
 
 import org.neo4j.function.ThrowingConsumer;
+import org.neo4j.graphdb.security.AuthorizationViolationException;
 import org.neo4j.kernel.api.exceptions.Status;
 import org.neo4j.logging.Log;
 import org.neo4j.logging.LogProvider;
@@ -115,7 +116,14 @@ public void doFilter( ServletRequest servletRequest, ServletResponse servletResp
                 }
                 // fall through
             case SUCCESS:
-                filterChain.doFilter( new AuthorizedRequestWrapper( BASIC_AUTH, username, request ), servletResponse );
+                try
+                {
+                    filterChain.doFilter( new AuthorizedRequestWrapper( BASIC_AUTH, username, request ), servletResponse );
+                }
+                catch ( AuthorizationViolationException e )
+                {
+                    unauthorizedAccess( e.getMessage() ).accept( response );
+                }
                 return;
             case TOO_MANY_ATTEMPTS:
                 tooManyAttempts.accept( response );
@@ -160,6 +168,14 @@ private static ThrowingConsumer<HttpServletResponse, IOException> error( int cod
                             "code", Status.Security.AuthenticationRateLimit.code().serialize(),
                             "message", "Too many failed authentication requests. Please wait 5 seconds and try again." ) ) ) );
 
+    private static ThrowingConsumer<HttpServletResponse, IOException> unauthorizedAccess( final String message )
+    {
+        return error( 403,
+                map( "errors", singletonList( map(
+                        "code", Status.Security.AuthorizationFailed.code().serialize(),
+                        "message", String.format("Unauthorized access violation: %s.", message ) ) ) ) );
+    }
+
     private static ThrowingConsumer<HttpServletResponse, IOException> passwordChangeRequired( final String username, final String baseURL )
     {
         URI path = UriBuilder.fromUri( baseURL ).path( format( "/user/%s/password", username ) ).build();