Skip to content

User permissions - #3908

Closed
ssddanbrown wants to merge 24 commits into
developmentfrom
user_permissions
Closed

User permissions#3908
ssddanbrown wants to merge 24 commits into
developmentfrom
user_permissions

Conversation

@ssddanbrown

@ssddanbrown ssddanbrown commented Dec 7, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

As start of user permissions work. Related to #1747.

Todo

  • Update entity permissions table
  • Update "Other users" permission handling to be denoted by entity permissions with user_id = null AND role_id = null instead of role_id = 0.
  • Update joint_permissions table to support user_id?
  • Update joint_permission handling.
    • Fix misalignment between joint and non-joint handling, Currently seen with 2-role user, granted view on parent book on Role A, prevented view on chapter on Role B.
      • Check scenario against release.
        • Release has this same scenario. Works if inherit permissions is inactive, otherwise chapter (and content) is visible but not accessible, even if both roles are set to not allow view while inherit is active.
    • Chapter permission fallback "Other users" option not taking account.
    • Check scenario of "Role A" granting permission via role permissions to sub-item that has blocked "Role B" permission at entity-level.
      • Check on release - Can view (query), can't access (userCan).
      • Check view via joint permission behaviour
      • Check edit via userCan behaviour.
  • Add user interface for user permissions.
  • Delete entity permissions and joint permissions on user delete.
  • Wrap gen/delete in transactions to avoid visibility change during generation.
  • Check for todos in changes.
  • Testing.
    • Create dev-doc for permission scenario testing, with test case IDs that link to PHPUnit test cases.
    • Test admin system role retains full item-level permission.
    • Manually & thoroughly scenario test permission application.
    • Ensure view (joint permission) logic aligns.
    • Check prevention (Lack of permission) aligns with last release, and pre-v22.10 release (Where applicable), in that role permissions will prevent access where defined (Unless another role for the user specifically allows).
    • Check copying of shelf permissions.
    • Check copying of items (If permissions come across.)
  • Extract text to language files.

Docs update

  • Update permissions user doc page.
    • Provide an "advanced" overview of deeper combination logic
  • Add upgrade notice, linking to above added section for more details.

@ssddanbrown
Updated entity_permissions table for user perms.
1c53ffc 
As start of user permissions work
@ssddanbrown ssddanbrown self-assigned this Dec 7, 2022
@ssddanbrown ssddanbrown added this to the Next Feature Release milestone Dec 7, 2022
@ssddanbrown
Aligned logic to entity_permission role_id usage change
f8c4725 
Now idenitifies fallback using role_id and user_id = null.
Lays some foundations for handling user_id.
@ssddanbrown
Added users to permission form interface
7a269e7 
Also updated non-joint permission handling to support user permissions.
@ssddanbrown
Improved user-permissions adding ux
93cbd3b 
- Reset input after user selection.
- Corrected permission row title text for user rows.
@ssddanbrown
Added, and built perm. gen for, joint_user_permissions table
0411185
@ssddanbrown
Added joint_user_permissions handling to query system
60bf838 
Some issues exist to resolve though, not in final state.
@ssddanbrown
Started aligning permission behaviour across application methods
e8a8fed
@ssddanbrown
Started more formal permission test case definitions
d54ea1b
@ssddanbrown
Create additional test helper classes
f844ae0 
Following recent similar actions done for entities.
Required at this stage to provider better & cleaner helpers
for common user and permission actions to built out permission testing.
@ssddanbrown
Added additional entity_role_permission scenario tests
491beee
@ssddanbrown
Added a bunch of role content permissions
6a6f5e4
@ssddanbrown
Added entity user permission scenarios
5ffc10e 
Also added definitions for general expected behaviour to readme doc, and
added some entity role inherit scenarios to check they meet expectations.
Currently failing role test but not an issue with test, needs fixing to
app logic.
@ssddanbrown
Added more inter-method permissions test cases
adabf06
@ssddanbrown
Added additional case thats known to currently fail
2d9d2bb 
Also removed so no-longer-relevant todo/comments.
@ssddanbrown
Started new permission-caching/querying model
39acbea
@ssddanbrown
Created big scary query to apply permissions via new format
7330139
@ssddanbrown
Fixed collapsed perm. gen for book sub-items.
451e4ac 
Also converted the existing "JointPermission" usage to the new
collapsed permission system.
@ssddanbrown
Reworked userCan permission check to follow defined logic.
026e903 
Got all current scenario tests passing.
Also fixes own permission which was using the wrong field.
@ssddanbrown ssddanbrown mentioned this pull request Jan 4, 2023
Closed
@ssddanbrown
Split out 'restrictEntityQuery' function components
2f1491c 
Also fixed search query issue with abiguous column
@ssddanbrown
Got entity relation query permission application working
932e1d7 
May be issues at points of use though, Added todo for this in code.
Also added extra indexes to collapsed table for better query
performance.
@ssddanbrown
Updated additional relation queries to apply permissions correctly
a825f27
@ssddanbrown
Aligned admin permission check restriction ignore
93ba572
@ssddanbrown
Attempted fix of issues, realised new query system is a failure
55642a3 
As part of the permission checking we need to check owner user status.
Upon this, we'd also want to check page draft status (and its
creator/owner).
These, for cross-entity/relation queries would need up to another 4 joins.
The performance/index usage is already questionable here.
@ssddanbrown

ssddanbrown commented Jan 14, 2023

Copy link
Copy Markdown
Member Author

Note of approach failure

Written at 55642a3

As part of this work I tried to bring the "cached" permissions, stored in the database, up a level so they're not per-role, per-item.
This was a entity_permissions_collapsed that would store the collapsed/flattened/inherited role & user permission overrides only where item-level permission overrides were active. Permissions were then joined into queries so the fallback, role & user status would be worked out at query time.

As part of the permission checking we need to check owner user status.
Upon this, we'd also want to check page draft status (and its
creator/owner).
These, for cross-entity/relation queries would need up to another 4 joins (Totalling 7)
The performance/index usage was already questionable here.

This approach now looks like the wrong path, since we're implementing large changes, adding significant risk, without proof of overall benefit. The improved cache-time performance and reduced joint permission table would be risking query-time performance, which may not work out long-term.
Think we should hold back on on changing the permission system in this way until we can zoom out and look at more fundamental potential changes to support (Aligning entities to single table).

For now, might need to look to tack-on user permissions to existing system, but we should be able to keep changes in non-query work.

@ssddanbrown
Added method for using enity ownership in relation queries
3083979 
It has a large linear-entity-scaling performance impact though.
@ssddanbrown ssddanbrown mentioned this pull request Jan 21, 2023
Merged
@ssddanbrown

ssddanbrown commented Jan 24, 2023

Copy link
Copy Markdown
Member Author

Superseded by #3986, although in a different scope (No user permissions, mainly testing and alignment work)

@ssddanbrown ssddanbrown removed this from the v23.01 milestone Jan 29, 2023
@ssddanbrown ssddanbrown mentioned this pull request May 23, 2025
Closed
@ssddanbrown
ssddanbrown deleted the user_permissions branch June 11, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@ssddanbrown ssddanbrown

Labels

📖 Docs Update 🏭 Back-End 🛠️ Enhancement

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ssddanbrown