-
Notifications
You must be signed in to change notification settings - Fork 25
[feat] Role and Permission system #2737
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.
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
base: develop
Are you sure you want to change the base?
Changes from all commits
6370404
16da990
4e1f9ff
b5d248b
f06ff6d
bd0946f
3ec2868
70ae8b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -934,6 +934,77 @@ CREATE INDEX forum_post_parent_idx ON forum_post (parent_post_id); | |
| CREATE INDEX forum_post_latest_revision_idx ON forum_post (latest_revision_id); | ||
| CREATE INDEX forum_post_revision_lookup_idx ON forum_post_revision (forum_post_id, revision_number DESC); | ||
|
|
||
| -- | ||
| -- Role / permission system | ||
| -- | ||
|
|
||
| -- Lookup table for permissions. This is shared across all sites. | ||
| CREATE TABLE permission ( | ||
| permission_id BIGSERIAL PRIMARY KEY, | ||
| description TEXT NOT NULL, | ||
| resource_type TEXT NOT NULL, | ||
| action TEXT NOT NULL, | ||
|
|
||
| UNIQUE (resource_type, action) | ||
| ); | ||
|
|
||
| -- Roles in a site. | ||
| CREATE TABLE role ( | ||
| role_id BIGSERIAL PRIMARY KEY, | ||
|
|
||
| -- Denotes a unique role in a site. A user may be an admin in one site but a regular site member in another. | ||
| site_id BIGINT NOT NULL REFERENCES site(site_id), | ||
| name TEXT NOT NULL, | ||
| description TEXT NOT NULL, | ||
| from_wikidot BOOLEAN NOT NULL DEFAULT false, | ||
|
|
||
| -- Virtual roles are invisible to the user, granted by the system under specific conditions | ||
| -- i.e. Guest role granted when a non-member visits the site, | ||
| -- or Author role granted when a user interacts with a page they authored. | ||
| -- Virtual roles are visible to admins where they can select the role's permissions. | ||
| -- Virtual roles cannot be manually assigned. | ||
| is_virtual BOOLEAN NOT NULL DEFAULT false, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Design question: Would it be better for virtual roles to be in the database (and automatically added by jobs or code logic)? This would mean we can just query roles and get all of them. Or should virtual roles be added automatically in the future I'm leaning more towards the latter but I'm curious as to your thoughts.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you talking about a
In my latest commit I've actually implemented that logic in |
||
|
|
||
| -- System roles cannot be deleted (i.e. Admin) | ||
| is_system BOOLEAN NOT NULL DEFAULT false, | ||
loop-index marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| -- Rudimentary role hierarchy. | ||
| -- Roles with higher level are granted more permissions than roles with lower levels. | ||
| -- Users with role management permissions can only grant roles with lower levels and affect users of lower levels. | ||
| level INTEGER NOT NULL CHECK (level >= 0), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This system would certainly work, though I am wondering if there are hierarchical systems that would be better than simple numerical precedence, as this can cause some issues in Discord (which you have noted is an understandable inspiration for this work). For instance, it could be a tree. There is one role that is the "root" that can manage any other role (or maybe multiple roots?), its children which can manage any roles under them, their children, etc. This way there isn't a strict total ranking, allowing some roles to be "equal" to each other because they manage different domains. This solution also wouldn't be too complicated to represent in the database, just have a
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, I also thought about several options, hierarchy tree (basically an org chart) included. My only concern would be implementation complexity in doing lookups up/down the tree, because we are essentially doing a new query for each up/down level we traverse. Also would there be any possible cases where we might need more than one org root? That might require a dummy root node.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that lookups up and down the tree are non-ideal, but considering tree depths aren't expected to be extreme I am leaning towards saying the cost is acceptable. (We can also set up caching in redis etc for permissions too) I cannot think of real reasons why more than one root would make sense, I mostly brought it up for completeness. Maybe a separation-of-powers thing in a site? But that could still have a dummy root note in effective implementation. |
||
|
|
||
| created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), | ||
loop-index marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| updated_at TIMESTAMP WITH TIME ZONE, | ||
| deleted_at TIMESTAMP WITH TIME ZONE, | ||
|
|
||
| UNIQUE (site_id, name) | ||
| ); | ||
|
|
||
| -- Role permissions (many-to-many) | ||
| CREATE TABLE role_permission ( | ||
| role_id BIGINT NOT NULL REFERENCES role(role_id), | ||
| permission_id BIGINT NOT NULL REFERENCES permission(permission_id), | ||
| PRIMARY KEY (role_id, permission_id) | ||
| ); | ||
|
|
||
| -- User role assignments (many-to-many) | ||
| CREATE TABLE user_role ( | ||
| user_id BIGINT NOT NULL REFERENCES "user"(user_id), | ||
| role_id BIGINT NOT NULL REFERENCES role(role_id), | ||
loop-index marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| -- Denormalized FK to avoid a join. | ||
| site_id BIGINT NOT NULL REFERENCES site(site_id), | ||
|
|
||
| assigned_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), | ||
| assigned_by BIGINT NOT NULL REFERENCES "user"(user_id), | ||
| expires_at TIMESTAMP WITH TIME ZONE, | ||
| deleted_at TIMESTAMP WITH TIME ZONE, | ||
| PRIMARY KEY (user_id, role_id) | ||
| ); | ||
|
|
||
| -- Index for permission lookup. | ||
| CREATE UNIQUE INDEX permission_resource_action_idx ON permission | ||
| (resource_type, action); | ||
|
|
||
| -- | ||
| -- Audit Log | ||
| -- | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| [ | ||
| { | ||
| "description": "Can view and read pages", | ||
| "resource-type": "page", | ||
| "action": "view" | ||
| }, | ||
| { | ||
| "description": "Can edit existing pages", | ||
| "resource-type": "page", | ||
| "action": "edit" | ||
| }, | ||
| { | ||
| "description": "Can create new pages", | ||
| "resource-type": "page", | ||
| "action": "create" | ||
| }, | ||
| { | ||
| "description": "Can delete pages", | ||
| "resource-type": "page", | ||
| "action": "delete" | ||
| }, | ||
| { | ||
| "description": "Can rename/move pages", | ||
| "resource-type": "page", | ||
| "action": "rename" | ||
| }, | ||
| { | ||
| "description": "Can view role assignments", | ||
| "resource-type": "role", | ||
| "action": "view" | ||
| }, | ||
| { | ||
| "description": "Can assign roles to users", | ||
| "resource-type": "role", | ||
| "action": "assign" | ||
| }, | ||
| { | ||
| "description": "Can remove roles from users", | ||
| "resource-type": "role", | ||
| "action": "remove" | ||
| } | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| [ | ||
| { | ||
| "name": "root", | ||
| "description": "Root user with full site permissions", | ||
| "is-virtual": false, | ||
| "is-system": true, | ||
| "level": 100, | ||
| "permissions": [ | ||
| "page:view", | ||
| "page:edit", | ||
| "page:create", | ||
| "page:delete", | ||
| "page:rename", | ||
| "role:view", | ||
| "role:assign", | ||
| "role:remove" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "admin", | ||
| "description": "Site administrator with full control", | ||
| "is-virtual": false, | ||
| "is-system": false, | ||
| "level": 99, | ||
| "permissions": [ | ||
| "page:view", | ||
| "page:edit", | ||
| "page:create", | ||
| "page:delete", | ||
| "page:rename", | ||
| "role:view", | ||
| "role:assign", | ||
| "role:remove" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "moderator", | ||
| "description": "Site moderator with full control over pages", | ||
| "is-virtual": false, | ||
| "is-system": false, | ||
| "level": 90, | ||
| "permissions": [ | ||
| "page:view", | ||
| "page:edit", | ||
| "page:create", | ||
| "page:delete", | ||
| "role:view" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "member", | ||
| "description": "Regular site member", | ||
| "is-virtual": false, | ||
| "is-system": true, | ||
| "level": 10, | ||
| "permissions": [ | ||
| "page:view", | ||
| "page:edit", | ||
| "page:create" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "guest", | ||
| "description": "Virtual role for all non-members, including registered and anonymous users.", | ||
| "is-virtual": true, | ||
| "is-system": true, | ||
| "level": 1, | ||
| "permissions": [ | ||
| "page:view" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "registered", | ||
| "description": "Virtual role for registered and logged-in users.", | ||
| "is-virtual": true, | ||
| "is-system": true, | ||
| "level": 1, | ||
| "permissions": [ | ||
| "page:view" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "anonymous", | ||
| "description": "Virtual role for unregistered or logged out users.", | ||
| "is-virtual": true, | ||
| "is-system": true, | ||
| "level": 1, | ||
| "permissions": [ | ||
| "page:view" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "everyone", | ||
| "description": "Virtual role for all users, regardless of membership or registration status.", | ||
| "is-virtual": true, | ||
| "is-system": true, | ||
| "level": 1, | ||
| "permissions": [ | ||
| "page:view" | ||
| ] | ||
| }, | ||
| { | ||
| "name": "page-author", | ||
| "description": "Virtual role granted to the user when they visit a page that they authored, based on the page attribution metadata.", | ||
| "is-virtual": true, | ||
| "is-system": true, | ||
| "level": 1, | ||
| "permissions": [ | ||
| "page:view", | ||
| "page:edit", | ||
| "page:create", | ||
| "page:delete", | ||
| "page:rename" | ||
| ] | ||
| } | ||
loop-index marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the idea here is that permissions like
page:readandpage:editwill be in this table? I suppose that actions would be implemented in code (e.g.edit) vs the whole permission?I suppose the reason for that would be that some permissions may be more finely-tuned, like for page categories?
Also if it is platform-wide, keep in mind that permission descriptions need to be localizable, which wouldn't really work with a single
descriptioncolumn. (This would be fine if they were per-site, since then it would site admins setting the description, which would then be localized/customized to their site's audience.)Additionally, if we want to have the concepts of platform permissions and custom site permissions, we can add a nullable
site_idcolumn (NULL= platform, otherwise = for that site). Though I'm undecided about this given the localization issue.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair concern - putting page category aside, I think one way to solve the localization problem is using the [resource:action] pair as the key to look up in the locale file. Then we don't have to bake the description inside the entry. That would still not work with the custom site permissions though.
I think for most actions, just calling a lookup to see if user has permission for a resource would suffice. Checks needing additional logic can have a wrapper over the lookup function to allow for more granular checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To explore this a bit more, what exactly do you have in mind for custom site permissions? Any illustrative examples?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only use case that I can think of right now is per PageCategory permissions - since each site may have a different list of page categories, the permissions will also have to be scoped per site like how Wikidot currently does it.