igny8/Tenancy_Audit_Report.md

## Tenancy Audit — Detailed Report (Dec 2025)

This is a detailed, actionable audit of the tenancy model, controls, and billing/payment gaps in the codebase as of Dec 2025. It includes precise findings with file references, concrete fixes (code + DB migration), recommended tests, rollout/rollback guidance, and prioritised next steps.

Summary of actions already applied
- Removed role/system bypasses from account/site/sector filtering and permissions (changes under `backend/igny8_core/...` and mirrored in `tenant/backend/igny8_core/...`). These changes make tenant isolation strict: queries, creates and updates now require an account context and will fail/return empty if none is present.
- Updated `Final_Flow_Tenancy.md` to present Stripe and PayPal as placeholders and Bank Transfer as the active payment path (document-only change).

Methodology
- Code locations scanned (primary):  
  - `backend/igny8_core/auth/middleware.py` (account injection & plan validation)  
  - `backend/igny8_core/api/base.py` (AccountModelViewSet, SiteSectorModelViewSet)  
  - `backend/igny8_core/api/permissions.py` (HasTenantAccess, role-based perms)  
  - `backend/igny8_core/api/throttles.py` (DebugScopedRateThrottle)  
  - `backend/igny8_core/auth/models.py` (User, Account, Plan, Subscription)  
  - `backend/igny8_core/api/authentication.py` (JWTAuthentication, APIKeyAuthentication)  
  - `backend/igny8_core/business/billing/services/credit_service.py` & `backend/igny8_core/ai/engine.py` (credits handling)  
  - `backend/igny8_core/auth/views.py` (Site/Sector ViewSets)
- I searched occurrences of `is_admin_or_developer`, `is_system_account`, `stripe_subscription_id`, `APIKeyAuthentication`, and rate-throttle bypasses and inspected relevant code paths.

Detailed findings (evidence + impact)
1) Subscription / Payment model mismatch (High)
   - Evidence:
     - `backend/igny8_core/auth/models.py::Subscription` defines `stripe_subscription_id = models.CharField(..., unique=True)` and links subscription to account. There is no `payment_method` or `external_payment_id` field.
   - Impact:
     - Can't represent non-Stripe payments (bank transfer, PayPal) without schema changes. Current docs (Bank Transfer active) are inaccurate relative to DB constraints.
   - Recommended fix:
     - DB migration to:
       - Add `payment_method` enum/choices to `Subscription` and `Account` (values: `stripe`, `paypal`, `bank_transfer`), default `stripe` for backwards compatibility.
       - Add `external_payment_id` (nullable) to record external payment reference (e.g., bank transfer reference number / PayPal transaction id).
       - Make `stripe_subscription_id` nullable and remove unique constraint if you must support multiple non-Stripe flows. Migration steps:
         1. Add new nullable fields and keep old fields unchanged.
         2. Backfill `payment_method='stripe'` where `stripe_subscription_id` exists.
         3. Make `stripe_subscription_id` nullable and drop unique index if needed.
       - Update serializers, admin forms, and registration/subscription endpoints to accept `payment_method`.
   - Files: `backend/igny8_core/auth/models.py`, `backend/igny8_core/api/serializers.py` (where Subscription serializer exists), frontend forms.

2) API-key auth bypasses plan/status validation (High)
   - Evidence:
     - `backend/igny8_core/api/authentication.py::APIKeyAuthentication` sets `request.account` and `request.site` but does not call `_validate_account_and_plan` from middleware. The middleware (`auth/middleware.py`) runs plan validation only for session/JWT flows.
   - Impact:
     - WordPress bridge/API-key clients can access tenant-scoped endpoints even if the account is suspended or plan inactive.
   - Recommended fix:
     - In `APIKeyAuthentication.authenticate()`, after resolving `account`, call a shared validation helper (e.g., factor `_validate_account_and_plan` into `auth/utils.py` and call it here). If validation fails, raise `AuthenticationFailed` or return `None`.
     - Files: `backend/igny8_core/api/authentication.py`, `backend/igny8_core/auth/middleware.py` (extract validation helper), `backend/igny8_core/auth/utils.py`.
     - Tests: add API-key request tests for suspended/past-due accounts returning 402/403.

