Make Permissions into one table
Discuss your ideas or share your views:
while discussion chapter permission with Oliver, we noticed that the instance.permission and chapter.permission are confusing as they push for wrong impressions of the data nature
We can merge the permissions and roles into their own big table for simpler readability and easier maintenance
To elaborate: there's nothing special about instance_permissions vs chapter_permissions from an authorization perspective. They're all lumped together in https://github.com/freeCodeCamp/chapter/blob/c3e4b1b2b760fe75d92b3e5cf45c6c5bb2798aec/common/permissions.ts#L1-L33, and we only care about the Permission object.
So, there's no obvious reason why they should have separate tables in the database. That makes it harder to work with and obscures the fact that they're really the same types of thing. It's particularly confusing because it looks like an instance owner can't have ChapterPermissions, but they can (and do).
The difference comes from the type of role. i.e. if an instance_role has a permission, then users with that role have that permission for every chapter, whereas if a chapter_role has the same permission, it's tied to a particular chapter.
I feel like we started out with fewer tables and one of the fCC folks, I think Tom, did a full rework and that's when these got introduced in #958, so I don't remember a specific reason or conversation about that need.
https://github.com/freeCodeCamp/chapter/issues/967#issuecomment-1113803962
I do recall we separated the user permissions and preferences into different tables as an added layer of security since we didn't want people will lesser permissions being able to have read or write address to a table that could allow for a potential escalation of a role or permissions.
In case of throwing all roles together. How would be differentiated between role that's a chapter role - specific for single chapter only - and an instance role?
I do recall we separated the user permissions and preferences into different tables as an added layer of security since we didn't want people will lesser permissions being able to have read or write address to a table that could allow for a potential escalation of a role or permissions.
I believe that was the rationale. However, the reduced complexity probably outweighs that since complexity invites bugs. Not that this is desperately complex, just more than it needs to be.
In case of throwing all roles together. How would be differentiated between role that's a chapter role - specific for single chapter only - and an instance role?
I don't think we'd want to throw the roles together, just the permissions. We'd definitely have a hard time writing code to differentiate them.
So, in this schema we'd still have instance_roles, instance_role_permissions, chapter_roles, chapter_role_permissions, event_roles and event_role_permissions. The difference is that we'd replace the three x_permissions table with a single permissions table.
Since the instance_permission chapter-edit really is the same thing as the chapter_permission chapter-edit we end up cutting down on data duplication and reduce the complexity of migrations.
That sounds reasonable. It's anyway currently not possible to use (add) permission with the same name to both InstancePermission and ChapterPermission.