trino#29302: OPA extra-credentials tests — fromJson converter, whole-object assertions, canonical TEST_QUERY_OWNER/EXPECTED_QUERY_OWNER (uncommitted draft)

opa-test-refactor.diff

diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/RequestTestUtilities.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/RequestTestUtilities.java
index 59a2ec14467..47a89af1f5d 100644
--- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/RequestTestUtilities.java
+++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/RequestTestUtilities.java
@@ -15,8 +15,11 @@ package io.trino.plugin.opa;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.json.JsonMapper;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import io.trino.plugin.opa.HttpClientUtils.MockResponse;
+import io.trino.plugin.opa.schema.TrinoIdentity;
+import io.trino.plugin.opa.schema.TrinoUser;
 import io.trino.spi.security.Identity;
 
 import java.io.IOException;
@@ -50,6 +53,27 @@ public final class RequestTestUtilities
         assertThat(extractedActualRequests).containsExactlyInAnyOrderElementsOf(parsedExpectedRequests);
     }
 
+    public static TrinoUser trinoUserFromJson(JsonNode userNode)
+    {
+        // A serialized TrinoUser has two shapes: a bare user name, or a @JsonUnwrapped TrinoIdentity,
+        // which always carries "groups". Jackson cannot disambiguate the shared "user" key on read,
+        // so the shapes are parsed manually.
+        if (userNode.has("groups")) {
+            return new TrinoUser(null, trinoIdentityFromJson(userNode));
+        }
+        return TrinoUser.createUser(userNode.path("user").asText());
+    }
+
+    public static TrinoIdentity trinoIdentityFromJson(JsonNode identityNode)
+    {
+        ImmutableSet.Builder<String> groups = ImmutableSet.builder();
+        identityNode.path("groups").forEach(group -> groups.add(group.asText()));
+        JsonNode credentialsNode = identityNode.path("extraCredentials");
+        ImmutableMap.Builder<String, String> extraCredentials = ImmutableMap.builder();
+        credentialsNode.fieldNames().forEachRemaining(key -> extraCredentials.put(key, credentialsNode.get(key).asText()));
+        return new TrinoIdentity(identityNode.path("user").asText(), groups.build(), extraCredentials.buildOrThrow());
+    }
+
     public static Function<JsonNode, MockResponse> buildValidatingRequestHandler(Identity expectedUser, int statusCode, String responseContents)
     {
         return buildValidatingRequestHandler(expectedUser, new MockResponse(responseContents, statusCode));
diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java
index d04b9f8f75c..05a6e16518b 100644
--- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java
+++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestConstants.java
@@ -13,9 +13,12 @@
  */
 package io.trino.plugin.opa;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import io.trino.execution.QueryIdGenerator;
 import io.trino.plugin.base.security.testing.TestingSystemAccessControlContext;
+import io.trino.plugin.opa.schema.TrinoIdentity;
+import io.trino.plugin.opa.schema.TrinoUser;
 import io.trino.spi.QueryId;
 import io.trino.spi.connector.CatalogSchemaTableName;
 import io.trino.spi.security.Identity;
@@ -64,6 +67,16 @@ public final class TestConstants
     public static final QueryId TEST_QUERY_ID = QueryId.valueOf("abcde");
     public static final SystemSecurityContext TEST_SECURITY_CONTEXT = new SystemSecurityContext(TEST_IDENTITY, new QueryIdGenerator().createNextQueryId(), Instant.now());
     public static final CatalogSchemaTableName TEST_COLUMN_MASKING_TABLE_NAME = new CatalogSchemaTableName("some_catalog", "some_schema", "some_table");
+    public static final Identity TEST_QUERY_OWNER = Identity.forUser("query-owner")
+            .withGroups(ImmutableSet.of("owner-group"))
+            .withAdditionalExtraCredentials(ImmutableMap.of("ai-service", "owner-ai-app", "otherKey", "owner-other-value"))
+            .build();
+    // TEST_QUERY_OWNER as OPA sees it with only "ai-service" allow-listed; spelled out literally
+    // rather than derived through createUser, so the whitelist filtering under test cannot leak
+    // into the expected value
+    public static final TrinoUser EXPECTED_QUERY_OWNER = new TrinoUser(
+            null,
+            new TrinoIdentity("query-owner", ImmutableSet.of("owner-group"), ImmutableMap.of("ai-service", "owner-ai-app")));
 
     public static OpaConfig simpleOpaConfig()
     {
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
index 0ffdc340efc..dda59f54b46 100644
--- 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
@@ -24,6 +24,8 @@ import io.trino.plugin.opa.AccessControlMethodHelpers.ThrowingMethodWrapper;
 import io.trino.plugin.opa.HttpClientUtils.InstrumentedHttpClient;
 import io.trino.plugin.opa.HttpClientUtils.MockResponse;
 import io.trino.plugin.opa.schema.OpaViewExpression;
+import io.trino.plugin.opa.schema.TrinoIdentity;
+import io.trino.plugin.opa.schema.TrinoUser;
 import io.trino.spi.QueryId;
 import io.trino.spi.connector.CatalogSchemaName;
 import io.trino.spi.connector.CatalogSchemaRoutineName;
@@ -54,7 +56,10 @@ import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static io.trino.plugin.opa.RequestTestUtilities.assertStringRequestsEqual;
 import static io.trino.plugin.opa.RequestTestUtilities.buildValidatingRequestHandler;
+import static io.trino.plugin.opa.RequestTestUtilities.trinoIdentityFromJson;
+import static io.trino.plugin.opa.RequestTestUtilities.trinoUserFromJson;
 import static io.trino.plugin.opa.TestConstants.BAD_REQUEST_RESPONSE;
+import static io.trino.plugin.opa.TestConstants.EXPECTED_QUERY_OWNER;
 import static io.trino.plugin.opa.TestConstants.MALFORMED_RESPONSE;
 import static io.trino.plugin.opa.TestConstants.NO_ACCESS_RESPONSE;
 import static io.trino.plugin.opa.TestConstants.OK_RESPONSE;
@@ -66,6 +71,7 @@ import static io.trino.plugin.opa.TestConstants.SERVER_ERROR_RESPONSE;
 import static io.trino.plugin.opa.TestConstants.TEST_COLUMN_MASKING_TABLE_NAME;
 import static io.trino.plugin.opa.TestConstants.TEST_IDENTITY;
 import static io.trino.plugin.opa.TestConstants.TEST_QUERY_ID;
+import static io.trino.plugin.opa.TestConstants.TEST_QUERY_OWNER;
 import static io.trino.plugin.opa.TestConstants.TEST_SECURITY_CONTEXT;
 import static io.trino.plugin.opa.TestConstants.UNDEFINED_RESPONSE;
 import static io.trino.plugin.opa.TestConstants.columnMaskingOpaConfig;
@@ -78,6 +84,7 @@ import static io.trino.plugin.opa.TestHelpers.createColumnSchema;
 import static io.trino.plugin.opa.TestHelpers.createMockHttpClient;
 import static io.trino.plugin.opa.TestHelpers.createOpaAuthorizer;
 import static io.trino.plugin.opa.TestHelpers.createResponseHandlerForParallelColumnMasking;
+import static io.trino.plugin.opa.schema.TrinoUser.createUser;
 import static org.assertj.core.api.Assertions.assertThat;
 
 final class TestOpaAccessControl
@@ -1076,10 +1083,8 @@ final class TestOpaAccessControl
         OpaAccessControl authorizer = createOpaAuthorizer(simpleOpaConfig().setExtraCredentialsKeys(ImmutableSet.of("ai-service", "ai-scope")), mockClient);
         operation.accept(authorizer);
 
-        JsonNode extraCredentials = mockClient.getRequests().get(0).at("/input/context/identity/extraCredentials");
-        assertThat(extraCredentials.path("ai-service").asText()).isEqualTo("my-ai-app");
-        assertThat(extraCredentials.path("ai-scope").asText()).isEqualTo("read-only");
-        assertThat(extraCredentials.has("otherKey")).isFalse();
+        TrinoIdentity identity = trinoIdentityFromJson(mockClient.getRequests().get(0).at("/input/context/identity"));
+        assertThat(identity).isEqualTo(new TrinoIdentity("source-user", ImmutableSet.of("some-group"), ImmutableMap.of("ai-service", "my-ai-app", "ai-scope", "read-only")));
     }
 
     @Test
@@ -1094,8 +1099,8 @@ final class TestOpaAccessControl
         OpaAccessControl authorizer = createOpaAuthorizer(simpleOpaConfig(), mockClient);
         authorizer.checkCanExecuteQuery(identityWithCredentials, TEST_QUERY_ID);
 
-        JsonNode identityNode = mockClient.getRequests().get(0).path("input").path("context").path("identity");
-        assertThat(identityNode.has("extraCredentials")).isFalse();
+        TrinoIdentity identity = trinoIdentityFromJson(mockClient.getRequests().get(0).at("/input/context/identity"));
+        assertThat(identity).isEqualTo(new TrinoIdentity("source-user", ImmutableSet.of("some-group"), ImmutableMap.of()));
     }
 
     @Test
