# Branch Isolation Plan

## Goal

Make branch isolation a system-level security boundary instead of a collection of controller checks.

A non-super-administrator must only be able to read, create, update, delete, export, download, aggregate, search, or otherwise interact with data that belongs to one of that user's assigned branches. A super-administrator keeps system-wide access.

This plan is intentionally broader than the current `EnsureBranchAccess` middleware because request middleware alone cannot protect every query, report, relationship, file download, background job, aggregate, or indirect child record.

---

## Current State and Known Problems

### 1. There are currently multiple definitions of "accessible branches"

`App\Models\User::accessibleBranchIds()` currently combines:

- `primary_branch_id`
- the `branch_user` pivot assignments
- every branch for a super-administrator

However, several controllers still read branch access directly from `$user->branches()`.

Examples already visible in the current codebase:

- `CustomerController::index()` filters with `$request->user()->branches()`.
- `CustomerController::accessibleBranches()` returns `$request->user()->branches()`.
- `Admin\UserController` uses `$request->user()->branches()` for listing staff, editing staff, assignable branches, and assignment validation.

This creates a split-brain access model. A legacy user whose `primary_branch_id` exists but whose pivot row is missing may pass `canAccessBranch()` but still have records hidden in some screens. The reverse can also happen when different code paths derive branch access differently.

### 2. `EnsureBranchAccess` is only a request guard

The middleware protects:

- route-bound models that directly expose a `branch_id`
- `Branch` route parameters
- `User` route parameters by comparing accessible branches
- submitted fields whose names end with `branch_id` or `branch_ids`

It does **not** automatically constrain ordinary database queries.

A controller, service, report, export, command, scheduled job, relationship query, or raw query can still request rows from all branches unless the developer remembered to add a branch filter.

### 3. Indirect child models can bypass direct branch checks

Some records belong to a branch through a parent but do not contain their own `branch_id`.

Examples:

- `LoanDocument` -> `loan` -> `branch_id`
- `CustomerDocument` -> `customer` -> `branch_id`

The middleware cannot infer branch ownership from these models merely by calling `$model->getAttribute('branch_id')`.

This same risk is expected for other child records such as installment history, allocations, status history, approval decisions, collateral files/movements, customer address/kin records, and similar nested records. These must be inventoried and secured explicitly.

### 4. Controller-level filtering is duplicated and easy to miss

Current controllers contain local branch checks such as:

```php
$query->whereIn('branch_id', ...);
```

and helper methods such as:

```php
assertBranchAccess(...)
```

These are useful as defense in depth, but they should not be the primary isolation mechanism. As the system grows, every new endpoint creates another opportunity to forget a filter.

### 5. Current tests cover only part of the attack surface

`StrictBranchIsolationTest` currently verifies important cases around customers, staff, request payloads, audit logs, legacy primary branches, and super-administrator access.

The test suite still needs coverage for:

- loan lists and direct loan URLs
- repayments and receipt access
- collateral
- expenses and financial accounts
- accounting ledger
- dashboards and reports
- CSV/export endpoints
- global search
- document review/download endpoints
- relationship-loaded child records
- AJAX/lookup endpoints
- nested route models
- branch assignment changes
- queues/commands/services
- cross-branch foreign-key submissions

---

## Target Architecture

Branch isolation should use **one canonical branch-access resolver** and multiple layers of enforcement.

### Layer 1: Canonical branch access service

Create a dedicated service, for example:

```text
App\Support\BranchAccess
or
App\Services\BranchAccessService
```

Responsibilities:

- return the authenticated user's accessible branch IDs
- determine whether the user is system-wide/super-admin
- answer `canAccessBranch($branchId)`
- provide a reusable Eloquent constraint
- validate branch IDs submitted by forms/services
- expose a safe explicit bypass for trusted system operations only

All controllers, policies, scopes, reports, services, and validation rules must use this service instead of querying `$user->branches()` directly.

`User::accessibleBranchIds()` can remain as a thin convenience wrapper, but there must be only one implementation of the business rule.

### Layer 2: Canonicalize user branch assignments

The primary branch and pivot assignments must never disagree.

Rules:

1. Every non-null `primary_branch_id` must also exist in `branch_user`.
2. Exactly one pivot row should have `is_primary = true` when the user has a primary branch.
3. A primary branch change must update both `users.primary_branch_id` and the pivot atomically.
4. Removing a branch assignment must be blocked when that branch is still the user's primary branch, unless a new primary branch is supplied in the same transaction.
5. All branch assignment writes should go through one service, not directly through `sync()` in unrelated controllers.

Create a one-time repair command/migration to synchronize legacy users before stricter scoping is enabled.

Suggested command:

```text
php artisan branches:repair-user-assignments
```

The command should support a dry-run mode and report every corrected user.

### Layer 3: Global query scope for directly branch-owned models

Create a reusable trait, for example:

```text
App\Models\Concerns\BelongsToBranch
```

and a scope such as:

```text
App\Models\Scopes\BranchScope
```

For authenticated non-super-admin web requests, the scope automatically applies:

```sql
WHERE branch_id IN (...accessible branch ids...)
```

This makes ordinary model queries safe by default:

```php
Customer::query()
Loan::query()
LoanPayment::query()
FinancialAccount::query()
Expense::query()
```

The developer should not need to remember a branch filter every time.

The trait should also validate model writes:

- on `creating`, reject a `branch_id` outside scope
- on `updating`, reject moving a record to an inaccessible branch
- optionally block changing `branch_id` after creation for records that should never move between branches

Candidate direct branch-owned models must be inventoried. Likely groups include:

- customers
- loans
- loan payments
- collateral
- financial accounts and branch financial transactions
- expenses
- accounting entries
- cash reconciliations/closings
- capital injections where branch-specific
- audit logs
- collection follow-ups
- disbursements where branch-specific
- other operational records with a real `branch_id`

Do not blindly add the trait to global configuration tables that are intentionally system-wide.

### Layer 4: Secure indirectly branch-owned child models

For child records that do not contain `branch_id`, use one of two strategies.

#### Preferred long-term strategy: denormalized `branch_id`

Add `branch_id` to security-sensitive child tables and populate it from their parent.

Examples:

- loan documents
- customer documents
- loan installments
- payment allocations
- loan status history
- approval decisions
- write-off/restructure records
- collateral files/movements
- customer statement entries

Benefits:

- simple global scoping
- safer route model binding
- simpler reports and exports
- easier indexes
- easier database constraints
- less dependence on multi-table relationship logic for authorization

The child `branch_id` must be set server-side from the parent, never trusted from request input.

#### Transitional strategy: relationship-based scopes/policies

Before every child table is migrated, child access must resolve the branch through the parent relationship.

Example concept:

```php
LoanDocument::whereHas('loan', fn ($q) => $q->forAccessibleBranches())
```

Direct child URLs must call authorization that checks the parent branch.

No document download or review endpoint should authorize only by document ID.

### Layer 5: Route model binding must be branch-aware

Direct URLs must fail before a controller can accidentally operate on an out-of-scope record.

For branch-scoped models, global scopes should make implicit route model binding return 404 for inaccessible rows where appropriate.

For cases where the application intentionally wants 403 rather than 404, policies can be used after binding. The security requirement is the same: the controller must never receive an unrestricted cross-branch model.

Nested records must validate parent-child consistency. For example:

```text
/loans/{loan}/documents/{document}
```

must verify that the document belongs to that loan **and** the loan is in the user's branch scope.

### Layer 6: Cross-branch relationship validation

Every write containing foreign keys must ensure related records belong to the same branch.

Examples:

- selected customer and loan branch must match
- collateral attached to a loan must belong to the same branch
- financial account used for a repayment/disbursement must belong to the same branch
- guarantor/customer combinations must not silently cross branch boundaries unless explicitly allowed by a documented business rule
- expense account/category references must respect the owning branch/system scope
- staff assigned as approver/collector/creator must be valid for the operation where required

Create reusable validation rules such as:

```text
AccessibleBranch
BelongsToAccessibleBranch
SameBranchAs
```

Do not rely only on HTML dropdowns to prevent cross-branch IDs.

### Layer 7: Reports, dashboard, search, and exports must start from scoped queries

The following are high-risk because they often use aggregates, joins, or raw query builder operations:

- management dashboard
- accounting ledger
- financial reports
- loan/branch reports
- arrears reports
- cash reports
- global search
- CSV exports
- printable statements and receipts

Rules:

1. Resolve accessible branch IDs once at the start of the request.
2. Apply the branch predicate before any aggregation.
3. Every joined branch-owned table must preserve branch scope.
4. Never calculate a total from all branches and filter only the visible rows afterward.
5. Export queries must use exactly the same scope as the on-screen query.
6. Global search must search only inside scoped models.

### Layer 8: File access must authorize the owning branch

`SecureFileService` protects storage mechanics, but file authorization must happen before content is streamed.

For every download/review route:

1. resolve the owning customer/loan/collateral/etc.
2. verify that owner is inside the authenticated user's accessible branch scope
3. verify that the child record belongs to that owner
4. only then stream the file

Use signed URLs only as an additional control, never as a substitute for branch authorization.

### Layer 9: Background jobs and console commands require explicit branch context

