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

Adds auth coverage #9886

Merged
merged 1 commit into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 12 additions & 15 deletions libs/model/src/middleware/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,9 @@ async function hasTopicPermissions(
}
>(
`
SELECT
g.*,
gp.allowed_actions as allowed_actions
FROM "Groups" as g
LEFT JOIN "GroupPermissions" gp ON g.id = gp.group_id AND gp.topic_id = :topic_id
WHERE g.community_id = :community_id
SELECT g.*, gp.topic_id, gp.allowed_actions
FROM "Groups" as g JOIN "GroupPermissions" gp ON g.id = gp.group_id
WHERE g.community_id = :community_id AND gp.topic_id = :topic_id
`,
{
type: QueryTypes.SELECT,
Expand All @@ -226,16 +223,16 @@ async function hasTopicPermissions(
// There are 2 cases here. We either have the old group permission system where the group doesn't have
// any group_allowed_actions, or we have the new fine-grained permission system where the action must be in
// the group_allowed_actions list.
const allowedGroupActions = groups.filter(
const allowedActions = groups.filter(
(g) => !g.allowed_actions || g.allowed_actions.includes(action),
);
if (!allowedGroupActions.length!)
if (allowedActions.length === 0)
throw new NonMember(actor, topic.name, action);

// check membership for all groups of topic
const memberships = await models.Membership.findAll({
where: {
group_id: { [Op.in]: allowedGroupActions.map((g) => g.id!) },
group_id: { [Op.in]: allowedActions.map((g) => g.id!) },
address_id,
},
include: [
Expand All @@ -245,7 +242,7 @@ async function hasTopicPermissions(
},
],
});
if (!memberships.length) throw new NonMember(actor, topic.name, action);
if (memberships.length === 0) throw new NonMember(actor, topic.name, action);

const rejects = memberships.filter((m) => m.reject_reason);
if (rejects.length === memberships.length)
Expand All @@ -269,7 +266,7 @@ async function mustBeAuthorized(
};
author?: boolean;
collaborators?: z.infer<typeof Address>[];
} = {},
},
) {
// System actors are always allowed
if (actor.is_system_actor) return;
Expand Down Expand Up @@ -361,7 +358,7 @@ export function authRoles(...roles: Role[]) {
community_id: ctx.payload.community_id,
};

await mustBeAuthorized(ctx);
await mustBeAuthorized(ctx, {});
};
}

