From 334b9c239d581a711c4dd23cb8d87a21b6ebd3aa Mon Sep 17 00:00:00 2001 From: samwel141 Date: Tue, 29 Sep 2026 07:35:31 +0300 Subject: [PATCH] fix(action-token): promise a count, not named tokens A bundle link resolved tokens the moment it was created and wrote their ids into the link, throwing the requested number away. Nothing in the database marked those tokens, so an ordinary send could take them, and the link then died at claim with a 409 while the sender still held plenty of other tokens. A bundle link now carries the count. The tokens are chosen at claim time from whatever the sender wallet still holds, skipping ones already pending or claimed, so a send can no longer kill a link. Links that name their tokens are unchanged, which keeps the case for sharing specific tokens and keeps already issued links working. Selection happens on the model bound to the open transaction, not through TokenService, which opens a session of its own. Closes #574 Closes #571 --- __tests__/actiontoken-bundle-link.spec.js | 109 ++++++++++++++++++++++ server/models/Token.js | 7 ++ server/services/ActionTokenService.js | 24 +++-- server/services/TransferService.js | 33 +++++-- 4 files changed, 157 insertions(+), 16 deletions(-) create mode 100644 __tests__/actiontoken-bundle-link.spec.js diff --git a/__tests__/actiontoken-bundle-link.spec.js b/__tests__/actiontoken-bundle-link.spec.js new file mode 100644 index 00000000..f237369a --- /dev/null +++ b/__tests__/actiontoken-bundle-link.spec.js @@ -0,0 +1,109 @@ +require('dotenv').config(); +const request = require('supertest'); +const { expect } = require('chai'); +const server = require('../server/app'); +const seed = require('./seed'); + +function decode(actionToken) { + const part = actionToken.split('.')[1]; + return JSON.parse(Buffer.from(part, 'base64url').toString()); +} + +describe('Action token: a bundle link promises a count, not named tokens', () => { + let bearerTokenA; + let bearerTokenB; + + beforeEach(async () => { + await seed.clear(); + await seed.seed(); + bearerTokenA = seed.wallet.keycloak_account_id; + bearerTokenB = seed.walletB.keycloak_account_id; + }); + + async function issueBundleLink(size) { + const res = await request(server) + .post('/action-tokens') + .set('Authorization', `Bearer ${bearerTokenA}`) + .send({ + recipient_email: 'samwel@example.com', + bundle: { bundle_size: size }, + }); + expect(res).to.have.property('statusCode', 201); + return res.body.action_token; + } + + it('carries a count and names no tokens', async () => { + const payload = decode(await issueBundleLink(1)); + + expect(payload).to.have.property('token_count', 1); + expect(payload).to.not.have.property('token_ids'); + }); + + it('survives the sender spending the token it would have named', async () => { + const link = await issueBundleLink(1); + const [spare] = await seed.addTokenToWallet(seed.wallet.id); + + await request(server) + .post('/transfers') + .set('content-type', 'application/json') + .set('Authorization', `Bearer ${bearerTokenA}`) + .send({ + tokens: [seed.token.id], + sender_wallet: seed.wallet.name, + receiver_wallet: seed.walletB.name, + }) + .expect(202); + + const redeemed = await request(server) + .post('/action-tokens/redeem') + .set('Authorization', `Bearer ${bearerTokenB}`) + .send({ action_token: link }); + + expect(redeemed).to.have.property('statusCode', 200); + expect(redeemed.body.parameters.tokens).eql([spare.id]); + }); + + it('refuses when the wallet has dropped below the promised count', async () => { + const link = await issueBundleLink(1); + + await request(server) + .post('/transfers') + .set('content-type', 'application/json') + .set('Authorization', `Bearer ${bearerTokenA}`) + .send({ + tokens: [seed.token.id], + sender_wallet: seed.wallet.name, + receiver_wallet: seed.walletB.name, + }) + .expect(202); + + const redeemed = await request(server) + .post('/action-tokens/redeem') + .set('Authorization', `Bearer ${bearerTokenB}`) + .send({ action_token: link }); + + expect(redeemed).to.have.property('statusCode', 409); + expect(redeemed.body.message).match(/does not have 1 tokens available/); + }); + + it('still claims a link that names its tokens', async () => { + const issued = await request(server) + .post('/action-tokens') + .set('Authorization', `Bearer ${bearerTokenA}`) + .send({ + recipient_email: 'samwel@example.com', + tokens: [seed.token.id], + }) + .expect(201); + + expect(decode(issued.body.action_token)).to.have.property('token_ids'); + + const redeemed = await request(server) + .post('/action-tokens/redeem') + .set('Authorization', `Bearer ${bearerTokenB}`) + .send({ action_token: issued.body.action_token }); + + expect(redeemed).to.have.property('statusCode', 200); + expect(redeemed.body.parameters.tokens).to.include(seed.token.id); + }); +}); diff --git a/server/models/Token.js b/server/models/Token.js index ae52c643..f02577fe 100644 --- a/server/models/Token.js +++ b/server/models/Token.js @@ -125,6 +125,13 @@ class Token { return transactions; } + async getTransferableByOwner(walletId, limit) { + return this._tokenRepository.getByFilter( + { wallet_id: walletId, transfer_pending: false, claim: false }, + { limit }, + ); + } + async getByOwner(walletId, limit, offset) { const tokens = await this._tokenRepository.getByFilter( { wallet_id: walletId }, diff --git a/server/services/ActionTokenService.js b/server/services/ActionTokenService.js index 89ef7e88..73375ec2 100644 --- a/server/services/ActionTokenService.js +++ b/server/services/ActionTokenService.js @@ -4,6 +4,8 @@ */ const jwt = require('jsonwebtoken'); const HttpError = require('../utils/HttpError'); +const Session = require('../infra/database/Session'); +const Token = require('../models/Token'); const TokenService = require('./TokenService'); const TransferService = require('./TransferService'); const WalletService = require('./WalletService'); @@ -30,6 +32,8 @@ const ACTION_TOKEN_SECRET = resolveActionTokenSecret(); class ActionTokenService { constructor() { + this._session = new Session(); + this._token = new Token(this._session); this._tokenService = new TokenService(); this._transferService = new TransferService(); this._walletService = new WalletService(); @@ -80,6 +84,7 @@ class ActionTokenService { keycloakId, ) { let tokenIds; + let tokenCount; let senderWalletId = walletLoginId; if (sender_wallet) { @@ -111,32 +116,30 @@ class ActionTokenService { } tokenIds = resolved.map((token) => token.id); } else { - const resolved = await this._tokenService.getTokens({ - wallet: sender_wallet, - limit: bundle.bundle_size, - offset: 0, - walletLoginId, - }); - if (resolved.length < bundle.bundle_size) { + const available = await this._token.getTransferableByOwner( + senderWalletId, + bundle.bundle_size, + ); + if (available.length < bundle.bundle_size) { throw new HttpError( 409, `Wallet does not have ${bundle.bundle_size} tokens available`, ); } - tokenIds = resolved.map((token) => token.id); + tokenCount = bundle.bundle_size; } const actionToken = ActionTokenService.signActionToken({ sub: recipient_email, sender_wallet_id: senderWalletId, - token_ids: tokenIds, + ...(tokenIds ? { token_ids: tokenIds } : { token_count: tokenCount }), }); const { exp } = ActionTokenService.verifyActionToken(actionToken); return { action_token: actionToken, expires_at: new Date(exp * 1000).toISOString(), - token_count: tokenIds.length, + token_count: tokenIds ? tokenIds.length : tokenCount, }; } @@ -146,6 +149,7 @@ class ActionTokenService { senderWalletId: payload.sender_wallet_id, receiverWalletId: walletLoginId, tokenIds: payload.token_ids, + tokenCount: payload.token_count, }); } } diff --git a/server/services/TransferService.js b/server/services/TransferService.js index f0f8f70a..87074aa2 100644 --- a/server/services/TransferService.js +++ b/server/services/TransferService.js @@ -1,5 +1,6 @@ const Session = require('../infra/database/Session'); const Transfer = require('../models/Transfer'); +const Token = require('../models/Token'); const HttpError = require('../utils/HttpError'); const WalletService = require('./WalletService'); const TokenService = require('./TokenService'); @@ -11,6 +12,7 @@ class TransferService { constructor() { this._session = new Session(); this._transfer = new Transfer(this._session); + this._token = new Token(this._session); this._walletService = new WalletService(); this._eventService = new EventService(); } @@ -441,7 +443,12 @@ class TransferService { // Transfer the tokens promised by a redeemed action token - async redeemActionToken({ senderWalletId, receiverWalletId, tokenIds }) { + async redeemActionToken({ + senderWalletId, + receiverWalletId, + tokenIds, + tokenCount, + }) { try { await this._session.beginTransaction(); @@ -449,10 +456,24 @@ class TransferService { const receiverWallet = await this._walletService.getById(receiverWalletId); - const tokenService = new TokenService(); - const tokens = await Promise.all( - tokenIds.map((id) => tokenService.getById({ id }, true)), - ); + let tokens; + if (tokenIds) { + const tokenService = new TokenService(); + tokens = await Promise.all( + tokenIds.map((id) => tokenService.getById({ id }, true)), + ); + } else { + tokens = await this._token.getTransferableByOwner( + senderWallet.id, + tokenCount, + ); + if (tokens.length < tokenCount) { + throw new HttpError( + 409, + `Wallet does not have ${tokenCount} tokens available`, + ); + } + } const result = await this._transfer.transferActionToken( receiverWallet.id, @@ -464,7 +485,7 @@ class TransferService { const payload = { walletSender: senderWallet.name, walletReceiver: receiverWallet.name, - tokenTransferred: tokenIds, + tokenTransferred: tokens.map((token) => token.id), }; await this._eventService.logEvent({