# ERPdash — Remediation Plan

*Companion to `PROJECT_AUDIT_REPORT.md`. Created 2026-05-31.*

## Context

The audit found a **strong ERP core** (Finance/Inventory/Sales/HR built Controller→FormRequest→Service→Model, transactional posting, granular RBAC) coexisting with a **weak legacy template layer** (CMS/Site + lowercase `settings` + `Users`) that carries the critical security defects. This plan fixes everything in audit §3–§13, **phased by risk and ROI**, with these decisions locked in:

- **Scope:** Everything, phased (Critical → Quick Wins → Medium → Long-term → Infra).
- **Legacy CMS/Site + lowercase `settings` modules:** **Remove now** (verified self-contained — no ERP module references `setting\City/Country/District/Nationality` or `Site\*`).
- **Infrastructure:** Include everything (CI, scheduler, Redis, object storage, backups, prod hardening).

**Working rules** (per project memory): one task at a time, **a commit per task**, confirm before consequential/financial/destructive steps. Each task should keep `php artisan test` green.

---

## Phase 0 — Critical Security (Sprint 1, do first)

Smallest, highest-impact, low-risk. Each is an independent commit.

### 0.1 Login rate limiting (Critical)
- Implement `ensureIsNotRateLimited()` in `app/Http/Requests/Auth/LoginRequest.php` using `Illuminate\Support\Facades\RateLimiter` keyed by `email|ip`; clear on success; throw `ValidationException` with `auth.throttle` after 5 attempts/min.
- Add `->middleware('throttle:login')` (or `throttle:10,1`) to the two login routes in `routes/adminauth.php`. Define the `login` limiter in `AppServiceProvider::boot()` if using a named limiter.
- Mirror the same for the portal login (`app/Http/Requests/Portal/Auth/LoginRequest.php`).
- **Test:** new `tests/Feature/AdminAuthThrottleTest.php` — 6th attempt returns validation/429.

### 0.2 Remove plaintext-password design (Critical)
- Drop `real_password` assignment in `UsersController::store()`.
- Add `'password' => 'hashed'` to `App\Models\Admin::$casts` and remove manual `Hash::make` (cast handles it) — verify no double-hash.
- New migration to **drop the `echtes` column** from `admins`; remove `echtes` from `$fillable`/`$hidden`.
- **Test:** creating an admin stores a bcrypt hash; no clear-text column exists.

### 0.3 Eliminate state-changing GET routes / CSRF (High)
- In `routes/admin.php` convert every `*/delete/{id}` GET route to `DELETE` (users, permission, roles, branches, contact, district, typesetting, mainsetting). Keep controller `destroy()` signatures.
- Update the corresponding Blade actions to use `<form method="POST">@method('DELETE')@csrf` (or a small JS confirm-submit helper). Audit the sidebar/index views that emit these links.
- **Test:** GET to a former delete URL returns 405; DELETE with CSRF works.

### 0.4 Fix `UsersController` mass assignment + roles (High/Medium)
- Replace `$request->all()` with `$request->validated()` in `store()`/`update()` (the `AdminStoreRequest`/`AdminUpdateRequest` rules already exist and are sufficient).
- Restore `->assignRole()` using the validated `group_name`/role input so new admins get a role.
- **Test:** store rejects unexpected fields; created admin has the assigned role.

### 0.5 Escape DataTables HTML (Medium)
- Wrap `$row->name`/`$row->email` (and any user-supplied field) in `e()` inside the `UsersController::index` column closures; restrict `select('*')` to needed columns (exclude `password`/hidden).

**Phase 0 exit:** Security scorecard target 4 → 7. Run full suite + the new auth/CSRF tests.

---

## Phase 1 — Authorization Completion + Legacy Removal (Sprint 1–2)

### 1.1 Per-verb permission gating (High)
- Replace single-permission resource gating with per-verb middleware for `users`, `roles`, `permission`, `branches` in `routes/admin.php` (e.g. `core.users.create/edit/delete` — **these permissions already exist in `RolePermissionSeeder`**, only the routes need them).