@@ -1110,13 +1115,11 @@ final class TestOpaAccessControl
         authorizer.checkCanImpersonateUser(impersonator, "impersonated-user");
 
         JsonNode request = mockClient.getRequests().get(0);
-        JsonNode extraCredentials = request.at("/input/context/identity/extraCredentials");
-        assertThat(extraCredentials.path("ai-service").asText()).isEqualTo("my-ai-app");
-        assertThat(extraCredentials.has("otherKey")).isFalse();
+        TrinoIdentity impersonatorIdentity = trinoIdentityFromJson(request.at("/input/context/identity"));
+        assertThat(impersonatorIdentity).isEqualTo(new TrinoIdentity("impersonator", ImmutableSet.of(), ImmutableMap.of("ai-service", "my-ai-app")));
         // The impersonated user is identified by name only, so it carries no extra credentials of its own
-        JsonNode impersonatedUser = request.at("/input/action/resource/user");
-        assertThat(impersonatedUser.path("user").asText()).isEqualTo("impersonated-user");
-        assertThat(impersonatedUser.has("extraCredentials")).isFalse();
+        TrinoUser impersonatedUser = trinoUserFromJson(request.at("/input/action/resource/user"));
+        assertThat(impersonatedUser).isEqualTo(createUser("impersonated-user"));
     }
 
     @Test
