Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored groupPermissions #8528

Closed
wants to merge 47 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
a076462
Refactored groupPermissions
kurtisassad Jul 18, 2024
2328c70
Added group permission logic for all adding/updating/removing groups
kurtisassad Jul 25, 2024
c5230dc
Fixed UI issue
kurtisassad Jul 25, 2024
a585f93
Fixed lint errors
kurtisassad Jul 30, 2024
c96c360
Fixed UI for topic gating
kurtisassad Jul 30, 2024
b941aab
Fixed remaining bugs with UI
kurtisassad Aug 1, 2024
bb180b1
Merge branch 'master' into ka.groupPermissionsCleanup
kurtisassad Aug 1, 2024
d2be5be
Fixed logic
kurtisassad Aug 1, 2024
77185db
Fixed lint errors
kurtisassad Aug 1, 2024
b4f87c9
Fixed lint errors
kurtisassad Aug 1, 2024
00cda1a
Fixed lint errors
kurtisassad Aug 1, 2024
38f69ec
Fixed lint errors
kurtisassad Aug 1, 2024
71ed1cc
Fixed lint errors
kurtisassad Aug 1, 2024
f3686a2
Fixed lint errors
kurtisassad Aug 1, 2024
b7509e7
Fixed PR comments
kurtisassad Aug 7, 2024
e7b86d8
Fixed PR comments
kurtisassad Aug 8, 2024
1a3663b
Fixed PR comments
kurtisassad Aug 8, 2024
de14dc1
Merge branch 'master' into ka.groupPermissionsCleanup
kurtisassad Aug 8, 2024
7520513
Fixed merge conlflicts
kurtisassad Aug 8, 2024
371512e
Fixed lint errors
kurtisassad Aug 9, 2024
2920702
Fixed unit tests
kurtisassad Aug 9, 2024
d5155d9
Fixed drop table transaction
kurtisassad Aug 9, 2024
74ed80a
Merge branch 'master' into ka.groupPermissionsCleanup
kurtisassad Aug 12, 2024
dde914c
Resolved merge conflicts
kurtisassad Aug 12, 2024
aa69a9d
Merge branch 'master' into ka.groupPermissionsCleanup
kurtisassad Aug 16, 2024
b5c6934
Fixed lint error
kurtisassad Aug 16, 2024
d984825
Merge branch 'master' into ka.groupPermissionsCleanup
kurtisassad Aug 19, 2024
943e588
Merge branch 'master' into ka.groupPermissionsCleanup
kurtisassad Aug 19, 2024
2caadd7
Fixed PR comments
kurtisassad Aug 19, 2024
099e810
Updated migration and fixed lint errors
kurtisassad Aug 19, 2024
97380e3
Fixed test failure
kurtisassad Aug 19, 2024
9e984aa
Removed test
kurtisassad Aug 19, 2024
5eb64c4
Fixed lint errors
kurtisassad Aug 19, 2024
329f1b1
Merge branch 'master' into ka.groupPermissionsCleanup
kurtisassad Aug 22, 2024
ab6393d
Resolved merge conflicts
kurtisassad Aug 22, 2024
ccfeffc
Merge branch 'master' into ka.groupPermissionsCleanup
kurtisassad Sep 3, 2024
2020d4e
Resolved merge conflicts
kurtisassad Sep 3, 2024
77d678c
Merge branch 'master' into ka.groupPermissionsCleanup
kurtisassad Sep 4, 2024
f592b71
Fixed groups UI
kurtisassad Sep 4, 2024
3b81098
Fixed lint errors
kurtisassad Sep 4, 2024
fef29f6
Fixed lint errors
kurtisassad Sep 4, 2024
b9e8012
Fixed lint errors
kurtisassad Sep 4, 2024
34c1534
Fixed lint errors
kurtisassad Sep 4, 2024
39d0488
Fixed lint errors
kurtisassad Sep 4, 2024
c4579c5
Fixed lint errors
kurtisassad Sep 4, 2024
e289a20
Updated migration name
kurtisassad Sep 4, 2024
0f85378
Fixed lint errors
kurtisassad Sep 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 15 additions & 16 deletions libs/model/src/community/CreateGroup.command.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { InvalidState, type Command } from '@hicommonwealth/core';
import * as schemas from '@hicommonwealth/schemas';
import { ForumActionsEnum } from '@hicommonwealth/schemas';
import { Op } from 'sequelize';
import { models, sequelize } from '../database';
import { models } from '../database';
import { isCommunityAdminOrModerator } from '../middleware';
import { mustNotExist } from '../middleware/guards';
import { GroupAttributes } from '../models';
import { GroupAttributes, GroupPermissionAttributes } from '../models';

