[feat] Role and Permission system#2737
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #2737 +/- ##
==========================================
- Coverage 2.23% 2.10% -0.14%
==========================================
Files 181 190 +9
Lines 11468 12180 +712
==========================================
Hits 256 256
- Misses 11212 11924 +712
🚀 New features to boost your workflow:
|
|
Just a small note, temporary bans are implemented - this is done by adding a metadata property to the ban noting its expiration date. |
f08f82b to
70ae8b4
Compare
Needed for deepwell update requests.
Access to other containers is governed by the network, and this avoids the implicit container dependencies.
|
Made some updates to the system:
Sorry for the messy commit history, git hard |
| is_virtual BOOLEAN NOT NULL DEFAULT false, | ||
|
|
||
| -- System roles cannot be deleted (i.e. Admin) | ||
| is_system BOOLEAN NOT NULL DEFAULT false, |
There was a problem hiding this comment.
With our follow-up discussions, I'm clear on the is_virtual flag, but I'm not clear on is_system. It seems that the only non-virtual system roles root and page-author?
Basically, how should I conceive of what "system" is?
There was a problem hiding this comment.
Original intent was that system roles cannot be deleted (admin, mods) but since we narrowed it down to only make root user an undeletable role, I can remove this flag and use the SiteUser relation or something.
There was a problem hiding this comment.
on further examination, seems like SiteUser is the representative account of a site and not the root user/site creator. I'll keep this for now and think of a way to represent this role in a way that is permanent and transferable (like Master Admin on wikidot) such that there's always at least one user per site with full admin permissions.
There was a problem hiding this comment.
Correct, the "site user" is a special user type not controlled by any human which represents the site as a whole. So like, it's the user to message you on ban, etc.
As for root, I think we should avoid making it an equivalent of master admin. That one user being truly unevictable is a lot of trust and has caused issues on some branches if that trust turns out to be misplaced. I would prefer if it is possible for sites to construct their roles in such a way that all members of an admin-equivalent role have equal access, and then some kind of majority/multi approval system enables actions affecting the group as a whole.
Or put in more programmer terms, if root isn't a person but a role you can assume by doing "sudo" (whatever that means for a site's staff).
Implementing mechanisms for approved actions like this is out of scope for this PR, but I just wanted to give a sense of what I'm thinking to avoid social issues we have due to Wikidot design decisions.
| -- Polymorphic reference to a resource category. For example, if the resource_type is "forum_category", then this references forum_category_id. | ||
| -- NULL means the permission applies to all resources of the given type | ||
| resource_category_id BIGINT, | ||
| action TEXT NOT NULL, |
There was a problem hiding this comment.
Should we add a database enum type for either of resource_type or action?
There was a problem hiding this comment.
I did look into this before and the general consensus seems to be that Postgres enums are a pain to work with unless we have a solid list of all the variants that we want. But that is also mainly due to schema migration setups; since we are rebuilding the db anyway, that might not be that much of a hassle. lmk what you think.
There was a problem hiding this comment.
I'll start by noting that the recreating the database thing is a temporary measure to avoid having a long series of migrations as we stabilized the early schema. However after we reach the stage where we launch a prod instance, we will be using migrations iteratively as expected from then on.
So you raise a good point about enums, and something we should think about for the code. Overall, I have been using Postgres enums in our other tables, but you are correct that adding or removing types is more cumbersome. It is possible through ALTER TYPE etc., and native types do have the advantage that it enforces against typos or other incorrect value issues. We also have some CHECK constraints using these enums as well.
I've considered both angles, and I'm not sure. I would be fine with in-code enums if we set up a consistent trait system for it and checking to ensure that invalid values can't find their way into the table. Overall I think, whatever we decide, we should be consistent in the code.
What do your thoughts?
There was a problem hiding this comment.
another option worth considering is using reference tables instead of either enums or strings. so like a permission_resource table and a permission_action table, then foreign keys from role_permission into those
adding new resources/actions is just an INSERT rather than an ALTER TYPE, so no migration pain, but you still get db-level validation via foreign keys. and it fits the existing pattern in deepwell where everything uses integer ids and relations
postgres enums are fine when the variant list is truly static, but permissions are going to grow as we add features, so i think either reference tables or rust-side validation with text columns makes more sense here
There was a problem hiding this comment.
I actually started out this PR with that idea lol (DB side ref table). It definitely works but causes additional joinage and complexity when querying. I deprecated it in favor of flattening it onto role_permission and code enums.
I think code-side enums with some sort of validation makes the most sense. Making sure that the read/write entry points to the table only uses valid enum variants should be good. Maybe we can write some sort of trait that validates and generates query conditions instead of passing values directly to queries?
Bumps [eslint-plugin-svelte](https://github.com/sveltejs/eslint-plugin-svelte/tree/HEAD/packages/eslint-plugin-svelte) from 3.15.0 to 3.16.0. - [Release notes](https://github.com/sveltejs/eslint-plugin-svelte/releases) - [Changelog](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/CHANGELOG.md) - [Commits](https://github.com/sveltejs/eslint-plugin-svelte/commits/eslint-plugin-svelte@3.16.0/packages/eslint-plugin-svelte) --- updated-dependencies: - dependency-name: eslint-plugin-svelte dependency-version: 3.16.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [globals](https://github.com/sindresorhus/globals) from 17.3.0 to 17.4.0. - [Release notes](https://github.com/sindresorhus/globals/releases) - [Commits](sindresorhus/globals@v17.3.0...v17.4.0) --- updated-dependencies: - dependency-name: globals dependency-version: 17.4.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [stylelint](https://github.com/stylelint/stylelint) from 17.4.0 to 17.6.0. - [Release notes](https://github.com/stylelint/stylelint/releases) - [Changelog](https://github.com/stylelint/stylelint/blob/main/CHANGELOG.md) - [Commits](stylelint/stylelint@17.4.0...17.6.0) --- updated-dependencies: - dependency-name: stylelint dependency-version: 17.6.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [stylelint-config-recess-order](https://github.com/stormwarning/stylelint-config-recess-order) from 7.6.1 to 7.7.0. - [Release notes](https://github.com/stormwarning/stylelint-config-recess-order/releases) - [Changelog](https://github.com/stormwarning/stylelint-config-recess-order/blob/main/CHANGELOG.md) - [Commits](stormwarning/stylelint-config-recess-order@v7.6.1...v7.7.0) --- updated-dependencies: - dependency-name: stylelint-config-recess-order dependency-version: 7.7.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [typescript-eslint](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/typescript-eslint) from 8.57.1 to 8.57.2. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/typescript-eslint/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.57.2/packages/typescript-eslint) --- updated-dependencies: - dependency-name: typescript-eslint dependency-version: 8.57.2 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [uuid](https://github.com/uuid-rs/uuid) from 1.19.0 to 1.23.0. - [Release notes](https://github.com/uuid-rs/uuid/releases) - [Commits](uuid-rs/uuid@v1.19.0...v1.23.0) --- updated-dependencies: - dependency-name: uuid dependency-version: 1.23.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [toml](https://github.com/toml-rs/toml) from 1.0.7+spec-1.1.0 to 1.1.0+spec-1.1.0. - [Commits](toml-rs/toml@toml-v1.0.7...toml-v1.1.0) --- updated-dependencies: - dependency-name: toml dependency-version: 1.1.0+spec-1.1.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [sha2](https://github.com/RustCrypto/hashes) from 0.10.9 to 0.11.0. - [Commits](RustCrypto/hashes@sha2-v0.10.9...sha2-v0.11.0) --- updated-dependencies: - dependency-name: sha2 dependency-version: 0.11.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [askama](https://github.com/askama-rs/askama) from 0.15.5 to 0.15.6. - [Release notes](https://github.com/askama-rs/askama/releases) - [Commits](askama-rs/askama@v0.15.5...v0.15.6) --- updated-dependencies: - dependency-name: askama dependency-version: 0.15.6 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Implemented a simple RBAC-style role/permission system. Took some heavy inspiration from Discord's role system.
Planned improvements:
As a proof-of-concept, I've added a permission check for page edits. Currently only the admin account is allowed to edit pages.