Skip to content

Commit

Permalink
Filter index seek/scan
Browse files Browse the repository at this point in the history
  • Loading branch information
@OliviaYtterbrink
OliviaYtterbrink committed Feb 5, 2018
1 parent 1dae564 commit 082cb9d
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 10 deletions.
24 changes: 24 additions & 0 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Read.java
View file
Expand Up @@ -36,6 +36,7 @@
import org.neo4j.internal.kernel.api.RelationshipTraversalCursor;
import org.neo4j.internal.kernel.api.Scan;
import org.neo4j.internal.kernel.api.exceptions.KernelException;
import org.neo4j.internal.kernel.api.security.AccessMode;
import org.neo4j.io.pagecache.PageCursor;
import org.neo4j.kernel.api.ExplicitIndex;
import org.neo4j.kernel.api.ExplicitIndexHits;
Expand Down Expand Up @@ -90,6 +91,11 @@ public final void nodeIndexSeek(
IndexQuery... query ) throws IndexNotApplicableKernelException, IndexNotFoundKernelException
{
ktx.assertOpen();
if ( hasForbiddenProperties( index ) )
{
cursor.close();
return;
}

((DefaultNodeValueIndexCursor) cursor).setRead( this );
IndexProgressor.NodeValueClient target = (DefaultNodeValueIndexCursor) cursor;
Expand Down Expand Up @@ -139,13 +145,31 @@ public final void nodeIndexScan(
IndexOrder indexOrder ) throws KernelException
{
ktx.assertOpen();
if ( hasForbiddenProperties( index ) )
{
cursor.close();
return;
}

// for a scan, we simply query for existence of the first property, which covers all entries in an index
int firstProperty = index.properties()[0];
((DefaultNodeValueIndexCursor) cursor).setRead( this );
indexReader( index ).query( (DefaultNodeValueIndexCursor) cursor, indexOrder, IndexQuery.exists( firstProperty ) );
}

private boolean hasForbiddenProperties( IndexReference index )
{
AccessMode mode = ktx.securityContext().mode();
for ( int prop : index.properties() )
{
if ( !mode.allowsPropertyReads( prop ) )
{
return true;
}
}
return false;
}

@Override
public final void nodeLabelScan( int label, NodeLabelIndexCursor cursor )
{
Expand Down
3 changes: 2 additions & 1 deletion ...curity/src/main/java/org/neo4j/server/security/enterprise/auth/MultiRealmAuthManager.java
View file
Expand Up @@ -44,6 +44,7 @@
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.function.IntPredicate;

import org.neo4j.collection.primitive.Primitive;
import org.neo4j.collection.primitive.PrimitiveIntSet;
Expand Down Expand Up @@ -308,7 +309,7 @@ public Collection<AuthorizationInfo> getAuthorizationInfo( PrincipalCollection p
return infoList;
}

Function<Integer,Boolean> getPropertyPermissions( Set<String> roles, Token token )
IntPredicate getPropertyPermissions( Set<String> roles, Token token )
{
if ( propertyAuthorization )
{
Expand Down
9 changes: 5 additions & 4 deletions ...c/main/java/org/neo4j/server/security/enterprise/auth/StandardEnterpriseLoginContext.java
View file
Expand Up @@ -26,6 +26,7 @@
import java.util.Set;
import java.util.TreeSet;
import java.util.function.Function;
import java.util.function.IntPredicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -106,7 +107,7 @@ private Set<String> queryForRoleNames()
.collect( Collectors.toSet() );
}

private Function<Integer,Boolean> queryForPropertyPermissions( Token token )
private IntPredicate queryForPropertyPermissions( Token token )
{
return authManager.getPropertyPermissions( roles(), token );
}
Expand All @@ -119,10 +120,10 @@ private static class StandardAccessMode implements AccessMode
private final boolean allowsTokenCreates;
private final boolean passwordChangeRequired;
private final Set<String> roles;
private final Function<Integer,Boolean> propertyPermissions;
private final IntPredicate propertyPermissions;

StandardAccessMode( boolean allowsReads, boolean allowsWrites, boolean allowsTokenCreates, boolean allowsSchemaWrites,
boolean passwordChangeRequired, Set<String> roles, Function<Integer,Boolean> propertyPermissions )
boolean passwordChangeRequired, Set<String> roles, IntPredicate propertyPermissions )
{
this.allowsReads = allowsReads;
this.allowsWrites = allowsWrites;
Expand Down Expand Up @@ -160,7 +161,7 @@ public boolean allowsSchemaWrites()
@Override
public boolean allowsPropertyReads( int propertyKey )
{
return propertyPermissions.apply( propertyKey );
return propertyPermissions.test( propertyKey );
}

@Override
Expand Down
23 changes: 18 additions & 5 deletions ...c/test/java/org/neo4j/server/security/enterprise/auth/plugin/PropertyLevelSecurityIT.java
View file
Expand Up @@ -20,6 +20,7 @@
package org.neo4j.server.security.enterprise.auth.plugin;

import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
Expand All @@ -46,6 +47,7 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.neo4j.internal.kernel.api.Transaction.Type.explicit;
import static org.neo4j.server.security.auth.SecurityTestUtils.authToken;
Expand Down Expand Up @@ -329,7 +331,11 @@ public void shouldBehaveLikeDataIsMissingWhenFilteringWithIndex() throws Throwab

execute( neo, "MATCH (n {name: 'Andersson'}) SET n.alias = 'neo' ", Collections.emptyMap() ).close();

execute( smith, query, Collections.emptyMap(), r -> assertThat( r.hasNext(), equalTo( false ) ) );
execute( smith, query, Collections.emptyMap(), r ->
{
assertThat( r.getExecutionPlanDescription().toString(), containsString( "NodeIndexSeek" ) );
assertThat( r.hasNext(), equalTo( false ) );
} );

execute( neo, query, Collections.emptyMap(), r -> assertThat( r.hasNext(), equalTo( true ) ) );
}
Expand All @@ -341,14 +347,15 @@ public void shouldBehaveLikeDataIsMissingForExistsWithIndex() throws Throwable
execute( neo, "CALL db.awaitIndexes", Collections.emptyMap() ).close();
execute( neo, "CREATE (n:Person {name: 'Andersson'}) ", Collections.emptyMap() ).close();

String query = "MATCH (n:Person) WHERE exists(n.alias) RETURN n.alias";
String query = "MATCH (n:Person) USING INDEX n:Person(alias) WHERE exists(n.alias) RETURN n.alias";

execute( neo, query, Collections.emptyMap(), r -> assertThat( r.hasNext(), equalTo( false ) ) );

execute( neo, "MATCH (n {name: 'Andersson'}) SET n.alias = 'neo' ", Collections.emptyMap() ).close();

execute( neo, query, Collections.emptyMap(), r ->
{
assertThat( r.getExecutionPlanDescription().toString(), containsString( "NodeIndexScan" ) );
assertThat( r.hasNext(), equalTo( true ) );
assertThat( r.next().get( "n.alias" ), equalTo( "neo" ) );
} );
Expand All @@ -371,6 +378,7 @@ public void shouldBehaveLikeDataIsMissingForStringBeginsWithIndex() throws Throw

execute( neo, query, Collections.emptyMap(), r ->
{
assertThat( r.getExecutionPlanDescription().toString(), containsString( "NodeIndexSeekByRange" ) );
assertThat( r.hasNext(), equalTo( true ) );
assertThat( r.next().get( "n.alias" ), equalTo( "neo" ) );
} );
Expand All @@ -381,7 +389,7 @@ public void shouldBehaveLikeDataIsMissingForStringBeginsWithIndex() throws Throw
@Test
public void shouldBehaveLikeDataIsMissingForRangeWithIndex() throws Throwable
{
execute( neo, "CREATE INDEX ON :Person(alias)", Collections.emptyMap() ).close();
execute( neo, "CREATE INDEX ON :Person(secret)", Collections.emptyMap() ).close();
execute( neo, "CALL db.awaitIndexes", Collections.emptyMap() ).close();
execute( neo, "CREATE (n:Person {name: 'Andersson'}) ", Collections.emptyMap() ).close();

Expand All @@ -393,6 +401,7 @@ public void shouldBehaveLikeDataIsMissingForRangeWithIndex() throws Throwable

execute( neo, query, Collections.emptyMap(), r ->
{
assertThat( r.getExecutionPlanDescription().toString(), containsString( "NodeIndexSeek" ) );
assertThat( r.hasNext(), equalTo( true ) );
assertThat( r.next().get( "n.secret" ), equalTo( 42L ) );
} );
Expand All @@ -416,6 +425,7 @@ public void shouldBehaveLikeDataIsMissingForCompositeWithIndex() throws Throwabl

execute( neo, query, Collections.emptyMap(), r ->
{
assertThat( r.getExecutionPlanDescription().toString(), containsString( "NodeIndexSeek" ) );
assertThat( r.hasNext(), equalTo( true ) );
assertThat( r.next().get( "n.alias" ), equalTo( "neo" ) );
} );
Expand All @@ -424,8 +434,9 @@ public void shouldBehaveLikeDataIsMissingForCompositeWithIndex() throws Throwabl
}

// RELATIONSHIPS
// TODO: when the realtionship properties are returned through PropertyCursor as well this should be unignored and expanded upon

@Test
@Ignore
public void shouldBehaveLikeDataIsMissingForRelationshipProperties() throws Throwable
{
execute( neo, "CREATE (n {name: 'Andersson'}) CREATE (m { name: 'Betasson'}) CREATE (n)-[:Neighbour]->(m)", Collections.emptyMap() ).close();
Expand Down Expand Up @@ -484,11 +495,13 @@ public void shouldBehaveWithProcedures() throws Throwable
assertThat( r.next().get( "propertyKey" ), equalTo( "alias" ) );
assertThat( r.hasNext(), equalTo( true ) );
assertThat( r.next().get( "propertyKey" ), equalTo( "name" ) );
assertThat( r.hasNext(), equalTo( true ) );
assertThat( r.next().get( "propertyKey" ), equalTo( "secret" ) );
assertThat( r.hasNext(), equalTo( false ) );
} );
}

private void execute( LoginContext subject, String query, Map<String,Object> params, Consumer<ResourceIterator<Map<String,Object>>> consumer )
private void execute( LoginContext subject, String query, Map<String,Object> params, Consumer<Result> consumer )
{
Result result;
try ( InternalTransaction tx = db.beginTransaction( explicit, subject ) )
Expand Down

0 comments on commit 082cb9d

Please sign in to comment.