Skip to content

fix(AF-1099): merge denied_columns by union across a user's grants - #1100

Merged
babltiga merged 1 commit into
mainfrom
fix/AF-1099-denied-columns-union-merge
Sep 25, 2026
Merged

babltiga merged 1 commit into
mainfrom
fix/AF-1099-denied-columns-union-merge

Conversation

@babltiga

Copy link
Copy Markdown
Contributor

Closes #1099.

⚠️ Behaviour change for existing grants

A user's effective denied_columns used to be the intersection of their grants: a column stayed denied only while every direct, group and JIT grant on the datasource denied it. It is now the union: a column denied by any grant stays denied.

After upgrading, a user who holds several grants on a datasource where only some deny a column will be refused queries that reach that column. Previously a grant that denied nothing (typically a group grant) silently lifted the deny. Admins (QUERY_ADMIN) still bypass the per-datasource gate as before. Operators who relied on a permissive group grant to lift a direct grant's column deny must now remove the deny from the grant that carries it. Revoking or expiring that grant removes it too, which was already the case.

This matches the merge that #939 introduced for denied_schemas / denied_tables, and removes the documented "asymmetry".

Changes

  • core.api.DeniedColumns.union replaces intersect, which had no other callers. Entries are deduplicated by what they match, not by spelling: users.ssn already denies public.users.ssn (a schema-less entry matches that table in every schema), so the union keeps only the broader users.ssn. Two different schema-qualified entries (a.users.ssn, b.users.ssn) are both kept.
  • DefaultDatasourceUserPermissionLookupService.merge unions denied_columns, and its Javadoc now groups the three deny-lists as the union inversion.
  • Tests:
    • DeniedColumnsTest covers the union and its deduplication.
    • DefaultDatasourceUserPermissionLookupServiceTest: the old test that "a grant denying nothing lifts the deny" is replaced by permissiveGroupGrantCannotLiftADirectColumnDenial, plus a test that a group's column deny binds a member whose direct grant denies nothing.
  • The effective-access reverse index is table-level and doesn't read denied_columns. The access simulator tests work on an already-merged view. Neither needed changes; both suites pass.
  • Docs updated in docs/03-data-model.md, 04-api-spec.md, 05-backend.md and 07-security.md, with the asymmetry notes removed.
  • Website: the datasources and users-roles config pages are updated; their dates were already today. help-corpus/ is regenerated.

Verification

  • DeniedColumnsTest, DefaultDatasourceUserPermissionLookupServiceTest, DefaultAccessSimulationServiceTest, DefaultEffectiveAccessServiceTest, EffectiveAccessEnforcementParityTest, DatasourcePermissionVerifierTest, SchemaViewPermissionFilterTest, AccessGrantMaterializerTest, ApplicationModulesTest and ApiPackageDependencyTest all pass.
  • The frontend tests websitePages and websiteDocs pass (53 tests).
  • Not run locally: the full mvn verify, which is left to CI.

@github-actions

Copy link
Copy Markdown
Contributor

Frontend Test Results

    1 files  ±0    311 suites  ±0   9m 3s ⏱️ - 3m 21s
2 658 tests ±0  2 658 ✅ ±0  0 💤 ±0  0 ❌ ±0 
2 659 runs  ±0  2 659 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 008af06. ± Comparison against base commit 9fc3865.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for Frontend Coverage (frontend)

Status Category Percentage Covered / Total
🟢 Lines 96.05% (🎯 90%) 3672 / 3823
🟢 Statements 94.68% (🎯 90%) 4080 / 4309
🟢 Functions 94.05% (🎯 90%) 1124 / 1195
🟢 Branches 87.7% (🎯 80%) 2439 / 2781
File CoverageNo changed files found.
Generated in workflow #1414 for commit 008af06 by the Vitest Coverage Report Action

@github-actions

Copy link
Copy Markdown
Contributor

Backend Test Results

10 388 tests  +1   10 388 ✅ +1   11m 55s ⏱️ +22s
 1 147 suites ±0        0 💤 ±0 
 1 147 files   ±0        0 ❌ ±0 

Results for commit 008af06. ± Comparison against base commit 9fc3865.

This pull request removes 3 and adds 4 tests. Note that renamed tests count towards both.
com.bablsoft.accessflow.core.api.DeniedColumnsTest ‑ intersectMeetsEntriesByTheColumnTheyName
com.bablsoft.accessflow.core.internal.DefaultDatasourceUserPermissionLookupServiceTest ‑ findForDeniesNothingWhenOneGrantDeniesNothing
com.bablsoft.accessflow.core.internal.DefaultDatasourceUserPermissionLookupServiceTest ‑ findForIntersectsDeniedColumnsCaseInsensitively
com.bablsoft.accessflow.core.api.DeniedColumnsTest ‑ unionKeepsEveryDeniedColumnAndCollapsesOverlappingSpellings
com.bablsoft.accessflow.core.internal.DefaultDatasourceUserPermissionLookupServiceTest ‑ findForUnionsDeniedColumnsCaseInsensitively
com.bablsoft.accessflow.core.internal.DefaultDatasourceUserPermissionLookupServiceTest ‑ groupColumnDenialBindsAMemberWhoseDirectGrantDeniesNothing
com.bablsoft.accessflow.core.internal.DefaultDatasourceUserPermissionLookupServiceTest ‑ permissiveGroupGrantCannotLiftADirectColumnDenial

@github-actions

Copy link
Copy Markdown
Contributor

Backend Code Coverage

Overall Project 94.77% 🍏
Files changed 100% 🍏

File Coverage
DeniedColumns.java 99.8% 🍏
DefaultDatasourceUserPermissionLookupService.java 96.92% 🍏

@babltiga
babltiga merged commit cf06398 into main Sep 25, 2026
35 checks passed
@babltiga
babltiga deleted the fix/AF-1099-denied-columns-union-merge branch September 25, 2026 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

core: merge denied_columns by union across a user's grants

1 participant