|
| 1 | +From 60656d4fbfd9be5cd4675c1cfe5c118c733e0e3e Mon Sep 17 00:00:00 2001 |
| 2 | +From: Sebastian Bernauer < [email protected]> |
| 3 | +Date: Tue, 4 Nov 2025 13:30:07 +0100 |
| 4 | +Subject: Support sending user principal to OPA |
| 5 | + |
| 6 | +--- |
| 7 | + .../sphinx/security/opa-access-control.md | 2 ++ |
| 8 | + .../io/trino/plugin/opa/OpaAccessControl.java | 12 ++++--- |
| 9 | + .../plugin/opa/OpaBatchAccessControl.java | 2 +- |
| 10 | + .../java/io/trino/plugin/opa/OpaConfig.java | 14 ++++++++ |
| 11 | + .../plugin/opa/schema/TrinoIdentity.java | 12 +++++-- |
| 12 | + .../io/trino/plugin/opa/schema/TrinoUser.java | 4 +-- |
| 13 | + .../plugin/opa/SerializableTestPrincipal.java | 35 +++++++++++++++++++ |
| 14 | + .../plugin/opa/TestOpaAccessControl.java | 35 +++++++++++++++++++ |
| 15 | + .../io/trino/plugin/opa/TestOpaConfig.java | 7 ++-- |
| 16 | + 9 files changed, 110 insertions(+), 13 deletions(-) |
| 17 | + create mode 100644 plugin/trino-opa/src/test/java/io/trino/plugin/opa/SerializableTestPrincipal.java |
| 18 | + |
| 19 | +diff --git a/docs/src/main/sphinx/security/opa-access-control.md b/docs/src/main/sphinx/security/opa-access-control.md |
| 20 | +index 6a16bd9b6b..e0ed21cc1f 100644 |
| 21 | +--- a/docs/src/main/sphinx/security/opa-access-control.md |
| 22 | ++++ b/docs/src/main/sphinx/security/opa-access-control.md |
| 23 | +@@ -61,6 +61,8 @@ The following table lists the configuration properties for the OPA access contro |
| 24 | + * - `opa.allow-permission-management-operations` |
| 25 | + - Configure if permission management operations are allowed. Find more details in |
| 26 | + [](opa-permission-management). Defaults to `false`. |
| 27 | ++* - `opa.include-user-principal` |
| 28 | ++ - Whether to include the users principal when sending authorization requests to OPA. Defaults to `false`. |
| 29 | + * - `opa.http-client.*` |
| 30 | + - Optional HTTP client configurations for the connection from Trino to OPA, |
| 31 | + for example `opa.http-client.http-proxy` for configuring the HTTP proxy. |
| 32 | +diff --git a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java |
| 33 | +index a0b92afee6..16e1fa6eba 100644 |
| 34 | +--- a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java |
| 35 | ++++ b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java |
| 36 | +@@ -90,6 +90,7 @@ public sealed class OpaAccessControl |
| 37 | + private final LifeCycleManager lifeCycleManager; |
| 38 | + private final OpaHighLevelClient opaHighLevelClient; |
| 39 | + private final boolean allowPermissionManagementOperations; |
| 40 | ++ protected final boolean includeUserPrincipal; |
| 41 | + private final OpaPluginContext pluginContext; |
| 42 | + |
| 43 | + @Inject |
| 44 | +@@ -98,6 +99,7 @@ public sealed class OpaAccessControl |
| 45 | + this.lifeCycleManager = requireNonNull(lifeCycleManager, "lifeCycleManager is null"); |
| 46 | + this.opaHighLevelClient = requireNonNull(opaHighLevelClient, "opaHighLevelClient is null"); |
| 47 | + this.allowPermissionManagementOperations = config.getAllowPermissionManagementOperations(); |
| 48 | ++ this.includeUserPrincipal = config.getIncludeUserPrincipal(); |
| 49 | + this.pluginContext = requireNonNull(pluginContext, "pluginContext is null"); |
| 50 | + } |
| 51 | + |
| 52 | +@@ -124,7 +126,7 @@ public sealed class OpaAccessControl |
| 53 | + @Override |
| 54 | + public void checkCanViewQueryOwnedBy(Identity identity, Identity queryOwner) |
| 55 | + { |
| 56 | +- opaHighLevelClient.queryAndEnforce(buildQueryContext(identity), "ViewQueryOwnedBy", AccessDeniedException::denyViewQuery, OpaQueryInputResource.builder().user(new TrinoUser(queryOwner)).build()); |
| 57 | ++ opaHighLevelClient.queryAndEnforce(buildQueryContext(identity), "ViewQueryOwnedBy", AccessDeniedException::denyViewQuery, OpaQueryInputResource.builder().user(new TrinoUser(queryOwner, this.includeUserPrincipal)).build()); |
| 58 | + } |
| 59 | + |
| 60 | + @Override |
| 61 | +@@ -135,13 +137,13 @@ public sealed class OpaAccessControl |
| 62 | + queryOwner -> buildQueryInputForSimpleResource( |
| 63 | + buildQueryContext(identity), |
| 64 | + "FilterViewQueryOwnedBy", |
| 65 | +- OpaQueryInputResource.builder().user(new TrinoUser(queryOwner)).build())); |
| 66 | ++ OpaQueryInputResource.builder().user(new TrinoUser(queryOwner, this.includeUserPrincipal)).build())); |
| 67 | + } |
| 68 | + |
| 69 | + @Override |
| 70 | + public void checkCanKillQueryOwnedBy(Identity identity, Identity queryOwner) |
| 71 | + { |
| 72 | +- opaHighLevelClient.queryAndEnforce(buildQueryContext(identity), "KillQueryOwnedBy", AccessDeniedException::denyKillQuery, OpaQueryInputResource.builder().user(new TrinoUser(queryOwner)).build()); |
| 73 | ++ opaHighLevelClient.queryAndEnforce(buildQueryContext(identity), "KillQueryOwnedBy", AccessDeniedException::denyKillQuery, OpaQueryInputResource.builder().user(new TrinoUser(queryOwner, this.includeUserPrincipal)).build()); |
| 74 | + } |
| 75 | + |
| 76 | + @Override |
| 77 | +@@ -802,11 +804,11 @@ public sealed class OpaAccessControl |
| 78 | + |
| 79 | + OpaQueryContext buildQueryContext(Identity trinoIdentity) |
| 80 | + { |
| 81 | +- return new OpaQueryContext(TrinoIdentity.fromTrinoIdentity(trinoIdentity), pluginContext); |
| 82 | ++ return new OpaQueryContext(TrinoIdentity.fromTrinoIdentity(trinoIdentity, this.includeUserPrincipal), pluginContext); |
| 83 | + } |
| 84 | + |
| 85 | + OpaQueryContext buildQueryContext(SystemSecurityContext securityContext) |
| 86 | + { |
| 87 | +- return new OpaQueryContext(TrinoIdentity.fromTrinoIdentity(securityContext.getIdentity()), pluginContext); |
| 88 | ++ return new OpaQueryContext(TrinoIdentity.fromTrinoIdentity(securityContext.getIdentity(), this.includeUserPrincipal), pluginContext); |
| 89 | + } |
| 90 | + } |
| 91 | +diff --git a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaBatchAccessControl.java b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaBatchAccessControl.java |
| 92 | +index d836ef9a63..53853c9401 100644 |
| 93 | +--- a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaBatchAccessControl.java |
| 94 | ++++ b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaBatchAccessControl.java |
| 95 | +@@ -74,7 +74,7 @@ public final class OpaBatchAccessControl |
| 96 | + "FilterViewQueryOwnedBy", |
| 97 | + queryOwners, |
| 98 | + queryOwner -> OpaQueryInputResource.builder() |
| 99 | +- .user(new TrinoUser(queryOwner)) |
| 100 | ++ .user(new TrinoUser(queryOwner, this.includeUserPrincipal)) |
| 101 | + .build()); |
| 102 | + } |
| 103 | + |
| 104 | +diff --git a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaConfig.java b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaConfig.java |
| 105 | +index 5442f49490..605184f61f 100644 |
| 106 | +--- a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaConfig.java |
| 107 | ++++ b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaConfig.java |
| 108 | +@@ -28,6 +28,7 @@ public class OpaConfig |
| 109 | + private boolean logRequests; |
| 110 | + private boolean logResponses; |
| 111 | + private boolean allowPermissionManagementOperations; |
| 112 | ++ private boolean includeUserPrincipal; |
| 113 | + private Optional<URI> opaRowFiltersUri = Optional.empty(); |
| 114 | + private Optional<URI> opaColumnMaskingUri = Optional.empty(); |
| 115 | + private Optional<URI> opaBatchColumnMaskingUri = Optional.empty(); |
| 116 | +@@ -99,6 +100,19 @@ public class OpaConfig |
| 117 | + return this; |
| 118 | + } |
| 119 | + |
| 120 | ++ public boolean getIncludeUserPrincipal() |
| 121 | ++ { |
| 122 | ++ return this.includeUserPrincipal; |
| 123 | ++ } |
| 124 | ++ |
| 125 | ++ @Config("opa.include-user-principal") |
| 126 | ++ @ConfigDescription("Whether to include the users principal when sending authorization requests to OPA") |
| 127 | ++ public OpaConfig setIncludeUserPrincipal(boolean includeUserPrincipal) |
| 128 | ++ { |
| 129 | ++ this.includeUserPrincipal = includeUserPrincipal; |
| 130 | ++ return this; |
| 131 | ++ } |
| 132 | ++ |
| 133 | + @NotNull |
| 134 | + public Optional<URI> getOpaRowFiltersUri() |
| 135 | + { |
| 136 | +diff --git a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/TrinoIdentity.java b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/TrinoIdentity.java |
| 137 | +index feeface478..81768a764e 100644 |
| 138 | +--- a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/TrinoIdentity.java |
| 139 | ++++ b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/TrinoIdentity.java |
| 140 | +@@ -16,24 +16,30 @@ package io.trino.plugin.opa.schema; |
| 141 | + import com.google.common.collect.ImmutableSet; |
| 142 | + import io.trino.spi.security.Identity; |
| 143 | + |
| 144 | ++import java.security.Principal; |
| 145 | ++import java.util.Optional; |
| 146 | + import java.util.Set; |
| 147 | + |
| 148 | + import static java.util.Objects.requireNonNull; |
| 149 | + |
| 150 | + public record TrinoIdentity( |
| 151 | + String user, |
| 152 | +- Set<String> groups) |
| 153 | ++ Set<String> groups, |
| 154 | ++ Optional<Principal> principal) |
| 155 | + { |
| 156 | +- public static TrinoIdentity fromTrinoIdentity(Identity identity) |
| 157 | ++ public static TrinoIdentity fromTrinoIdentity(Identity identity, boolean includeUserPrincipal) |
| 158 | + { |
| 159 | + return new TrinoIdentity( |
| 160 | + identity.getUser(), |
| 161 | +- identity.getGroups()); |
| 162 | ++ identity.getGroups(), |
| 163 | ++ includeUserPrincipal ? identity.getPrincipal() : Optional.empty() |
| 164 | ++ ); |
| 165 | + } |
| 166 | + |
| 167 | + public TrinoIdentity |
| 168 | + { |
| 169 | + requireNonNull(user, "user is null"); |
| 170 | + groups = ImmutableSet.copyOf(requireNonNull(groups, "groups is null")); |
| 171 | ++ requireNonNull(principal, "principal is null"); |
| 172 | + } |
| 173 | + } |
| 174 | +diff --git a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/TrinoUser.java b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/TrinoUser.java |
| 175 | +index 90829cd427..53fb21f806 100644 |
| 176 | +--- a/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/TrinoUser.java |
| 177 | ++++ b/plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/TrinoUser.java |
| 178 | +@@ -38,8 +38,8 @@ public record TrinoUser(String user, @JsonUnwrapped TrinoIdentity identity) |
| 179 | + this(name, null); |
| 180 | + } |
| 181 | + |
| 182 | +- public TrinoUser(Identity identity) |
| 183 | ++ public TrinoUser(Identity identity, boolean includeUserPrincipal) |
| 184 | + { |
| 185 | +- this(null, TrinoIdentity.fromTrinoIdentity(identity)); |
| 186 | ++ this(null, TrinoIdentity.fromTrinoIdentity(identity, includeUserPrincipal)); |
| 187 | + } |
| 188 | + } |
| 189 | +diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/SerializableTestPrincipal.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/SerializableTestPrincipal.java |
| 190 | +new file mode 100644 |
| 191 | +index 0000000000..0de95954a7 |
| 192 | +--- /dev/null |
| 193 | ++++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/SerializableTestPrincipal.java |
| 194 | +@@ -0,0 +1,35 @@ |
| 195 | ++/* |
| 196 | ++ * Licensed under the Apache License, Version 2.0 (the "License"); |
| 197 | ++ * you may not use this file except in compliance with the License. |
| 198 | ++ * You may obtain a copy of the License at |
| 199 | ++ * |
| 200 | ++ * http://www.apache.org/licenses/LICENSE-2.0 |
| 201 | ++ * |
| 202 | ++ * Unless required by applicable law or agreed to in writing, software |
| 203 | ++ * distributed under the License is distributed on an "AS IS" BASIS, |
| 204 | ++ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
| 205 | ++ * See the License for the specific language governing permissions and |
| 206 | ++ * limitations under the License. |
| 207 | ++ */ |
| 208 | ++package io.trino.plugin.opa; |
| 209 | ++ |
| 210 | ++import com.fasterxml.jackson.annotation.JsonProperty; |
| 211 | ++import com.fasterxml.jackson.databind.annotation.JsonSerialize; |
| 212 | ++ |
| 213 | ++import java.security.Principal; |
| 214 | ++ |
| 215 | ++// A Principal for tests that can be serialized using Jackson |
| 216 | ++@JsonSerialize |
| 217 | ++public class SerializableTestPrincipal implements Principal { |
| 218 | ++ private String name; |
| 219 | ++ |
| 220 | ++ public SerializableTestPrincipal(String name) { |
| 221 | ++ this.name = name; |
| 222 | ++ } |
| 223 | ++ |
| 224 | ++ @Override |
| 225 | ++ @JsonProperty |
| 226 | ++ public String getName() { |
| 227 | ++ return name; |
| 228 | ++ } |
| 229 | ++} |
| 230 | +diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java |
| 231 | +index 4740ebdf94..abeda71245 100644 |
| 232 | +--- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java |
| 233 | ++++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java |
| 234 | +@@ -28,6 +28,7 @@ import io.trino.spi.connector.CatalogSchemaName; |
| 235 | + import io.trino.spi.connector.CatalogSchemaRoutineName; |
| 236 | + import io.trino.spi.connector.CatalogSchemaTableName; |
| 237 | + import io.trino.spi.connector.ColumnSchema; |
| 238 | ++import io.trino.spi.security.BasicPrincipal; |
| 239 | + import io.trino.spi.security.Identity; |
| 240 | + import io.trino.spi.security.PrincipalType; |
| 241 | + import io.trino.spi.security.SystemAccessControlFactory; |
| 242 | +@@ -221,6 +222,7 @@ final class TestOpaAccessControl |
| 243 | + { |
| 244 | + Identity dummyIdentity = Identity.forUser("dummy-user") |
| 245 | + .withGroups(ImmutableSet.of("some-group")) |
| 246 | ++ .withPrincipal(new BasicPrincipal("basic-principal")) |
| 247 | + .build(); |
| 248 | + ThrowingMethodWrapper wrappedMethod = new ThrowingMethodWrapper( |
| 249 | + accessControl -> callable.accept(accessControl, TEST_IDENTITY, dummyIdentity)); |
| 250 | +@@ -687,6 +689,39 @@ final class TestOpaAccessControl |
| 251 | + assertStringRequestsEqual(ImmutableSet.of(expectedRequest), mockClient.getRequests(), "/input"); |
| 252 | + } |
| 253 | + |
| 254 | ++ @Test |
| 255 | ++ void testIncludeUserPrincipal() { |
| 256 | ++ InstrumentedHttpClient mockClient = createMockHttpClient(OPA_SERVER_URI, request -> OK_RESPONSE); |
| 257 | ++ OpaAccessControl authorizer = (OpaAccessControl) OpaAccessControlFactory.create( |
| 258 | ++ ImmutableMap.of("opa.policy.uri", OPA_SERVER_URI.toString(), "opa.include-user-principal", "true"), |
| 259 | ++ Optional.of(mockClient), |
| 260 | ++ Optional.empty()); |
| 261 | ++ Identity sampleIdentityWithGroupsAndPrincipal = Identity.forUser("test_user").withGroups(ImmutableSet.of("some_group")).withPrincipal(new SerializableTestPrincipal("test_principal")).build(); |
| 262 | ++ authorizer.checkCanExecuteQuery(sampleIdentityWithGroupsAndPrincipal, TEST_QUERY_ID); |
| 263 | ++ |
| 264 | ++ String expectedRequest = |
| 265 | ++ """ |
| 266 | ++ { |
| 267 | ++ "action": { |
| 268 | ++ "operation": "ExecuteQuery" |
| 269 | ++ }, |
| 270 | ++ "context": { |
| 271 | ++ "identity": { |
| 272 | ++ "user": "test_user", |
| 273 | ++ "groups": ["some_group"], |
| 274 | ++ "principal": { |
| 275 | ++ "name": "test_principal" |
| 276 | ++ } |
| 277 | ++ }, |
| 278 | ++ "softwareStack": { |
| 279 | ++ "trinoVersion": "UNKNOWN" |
| 280 | ++ } |
| 281 | ++ } |
| 282 | ++ }\ |
| 283 | ++ """; |
| 284 | ++ assertStringRequestsEqual(ImmutableSet.of(expectedRequest), mockClient.getRequests(), "/input"); |
| 285 | ++ } |
| 286 | ++ |
| 287 | + @Test |
| 288 | + void testGetRowFiltersThrowsForIllegalResponse() |
| 289 | + { |
| 290 | +diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaConfig.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaConfig.java |
| 291 | +index 992b645302..d70975eb27 100644 |
| 292 | +--- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaConfig.java |
| 293 | ++++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaConfig.java |
| 294 | +@@ -36,7 +36,8 @@ final class TestOpaConfig |
| 295 | + .setOpaBatchColumnMaskingUri(null) |
| 296 | + .setLogRequests(false) |
| 297 | + .setLogResponses(false) |
| 298 | +- .setAllowPermissionManagementOperations(false)); |
| 299 | ++ .setAllowPermissionManagementOperations(false) |
| 300 | ++ .setIncludeUserPrincipal(false)); |
| 301 | + } |
| 302 | + |
| 303 | + @Test |
| 304 | +@@ -51,6 +52,7 @@ final class TestOpaConfig |
| 305 | + .put("opa.log-requests", "true") |
| 306 | + .put("opa.log-responses", "true") |
| 307 | + .put("opa.allow-permission-management-operations", "true") |
| 308 | ++ .put("opa.include-user-principal", "true") |
| 309 | + .buildOrThrow(); |
| 310 | + |
| 311 | + OpaConfig expected = new OpaConfig() |
| 312 | +@@ -61,7 +63,8 @@ final class TestOpaConfig |
| 313 | + .setOpaBatchColumnMaskingUri(URI.create("https://opa-column-masking.example.com")) |
| 314 | + .setLogRequests(true) |
| 315 | + .setLogResponses(true) |
| 316 | +- .setAllowPermissionManagementOperations(true); |
| 317 | ++ .setAllowPermissionManagementOperations(true) |
| 318 | ++ .setIncludeUserPrincipal(true); |
| 319 | + |
| 320 | + assertFullMapping(properties, expected); |
| 321 | + } |
0 commit comments