@@ -1125,61 +1128,41 @@ final class TestOpaAccessControl
         Identity executor = Identity.forUser("executor-user")
                 .withAdditionalExtraCredentials(ImmutableMap.of("ai-service", "executor-ai-app", "otherKey", "executor-other-value"))
                 .build();
-        Identity queryOwner = Identity.forUser("query-owner")
-                .withGroups(ImmutableSet.of("owner-group"))
-                .withAdditionalExtraCredentials(ImmutableMap.of("ai-service", "owner-ai-app", "otherKey", "owner-other-value"))
-                .build();
 
         OpaConfig configWithWhitelist = simpleOpaConfig().setExtraCredentialsKeys(ImmutableSet.of("ai-service"));
         InstrumentedHttpClient mockClient = createMockHttpClient(OPA_SERVER_URI, _ -> OK_RESPONSE);
         OpaAccessControl authorizer = createOpaAuthorizer(configWithWhitelist, mockClient);
-        authorizer.checkCanKillQueryOwnedBy(executor, queryOwner);
+        authorizer.checkCanKillQueryOwnedBy(executor, TEST_QUERY_OWNER);
 
         JsonNode request = mockClient.getRequests().get(0);
         // The executor's own extra credentials are propagated in the identity context
-        JsonNode executorNode = request.path("input").path("context").path("identity");
-        assertThat(executorNode.path("user").asText()).isEqualTo("executor-user");
-        assertThat(executorNode.path("extraCredentials").path("ai-service").asText()).isEqualTo("executor-ai-app");
-        assertThat(executorNode.path("extraCredentials").has("otherKey")).isFalse();
+        TrinoIdentity executorIdentity = trinoIdentityFromJson(request.at("/input/context/identity"));
+        assertThat(executorIdentity).isEqualTo(new TrinoIdentity("executor-user", ImmutableSet.of(), ImmutableMap.of("ai-service", "executor-ai-app")));
         // The query owner's extra credentials are propagated separately in the resource