export const MAX_GROUPS_PER_COMMUNITY = 20;
export const Errors = {
Expand Down Expand Up @@ -55,21 +56,19 @@ export function CreateGroup(): Command<typeof schemas.CreateGroup> {
{ transaction },
);
if (topicsToAssociate.length > 0) {
// add group to all specified topics
await models.Topic.update(
{
group_ids: sequelize.fn(
'array_append',
sequelize.col('group_ids'),
group.id,
),
},
const permissions = topicsToAssociate.map((topic) => ({
group_id: group.id,
topic_id: topic.id,
allowed_actions: models.sequelize.literal(
`ARRAY[${Object.values(ForumActionsEnum)
.map((value) => `'${value}'`)
.join(', ')}]::"enum_GroupPermissions_allowed_actions"[]`,
),
}));

await models.GroupPermission.bulkCreate(
permissions as unknown as GroupPermissionAttributes[],
{
where: {
id: {
[Op.in]: topicsToAssociate.map(({ id }) => id!),
},
},
transaction,
},
);
Expand Down
54 changes: 26 additions & 28 deletions libs/model/src/middleware/authorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import {
type CommandHandler,
} from '@hicommonwealth/core';
import * as schemas from '@hicommonwealth/schemas';
import { Address, Group, GroupPermissionAction } from '@hicommonwealth/schemas';
import { Address, ForumActions } from '@hicommonwealth/schemas';
import { Role } from '@hicommonwealth/shared';
import { Op, QueryTypes } from 'sequelize';
import { ZodObject, ZodSchema, ZodString, z } from 'zod';
import { models } from '..';
import { GroupInstance, models } from '..';

export type CommunityAuth = CommandHandler<ZodSchema, ZodSchema>;
export type ThreadAuth = CommandHandler<ZodSchema, typeof schemas.Thread>;
Expand All @@ -29,7 +29,7 @@ export class NonMember extends InvalidActor {
constructor(
public actor: Actor,
public topic: string,
public action: GroupPermissionAction,
public action: ForumActions,
) {
super(
actor,
Expand Down Expand Up @@ -103,44 +103,40 @@ const authorizeAddress = async (
*/
async function isTopicMember(
{ actor, payload }: CommandContext<ZodSchema>,
action: GroupPermissionAction,
action: ForumActions,
): Promise<void> {
// By convention, topic_id must by part of the body
const topic_id = 'topic_id' in payload && payload.topic_id;
if (!topic_id) throw new InvalidInput('Must provide a topic id');

const topic = await models.Topic.findOne({ where: { id: topic_id } });
if (!topic) throw new InvalidInput('Topic not found');
if (topic.group_ids?.length === 0) return;

const groups = await models.sequelize.query<
z.infer<typeof Group> & {
allowed_actions?: GroupPermissionAction[];
}
>(
const groups: (GroupInstance & {
allowed_actions?: ForumActions[];
})[] = await models.sequelize.query(
`
SELECT g.*, gp.allowed_actions
FROM "Groups" as g
LEFT JOIN "GroupPermissions" gp ON g.id = gp.group_id
WHERE g.community_id = :community_id AND g.id IN (:group_ids);
`,
SELECT g.*, gp.allowed_actions FROM "Groups" as g LEFT JOIN "GroupPermissions" gp ON g.id = gp.group_id
WHERE gp.topic_id = :topicId;
`,
{
type: QueryTypes.SELECT,
raw: true,
replacements: {
community_id: topic.community_id,
group_ids: topic.group_ids,
},
replacements: { topicId: topic_id },
},
);

// There are 2 cases here. We either have the old group permission system where the group doesn't have
// any allowed_actions, or we have the new fine-grained permission system where the action must be in
// the allowed_actions list.
// if there are no permissions for this topic, then anyone can perform any action on it (default behaviour).
if (groups.length === 0) {
return;
}

const allowed = groups.filter(
(g) => !g.allowed_actions || g.allowed_actions.includes(action),
);
if (!allowed.length!) throw new NonMember(actor, topic.name, action);

// if no group allows the specified action for the given topic, then reject because regardless of membership the user
// will not be allowed.
if (allowed?.length === 0) {
throw new NonMember(actor, groups[0]!.metadata!.name, action);
}

// check membership for all groups of topic
const memberships = await models.Membership.findAll({
Expand All @@ -155,7 +151,9 @@ async function isTopicMember(
},
],
});
if (!memberships.length) throw new NonMember(actor, topic.name, action);

if (!memberships.length)
throw new NonMember(actor, groups[0]!.metadata!.name, action);

const rejects = memberships.filter((m) => m.reject_reason);
if (rejects.length === memberships.length)
Expand Down Expand Up @@ -202,7 +200,7 @@ export const isCommunityAdminOrModerator: CommunityAuth = async (ctx) => {
};

export function isCommunityAdminOrTopicMember(
action: GroupPermissionAction,
action: ForumActions,
): CommunityAuth {
return async (ctx) => {
// super admin is always allowed
Expand Down
4 changes: 3 additions & 1 deletion libs/model/src/models/associations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ export const buildAssociations = (db: DB) => {
asMany: 'threads',
onUpdate: 'CASCADE',
onDelete: 'SET NULL',
}).withMany(db.ContestTopic);
})
.withMany(db.ContestTopic)
.withMany(db.GroupPermission);

db.Thread.withMany(db.Poll)
.withMany(db.ContestAction, {
Expand Down
2 changes: 2 additions & 0 deletions libs/model/src/models/group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ import { Group } from '@hicommonwealth/schemas';
import Sequelize from 'sequelize';
import z from 'zod';
import type { CommunityAttributes } from './community';
import { GroupPermissionAttributes } from './groupPermission';
import type { MembershipAttributes } from './membership';
import type { ModelInstance } from './types';

export type GroupAttributes = z.infer<typeof Group> & {
// associations
community?: CommunityAttributes;
memberships?: MembershipAttributes[];
groupPermissions?: GroupPermissionAttributes[];
};

export type GroupInstance = ModelInstance<GroupAttributes>;
Expand Down
5 changes: 5 additions & 0 deletions libs/model/src/models/groupPermission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ export default (
type: Sequelize.ARRAY(Sequelize.STRING),
allowNull: false,
},
topic_id: {
type: Sequelize.INTEGER,
allowNull: false,
primaryKey: true,
},
},
{
tableName: 'GroupPermissions',
Expand Down
1 change: 1 addition & 0 deletions libs/model/src/models/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export * from './discord_bot_config';
export * from './email_update_token';
export * from './evmEventSource';
export * from './group';
export * from './groupPermission';
export * from './lastProcessedEvmBlock';
export * from './membership';
export * from './notification';
Expand Down
6 changes: 0 additions & 6 deletions libs/model/src/models/topic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export type TopicAttributes = {
updated_at?: Date;
deleted_at?: Date;
default_offchain_template?: string;
group_ids?: number[];
telegram?: string;

// associations
Expand Down Expand Up @@ -57,11 +56,6 @@ export default (
allowNull: true,
},
channel_id: { type: Sequelize.STRING, allowNull: true },
group_ids: {
type: Sequelize.ARRAY(Sequelize.INTEGER),
allowNull: false,
defaultValue: [],
},
telegram: { type: Sequelize.STRING, allowNull: true },
},
{
Expand Down
2 changes: 1 addition & 1 deletion libs/model/src/thread/CreateThread.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export function CreateThread(): Command<typeof schemas.CreateThread> {
return {
...schemas.CreateThread,
auth: [
isCommunityAdminOrTopicMember(schemas.PermissionEnum.CREATE_THREAD),
isCommunityAdminOrTopicMember(schemas.ForumActionsEnum.CREATE_THREAD),
verifyThreadSignature,
],
body: async ({ actor, payload }) => {
Expand Down
5 changes: 2 additions & 3 deletions libs/model/test/thread/thread-lifecycle.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Actor, command, dispose } from '@hicommonwealth/core';
import { models } from '@hicommonwealth/model';
import { PermissionEnum } from '@hicommonwealth/schemas';
import { ForumActionsEnum } from '@hicommonwealth/schemas';
import { Chance } from 'chance';
import { BannedActor, NonMember, RejectedMember } from 'model/src/middleware';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
Expand Down Expand Up @@ -47,11 +47,10 @@ describe('Thread lifecycle', () => {
is_banned: role === 'banned',
})),
groups: [{ id: groupId }],
topics: [{ group_ids: [groupId] }],
});
await seed('GroupPermission', {
group_id: groupId,
allowed_actions: [PermissionEnum.CREATE_THREAD],
allowed_actions: [ForumActionsEnum.CREATE_THREAD],
});

roles.forEach((role) => {
Expand Down
7 changes: 4 additions & 3 deletions libs/schemas/src/entities/group-permission.schemas.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import { z } from 'zod';
import { PG_INT } from '../utils';

export enum PermissionEnum {
export enum ForumActionsEnum {
CREATE_THREAD = 'CREATE_THREAD',
CREATE_COMMENT = 'CREATE_COMMENT',
CREATE_THREAD_REACTION = 'CREATE_THREAD_REACTION',
CREATE_COMMENT_REACTION = 'CREATE_COMMENT_REACTION',
UPDATE_POLL = 'UPDATE_POLL',
}

export type GroupPermissionAction = keyof typeof PermissionEnum;
export type ForumActions = keyof typeof ForumActionsEnum;

export const GroupPermission = z.object({
group_id: PG_INT.optional(),
allowed_actions: z.array(z.nativeEnum(PermissionEnum)),
allowed_actions: z.array(z.nativeEnum(ForumActionsEnum)),
topic_id: PG_INT.optional(),

created_at: z.coerce.date().optional(),
updated_at: z.coerce.date().optional(),
Expand Down
1 change: 0 additions & 1 deletion libs/schemas/src/entities/topic.schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,5 @@ export const Topic = z.object({
default_offchain_template: z.string().nullish(),
order: PG_INT.nullish(),
channel_id: z.string().max(255).nullish(),
group_ids: z.array(PG_INT).default([]),
default_offchain_template_backup: z.string().nullish(),
});
108 changes: 108 additions & 0 deletions packages/commonwealth/client/scripts/hooks/useForumActionGated.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { ForumActions, ForumActionsEnum } from '@hicommonwealth/schemas';
import { useRefreshMembershipQuery } from '../state/api/groups';

type UseAllowedGroupsParams = {
communityId: string;
address: string;
topicId?: number;
isAdmin: boolean;
};

export type UseForumActionGatedResponse = {
canCreateThread: boolean;
canCreateComment: boolean;
canReactToThread: boolean;
canReactToComment: boolean;
canUpdatePoll: boolean;
};

const allowEverything = {
canCreateThread: true,
canCreateComment: true,
canReactToThread: true,
canReactToComment: true,
canUpdatePoll: true,
};

const notInGroup = {
canCreateThread: false,
canCreateComment: false,
canReactToThread: false,
canReactToComment: false,
canUpdatePoll: false,
};

export const useForumActionGated = ({
communityId,
address,
topicId,
isAdmin,
}: UseAllowedGroupsParams): UseForumActionGatedResponse => {
const { data: memberships = [] } = useRefreshMembershipQuery({
communityId,
address,
topicId: topicId?.toString(),
apiEnabled: !!address,
});

if (memberships.length === 0 || isAdmin || !topicId) {
return allowEverything;
}

const validGroups = (memberships || []).filter(
(membership) => membership?.rejectReason?.length === 0,
);

const flatMemberships = validGroups
.flatMap((m) =>
m.topicIds.map((t) => ({
topic_id: t,
allowedActions: m.allowedActions,
})),
)
.map((g) => [g.topic_id, g.allowedActions]) as unknown as [
number,
ForumActions[],
][];

const topicIdToIsAllowedMap: Map<number, ForumActions[]> = new Map(
kurtisassad marked this conversation as resolved.
Show resolved Hide resolved
flatMemberships,
);

// Each map entry represents a topic and associated forumActions they are allowed to perform. We want to find for
// each topic, the set union of all associated forumActions to get the actions that the address can perform.
topicIdToIsAllowedMap.forEach((value, key) => {
const oldAllowList = topicIdToIsAllowedMap.get(key);
if (!oldAllowList) {
topicIdToIsAllowedMap.set(key, Array.from(new Set(value)));
}
topicIdToIsAllowedMap.set(
key,
Array.from(new Set([...(oldAllowList as []), ...(value as [])])),
);
});

const allowedActionsForTopic = topicIdToIsAllowedMap.get(topicId ?? 0);

if (!allowedActionsForTopic) {
return notInGroup;
}

return {
canCreateThread: allowedActionsForTopic.includes(
ForumActionsEnum.CREATE_THREAD,
),
canCreateComment: allowedActionsForTopic.includes(
ForumActionsEnum.CREATE_COMMENT,
),
canReactToThread: allowedActionsForTopic.includes(
ForumActionsEnum.CREATE_THREAD_REACTION,
),
canReactToComment: allowedActionsForTopic.includes(
ForumActionsEnum.CREATE_COMMENT_REACTION,
),
canUpdatePoll: allowedActionsForTopic.includes(
ForumActionsEnum.UPDATE_POLL,
),
};
};
Loading
Loading