From eb4a6eca63a5d3c0114e6ed99ba6f9e3a884d636 Mon Sep 17 00:00:00 2001 From: Slavcho Ivanov Date: Wed, 1 Nov 2023 22:00:08 +0200 Subject: [PATCH 1/5] Reduce the cache ttl for public donations and total money collected. The idea of the cache is to help in extreme scenarios when many requests are being fired. One request every 2 seconds should be easy to handle by the backend. --- apps/api/src/donations/donations.controller.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/api/src/donations/donations.controller.ts b/apps/api/src/donations/donations.controller.ts index 200733c51..fed254b41 100644 --- a/apps/api/src/donations/donations.controller.ts +++ b/apps/api/src/donations/donations.controller.ts @@ -30,7 +30,7 @@ import { DonationQueryDto } from '../common/dto/donation-query-dto' import { CancelPaymentIntentDto } from './dto/cancel-payment-intent.dto' import { DonationsApiQuery } from './queries/donations.apiquery' import { PersonService } from '../person/person.service' -import { CacheInterceptor } from '@nestjs/cache-manager' +import { CacheInterceptor, CacheTTL } from '@nestjs/cache-manager' import { UseInterceptors } from '@nestjs/common' @ApiTags('donation') @@ -111,6 +111,7 @@ export class DonationsController { } @Get('money') + @CacheTTL(5 * 1000) @UseInterceptors(CacheInterceptor) @Public() async totalDonatedMoney() { @@ -126,6 +127,7 @@ export class DonationsController { @Get('listPublic') @UseInterceptors(CacheInterceptor) + @CacheTTL(2 * 1000) @Public() @ApiQuery({ name: 'campaignId', required: false, type: String }) @ApiQuery({ name: 'status', required: false, enum: DonationStatus }) From d363cbf426d55ee852c65c02b5b15421c9f5037c Mon Sep 17 00:00:00 2001 From: Slavcho Ivanov Date: Mon, 18 Dec 2023 19:06:45 +0200 Subject: [PATCH 2/5] The expense original filenames are encoded in base64. This allows us to upload files with cyrilic names. But it adds a bit of complexity in the backend. --- apps/api/src/common/files.ts | 10 ++++++++-- apps/api/src/expenses/expenses.controller.ts | 10 +++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/apps/api/src/common/files.ts b/apps/api/src/common/files.ts index e4c429452..6f95900db 100644 --- a/apps/api/src/common/files.ts +++ b/apps/api/src/common/files.ts @@ -39,9 +39,15 @@ export function validateFileType( const allowedExtensions = /txt|json|pdf|jpeg|jpg|png|xml|xlsx|xls|docx/ - const isExtensionSupported = allowedExtensions.test(path.extname(file.originalname).toLowerCase()) + const filename = file.originalname + let ext = path.extname(filename).toLowerCase() + if (ext == '') { + // for the expense files, the original filename is encoded in base64 + ext = path.extname(file.filename).toLowerCase() + } + const isExtensionSupported = allowedExtensions.test(ext) if (!isExtensionSupported) { - return cb(new Error('File extension is not allowed'), false) + return cb(new Error('File extension is not allowed: ' + file.filename), false) } cb(null, true) diff --git a/apps/api/src/expenses/expenses.controller.ts b/apps/api/src/expenses/expenses.controller.ts index 227eb3362..0dd74dc56 100644 --- a/apps/api/src/expenses/expenses.controller.ts +++ b/apps/api/src/expenses/expenses.controller.ts @@ -10,6 +10,7 @@ import { StreamableFile, NotFoundException, UnauthorizedException, + Logger, } from '@nestjs/common' import { AuthenticatedUser, Public, RoleMatchingMode, Roles } from 'nest-keycloak-connect' @@ -71,8 +72,15 @@ export class ExpensesController { @Post(':expenseId/files') @UseInterceptors( FilesInterceptor('file', 5, { - limits: { fileSize: 1024 * 1024 * 10 }, //limit uploaded files to 5 at once and 10MB each + limits: { fileSize: 1024 * 1024 * 30 }, //limit uploaded files to 5 at once and 30MB each fileFilter: (_req: Request, file, cb) => { + try { + // decode the name from base64 + file.filename = Buffer.from(file.originalname, 'base64').toString('utf-8') + } catch { + Logger.error('Error decoding filename from base64: ', file.originalname) + } + validateFileType(file, cb) }, }), From 0f1135e075a45c0d69ccd0b3fd4d61598e288dd0 Mon Sep 17 00:00:00 2001 From: Slavcho Ivanov Date: Tue, 16 Jan 2024 11:33:08 +0200 Subject: [PATCH 3/5] Add support for making a donation invalid. Sometimes we can have a buggy stripe donation and we would like to sort of remove it. This is allowed only if one has special credentials, of course. --- .../donations/donations.controller.spec.ts | 24 ++++++++++++++++++ .../api/src/donations/donations.controller.ts | 14 +++++++++-- apps/api/src/donations/donations.service.ts | 25 +++++++++++++++++++ .../src/lib/roles/team/edit-financials.ts | 3 +++ .../src/lib/roles/team/index.ts | 1 + 5 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 libs/podkrepi-types/src/lib/roles/team/edit-financials.ts diff --git a/apps/api/src/donations/donations.controller.spec.ts b/apps/api/src/donations/donations.controller.spec.ts index 1ff530713..909625621 100644 --- a/apps/api/src/donations/donations.controller.spec.ts +++ b/apps/api/src/donations/donations.controller.spec.ts @@ -297,4 +297,28 @@ describe('DonationsController', () => { reason: 'requested_by_customer', }) }) + + it('should invalidate a donation and update the vault if needed', async () => { + const existingDonation = { ...mockDonation, status: DonationStatus.succeeded } + jest.spyOn(prismaMock, '$transaction').mockImplementation((callback) => callback(prismaMock)) + + prismaMock.donation.findFirstOrThrow.mockResolvedValueOnce(existingDonation) + + await controller.invalidate('123') + + expect(prismaMock.donation.update).toHaveBeenCalledWith({ + where: { id: '123' }, + data: { + status: DonationStatus.invalid, + }, + }) + expect(prismaMock.vault.update).toHaveBeenCalledWith({ + where: { id: existingDonation.targetVaultId }, + data: { + amount: { + decrement: existingDonation.amount, + }, + }, + }) + }) }) diff --git a/apps/api/src/donations/donations.controller.ts b/apps/api/src/donations/donations.controller.ts index 90e004cee..c7e5d0779 100644 --- a/apps/api/src/donations/donations.controller.ts +++ b/apps/api/src/donations/donations.controller.ts @@ -16,7 +16,7 @@ import { import { ApiQuery, ApiTags } from '@nestjs/swagger' import { DonationStatus } from '@prisma/client' import { AuthenticatedUser, Public, RoleMatchingMode, Roles } from 'nest-keycloak-connect' -import { RealmViewSupporters, ViewSupporters } from '@podkrepi-bg/podkrepi-types' +import { RealmViewSupporters, ViewSupporters, EditFinancialsRequests } from '@podkrepi-bg/podkrepi-types' import { isAdmin, KeycloakTokenParsed } from '../auth/keycloak' import { DonationsService } from './donations.service' @@ -260,9 +260,19 @@ export class DonationsController { return this.donationsService.update(id, updatePaymentDto) } + @Post('/invalidate-stripe-payment/:id') + @Roles({ + roles: [EditFinancialsRequests.role], + mode: RoleMatchingMode.ANY, + }) + invalidate(@Param('id') id: string) { + Logger.debug(`Invalidating donation with id ${id}`) + return this.donationsService.invalidate(id) + } + @Post('delete') @Roles({ - roles: [RealmViewSupporters.role, ViewSupporters.role], + roles: [EditFinancialsRequests.role], mode: RoleMatchingMode.ANY, }) delete( diff --git a/apps/api/src/donations/donations.service.ts b/apps/api/src/donations/donations.service.ts index 6160f51de..64ef0ccb3 100644 --- a/apps/api/src/donations/donations.service.ts +++ b/apps/api/src/donations/donations.service.ts @@ -723,6 +723,31 @@ export class DonationsService { } } + async invalidate(id: string) { + try { + await this.prisma.$transaction(async (tx) => { + const donation = await this.getDonationById(id) + + if (donation.status === DonationStatus.succeeded) { + await this.vaultService.decrementVaultAmount(donation.targetVaultId, donation.amount, tx) + } + + await this.prisma.donation.update({ + where: { id }, + data: { + status: DonationStatus.invalid, + }, + }) + }) + } catch (err) { + Logger.warn(err.message || err) + const msg = `Invalidation failed. No Donation found with given ID.` + + Logger.warn(msg) + throw new NotFoundException(msg) + } + } + async getDonationsByUser(keycloakId: string, email?: string) { const donations = await this.prisma.donation.findMany({ where: { diff --git a/libs/podkrepi-types/src/lib/roles/team/edit-financials.ts b/libs/podkrepi-types/src/lib/roles/team/edit-financials.ts new file mode 100644 index 000000000..121a0f1ac --- /dev/null +++ b/libs/podkrepi-types/src/lib/roles/team/edit-financials.ts @@ -0,0 +1,3 @@ +export class EditFinancialsRequests { + static readonly role = 'realm:account-edit-financials-requests' +} diff --git a/libs/podkrepi-types/src/lib/roles/team/index.ts b/libs/podkrepi-types/src/lib/roles/team/index.ts index 2fd851eaa..25ed5f954 100644 --- a/libs/podkrepi-types/src/lib/roles/team/index.ts +++ b/libs/podkrepi-types/src/lib/roles/team/index.ts @@ -1,2 +1,3 @@ export * from './view-supporters' export * from './view-contact-requests' +export * from './edit-financials' From fdd1a8ee25e508d9c978294a51052467c8b68d12 Mon Sep 17 00:00:00 2001 From: Slavcho Ivanov Date: Tue, 16 Jan 2024 12:53:50 +0200 Subject: [PATCH 4/5] Refund payment should also be covered by the account-edit-financials-requests role. --- apps/api/src/donations/donations.controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/src/donations/donations.controller.ts b/apps/api/src/donations/donations.controller.ts index c7e5d0779..8704ff703 100644 --- a/apps/api/src/donations/donations.controller.ts +++ b/apps/api/src/donations/donations.controller.ts @@ -227,7 +227,7 @@ export class DonationsController { @Post('/refund-stripe-payment/:id') @Roles({ - roles: [RealmViewSupporters.role, ViewSupporters.role], + roles: [EditFinancialsRequests.role], mode: RoleMatchingMode.ANY, }) refundStripePaymet(@Param('id') paymentIntentId: string) { From 275dd78f7ebeedc35da6fd345905140b444440ef Mon Sep 17 00:00:00 2001 From: Slavcho Ivanov Date: Wed, 24 Jan 2024 00:48:09 +0200 Subject: [PATCH 5/5] Rename the post /invalidate-stripe-payment/:id to a patch /:id/invalidate to match the REST guidelines better. --- .../api/src/donations/donations.controller.ts | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/apps/api/src/donations/donations.controller.ts b/apps/api/src/donations/donations.controller.ts index 1864c46de..e7bf46fe2 100644 --- a/apps/api/src/donations/donations.controller.ts +++ b/apps/api/src/donations/donations.controller.ts @@ -240,6 +240,16 @@ export class DonationsController { return this.donationsService.createUpdateBankPayment(bankPaymentDto) } + @Patch('/:id/invalidate') + @Roles({ + roles: [EditFinancialsRequests.role], + mode: RoleMatchingMode.ANY, + }) + invalidate(@Param('id') id: string) { + Logger.debug(`Invalidating donation with id ${id}`) + return this.donationsService.invalidate(id) + } + @Patch(':id') @Roles({ roles: [RealmViewSupporters.role, ViewSupporters.role], @@ -251,17 +261,9 @@ export class DonationsController { @Body() updatePaymentDto: UpdatePaymentDto, ) { - return this.donationsService.update(id, updatePaymentDto) - } + Logger.debug(`Updating donation with id ${id}`) - @Post('/invalidate-stripe-payment/:id') - @Roles({ - roles: [EditFinancialsRequests.role], - mode: RoleMatchingMode.ANY, - }) - invalidate(@Param('id') id: string) { - Logger.debug(`Invalidating donation with id ${id}`) - return this.donationsService.invalidate(id) + return this.donationsService.update(id, updatePaymentDto) } @Post('delete')