fix: Check IDOR vulnerabilities (#60)
This commit is contained in:
parent
cae804c588
commit
80117c3ede
|
|
@ -3,6 +3,7 @@
|
|||
namespace App\Http\Requests;
|
||||
|
||||
use Illuminate\Foundation\Http\FormRequest;
|
||||
use Illuminate\Validation\Rule;
|
||||
|
||||
class BulkUpdateTransactionsRequest extends FormRequest
|
||||
{
|
||||
|
|
@ -22,17 +23,41 @@ class BulkUpdateTransactionsRequest extends FormRequest
|
|||
'filters.amount_min' => ['nullable', 'numeric'],
|
||||
'filters.amount_max' => ['nullable', 'numeric'],
|
||||
'filters.category_ids' => ['nullable', 'array'],
|
||||
'filters.category_ids.*' => ['string', 'uuid', 'exists:categories,id'],
|
||||
'filters.category_ids.*' => [
|
||||
'string',
|
||||
'uuid',
|
||||
Rule::exists('categories', 'id')->where(function ($query) {
|
||||
$query->where('user_id', $this->user()->id);
|
||||
}),
|
||||
],
|
||||
'filters.account_ids' => ['nullable', 'array'],
|
||||
'filters.account_ids.*' => ['string', 'uuid'],
|
||||
'filters.label_ids' => ['nullable', 'array'],
|
||||
'filters.label_ids.*' => ['string', 'uuid', 'exists:labels,id'],
|
||||
'filters.label_ids.*' => [
|
||||
'string',
|
||||
'uuid',
|
||||
Rule::exists('labels', 'id')->where(function ($query) {
|
||||
$query->where('user_id', $this->user()->id);
|
||||
}),
|
||||
],
|
||||
'filters.search_text' => ['nullable', 'string'],
|
||||
'category_id' => ['nullable', 'exists:categories,id'],
|
||||
'category_id' => [
|
||||
'nullable',
|
||||
Rule::exists('categories', 'id')->where(function ($query) {
|
||||
$query->where('user_id', $this->user()->id);
|
||||
}),
|
||||
],
|
||||
'notes' => ['nullable', 'string'],
|
||||
'notes_iv' => ['nullable', 'string', 'size:16'],
|
||||
'label_ids' => ['nullable', 'array'],
|
||||
'label_ids.*' => ['required', 'string', 'uuid', 'exists:labels,id'],
|
||||
'label_ids.*' => [
|
||||
'required',
|
||||
'string',
|
||||
'uuid',
|
||||
Rule::exists('labels', 'id')->where(function ($query) {
|
||||
$query->where('user_id', $this->user()->id);
|
||||
}),
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
|
|
@ -40,7 +65,10 @@ class BulkUpdateTransactionsRequest extends FormRequest
|
|||
{
|
||||
return [
|
||||
'transaction_ids.*.uuid' => 'Invalid transaction ID format.',
|
||||
'category_id.exists' => 'The selected category does not exist.',
|
||||
'category_id.exists' => 'The selected category does not exist or does not belong to you.',
|
||||
'filters.category_ids.*.exists' => 'One or more filter categories do not exist or do not belong to you.',
|
||||
'filters.label_ids.*.exists' => 'One or more filter labels do not exist or do not belong to you.',
|
||||
'label_ids.*.exists' => 'One or more selected labels do not exist or do not belong to you.',
|
||||
'notes_iv.size' => 'The notes IV must be exactly 16 characters.',
|
||||
];
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
namespace App\Http\Requests;
|
||||
|
||||
use Illuminate\Foundation\Http\FormRequest;
|
||||
use Illuminate\Validation\Rule;
|
||||
|
||||
class StoreAccountBalanceRequest extends FormRequest
|
||||
{
|
||||
|
|
@ -15,7 +16,12 @@ class StoreAccountBalanceRequest extends FormRequest
|
|||
{
|
||||
return [
|
||||
'id' => ['sometimes', 'uuid'],
|
||||
'account_id' => ['required', 'exists:accounts,id'],
|
||||
'account_id' => [
|
||||
'required',
|
||||
Rule::exists('accounts', 'id')->where(function ($query) {
|
||||
$query->where('user_id', $this->user()->id);
|
||||
}),
|
||||
],
|
||||
'balance_date' => ['required', 'date'],
|
||||
'balance' => ['required', 'integer'],
|
||||
];
|
||||
|
|
|
|||
|
|
@ -17,8 +17,18 @@ class StoreTransactionRequest extends FormRequest
|
|||
{
|
||||
return [
|
||||
'id' => ['sometimes', 'uuid'],
|
||||
'account_id' => ['required', 'exists:accounts,id'],
|
||||
'category_id' => ['nullable', 'exists:categories,id'],
|
||||
'account_id' => [
|
||||
'required',
|
||||
Rule::exists('accounts', 'id')->where(function ($query) {
|
||||
$query->where('user_id', $this->user()->id);
|
||||
}),
|
||||
],
|
||||
'category_id' => [
|
||||
'nullable',
|
||||
Rule::exists('categories', 'id')->where(function ($query) {
|
||||
$query->where('user_id', $this->user()->id);
|
||||
}),
|
||||
],
|
||||
'description' => ['required', 'string'],
|
||||
'description_iv' => ['required', 'string', 'size:16'],
|
||||
'transaction_date' => ['required', 'date'],
|
||||
|
|
@ -28,7 +38,13 @@ class StoreTransactionRequest extends FormRequest
|
|||
'notes_iv' => ['nullable', 'string', 'size:16'],
|
||||
'source' => ['required', Rule::enum(TransactionSource::class)],
|
||||
'label_ids' => ['nullable', 'array'],
|
||||
'label_ids.*' => ['string', 'uuid', 'exists:labels,id'],
|
||||
'label_ids.*' => [
|
||||
'string',
|
||||
'uuid',
|
||||
Rule::exists('labels', 'id')->where(function ($query) {
|
||||
$query->where('user_id', $this->user()->id);
|
||||
}),
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
|
|
@ -36,8 +52,9 @@ class StoreTransactionRequest extends FormRequest
|
|||
{
|
||||
return [
|
||||
'account_id.required' => 'The account is required.',
|
||||
'account_id.exists' => 'The selected account does not exist.',
|
||||
'category_id.exists' => 'The selected category does not exist.',
|
||||
'account_id.exists' => 'The selected account does not exist or does not belong to you.',
|
||||
'category_id.exists' => 'The selected category does not exist or does not belong to you.',
|
||||
'label_ids.*.exists' => 'One or more selected labels do not exist or do not belong to you.',
|
||||
'description.required' => 'The description is required.',
|
||||
'description_iv.required' => 'The description IV is required.',
|
||||
'description_iv.size' => 'The description IV must be exactly 16 characters.',
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
namespace App\Http\Requests;
|
||||
|
||||
use Illuminate\Foundation\Http\FormRequest;
|
||||
use Illuminate\Validation\Rule;
|
||||
|
||||
class UpdateTransactionRequest extends FormRequest
|
||||
{
|
||||
|
|
@ -14,7 +15,12 @@ class UpdateTransactionRequest extends FormRequest
|
|||
public function rules(): array
|
||||
{
|
||||
return [
|
||||
'category_id' => ['nullable', 'exists:categories,id'],
|
||||
'category_id' => [
|
||||
'nullable',
|
||||
Rule::exists('categories', 'id')->where(function ($query) {
|
||||
$query->where('user_id', $this->user()->id);
|
||||
}),
|
||||
],
|
||||
'description' => ['sometimes', 'string'],
|
||||
'description_iv' => ['sometimes', 'string', 'size:16'],
|
||||
'notes' => ['nullable', 'string'],
|
||||
|
|
|
|||
|
|
@ -26,8 +26,12 @@
|
|||
<env name="BCRYPT_ROUNDS" value="4"/>
|
||||
<env name="BROADCAST_CONNECTION" value="null"/>
|
||||
<env name="CACHE_STORE" value="array"/>
|
||||
<env name="DB_HOST" value="127.0.0.1"/>
|
||||
<env name="DB_PORT" value="3307"/>
|
||||
<env name="DB_CONNECTION" value="mysql"/>
|
||||
<env name="DB_DATABASE" value="testing"/>
|
||||
<env name="DB_USERNAME" value="whisper_money"/>
|
||||
<env name="DB_PASSWORD" value="password"/>
|
||||
<env name="MAIL_MAILER" value="array"/>
|
||||
<env name="QUEUE_CONNECTION" value="sync"/>
|
||||
<env name="SESSION_DRIVER" value="array"/>
|
||||
|
|
|
|||
|
|
@ -150,12 +150,13 @@ it('can update an account balance', function () {
|
|||
|
||||
it('cannot update another user account balance', function () {
|
||||
$user = User::factory()->create();
|
||||
$userAccount = Account::factory()->for($user)->create();
|
||||
$otherUser = User::factory()->create();
|
||||
$account = Account::factory()->for($otherUser)->create();
|
||||
$balance = AccountBalance::factory()->for($account)->create();
|
||||
$otherAccount = Account::factory()->for($otherUser)->create();
|
||||
$balance = AccountBalance::factory()->for($otherAccount)->create();
|
||||
|
||||
$updateData = [
|
||||
'account_id' => $account->id,
|
||||
'account_id' => $userAccount->id,
|
||||
'balance_date' => $balance->balance_date->toDateString(),
|
||||
'balance' => 999999,
|
||||
];
|
||||
|
|
@ -163,6 +164,11 @@ it('cannot update another user account balance', function () {
|
|||
$response = $this->actingAs($user)->patchJson("/api/sync/account-balances/{$balance->id}", $updateData);
|
||||
|
||||
$response->assertForbidden();
|
||||
|
||||
$this->assertDatabaseHas('account_balances', [
|
||||
'id' => $balance->id,
|
||||
'account_id' => $otherAccount->id,
|
||||
]);
|
||||
});
|
||||
|
||||
it('updates existing balance when creating with duplicate account_id and balance_date', function () {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,752 @@
|
|||
<?php
|
||||
|
||||
use App\Models\Account;
|
||||
use App\Models\AccountBalance;
|
||||
use App\Models\AutomationRule;
|
||||
use App\Models\Bank;
|
||||
use App\Models\Category;
|
||||
use App\Models\Label;
|
||||
use App\Models\Transaction;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
|
||||
use function Pest\Laravel\actingAs;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
describe('Account Balance IDOR Protection', function () {
|
||||
it('cannot update current balance for another user account via direct manipulation', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
// Attacker tries to update victim's account balance
|
||||
$response = $this->putJson("/api/accounts/{$victimAccount->id}/balance/current", [
|
||||
'balance' => 9999999,
|
||||
]);
|
||||
|
||||
$response->assertForbidden();
|
||||
|
||||
$this->assertDatabaseMissing('account_balances', [
|
||||
'account_id' => $victimAccount->id,
|
||||
'balance' => 9999999,
|
||||
]);
|
||||
});
|
||||
|
||||
it('cannot update current balance for another user account', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->putJson("/api/accounts/{$victimAccount->id}/balance/current", [
|
||||
'balance' => 5000000,
|
||||
]);
|
||||
|
||||
$response->assertForbidden();
|
||||
|
||||
$this->assertDatabaseMissing('account_balances', [
|
||||
'account_id' => $victimAccount->id,
|
||||
'balance' => 5000000,
|
||||
]);
|
||||
});
|
||||
|
||||
it('cannot update existing balance for another user account', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimBalance = AccountBalance::factory()->for($victimAccount)->create([
|
||||
'balance' => 1000000,
|
||||
]);
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->putJson("/api/accounts/{$victimAccount->id}/balance/current", [
|
||||
'balance' => 9999999,
|
||||
]);
|
||||
|
||||
$response->assertForbidden();
|
||||
|
||||
$this->assertDatabaseHas('account_balances', [
|
||||
'id' => $victimBalance->id,
|
||||
'balance' => 1000000, // Balance should remain unchanged
|
||||
]);
|
||||
});
|
||||
|
||||
it('cannot delete balance from another user account', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimBalance = AccountBalance::factory()->for($victimAccount)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->deleteJson("/api/accounts/{$victimAccount->id}/balances/{$victimBalance->id}");
|
||||
|
||||
$response->assertForbidden();
|
||||
|
||||
$this->assertDatabaseHas('account_balances', [
|
||||
'id' => $victimBalance->id,
|
||||
]);
|
||||
});
|
||||
|
||||
it('cannot list balances from another user account', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
AccountBalance::factory()->for($victimAccount)->count(3)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->getJson("/api/accounts/{$victimAccount->id}/balances");
|
||||
|
||||
$response->assertForbidden();
|
||||
});
|
||||
|
||||
it('cannot delete balance using mismatched account and balance IDs', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$attackerAccount = Account::factory()->for($attacker)->create();
|
||||
$victim = User::factory()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimBalance = AccountBalance::factory()->for($victimAccount)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
// Attempt to delete victim's balance using attacker's account ID
|
||||
$response = $this->deleteJson("/api/accounts/{$attackerAccount->id}/balances/{$victimBalance->id}");
|
||||
|
||||
$response->assertNotFound();
|
||||
|
||||
$this->assertDatabaseHas('account_balances', [
|
||||
'id' => $victimBalance->id,
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Transaction IDOR Protection', function () {
|
||||
it('cannot create transaction for another user account', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$attackerAccount = Account::factory()->for($attacker)->create();
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->post('/transactions', [
|
||||
'account_id' => $victimAccount->id,
|
||||
'description' => 'Malicious transaction',
|
||||
'description_iv' => str_repeat('a', 16),
|
||||
'transaction_date' => now()->format('Y-m-d'),
|
||||
'amount' => -50000,
|
||||
'currency_code' => 'USD',
|
||||
'source' => 'manual',
|
||||
]);
|
||||
|
||||
// The transaction might be created but should belong to attacker, not victim
|
||||
// OR it should fail if account validation includes ownership check
|
||||
if ($response->status() === 201) {
|
||||
$transaction = Transaction::latest()->first();
|
||||
expect($transaction->user_id)->toBe($attacker->id)
|
||||
->and($transaction->account_id)->not->toBe($victimAccount->id);
|
||||
} else {
|
||||
// If there's validation, it should fail with 422 or redirect with errors
|
||||
expect($response->status())->toBeIn([302, 422]);
|
||||
}
|
||||
});
|
||||
|
||||
it('cannot update transaction from another user', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$attackerCategory = Category::factory()->for($attacker)->create();
|
||||
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimCategory = Category::factory()->for($victim)->create();
|
||||
$victimTransaction = Transaction::factory()
|
||||
->for($victim)
|
||||
->for($victimAccount)
|
||||
->for($victimCategory)
|
||||
->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->patch("/transactions/{$victimTransaction->id}", [
|
||||
'category_id' => $attackerCategory->id,
|
||||
'notes' => 'Hacked notes',
|
||||
'notes_iv' => str_repeat('b', 16),
|
||||
]);
|
||||
|
||||
$response->assertForbidden();
|
||||
|
||||
$this->assertDatabaseHas('transactions', [
|
||||
'id' => $victimTransaction->id,
|
||||
'category_id' => $victimCategory->id,
|
||||
'user_id' => $victim->id,
|
||||
]);
|
||||
|
||||
$this->assertDatabaseMissing('transactions', [
|
||||
'id' => $victimTransaction->id,
|
||||
'notes' => 'Hacked notes',
|
||||
]);
|
||||
});
|
||||
|
||||
it('cannot delete transaction from another user', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimTransaction = Transaction::factory()
|
||||
->for($victim)
|
||||
->for($victimAccount)
|
||||
->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->delete("/transactions/{$victimTransaction->id}");
|
||||
|
||||
$response->assertForbidden();
|
||||
|
||||
$this->assertDatabaseHas('transactions', [
|
||||
'id' => $victimTransaction->id,
|
||||
'deleted_at' => null,
|
||||
]);
|
||||
});
|
||||
|
||||
it('cannot bulk update transactions from another user by ID', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$attackerAccount = Account::factory()->for($attacker)->create();
|
||||
$attackerCategory = Category::factory()->for($attacker)->create();
|
||||
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimCategory = Category::factory()->for($victim)->create();
|
||||
$victimTransactions = Transaction::factory()
|
||||
->for($victim)
|
||||
->for($victimAccount)
|
||||
->for($victimCategory)
|
||||
->count(3)
|
||||
->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->patch('/transactions/bulk', [
|
||||
'transaction_ids' => $victimTransactions->pluck('id')->toArray(),
|
||||
'category_id' => $attackerCategory->id,
|
||||
]);
|
||||
|
||||
// Should return 403 because transaction IDs don't belong to attacker
|
||||
$response->assertForbidden();
|
||||
|
||||
// Verify victim's transactions remain unchanged
|
||||
foreach ($victimTransactions as $transaction) {
|
||||
$this->assertDatabaseHas('transactions', [
|
||||
'id' => $transaction->id,
|
||||
'category_id' => $victimCategory->id,
|
||||
'user_id' => $victim->id,
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
it('cannot bulk update transactions from another user using filters', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$attackerCategory = Category::factory()->for($attacker)->create();
|
||||
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimCategory = Category::factory()->for($victim)->create();
|
||||
$victimTransactions = Transaction::factory()
|
||||
->for($victim)
|
||||
->for($victimAccount)
|
||||
->for($victimCategory)
|
||||
->count(5)
|
||||
->create([
|
||||
'transaction_date' => now()->subDays(5),
|
||||
'amount' => 10000,
|
||||
]);
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
// Attempt to bulk update victim's transactions using date/amount filters
|
||||
$response = $this->patch('/transactions/bulk', [
|
||||
'filters' => [
|
||||
'date_from' => now()->subDays(10)->format('Y-m-d'),
|
||||
'date_to' => now()->format('Y-m-d'),
|
||||
'amount_min' => 50,
|
||||
'amount_max' => 150,
|
||||
],
|
||||
'category_id' => $attackerCategory->id,
|
||||
]);
|
||||
|
||||
$response->assertOk();
|
||||
|
||||
// Bulk update should only affect attacker's own transactions (0 in this case)
|
||||
expect($response->json('count'))->toBe(0);
|
||||
|
||||
// Verify victim's transactions remain unchanged
|
||||
foreach ($victimTransactions as $transaction) {
|
||||
$this->assertDatabaseHas('transactions', [
|
||||
'id' => $transaction->id,
|
||||
'category_id' => $victimCategory->id,
|
||||
'user_id' => $victim->id,
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
it('bulk update only affects authenticated user transactions', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$attackerAccount = Account::factory()->for($attacker)->create();
|
||||
$attackerCategory = Category::factory()->for($attacker)->create();
|
||||
$attackerTransactions = Transaction::factory()
|
||||
->for($attacker)
|
||||
->for($attackerAccount)
|
||||
->count(2)
|
||||
->create();
|
||||
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimCategory = Category::factory()->for($victim)->create();
|
||||
$victimTransactions = Transaction::factory()
|
||||
->for($victim)
|
||||
->for($victimAccount)
|
||||
->for($victimCategory)
|
||||
->count(3)
|
||||
->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
// Update all transactions without filters
|
||||
$response = $this->patch('/transactions/bulk', [
|
||||
'category_id' => $attackerCategory->id,
|
||||
]);
|
||||
|
||||
$response->assertOk();
|
||||
expect($response->json('count'))->toBe(2); // Only attacker's transactions
|
||||
|
||||
// Verify only attacker's transactions were updated
|
||||
foreach ($attackerTransactions as $transaction) {
|
||||
$this->assertDatabaseHas('transactions', [
|
||||
'id' => $transaction->id,
|
||||
'category_id' => $attackerCategory->id,
|
||||
]);
|
||||
}
|
||||
|
||||
// Verify victim's transactions remain unchanged
|
||||
foreach ($victimTransactions as $transaction) {
|
||||
$this->assertDatabaseHas('transactions', [
|
||||
'id' => $transaction->id,
|
||||
'category_id' => $victimCategory->id,
|
||||
'user_id' => $victim->id,
|
||||
]);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('Cross-Resource IDOR Protection', function () {
|
||||
it('cannot use another user category when creating transaction', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$attackerAccount = Account::factory()->for($attacker)->create();
|
||||
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimCategory = Category::factory()->for($victim)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->post('/transactions', [
|
||||
'account_id' => $attackerAccount->id,
|
||||
'category_id' => $victimCategory->id, // Attempt to use victim's category
|
||||
'description' => 'Test transaction',
|
||||
'description_iv' => str_repeat('a', 16),
|
||||
'transaction_date' => now()->format('Y-m-d'),
|
||||
'amount' => -10000,
|
||||
'currency_code' => 'USD',
|
||||
'source' => 'manual',
|
||||
]);
|
||||
|
||||
// Should either fail validation or ignore the category
|
||||
if ($response->status() === 201) {
|
||||
$transaction = Transaction::latest()->first();
|
||||
expect($transaction->category_id)->not->toBe($victimCategory->id);
|
||||
} else {
|
||||
expect($response->status())->toBeIn([302, 422]);
|
||||
}
|
||||
});
|
||||
|
||||
it('cannot use another user category when updating transaction', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$attackerAccount = Account::factory()->for($attacker)->create();
|
||||
$attackerCategory = Category::factory()->for($attacker)->create();
|
||||
$attackerTransaction = Transaction::factory()
|
||||
->for($attacker)
|
||||
->for($attackerAccount)
|
||||
->for($attackerCategory)
|
||||
->create();
|
||||
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimCategory = Category::factory()->for($victim)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->patch("/transactions/{$attackerTransaction->id}", [
|
||||
'category_id' => $victimCategory->id, // Attempt to use victim's category
|
||||
]);
|
||||
|
||||
// Should fail validation with 422 or redirect with errors
|
||||
expect($response->status())->toBeIn([302, 422]);
|
||||
|
||||
// Verify transaction category remains unchanged
|
||||
$attackerTransaction->refresh();
|
||||
expect($attackerTransaction->category_id)->toBe($attackerCategory->id);
|
||||
});
|
||||
|
||||
it('cannot use another user account when creating transaction', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->post('/transactions', [
|
||||
'account_id' => $victimAccount->id, // Attempt to use victim's account
|
||||
'description' => 'Malicious transaction',
|
||||
'description_iv' => str_repeat('a', 16),
|
||||
'transaction_date' => now()->format('Y-m-d'),
|
||||
'amount' => -50000,
|
||||
'currency_code' => 'USD',
|
||||
'source' => 'manual',
|
||||
]);
|
||||
|
||||
// Should either fail validation or the transaction should not be linked to victim's account
|
||||
if ($response->status() === 201) {
|
||||
$transaction = Transaction::latest()->first();
|
||||
expect($transaction->account_id)->not->toBe($victimAccount->id);
|
||||
} else {
|
||||
expect($response->status())->toBeIn([302, 422]);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('Sync Endpoints IDOR Protection', function () {
|
||||
describe('AccountBalanceSyncController', function () {
|
||||
it('cannot list account balances from another user account', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
AccountBalance::factory()->for($victimAccount)->count(3)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->getJson('/api/sync/account-balances');
|
||||
|
||||
$response->assertSuccessful();
|
||||
$balances = $response->json('data');
|
||||
expect($balances)->toBeArray();
|
||||
expect(count($balances))->toBe(0);
|
||||
});
|
||||
|
||||
it('cannot create account balance for another user account', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->postJson('/api/sync/account-balances', [
|
||||
'account_id' => $victimAccount->id,
|
||||
'balance_date' => now()->format('Y-m-d'),
|
||||
'balance' => 9999999,
|
||||
]);
|
||||
|
||||
$response->assertUnprocessable();
|
||||
expect($response->json('errors.account_id'))->toBeArray();
|
||||
|
||||
$this->assertDatabaseMissing('account_balances', [
|
||||
'account_id' => $victimAccount->id,
|
||||
'balance' => 9999999,
|
||||
]);
|
||||
});
|
||||
|
||||
it('cannot update account balance from another user account', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$attackerAccount = Account::factory()->for($attacker)->create();
|
||||
$victim = User::factory()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimBalance = AccountBalance::factory()->for($victimAccount)->create([
|
||||
'balance' => 1000000,
|
||||
]);
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->patchJson("/api/sync/account-balances/{$victimBalance->id}", [
|
||||
'account_id' => $attackerAccount->id,
|
||||
'balance_date' => $victimBalance->balance_date->format('Y-m-d'),
|
||||
'balance' => 9999999,
|
||||
]);
|
||||
|
||||
$response->assertForbidden();
|
||||
|
||||
$this->assertDatabaseHas('account_balances', [
|
||||
'id' => $victimBalance->id,
|
||||
'balance' => 1000000,
|
||||
'account_id' => $victimAccount->id,
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('AccountSyncController', function () {
|
||||
it('cannot list accounts from another user', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
Account::factory()->for($victim)->count(3)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->getJson('/api/sync/accounts');
|
||||
|
||||
$response->assertSuccessful();
|
||||
$accounts = $response->json('data');
|
||||
expect($accounts)->toBeArray();
|
||||
expect(count($accounts))->toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('TransactionSyncController', function () {
|
||||
it('cannot list transactions from another user', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
Transaction::factory()->for($victim)->for($victimAccount)->count(3)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->getJson('/api/sync/transactions');
|
||||
|
||||
$response->assertSuccessful();
|
||||
$transactions = $response->json('data');
|
||||
expect($transactions)->toBeArray();
|
||||
expect(count($transactions))->toBe(0);
|
||||
});
|
||||
|
||||
it('cannot create transaction for another user account via sync endpoint', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->postJson('/api/sync/transactions', [
|
||||
'account_id' => $victimAccount->id,
|
||||
'description' => 'Malicious transaction',
|
||||
'description_iv' => str_repeat('a', 16),
|
||||
'transaction_date' => now()->format('Y-m-d'),
|
||||
'amount' => -50000,
|
||||
'currency_code' => 'USD',
|
||||
'source' => 'manual',
|
||||
]);
|
||||
|
||||
$response->assertUnprocessable();
|
||||
expect($response->json('errors.account_id'))->toBeArray();
|
||||
});
|
||||
|
||||
it('cannot create transaction using another user transaction ID via sync endpoint', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$attackerAccount = Account::factory()->for($attacker)->create();
|
||||
$attackerCategory = Category::factory()->for($attacker)->create();
|
||||
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimCategory = Category::factory()->for($victim)->create();
|
||||
$victimTransaction = Transaction::factory()
|
||||
->for($victim)
|
||||
->for($victimAccount)
|
||||
->for($victimCategory)
|
||||
->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->postJson('/api/sync/transactions', [
|
||||
'id' => $victimTransaction->id,
|
||||
'account_id' => $attackerAccount->id,
|
||||
'description' => 'Attempted ID reuse',
|
||||
'description_iv' => str_repeat('a', 16),
|
||||
'transaction_date' => now()->format('Y-m-d'),
|
||||
'amount' => -50000,
|
||||
'currency_code' => 'USD',
|
||||
'source' => 'manually_created',
|
||||
]);
|
||||
|
||||
$response->assertStatus(500);
|
||||
|
||||
$this->assertDatabaseHas('transactions', [
|
||||
'id' => $victimTransaction->id,
|
||||
'user_id' => $victim->id,
|
||||
]);
|
||||
|
||||
$this->assertDatabaseMissing('transactions', [
|
||||
'id' => $victimTransaction->id,
|
||||
'user_id' => $attacker->id,
|
||||
]);
|
||||
});
|
||||
|
||||
it('idempotent create returns existing transaction when same user provides same ID', function () {
|
||||
$user = User::factory()->onboarded()->create();
|
||||
$account = Account::factory()->for($user)->create();
|
||||
$category = Category::factory()->for($user)->create();
|
||||
$existingTransaction = Transaction::factory()
|
||||
->for($user)
|
||||
->for($account)
|
||||
->for($category)
|
||||
->create([
|
||||
'description' => 'Original transaction',
|
||||
'description_iv' => str_repeat('a', 16),
|
||||
]);
|
||||
|
||||
actingAs($user);
|
||||
|
||||
$response = $this->postJson('/api/sync/transactions', [
|
||||
'id' => $existingTransaction->id,
|
||||
'account_id' => $account->id,
|
||||
'description' => 'Attempted duplicate',
|
||||
'description_iv' => str_repeat('b', 16),
|
||||
'transaction_date' => now()->format('Y-m-d'),
|
||||
'amount' => -50000,
|
||||
'currency_code' => 'USD',
|
||||
'source' => 'manually_created',
|
||||
]);
|
||||
|
||||
$response->assertSuccessful();
|
||||
expect($response->json('data.id'))->toBe($existingTransaction->id);
|
||||
expect($response->json('data.description'))->toBe('Original transaction');
|
||||
|
||||
$this->assertDatabaseHas('transactions', [
|
||||
'id' => $existingTransaction->id,
|
||||
'description' => 'Original transaction',
|
||||
]);
|
||||
});
|
||||
|
||||
it('cannot update transaction from another user via sync endpoint', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$attackerAccount = Account::factory()->for($attacker)->create();
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimCategory = Category::factory()->for($victim)->create();
|
||||
$victimTransaction = Transaction::factory()
|
||||
->for($victim)
|
||||
->for($victimAccount)
|
||||
->for($victimCategory)
|
||||
->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->patchJson("/api/sync/transactions/{$victimTransaction->id}", [
|
||||
'account_id' => $attackerAccount->id,
|
||||
'description' => 'Hacked transaction',
|
||||
'description_iv' => str_repeat('b', 16),
|
||||
'transaction_date' => $victimTransaction->transaction_date->format('Y-m-d'),
|
||||
'amount' => $victimTransaction->amount,
|
||||
'currency_code' => $victimTransaction->currency_code,
|
||||
'source' => $victimTransaction->source->value,
|
||||
]);
|
||||
|
||||
$response->assertForbidden();
|
||||
|
||||
$this->assertDatabaseHas('transactions', [
|
||||
'id' => $victimTransaction->id,
|
||||
'user_id' => $victim->id,
|
||||
'account_id' => $victimAccount->id,
|
||||
]);
|
||||
});
|
||||
|
||||
it('cannot delete transaction from another user via sync endpoint', function () {
|
||||
$attacker = User::factory()->onboarded()->create();
|
||||
$victim = User::factory()->onboarded()->create();
|
||||
$victimAccount = Account::factory()->for($victim)->create();
|
||||
$victimTransaction = Transaction::factory()
|
||||
->for($victim)
|
||||
->for($victimAccount)
|
||||
->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->deleteJson("/api/sync/transactions/{$victimTransaction->id}");
|
||||
|
||||
$response->assertForbidden();
|
||||
|
||||
$this->assertDatabaseHas('transactions', [
|
||||
'id' => $victimTransaction->id,
|
||||
'deleted_at' => null,
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('CategorySyncController', function () {
|
||||
it('cannot list categories from another user', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
Category::factory()->for($victim)->count(3)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->getJson('/api/sync/categories');
|
||||
|
||||
$response->assertSuccessful();
|
||||
$categories = $response->json('data');
|
||||
expect($categories)->toBeArray();
|
||||
expect(count($categories))->toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('LabelSyncController', function () {
|
||||
it('cannot list labels from another user', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
Label::factory()->for($victim)->count(3)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->getJson('/api/sync/labels');
|
||||
|
||||
$response->assertSuccessful();
|
||||
$labels = $response->json('data');
|
||||
expect($labels)->toBeArray();
|
||||
expect(count($labels))->toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('BankSyncController', function () {
|
||||
it('only returns global banks or user banks', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
$victimBank = Bank::factory()->for($victim)->create();
|
||||
$globalBank = Bank::factory()->create(['user_id' => null]);
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->getJson('/api/sync/banks');
|
||||
|
||||
$response->assertSuccessful();
|
||||
$banks = $response->json('data');
|
||||
expect($banks)->toBeArray();
|
||||
$bankIds = collect($banks)->pluck('id')->toArray();
|
||||
expect($bankIds)->toContain($globalBank->id);
|
||||
expect($bankIds)->not->toContain($victimBank->id);
|
||||
});
|
||||
});
|
||||
|
||||
describe('AutomationRuleSyncController', function () {
|
||||
it('cannot list automation rules from another user', function () {
|
||||
$attacker = User::factory()->create();
|
||||
$victim = User::factory()->create();
|
||||
AutomationRule::factory()->for($victim)->count(3)->create();
|
||||
|
||||
actingAs($attacker);
|
||||
|
||||
$response = $this->getJson('/api/sync/automation-rules');
|
||||
|
||||
$response->assertSuccessful();
|
||||
$rules = $response->json();
|
||||
expect($rules)->toBeArray();
|
||||
expect(count($rules))->toBe(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -192,12 +192,13 @@ it('can update a transaction', function () {
|
|||
|
||||
it('cannot update another user transaction', function () {
|
||||
$user = User::factory()->create();
|
||||
$userAccount = Account::factory()->for($user)->create();
|
||||
$otherUser = User::factory()->create();
|
||||
$account = Account::factory()->for($otherUser)->create();
|
||||
$transaction = Transaction::factory()->for($otherUser)->for($account)->create();
|
||||
$otherAccount = Account::factory()->for($otherUser)->create();
|
||||
$transaction = Transaction::factory()->for($otherUser)->for($otherAccount)->create();
|
||||
|
||||
$updateData = [
|
||||
'account_id' => $account->id,
|
||||
'account_id' => $userAccount->id,
|
||||
'category_id' => null,
|
||||
'description' => 'hacked',
|
||||
'description_iv' => $transaction->description_iv,
|
||||
|
|
@ -212,6 +213,12 @@ it('cannot update another user transaction', function () {
|
|||
$response = $this->actingAs($user)->patchJson("/api/sync/transactions/{$transaction->id}", $updateData);
|
||||
|
||||
$response->assertForbidden();
|
||||
|
||||
$this->assertDatabaseHas('transactions', [
|
||||
'id' => $transaction->id,
|
||||
'user_id' => $otherUser->id,
|
||||
'account_id' => $otherAccount->id,
|
||||
]);
|
||||
});
|
||||
|
||||
it('can delete a transaction', function () {
|
||||
|
|
|
|||
Loading…
Reference in New Issue