3) Role/system overrides removed but system-account function remains (Design decision needed) (Medium)
   - Evidence:
     - `auth/models.py::Account.is_system_account()` returns slug in `['aws-admin', 'default-account', 'default']`. Many code paths previously used admin/dev/system overrides (these were removed).
   - Impact:
     - Potential residual special-casing exists (e.g., `is_system_account_user()`), and system accounts may need controlled, explicit privileges rather than silent bypasses.
   - Recommended action:
     - Decide: keep `system accounts` with tightly-scoped features (e.g., access to global monitoring endpoints only) OR remove hard-coded system slugs and manage via a `features` or `is_system_account` boolean that must be explicitly set.
     - Files to update/document: `backend/igny8_core/auth/models.py`, `master-docs/00-system/*`.

4) Throttle bypass broadness (Medium)
   - Evidence:
     - `backend/igny8_core/api/throttles.py::DebugScopedRateThrottle` currently bypasses throttling for any authenticated user (previously intended for admin/dev/system). This was simplified but remains permissive.
   - Impact:
     - Authenticated tenants are effectively unthrottled; DoS or excessive usage by a tenant is possible without tenant-specific limits.
   - Recommended fix:
     - Change to: bypass only when DEBUG or specific env var set OR user is in `system account` or developer role (if you reintroduce that concept intentionally). Otherwise enforce per-tenant/per-scope throttling.
     - Add tenant-specific rate bucket logic (e.g., throttle keys by `account.id`).
     - Files: `backend/igny8_core/api/throttles.py`.
     - Tests: verify throttling triggers for authenticated tenant requests above threshold.

5) Account injection & validation behavior (Medium)
   - Evidence:
     - `backend/igny8_core/auth/middleware.py::AccountContextMiddleware` skips middleware for `request.path.startswith('/api/v1/auth/')` and `/admin/` and returns immediately in some JWT failure cases (silently allowing `request.account = None`).
   - Impact:
     - Some endpoints may not get account validated; middleware may "fail silently" and allowing unauthenticated or un-validated access in edge cases.
   - Recommended fix:
     - Harden error handling: log token decode errors at WARNING/ERROR, and fail requests explicitly where appropriate. For API-key or JWT failure, ensure any downstream endpoints guard properly (permissions should reject).
     - Consider removing silent fallback in production; allow session auth to handle user but ensure `request.account` is present for tenant endpoints.
     - Files: `backend/igny8_core/auth/middleware.py`.

6) Credit deduction consistent but missing pre-check hooks (Low)
   - Evidence:
     - `backend/igny8_core/ai/engine.py` automatically deducts credits via `CreditService.deduct_credits_for_operation()` after AI responses.
   - Impact:
     - Good centralisation; ensure pre-check (sufficient credits) is always called before AI invocation to avoid wasted external API calls.
   - Recommended fix:
     - Verify `AIEngine.execute()` or `AICore` calls `CreditService.check_credits_legacy()` or similar BEFORE making AI calls. If not, move pre-check earlier and make AI call cancellable if funds insufficient.
     - Files: `backend/igny8_core/ai/engine.py`, `backend/igny8_core/business/billing/services/credit_service.py`.

7) Registration & initial credits (Medium)
   - Evidence:
     - `Final_Flow_Tenancy.md` describes initial credits seeded from `Plan.get_effective_credits_per_month()` on registration.
     - Implementation: `auth/views.py::register` (inspected earlier) sets `Account.credits` on creation (confirm serializer/implementation).
   - Impact:
     - Ensure atomicity: create User + Account + Subscription and seed credits in a transaction to avoid partial creates.
   - Recommended fix:
     - Wrap registration flow in a DB transaction; move credit seeding into an idempotent service method.
     - Files: `backend/igny8_core/auth/views.py`, billing service modules.

