From b49929e80a2b2170bb04c086987ceb4cb95e9439 Mon Sep 17 00:00:00 2001 From: Rohan Khanderia Date: Thu, 2 Nov 2023 21:19:13 -0400 Subject: [PATCH] [keyserver] Write a script to compare database role permissions match expectations 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: 1. Fetch roles for community roots and community announcement roots 2. Extract the relevant information for each role 3. Get the expected permissions for the role and the actual/existing permissions for the role 4. Call `deepDiff` two ways on these two permission blobs 5. 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 [[ https://linear.app/comm/issue/ENG-5621/write-a-script-to-compare-database-role-permissions-match-expectations | 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: ==================================== ``` Reviewers: ashoat, atul, ginsu Reviewed By: ashoat Subscribers: tomek, wyilio Differential Revision: https://phab.comm.dev/D9675 --- .../src/scripts/validate-role-permissions.js | 162 ++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 keyserver/src/scripts/validate-role-permissions.js diff --git a/keyserver/src/scripts/validate-role-permissions.js b/keyserver/src/scripts/validate-role-permissions.js new file mode 100644 index 0000000000..32f2a3a80d --- /dev/null +++ b/keyserver/src/scripts/validate-role-permissions.js @@ -0,0 +1,162 @@ +// @flow + +import { getRolePermissionBlobs } from 'lib/permissions/thread-permissions.js'; +import { + configurableCommunityPermissions, + userSurfacedPermissions, + universalCommunityPermissions, +} from 'lib/types/thread-permission-types.js'; +import { threadTypes } from 'lib/types/thread-types-enum.js'; +import { deepDiff, values } from 'lib/utils/objects.js'; + +import { main } from './utils.js'; +import { SQL, dbQuery } from '../database/database.js'; + +async function validateRolePermissions() { + // Get all roles for existing communities since custom roles are at a + // community-level rather than a thread-level. + const fetchRolesQuery = SQL` + SELECT r.id, r.name, r.permissions, r.thread, t.type, t.default_role + FROM roles r + INNER JOIN threads t + ON t.id = r.thread + WHERE t.type IN (${[ + threadTypes.COMMUNITY_ANNOUNCEMENT_ROOT, + threadTypes.COMMUNITY_ROOT, + ]}) + `; + const [results] = await dbQuery(fetchRolesQuery); + + for (const result of results) { + const roleID = result.id.toString(); + const roleName = result.name; + const existingRolePermissions = JSON.parse(result.permissions); + const threadID = result.thread.toString(); + const threadType = result.type; + const threadDefaultRole = result.default_role.toString(); + + // Get the 'expected permissions' set for the role. If the role is + // default (Members) or Admins, these permission blobs can be retrieved + // by calling getRolePermissionBlobs with the threadType. Otherwise, the + // role is a custom role and the expected permissions are the universal + // community permissions assuming the role has not been edited. + // The case of a role being edited is handled below. + const expectedPermissionBlobs = getRolePermissionBlobs(threadType); + let baseExpectedPermissionBlob; + if (roleID === threadDefaultRole) { + baseExpectedPermissionBlob = expectedPermissionBlobs.Members; + } else if (roleName === 'Admins') { + baseExpectedPermissionBlob = expectedPermissionBlobs.Admins; + } else if (roleName) { + baseExpectedPermissionBlob = Object.fromEntries( + universalCommunityPermissions.map(permission => [permission, true]), + ); + } else { + baseExpectedPermissionBlob = {}; + } + console.log('===================================='); + + // Ideally, this should never happen, but we'll skip over this in case. + if (!baseExpectedPermissionBlob) { + console.log( + `Skipping role ${roleName} with ID (${roleID}) in thread ${threadID}`, + ); + continue; + } + + // Deep diff seems to compare objects one-way (so deepDiff(a, b) !== + // deepDiff(b, a)). This means that if a key is not in `a` but not in `b`, + // the diff will not include that key. As a result, we need to compare both + // ways to ensure that we're not missing any permission discrepancies. + const expectedPermissionsToExistingPermissions = deepDiff( + baseExpectedPermissionBlob, + existingRolePermissions, + ); + const existingPermissionsToExpectedPermissions = deepDiff( + existingRolePermissions, + baseExpectedPermissionBlob, + ); + + console.log( + `Validating: Role Name (${roleName}) | Role ID (${roleID}) | ` + + `Thread Type (${threadType}) | Thread ID (${threadID})\n`, + ); + console.log( + `deepDiff(baseExpectedPermissionBlob, existingRolePermissions) = ${JSON.stringify( + expectedPermissionsToExistingPermissions, + null, + 2, + )}\n`, + ); + console.log( + `deepDiff(existingRolePermissions, baseExpectedPermissionBlob) = ${JSON.stringify( + existingPermissionsToExpectedPermissions, + null, + 2, + )}\n`, + ); + + // Now, we want to see if the permission discrepancies are due to the user + // editing the role. To do this, we need to identify any permission + // discrepancies that could be linked to a specific user-surfaced + // permission. This could be useful in manually parsing through the + // script results to 'write off' discrepancies as user role edits. + const userSurfacedExpectedPermissionsToExistingPermissions = new Set(); + const userSurfacedExistingPermissionsToExpectedPermissions = new Set(); + + for (const permission of values(userSurfacedPermissions)) { + const permissionSet = Array.from( + configurableCommunityPermissions[permission], + ); + for (const p of permissionSet) { + if (expectedPermissionsToExistingPermissions[p] === true) { + userSurfacedExpectedPermissionsToExistingPermissions.add(permission); + } + if (existingPermissionsToExpectedPermissions[p] === true) { + userSurfacedExistingPermissionsToExpectedPermissions.add(permission); + } + } + } + + const expectedPermissionsToExistingPermissionsValues = values( + expectedPermissionsToExistingPermissions, + ); + const existingPermissionsToExpectedPermissionsValues = values( + existingPermissionsToExpectedPermissions, + ); + + if ( + expectedPermissionsToExistingPermissionsValues.length > 0 || + existingPermissionsToExpectedPermissionsValues.length > 0 + ) { + console.log( + `Potential permission discrepancies for role ${roleName} that ` + + `could be linked back to user surfaced permissions (i.e. not an ` + + `actual discrepancy, but rather a user edited a role): \n`, + ); + + if (expectedPermissionsToExistingPermissionsValues.length > 0) { + console.log( + `userSurfacedExpectedPermissionsToExistingPermissions = ${JSON.stringify( + [...userSurfacedExpectedPermissionsToExistingPermissions], + null, + 2, + )}\n`, + ); + } + if (existingPermissionsToExpectedPermissionsValues.length > 0) { + console.log( + `userSurfacedExistingPermissionsToExpectedPermissions = ${JSON.stringify( + [...userSurfacedExistingPermissionsToExpectedPermissions], + null, + 2, + )}`, + ); + } + } + } + + console.log('===================================='); +} + +main([validateRolePermissions]);