A global Eloquent scope that depends on an HTTP user can behave incorrectly in queues, scheduled commands, seeders, and maintenance scripts.

Introduce an explicit branch context API, for example:

```php
BranchContext::forUser($user, fn () => ...);
BranchContext::forBranch($branchId, fn () => ...);
BranchContext::withoutScope(fn () => ...); // trusted system code only
```

Rules:

- HTTP requests resolve context from the authenticated user.
- branch-specific jobs serialize the branch ID/user ID they are allowed to process.
- system-wide jobs must opt into an explicit trusted bypass.
- unscoped access should be rare, named clearly, and covered by tests.

### Layer 10: Policies remain as a second authorization layer

Global scoping answers: **which rows can this user see?**

Policies/permissions answer: **what actions can this user perform on those rows?**

Examples:

- a collector may see a loan but not approve it
- a branch administrator may edit staff in their branch but not assign super-admin role
- an inspector may review loans but not change financial accounts

Branch scope must not be mixed with feature permission logic more than necessary.

---

## Implementation Phases

## Phase 0 - Reproduce the current isolation failures

Before changing architecture, add failing tests for every known real-world problem.

At minimum create two normal branches, one user for each branch, a multi-branch user, and a super-administrator.

Document the exact endpoints currently leaking or incorrectly denying access.

Deliverable:

```text
tests/Feature/BranchIsolation/
```

with failing regression tests that describe the real problem.

## Phase 1 - Create the canonical BranchAccess service

Tasks:

- create one resolver for accessible branch IDs
- make `User::accessibleBranchIds()` delegate to it or vice versa
- replace direct `$user->branches()->pluck(...)` authorization logic
- replace controller-local definitions of accessible branches
- add unit tests for primary-only, pivot-only, multi-branch, no-branch, and super-admin users

Important: no controller should independently define branch scope after this phase.

## Phase 2 - Repair and enforce user assignment consistency

Tasks:

- create assignment service
- synchronize primary branch and pivot in one transaction
- create dry-run repair command
- repair legacy data
- add database indexes/uniqueness rules where possible
- add tests for changing primary branch, adding/removing branches, and legacy users

## Phase 3 - Add global scope to direct branch-owned models

Tasks:

- create `BranchScope`
- create `BelongsToBranch` trait
- inventory models with `branch_id`
- apply trait model-by-model
- add create/update guards
- remove redundant controller filters only after tests prove equivalent behavior

Roll this out in groups rather than all models in one commit:

1. Customer + core customer operations
2. Loan + loan payments + arrears
3. Collateral
4. Financial accounts + expenses + accounting
5. audit/management operational data

## Phase 4 - Secure indirect child models

Tasks:

- inventory every model without `branch_id` that belongs to a branch-owned parent
- prioritize documents and downloadable records first
- add parent-scope authorization immediately
- progressively add denormalized `branch_id` migrations to important child tables
- backfill branch IDs from parents
- add indexes on `branch_id`
- ensure child `branch_id` is assigned from the parent on create

## Phase 5 - Refactor reports, dashboard, search, and exports

Tasks:

- audit every aggregate/query-builder/raw-SQL path
- make branch scope the first query condition
- compare dashboard totals against branch-specific report totals
- test multi-branch selections
- test "all accessible branches"
- ensure branch filters can narrow access but never expand it

A submitted `branch_id` or `branch_ids[]` must always be interpreted as a subset of accessible branches.

## Phase 6 - Enforce cross-branch data integrity

Tasks:

- add reusable validation rules
- verify parent/child branch equality during writes
- add model/service guards for programmatic writes
- where practical, add composite database constraints using `(id, branch_id)` relationships
- add negative tests using crafted HTTP payloads

## Phase 7 - Secure background and privileged operations

Tasks:

- add explicit `BranchContext`
- update queue jobs and scheduled commands
- make unscoped/system-wide access explicit
- audit seeders, maintenance scripts, reconciliation jobs, and report generation

## Phase 8 - Remove legacy branch checks and finalize

Only after the new isolation tests are green:

- remove duplicate controller filtering
- reduce `EnsureBranchAccess` to defense-in-depth request validation
- keep middleware protection for submitted branch selectors
- document approved unscoped code paths
- run the complete test suite

---

## Required Test Matrix

Every branch-owned feature should be tested against these personas:

| Persona | Expected access |
| --- | --- |
| User assigned only Branch A | Branch A only |
| User assigned only Branch B | Branch B only |
| User assigned A + B | A and B only |
| Legacy user with only `primary_branch_id = A` | A only until repaired; behavior must remain safe |
| User with no branch | No branch-owned data |
| Super-administrator | All branches |

For each major feature, test:

