From 9efd904202f26cea36e6049a0e2b11a4b72f0524 Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Wed, 2 Sep 2026 16:07:04 +0300 Subject: [PATCH 1/2] test: compare access decisions across the two storage layouts #1870 Seeds durable state - an owned resource, a read share, a write share, a published resource, a pending review copy, an application, app data - and then asks 21 access questions of both layouts, comparing the decisions. The questions go over HTTP rather than into AccessService directly. populatePermissions runs the same eleven-rule chain and puts its result in the response, so this observes the real chain with a real context; a hand-built context would mostly prove the mock was set up the way the test expected. Every cell names the rule it exists to exercise and the permissions it must yield, and some name permissions it must not. Without that a matrix that stopped exercising the chain would compare denied to denied and report the layouts identical. The matrix also declares the whole chain, so a rule that no cell reaches fails unless it is listed as uncovered with a reason - one is, and the reason is that it needs a deployment on the context, which no request this differ can make ever has. It should come back clean, and does: shares and rules key on logical urls and bucketLocation stays legacy, so the inputs never change. The reason to build it anyway is that the same differ answers "does the new engine decide identically?" when the chain is replaced, and it has to exist while there is still a working reference implementation to capture behaviour from. Found api_key_data/ unmapped, which is fixed in the branch below. Co-Authored-By: Claude Opus 5 (1M context) --- .../core/server/layout/AccessMatrix.java | 175 +++++++++++ .../core/server/layout/CorpusRunner.java | 10 +- .../core/server/layout/DialInstance.java | 56 +++- .../server/layout/LayoutAccessDifferTest.java | 157 ++++++++++ .../layout-diff/access-corpus/01-seed.json | 273 +++++++++++++++++ .../resources/layout-diff/access-matrix.json | 275 ++++++++++++++++++ 6 files changed, 943 insertions(+), 3 deletions(-) create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/AccessMatrix.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/layout/LayoutAccessDifferTest.java create mode 100644 server/src/test/resources/layout-diff/access-corpus/01-seed.json create mode 100644 server/src/test/resources/layout-diff/access-matrix.json diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/AccessMatrix.java b/server/src/test/java/com/epam/aidial/core/server/layout/AccessMatrix.java new file mode 100644 index 000000000..a206f0c7d --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/AccessMatrix.java @@ -0,0 +1,175 @@ +package com.epam.aidial.core.server.layout; + +import com.epam.aidial.core.server.util.ProxyUtil; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.core.type.TypeReference; +import io.vertx.core.json.JsonObject; +import lombok.SneakyThrows; + +import java.io.InputStream; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; + +/** + * Asks "what may this subject do to this resource?" of a running instance, once per cell, and records the + * answer so the two layouts can be compared on decisions rather than on responses. + * + *

