# Accounting API Audit — Nature ERP

Tested: 2026-05-11
Target: `https://nature.elbaset.com/api`
Auth: `X-Authorization: Bearer <token>` from `hazem@gt4it.com`
Company: Nature (id=2), Fiscal Year 2026
Test accounts: 64 (main save, asset/debit), 36 (الذمم الدائنة, liability/credit)

---

## Severity summary

| # | Finding | Severity |
|---|---------|----------|
| 1 | Unbalanced journal entry accepted on CREATE (caught only at APPROVE) | HIGH |
| 2 | Zero-amount journal entries can be created AND approved | MEDIUM |
| 3 | Same line allowed to carry both debit AND credit values | HIGH |
| 4 | Same posted entry can be reversed multiple times with no link/guard | CRITICAL |
| 5 | Foreign-currency `exchange_rate` and `currency_id` silently ignored; base amounts wrong | CRITICAL |
| 6 | Bank reconciliation completes when `closing_balance != reconciled_balance` ; difference reported as 0 incorrectly | CRITICAL |
| 7 | Depreciation run is NOT idempotent — same period can be run multiple times producing duplicate JEs | CRITICAL |
| 8 | General ledger running balance ordered by ID/created_at, not by entry date | HIGH |
| 9 | Trial balance accepts non-existent `fiscal_year_id` and returns empty 200 instead of 404/422 | MEDIUM |
| 10 | Calling reconciliation endpoint with empty ID returns HTTP 500 (server error) | LOW |
| 11 | No HTML sanitisation on description fields (XSS risk depends on frontend) | LOW |
| 12 | No reopen endpoint for closed fiscal periods (Closure is one-way) | LOW (design gap) |

Positive findings (working correctly):
- Negative debit/credit blocked (422).
- Missing `lines` blocked (422).
- Post requires `approved` status.
- Cannot double-post.
- Cannot delete posted entry.
- Cannot close period containing unposted entries.
- Cannot create JE dated in closed period.
- Reversal flips debit/credit correctly and links via `reversed_entry_id`.
- Trial balance and balance sheet figures balance (Σdr = Σcr ; assets = liab + equity).
- Cannot delete account with linked JEs.
- SQL/XSS payload safely escaped via Laravel ORM.
- Concurrent posts generate unique sequential entry numbers (atomic seq).
- Server recalculates `tax_amount` from `tax_rate_id` even if client supplies wrong value.

---

## Scenario 1 — Journal entry validation

### 1A Unbalanced 100/50
```
POST /accounting/journal-entries
{"date":"2026-05-11","description":"AUDIT unbalanced",
 "lines":[{"account_id":64,"debit":100,"credit":0},
          {"account_id":36,"debit":0,"credit":50}]}
```
HTTP 201 — entry created in `draft`, `is_balanced:false`, `total_debit=100`, `total_credit=50`.
Expected: 422.
Approve attempt returned 422 "القيد غير متوازن". So validation is at approve-time, not create-time. **HIGH** — drafts can accumulate broken data and clutter UI.

### 1B Negative values
```
{"lines":[{"account_id":64,"debit":-100,"credit":0},{"account_id":36,"debit":0,"credit":-100}]}
```
HTTP 422 — "must be at least 0". Good.

### 1C Zero amount
HTTP 201; subsequently `POST /:id/approve` -> HTTP 200, status `approved`. **MEDIUM** — no purpose-served zero JEs pollute ledger.

### 1D Missing lines
HTTP 422 — "lines field is required". Good.

### 1E 3-line balanced (60+40 dr; 100 cr)
HTTP 201, `is_balanced:true`. Good.

### 1F Same line both debit AND credit
```
{"lines":[{"account_id":64,"debit":100,"credit":100},
          {"account_id":36,"debit":50,"credit":50}]}
```
HTTP 201, status approved on next call. Totals 150/150. **HIGH** — standard double-entry should never let a single line have both. Inflates trial-balance period_debit and period_credit by phantom amounts (account 64 appears as both 100 dr and 100 cr).

---

## Scenario 2 — Post protection