- index/list
- show/direct URL
- create
- update
- delete where supported
- dropdown/lookups
- nested relationship data
- reports/aggregates
- exports
- document download/review
- crafted `branch_id`
- crafted related IDs from another branch

### Minimum high-priority feature coverage

1. Customers
2. Customer documents
3. Loans
4. Loan documents/contracts
5. Loan approval/review
6. Disbursements
7. Repayments and receipts
8. Arrears and follow-ups
9. Collateral and collateral files
10. Expenses
11. Financial accounts and account transactions
12. Accounting ledger
13. Cash closing/reconciliation
14. Dashboard
15. Financial reports
16. Loan/branch reports
17. Global search
18. Staff management
19. Audit logs
20. Management alerts

---

## Security Rules That Must Not Be Violated

1. **Never trust `branch_id` from the browser.** It must be checked against accessible branches.
2. **A branch filter may narrow access, never expand it.**
3. **Do not use `$user->branches()` as an independent access rule.** Use the canonical resolver.
4. **Do not authorize child records only by child ID.** Resolve and authorize their owning branch.
5. **Do not perform unscoped branch-owned queries in normal HTTP code.**
6. **Do not hide unauthorized rows only in the UI.** The database query itself must be scoped.
7. **Do not calculate system-wide aggregates and then hide detail rows.** Aggregate queries must be scoped first.
8. **Do not permit cross-branch foreign-key relationships unless the business rule explicitly requires it and has dedicated authorization.**
9. **Super-admin bypass must be explicit and tested.**
10. **Queue/console code must declare its branch/system context.**

---

## Recommended Class Structure

A possible implementation layout:

```text
app/
├── Models/
│   ├── Concerns/
│   │   └── BelongsToBranch.php
│   └── Scopes/
│       └── BranchScope.php
├── Services/
│   ├── BranchAccessService.php
│   └── UserBranchAssignmentService.php
├── Support/
│   └── BranchContext.php
├── Rules/
│   ├── AccessibleBranch.php
│   ├── BelongsToAccessibleBranch.php
│   └── SameBranchAs.php
└── Console/Commands/
    └── RepairUserBranchAssignments.php

tests/Feature/BranchIsolation/
├── CustomerIsolationTest.php
├── LoanIsolationTest.php
├── FinancialIsolationTest.php
├── DocumentIsolationTest.php
├── ReportIsolationTest.php
├── StaffIsolationTest.php
└── CrossBranchWriteTest.php
```

Names can change, but responsibilities should remain separated.

---

## Suggested Pull Request Sequence

To keep the change reviewable and avoid a large high-risk rewrite:

### PR 1 - Canonical branch access

- BranchAccess service
- replace inconsistent access-ID derivation
- tests

### PR 2 - User assignment integrity

- assignment service
- repair command
- legacy data synchronization
- tests

### PR 3 - Core customer scoping

- BranchScope/trait
- Customer and directly owned customer records
- customer/document tests

### PR 4 - Loan scoping

- Loan, repayments, arrears, disbursement
- loan child authorization
- tests

### PR 5 - Financial scoping

- accounts
- transactions
- expenses
- accounting ledger
- cash closing
- tests

### PR 6 - Reports/search/dashboard/exports

- all aggregate paths
- global search
- CSV/print paths
- tests

### PR 7 - Indirect child `branch_id` hardening

- schema migrations
- backfills
- indexes/composite constraints where practical
- tests

### PR 8 - Cleanup

- remove obsolete duplicated controller filters
- document trusted unscoped paths
- full regression suite

---

## Acceptance Criteria

Branch isolation is considered complete only when all of the following are true:

- A Branch A user cannot retrieve Branch B records by changing a URL ID.
- A Branch A user cannot create/update records using Branch B IDs in crafted payloads.
- A Branch A user cannot see Branch B records in search, exports, dashboards, reports, counts, or totals.
- A Branch A user cannot download or review files owned by Branch B.
- Nested/child records cannot bypass parent branch authorization.
- Multi-branch users see exactly the union of their assigned branches.
- `primary_branch_id` and pivot assignments are synchronized.
- A user with no branches sees no branch-owned operational data.
- Super-administrator behavior remains system-wide.
- Background jobs cannot accidentally process all branches because branch context was missing.
- Every branch-owned model or query path has automated isolation coverage.
- The application no longer depends on developers remembering ad-hoc `whereIn('branch_id', ...)` clauses.

---

## Immediate Next Step

Start with **PR 1: Canonical branch access** before adding more middleware rules.

The first implementation should eliminate all competing branch-ID derivations and make every controller/service call the same resolver. Once branch identity is consistent, add the global model scope and harden indirect child records in controlled phases.

This order addresses the current inconsistency without introducing a large global scope before the underlying user-branch data model is reliable.
