feat: granular capability model for Data feature permissions (PR #346 rebased on develop) - #413
Closed
esafwan wants to merge 5 commits into
Closed
feat: granular capability model for Data feature permissions (PR #346 rebased on develop)#413esafwan wants to merge 5 commits into
esafwan wants to merge 5 commits into
Conversation
The Data feature had zero capability enforcement server-side, and every dynamically-generated data-table DocType hardcoded its Frappe permission row to System Manager only - meaning non-admins got a hard PermissionError on any record, in any table, despite the sidebar implying broader access via a misused agent.view_all capability. Adds 6 new capabilities (data.tables.manage, data.records.create, data.records.view_own/view_all, data.records.edit_own/edit_all), configurable per Huf Role via the existing Roles system. Enforcement: - Schema management (create/update/delete table) now requires data.tables.manage, checked server-side in huf_data_table/api.py. - Record-level access uses Frappe's native if_owner permission mechanism instead of hand-rolled filtering: sync_data_table_permissions() rebuilds each generated DocType's permission rows from every Huf Role's current data.* grants, merging view/edit/create into the correct (role, if_owner) row pairs while always preserving System Manager's full-access row. This runs on table creation and whenever a Huf Role is saved, so permission changes take effect immediately. - A migration patch backfills permissions onto tables created before this change; it's a no-op on fresh installs with no data tables yet. Frontend: sidebar Data nav item now checks any of the 3 view/manage capabilities instead of the mismatched agent.view_all; DataTableBuilderPage shows an access-denied state for users without data.tables.manage; RolesPage groups the new capabilities under a "Data" category. Also fixes the Huf Role Permission doctype's capability Select field, which had a static options list that rejected the new values before custom Python validation even ran (caught by running a real bench migrate against the site, not just static analysis).
…ame, add view checks
Found in a final cross-branch security/UX review:
- apply_data_permissions() was whitelisted with no capability check,
letting any authenticated user trigger a full permission rebuild
across every data table.
- table_name was only trimmed before being interpolated into the
generated DocType name (f"HF {table_name}"), with no character
restriction.
- get_table_record_counts/get_table_schema had no capability check at
all, letting any user enumerate table schemas and record counts.
Adds a _require_data_manage()/_require_data_view() guard to each,
and a conservative identifier regex on table_name.
… options, migration patch, RolesPage Restores the pieces dropped by the over-narrow task scope: HufRole.on_update syncs data-table permissions, huf_role_permission options gain the data.* capabilities, patches.txt registers sync_data_table_permissions (last line, per intake dossier), and the RolesPage frontend change.
…p request-scoped commit
Contributor
Author
|
Superseded by consolidated Data permissions PR #478: feat(data): consolidate granular table permissions and capability gating. |
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
1 participant
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.
Problem
Repairs #346: the Data feature (dynamic data tables) had no capability gating — complements #409's route-level gating (finding 1.6, which #340 explicitly deferred). The original PR was CONFLICTING (sidebar restructure +
patches.txtEOF). This is the same change rebased onto current develop with both conflicts resolved.Scope
huf/permissions.py: 6 newdata.*capabilities in the catalogue.huf/huf/doctype/huf_data_table/permissions.py(new): capability→DocType-permission sync;api.py: guarded endpoints (validate table_name, view checks).huf/huf/doctype/huf_role/huf_role.py:on_updatere-syncs data-table permissions (guarded, logs on failure).huf_role_permission.json: options gain thedata.*capabilities (needsbench migrate).patches.txt: registerssync_data_table_permissionsbackfill (last line, per intake dossier).data.tables.manage/data.records.*), DataTableBuilderPage guard, RolesPage.Key decisions
patches.txtconflict resolved keeping both develop'sfix_agent_message_statusand the new patch (PR's last, per dossier).Agent contributions
Rebase + sidebar conflict resolution by the release-orchestration pipeline; a task-scope packaging error accidentally dropped 4 files — restored by the pipeline (huf_role hook, options JSON, patches, RolesPage). Intake dossier by the pipeline.
Tests executed
py_compileon changed Python; JSON validated;tsc+vite build green.bench migrateclean, patch ran (Patch Log verified),data.*capabilities present in the catalogue.Risks and limitations
sync_data_table_permissions()rewrites the permissions child table of every HF* DocType; manual customizations to those are overwritten (by design; noted in the original PR).agent.view_allbut hold nodata.*capability lose the menu item (intended gating).Rollback
Revert the branch; re-run migrate (perm rows revert on DocType sync).
Remaining work
data.records.view_own-only user.