From af717539b208cb584bcc7f72663627b9fc6c12bd Mon Sep 17 00:00:00 2001 From: samwel141 Date: Tue, 29 Sep 2026 08:00:25 +0300 Subject: [PATCH] fix(transfer): list transfers of every wallet the account has The transfers list filtered on the login wallet alone, so a transfer sent to any other wallet of the account matched nothing. The recipient never saw it, could not accept it, and the sender's tokens sat pending with only the sender able to cancel. The filter now covers every wallet getAllWallets returns, which since #590 is the account's own wallets plus any it manages. The list is seeded with the login wallet before the others are added: knex drops an empty `or`, and the ownership restriction would go with it. Closes Greenstand/treetracker-wallet-app#852 --- __tests__/transfers-second-wallet.spec.js | 84 +++++++++++++++++++++++ server/models/Transfer.js | 29 ++++++-- 2 files changed, 106 insertions(+), 7 deletions(-) create mode 100644 __tests__/transfers-second-wallet.spec.js diff --git a/__tests__/transfers-second-wallet.spec.js b/__tests__/transfers-second-wallet.spec.js new file mode 100644 index 00000000..ef6dc452 --- /dev/null +++ b/__tests__/transfers-second-wallet.spec.js @@ -0,0 +1,84 @@ +require('dotenv').config(); +const request = require('supertest'); +const { expect } = require('chai'); +const server = require('../server/app'); +const seed = require('./seed'); + +describe('Transfers sent to a second wallet of the account', () => { + let bearerTokenA; + let bearerTokenB; + let secondWalletId; + + beforeEach(async () => { + await seed.clear(); + await seed.seed(); + bearerTokenA = seed.wallet.keycloak_account_id; + bearerTokenB = seed.walletB.keycloak_account_id; + + const created = await request(server) + .post('/wallets') + .set('content-type', 'application/json') + .set('Authorization', `Bearer ${bearerTokenA}`) + .send({ wallet: 'walletA-second' }) + .expect(201); + secondWalletId = created.body.id; + + await request(server) + .post('/transfers') + .set('content-type', 'application/json') + .set('Authorization', `Bearer ${bearerTokenB}`) + .send({ + tokens: [seed.tokenB.id], + sender_wallet: seed.walletC.name, + receiver_wallet: 'walletA-second', + }) + .expect(202); + }); + + it('are visible to the account that owns the second wallet', async () => { + const res = await request(server) + .get('/transfers') + .set('Authorization', `Bearer ${bearerTokenA}`) + .expect(200); + + const mine = res.body.transfers.filter( + (t) => t.destination_wallet === 'walletA-second', + ); + expect(mine).lengthOf(1); + expect(mine[0]).to.have.property('state', 'pending'); + }); + + it('can be accepted by that account', async () => { + const listed = await request(server) + .get('/transfers') + .set('Authorization', `Bearer ${bearerTokenA}`) + .expect(200); + + const pending = listed.body.transfers.find( + (t) => t.destination_wallet === 'walletA-second', + ); + + const accepted = await request(server) + .post(`/transfers/${pending.id}/accept`) + .set('content-type', 'application/json') + .set('Authorization', `Bearer ${bearerTokenA}`); + + expect(accepted).to.have.property('statusCode', 200); + expect(accepted.body).to.have.property('state', 'completed'); + }); + + it('does not leak transfers between unrelated accounts', async () => { + const res = await request(server) + .get('/transfers') + .set('Authorization', `Bearer ${seed.walletC.keycloak_account_id}`) + .expect(200); + + const walletIds = [seed.wallet.id, secondWalletId]; + const leaked = res.body.transfers.filter( + (t) => + walletIds.includes(t.source_wallet_id) || + walletIds.includes(t.destination_wallet_id), + ); + expect(leaked).lengthOf(0); + }); +}); diff --git a/server/models/Transfer.js b/server/models/Transfer.js index b7e1c47b..9659e2ef 100644 --- a/server/models/Transfer.js +++ b/server/models/Transfer.js @@ -73,13 +73,28 @@ class Transfer { const filter = { and: [], }; - filter.and.push({ - or: [ - { source_wallet_id: walletLoginId }, - { destination_wallet_id: walletLoginId }, - { originator_wallet_id: walletLoginId }, - ], - }); + const { wallets } = await this._wallet.getAllWallets( + walletLoginId, + undefined, + undefined, + 'created_at', + 'desc', + ); + // Never let this list go empty: knex drops an empty `or` and every + // transfer in the table comes back. + const orConditions = [ + { source_wallet_id: walletLoginId }, + { destination_wallet_id: walletLoginId }, + { originator_wallet_id: walletLoginId }, + ]; + wallets + .filter((wallet) => wallet.id !== walletLoginId) + .forEach((wallet) => { + orConditions.push({ source_wallet_id: wallet.id }); + orConditions.push({ destination_wallet_id: wallet.id }); + orConditions.push({ originator_wallet_id: wallet.id }); + }); + filter.and.push({ or: orConditions }); if (state) { filter.and.push({ state }); }