Skip to content

Commit dd4f02d

Browse files
sbernauersbernauer
committed
Support sending user principal to OPA
1 parent 1718f8f commit dd4f02d

File tree

9 files changed

+110
-13
lines changed

9 files changed

+110
-13
lines changed

‎docs/src/main/sphinx/security/opa-access-control.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ The following table lists the configuration properties for the OPA access contro
6161
* - `opa.allow-permission-management-operations`
6262
- Configure if permission management operations are allowed. Find more details in
6363
[](opa-permission-management). Defaults to `false`.
64+
* - `opa.include-user-principal`
65+
- Whether to include the users principal when sending authorization requests to OPA. Defaults to `false`.
6466
* - `opa.http-client.*`
6567
- Optional HTTP client configurations for the connection from Trino to OPA,
6668
for example `opa.http-client.http-proxy` for configuring the HTTP proxy.

‎plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaAccessControl.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ public sealed class OpaAccessControl
9090
private final LifeCycleManager lifeCycleManager;
9191
private final OpaHighLevelClient opaHighLevelClient;
9292
private final boolean allowPermissionManagementOperations;
93+
protected final boolean includeUserPrincipal;
9394
private final OpaPluginContext pluginContext;
9495

9596
@Inject
@@ -98,6 +99,7 @@ public OpaAccessControl(LifeCycleManager lifeCycleManager, OpaHighLevelClient op
9899
this.lifeCycleManager = requireNonNull(lifeCycleManager, "lifeCycleManager is null");
99100
this.opaHighLevelClient = requireNonNull(opaHighLevelClient, "opaHighLevelClient is null");
100101
this.allowPermissionManagementOperations = config.getAllowPermissionManagementOperations();
102+
this.includeUserPrincipal = config.getIncludeUserPrincipal();
101103
this.pluginContext = requireNonNull(pluginContext, "pluginContext is null");
102104
}
103105

@@ -124,7 +126,7 @@ public void checkCanExecuteQuery(Identity identity, QueryId queryId)
124126
@Override
125127
public void checkCanViewQueryOwnedBy(Identity identity, Identity queryOwner)
126128
{
127-
opaHighLevelClient.queryAndEnforce(buildQueryContext(identity), "ViewQueryOwnedBy", AccessDeniedException::denyViewQuery, OpaQueryInputResource.builder().user(new TrinoUser(queryOwner)).build());
129+
opaHighLevelClient.queryAndEnforce(buildQueryContext(identity), "ViewQueryOwnedBy", AccessDeniedException::denyViewQuery, OpaQueryInputResource.builder().user(new TrinoUser(queryOwner, this.includeUserPrincipal)).build());
128130
}
129131

130132
@Override
@@ -135,13 +137,13 @@ public Collection<Identity> filterViewQueryOwnedBy(Identity identity, Collection
135137
queryOwner -> buildQueryInputForSimpleResource(
136138
buildQueryContext(identity),
137139
"FilterViewQueryOwnedBy",
138-
OpaQueryInputResource.builder().user(new TrinoUser(queryOwner)).build()));
140+
OpaQueryInputResource.builder().user(new TrinoUser(queryOwner, this.includeUserPrincipal)).build()));
139141
}
140142

141143
@Override
142144
public void checkCanKillQueryOwnedBy(Identity identity, Identity queryOwner)
143145
{
144-
opaHighLevelClient.queryAndEnforce(buildQueryContext(identity), "KillQueryOwnedBy", AccessDeniedException::denyKillQuery, OpaQueryInputResource.builder().user(new TrinoUser(queryOwner)).build());
146+
opaHighLevelClient.queryAndEnforce(buildQueryContext(identity), "KillQueryOwnedBy", AccessDeniedException::denyKillQuery, OpaQueryInputResource.builder().user(new TrinoUser(queryOwner, this.includeUserPrincipal)).build());
145147
}
146148

