Skip to content

Commit

Permalink
Add log and test coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
@iamacook
iamacook committed Jul 27, 2024
1 parent 8088fcd commit 663d901
Show file tree
Hide file tree
Showing 2 changed files with 194 additions and 18 deletions.
34 changes: 18 additions & 16 deletions src/domain/notifications/notifications.repository.v2.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
178 changes: 176 additions & 2 deletions src/routes/hooks/hooks-notifications.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand All @@ -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();
Expand Down Expand Up @@ -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([
Expand Down

0 comments on commit 663d901

Please sign in to comment.