- Create draft -> approve -> post -> entry_number `JV-2026-0001` assigned. Good.
- Double-post: 422 "Only approved entries can be posted" (status now `posted`). Good.
- Delete posted: 422 "Only draft entries can be deleted". Good.

---

## Scenario 3 — Period locking

- Closed period 1 (Jan 2026). Subsequent `POST /journal-entries date:2026-01-15` -> 422 "no open fiscal period for the given date". Good.
- `POST /fiscal-periods/2/close` with an unposted draft -> 422 "cannot close period with unposted entries". Good.
- **Design gap**: No reopen endpoint. After closing period 1 in this audit, there is no API to reopen it. **LOW**.

---

## Scenario 4 — Reversal

- `POST /journal-entries/6/reverse` -> HTTP 201, new entry id=8, `entry_type:reversal`, `reversed_entry_id:6`, lines flipped (acc 64: 500cr; acc 36: 500dr). Good.
- Calling reverse AGAIN on entry 6 -> HTTP 201, creates entry id=9 ALSO referencing `reversed_entry_id:6`. **CRITICAL** — if both reversals are approved+posted, the original posting is reversed twice, creating a phantom mirror transaction. Need to mark source entry as reversed and block subsequent reverse calls.

---

## Scenario 5 — Account balance integrity

`GET /accounting/reports/general-ledger?account_id=64&fiscal_year_id=1` returned two posted lines:
- JV-2026-0001 (2026-05-11) debit 500 -> running 500
- JV-2026-0002 (2026-02-15) debit 100 -> running 600

The 2026-02-15 entry should come FIRST in date-ordered ledger (running 100, then May entry running 600). System ordered by ID/created_at not by `date`. Final balance is correct, but running balances mid-list are misleading. **HIGH** — auditors rely on chronological running balance.

---

## Scenario 6 — Tax

`POST /accounting/expenses` with `amount=1000, tax_rate_id=1 (15%), tax_amount=999` -> server stored `tax_amount=150`, `total_amount=1150`. Server recomputes correctly from rate. Good.

---

## Scenario 7 — FX conversion

Created USD currency (id=2). Then:
```
POST /accounting/journal-entries
{"lines":[{"account_id":64,"debit":100,"credit":0,"currency_id":2,"exchange_rate":3.75},
          {"account_id":36,"debit":0,"credit":100,"currency_id":2,"exchange_rate":3.75}]}
```
HTTP 201 — but stored: `currency_id:2`, `exchange_rate:"1.000000"`, `base_debit:"100.000"`. **CRITICAL** — `exchange_rate` from request was silently overwritten to 1.0. Base currency amount = 100 SAR for a 100 USD line that should be 375 SAR. Any company using multi-currency entries will record wrong base amounts. Probably tied to absence of exchange-rate records and currency lookup logic.

---

## Scenario 8 — Depreciation

- Created Equipment category (useful_life 60 months), asset Laptop cost 12000.
- `POST /accounting/fixed-assets/run-depreciation period_date:2026-05-31` -> HTTP 200, recorded 200 SAR, journal_entry_id 11, `accumulated_after:200`. Math correct (12000/60=200).
- Ran the SAME call AGAIN immediately: HTTP 200, NEW record id=2, 200 SAR, journal_entry_id 12, `accumulated_after:400`. **CRITICAL** — no idempotency. A cron job overlap or accidental manual click doubles depreciation expense and accumulated depreciation. Must enforce unique (fixed_asset_id, period_date).

---

## Scenario 9 — Bank reconciliation

- bank_account 1 (linked to account 65). Created reconciliation id=1 with `opening_balance:0, closing_balance:1000, statement_balance:1000, book_balance:900` and no matched lines.
- `POST /bank-reconciliations/1/complete` -> HTTP 200 "تمت المطابقة". Response: `reconciled_balance:0, difference:0, status:completed`. **CRITICAL** — should fail because `closing_balance (1000) != reconciled_balance (0)`. Returning `difference:0` while closing!=reconciled is a math bug; allows fake "completed" reconciliations with hidden unreconciled balances.

---

## Scenario 10 — Reports

