Skip to content

Commit

Permalink
[keyserver] Write a script to compare database role permissions match…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
RohanK6 committed Nov 3, 2023
1 parent 0eb7a82 commit b49929e
Showing 1 changed file with 162 additions and 0 deletions.
162 changes: 162 additions & 0 deletions keyserver/src/scripts/validate-role-permissions.js
Original file line number Diff line number Diff line change
@@ -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]);

0 comments on commit b49929e

Please sign in to comment.