fix(AF-1099): merge denied_columns by union across a user's grants - #1100
Merged
Merged
Conversation
Contributor
Contributor
Coverage Report for Frontend Coverage (frontend)
File CoverageNo changed files found. |
Contributor
Backend Test Results10 388 tests +1 10 388 ✅ +1 11m 55s ⏱️ +22s 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. |
Contributor
Backend Code Coverage
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1099.
A user's effective
denied_columnsused 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.unionreplacesintersect, which had no other callers. Entries are deduplicated by what they match, not by spelling:users.ssnalready deniespublic.users.ssn(a schema-less entry matches that table in every schema), so the union keeps only the broaderusers.ssn. Two different schema-qualified entries (a.users.ssn,b.users.ssn) are both kept.DefaultDatasourceUserPermissionLookupService.mergeunionsdenied_columns, and its Javadoc now groups the three deny-lists as the union inversion.DeniedColumnsTestcovers the union and its deduplication.DefaultDatasourceUserPermissionLookupServiceTest: the old test that "a grant denying nothing lifts the deny" is replaced bypermissiveGroupGrantCannotLiftADirectColumnDenial, plus a test that a group's column deny binds a member whose direct grant denies nothing.denied_columns. The access simulator tests work on an already-merged view. Neither needed changes; both suites pass.docs/03-data-model.md,04-api-spec.md,05-backend.mdand07-security.md, with the asymmetry notes removed.help-corpus/is regenerated.Verification
DeniedColumnsTest,DefaultDatasourceUserPermissionLookupServiceTest,DefaultAccessSimulationServiceTest,DefaultEffectiveAccessServiceTest,EffectiveAccessEnforcementParityTest,DatasourcePermissionVerifierTest,SchemaViewPermissionFilterTest,AccessGrantMaterializerTest,ApplicationModulesTestandApiPackageDependencyTestall pass.websitePagesandwebsiteDocspass (53 tests).mvn verify, which is left to CI.