From 9867bb6a6f7397d6fc8ce9ec78176076d18d5cf0 Mon Sep 17 00:00:00 2001 From: elweyn Date: Tue, 5 Jul 2022 09:21:29 +0200 Subject: [PATCH 1/8] Add RIGHT for DELETE_CONTRIBUTION. --- backend/src/auth/RIGHTS.ts | 1 + backend/src/auth/ROLES.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/backend/src/auth/RIGHTS.ts b/backend/src/auth/RIGHTS.ts index c10fc96de..abe600c99 100644 --- a/backend/src/auth/RIGHTS.ts +++ b/backend/src/auth/RIGHTS.ts @@ -26,6 +26,7 @@ export enum RIGHTS { LIST_TRANSACTION_LINKS = 'LIST_TRANSACTION_LINKS', GDT_BALANCE = 'GDT_BALANCE', CREATE_CONTRIBUTION = 'CREATE_CONTRIBUTION', + DELETE_CONTRIBUTION = 'DELETE_CONTRIBUTION', // Admin SEARCH_USERS = 'SEARCH_USERS', SET_USER_ROLE = 'SET_USER_ROLE', diff --git a/backend/src/auth/ROLES.ts b/backend/src/auth/ROLES.ts index 2d9ac2deb..539158986 100644 --- a/backend/src/auth/ROLES.ts +++ b/backend/src/auth/ROLES.ts @@ -24,6 +24,7 @@ export const ROLE_USER = new Role('user', [ RIGHTS.LIST_TRANSACTION_LINKS, RIGHTS.GDT_BALANCE, RIGHTS.CREATE_CONTRIBUTION, + RIGHTS.DELETE_CONTRIBUTION, ]) export const ROLE_ADMIN = new Role('admin', Object.values(RIGHTS)) // all rights From 62e7abe76efd5974f7840fbf8f881e28f0fa73ea Mon Sep 17 00:00:00 2001 From: elweyn Date: Tue, 5 Jul 2022 09:22:32 +0200 Subject: [PATCH 2/8] Add mutation to soft delete a contribution. --- .../src/graphql/resolver/ContributionResolver.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/ContributionResolver.ts b/backend/src/graphql/resolver/ContributionResolver.ts index 924108f87..e75471c08 100644 --- a/backend/src/graphql/resolver/ContributionResolver.ts +++ b/backend/src/graphql/resolver/ContributionResolver.ts @@ -2,7 +2,7 @@ import { RIGHTS } from '@/auth/RIGHTS' import { Context, getUser } from '@/server/context' import { backendLogger as logger } from '@/server/logger' import { Contribution } from '@entity/Contribution' -import { Args, Authorized, Ctx, Mutation, Resolver } from 'type-graphql' +import { Arg, Args, Authorized, Ctx, Int, Mutation, Resolver } from 'type-graphql' import ContributionArgs from '../arg/ContributionArgs' import { UnconfirmedContribution } from '../model/UnconfirmedContribution' import { isContributionValid, getUserCreation } from './util/isContributionValid' @@ -32,4 +32,15 @@ export class ContributionResolver { await Contribution.save(contribution) return new UnconfirmedContribution(contribution, user, creations) } + + @Authorized([RIGHTS.DELETE_CONTRIBUTION]) + @Mutation(() => Boolean) + async adminDeleteContribution(@Arg('id', () => Int) id: number): Promise { + const contribution = await Contribution.findOne(id) + if (!contribution) { + throw new Error('Contribution not found for given id.') + } + const res = await contribution.softRemove() + return !!res + } } From 9bd57109333274928ed6056316fbe2cd0d9588ff Mon Sep 17 00:00:00 2001 From: elweyn Date: Tue, 5 Jul 2022 09:37:41 +0200 Subject: [PATCH 3/8] Add check if user is owner of the contribution before deleting. --- backend/src/graphql/resolver/ContributionResolver.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/backend/src/graphql/resolver/ContributionResolver.ts b/backend/src/graphql/resolver/ContributionResolver.ts index e75471c08..bdd8e74a4 100644 --- a/backend/src/graphql/resolver/ContributionResolver.ts +++ b/backend/src/graphql/resolver/ContributionResolver.ts @@ -35,11 +35,18 @@ export class ContributionResolver { @Authorized([RIGHTS.DELETE_CONTRIBUTION]) @Mutation(() => Boolean) - async adminDeleteContribution(@Arg('id', () => Int) id: number): Promise { + async adminDeleteContribution( + @Arg('id', () => Int) id: number, + @Ctx() context: Context, + ): Promise { + const user = getUser(context) const contribution = await Contribution.findOne(id) if (!contribution) { throw new Error('Contribution not found for given id.') } + if (contribution.userId !== user.id) { + throw new Error('Can not delete contribution of another user') + } const res = await contribution.softRemove() return !!res } From da3574edb81aa93a362aa61a69ff5cbd459fe5ba Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 12:35:42 +0200 Subject: [PATCH 4/8] Fix merge conflict and reviewed name changes. --- .../graphql/resolver/ContributionResolver.ts | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/backend/src/graphql/resolver/ContributionResolver.ts b/backend/src/graphql/resolver/ContributionResolver.ts index 20eeaa199..90ec6b5a6 100644 --- a/backend/src/graphql/resolver/ContributionResolver.ts +++ b/backend/src/graphql/resolver/ContributionResolver.ts @@ -1,13 +1,6 @@ import { RIGHTS } from '@/auth/RIGHTS' import { Context, getUser } from '@/server/context' import { backendLogger as logger } from '@/server/logger' -<<<<<<< HEAD -import { Contribution } from '@entity/Contribution' -import { Arg, Args, Authorized, Ctx, Int, Mutation, Resolver } from 'type-graphql' -import ContributionArgs from '../arg/ContributionArgs' -import { UnconfirmedContribution } from '../model/UnconfirmedContribution' -import { validateContribution, getUserCreation } from './util/creations' -======= import { Contribution as dbContribution } from '@entity/Contribution' import { Arg, Args, Authorized, Ctx, Int, Mutation, Query, Resolver } from 'type-graphql' import { FindOperator, IsNull } from '@dbTools/typeorm' @@ -18,7 +11,6 @@ import { Contribution, ContributionListResult } from '@model/Contribution' import { UnconfirmedContribution } from '@model/UnconfirmedContribution' import { User } from '@model/User' import { validateContribution, getUserCreation, updateCreations } from './util/creations' ->>>>>>> master @Resolver() export class ContributionResolver { @@ -46,15 +38,14 @@ export class ContributionResolver { return new UnconfirmedContribution(contribution, user, creations) } -<<<<<<< HEAD @Authorized([RIGHTS.DELETE_CONTRIBUTION]) @Mutation(() => Boolean) - async adminDeleteContribution( + async deleteContribution( @Arg('id', () => Int) id: number, @Ctx() context: Context, ): Promise { const user = getUser(context) - const contribution = await Contribution.findOne(id) + const contribution = await dbContribution.findOne(id) if (!contribution) { throw new Error('Contribution not found for given id.') } @@ -63,7 +54,8 @@ export class ContributionResolver { } const res = await contribution.softRemove() return !!res -======= + } + @Authorized([RIGHTS.LIST_CONTRIBUTIONS]) @Query(() => [Contribution]) async listContributions( @@ -147,6 +139,5 @@ export class ContributionResolver { dbContribution.save(contributionToUpdate) return new UnconfirmedContribution(contributionToUpdate, user, creations) ->>>>>>> master } } From 4c33d3784b4dfcbe13ad0d2054c0e3d3863f904d Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 13:07:44 +0200 Subject: [PATCH 5/8] Added mutation for deleteContribution --- backend/src/seeds/graphql/mutations.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/backend/src/seeds/graphql/mutations.ts b/backend/src/seeds/graphql/mutations.ts index 4e7fa8a90..bf898bd7d 100644 --- a/backend/src/seeds/graphql/mutations.ts +++ b/backend/src/seeds/graphql/mutations.ts @@ -255,3 +255,9 @@ export const updateContribution = gql` } } ` + +export const deleteContribution = gql` + mutation ($id: Int!) { + deleteContribution(id: $id) + } +` From e84061c412a9ba5ecb3b7f0c35dff2ba8702c35d Mon Sep 17 00:00:00 2001 From: elweyn Date: Mon, 18 Jul 2022 13:08:27 +0200 Subject: [PATCH 6/8] Added tests for deleteContribution --- .../resolver/ContributionResolver.test.ts | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/backend/src/graphql/resolver/ContributionResolver.test.ts b/backend/src/graphql/resolver/ContributionResolver.test.ts index e6478ffc2..37cb2c645 100644 --- a/backend/src/graphql/resolver/ContributionResolver.test.ts +++ b/backend/src/graphql/resolver/ContributionResolver.test.ts @@ -5,6 +5,7 @@ import { bibiBloxberg } from '@/seeds/users/bibi-bloxberg' import { adminUpdateContribution, createContribution, + deleteContribution, updateContribution, } from '@/seeds/graphql/mutations' import { listAllContributions, listContributions, login } from '@/seeds/graphql/queries' @@ -481,6 +482,11 @@ describe('ContributionResolver', () => { }) }) + afterAll(async () => { + await cleanDB() + resetToken() + }) + it('returns allCreation', async () => { await expect( query({ @@ -516,4 +522,98 @@ describe('ContributionResolver', () => { }) }) }) + + describe('deleteContribution', () => { + describe('unauthenticated', () => { + it('returns an error', async () => { + await expect( + query({ + query: deleteContribution, + variables: { + id: -1, + }, + }), + ).resolves.toEqual( + expect.objectContaining({ + errors: [new GraphQLError('401 Unauthorized')], + }), + ) + }) + }) + + describe('authenticated', () => { + beforeAll(async () => { + await userFactory(testEnv, bibiBloxberg) + await userFactory(testEnv, peterLustig) + await query({ + query: login, + variables: { email: 'bibi@bloxberg.de', password: 'Aa12345_' }, + }) + result = await mutate({ + mutation: createContribution, + variables: { + amount: 100.0, + memo: 'Test env contribution', + creationDate: new Date().toString(), + }, + }) + }) + + afterAll(async () => { + await cleanDB() + resetToken() + }) + + describe('wrong contribution id', () => { + it('returns an error', async () => { + await expect( + query({ + query: deleteContribution, + variables: { + id: -1, + }, + }), + ).resolves.toEqual( + expect.objectContaining({ + errors: [new GraphQLError('Contribution not found for given id.')], + }), + ) + }) + }) + + describe('other user sends a deleteContribtuion', () => { + it('returns an error', async () => { + await query({ + query: login, + variables: { email: 'peter@lustig.de', password: 'Aa12345_' }, + }) + await expect( + query({ + query: deleteContribution, + variables: { + id: result.data.createContribution.id, + }, + }), + ).resolves.toEqual( + expect.objectContaining({ + errors: [new GraphQLError('Can not delete contribution of another user')], + }), + ) + }) + }) + + describe('User deletes own contribution', () => { + it('deletes successfully', async () => { + await expect( + query({ + query: deleteContribution, + variables: { + id: result.data.createContribution.id, + }, + }), + ).resolves.toBeTruthy() + }) + }) + }) + }) }) From b4a4a9e12699ce49ba77bafe927a0f460b874d09 Mon Sep 17 00:00:00 2001 From: elweyn Date: Tue, 19 Jul 2022 13:09:03 +0200 Subject: [PATCH 7/8] Throw error if confirmedAt is present. --- backend/src/graphql/resolver/ContributionResolver.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/src/graphql/resolver/ContributionResolver.ts b/backend/src/graphql/resolver/ContributionResolver.ts index 8ee994989..ef4467a71 100644 --- a/backend/src/graphql/resolver/ContributionResolver.ts +++ b/backend/src/graphql/resolver/ContributionResolver.ts @@ -52,6 +52,9 @@ export class ContributionResolver { if (contribution.userId !== user.id) { throw new Error('Can not delete contribution of another user') } + if (contribution.confirmedAt) { + throw new Error('A confirmed contribution can not be deleted') + } const res = await contribution.softRemove() return !!res } From ccc16b4083acb5c2607af5e20b7dd58c92d27bd7 Mon Sep 17 00:00:00 2001 From: elweyn Date: Tue, 19 Jul 2022 13:09:44 +0200 Subject: [PATCH 8/8] Test that an already confirmed contribution can not be deleted. --- .../resolver/ContributionResolver.test.ts | 44 ++++++++++++++++--- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/backend/src/graphql/resolver/ContributionResolver.test.ts b/backend/src/graphql/resolver/ContributionResolver.test.ts index 0fcd0f5af..b584624c2 100644 --- a/backend/src/graphql/resolver/ContributionResolver.test.ts +++ b/backend/src/graphql/resolver/ContributionResolver.test.ts @@ -4,6 +4,7 @@ import { bibiBloxberg } from '@/seeds/users/bibi-bloxberg' import { adminUpdateContribution, + confirmContribution, createContribution, deleteContribution, updateContribution, @@ -573,8 +574,8 @@ describe('ContributionResolver', () => { describe('wrong contribution id', () => { it('returns an error', async () => { await expect( - query({ - query: deleteContribution, + mutate({ + mutation: deleteContribution, variables: { id: -1, }, @@ -594,8 +595,8 @@ describe('ContributionResolver', () => { variables: { email: 'peter@lustig.de', password: 'Aa12345_' }, }) await expect( - query({ - query: deleteContribution, + mutate({ + mutation: deleteContribution, variables: { id: result.data.createContribution.id, }, @@ -611,8 +612,8 @@ describe('ContributionResolver', () => { describe('User deletes own contribution', () => { it('deletes successfully', async () => { await expect( - query({ - query: deleteContribution, + mutate({ + mutation: deleteContribution, variables: { id: result.data.createContribution.id, }, @@ -620,6 +621,37 @@ describe('ContributionResolver', () => { ).resolves.toBeTruthy() }) }) + + describe('User deletes already confirmed contribution', () => { + it('throws an error', async () => { + await query({ + query: login, + variables: { email: 'peter@lustig.de', password: 'Aa12345_' }, + }) + await mutate({ + mutation: confirmContribution, + variables: { + id: result.data.createContribution.id, + }, + }) + await query({ + query: login, + variables: { email: 'bibi@bloxberg.de', password: 'Aa12345_' }, + }) + await expect( + mutate({ + mutation: deleteContribution, + variables: { + id: result.data.createContribution.id, + }, + }), + ).resolves.toEqual( + expect.objectContaining({ + errors: [new GraphQLError('A confirmed contribution can not be deleted')], + }), + ) + }) + }) }) }) })