8) Billing webhook architecture (Medium)
   - Evidence:
     - Billing flows refer largely to Stripe webhooks. Bank transfer is manual in doc; no webhook or admin confirmation API exists.
   - Impact:
     - Manual processes can lead to inconsistent states (account remains suspended when payment received).
   - Recommended changes:
     - Add `admin` endpoint to confirm bank transfer (secure endpoint, permissioned) which:
       - Accepts account_id/subscription_id, `external_payment_id`, payer details, proof (file or URL), amount, and sets `Subscription.status='active'` and updates `Account.credits`.
     - Add `payment_proofs` model or attach to `CreditTransaction`.
     - Files: new `backend/igny8_core/billing/views.py`, updates to `backend/igny8_core/auth/models.py` (Subscription), `backend/igny8_core/business/billing/services/`.

9) Tests & CI regression gaps (High)
   - Evidence:
     - No test coverage found for strict tenant filtering removal. No tests for API-key gate or bank-transfer flows.
   - Recommended test matrix:
     - Unit tests:
       - `AccountModelViewSet.get_queryset()` filters by request.account; returns none when `request.account` missing.
       - `perform_create()` raises PermissionDenied when account context missing for account-scoped models.
       - `APIKeyAuthentication` denies access when account is suspended or plan inactive.
       - `DebugScopedRateThrottle` enforces throttle when expected.
     - Integration tests:
       - Registration -> account created with credits seeded.
       - Bank transfer admin confirmation endpoint flow (once implemented).
     - Add to CI pipeline; fail on regressions.

Concrete code and DB changes (step-by-step)
1) Add payment fields migration (priority: HIGH)
   - Migration pseudo-steps:
     - Add `payment_method = models.CharField(max_length=30, choices=PAYMENT_CHOICES, default='stripe')` to `Subscription` and `Account` (nullable initially if you prefer).
     - Add `external_payment_id = models.CharField(max_length=255, null=True, blank=True)`.
     - Make `stripe_subscription_id` nullable and drop unique constraint if necessary.
   - Update serializers and forms to accept `payment_method`.
   - Update `register` flow and any subscription creation flows to accept `payment_method`.

2) API-key validation (priority: HIGH)
   - Implementation:
     - Extract `_validate_account_and_plan()` from `auth/middleware.py` into `auth/utils.py` as `validate_account_and_plan(account_or_user) -> None | (error, status)`.
     - Call that helper from `APIKeyAuthentication.authenticate()` immediately after obtaining `account` and before returning `(user, api_key)`. On failure, raise `AuthenticationFailed` with the same message & HTTP semantics as middleware uses (map 402/403 to DRF errors).
   - Tests:
     - Add tests covering API key access for `active` vs `past_due` / `canceled`.

3) Throttle policy update (priority: MEDIUM)
   - Implementation:
     - Remove blanket authenticated bypass. Instead:
       - Keep `debug_bypass` and env variable bypass.
       - Add `if is_system_account_user() or is_developer()` optionally for debug environments only.
       - Add per-account throttle key using `account.id` or `request.user.account.id`.
     - Files: `backend/igny8_core/api/throttles.py`.

4) Harden middleware & logging (priority: MEDIUM)
   - Implementation:
     - Ensure token decode errors and other exceptions are logged. Fail louder in production (configurable).
     - For JWT absent but APIKey present, ensure account validation runs or permission classes enforce it.
     - Files: `backend/igny8_core/auth/middleware.py`.

