Skip to content

Commit

Permalink
Change error message for PERMISSION_DENIED
Browse files Browse the repository at this point in the history
We augment the error message on permission denied to detail both the
AuthSubject (when available) and the full AccessMode, including roles in enterprise.
  • Loading branch information
fickludd committed Oct 21, 2016
1 parent 696a13f commit ec68aac
Show file tree
Hide file tree
Showing 15 changed files with 370 additions and 128 deletions.
Expand Up @@ -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
Expand Down
Expand Up @@ -62,7 +62,7 @@ public SecurityContext freeze()
}

@Override
public SecurityContext freeze( AccessMode mode )
public SecurityContext withMode( AccessMode mode )
{
return new Frozen( subject(), mode );
}
Expand Down
Expand Up @@ -26,20 +26,40 @@ 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 )
{
return String.format( "%s{ username=%s, accessMode=%s }", name, subject().username(), mode() );
}

/** 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
Expand All @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -97,7 +117,7 @@ public SecurityContext freeze()
}

@Override
public SecurityContext freeze( AccessMode mode )
public SecurityContext withMode( AccessMode mode )
{
return new Frozen( subject, mode );
}
Expand Down
Expand Up @@ -255,8 +255,8 @@ private void assertAllows( Function<AccessMode,Boolean> 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() ) );
}
}
}
Expand Up @@ -1553,7 +1553,7 @@ private RawIterator<Object[],ProcedureException> callProcedure(
{
statement.assertOpen();

final SecurityContext procedureSecurityContext = tx.securityContext().freeze( override );
final SecurityContext procedureSecurityContext = tx.securityContext().withMode( override );
final RawIterator<Object[],ProcedureException> procedureCall;
try ( KernelTransaction.Revertable ignore = tx.overrideWith( procedureSecurityContext ) )
{
Expand Down Expand Up @@ -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 );
Expand Down
Expand Up @@ -125,7 +125,7 @@ public SecurityContext freeze()
}

@Override
public SecurityContext freeze( AccessMode mode )
public SecurityContext withMode( AccessMode mode )
{
return new Frozen( authSubject, mode );
}
Expand Down
@@ -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 <http://www.gnu.org/licenses/>.
*/
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" ) );
}

}
Expand Up @@ -32,20 +32,30 @@ 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()
{
return this;
}

@Override
public EnterpriseSecurityContext freeze( AccessMode mode )
public EnterpriseSecurityContext withMode( AccessMode mode )
{
return new Frozen( subject(), mode, isAdmin() );
return new EnterpriseSecurityContext.AuthDisabled( mode );
}

@Override
Expand All @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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 );
}
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -90,7 +94,7 @@ public EnterpriseSecurityContext freeze()
}

@Override
public EnterpriseSecurityContext freeze( AccessMode mode )
public EnterpriseSecurityContext withMode( AccessMode mode )
{
return new Frozen( neoShiroSubject, mode, isAdmin() );
}
Expand Down Expand Up @@ -134,18 +138,12 @@ public boolean allowsSchemaWrites()
@Override
public boolean allowsProcedureWith( String[] roleNames ) throws InvalidArgumentsException
{
for ( AuthorizationInfo info : authorizationInfoSnapshot )
Set<String> roles = roleNames();
for ( int i = 0; i < roleNames.length; i++ )
{
Collection<String> 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;
Expand All @@ -167,7 +165,18 @@ public AuthorizationViolationException onViolation( String msg )
@Override
public String name()
{
return "";
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() );
}
}

Expand Down
Expand Up @@ -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() );
}
Expand Down

0 comments on commit ec68aac

Please sign in to comment.