Skip to content

Commit c37554a

Browse files
fix(transactions): validate action status allowlist
Amp-Thread-ID: https://ampcode.com/threads/T-019ca093-14b6-767d-af4e-6fa737f1254f Co-authored-by: Amp <amp@ampcode.com>
1 parent 6db680c commit c37554a

2 files changed

Lines changed: 148 additions & 1 deletion

File tree

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest'
2+
import { action } from '../dashboard.transactions.$transactionId.$action'
3+
import { TRANSACTION_STATUS } from '~/models/enum'
4+
import { requireUser } from '~/services/auth.server'
5+
import { getFirstCourse } from '~/models/course'
6+
import { requireCourseAuthor } from '~/utils/permissions'
7+
import {
8+
getTransactionById,
9+
updateTransactionStatus,
10+
} from '~/models/transaction'
11+
import {
12+
activateSubscription,
13+
deactivateSubscription,
14+
} from '~/models/subscription'
15+
16+
vi.mock('~/services/auth.server', () => ({
17+
requireUser: vi.fn(),
18+
}))
19+
20+
vi.mock('~/models/course', () => ({
21+
getFirstCourse: vi.fn(),
22+
}))
23+
24+
vi.mock('~/utils/permissions', () => ({
25+
requireCourseAuthor: vi.fn(),
26+
}))
27+
28+
vi.mock('~/models/transaction', () => ({
29+
getTransactionById: vi.fn(),
30+
updateTransactionStatus: vi.fn(),
31+
}))
32+
33+
vi.mock('~/models/subscription', () => ({
34+
activateSubscription: vi.fn(),
35+
deactivateSubscription: vi.fn(),
36+
}))
37+
38+
function buildRequest(status: string, notes = 'Catatan verifikasi') {
39+
const formData = new URLSearchParams({ status, notes })
40+
41+
return new Request(
42+
'http://localhost:3000/dashboard/transactions/tx-1/verify',
43+
{
44+
method: 'POST',
45+
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
46+
body: formData,
47+
}
48+
)
49+
}
50+
51+
async function runAction(status: string) {
52+
return action({
53+
request: buildRequest(status),
54+
params: { transactionId: 'tx-1' },
55+
context: {},
56+
} as never)
57+
}
58+
59+
describe('dashboard.transactions.$transactionId.$action action', () => {
60+
beforeEach(() => {
61+
vi.resetAllMocks()
62+
63+
vi.mocked(requireUser).mockResolvedValue({ id: 'author-1' } as never)
64+
vi.mocked(getFirstCourse).mockResolvedValue({ id: 'course-1' } as never)
65+
vi.mocked(requireCourseAuthor).mockReturnValue(true)
66+
vi.mocked(getTransactionById).mockResolvedValue({
67+
id: 'tx-1',
68+
status: TRANSACTION_STATUS.SUBMITTED,
69+
userId: 'user-1',
70+
} as never)
71+
vi.mocked(updateTransactionStatus).mockResolvedValue({
72+
id: 'tx-1',
73+
status: TRANSACTION_STATUS.VERIFIED,
74+
userId: 'user-1',
75+
} as never)
76+
vi.mocked(activateSubscription).mockResolvedValue({ id: 'sub-1' } as never)
77+
vi.mocked(deactivateSubscription).mockResolvedValue({
78+
id: 'sub-1',
79+
} as never)
80+
})
81+
82+
it('rejects tampered status payloads that are outside allowlist', async () => {
83+
try {
84+
await runAction(TRANSACTION_STATUS.SUBMITTED)
85+
throw new Error('Expected action to throw Response')
86+
} catch (error) {
87+
expect(error).toBeInstanceOf(Response)
88+
const response = error as Response
89+
expect(response.status).toBe(400)
90+
await expect(response.json()).resolves.toMatchObject({
91+
formError: 'Status transaksi tidak valid.',
92+
})
93+
}
94+
95+
expect(updateTransactionStatus).not.toHaveBeenCalled()
96+
})
97+
98+
it('rejects forbidden transition from VERIFIED to REJECTED', async () => {
99+
vi.mocked(getTransactionById).mockResolvedValue({
100+
id: 'tx-1',
101+
status: TRANSACTION_STATUS.VERIFIED,
102+
userId: 'user-1',
103+
} as never)
104+
105+
try {
106+
await runAction(TRANSACTION_STATUS.REJECTED)
107+
throw new Error('Expected action to throw Response')
108+
} catch (error) {
109+
expect(error).toBeInstanceOf(Response)
110+
const response = error as Response
111+
expect(response.status).toBe(400)
112+
await expect(response.json()).resolves.toMatchObject({
113+
formError:
114+
'Transaksi yang sudah diverifikasi tidak dapat diubah menjadi ditolak.',
115+
})
116+
}
117+
118+
expect(updateTransactionStatus).not.toHaveBeenCalled()
119+
})
120+
121+
it('accepts VERIFIED status and updates transaction', async () => {
122+
const response = await runAction(TRANSACTION_STATUS.VERIFIED)
123+
124+
expect(response).toBeInstanceOf(Response)
125+
expect(response.status).toBe(302)
126+
expect(response.headers.get('Location')).toBe(
127+
'/dashboard/transactions/tx-1'
128+
)
129+
expect(updateTransactionStatus).toHaveBeenCalledWith(
130+
'tx-1',
131+
'Catatan verifikasi',
132+
TRANSACTION_STATUS.VERIFIED
133+
)
134+
})
135+
})

app/routes/dashboard.transactions.$transactionId.$action.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,18 @@ export const action: ActionFunction = async ({ request, params }) => {
7676
}
7777
}
7878

79+
if (
80+
status !== TRANSACTION_STATUS.VERIFIED &&
81+
status !== TRANSACTION_STATUS.REJECTED
82+
) {
83+
throw json(
84+
{
85+
formError: 'Status transaksi tidak valid.',
86+
},
87+
{ status: 400 }
88+
)
89+
}
90+
7991
const existingTransaction = await getTransactionById(transactionId)
8092
if (!existingTransaction) {
8193
return redirect('/dashboard/transactions')
@@ -99,7 +111,7 @@ export const action: ActionFunction = async ({ request, params }) => {
99111
const transaction = await updateTransactionStatus(
100112
transactionId,
101113
notes,
102-
status as TransactionStatus
114+
status
103115
)
104116
if (!transaction) {
105117
throw json({ transaction, notes, status }, { status: 500 })

0 commit comments

Comments
 (0)