147149
@Override
@@ -802,11 +804,11 @@ private static Map<String, Optional<Object>> convertProperties(Map<String, Objec
802804

803805
OpaQueryContext buildQueryContext(Identity trinoIdentity)
804806
{
805-
return new OpaQueryContext(TrinoIdentity.fromTrinoIdentity(trinoIdentity), pluginContext, Optional.empty());
807+
return new OpaQueryContext(TrinoIdentity.fromTrinoIdentity(trinoIdentity, this.includeUserPrincipal), pluginContext, Optional.empty());
806808
}
807809

808810
OpaQueryContext buildQueryContext(SystemSecurityContext securityContext)
809811
{
810-
return new OpaQueryContext(TrinoIdentity.fromTrinoIdentity(securityContext.getIdentity()), pluginContext, Optional.of(securityContext.getQueryId()));
812+
return new OpaQueryContext(TrinoIdentity.fromTrinoIdentity(securityContext.getIdentity(), this.includeUserPrincipal), pluginContext, Optional.of(securityContext.getQueryId()));
811813
}
812814
}

‎plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaBatchAccessControl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public Collection<Identity> filterViewQueryOwnedBy(Identity identity, Collection
7474
"FilterViewQueryOwnedBy",
7575
queryOwners,
7676
queryOwner -> OpaQueryInputResource.builder()
77-
.user(new TrinoUser(queryOwner))
77+
.user(new TrinoUser(queryOwner, this.includeUserPrincipal))
7878
.build());
7979
}
8080

‎plugin/trino-opa/src/main/java/io/trino/plugin/opa/OpaConfig.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ public class OpaConfig
2828
private boolean logRequests;
2929
private boolean logResponses;
3030
private boolean allowPermissionManagementOperations;
31+
private boolean includeUserPrincipal;
3132
private Optional<URI> opaRowFiltersUri = Optional.empty();
3233
private Optional<URI> opaColumnMaskingUri = Optional.empty();
3334
private Optional<URI> opaBatchColumnMaskingUri = Optional.empty();
@@ -99,6 +100,19 @@ public OpaConfig setAllowPermissionManagementOperations(boolean allowPermissionM
99100
return this;
100101
}
101102

103+
public boolean getIncludeUserPrincipal()
104+
{
105+
return this.includeUserPrincipal;
106+
}
107+
108+
@Config("opa.include-user-principal")
109+
@ConfigDescription("Whether to include the users principal when sending authorization requests to OPA")
110+
public OpaConfig setIncludeUserPrincipal(boolean includeUserPrincipal)
111+
{
112+
this.includeUserPrincipal = includeUserPrincipal;
113+
return this;
114+
}
115+
102116
@NotNull
103117
public Optional<URI> getOpaRowFiltersUri()
104118
{

‎plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/TrinoIdentity.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,24 +16,30 @@
1616
import com.google.common.collect.ImmutableSet;
1717
import io.trino.spi.security.Identity;
1818

19+
import java.security.Principal;
20+
import java.util.Optional;
1921
import java.util.Set;
2022

2123
import static java.util.Objects.requireNonNull;
2224

2325
public record TrinoIdentity(
2426
String user,
25-
Set<String> groups)
27+
Set<String> groups,
28+
Optional<Principal> principal)
2629
{
27-
public static TrinoIdentity fromTrinoIdentity(Identity identity)
30+
public static TrinoIdentity fromTrinoIdentity(Identity identity, boolean includeUserPrincipal)
2831
{
2932
return new TrinoIdentity(
3033
identity.getUser(),
31-
identity.getGroups());
34+
identity.getGroups(),
35+
includeUserPrincipal ? identity.getPrincipal() : Optional.empty()
36+
);
3237
}
3338

3439
public TrinoIdentity
3540
{
3641
requireNonNull(user, "user is null");
3742
groups = ImmutableSet.copyOf(requireNonNull(groups, "groups is null"));
43+
requireNonNull(principal, "principal is null");
3844
}
3945
}

‎plugin/trino-opa/src/main/java/io/trino/plugin/opa/schema/TrinoUser.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ public TrinoUser(String name)
3838
this(name, null);
3939
}
4040

41-
public TrinoUser(Identity identity)
41+
public TrinoUser(Identity identity, boolean includeUserPrincipal)
4242
{
43-
this(null, TrinoIdentity.fromTrinoIdentity(identity));
43+
this(null, TrinoIdentity.fromTrinoIdentity(identity, includeUserPrincipal));
4444
}
4545
}

‎plugin/trino-opa/src/test/java/io/trino/plugin/opa/SerializableTestPrincipal.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package io.trino.plugin.opa;
15+
16+
import com.fasterxml.jackson.annotation.JsonProperty;
17+
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
18+
19+
import java.security.Principal;
20+
21+
// A Principal for tests that can be serialized using Jackson
22+
@JsonSerialize
23+
public class SerializableTestPrincipal implements Principal {
24+
private String name;
25+
26+
public SerializableTestPrincipal(String name) {
27+
this.name = name;
28+
}
29+
30+
@Override
31+
@JsonProperty
32+
public String getName() {
33+
return name;
34+
}
35+
}

