Skip to content

Commit

Permalink
Improve permission checks in File connector [HZ-2991] [5.2.5] (#791)
Browse files Browse the repository at this point in the history 
Backport port of: #25674 and
https://github.com/hazelcast/hazelcast-enterprise/pull/6631

Original PRs in master:
#25348 and
https://github.com/hazelcast/hazelcast-enterprise/pull/6421

---------

Co-authored-by: František Hartman <frant.hartm@gmail.com>
GitOrigin-RevId: 836979eec9b74102f67e9f2a44f49c1544dad014
  • Loading branch information
@k-jamroz @frant-hartm @actions-user
2 people authored and actions-user committed Feb 26, 2024
1 parent 0a209f1 commit 7f229f9
Show file tree
Hide file tree
Showing 12 changed files with 101 additions and 40 deletions.
5 changes: 3 additions & 2 deletions hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
import com.hazelcast.sql.impl.schema.type.Type;
import com.hazelcast.sql.impl.schema.type.TypeKind;
import com.hazelcast.sql.impl.schema.view.View;
import com.hazelcast.sql.impl.security.SqlSecurityContext;
import com.hazelcast.sql.impl.state.QueryResultRegistry;
import com.hazelcast.sql.impl.type.QueryDataType;
import org.apache.calcite.rel.RelNode;
Expand Down Expand Up @@ -133,8 +134,8 @@ public PlanExecutor(
this.resultRegistry = resultRegistry;
}

SqlResult execute(CreateMappingPlan plan) {
catalog.createMapping(plan.mapping(), plan.replace(), plan.ifNotExists());
SqlResult execute(CreateMappingPlan plan, SqlSecurityContext ssc) {
catalog.createMapping(plan.mapping(), plan.replace(), plan.ifNotExists(), ssc);
return UpdateSqlResultImpl.createUpdateCountResult(0);
}

Expand Down
46 changes: 23 additions & 23 deletions hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/SqlPlanImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,10 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("CREATE MAPPING", arguments);
SqlPlanImpl.ensureNoTimeout("CREATE MAPPING", timeout);
return planExecutor.execute(this);
return planExecutor.execute(this, ssc);
}
}

Expand Down Expand Up @@ -189,7 +189,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("DROP MAPPING", arguments);
SqlPlanImpl.ensureNoTimeout("DROP MAPPING", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -266,7 +266,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("CREATE INDEX", arguments);
SqlPlanImpl.ensureNoTimeout("CREATE INDEX", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -315,7 +315,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
throw QueryException.error("DROP INDEX is not supported.");
}
}
Expand Down Expand Up @@ -393,7 +393,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoTimeout("CREATE JOB", timeout);
return planExecutor.execute(this, arguments);
}
Expand Down Expand Up @@ -436,7 +436,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("ALTER JOB", arguments);
SqlPlanImpl.ensureNoTimeout("ALTER JOB", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -487,7 +487,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("DROP JOB", arguments);
SqlPlanImpl.ensureNoTimeout("DROP JOB", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -531,7 +531,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("CREATE SNAPSHOT", arguments);
SqlPlanImpl.ensureNoTimeout("CREATE SNAPSHOT", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -575,7 +575,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("DROP SNAPSHOT", arguments);
SqlPlanImpl.ensureNoTimeout("DROP SNAPSHOT", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -645,7 +645,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("CREATE VIEW", arguments);
SqlPlanImpl.ensureNoTimeout("CREATE VIEW", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -694,7 +694,7 @@ public void checkPermissions(SqlSecurityContext context) {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("DROP VIEW", arguments);
SqlPlanImpl.ensureNoTimeout("DROP VIEW", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -743,7 +743,7 @@ public void checkPermissions(SqlSecurityContext context) {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("DROP TYPE", arguments);
SqlPlanImpl.ensureNoTimeout("DROP TYPE", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -780,7 +780,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("SHOW " + showTarget, arguments);
SqlPlanImpl.ensureNoTimeout("SHOW " + showTarget, timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -816,7 +816,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoTimeout("EXPLAIN", timeout);
return planExecutor.execute(this);
}
Expand Down Expand Up @@ -897,7 +897,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, queryId, arguments, timeout);
}
}
Expand Down Expand Up @@ -977,7 +977,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, queryId, arguments, timeout);
}
}
Expand Down Expand Up @@ -1057,7 +1057,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, queryId, arguments, timeout);
}
}
Expand Down Expand Up @@ -1123,7 +1123,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, arguments, timeout);
}
}
Expand Down Expand Up @@ -1189,7 +1189,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, arguments, timeout);
}
}
Expand Down Expand Up @@ -1263,7 +1263,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, arguments, timeout);
}
}
Expand Down Expand Up @@ -1330,7 +1330,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, arguments, timeout);
}
}
Expand Down Expand Up @@ -1401,7 +1401,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("CREATE TYPE", arguments);
SqlPlanImpl.ensureNoTimeout("CREATE TYPE", timeout);
return planExecutor.execute(this);
Expand Down
19 changes: 19 additions & 0 deletions hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/SqlConnector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,13 @@
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.lang.reflect.Method;
import java.security.Permission;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;

import static java.util.Collections.emptyList;

/**
* An API to bridge Jet connectors and SQL. Allows the use of a Jet
* connector from SQL and create a mapping for remote objects available
Expand Down Expand Up @@ -221,6 +224,22 @@ List<MappingField> resolveAndValidateFields(
@Nonnull String externalName
);

/**
* Returns the required permissions to execute
* {@link #resolveAndValidateFields} method.
* <p>
* Implementors of {@link SqlConnector} don't need to override this method when {@code resolveAndValidateFields}
* doesn't support field resolution or when validation doesn't access the external resource.
* <p>
* The permissions are usually the same as required permissions to read from the external resource.
*
* @return list of permissions required to run {@link #resolveAndValidateFields}
*/
@Nonnull
default List<Permission> permissionsForResolve(@Nonnull Map<String, String> options, NodeEngine nodeEngine) {
return emptyList();
}