### 1.2 Remove legacy CMS/Site + lowercase `settings` modules (consequential — confirm before deleting)
Verified self-contained. Removal surface:
- **Routes:** the unguarded blocks in `routes/admin.php` (`mdata`, `about`, `contact`, `polices`, `banner`, `statistics`, the entire `setting.*` group, and the `get*` AJAX lookups in `Admin\MainController`).
- **Controllers:** `app/Http/Controllers/Admin/Site/*`, `app/Http/Controllers/Admin/settings/*`, `Admin/MainController.php`.
- **Models:** `app/Models/Site/*`, `app/Models/setting/*`.
- **Requests:** `app/Http/Requests/Site/*`, `app/Http/Requests/setting/*`, `EditeCityRequest`, `EditeDistrictRequest`.
- **Views:** `resources/views/dashboard/admin/Site/*`, `.../setting/*`, `.../maindata/*`; remove their links from `resources/views/dashboard/admin/layouts/main-sidebar.blade.php`.
- **Migrations/tables:** add **down-only cleanup migration(s)** to drop `cities/countries/districts/nationalities/categories/services/main_settings/type_settings/files` tables (keep the migration files for history; don't edit applied migrations).
- Do this as **one reviewable commit** (or a few grouped commits: routes+views, then controllers+models, then table-drop migration). Run the suite after each.

**Note:** Keep the new ERP `Core\Setting` (`core.settings.*`) — that is the real settings system, distinct from the legacy lowercase `settings`.

### 1.3 Negative-authorization tests
- Add 403-without-permission assertions (extend `tests/Concerns/InteractsWithAdminPermissions.php` with an `assertRequiresPermission()` helper — ARCH-04) across modules; this is the regression net that would have caught the unguarded routes.

---

## Phase 2 — Quick-Win Performance + Scheduler (Sprint 2)

### 2.1 Cache settings (Perf, high ROI)
- Add a cache layer to `App\Services\Core\SettingService::get()` (key `setting:{scope}:{branch}:{key}`), invalidate in `set()`. Fix the `created_by` overwrite bug in `set()` (don't null it on update).

### 2.2 Cache dashboard analytics
- Cache `DashboardAnalyticsService::getAnalytics()` with a short TTL (e.g. 60s) per branch; remove the per-metric `Schema::hasTable()` guards once migrations are guaranteed in all envs.

### 2.3 Wire the scheduler
- Add `->withSchedule(...)` in `bootstrap/app.php`. Register: queue maintenance and **FEAT-10 contract-expiry alert** (`ContractExpiryAlertService` daily). Add the alert command/service.

---

## Phase 3 — Maintainability / Technical Debt (Sprint 3–4)

### 3.1 Consolidate duplicate/shadow models
- Resolve `HR\Employee` vs `HR\Employee\Employee` and `HR\Payroll` vs `HR\Payroll\*`. Pick the namespaced canonical versions, repoint all references (controllers/services/configs incl. `config/auth.php` employee provider), delete the rest. Run suite after each model migration.

### 3.2 Status Enums (ARCH-05)
- Add `app/Enums/{JournalEntryStatus,InvoiceStatus,PayrollStatus,VoucherStatus,ExpenseStatus,HrRequestStatus}.php`; replace magic strings in models/services/validation incrementally.

### 3.3 Standardize AJAX responses (ARCH-01)
- Make `App\Traits\ResponseApi` a real envelope (`success/data/message`); apply to lookup endpoints (`customer-invoices`, `invoice-lines`, etc.).

### 3.4 Route file modularization
- Split the 1,411-line `routes/admin.php` into per-module files (`routes/admin/finance.php`, `.../hr.php`, …) included from `admin.php`; standardize name prefixes.

### 3.5 Remove dead code
- Remove/replace the no-op `toastr()` stub in `app/Helpers/main_helper.php` (use real session-flash notifications or delete calls); move helper loading to Composer `files` autoload; drop the empty `RouteServiceProvider` if unused.

---

## Phase 4 — Money & Reporting Hardening (Sprint 4–5)

### 4.1 Money comparison robustness
- Replace float `!==` balance check in `JournalPostingService::assertBalanced()` with `bccomp`/integer minor-units; sweep posting/report services for float equality on money.

### 4.2 N+1 sweep
- Enable `Model::preventLazyLoading()` in `AppServiceProvider::boot()` for local/testing; fix offenders surfaced in list/report controllers.

### 4.3 Report query/index review
- Verify composite index on `account_period_balances (branch_id, fiscal_period_id, account_id, cost_center_key)`; add if missing (migration). Confirm GL/Trial Balance/Aging use it.

---

## Phase 5 — Infrastructure / DevOps (parallelizable from Sprint 1)

### 5.1 CI pipeline (do early — it guards every later phase)
- `.github/workflows/ci.yml`: PHP 8.2 matrix, `composer install`, `vendor/bin/pint --test`, `php artisan test`. Optionally run tests against **MySQL** in addition to SQLite.

### 5.2 Scheduler + queue workers
- Document supervised `php artisan queue:work` (Supervisor/systemd) and `schedule:run` cron. (App side wired in 2.3.)

### 5.3 Redis
- Switch `cache`, `session`, `queue` to Redis via env; add Horizon for queue visibility. Document infra requirement.

### 5.4 Object storage for uploads
- Configure an S3-compatible disk; route `ImageUploadTrait`/attachment writes through it; audit upload validation (extension allow-list, size, store outside web root). Required before multi-node scaling.

### 5.5 Backups + prod hardening
- DB backup schedule + restore runbook. Prod env: `APP_DEBUG=false`, secure session cookies (HTTPS-only), per-env `LOG_LEVEL`, error tracking (Sentry/Flare).

---

## Sequencing summary

```
Sprint 1:  Phase 0 (all) + 1.1 + 5.1 (CI)
Sprint 2:  1.2 legacy removal + 1.3 + Phase 2
Sprint 3:  3.1 + 3.2
Sprint 4:  3.3 + 3.4 + 3.5 + 4.1
Sprint 5:  4.2 + 4.3 + Phase 5 infra (5.2–5.5)
```

## Verification (each task)
- `php artisan test` stays green; add the task's own test first.
- For security tasks: explicit regression tests (throttle lockout, 405 on old GET-deletes, 403 without permission, no clear-text password column).
- For legacy removal: `php artisan route:list` shows the routes gone; full suite + manual smoke of the admin dashboard/sidebar.
- For infra: CI green on a PR; scheduler shows the contract-alert command in `schedule:list`.

## Expected outcome
Security 4→8, DevOps 3→7, Maintainability 6→8, overall ~5.6 → ~7.5. Critical findings closed in Sprint 1; production-readiness reached by end of Sprint 2 for code, Sprint 5 for full infra.