‎plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaAccessControl.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import io.trino.spi.connector.CatalogSchemaRoutineName;
3030
import io.trino.spi.connector.CatalogSchemaTableName;
3131
import io.trino.spi.connector.ColumnSchema;
32+
import io.trino.spi.security.BasicPrincipal;
3233
import io.trino.spi.security.Identity;
3334
import io.trino.spi.security.PrincipalType;
3435
import io.trino.spi.security.SystemAccessControlFactory;
@@ -223,6 +224,7 @@ private void testIdentityResourceActions(
223224
{
224225
Identity dummyIdentity = Identity.forUser("dummy-user")
225226
.withGroups(ImmutableSet.of("some-group"))
227+
.withPrincipal(new BasicPrincipal("basic-principal"))
226228
.build();
227229
ThrowingMethodWrapper wrappedMethod = new ThrowingMethodWrapper(
228230
accessControl -> callable.accept(accessControl, TEST_IDENTITY, dummyIdentity));
@@ -689,6 +691,39 @@ private void testRequestContextContentsForGivenTrinoVersion(Optional<SystemAcces
689691
assertStringRequestsEqual(ImmutableSet.of(expectedRequest), mockClient.getRequests(), "/input");
690692
}
691693

694+
@Test
695+
void testIncludeUserPrincipal() {
696+
InstrumentedHttpClient mockClient = createMockHttpClient(OPA_SERVER_URI, request -> OK_RESPONSE);
697+
OpaAccessControl authorizer = (OpaAccessControl) OpaAccessControlFactory.create(
698+
ImmutableMap.of("opa.policy.uri", OPA_SERVER_URI.toString(), "opa.include-user-principal", "true"),
699+
Optional.of(mockClient),
700+
Optional.empty());
701+
Identity sampleIdentityWithGroupsAndPrincipal = Identity.forUser("test_user").withGroups(ImmutableSet.of("some_group")).withPrincipal(new SerializableTestPrincipal("test_principal")).build();
702+
authorizer.checkCanExecuteQuery(sampleIdentityWithGroupsAndPrincipal, TEST_QUERY_ID);
703+
704+
String expectedRequest =
705+
"""
706+
{
707+
"action": {
708+
"operation": "ExecuteQuery"
709+
},
710+
"context": {
711+
"identity": {
712+
"user": "test_user",
713+
"groups": ["some_group"],
714+
"principal": {
715+
"name": "test_principal"
716+
}
717+
},
718+
"softwareStack": {
719+
"trinoVersion": "UNKNOWN"
720+
}
721+
}
722+
}\
723+
""";
724+
assertStringRequestsEqual(ImmutableSet.of(expectedRequest), mockClient.getRequests(), "/input");
725+
}
726+
692727
@Test
693728
void testGetRowFiltersThrowsForIllegalResponse()
694729
{

‎plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaConfig.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ void testDefaults()
3636
.setOpaBatchColumnMaskingUri(null)
3737
.setLogRequests(false)
3838
.setLogResponses(false)
39-
.setAllowPermissionManagementOperations(false));
39+
.setAllowPermissionManagementOperations(false)
40+
.setIncludeUserPrincipal(false));
4041
}
4142

4243
@Test
@@ -51,6 +52,7 @@ void testExplicitPropertyMappings()
5152
.put("opa.log-requests", "true")
5253
.put("opa.log-responses", "true")
5354
.put("opa.allow-permission-management-operations", "true")
55+
.put("opa.include-user-principal", "true")
5456
.buildOrThrow();
5557

5658
OpaConfig expected = new OpaConfig()
@@ -61,7 +63,8 @@ void testExplicitPropertyMappings()
6163
.setOpaBatchColumnMaskingUri(URI.create("https://opa-column-masking.example.com"))
6264
.setLogRequests(true)
6365
.setLogResponses(true)
64-
.setAllowPermissionManagementOperations(true);
66+
.setAllowPermissionManagementOperations(true)
67+
.setIncludeUserPrincipal(true);
6568

6669
assertFullMapping(properties, expected);
6770
}

0 commit comments

Comments
 (0)