From 663d9016dd7226f0da55c1cef1048f4fa8ef02d3 Mon Sep 17 00:00:00 2001 From: iamacook Date: Sat, 27 Jul 2024 20:40:47 +0200 Subject: [PATCH] Add log and test coverage --- .../notifications.repository.v2.ts | 34 ++-- src/routes/hooks/hooks-notifications.spec.ts | 178 +++++++++++++++++- 2 files changed, 194 insertions(+), 18 deletions(-) diff --git a/src/domain/notifications/notifications.repository.v2.ts b/src/domain/notifications/notifications.repository.v2.ts index 62ef027d57..d38c575663 100644 --- a/src/domain/notifications/notifications.repository.v2.ts +++ b/src/domain/notifications/notifications.repository.v2.ts @@ -93,33 +93,35 @@ export class NotificationsRepositoryV2 implements INotificationsRepositoryV2 { // Only allow owners or delegates to subscribe to notifications // We don't Promise.all getSafe/getDelegates to prevent unnecessary calls for (const safeToSubscribe of args.safes) { - const safe = await this.safeRepository - .getSafe({ - chainId: safeToSubscribe.chainId, - address: safeToSubscribe.address, - }) - .catch(() => null); + const safe = await this.safeRepository.getSafe({ + chainId: safeToSubscribe.chainId, + address: safeToSubscribe.address, + }); const isOwner = !!safe?.owners.includes(args.account); if (isOwner) { continue; } - const delegates = await this.delegatesRepository - .getDelegates({ - chainId: safeToSubscribe.chainId, - safeAddress: safeToSubscribe.address, - delegate: args.account, - }) - .catch(() => null); + const delegates = await this.delegatesRepository.getDelegates({ + chainId: safeToSubscribe.chainId, + safeAddress: safeToSubscribe.address, + delegate: args.account, + }); - const isDelegate = !!delegates?.results.some( - ({ delegate }) => delegate === args.account, - ); + const isDelegate = !!delegates?.results.some((delegate) => { + return ( + delegate.delegate === args.account && + delegate.safe === safeToSubscribe.address + ); + }); if (isDelegate) { continue; } + this.loggingService.info( + `Non-owner/delegate ${args.account} tried to subscribe to Safe ${safeToSubscribe.address}`, + ); throw new UnauthorizedException(); } diff --git a/src/routes/hooks/hooks-notifications.spec.ts b/src/routes/hooks/hooks-notifications.spec.ts index c8253eccac..3724761851 100644 --- a/src/routes/hooks/hooks-notifications.spec.ts +++ b/src/routes/hooks/hooks-notifications.spec.ts @@ -474,11 +474,11 @@ describe('Post Hook Events for Notifications (Unit)', () => { }); } else if ( url === - `${chain.transactionService}/api/v1/safes/${event.address}/multisig-transactions/` + `${chain.transactionService}/api/v1/multisig-transactions/${event.safeTxHash}/` ) { return Promise.resolve({ status: 200, - data: pageBuilder().with('results', [multisigTransaction]).build(), + data: multisigTransaction, }); } else { return Promise.reject(`No matching rule for url: ${url}`); @@ -494,6 +494,93 @@ describe('Post Hook Events for Notifications (Unit)', () => { expect(pushNotificationsApi.enqueueNotification).not.toHaveBeenCalled(); }); + it("should only enqueue PENDING_MULTISIG_TRANSACTION event notifications for those that haven't signed", async () => { + const event = pendingTransactionEventBuilder().build(); + const chain = chainBuilder().with('chainId', event.chainId).build(); + const owners = [ + getAddress(faker.finance.ethereumAddress()), + getAddress(faker.finance.ethereumAddress()), + getAddress(faker.finance.ethereumAddress()), + getAddress(faker.finance.ethereumAddress()), + getAddress(faker.finance.ethereumAddress()), + ]; + const safe = safeBuilder() + .with('address', event.address) + .with('threshold', faker.number.int({ min: 2 })) + .with('owners', owners) + .build(); + const subscribers = owners.map((owner) => ({ + subscriber: owner, + deviceUuid: faker.string.uuid() as Uuid, + cloudMessagingToken: faker.string.alphanumeric(), + })); + notificationsDatasource.getSubscribersBySafe.mockResolvedValue(subscribers); + const confirmations = faker.helpers + .arrayElements(owners, { min: 1, max: owners.length - 1 }) + .map((owner) => { + return confirmationBuilder().with('owner', owner).build(); + }); + const multisigTransaction = multisigTransactionBuilder() + .with('safe', event.address) + .with('confirmations', confirmations) + .build(); + + networkService.get.mockImplementation(({ url }) => { + if (url === `${safeConfigUrl}/api/v1/chains/${event.chainId}`) { + return Promise.resolve({ + data: chain, + status: 200, + }); + } else if ( + url === `${chain.transactionService}/api/v1/safes/${event.address}` + ) { + return Promise.resolve({ + status: 200, + data: safe, + }); + } else if ( + url === + `${chain.transactionService}/api/v1/multisig-transactions/${event.safeTxHash}/` + ) { + return Promise.resolve({ + status: 200, + data: multisigTransaction, + }); + } else { + return Promise.reject(`No matching rule for url: ${url}`); + } + }); + + await request(app.getHttpServer()) + .post(`/hooks/events`) + .set('Authorization', `Basic ${authToken}`) + .send(event) + .expect(202); + + expect(pushNotificationsApi.enqueueNotification).toHaveBeenCalledTimes( + subscribers.length - confirmations.length, + ); + expect(pushNotificationsApi.enqueueNotification.mock.calls).toStrictEqual( + expect.arrayContaining( + subscribers + .filter((subscriber) => { + return confirmations.every((confirmation) => { + return confirmation.owner !== subscriber.subscriber; + }); + }) + .map((subscriber) => [ + subscriber.cloudMessagingToken, + { + data: { + ...event, + type: 'CONFIRMATION_REQUEST', + }, + }, + ]), + ), + ); + }); + it("should enqueue MESSAGE_CONFIRMATION_REQUEST event notifications if the Safe has a threshold > 1 and the subscriber hasn't yet signed", async () => { const event = messageCreatedEventBuilder().build(); const chain = chainBuilder().with('chainId', event.chainId).build(); @@ -676,6 +763,93 @@ describe('Post Hook Events for Notifications (Unit)', () => { expect(pushNotificationsApi.enqueueNotification).not.toHaveBeenCalled(); }); + it("should only enqueue MESSAGE_CONFIRMATION_REQUEST event notifications for those that haven't signed", async () => { + const event = messageCreatedEventBuilder().build(); + const chain = chainBuilder().with('chainId', event.chainId).build(); + const owners = [ + getAddress(faker.finance.ethereumAddress()), + getAddress(faker.finance.ethereumAddress()), + getAddress(faker.finance.ethereumAddress()), + getAddress(faker.finance.ethereumAddress()), + getAddress(faker.finance.ethereumAddress()), + ]; + const safe = safeBuilder() + .with('address', event.address) + .with('threshold', faker.number.int({ min: 2 })) + .with('owners', owners) + .build(); + const subscribers = owners.map((owner) => ({ + subscriber: owner, + deviceUuid: faker.string.uuid() as Uuid, + cloudMessagingToken: faker.string.alphanumeric(), + })); + notificationsDatasource.getSubscribersBySafe.mockResolvedValue(subscribers); + const confirmations = faker.helpers + .arrayElements(owners, { min: 1, max: owners.length - 1 }) + .map((owner) => { + return messageConfirmationBuilder().with('owner', owner).build(); + }); + const message = messageBuilder() + .with('messageHash', event.messageHash as `0x${string}`) + .with('confirmations', confirmations) + .build(); + + networkService.get.mockImplementation(({ url }) => { + if (url === `${safeConfigUrl}/api/v1/chains/${event.chainId}`) { + return Promise.resolve({ + data: chain, + status: 200, + }); + } else if ( + url === `${chain.transactionService}/api/v1/safes/${event.address}` + ) { + return Promise.resolve({ + status: 200, + data: safe, + }); + } else if ( + url === + `${chain.transactionService}/api/v1/messages/${event.messageHash}` + ) { + return Promise.resolve({ + status: 200, + data: message, + }); + } else { + return Promise.reject(`No matching rule for url: ${url}`); + } + }); + + await request(app.getHttpServer()) + .post(`/hooks/events`) + .set('Authorization', `Basic ${authToken}`) + .send(event) + .expect(202); + + expect(pushNotificationsApi.enqueueNotification).toHaveBeenCalledTimes( + subscribers.length - confirmations.length, + ); + expect(pushNotificationsApi.enqueueNotification.mock.calls).toStrictEqual( + expect.arrayContaining( + subscribers + .filter((subscriber) => { + return confirmations.every((confirmation) => { + return confirmation.owner !== subscriber.subscriber; + }); + }) + .map((subscriber) => [ + subscriber.cloudMessagingToken, + { + data: { + ...event, + type: 'MESSAGE_CONFIRMATION_REQUEST', + }, + }, + ]), + ), + ); + }); + it('should cleanup unregistered tokens', async () => { // Events that are notified "as is" for simplicity const event = faker.helpers.arrayElement([