-        JsonNode ownerNode = request.path("input").path("action").path("resource").path("user");
-        assertThat(ownerNode.path("user").asText()).isEqualTo("query-owner");
-        assertThat(ownerNode.path("extraCredentials").path("ai-service").asText()).isEqualTo("owner-ai-app");
-        assertThat(ownerNode.path("extraCredentials").has("otherKey")).isFalse();
+        TrinoUser owner = trinoUserFromJson(request.at("/input/action/resource/user"));
+        assertThat(owner).isEqualTo(EXPECTED_QUERY_OWNER);
     }
 
     @Test
     void testViewQueryOwnedByForwardsOwnerExtraCredentials()
     {
-        Identity queryOwner = Identity.forUser("query-owner")
-                .withAdditionalExtraCredentials(ImmutableMap.of("ai-service", "owner-ai-app", "otherKey", "owner-other-value"))
-                .build();
-
         InstrumentedHttpClient mockClient = createMockHttpClient(OPA_SERVER_URI, _ -> OK_RESPONSE);
         OpaAccessControl authorizer = createOpaAuthorizer(simpleOpaConfig().setExtraCredentialsKeys(ImmutableSet.of("ai-service")), mockClient);
-        authorizer.checkCanViewQueryOwnedBy(TEST_IDENTITY, queryOwner);
+        authorizer.checkCanViewQueryOwnedBy(TEST_IDENTITY, TEST_QUERY_OWNER);
 
-        JsonNode ownerNode = mockClient.getRequests().get(0).at("/input/action/resource/user");
-        assertThat(ownerNode.path("user").asText()).isEqualTo("query-owner");
-        assertThat(ownerNode.path("extraCredentials").path("ai-service").asText()).isEqualTo("owner-ai-app");
-        assertThat(ownerNode.path("extraCredentials").has("otherKey")).isFalse();
+        TrinoUser owner = trinoUserFromJson(mockClient.getRequests().get(0).at("/input/action/resource/user"));
+        assertThat(owner).isEqualTo(EXPECTED_QUERY_OWNER);
     }
 
     @Test
     void testFilterViewQueryOwnedByForwardsOwnerExtraCredentials()
     {
-        Identity queryOwner = Identity.forUser("query-owner")
-                .withAdditionalExtraCredentials(ImmutableMap.of("ai-service", "owner-ai-app", "otherKey", "owner-other-value"))
-                .build();
-
         InstrumentedHttpClient mockClient = createMockHttpClient(OPA_SERVER_URI, _ -> OK_RESPONSE);
         OpaAccessControl authorizer = createOpaAuthorizer(simpleOpaConfig().setExtraCredentialsKeys(ImmutableSet.of("ai-service")), mockClient);
-        authorizer.filterViewQueryOwnedBy(TEST_IDENTITY, ImmutableList.of(queryOwner));
+        authorizer.filterViewQueryOwnedBy(TEST_IDENTITY, ImmutableList.of(TEST_QUERY_OWNER));
 
-        JsonNode ownerNode = mockClient.getRequests().get(0).at("/input/action/resource/user");
-        assertThat(ownerNode.path("user").asText()).isEqualTo("query-owner");
-        assertThat(ownerNode.path("extraCredentials").path("ai-service").asText()).isEqualTo("owner-ai-app");
-        assertThat(ownerNode.path("extraCredentials").has("otherKey")).isFalse();
+        TrinoUser owner = trinoUserFromJson(mockClient.getRequests().get(0).at("/input/action/resource/user"));
+        assertThat(owner).isEqualTo(EXPECTED_QUERY_OWNER);
     }
 
     private void testGetColumnMasks(Map<ColumnSchema, String> columnResponseContent, Map<ColumnSchema, OpaViewExpression> expectedResult)