- Trial balance: `total_period_debit=600, total_period_credit=600` -> balanced. Good.
- Balance sheet `as_of_date:2026-05-31`: total_assets=600, total_liabilities=600, total_equity=0 -> A=L+E balanced. Good.

---

## Scenario 11 — Multi-tenant isolation

- Filter `?company_id=1` ignored; all rows returned belong to `company_id:2`.
- Sequential probing of JE ids 100/200/300/500/1000 -> 404. No cross-tenant access.
- `GET /reports/trial-balance?fiscal_year_id=10` (non-existent / other tenant) -> HTTP 200 with empty data instead of 422/404. **MEDIUM** — does not leak data, but soft validation lets clients invent IDs silently.

---

## Scenario 12 — Permissions

Tested as admin (297 perms). Did not create low-perm user. Skipped this scenario.

---

## Scenario 13 — Race / duplicate entry_number

- User-supplied `entry_number` is IGNORED — system always generates `JV-YYYY-NNNN` at POST time.
- 5 parallel POST operations on different draft entries produced JV-2026-0003..0007 unique sequential. Good. Atomic sequence.

---

## Scenario 14 — Injection / XSS

`description:"'; DROP TABLE accounts; -- <script>alert(1)</script>"` -> stored verbatim; `accounts` table intact afterward. Laravel ORM safely parameterised. JSON-encoding escapes `</script>` -> `<\/script>`. **LOW** — backend doesn't strip raw HTML; client must escape on render.

---

## Scenario 15 — Soft delete behaviour

- Delete account 64 (with JEs) -> 422 "cannot delete linked account". Good.
- Created account 67, posted JE 50 against it, set status `inactive` via PUT. `GET /reports/trial-balance?fiscal_year_id=1` STILL includes account 67 with balance 50. Correct — historical reporting must include deactivated accounts. Good.

---

## Recommended fixes (priority order)

1. **Bank reconciliation completion guard** — fail if `closing_balance != reconciled_balance` (or matched-line sum differs from `statement_balance`).
2. **Depreciation idempotency** — unique constraint on `(fixed_asset_id, period_date)`; reject second run with 422.
3. **Foreign currency conversion** — respect `exchange_rate` from request OR look up rate by currency+date; compute `base_debit/credit` correctly; reject if no rate available.
4. **Reversal guard** — mark source entry `is_reversed=true` on first reversal; block subsequent reverse calls with 422.
5. **Same-line debit+credit** — validation rule: `debit==0 OR credit==0` per line.
6. **Balanced at creation** — also validate `sum(debits)==sum(credits)` at POST `/journal-entries`, not just at approve.
7. **Zero-amount JEs** — reject when `total_debit==0 AND total_credit==0` (or all lines zero).
8. **Ledger ordering** — order by `(date, id)` ascending in general-ledger response so running balance is chronological.
9. **fiscal_year_id validation** — return 422/404 for unknown IDs in report endpoints.
10. **Reconciliation completion route HTTP 500** — handle "missing id" gracefully.
11. **Period reopen endpoint** — admin-only API to reopen mistakenly closed periods.
12. **Input sanitisation guidance** — document XSS handling for frontends consuming descriptions.

---

## Cleanup status

- Deleted: drafts JE 2,4,8,9,10,13,19; expenses 1,2; reconciliation 1.
- LEFT BEHIND (could not delete via API as we are not admin in the backend sense):
  - JE 3 (zero-amount, approved) — needs admin reversal.
  - JE 5 (same-line both, approved) — needs admin reversal.
  - JE 6, 7, 14-18, 20 (posted test entries totalling small amounts on account 64 / 36 / 67) — needs reversal.
  - Depreciation JE 11 and 12 (Laptop double-depreciation) — needs reversal.
  - Reversal drafts (none left after cleanup).
  - Fixed asset 1 "Laptop", asset category 1 "Equipment", expense category 1 "Office", tax rate 1 "VAT 15", account 67 "AUDIT-DEL", currency 2 "USD" — left for client to delete or keep.
  - Period 1 (Jan 2026) is now `closed` and there is no reopen API.

Send to backend (Ahmed): please reverse posted JE 6, 7, 14-20 and depreciation JEs 11-12, and reopen period 1.