Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions routes/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,24 @@ import { type Request, type Response, type NextFunction } from 'express'
import { WalletModel } from '../models/wallet'
import { CardModel } from '../models/card'

const MIN_TOP_UP_AMOUNT = 10
const MAX_TOP_UP_AMOUNT = 1000

function parseTopUpAmount (value: unknown): number | null {
const amount = typeof value === 'number' ? value : (typeof value === 'string' && value.trim() !== '' ? Number(value) : NaN)
if (!Number.isInteger(amount) || amount < MIN_TOP_UP_AMOUNT || amount > MAX_TOP_UP_AMOUNT) {
return null
Comment on lines +15 to +16

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Fractional wallet deposits fail after card selection

When balanceControl accepts a fractional amount, parseTopUpAmount rejects it after card selection. Users traverse the payment flow before learning that the accepted deposit cannot complete.

Learn more

The wallet form defines only required, minimum, and maximum validators. Angular therefore treats values such as 10.5 as valid and enables the continue button. The payment page converts the stored value with parseFloat and submits it through choosePayment. The new API integer check then returns 400, despite both client screens allowing the user to proceed.

Example: A user enters 10.5. The deposit button enables, card selection succeeds, and the final request returns “Top-up amount must be a whole number” instead of adding the displayed amount.

Recommended fix: Add an integer validator to balanceControl and display its validation error before navigation. Keep the frontend and API bounds and integer contract synchronized.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}
return amount
}

function isCardExpired (card: CardModel): boolean {
const now = new Date()
const year = now.getFullYear()
const month = now.getMonth() + 1
return card.expYear < year || (card.expYear === year && card.expMonth < month)
}

export function getWalletBalance () {
return async (req: Request, res: Response, next: NextFunction) => {
const wallet = await WalletModel.findOne({ where: { UserId: req.body.UserId } })
Expand All @@ -20,12 +38,17 @@ export function getWalletBalance () {

export function addWalletBalance () {
return async (req: Request, res: Response, next: NextFunction) => {
const cardId = req.body.paymentId
const card = cardId ? await CardModel.findOne({ where: { id: cardId, UserId: req.body.UserId } }) : null
if (card != null) {
const amount = parseTopUpAmount(req.body.balance)
if (amount === null) {
res.status(400).json({ status: 'error', message: `Top-up amount must be a whole number between ${MIN_TOP_UP_AMOUNT} and ${MAX_TOP_UP_AMOUNT}.` })
return
}
const cardId = Number(req.body.paymentId)
const card = Number.isInteger(cardId) && cardId > 0 ? await CardModel.findOne({ where: { id: cardId, UserId: req.body.UserId } }) : null
if (card != null && !isCardExpired(card)) {
try {
await WalletModel.increment({ balance: req.body.balance }, { where: { UserId: req.body.UserId } })
res.status(200).json({ status: 'success', data: req.body.balance })
await WalletModel.increment({ balance: amount }, { where: { UserId: req.body.UserId } })

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟨 Repeated top-ups bypass the credit cap

A card owner can repeat valid addWalletBalance requests without limit. Each request credits uncharged funds, so the wallet remains unbounded.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

res.status(200).json({ status: 'success', data: amount })
} catch {
res.status(404).json({ status: 'error' })
}
Expand Down
37 changes: 37 additions & 0 deletions test/api/wallet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,43 @@ void describe('/api/Wallets', () => {
assert.equal(balanceRes.body.data, 210)
})

void it('PUT charge wallet with negative amount is rejected', async () => {
const res = await request(app)
.put('/rest/wallet/balance')
.set(authHeader)
.send({ balance: -500, paymentId: 2 })
assert.equal(res.status, 400)
})

void it('PUT charge wallet with amount above maximum is rejected', async () => {
const res = await request(app)
.put('/rest/wallet/balance')
.set(authHeader)
.send({ balance: 999999, paymentId: 2 })
assert.equal(res.status, 400)

const balanceRes = await request(app)
.get('/rest/wallet/balance')
.set(authHeader)
assert.equal(balanceRes.body.data, 210)
})

void it('PUT charge wallet with non-numeric amount is rejected', async () => {
const res = await request(app)
.put('/rest/wallet/balance')
.set(authHeader)
.send({ balance: 'lots', paymentId: 2 })
assert.equal(res.status, 400)
})

void it('PUT charge wallet with non-integer amount is rejected', async () => {
const res = await request(app)
.put('/rest/wallet/balance')
.set(authHeader)
.send({ balance: 10.5, paymentId: 2 })
assert.equal(res.status, 400)
})

void it('PUT charge wallet from foreign credit card is forbidden', async () => {
const res = await request(app)
.put('/rest/wallet/balance')
Expand Down
Loading