Skip to content

Commit

Permalink
Cleanly reject session authorization queries for unsupported clients
Browse files Browse the repository at this point in the history
Adds SESSION_AUTHORIZATION to ClientCapabilities, so that the
client side support for this feature can be identified using
the headers sent to Trino.
  • Loading branch information
b-slim authored and phd3 committed Oct 18, 2023
1 parent 8b9f55e commit 60b9fc1
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,7 @@ public enum ClientCapabilities
// time(p) without time zone
// interval X(p1) to Y(p2)
// When this capability is not set, the server returns datetime types with precision = 3
PARAMETRIC_DATETIME;
PARAMETRIC_DATETIME,
// Whether clients support the session authorization set/reset feature
SESSION_AUTHORIZATION;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.inject.Inject;
import io.trino.Session;
import io.trino.client.ClientCapabilities;
import io.trino.execution.warnings.WarningCollector;
import io.trino.spi.TrinoException;
import io.trino.sql.tree.Expression;
Expand All @@ -26,6 +27,7 @@

import static com.google.common.util.concurrent.Futures.immediateFuture;
import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR;
import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED;
import static java.util.Objects.requireNonNull;

public class ResetSessionAuthorizationTask
Expand Down Expand Up @@ -53,6 +55,9 @@ public ListenableFuture<Void> execute(
WarningCollector warningCollector)
{
Session session = stateMachine.getSession();
if (!session.getClientCapabilities().contains(ClientCapabilities.SESSION_AUTHORIZATION.toString())) {
throw new TrinoException(NOT_SUPPORTED, "RESET SESSION AUTHORIZATION not supported by client");
}
session.getTransactionId().ifPresent(transactionId -> {
if (!transactionManager.getTransactionInfo(transactionId).isAutoCommitContext()) {
throw new TrinoException(GENERIC_USER_ERROR, "Can't reset authorization user in the middle of a transaction");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.inject.Inject;
import io.trino.Session;
import io.trino.client.ClientCapabilities;
import io.trino.execution.warnings.WarningCollector;
import io.trino.security.AccessControl;
import io.trino.spi.TrinoException;
Expand All @@ -31,6 +32,7 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.util.concurrent.Futures.immediateFuture;
import static io.trino.spi.StandardErrorCode.GENERIC_USER_ERROR;
import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED;
import static java.util.Objects.requireNonNull;

public class SetSessionAuthorizationTask
Expand Down Expand Up @@ -60,6 +62,9 @@ public ListenableFuture<Void> execute(
WarningCollector warningCollector)
{
Session session = stateMachine.getSession();
if (!session.getClientCapabilities().contains(ClientCapabilities.SESSION_AUTHORIZATION.toString())) {
throw new TrinoException(NOT_SUPPORTED, "SET SESSION AUTHORIZATION not supported by client");
}
Identity originalIdentity = session.getOriginalIdentity();
// Set authorization user in the middle of a transaction is disallowed by the SQL spec
session.getTransactionId().ifPresent(transactionId -> {
Expand Down
29 changes: 29 additions & 0 deletions testing/trino-tests/src/test/java/io/trino/tests/TestServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import static io.trino.SystemSessionProperties.MAX_HASH_PARTITION_COUNT;
import static io.trino.SystemSessionProperties.QUERY_MAX_MEMORY;
import static io.trino.client.ClientCapabilities.PATH;
import static io.trino.client.ClientCapabilities.SESSION_AUTHORIZATION;
import static io.trino.client.ProtocolHeaders.TRINO_HEADERS;
import static io.trino.spi.StandardErrorCode.INCOMPATIBLE_CLIENT;
import static io.trino.testing.TestingSession.testSessionBuilder;
Expand Down Expand Up @@ -291,6 +292,34 @@ public void testSetPathSupportByClient()
}
}

@Test
public void testSetSessionSupportByClient()
{
try (TestingTrinoClient testingClient = new TestingTrinoClient(server, testSessionBuilder().setClientCapabilities(Set.of()).build())) {
assertThatThrownBy(() -> testingClient.execute("SET SESSION AUTHORIZATION userA"))
.hasMessage("SET SESSION AUTHORIZATION not supported by client");
}

try (TestingTrinoClient testingClient = new TestingTrinoClient(server, testSessionBuilder().setClientCapabilities(Set.of(
SESSION_AUTHORIZATION.name())).build())) {
testingClient.execute("SET SESSION AUTHORIZATION userA");
}
}

@Test
public void testResetSessionSupportByClient()
{
try (TestingTrinoClient testingClient = new TestingTrinoClient(server, testSessionBuilder().setClientCapabilities(Set.of()).build())) {
assertThatThrownBy(() -> testingClient.execute("RESET SESSION AUTHORIZATION"))
.hasMessage("RESET SESSION AUTHORIZATION not supported by client");
}

try (TestingTrinoClient testingClient = new TestingTrinoClient(server, testSessionBuilder().setClientCapabilities(Set.of(
SESSION_AUTHORIZATION.name())).build())) {
testingClient.execute("RESET SESSION AUTHORIZATION");
}
}

private void checkVersionOnError(String query, @Language("RegExp") String proofOfOrigin)
{
QueryResults queryResults = postQuery(request -> request
Expand Down

0 comments on commit 60b9fc1

Please sign in to comment.