Files
igny8/final-tenancy-accounts-payments/Tenancy_Audit_Report.md
IGNY8 VPS (Salman) 191287829f asdasd
2025-12-08 05:53:32 +00:00

16 KiB
Raw Blame History

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 (13 days): Add payment_method, external_payment_id fields (migration + serializer updates), API-key validation change, unit tests for API-key gating.
  • Short (37 days): Implement bank-transfer confirmation endpoint, admin UI changes, and throttling policy adjustments + tests.
  • Medium (13 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.