diff --git a/libs/model/src/comment/DeleteComment.command.ts b/libs/model/src/comment/DeleteComment.command.ts new file mode 100644 index 00000000000..e2fdb2096c9 --- /dev/null +++ b/libs/model/src/comment/DeleteComment.command.ts @@ -0,0 +1,52 @@ +import { InvalidActor, type Command } from '@hicommonwealth/core'; +import * as schemas from '@hicommonwealth/schemas'; +import { models } from '../database'; +import { isAuthorized, type AuthContext } from '../middleware'; +import { mustBeAuthorized, mustExist } from '../middleware/guards'; + +export function DeleteComment(): Command< + typeof schemas.DeleteComment, + AuthContext +> { + return { + ...schemas.DeleteComment, + auth: [isAuthorized({})], + body: async ({ actor, payload, auth }) => { + const { address } = mustBeAuthorized(actor, auth); + const { comment_id, message_id } = payload; + + const comment = await models.Comment.findOne({ + where: message_id + ? { discord_meta: { message_id } } + : { id: comment_id }, + include: [ + { + model: models.Thread, + attributes: ['community_id'], + required: true, + }, + ], + }); + mustExist('Comment', comment); + + if (comment.address_id !== address!.id && address.role === 'member') + throw new InvalidActor(actor, 'Not authorized author'); + + // == mutation transaction boundary == + await models.sequelize.transaction(async (transaction) => { + await models.CommentSubscription.destroy({ + where: { comment_id: comment.id }, + transaction, + }); + await comment.destroy({ transaction }); + }); + // == end of transaction boundary == + + return { + comment_id: comment.id!, + canvas_signed_data: comment.canvas_signed_data, + canvas_msg_id: comment.canvas_msg_id, + }; + }, + }; +} diff --git a/libs/model/src/comment/index.ts b/libs/model/src/comment/index.ts index ed1191b79b9..a61c205b4f5 100644 --- a/libs/model/src/comment/index.ts +++ b/libs/model/src/comment/index.ts @@ -1,5 +1,6 @@ export * from './CreateComment.command'; export * from './CreateCommentReaction.command'; +export * from './DeleteComment.command'; export * from './GetComments.query'; export * from './SearchComments.query'; export * from './UpdateComment.command'; diff --git a/libs/model/src/middleware/authorization.ts b/libs/model/src/middleware/authorization.ts index 6dbb1149683..42d4de2603e 100644 --- a/libs/model/src/middleware/authorization.ts +++ b/libs/model/src/middleware/authorization.ts @@ -250,16 +250,19 @@ export const isSuperAdmin: AuthHandler = async (ctx) => { * * @param roles specific community roles - all by default * @param action specific group permission action + * @param author when true, rejects members that are not the author * @param collaborators authorize thread collaborators * @throws InvalidActor when not authorized */ export function isAuthorized({ roles = ['admin', 'moderator', 'member'], action, + author = false, collaborators = false, }: { roles?: Role[]; action?: GroupPermissionAction; + author?: boolean; collaborators?: boolean; }): AuthHandler { return async (ctx) => { @@ -292,7 +295,9 @@ export function isAuthorized({ throw new InvalidActor(ctx.actor, 'Not authorized collaborator'); } + if (author) throw new InvalidActor(ctx.actor, 'Not authorized member'); + // at this point, the address is either a moderator or member - // without any action or collaboration requirements + // without any security requirements for action, author, or collaboration }; } diff --git a/libs/model/test/thread/thread-lifecycle.spec.ts b/libs/model/test/thread/thread-lifecycle.spec.ts index 43c36f044fa..f6250252242 100644 --- a/libs/model/test/thread/thread-lifecycle.spec.ts +++ b/libs/model/test/thread/thread-lifecycle.spec.ts @@ -7,6 +7,7 @@ const getNamespaceBalanceStub = sinon.stub( import { Actor, + InvalidActor, InvalidInput, InvalidState, command, @@ -28,6 +29,7 @@ import { CreateComment, CreateCommentErrors, CreateCommentReaction, + DeleteComment, MAX_COMMENT_DEPTH, UpdateComment, } from '../../src/comment'; @@ -590,6 +592,59 @@ describe('Thread lifecycle', () => { }), ).rejects.toThrowError(InvalidInput); }); + + it('should delete a comment as author', async () => { + const text = 'to be deleted'; + const tbd = await command(CreateComment(), { + actor: actors.member, + payload: { + thread_id: thread.id!, + text, + }, + }); + expect(tbd).to.include({ + thread_id: thread!.id, + text, + community_id: thread!.community_id, + }); + const deleted = await command(DeleteComment(), { + actor: actors.member, + payload: { comment_id: tbd!.id! }, + }); + expect(deleted).to.include({ comment_id: tbd!.id! }); + }); + + it('should delete a comment as admin', async () => { + const text = 'to be deleted'; + const tbd = await command(CreateComment(), { + actor: actors.member, + payload: { + thread_id: thread.id!, + text, + }, + }); + expect(tbd).to.include({ + thread_id: thread!.id, + text, + community_id: thread!.community_id, + }); + const deleted = await command(DeleteComment(), { + actor: actors.admin, + payload: { comment_id: tbd!.id! }, + }); + expect(deleted).to.include({ comment_id: tbd!.id! }); + }); + + it('should throw delete when user is not author', async () => { + await expect( + command(DeleteComment(), { + actor: actors.rejected, + payload: { + comment_id: comment!.id, + }, + }), + ).rejects.toThrowError(InvalidActor); + }); }); describe('thread reaction', () => { diff --git a/libs/schemas/src/commands/comment.schemas.ts b/libs/schemas/src/commands/comment.schemas.ts index 8cf95383b23..c6b1a155a58 100644 --- a/libs/schemas/src/commands/comment.schemas.ts +++ b/libs/schemas/src/commands/comment.schemas.ts @@ -43,3 +43,18 @@ export const CreateCommentReaction = { input: CommentCanvasReaction, output: Reaction.extend({ community_id: z.string() }), }; + +export const DeleteComment = { + input: z.object({ + comment_id: PG_INT, + + // discord integration + thread_id: PG_INT.optional(), + message_id: z.string().optional(), + }), + output: z.object({ + comment_id: PG_INT, + canvas_signed_data: z.string().nullish(), + canvas_msg_id: z.string().nullish(), + }), +}; diff --git a/packages/commonwealth/client/scripts/state/api/comments/deleteComment.ts b/packages/commonwealth/client/scripts/state/api/comments/deleteComment.ts index 10360dbb0fc..c94eaffbf20 100644 --- a/packages/commonwealth/client/scripts/state/api/comments/deleteComment.ts +++ b/packages/commonwealth/client/scripts/state/api/comments/deleteComment.ts @@ -1,61 +1,12 @@ -import { toCanvasSignedDataApiArgs } from '@hicommonwealth/shared'; -import { useMutation, useQueryClient } from '@tanstack/react-query'; -import axios from 'axios'; -import { signDeleteComment } from 'controllers/server/sessions'; +import { useQueryClient } from '@tanstack/react-query'; +import { trpc } from 'client/scripts/utils/trpcClient'; import Comment from 'models/Comment'; import { IUniqueId } from 'models/interfaces'; -import { ApiEndpoints, SERVER_URL } from 'state/api/config'; +import { ApiEndpoints } from 'state/api/config'; import { useAuthModalStore } from '../../ui/modals'; -import { userStore } from '../../ui/user'; import { updateThreadInAllCaches } from '../threads/helpers/cache'; import useFetchCommentsQuery from './fetchComments'; -interface DeleteCommentProps { - address: string; - communityId: string; - commentMsgId: string; - commentId: number; - existingNumberOfComments: number; -} - -const deleteComment = async ({ - address, - communityId, - commentId, - commentMsgId, -}: DeleteCommentProps) => { - const canvasSignedData = await signDeleteComment( - userStore.getState().activeAccount?.address || '', - { - comment_id: commentMsgId, - }, - ); - - await axios.delete(`${SERVER_URL}/comments/${commentId}`, { - data: { - author_community_id: communityId, - address: address, - community_id: communityId, - jwt: userStore.getState().jwt, - ...toCanvasSignedDataApiArgs(canvasSignedData), - }, - }); - - // Important: we render comments in a tree, if the deleted comment is a - // leaf node, remove it, but if it has replies, then preserve it with - // [deleted] msg. - return { - softDeleted: { - id: commentId, - deleted: true, - text: '[deleted]', - plaintext: '[deleted]', - versionHistory: [], - ...toCanvasSignedDataApiArgs(canvasSignedData), - }, - }; -}; - interface UseDeleteCommentMutationProps { communityId: string; threadId: number; @@ -75,18 +26,30 @@ const useDeleteCommentMutation = ({ const { checkForSessionKeyRevalidationErrors } = useAuthModalStore(); - return useMutation({ - mutationFn: deleteComment, + return trpc.comment.deleteComment.useMutation({ onSuccess: async (response) => { + // Important: we render comments in a tree, if the deleted comment is a + // leaf node, remove it, but if it has replies, then preserve it with + // [deleted] msg. + const softDeleted = { + id: response.comment_id, + deleted: true, + text: '[deleted]', + plaintext: '[deleted]', + versionHistory: [], + canvas_signed_data: response.canvas_signed_data, + canvas_msg_id: response.canvas_msg_id, + }; + // find the existing comment index const foundCommentIndex = comments.findIndex( - (x) => x.id === response.softDeleted.id, + (x) => x.id === softDeleted.id, ); if (foundCommentIndex > -1) { const softDeletedComment = Object.assign( { ...comments[foundCommentIndex] }, - { ...response.softDeleted }, + { ...softDeleted }, ); // update fetch comments query state @@ -105,13 +68,11 @@ const useDeleteCommentMutation = ({ communityId, threadId, { - recentComments: [ - { id: response.softDeleted.id }, - ] as Comment[], + recentComments: [{ id: softDeleted.id }] as Comment[], }, 'removeFromExisting', ); - return response; + return softDeleted; }, onError: (error) => checkForSessionKeyRevalidationErrors(error), }); diff --git a/packages/commonwealth/client/scripts/views/pages/discussions/CommentTree/CommentTree.tsx b/packages/commonwealth/client/scripts/views/pages/discussions/CommentTree/CommentTree.tsx index 447176ee54f..35c2a4abe34 100644 --- a/packages/commonwealth/client/scripts/views/pages/discussions/CommentTree/CommentTree.tsx +++ b/packages/commonwealth/client/scripts/views/pages/discussions/CommentTree/CommentTree.tsx @@ -195,19 +195,13 @@ export const CommentTree = ({ buttonHeight: 'sm', onClick: async () => { try { - await deleteComment({ - communityId, - commentId: comment.id, - commentMsgId: comment.canvasMsgId, - address: user.activeAccount?.address || '', - existingNumberOfComments: thread.numberOfComments, - }); + await deleteComment({ comment_id: comment.id }); } catch (err) { if (err instanceof SessionKeyError) { checkForSessionKeyRevalidationErrors(err); return; } - console.error(err.response.data.error || err?.message); + console.error(err.message); notifyError('Failed to delete comment'); } }, diff --git a/packages/commonwealth/server/api/comment.ts b/packages/commonwealth/server/api/comment.ts index bd3334cedd8..a1205b8532c 100644 --- a/packages/commonwealth/server/api/comment.ts +++ b/packages/commonwealth/server/api/comment.ts @@ -18,4 +18,5 @@ export const trpcRouter = trpc.router({ ), searchComments: trpc.query(Comment.SearchComments, trpc.Tag.Comment), getComments: trpc.query(Comment.GetComments, trpc.Tag.Comment), + deleteComment: trpc.command(Comment.DeleteComment, trpc.Tag.Comment), }); diff --git a/packages/commonwealth/server/api/external-router.ts b/packages/commonwealth/server/api/external-router.ts index a63c4b66ffa..76a5c4e8990 100644 --- a/packages/commonwealth/server/api/external-router.ts +++ b/packages/commonwealth/server/api/external-router.ts @@ -15,8 +15,13 @@ const { getMembers, } = community.trpcRouter; const { createThread, updateThread, createThreadReaction } = thread.trpcRouter; -const { createComment, createCommentReaction, updateComment, getComments } = - comment.trpcRouter; +const { + createComment, + createCommentReaction, + updateComment, + getComments, + deleteComment, +} = comment.trpcRouter; const api = { createCommunity, @@ -31,6 +36,7 @@ const api = { createComment, updateComment, createCommentReaction, + deleteComment, }; const PATH = '/api/v1'; diff --git a/packages/commonwealth/server/api/integration-router.ts b/packages/commonwealth/server/api/integration-router.ts index 595a75b1540..ab5ee102083 100644 --- a/packages/commonwealth/server/api/integration-router.ts +++ b/packages/commonwealth/server/api/integration-router.ts @@ -4,7 +4,6 @@ import { RequestHandler, Router, raw } from 'express'; // TODO: remove as we migrate to tRPC commands import DatabaseValidationService from 'server/middleware/databaseValidationService'; -import { deleteBotCommentHandler } from 'server/routes/comments/delete_comment_bot_handler'; import { deleteBotThreadHandler } from 'server/routes/threads/delete_thread_bot_handler'; import { ServerControllers } from 'server/routing/router'; @@ -18,9 +17,6 @@ function build( const isBotUser: RequestHandler = (req, res, next) => { validator.validateBotUser(req, res, next).catch(next); }; - const isAuthor: RequestHandler = (req, res, next) => { - validator.validateAuthor(req, res, next).catch(next); - }; const router = Router(); router.use(express.statsMiddleware); @@ -75,8 +71,7 @@ function build( router.delete( '/bot/comments/:message_id', isBotUser, - isAuthor, - deleteBotCommentHandler.bind(this, controllers), + express.command(Comment.DeleteComment()), ); return router; diff --git a/packages/commonwealth/server/controllers/server_comments_controller.ts b/packages/commonwealth/server/controllers/server_comments_controller.ts index ff03811363d..101d5dca915 100644 --- a/packages/commonwealth/server/controllers/server_comments_controller.ts +++ b/packages/commonwealth/server/controllers/server_comments_controller.ts @@ -1,9 +1,4 @@ import { DB, GlobalActivityCache } from '@hicommonwealth/model'; -import { - DeleteCommentOptions, - DeleteCommentResult, - __deleteComment, -} from './server_comments_methods/delete_comment'; import { SearchCommentsOptions, SearchCommentsResult, @@ -29,14 +24,4 @@ export class ServerCommentsController { ): Promise { return __searchComments.call(this, options); } - - /** - * Deletes a comment. - * - */ - async deleteComment( - options: DeleteCommentOptions, - ): Promise { - return __deleteComment.call(this, options); - } } diff --git a/packages/commonwealth/server/controllers/server_comments_methods/delete_comment.ts b/packages/commonwealth/server/controllers/server_comments_methods/delete_comment.ts deleted file mode 100644 index a98fb81a1cf..00000000000 --- a/packages/commonwealth/server/controllers/server_comments_methods/delete_comment.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { AppError } from '@hicommonwealth/core'; -import { - AddressInstance, - CommentAttributes, - UserInstance, -} from '@hicommonwealth/model'; -import { WhereOptions } from 'sequelize'; -import { validateOwner } from 'server/util/validateOwner'; -import { ServerCommentsController } from '../server_comments_controller'; - -const Errors = { - CommentNotFound: 'Comment not found', - BanError: 'Ban error', - NotOwned: 'Not owned by this user', -}; - -export type DeleteCommentOptions = { - user: UserInstance; - address: AddressInstance; - commentId?: number; - messageId?: string; - commentMsgId?: string; -}; - -export type DeleteCommentResult = void; - -export async function __deleteComment( - this: ServerCommentsController, - { user, address, commentId, messageId, commentMsgId }: DeleteCommentOptions, -): Promise { - const commentWhere: WhereOptions = {}; - if (commentId) { - commentWhere.id = commentId; - } - if (messageId) { - commentWhere.discord_meta = { - message_id: messageId, - }; - } - - const comment = await this.models.Comment.findOne({ - where: commentWhere, - include: [{ model: this.models.Thread, attributes: ['community_id'] }], - }); - const community_id = comment?.Thread?.community_id; - - if (!comment || !community_id) { - throw new AppError(Errors.CommentNotFound); - } - - // if the commentMsgId is given, validate that it is the same as the field on - // the comment to be deleted - if (commentMsgId && comment.canvas_msg_id !== commentMsgId) { - throw new AppError( - `comment.canvas_msg_id (${comment.canvas_msg_id}) !== commentMsgId (${commentMsgId})`, - ); - } - - if (address.is_banned) throw new AppError('Banned User'); - - const isAdminOrOwner = await validateOwner({ - models: this.models, - user, - entity: comment, - communityId: community_id, - allowMod: true, - allowAdmin: true, - allowSuperAdmin: true, - }); - if (!isAdminOrOwner) { - throw new AppError(Errors.NotOwned); - } - - await this.models.sequelize.transaction(async (transaction) => { - // find and delete all associated subscriptions - await this.models.CommentSubscription.destroy({ - where: { - comment_id: comment.id!, - }, - transaction, - }); - - // actually delete - await comment.destroy({ transaction }); - }); -} diff --git a/packages/commonwealth/server/routes/comments/delete_comment_bot_handler.ts b/packages/commonwealth/server/routes/comments/delete_comment_bot_handler.ts deleted file mode 100644 index a9fabd69c4b..00000000000 --- a/packages/commonwealth/server/routes/comments/delete_comment_bot_handler.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { CommentAttributes } from '@hicommonwealth/model'; -import { ServerControllers } from '../../routing/router'; -import { TypedRequestParams, TypedResponse, success } from '../../types'; - -type DeleteBotCommentRequestParams = { - message_id: string; -}; - -type DeleteCommentResponse = CommentAttributes; - -export const deleteBotCommentHandler = async ( - controllers: ServerControllers, - req: TypedRequestParams, - res: TypedResponse, -) => { - const { user, address } = req; - const { message_id } = req.params; - - await controllers.comments.deleteComment({ - // @ts-expect-error StrictNullChecks - user, - // @ts-expect-error StrictNullChecks - address, - messageId: message_id, // Discord bot only - }); - - return success(res, undefined); -}; diff --git a/packages/commonwealth/server/routes/comments/delete_comment_handler.ts b/packages/commonwealth/server/routes/comments/delete_comment_handler.ts deleted file mode 100644 index 7132b035a8c..00000000000 --- a/packages/commonwealth/server/routes/comments/delete_comment_handler.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { CommentAttributes } from '@hicommonwealth/model'; -import { - fromCanvasSignedDataApiArgs, - hasCanvasSignedDataApiArgs, - verifyDeleteComment, -} from '@hicommonwealth/shared'; -import { DeleteCommentOptions } from 'server/controllers/server_comments_methods/delete_comment'; -import { applyCanvasSignedData } from 'server/federation'; -import { ServerControllers } from '../../routing/router'; -import { TypedRequest, TypedResponse, success } from '../../types'; - -type DeleteCommentRequestBody = { - canvas_signed_data?: string; - canvas_msg_id?: string; -}; -type DeleteCommentRequestParams = { - id: string; -}; -type DeleteCommentResponse = CommentAttributes; - -export const deleteCommentHandler = async ( - controllers: ServerControllers, - req: TypedRequest, - res: TypedResponse, -) => { - const { user, address } = req; - const { id: commentId } = req.params!; - - const commentFields: DeleteCommentOptions = { - // @ts-expect-error StrictNullChecks - user, - // @ts-expect-error StrictNullChecks - address, - commentId: parseInt(commentId, 10), - }; - if (hasCanvasSignedDataApiArgs(req.body)) { - const { canvasSignedData } = fromCanvasSignedDataApiArgs(req.body); - const comment_id = canvasSignedData.actionMessage.payload.args.comment_id; - await verifyDeleteComment(canvasSignedData, { comment_id }); - } - - await controllers.comments.deleteComment(commentFields); - - // publish signed data - if (hasCanvasSignedDataApiArgs(req.body)) { - const { canvasSignedData } = fromCanvasSignedDataApiArgs(req.body); - if (canvasSignedData.actionMessage.payload.args.comment_id !== null) { - await applyCanvasSignedData(canvasSignedData); - } - } - - return success(res, undefined); -}; diff --git a/packages/commonwealth/server/routing/router.ts b/packages/commonwealth/server/routing/router.ts index 70892e5f196..d59f555293a 100644 --- a/packages/commonwealth/server/routing/router.ts +++ b/packages/commonwealth/server/routing/router.ts @@ -90,7 +90,6 @@ import { getNamespaceMetadata } from 'server/routes/communities/get_namespace_me import { config } from '../config'; import { getStatsHandler } from '../routes/admin/get_stats_handler'; import { getCanvasClockHandler } from '../routes/canvas/get_canvas_clock_handler'; -import { deleteCommentHandler } from '../routes/comments/delete_comment_handler'; import { searchCommentsHandler } from '../routes/comments/search_comments_handler'; import { createChainNodeHandler } from '../routes/communities/create_chain_node_handler'; import { deleteCommunityHandler } from '../routes/communities/delete_community_handler'; @@ -406,14 +405,6 @@ function setupRouter( registerRoute(router, 'get', '/profile/v2', getProfileNew.bind(this, models)); // comments - registerRoute( - router, - 'delete', - '/comments/:id', - passport.authenticate('jwt', { session: false }), - databaseValidationService.validateAuthor, - deleteCommentHandler.bind(this, serverControllers), - ); registerRoute( router, 'get', diff --git a/packages/commonwealth/test/integration/api/createComment.spec.ts b/packages/commonwealth/test/integration/api/createComment.spec.ts index 27de34ecdf3..9cbdfd575d1 100644 --- a/packages/commonwealth/test/integration/api/createComment.spec.ts +++ b/packages/commonwealth/test/integration/api/createComment.spec.ts @@ -57,7 +57,8 @@ describe('createComment Integration Tests', () => { }; return await chai .request(server.app) - .del(`/api/comments/${commentId}`) + .post(`/api/v1/DeleteComment`) + .set('address', validRequest.address) .send(validRequest); }; diff --git a/packages/commonwealth/test/unit/server_controllers/server_comments_controller.spec.ts b/packages/commonwealth/test/unit/server_controllers/server_comments_controller.spec.ts index cede0a42f4c..94885065336 100644 --- a/packages/commonwealth/test/unit/server_controllers/server_comments_controller.spec.ts +++ b/packages/commonwealth/test/unit/server_controllers/server_comments_controller.spec.ts @@ -56,56 +56,4 @@ describe('ServerCommentsController', () => { expect(comments.totalResults).to.equal(11); }); }); - - describe('#deleteComment', () => { - test('should delete a comment', async () => { - let didDestroy = false; - const db = { - sequelize: { - query: Promise.resolve([]), - transaction: (callback) => Promise.resolve(callback()), - }, - Address: { - findAll: async () => [{ address_id: 1 }], // used in findOneRole - }, - Comment: { - findOne: async () => ({ - address_id: 1, - Thread: { community_id: 1 }, - destroy: async () => { - didDestroy = true; - }, - }), - update: () => ({}), - }, - CommentSubscription: { - destroy: async () => ({}), - }, - }; - - // @ts-expect-error ignore type - const serverCommentsController = new ServerCommentsController(db); - const user = { - getAddresses: async () => [{ id: 1, verified: true }], - }; - const address = { id: 1 }; - const commentId = 1; - await serverCommentsController.deleteComment({ - // @ts-expect-error ignore type - user, - // @ts-expect-error ignore type - address, - commentId, - }); - expect(didDestroy).to.be.true; - - serverCommentsController.deleteComment({ - // @ts-expect-error ignore type - user, - // @ts-expect-error ignore type - address, - commentId, - }); - }); - }); }); diff --git a/packages/discord-bot/src/discord-consumer/handlers.ts b/packages/discord-bot/src/discord-consumer/handlers.ts index cd9339c152d..e52a4179014 100644 --- a/packages/discord-bot/src/discord-consumer/handlers.ts +++ b/packages/discord-bot/src/discord-consumer/handlers.ts @@ -101,6 +101,9 @@ export async function handleCommentMessages( await axios.delete(`${bot_path}/comments/${message.message_id}`, { data: { ...sharedReqData, + comment_id: 0, // required in command + thread_id: thread.id, // to auth command + message_id: message.message_id, }, }); break;