/**
* Creates a {@link Table} object with the given fields. Should return
* quickly; specifically it should not attempt to connect to the remote
Expand Down
10 changes: 10 additions & 0 deletions hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/file/FileSqlConnector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,14 @@

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.security.Permission;
import java.util.List;
import java.util.Map;

import static com.hazelcast.jet.core.Edge.between;
import static com.hazelcast.security.permission.ActionConstants.ACTION_READ;
import static com.hazelcast.security.permission.ConnectorPermission.file;
import static java.util.Collections.singletonList;

public class FileSqlConnector implements SqlConnector {

Expand Down Expand Up @@ -85,6 +89,12 @@ static List<MappingField> resolveAndValidateFields(
return METADATA_RESOLVERS.resolveAndValidateFields(userFields, options);
}

@Nonnull
@Override
public List<Permission> permissionsForResolve(@Nonnull Map<String, String> options, NodeEngine nodeEngine) {
return singletonList(file(options.get(OPTION_PATH), ACTION_READ));
}

@Nonnull
@Override
public Table createTable(
Expand Down
14 changes: 11 additions & 3 deletions hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/schema/TableResolverImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@
import com.hazelcast.sql.impl.schema.TableResolver;
import com.hazelcast.sql.impl.schema.type.Type;
import com.hazelcast.sql.impl.schema.view.View;
import com.hazelcast.sql.impl.security.SqlSecurityContext;

import javax.annotation.Nonnull;
import java.security.Permission;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -118,8 +120,8 @@ public void entryRemoved(EntryEvent<String, Object> event) {

// region mapping

public void createMapping(Mapping mapping, boolean replace, boolean ifNotExists) {
Mapping resolved = resolveMapping(mapping);
public void createMapping(Mapping mapping, boolean replace, boolean ifNotExists, SqlSecurityContext securityContext) {
Mapping resolved = resolveMapping(mapping, securityContext);

String name = resolved.name();
if (ifNotExists) {
Expand All @@ -132,11 +134,17 @@ public void createMapping(Mapping mapping, boolean replace, boolean ifNotExists)
}
}

private Mapping resolveMapping(Mapping mapping) {
private Mapping resolveMapping(Mapping mapping, SqlSecurityContext securityContext) {
String type = mapping.type();
Map<String, String> options = mapping.options();

SqlConnector connector = connectorCache.forType(type);

List<Permission> permissions = connector.permissionsForResolve(mapping.options(), nodeEngine);
for (Permission permission : permissions) {
securityContext.checkPermission(permission);
}

List<MappingField> resolvedFields = connector.resolveAndValidateFields(
nodeEngine,
options,
Expand Down
4 changes: 2 additions & 2 deletions hazelcast-sql/src/test/java/com/hazelcast/jet/sql/impl/PlanExecutorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ public void test_createMappingExecution(boolean replace, boolean ifNotExists) {
CreateMappingPlan plan = new CreateMappingPlan(planKey(), mapping, replace, ifNotExists, planExecutor);

// when
SqlResult result = planExecutor.execute(plan);
SqlResult result = planExecutor.execute(plan, null);

// then
assertThat(result.updateCount()).isEqualTo(0);
verify(catalog).createMapping(mapping, replace, ifNotExists);
verify(catalog).createMapping(mapping, replace, ifNotExists, null);
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion hazelcast-sql/src/test/java/com/hazelcast/jet/sql/impl/cache/PlanCacheTestSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
throw new UnsupportedOperationException();
}
}
Expand Down
8 changes: 4 additions & 4 deletions hazelcast-sql/src/test/java/com/hazelcast/jet/sql/impl/schema/TableResolverImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public void when_createsInvalidMapping_then_throws() {

// when
// then
assertThatThrownBy(() -> catalog.createMapping(mapping, true, true))
assertThatThrownBy(() -> catalog.createMapping(mapping, true, true, null))
.hasMessageContaining("expected test exception");
verify(tableStorage, never()).putIfAbsent(anyString(), (Mapping) any());
verify(tableStorage, never()).put(anyString(), (Mapping) any());
Expand All @@ -122,7 +122,7 @@ public void when_createsDuplicateMapping_then_throws() {

// when
// then
assertThatThrownBy(() -> catalog.createMapping(mapping, false, false))
assertThatThrownBy(() -> catalog.createMapping(mapping, false, false, null))
.isInstanceOf(QueryException.class)
.hasMessageContaining("Mapping or view already exists: name");
verifyNoInteractions(listener);
Expand All @@ -139,7 +139,7 @@ public void when_createsDuplicateMappingWithIfNotExists_then_succeeds() {
given(tableStorage.putIfAbsent(eq(mapping.name()), isA(Mapping.class))).willReturn(false);

// when
catalog.createMapping(mapping, false, true);
catalog.createMapping(mapping, false, true, null);

// then
verifyNoInteractions(listener);
Expand All @@ -155,7 +155,7 @@ public void when_replacesMapping_then_succeeds() {
.willReturn(singletonList(new MappingField("field_name", INT)));

// when
catalog.createMapping(mapping, true, false);
catalog.createMapping(mapping, true, false, null);

// then
verify(tableStorage).put(eq(mapping.name()), isA(Mapping.class));
Expand Down
Loading

0 comments on commit 7f229f9

Please sign in to comment.