Details

Reviewers

ashoat
atul
ginsu

Commits

rCOMMb49929e80a2b: [keyserver] Write a script to compare database role permissions match…

Summary

This is a script that addresses some feedback from D9599. Ideally, before I put up a diff to attempt to unify universalCommunityPermissions and the general thread permission blobs created for Admins and Members in some way to acheive consistency, it'll be good to run a script against the database contents for roles and permissions to see if there's anything else I'll need to take into consideration. I've already noted that join_thread can probably be removed from universalCommunityPermissions, but the script should tell me the rest.

The flow of the script is as follows:

Fetch roles for community roots and community announcement roots
Extract the relevant information for each role
Get the expected permissions for the role and the actual/existing permissions for the role
Call deepDiff two ways on these two permission blobs
If there are any disrepencies, attempt to link them back to some user surfaced permissions that could indicate that it's only a result of a user editing a role and not a malformed database

I'm not really sure how this script will behave against a production database, but some thorough testing hasn't led to any glaring issues. I'm expecting a ton of output though that I'll need to sit and parse through.

Resolves ENG-5621

Test Plan

Edited a role's permissions to trigger some changes between the expected and actual role permissions for a role. This is the output of the script:

====================================
Validating: Role Name (Members) | Role ID (90477) | Thread Type (8) | Thread ID (90476)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {
  "join_thread": true,
  "descendant_react_to_message": true,
  "descendant_edit_message": true,
  "descendant_add_members": true,
  "descendant_edit_entries": true,
  "descendant_edit_thread": true,
  "descendant_edit_thread_description": true,
  "descendant_edit_thread_color": true,
  "descendant_toplevel_create_subthreads": true,
  "descendant_edit_thread_avatar": true,
  "descendant_toplevel_create_sidebars": true
}

Potential permission disrecepencies for role Members that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

userSurfacedExistingPermissionsToExpectedPermissions = [
  "edit_calendar",
  "create_and_edit_channels",
  "add_members",
  "react_to_messages",
  "edit_messages"
]
====================================
Validating: Role Name (Admins) | Role ID (90478) | Thread Type (8) | Thread ID (90476)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {}

Potential permission disrecepencies for role Admins that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

====================================
Validating: Role Name (Members) | Role ID (90496) | Thread Type (8) | Thread ID (90495)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {}

Potential permission disrecepencies for role Members that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

====================================
Validating: Role Name (Admins) | Role ID (90497) | Thread Type (8) | Thread ID (90495)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {}

Potential permission disrecepencies for role Admins that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

====================================
Validating: Role Name (Members) | Role ID (90515) | Thread Type (8) | Thread ID (90514)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {}

Potential permission disrecepencies for role Members that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

====================================
Validating: Role Name (Admins) | Role ID (90516) | Thread Type (8) | Thread ID (90514)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {}

Potential permission disrecepencies for role Admins that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

====================================
Validating: Role Name (Members) | Role ID (90534) | Thread Type (9) | Thread ID (90533)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {}

Potential permission disrecepencies for role Members that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

====================================
Validating: Role Name (Admins) | Role ID (90535) | Thread Type (9) | Thread ID (90533)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {}

Potential permission disrecepencies for role Admins that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

====================================
Validating: Role Name (Members) | Role ID (90547) | Thread Type (9) | Thread ID (90546)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {}

Potential permission disrecepencies for role Members that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

====================================
Validating: Role Name (Admins) | Role ID (90548) | Thread Type (9) | Thread ID (90546)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {}

Potential permission disrecepencies for role Admins that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

====================================
Validating: Role Name (Members) | Role ID (90560) | Thread Type (9) | Thread ID (90559)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {}

Potential permission disrecepencies for role Members that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

====================================
Validating: Role Name (Admins) | Role ID (90561) | Thread Type (9) | Thread ID (90559)

deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = {}

deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = {}

Potential permission disrecepencies for role Admins that could be linked back to user surfaced permissions (i.e. not an actual discrepency, but rather a user edited a role: 

====================================

Diff Detail

Repository

rCOMM Comm

Branch

combined_branch

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

rohan created this revision.Nov 2 2023, 9:04 AM

Herald added subscribers: will, tomek. · View Herald TranscriptNov 2 2023, 9:04 AM

Prevent unnecessary console logging when there are no differences

Use > instead of !== to keep consistent

Harbormaster completed remote builds in B23669: Diff 32623.Nov 2 2023, 9:23 AM

Harbormaster completed remote builds in B23670: Diff 32624.Nov 2 2023, 9:36 AM

Harbormaster completed remote builds in B23671: Diff 32625.Nov 2 2023, 9:51 AM

rohan requested review of this revision.Nov 2 2023, 9:51 AM

ashoat accepted this revision.Nov 2 2023, 10:00 AM

ashoat added inline comments.

keyserver/src/scripts/validate-role-permissions.js
43 ↗	(On Diff #32625)	Typo ("rule")
101 ↗	(On Diff #32625)	Typo ("discrepenices")
103 ↗	(On Diff #32625)	Same typo (should be "discrepancies")
133 ↗	(On Diff #32625)	Same typo
135 ↗	(On Diff #32625)	Missing close paren

This revision is now accepted and ready to land.Nov 2 2023, 10:00 AM

Typos

Harbormaster completed remote builds in B23681: Diff 32635.Nov 2 2023, 11:20 AM

rohan edited the summary of this revision. (Show Details)Nov 2 2023, 6:18 PM

rohan removed a parent revision: D9514: [lib/web/native] Show voiced_in_announcement_channels user-surfaced permission for all community types.

Closed by commit rCOMMb49929e80a2b: [keyserver] Write a script to compare database role permissions match… (authored by rohan). · Explain WhyNov 2 2023, 6:56 PM

This revision was automatically updated to reflect the committed changes.

rohan added a commit: rCOMMb49929e80a2b: [keyserver] Write a script to compare database role permissions match….

Let me know if you want me to run this in production!

In D9675#284073, @ashoat wrote:

Let me know if you want me to run this in production!

It'd be great if you could when you have a chance! It'll be useful to make sure I have covered all cases and haven't missed out on any

[keyserver] Write a script to compare database role permissions match expectations
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 32623

keyserver/src/scripts/validate-role-permissions.js

[keyserver] Write a script to compare database role permissions match expectationsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 32623

keyserver/src/scripts/validate-role-permissions.js

[keyserver] Write a script to compare database role permissions match expectations
ClosedPublic
Actions

Revision Contents
Changeset List