diff --git a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaBatchAccessControlFiltering.java b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaBatchAccessControlFiltering.java
index 2648390401d..1c5233dc24f 100644
--- a/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaBatchAccessControlFiltering.java
+++ b/plugin/trino-opa/src/test/java/io/trino/plugin/opa/TestOpaBatchAccessControlFiltering.java
@@ -13,13 +13,13 @@
  */
 package io.trino.plugin.opa;
 
-import com.fasterxml.jackson.databind.JsonNode;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import io.trino.plugin.opa.FunctionalHelpers.Pair;
 import io.trino.plugin.opa.HttpClientUtils.InstrumentedHttpClient;
 import io.trino.plugin.opa.HttpClientUtils.MockResponse;
+import io.trino.plugin.opa.schema.TrinoUser;
 import io.trino.spi.connector.SchemaTableName;
 import io.trino.spi.function.SchemaFunctionName;
 import io.trino.spi.security.Identity;
@@ -37,9 +37,12 @@ import java.util.stream.Stream;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static io.trino.plugin.opa.RequestTestUtilities.assertStringRequestsEqual;
 import static io.trino.plugin.opa.RequestTestUtilities.buildValidatingRequestHandler;
+import static io.trino.plugin.opa.RequestTestUtilities.trinoUserFromJson;
+import static io.trino.plugin.opa.TestConstants.EXPECTED_QUERY_OWNER;
 import static io.trino.plugin.opa.TestConstants.OK_RESPONSE;
 import static io.trino.plugin.opa.TestConstants.OPA_SERVER_BATCH_URI;
 import static io.trino.plugin.opa.TestConstants.TEST_IDENTITY;
+import static io.trino.plugin.opa.TestConstants.TEST_QUERY_OWNER;
 import static io.trino.plugin.opa.TestConstants.TEST_SECURITY_CONTEXT;
 import static io.trino.plugin.opa.TestConstants.batchFilteringOpaConfig;
 import static io.trino.plugin.opa.TestHelpers.assertAccessControlMethodThrowsForIllegalResponses;
@@ -94,17 +97,12 @@ final class TestOpaBatchAccessControlFiltering
     @Test
     void testFilterViewQueryOwnedByForwardsOwnerExtraCredentials()
     {
-        Identity queryOwner = Identity.forUser("query-owner")
-                .withAdditionalExtraCredentials(ImmutableMap.of("ai-service", "owner-ai-app", "otherKey", "other-value"))
-                .build();
-
         InstrumentedHttpClient mockClient = createMockHttpClient(OPA_SERVER_BATCH_URI, _ -> new MockResponse("{\"result\": [0]}", 200));
         OpaAccessControl authorizer = createOpaAuthorizer(batchFilteringOpaConfig().setExtraCredentialsKeys(ImmutableSet.of("ai-service")), mockClient);
-        authorizer.filterViewQueryOwnedBy(TEST_IDENTITY, ImmutableList.of(queryOwner));
+        authorizer.filterViewQueryOwnedBy(TEST_IDENTITY, ImmutableList.of(TEST_QUERY_OWNER));
 
-        JsonNode ownerNode = mockClient.getRequests().get(0).at("/input/action/filterResources/0/user");
-        assertThat(ownerNode.path("extraCredentials").path("ai-service").asText()).isEqualTo("owner-ai-app");
-        assertThat(ownerNode.path("extraCredentials").has("otherKey")).isFalse();
+        TrinoUser owner = trinoUserFromJson(mockClient.getRequests().get(0).at("/input/action/filterResources/0/user"));
+        assertThat(owner).isEqualTo(EXPECTED_QUERY_OWNER);
     }
 
     @Test