Skip to content

fix(AF-1089): match table preview allow-list to the query gate #1090

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
babltiga merged 2 commits into from
Sep 24, 2026
Merged

fix(AF-1089): match table preview allow-list to the query gate #1090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc64d77
fix(AF-1089): match table preview allow-list to the query gate
babltiga Sep 24, 2026
22e861a
fix(AF-1089): count bare-entry twins over the unfiltered catalog
babltiga Sep 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 5 additions & 36 deletions backend/src/main/java/com/bablsoft/accessflow/proxy/internal/DefaultQueryDryRunService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.bablsoft.accessflow.proxy.internal;

import com.bablsoft.accessflow.core.api.AllowedTables;
import com.bablsoft.accessflow.core.api.DatasourceAdminService;
import com.bablsoft.accessflow.core.api.DatasourceUserPermissionLookupService;
import com.bablsoft.accessflow.core.api.DatasourceUserPermissionView;
Expand All @@ -21,9 +22,7 @@
import org.springframework.security.access.AccessDeniedException;
import org.springframework.stereotype.Service;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import java.util.TreeSet;
import java.util.UUID;
Expand Down Expand Up @@ -106,8 +105,8 @@ private void verifyPermission(UUID userId, UUID datasourceId, SqlParseResult par

private void verifyAllowedTables(DatasourceUserPermissionView permission, UUID datasourceId,
Set<String> referencedTables) {
var allowedSchemas = normalizeList(permission.allowedSchemas());
var allowedTables = normalizeList(permission.allowedTables());
var allowedSchemas = AllowedTables.normalize(permission.allowedSchemas());
var allowedTables = AllowedTables.normalize(permission.allowedTables());
if (allowedSchemas.isEmpty() && allowedTables.isEmpty()) {
return;
}
Expand All @@ -116,14 +115,9 @@ private void verifyAllowedTables(DatasourceUserPermissionView permission, UUID d
}
var rejected = new TreeSet<String>();
for (String table : referencedTables) {
if (allowedTables.contains(table)) {
continue;
if (AllowedTables.coveringEntry(allowedSchemas, allowedTables, table) == null) {
rejected.add(table);
}
int dotIdx = table.indexOf('.');
if (dotIdx > 0 && allowedSchemas.contains(table.substring(0, dotIdx))) {
continue;
}
rejected.add(table);
}
if (!rejected.isEmpty()) {
log.warn("Dry-run allow-list rejection on datasource {} for user {}: tables {}",
Expand All @@ -142,31 +136,6 @@ private static boolean hasCapability(DatasourceUserPermissionView permission, Qu
};
}

private static List<String> normalizeList(List<String> raw) {
if (raw == null || raw.isEmpty()) {
return List.of();
}
var out = new ArrayList<String>(raw.size());
for (String entry : raw) {
if (entry == null) {
continue;
}
var stripped = new StringBuilder(entry.length());
for (int i = 0; i < entry.length(); i++) {
char c = entry.charAt(i);
if (c == '"' || c == '`' || c == '[' || c == ']') {
continue;
}
stripped.append(c);
}
var normalized = stripped.toString().trim().toLowerCase(Locale.ROOT);
if (!normalized.isEmpty()) {
out.add(normalized);
}
}
return List.copyOf(out);
}

private String msg(String key, Object[] args) {
return messageSource.getMessage(key, args, LocaleContextHolder.getLocale());
}
Expand Down
83 changes: 39 additions & 44 deletions backend/src/main/java/com/bablsoft/accessflow/proxy/internal/DefaultSampleDataService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.bablsoft.accessflow.proxy.internal;

import com.bablsoft.accessflow.core.api.AllowedTables;
import com.bablsoft.accessflow.core.api.ColumnMaskDirective;
import com.bablsoft.accessflow.core.api.DatabaseSchemaView;
import com.bablsoft.accessflow.core.api.DatasourceAdminService;
Expand All @@ -21,12 +22,12 @@
import org.springframework.security.access.AccessDeniedException;
import org.springframework.stereotype.Service;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.function.Supplier;

@Service
@RequiredArgsConstructor
Expand Down Expand Up @@ -55,7 +56,9 @@ public SelectExecutionResult sample(UUID datasourceId, UUID organizationId, UUID
if (!isAdmin) {
// Non-admins additionally need read capability + the target inside their allow-list.
var view = permission.orElseThrow(() -> new TableNotFoundException(datasourceId, table));
if (!view.canRead() || !targetAllowed(view, target)) {
if (!view.canRead() || !targetAllowed(view, target,
() -> datasourceAdminService.introspectSchemaForSystem(datasourceId,
organizationId))) {
throw new TableNotFoundException(datasourceId, table);
}
// The preview reads every column, so a denied column on the table refuses it (#935).
Expand Down Expand Up @@ -123,59 +126,51 @@ private Optional<Target> resolveTarget(DatabaseSchemaView view, String schema, S
}

/**
* Mirrors the allow-list semantics of {@code DefaultQuerySubmissionService.verifyAllowedTables}:
* empty lists allow everything; otherwise the table (bare or {@code schema.table}) must be in
* {@code allowedTables}, or its schema in {@code allowedSchemas}.
* Matches through {@link AllowedTables#coveringEntry}, the query gate's matcher: the qualified
* target is covered by its own {@code schema.table} entry or by its schema. A bare
* {@code allowed_tables} entry covers only an unqualified reference in the gate — whatever table
* the database resolves that name to — which a preview of a concrete {@code schema.table} cannot
* know. It admits the target only when no other schema in the database has a table of that name
* (the fail-closed rule {@code SchemaViewPermissionFilter} applies to the view, #936), counted
* over the unfiltered catalog, fetched only for this fallback, since the caller's filtered view
* may already hide the other table. A target without a schema is covered by a bare entry only.
*/
private static boolean targetAllowed(DatasourceUserPermissionView permission, Target target) {
var allowedSchemas = normalizeList(permission.allowedSchemas());
var allowedTables = normalizeList(permission.allowedTables());
private static boolean targetAllowed(DatasourceUserPermissionView permission, Target target,
Supplier<DatabaseSchemaView> fullCatalog) {
var allowedSchemas = AllowedTables.normalize(permission.allowedSchemas());
var allowedTables = AllowedTables.normalize(permission.allowedTables());
if (allowedSchemas.isEmpty() && allowedTables.isEmpty()) {
return true;
}
var bare = normalize(target.table());
var qualified = target.schema() == null || target.schema().isBlank()
? bare
: normalize(target.schema()) + "." + bare;
if (allowedTables.contains(bare) || allowedTables.contains(qualified)) {
var bare = AllowedTables.normalizeEntry(target.table());
if (bare == null) {
return false;
}
var schema = AllowedTables.normalizeEntry(target.schema());
if (schema == null) {
return allowedTables.contains(bare);
}
if (AllowedTables.coveringEntry(allowedSchemas, allowedTables, schema + "." + bare) != null) {
return true;
}
return target.schema() != null && !target.schema().isBlank()
&& allowedSchemas.contains(normalize(target.schema()));
return allowedTables.contains(bare) && tablesNamed(fullCatalog.get(), bare) == 1;
}

private static List<String> normalizeList(List<String> raw) {
if (raw == null || raw.isEmpty()) {
return List.of();
}
var out = new ArrayList<String>(raw.size());
for (String entry : raw) {
if (entry == null) {
continue;
}
var normalized = normalize(entry);
if (!normalized.isEmpty()) {
out.add(normalized);
private static int tablesNamed(DatabaseSchemaView view, String bare) {
var count = 0;
for (var ns : view.schemas()) {
for (var t : ns.tables()) {
if (bare.equals(AllowedTables.normalizeEntry(t.name()))) {
count++;
}
}
}
return List.copyOf(out);
return count;
}

private static String qualifiedName(Target target) {
return target.schema() == null || target.schema().isBlank()
? normalize(target.table())
: normalize(target.schema()) + "." + normalize(target.table());
}

private static String normalize(String raw) {
var stripped = new StringBuilder(raw.length());
for (int i = 0; i < raw.length(); i++) {
char c = raw.charAt(i);
if (c == '"' || c == '`' || c == '[' || c == ']') {
continue;
}
stripped.append(c);
}
return stripped.toString().trim().toLowerCase(Locale.ROOT);
var table = Objects.requireNonNullElse(AllowedTables.normalizeEntry(target.table()), "");
var schema = AllowedTables.normalizeEntry(target.schema());
return schema == null ? table : schema + "." + table;
}
}
34 changes: 34 additions & 0 deletions ...d/src/test/java/com/bablsoft/accessflow/proxy/internal/DefaultQueryDryRunServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,40 @@ void nonAdminWithNoPermissionIsDenied() {
.isInstanceOf(AccessDeniedException.class);
}

@Test
void nonAdminReferencingTablesCoveredBySchemaOrQualifiedEntryPasses() {
when(datasourceAdminService.getForUser(datasourceId, orgId, userId)).thenReturn(view());
when(queryParser.parse(anyString(), any()))
.thenReturn(parse(QueryType.SELECT, Set.of("sales.orders", "public.users")));
when(permissionLookupService.findFor(userId, datasourceId))
.thenReturn(java.util.Optional.of(permission(true, List.of("Sales"),
List.of("\"PUBLIC\".\"USERS\""))));
when(rowSecurityResolutionService.resolveApplicable(orgId, datasourceId, userId))
.thenReturn(List.of());
var expected = QueryDryRunResult.of("postgresql", QueryType.SELECT, 1L, null, null,
Set.of(), Duration.ZERO);
when(queryExecutor.dryRun(any())).thenReturn(expected);

assertThat(service.dryRun(datasourceId, "SELECT 1", userId, orgId, false))
.isSameAs(expected);
}

@Test
void nonAdminWithAllowListAndNoReferencedTablesPasses() {
when(datasourceAdminService.getForUser(datasourceId, orgId, userId)).thenReturn(view());
when(queryParser.parse(anyString(), any())).thenReturn(parse(QueryType.SELECT, Set.of()));
when(permissionLookupService.findFor(userId, datasourceId))
.thenReturn(java.util.Optional.of(permission(true, List.of(), List.of("orders"))));
when(rowSecurityResolutionService.resolveApplicable(orgId, datasourceId, userId))
.thenReturn(List.of());
var expected = QueryDryRunResult.of("postgresql", QueryType.SELECT, 1L, null, null,
Set.of(), Duration.ZERO);
when(queryExecutor.dryRun(any())).thenReturn(expected);

assertThat(service.dryRun(datasourceId, "SELECT 1", userId, orgId, false))
.isSameAs(expected);
}

@Test
void nonAdminReferencingDisallowedTableIsDenied() {
when(datasourceAdminService.getForUser(datasourceId, orgId, userId)).thenReturn(view());
Expand Down
131 changes: 131 additions & 0 deletions ...nd/src/test/java/com/bablsoft/accessflow/proxy/internal/DefaultSampleDataServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,137 @@ void rowLimitPolicyOnTheSampledTableCapsThePreview() {
assertThat(captor.getValue().maxRowsOverride()).isEqualTo(7);
}

@Test
void bareEntryDoesNotAdmitASchemaQualifiedTargetWhenTheNameIsAmbiguous() {
// Defence in depth: even a view that still lists both tables must not admit either.
stubSchemas(schema("public", "orders"), schema("archive", "orders"));
stubCatalog(schema("public", "orders"), schema("archive", "orders"));
when(permissionLookupService.findFor(userId, datasourceId))
.thenReturn(Optional.of(permission(true, List.of(), List.of(), List.of("orders"))));

assertThatThrownBy(() -> service.sample(datasourceId, organizationId, userId, false,
"archive", "orders", 50))
.isInstanceOf(TableNotFoundException.class);
assertThatThrownBy(() -> service.sample(datasourceId, organizationId, userId, false,
"public", "orders", 50))
.isInstanceOf(TableNotFoundException.class);
verify(queryExecutor, never()).sampleTable(any());
}

@Test
void bareEntryAdmitsTheOnlyTableOfThatName() {
stubSchemas(schema("archive", "orders"));
stubCatalog(schema("archive", "orders"), schema("public", "customers"));
when(permissionLookupService.findFor(userId, datasourceId))
.thenReturn(Optional.of(permission(true, List.of(), List.of(), List.of("orders"))));

assertThat(service.sample(datasourceId, organizationId, userId, false, "archive", "orders",
50)).isSameAs(result);
}

@Test
void bareEntryCountsTheUnfilteredCatalogNotTheCallersView() {
// The filtered view shows archive.orders through a catalog-qualified grant and hides
// public.orders; the bare entry must still see the name is ambiguous in the database.
stubSchemas(schema("archive", "orders"));
stubCatalog(schema("public", "orders"), schema("archive", "orders"));
when(permissionLookupService.findFor(userId, datasourceId))
.thenReturn(Optional.of(permission(true, List.of(), List.of(),
List.of("orders", "cat.archive.orders"))));

assertThatThrownBy(() -> service.sample(datasourceId, organizationId, userId, false,
"archive", "orders", 50))
.isInstanceOf(TableNotFoundException.class);
verify(queryExecutor, never()).sampleTable(any());
}

@Test
void coveredTargetNeverIntrospectsTheUnfilteredCatalog() {
when(permissionLookupService.findFor(userId, datasourceId))
.thenReturn(Optional.of(permission(true, List.of(), List.of("public"), List.of())));

service.sample(datasourceId, organizationId, userId, false, "public", "users", 50);

verify(datasourceAdminService, never()).introspectSchemaForSystem(any(), any());
}

@Test
void qualifiedEntryDoesNotCoverTheSameNameInAnotherSchema() {
stubSchemas(schema("public", "orders"), schema("archive", "orders"));
when(permissionLookupService.findFor(userId, datasourceId))
.thenReturn(Optional.of(permission(true, List.of(), List.of(),
List.of("public.orders"))));

assertThatThrownBy(() -> service.sample(datasourceId, organizationId, userId, false,
"archive", "orders", 50))
.isInstanceOf(TableNotFoundException.class);
assertThat(service.sample(datasourceId, organizationId, userId, false, "public", "orders",
50)).isSameAs(result);
}

@Test
void allowedSchemaCoversAnAmbiguousNameInsideIt() {
stubSchemas(schema("public", "orders"), schema("archive", "orders"));
when(permissionLookupService.findFor(userId, datasourceId))
.thenReturn(Optional.of(permission(true, List.of(), List.of("archive"), List.of())));

assertThat(service.sample(datasourceId, organizationId, userId, false, "archive", "orders",
50)).isSameAs(result);
assertThatThrownBy(() -> service.sample(datasourceId, organizationId, userId, false,
"public", "orders", 50))
.isInstanceOf(TableNotFoundException.class);
}

@Test
void quotedMixedCaseEntriesAreNormalizedLikeTheQueryGate() {
when(permissionLookupService.findFor(userId, datasourceId))
.thenReturn(Optional.of(permission(true, List.of(), List.of(),
List.of(" \"PUBLIC\".[Users] ", ""))));

assertThat(service.sample(datasourceId, organizationId, userId, false, "public", "users",
50)).isSameAs(result);
}

@Test
void unnamedSchemaTargetIsCoveredOnlyByABareEntry() {
stubSchemas(schema("", "orders"));
when(permissionLookupService.findFor(userId, datasourceId))
.thenReturn(Optional.of(permission(true, List.of(), List.of(), List.of("orders"))));

assertThat(service.sample(datasourceId, organizationId, userId, false, null, "orders", 50))
.isSameAs(result);
var captor = ArgumentCaptor.forClass(SampleTableRequest.class);
verify(queryExecutor).sampleTable(captor.capture());
assertThat(captor.getValue().table()).isEqualTo("orders");
}

@Test
void unnamedSchemaTargetIsRefusedWithOnlyAQualifiedEntry() {
stubSchemas(schema("", "orders"));
when(permissionLookupService.findFor(userId, datasourceId))
.thenReturn(Optional.of(permission(true, List.of(), List.of("public"),
List.of("public.orders"))));

assertThatThrownBy(() -> service.sample(datasourceId, organizationId, userId, false, null,
"orders", 50))
.isInstanceOf(TableNotFoundException.class);
}

private void stubCatalog(DatabaseSchemaView.Schema... schemas) {
when(datasourceAdminService.introspectSchemaForSystem(datasourceId, organizationId))
.thenReturn(new DatabaseSchemaView(List.of(schemas)));
}

private void stubSchemas(DatabaseSchemaView.Schema... schemas) {
when(datasourceAdminService.introspectSchema(eq(datasourceId), eq(organizationId),
eq(userId), anyBoolean())).thenReturn(new DatabaseSchemaView(List.of(schemas)));
}

private static DatabaseSchemaView.Schema schema(String name, String table) {
return new DatabaseSchemaView.Schema(name, List.of(new DatabaseSchemaView.Table(table,
List.of(new DatabaseSchemaView.Column("id", "uuid", false, true)), List.of())));
}

private DatasourceUserPermissionView permission(boolean canRead, List<String> restrictedColumns,
List<String> allowedSchemas,
List<String> allowedTables) {
Expand Down
Loading