5) Bank-transfer admin confirmation endpoint (priority: MEDIUM)
   - Implementation:
     - Endpoint: `POST /api/v1/billing/confirm-bank-transfer/` (permissioned to `IsAdminOrOwner` or similar admin role).
     - Accepts: `subscription_id | account_id`, `external_payment_id`, `amount`, `payer_name`, `proof_url` or file upload.
     - Actions:
       - Mark `Subscription.status='active'`, set `current_period_start/end`.
       - Set `Subscription.payment_method='bank_transfer'`, store `external_payment_id`.
       - Add `CreditTransaction` entry and top up `Account.credits` to `Plan.get_effective_credits_per_month()` (or configured top-up).
     - Files: new `backend/igny8_core/billing/views.py`, `backend/igny8_core/business/billing/services/`.
     - Tests: bank transfer happy path; confirm idempotency (double-confirmation should be safe).

6) Tests & CI (priority: HIGH)
   - Add unit tests and integration tests described earlier. Run tests locally and include in CI gating.

Documentation & UX changes
- Update `Final_Flow_Tenancy.md` (already done) and `master-docs/00-system/02-MULTITENANCY-MODEL.md` to remove references to admin bypasses and document new account-only model.
- Update admin UI to:
  - Show `payment_method` on account/subscription pages.
  - Provide "Confirm Bank Transfer" action (with proof upload).
  - Mark Stripe/PayPal UI options as "coming soon" where relevant.

Rollout plan (safe incremental)
1. Create DB migrations (add nullable fields) and deploy with code that tolerates nulls. (no downtime)
2. Backfill `payment_method='stripe'` for existing subscriptions where `stripe_subscription_id` exists.
3. Deploy API changes to accept `payment_method` but keep Stripe/PayPal inactive in business logic.
4. Add admin confirmation endpoint and small UI changes; deploy.
5. Gradually switch registration/checkout UI to allow bank transfer (server-side honored immediately).
6. Add tests and enable stricter logging / monitoring.

Rollback guidance
- Revert deployments in order: UI -> backend code -> DB (reverse migration only if safe). Keep feature flags for enabling bank-transfer flows to toggle behavior without DB rollbacks.

Estimated effort & priorities
- Immediate (1–3 days): Add `payment_method`, `external_payment_id` fields (migration + serializer updates), API-key validation change, unit tests for API-key gating.
- Short (3–7 days): Implement bank-transfer confirmation endpoint, admin UI changes, and throttling policy adjustments + tests.
- Medium (1–3 weeks): Full rollout across frontend, integration with Stripe/PayPal placeholders, heavier monitoring, and end-to-end tests.

Appendix — Exact code touchpoints (recommendations)
- `backend/igny8_core/auth/middleware.py` — extract validation helper; improve logging.
- `backend/igny8_core/api/authentication.py` — add validation call in `APIKeyAuthentication`.
- `backend/igny8_core/api/base.py` — already updated to require account; verify all viewsets inherit this correctly.
- `backend/igny8_core/api/permissions.py` — reviewed and cleaned; add tests.
- `backend/igny8_core/api/throttles.py` — tighten bypass rules; add tenant-keyed throttling.
- `backend/igny8_core/auth/models.py` — add `payment_method` + `external_payment_id`, make `stripe_subscription_id` nullable.
- `backend/igny8_core/business/billing/services/credit_service.py` — ensure pre-checks before AI calls.
- New: `backend/igny8_core/billing/views.py` & `backend/igny8_core/billing/serializers.py` — bank transfer admin confirmation endpoint.

Next steps I can take (pick one and I'll implement):
1. Implement DB migration and model/serializer changes for `payment_method` + `external_payment_id` + make `stripe_subscription_id` nullable (create migrations, update code).  
2. Implement API-key plan/status validation (small PR).  
3. Implement bank-transfer confirmation endpoint and basic admin UI API (POST endpoint only).  
4. Produce test cases and a tests PR skeleton (unittests + integration outlines).

If you'd like, I can start with implementing the DB migration and model changes (#1) and update serializers and the admin list/detail views, then add corresponding tests.