Expand All @@ -378,7 +375,7 @@ type AggregateAuthOptions = {
* @param author when true, rejects members that are not the author
* @throws InvalidActor when not authorized
*/
export function authComment({ action, author }: AggregateAuthOptions = {}) {
export function authComment({ action, author }: AggregateAuthOptions) {
return async (
ctx: Context<typeof CommentContextInput, typeof CommentContext>,
) => {
Expand Down Expand Up @@ -414,7 +411,7 @@ export function authThread({
action,
author,
collaborators,
}: AggregateAuthOptions = {}) {
}: AggregateAuthOptions) {
return async (
ctx: Context<typeof ThreadContextInput, typeof ThreadContext>,
) => {
Expand Down Expand Up @@ -449,7 +446,7 @@ export function authThread({
* @param action specific group permission action
* @throws InvalidActor when not authorized
*/
export function authTopic({ roles, action }: AggregateAuthOptions = {}) {
export function authTopic({ roles, action }: AggregateAuthOptions) {
return async (
ctx: Context<typeof TopicContextInput, typeof TopicContext>,
) => {
Expand Down
22 changes: 22 additions & 0 deletions libs/model/test/community/community-lifecycle.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
UpdateCommunityErrors,
} from '../../src/community';
import { models } from '../../src/database';
import { systemActor } from '../../src/middleware';
import type {
ChainNodeAttributes,
CommunityAttributes,
Expand Down Expand Up @@ -544,6 +545,18 @@ describe('Community lifecycle', () => {
expect(updatedTopic.description).to.eq('newDesc');
});

test('should update a topic as a system actor', async () => {
const { topic: updatedTopic } = (await command(UpdateTopic(), {
actor: systemActor({}),
payload: {
topic_id: createdTopic.id!,
community_id: community.id,
description: 'newDesc by system actor',
},
}))!;
expect(updatedTopic.description).to.eq('newDesc by system actor');
});

test('should delete a topic', async () => {
const { topic } = (await command(CreateTopic(), {
actor: superAdminActor,
Expand Down Expand Up @@ -582,6 +595,15 @@ describe('Community lifecycle', () => {
).rejects.toThrow(InvalidActor);
});

test("should throw error when topic doesn't exist", async () => {
await expect(
command(DeleteTopic(), {
actor: ethAdminActor,
payload: { community_id: community.id, topic_id: 123456789 },
}),
).rejects.toThrow(InvalidInput);
});

test('should get topics', async () => {
const topics = await query(GetTopics(), {
actor: superAdminActor,
Expand Down
59 changes: 58 additions & 1 deletion libs/model/test/thread/thread-lifecycle.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ describe('Thread lifecycle', () => {
const signerInfo = await getSignersInfo(roles);
const threadGroupId = 123456;
const commentGroupId = 654321;
const emptyGroupId = 987654;
const [node] = await seed('ChainNode', { eth_chain_id: 1 });
const users = await seedRecord('User', roles, (role) => ({
profile: { name: role },
Expand All @@ -107,12 +108,25 @@ describe('Thread lifecycle', () => {
verified: new Date(),
};
}),
groups: [{ id: threadGroupId }, { id: commentGroupId }],
groups: [
{ id: threadGroupId },
{ id: commentGroupId },
{ id: emptyGroupId },
],
topics: [
{
name: 'topic with permissions',
group_ids: [threadGroupId, commentGroupId],
weighted_voting: TopicWeightedVoting.Stake,
},
{
name: 'topic without thread permissions',
group_ids: [emptyGroupId],
},
{
name: 'topic without groups',
group_ids: [],
},
],
CommunityStakes: [
{
Expand All @@ -138,6 +152,11 @@ describe('Thread lifecycle', () => {
topic_id: _community?.topics?.[0]?.id || 0,
allowed_actions: [schemas.PermissionEnum.CREATE_COMMENT],
});
await seed('GroupPermission', {
group_id: emptyGroupId,
topic_id: _community?.topics?.[1]?.id || 0,
allowed_actions: [],
});

community = _community!;
roles.forEach((role) => {
Expand Down Expand Up @@ -282,6 +301,32 @@ describe('Thread lifecycle', () => {
});
});

describe('topic permissions', () => {
test('should create thread in topic with no permissions', async () => {
const topic_id = community!.topics!.at(2)!.id!; // no groups
const thread = await command(CreateThread(), {
actor: actors.member,
payload: await signCreateThread(actors.member.address!, {
...payload,
topic_id,
}),
});
expect(thread?.topic_id).to.equal(topic_id);
});

test('should throw error when actor is not member of group with permission', async () => {
await expect(
command(CreateThread(), {
actor: actors.nonmember,
payload: await signCreateThread(actors.nonmember.address!, {
...payload,
topic_id: community!.topics!.at(1)!.id!,
}),
}),
).rejects.toThrowError(NonMember);
});
});

describe('updates', () => {
test('should patch content', async () => {
const payloadContent = {
Expand Down Expand Up @@ -483,6 +528,18 @@ describe('Thread lifecycle', () => {
}),
).rejects.toThrowError('Must be admin, moderator, or author');
});

test('should fail when collaborator not found', async () => {
await expect(
command(UpdateThread(), {
actor: actors.nonmember,
payload: {
thread_id: thread.id!,
title: 'new title',
},
}),
).rejects.toThrowError(InvalidActor);
});
});

describe('deletes', () => {
Expand Down
Loading