The question goes over HTTP rather than into {@code AccessService} directly. {@code populatePermissions} + * runs the same eleven-rule chain and puts its result in the response, so this observes the real chain with a + * real context — where a hand-built context would mostly prove that the mock was set up the way the test + * expected. + */ +public class AccessMatrix { + + private static final String MATRIX = "layout-diff/access-matrix.json"; + + /** + * @param subject who is asking — a key in the corpus's subject table + * @param resource logical url, with {@code ${…}} placeholders resolved from the seed run + * @param rule the permission rule this cell exists to exercise, for the coverage report + * @param expects the permissions this cell must yield. A cell that grants less than this is not + * exercising its rule, and a matrix that quietly stopped exercising the chain would + * compare "denied" to "denied" and call the layouts identical. + * @param forbids permissions this cell must not yield, for the boundaries worth pinning — a read-only + * share that starts granting write is not something to notice only in P3. + */ + @JsonIgnoreProperties(ignoreUnknown = false) + public record Cell(String name, String subject, String resource, String rule, + List expects, List forbids) { + + public List forbidsOrEmpty() { + return forbids == null ? List.of() : forbids; + } + } + + /** + * What a subject turns into on the wire. {@code perRequest} builds an application caller — a per-request + * key issued against {@code apiKey}, optionally carrying attached deployments or its own source app. + */ + @JsonIgnoreProperties(ignoreUnknown = false) + public record Subject(String name, String apiKey, String authorization, PerRequest perRequest) { + + @JsonIgnoreProperties(ignoreUnknown = false) + public record PerRequest(String sourceDeployment, Map> attach, + Map> share) { + } + } + + /** + * @param chain every rule in {@code AccessService}'s permission chain, so a rule that no cell reaches + * is a visible gap rather than an absence nobody notices + * @param uncovered rules deliberately not covered, and why + */ + @JsonIgnoreProperties(ignoreUnknown = false) + public record Definition(List chain, Map uncovered, + List subjects, List cells) { + } + + /** + * One cell's answer: the permissions granted, or why none were. + */ + public record Decision(String cell, String rule, int status, Set permissions) { + + public String describe() { + return status == 200 ? String.join(",", permissions) : "http " + status; + } + } + + @SneakyThrows + public static Definition load() { + try (InputStream in = AccessMatrix.class.getClassLoader().getResourceAsStream(MATRIX)) { + if (in == null) { + throw new IllegalStateException(MATRIX + " is missing"); + } + return ProxyUtil.MAPPER.readValue(in, new TypeReference() { }); + } + } + + /** + * Evaluates every cell against one instance. Placeholders resolve from {@code variables}, which the seed + * run filled in — buckets, and anything the seed scenarios captured. + */ + public static Map evaluate(DialInstance instance, Definition definition, + Map variables) { + Map> credentials = resolveSubjects(instance, definition.subjects(), variables); + + Map decisions = new LinkedHashMap<>(); + for (Cell cell : definition.cells()) { + Map headers = credentials.get(cell.subject()); + if (headers == null) { + throw new IllegalStateException("Cell '" + cell.name() + "' names unknown subject " + cell.subject()); + } + + String url = resolve(cell.resource(), variables); + RecordedResponse response = + instance.send("GET", "/v1/metadata/" + url, "permissions=true", null, headers, null); + + decisions.put(cell.name(), new Decision(cell.name(), cell.rule(), response.status(), + permissions(response))); + } + return decisions; + } + + private static Set permissions(RecordedResponse response) { + if (response.status() != 200 || response.body() == null) { + return Set.of(); + } + + Set granted = new TreeSet<>(); + var array = new JsonObject(response.body()).getJsonArray("permissions"); + if (array != null) { + array.forEach(permission -> granted.add(String.valueOf(permission))); + } + return granted; + } + + /** + * Turns each subject into the credential its requests carry. Per-request keys are minted here rather than + * declared in the corpus because only a live instance can issue one. + */ + private static Map> resolveSubjects(DialInstance instance, List subjects, + Map variables) { + Map> credentials = new LinkedHashMap<>(); + for (Subject subject : subjects) { + if (subject.authorization() != null) { + credentials.put(subject.name(), Map.of("authorization", subject.authorization())); + } else if (subject.perRequest() == null) { + credentials.put(subject.name(), Map.of("api-key", subject.apiKey())); + } else { + credentials.put(subject.name(), Map.of("api-key", instance.issuePerRequestKey(resolve(subject, variables)))); + } + } + return credentials; + } + + private static Subject resolve(Subject subject, Map variables) { + Subject.PerRequest perRequest = subject.perRequest(); + return new Subject(subject.name(), subject.apiKey(), subject.authorization(), + new Subject.PerRequest(resolve(perRequest.sourceDeployment(), variables), + resolve(perRequest.attach(), variables), resolve(perRequest.share(), variables))); + } + + private static Map> resolve(Map> urls, Map variables) { + if (urls == null) { + return null; + } + Map> resolved = new LinkedHashMap<>(); + urls.forEach((url, permissions) -> resolved.put(resolve(url, variables), permissions)); + return resolved; + } + + private static String resolve(String template, Map variables) { + String resolved = template; + for (Map.Entry variable : variables.entrySet()) { + resolved = resolved.replace("${" + variable.getKey() + "}", variable.getValue()); + } + int unresolved = resolved.indexOf("${"); + if (unresolved >= 0) { + throw new IllegalStateException("Unresolved placeholder in: " + resolved.substring(unresolved)); + } + return resolved; + } +} diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java b/server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java index f079717ef..7358f01db 100644 --- a/server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java +++ b/server/src/test/java/com/epam/aidial/core/server/layout/CorpusRunner.java @@ -29,6 +29,7 @@ public class CorpusRunner { public static final String REPLAY_CORPUS = "layout-diff/corpus"; + public static final String ACCESS_CORPUS = "layout-diff/access-corpus"; private static final String API_KEY_1 = "proxyKey1"; private static final String API_KEY_2 = "proxyKey2"; @@ -69,9 +70,12 @@ public static List loadCorpus(String dir) { * runs must agree on it before any response comparison means anything — buckets thread through nearly every * url in the corpus, so a difference there would make every other difference unreadable. {@code captures} * holds what each scenario captured, by scenario name: normalisation replaces a captured value with its - * role name wherever it appears, so the values themselves are only comparable here. + * role name wherever it appears, so the values themselves are only comparable here. {@code variables} is + * the flat view of the same — buckets plus every capture — which the access matrix resolves its + * templates with. */ public record Run(Map buckets, + Map variables, Map> captures, Map responses) { } @@ -88,6 +92,7 @@ public static Run replay(DialInstance instance, List scenarios) { Map recorded = new LinkedHashMap<>(); Map> captures = new LinkedHashMap<>(); + Map allVariables = new LinkedHashMap<>(buckets); for (Scenario scenario : scenarios) { Map variables = new HashMap<>(buckets); Map raw = new LinkedHashMap<>(); @@ -116,8 +121,9 @@ public static Run replay(DialInstance instance, List scenarios) { Map captured = new LinkedHashMap<>(variables); captured.keySet().removeAll(buckets.keySet()); captures.put(scenario.name(), captured); + allVariables.putAll(variables); } - return new Run(buckets, captures, recorded); + return new Run(buckets, allVariables, captures, recorded); } private static void capture(Scenario.Step step, RecordedResponse response, Map variables) { diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java b/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java index dc7ab5d2a..ea88855bb 100644 --- a/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java +++ b/server/src/test/java/com/epam/aidial/core/server/layout/DialInstance.java @@ -1,14 +1,19 @@ package com.epam.aidial.core.server.layout; +import com.epam.aidial.core.config.ResourceAccessType; import com.epam.aidial.core.server.AiDial; import com.epam.aidial.core.server.AiDialLifecycle; import com.epam.aidial.core.server.FileUtil; +import com.epam.aidial.core.server.data.ApiKeyData; +import com.epam.aidial.core.server.data.AutoSharedData; +import com.epam.aidial.core.server.data.permission.PerRequestSharedData; import com.epam.aidial.core.server.security.AccessTokenValidator; import com.epam.aidial.core.server.security.ExtractedClaims; import com.epam.aidial.core.server.util.ProxyUtil; import com.fasterxml.jackson.databind.node.ObjectNode; import io.vertx.core.Future; import io.vertx.core.json.Json; +import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; import lombok.Getter; import lombok.SneakyThrows; @@ -31,7 +36,9 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicLong; +import java.util.stream.Collectors; /** * One full DIAL stack — embedded Redis, filesystem blob store, HTTP server — brought up under a chosen @@ -44,6 +51,12 @@ */ public class DialInstance implements AutoCloseable { + /** + * Role the global-reader rule matches. The rule is off unless {@code access.globalReader} is configured, + * so the harness configures it — otherwise the differ would compare a rule that can never fire. + */ + public static final String GLOBAL_READER_ROLE = "layout-diff-global-reader"; + private static final String ID_SEED = "0"; private static final long FIRST_ID = 123; @@ -127,7 +140,12 @@ private AiDial start(JsonObject layoutSettings, int redisPort) throws Exception JsonObject settings = AiDial.settings() .mergeIn(new JsonObject(overrides), true) - .mergeIn(new JsonObject().put("storage", new JsonObject().put("layout", layoutSettings)), true); + .mergeIn(new JsonObject().put("storage", new JsonObject().put("layout", layoutSettings)), true) + .mergeIn(new JsonObject().put("access", new JsonObject().put("globalReader", new JsonObject() + .put("rules", new JsonArray().add(new JsonObject() + .put("source", "roles") + .put("function", "EQUAL") + .put("targets", new JsonArray().add(GLOBAL_READER_ROLE)))))), true); AiDial instance = new AiDial(); instance.setSettings(settings); @@ -138,6 +156,10 @@ private AiDial start(JsonObject layoutSettings, int redisPort) throws Exception return instance; } + private static Set accessTypes(List permissions) { + return permissions.stream().map(ResourceAccessType::valueOf).collect(Collectors.toSet()); + } + private static AccessTokenValidator claimsValidator() { AccessTokenValidator validator = Mockito.mock(AccessTokenValidator.class); Mockito.when(validator.extractClaims(Mockito.any())).thenAnswer(invocation -> { @@ -154,6 +176,38 @@ private static AccessTokenValidator claimsValidator() { return validator; } + /** + * Issues the api key an application-shaped caller uses. Four of the eleven permission rules only fire for + * a caller that is an application rather than a person, and a per-request key is what makes one; it can + * only be minted against a live instance, so the corpus declares the shape and this fills it in. + */ + public String issuePerRequestKey(AccessMatrix.Subject subject) { + ApiKeyData original = dial.getProxy().getApiKeyStore().getApiKeyData(subject.apiKey(), null).result(); + + ApiKeyData perRequest = new ApiKeyData(); + perRequest.setOriginalKey(original.getOriginalKey()); + perRequest.setExtractedClaims(original.getExtractedClaims()); + perRequest.setSourceDeployment(subject.perRequest().sourceDeployment()); + perRequest.setTraceId("layout-diff-trace"); + + Map> attach = subject.perRequest().attach(); + if (attach != null) { + Map attached = new LinkedHashMap<>(); + attach.forEach((url, permissions) -> attached.put(url, new AutoSharedData(accessTypes(permissions)))); + perRequest.setAttachedDeployments(attached); + } + + Map> share = subject.perRequest().share(); + if (share != null) { + Map shared = new LinkedHashMap<>(); + share.forEach((url, permissions) -> shared.put(url, new PerRequestSharedData(accessTypes(permissions)))); + perRequest.setPerRequestSharedResources(shared); + } + + dial.getProxy().getApiKeyStore().assignPerRequestApiKey(perRequest); + return perRequest.getPerRequestKey(); + } + /** * The bucket the given api key writes into. Derived from the caller identity and the encryption secret, * neither of which the layout touches, so the two instances are expected to report the same value — diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/LayoutAccessDifferTest.java b/server/src/test/java/com/epam/aidial/core/server/layout/LayoutAccessDifferTest.java new file mode 100644 index 000000000..1063894dd --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/layout/LayoutAccessDifferTest.java @@ -0,0 +1,157 @@ +package com.epam.aidial.core.server.layout; + +import com.epam.aidial.core.server.util.ProxyUtil; +import com.epam.aidial.core.storage.resource.LegacyStorageLayout; +import com.epam.aidial.core.storage.resource.StorageLayouts; +import io.vertx.core.json.JsonObject; +import lombok.SneakyThrows; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Asks the same access questions of both layouts and fails on any answer that differs. + * + *

It should come back clean: shares and rules key on logical urls and {@code bucketLocation} stays legacy, + * so the inputs to the permission chain never change. That is an assumption spanning eleven rules — cheap to + * verify, expensive to be wrong about — and the same differ is what answers "does the new engine decide + * identically?" in P3, when there is a genuine rewrite to check and no reference implementation left to + * capture behaviour from. + */ +@Tag("layout-diff") +public class LayoutAccessDifferTest { + + private static final String TENANT = "layout-diff-tenant"; + + private static final int LEGACY_REDIS_PORT = 16373; + private static final int TENANT_ROOTED_REDIS_PORT = 16374; + + private static final Path REPORT_DIR = Paths.get("build", "reports", "layout-diff"); + + private record Run(Map variables, Map decisions) { + } + + @AfterEach + public void restoreLegacyLayout() { + StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); + } + + @Test + public void testBothLayoutsDecideIdentically() { + List seed = CorpusRunner.loadCorpus(CorpusRunner.ACCESS_CORPUS); + AccessMatrix.Definition matrix = AccessMatrix.load(); + + Run legacy = run("access-legacy", new JsonObject().put("tenantRooted", false), + LEGACY_REDIS_PORT, seed, matrix); + Run tenantRooted = run("access-tenant-rooted", new JsonObject() + .put("tenantRooted", true) + .put("defaultTenant", TENANT), + TENANT_ROOTED_REDIS_PORT, seed, matrix); + + write("access-legacy.json", legacy.decisions()); + write("access-tenant-rooted.json", tenantRooted.decisions()); + + List problems = new ArrayList<>(); + problems.addAll(vacuousCells(matrix, legacy.decisions())); + problems.addAll(uncoveredRules(matrix, legacy.decisions())); + problems.addAll(divergences(legacy.decisions(), tenantRooted.decisions())); + + if (!problems.isEmpty()) { + fail(String.join("\n", problems)); + } + } + + /** + * A cell that grants less than it declares is not exercising its rule. Without this the suite would + * happily compare "denied" to "denied" across every cell and report the layouts identical. + */ + private static List vacuousCells(AccessMatrix.Definition matrix, + Map decisions) { + List problems = new ArrayList<>(); + for (AccessMatrix.Cell cell : matrix.cells()) { + AccessMatrix.Decision decision = decisions.get(cell.name()); + if (!decision.permissions().containsAll(cell.expects())) { + problems.add("Cell '" + cell.name() + "' (" + cell.rule() + ") expected at least " + + cell.expects() + " but got " + decision.describe() + + " — the matrix is not exercising the rule it claims to."); + } + for (String forbidden : cell.forbidsOrEmpty()) { + if (decision.permissions().contains(forbidden)) { + problems.add("Cell '" + cell.name() + "' (" + cell.rule() + ") must not grant " + + forbidden + " but got " + decision.describe() + "."); + } + } + } + return problems; + } + + /** + * The chain is a union over eleven rules, and the ones that fire rarely are the ones a re-addressing bug + * would break silently. A rule with no cell that actually grants through it is not being compared at all. + */ + private static List uncoveredRules(AccessMatrix.Definition matrix, + Map decisions) { + Set granting = new LinkedHashSet<>(); + for (AccessMatrix.Cell cell : matrix.cells()) { + if (!decisions.get(cell.name()).permissions().isEmpty()) { + granting.add(cell.rule()); + } + } + + List problems = new ArrayList<>(); + for (String rule : matrix.chain()) { + if (granting.contains(rule)) { + continue; + } + String reason = matrix.uncovered().get(rule); + if (reason == null) { + problems.add("Rule '" + rule + "' in the permission chain is not covered by any cell that " + + "grants, and is not listed under \"uncovered\" with a reason."); + } + } + return problems; + } + + private static List divergences(Map legacy, + Map tenantRooted) { + List problems = new ArrayList<>(); + for (Map.Entry entry : legacy.entrySet()) { + AccessMatrix.Decision left = entry.getValue(); + AccessMatrix.Decision right = tenantRooted.get(entry.getKey()); + if (right == null || !left.permissions().equals(right.permissions()) || left.status() != right.status()) { + problems.add("Access decision differs for '" + entry.getKey() + "' (" + left.rule() + "):\n" + + " legacy: " + left.describe() + "\n" + + " tenantRooted: " + (right == null ? "missing" : right.describe())); + } + } + return problems; + } + + private static Run run(String name, JsonObject layoutSettings, int redisPort, + List seed, AccessMatrix.Definition matrix) { + try (DialInstance instance = new DialInstance(name, layoutSettings, redisPort)) { + CorpusRunner.Run seeded = CorpusRunner.replay(instance, seed); + return new Run(seeded.variables(), AccessMatrix.evaluate(instance, matrix, seeded.variables())); + } finally { + StorageLayouts.useLayout(LegacyStorageLayout.INSTANCE); + } + } + + @SneakyThrows + private static void write(String file, Object content) { + Files.createDirectories(REPORT_DIR); + Files.writeString(REPORT_DIR.resolve(file), + ProxyUtil.MAPPER.writerWithDefaultPrettyPrinter().writeValueAsString(content)); + } +} diff --git a/server/src/test/resources/layout-diff/access-corpus/01-seed.json b/server/src/test/resources/layout-diff/access-corpus/01-seed.json new file mode 100644 index 000000000..90d44577f --- /dev/null +++ b/server/src/test/resources/layout-diff/access-corpus/01-seed.json @@ -0,0 +1,273 @@ +{ + "name": "access-seed", + "description": "Leaves behind the state the access matrix asks about. Unlike the replay corpus these scenarios deliberately do not clean up: a revoked share or a deleted resource would make every cell answer 'denied', and a matrix of denials would compare equal under both layouts while proving nothing.", + "steps": [ + { + "name": "create-own", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/access/own", + "body": { + "id": "own", + "name": "own", + "model": { + "id": "model_id" + }, + "prompt": "p", + "temperature": 1, + "folderId": "access", + "messages": [], + "assistantModelId": "a", + "lastActivityDate": 1 + } + }, + { + "name": "create-to-share", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/access/shared", + "body": { + "id": "shared", + "name": "shared", + "model": { + "id": "model_id" + }, + "prompt": "p", + "temperature": 1, + "folderId": "access", + "messages": [], + "assistantModelId": "a", + "lastActivityDate": 2 + } + }, + { + "name": "share-read", + "method": "POST", + "path": "/v1/ops/resource/share/create", + "body": { + "invitationType": "link", + "resources": [ + { + "url": "conversations/${bucket1}/access/shared" + } + ] + }, + "capture": { + "readInvitationId": { + "at": "/invitationLink", + "extract": "/v1/invitations/(.+)$" + } + } + }, + { + "name": "accept-read-share", + "method": "GET", + "path": "/v1/invitations/${readInvitationId}", + "query": "accept=true", + "headers": { + "api-key": "proxyKey2" + } + }, + { + "name": "create-to-share-write", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/access/writable", + "body": { + "id": "writable", + "name": "writable", + "model": { + "id": "model_id" + }, + "prompt": "p", + "temperature": 1, + "folderId": "access", + "messages": [], + "assistantModelId": "a", + "lastActivityDate": 3 + } + }, + { + "name": "share-write", + "method": "POST", + "path": "/v1/ops/resource/share/create", + "body": { + "invitationType": "link", + "resources": [ + { + "url": "conversations/${bucket1}/access/writable", + "permissions": [ + "READ", + "WRITE" + ] + } + ] + }, + "capture": { + "writeInvitationId": { + "at": "/invitationLink", + "extract": "/v1/invitations/(.+)$" + } + } + }, + { + "name": "accept-write-share", + "method": "GET", + "path": "/v1/invitations/${writeInvitationId}", + "query": "accept=true", + "headers": { + "api-key": "proxyKey2" + } + }, + { + "name": "create-application", + "method": "PUT", + "path": "/v1/applications/${bucket1}/access/app", + "body": { + "display_name": "app", + "display_version": "1.0", + "endpoint": "http://localhost:9999/completions", + "description": "access matrix" + } + }, + { + "name": "create-to-publish", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/access/publishable", + "body": { + "id": "publishable", + "name": "publishable", + "model": { + "id": "model_id" + }, + "prompt": "p", + "temperature": 1, + "folderId": "access", + "messages": [], + "assistantModelId": "a", + "lastActivityDate": 4 + } + }, + { + "name": "publish", + "method": "POST", + "path": "/v1/ops/publication/create", + "body": { + "name": "access matrix publication", + "targetFolder": "public/access/", + "resources": [ + { + "action": "ADD", + "sourceUrl": "conversations/${bucket1}/access/publishable", + "targetUrl": "conversations/public/access/published" + } + ], + "rules": [ + { + "source": "roles", + "function": "TRUE" + } + ] + }, + "capture": { + "approvedPublicationUrl": { + "at": "/url" + } + } + }, + { + "name": "approve", + "method": "POST", + "path": "/v1/ops/publication/approve", + "body": { + "url": "${approvedPublicationUrl}" + }, + "headers": { + "authorization": "admin" + } + }, + { + "name": "create-to-review", + "method": "PUT", + "path": "/v1/conversations/${bucket1}/access/reviewable", + "body": { + "id": "reviewable", + "name": "reviewable", + "model": { + "id": "model_id" + }, + "prompt": "p", + "temperature": 1, + "folderId": "access", + "messages": [], + "assistantModelId": "a", + "lastActivityDate": 5 + } + }, + { + "name": "submit-for-review", + "method": "POST", + "path": "/v1/ops/publication/create", + "body": { + "name": "access matrix review", + "targetFolder": "public/review/", + "resources": [ + { + "action": "ADD", + "sourceUrl": "conversations/${bucket1}/access/reviewable", + "targetUrl": "conversations/public/review/reviewed" + } + ], + "rules": [ + { + "source": "roles", + "function": "TRUE" + } + ] + }, + "capture": { + "reviewedUrl": { + "at": "/resources/0/reviewUrl" + } + } + }, + { + "name": "create-appdata", + "method": "PUT", + "path": "/v1/files/${bucket1}/appdata/testapp/data.txt", + "body": "application scoped data", + "multipart": { + "filename": "data.txt", + "contentType": "text/plain" + } + }, + { + "name": "create-other-app-data", + "method": "PUT", + "path": "/v1/files/${bucket1}/appdata/otherapp/data.txt", + "body": "another application's data", + "multipart": { + "filename": "data.txt", + "contentType": "text/plain" + } + }, + { + "name": "second-key-creates-own", + "method": "PUT", + "path": "/v1/conversations/${bucket2}/access/second", + "headers": { + "api-key": "proxyKey2" + }, + "body": { + "id": "second", + "name": "second", + "model": { + "id": "model_id" + }, + "prompt": "p", + "temperature": 1, + "folderId": "access", + "messages": [], + "assistantModelId": "a", + "lastActivityDate": 6 + } + } + ] +} diff --git a/server/src/test/resources/layout-diff/access-matrix.json b/server/src/test/resources/layout-diff/access-matrix.json new file mode 100644 index 000000000..96898c2f6 --- /dev/null +++ b/server/src/test/resources/layout-diff/access-matrix.json @@ -0,0 +1,275 @@ +{ + "chain": [ + "getOwnResourcesAccess", + "getAdminAccess", + "getGlobalReaderAccess", + "getAutoSharedAccess", + "getPerRequestPermissions", + "getAppResourceAccess", + "getReviewAccess", + "getPublicAccess", + "getSharedAccess", + "getAppSelfAccess", + "getOwnResourcesAccessForChainedSchemaRichApplication" + ], + "uncovered": { + "getOwnResourcesAccessForChainedSchemaRichApplication": "Needs context.getDeployment() to be an Application carrying an app type schema id, and only the deployment-routing controllers ever set a deployment \u2014 ResourceController does not, so the rule returns nothing for every request this differ can make. Covering it means driving a request through DeploymentPostController against a deployed schema-rich application, which is a different fixture from anything here." + }, + "subjects": [ + { + "name": "owner", + "apiKey": "proxyKey1" + }, + { + "name": "other", + "apiKey": "proxyKey2" + }, + { + "name": "stranger", + "apiKey": "proxyKey3" + }, + { + "name": "admin", + "authorization": "admin" + }, + { + "name": "user", + "authorization": "user" + }, + { + "name": "app", + "apiKey": "proxyKey1", + "perRequest": { + "sourceDeployment": "applications/${bucket1}/access/app" + } + }, + { + "name": "appWithAttachment", + "apiKey": "proxyKey1", + "perRequest": { + "sourceDeployment": "testapp", + "attach": { + "conversations/${bucket1}/access/own": [ + "READ" + ] + } + } + }, + { + "name": "globalReader", + "authorization": "layout-diff-global-reader" + }, + { + "name": "appdataApp", + "apiKey": "proxyKey1", + "perRequest": { + "sourceDeployment": "testapp" + } + }, + { + "name": "appWithPerRequestShare", + "apiKey": "proxyKey1", + "perRequest": { + "sourceDeployment": "testapp", + "share": { + "conversations/${bucket2}/access/second": [ + "READ" + ] + } + } + } + ], + "cells": [ + { + "name": "owner-reads-own", + "subject": "owner", + "resource": "conversations/${bucket1}/access/own", + "rule": "getOwnResourcesAccess", + "expects": [ + "READ", + "WRITE" + ] + }, + { + "name": "stranger-denied-own", + "subject": "stranger", + "resource": "conversations/${bucket1}/access/own", + "rule": "getOwnResourcesAccess", + "expects": [] + }, + { + "name": "admin-writes-public", + "subject": "admin", + "resource": "conversations/public/access/published", + "rule": "getAdminAccess", + "expects": [ + "READ", + "WRITE" + ] + }, + { + "name": "admin-denied-private-resource", + "subject": "admin", + "resource": "conversations/${bucket1}/access/own", + "rule": "getAdminAccess", + "expects": [] + }, + { + "name": "plain-user-only-reads-public", + "subject": "user", + "resource": "conversations/public/access/published", + "rule": "getPublicAccess", + "expects": [ + "READ" + ] + }, + { + "name": "recipient-reads-shared", + "subject": "other", + "resource": "conversations/${bucket1}/access/shared", + "rule": "getSharedAccess", + "expects": [ + "READ" + ], + "forbids": [ + "WRITE" + ] + }, + { + "name": "stranger-denied-shared", + "subject": "stranger", + "resource": "conversations/${bucket1}/access/shared", + "rule": "getSharedAccess", + "expects": [] + }, + { + "name": "recipient-writes-write-shared", + "subject": "other", + "resource": "conversations/${bucket1}/access/writable", + "rule": "getSharedAccess", + "expects": [ + "READ", + "WRITE" + ] + }, + { + "name": "anyone-reads-published", + "subject": "other", + "resource": "conversations/public/access/published", + "rule": "getPublicAccess", + "expects": [ + "READ" + ], + "forbids": [ + "WRITE" + ] + }, + { + "name": "stranger-reads-published", + "subject": "stranger", + "resource": "conversations/public/access/published", + "rule": "getPublicAccess", + "expects": [ + "READ" + ] + }, + { + "name": "owner-reads-review-copy", + "subject": "owner", + "resource": "${reviewedUrl}", + "rule": "getReviewAccess", + "expects": [ + "READ" + ], + "forbids": [ + "WRITE" + ] + }, + { + "name": "stranger-denied-review-copy", + "subject": "stranger", + "resource": "${reviewedUrl}", + "rule": "getReviewAccess", + "expects": [] + }, + { + "name": "app-reads-attached-resource", + "subject": "appWithAttachment", + "resource": "conversations/${bucket1}/access/own", + "rule": "getAutoSharedAccess", + "expects": [ + "READ" + ] + }, + { + "name": "app-denied-unattached-resource", + "subject": "appWithAttachment", + "resource": "conversations/${bucket1}/access/shared", + "rule": "getAutoSharedAccess", + "expects": [] + }, + { + "name": "app-reads-own-configuration", + "subject": "app", + "resource": "applications/${bucket1}/access/app", + "rule": "getAppSelfAccess", + "expects": [ + "READ" + ] + }, + { + "name": "global-reader-reads-private", + "subject": "globalReader", + "resource": "conversations/${bucket1}/access/own", + "rule": "getGlobalReaderAccess", + "expects": [ + "READ" + ] + }, + { + "name": "global-reader-cannot-write", + "subject": "globalReader", + "resource": "conversations/${bucket1}/access/own", + "rule": "getGlobalReaderAccess", + "expects": [ + "READ" + ], + "forbids": [ + "WRITE" + ] + }, + { + "name": "app-reads-own-appdata", + "subject": "appdataApp", + "resource": "files/${bucket1}/appdata/testapp/data.txt", + "rule": "getAppResourceAccess", + "expects": [ + "READ", + "WRITE" + ] + }, + { + "name": "app-denied-another-apps-appdata", + "subject": "appdataApp", + "resource": "files/${bucket1}/appdata/otherapp/data.txt", + "rule": "getAppResourceAccess", + "expects": [] + }, + { + "name": "app-reads-per-request-shared", + "subject": "appWithPerRequestShare", + "resource": "conversations/${bucket2}/access/second", + "rule": "getPerRequestPermissions", + "expects": [ + "READ" + ] + }, + { + "name": "app-denied-without-per-request-share", + "subject": "appdataApp", + "resource": "conversations/${bucket2}/access/second", + "rule": "getPerRequestPermissions", + "expects": [] + } + ] +} From 962ac0fb2593d18336f726f41d882779d8d17fb7 Mon Sep 17 00:00:00 2001 From: Dmytro Zaichenko Date: Wed, 2 Sep 2026 18:49:55 +0300 Subject: [PATCH 2/2] test: make denial cells in the access matrix assert denial #1870 Empty expects meant "no expectation", so all seven denied cells only pinned cross-layout equality: both layouts granting the stranger READ would have compared equal and passed. Empty expects now asserts that the whole chain granted nothing. Co-Authored-By: Claude Fable 5 --- .../core/server/layout/LayoutAccessDifferTest.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/com/epam/aidial/core/server/layout/LayoutAccessDifferTest.java b/server/src/test/java/com/epam/aidial/core/server/layout/LayoutAccessDifferTest.java index 1063894dd..e85bcdf9f 100644 --- a/server/src/test/java/com/epam/aidial/core/server/layout/LayoutAccessDifferTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/layout/LayoutAccessDifferTest.java @@ -74,13 +74,20 @@ public void testBothLayoutsDecideIdentically() { /** * A cell that grants less than it declares is not exercising its rule. Without this the suite would - * happily compare "denied" to "denied" across every cell and report the layouts identical. + * happily compare "denied" to "denied" across every cell and report the layouts identical. The dual + * holds for denial cells: empty {@code expects} asserts that nothing is granted, not that nothing is + * expected — otherwise both layouts granting the stranger READ would compare equal and pass, and + * lookupPermissions is a union over the whole chain, so denial means no rule granted anywhere. */ private static List vacuousCells(AccessMatrix.Definition matrix, Map decisions) { List problems = new ArrayList<>(); for (AccessMatrix.Cell cell : matrix.cells()) { AccessMatrix.Decision decision = decisions.get(cell.name()); + if (cell.expects().isEmpty() && !decision.permissions().isEmpty()) { + problems.add("Cell '" + cell.name() + "' (" + cell.rule() + ") expects denial but got " + + decision.describe() + "."); + } if (!decision.permissions().containsAll(cell.expects())) { problems.add("Cell '" + cell.name() + "' (" + cell.rule() + ") expected at least " + cell.expects() + " but got " + decision.describe()