diff --git a/Final_Flow_Tenancy.md b/Final_Flow_Tenancy.md new file mode 100644 index 00000000..93a62344 --- /dev/null +++ b/Final_Flow_Tenancy.md @@ -0,0 +1,190 @@ +Nice, this is exactly the sort of thing that cleans up the whole mental model. + +Below is a single structured view of flows, split into tight tables and mini text diagrams so you can literally walk them in frontend and API console and tick them off. + +I am describing how it is intended to work, aligned with current models and views. + +--- + +## 0. Key objects involved + +| Area | Model / Endpoint | Notes | +| ------------------------- | ---------------------------------------------------------------------------------------------------------- | --------------------------------------------------- | +| Plans | `Plan` model with `price`, `billing_cycle`, `included_credits`, `stripe_*` fields | Defines included monthly credits and Stripe product | +| Accounts | `Account` with `plan`, `credits`, `status` (active, trial, suspended, cancelled) | Tenant root, holds current credits | +| Subscription | `Subscription` with `current_period_start`, `current_period_end`, `status`, `cancel_at_period_end` | Mirrors Stripe subscription state | +| Auth endpoints | `/api/v1/auth/register/`, `/api/v1/auth/login/`, `/api/v1/auth/plans/` allowed without auth | Used before tenant context exists | +| Account/Subscription APIs | `AccountsViewSet`, `SubscriptionsViewSet` under `/api/v1/auth/accounts/` and `/api/v1/auth/subscriptions/` | Manage accounts and subscriptions | + +### Payment methods (current state) + +- Options shown: Stripe (placeholder), PayPal (placeholder), Bank Transfer (active). +- Stripe/PayPal: keep UI/API options but mark as "coming soon"; no charge attempts yet. +- Bank Transfer: the only active path; capture payer details + plan, mark subscription/account pending payment until confirmed manually. +- No country-specific rules for any method; keep global defaults. + +--- + +## Flow F1 - Sign up, plan selection, initial credits + +### Text diagram + +```text +Marketing Site → App Auth + 1) User chooses plan on pricing page + 2) Frontend loads plans from API + 3) User fills register form with plan + 4) Backend creates User + Account + (optionally) Subscription + 5) Account gets plan and starting credits +``` + +### Step table + +| Step | Frontend surface | Backend call | Backend effect (DB) | What to verify | +| ---- | ------------------------------- | ----------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------- | +| 1 | Pricing page (public) | `GET /api/v1/auth/plans/` | Reads active `Plan` records via `PlanSerializer` | Plans visible match Plan rows (name, price, billing_cycle, included_credits) | +| 2 | Register page shows chosen plan | no call or same `GET /auth/plans/` | - | Selected plan id matches an active Plan row | +| 3 | Register submit | `POST /api/v1/auth/register/` (body contains user + account name + `plan_id`) | Creates `User`, `Account(plan=Plan)` and possibly `Subscription` if Stripe created immediately | In DB check: new User, new Account with `plan_id` set and `status` = `trial` or `active` as per your rule; `account.owner` = user | +| 4 | Initial credits seed | same register flow or follow up billing hook | Account.credits should be set to `plan.get_effective_credits_per_month()` (from `included_credits` or legacy `credits_per_month`) | After signup, check `Account.credits` equals the plan credits for this plan | +| 5 | Payment method choice | UI shows Stripe (placeholder), PayPal (placeholder), Bank Transfer (active) | Record `payment_method` on Account/Subscription. For now only Bank Transfer can be marked paid; Stripe/PayPal stay "coming soon" | UI labels make clear Stripe/PayPal are not yet active; bank transfer selection stores choice and blocks entry until marked paid | + +For validation you can drive this entirely with Postman and Django admin plus the app UI. + +--- + +## Flow F2 - Login, tenant context, subscription gate + +This validates that a user can only enter the app when account and plan are healthy. + +### Diagram + +```text +Login form + → POST /auth/login/ + → Validate password + → Ensure User has Account + → Ensure Account has active Plan + → Issue JWT with account_id + → All subsequent requests: + → Middleware sets request.account from token + → Permissions enforce tenant access +``` + +### Step table + +| Step | Frontend surface | API | Backend logic | Verify | +| ---- | -------------------------- | --------------------------- | ------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------ | +| 1 | Login form | `POST /api/v1/auth/login/` | Finds user by email, checks password | Wrong password gives 401 with "Invalid credentials" | +| 2 | Account gate | same login handler | If `user.account` is missing, returns 403 "Account not configured" | Create a user without account and confirm it cannot log in | +| 3 | Plan gate | same | If `account.plan` is missing or inactive, returns 402 "Active subscription required" | Mark a plan inactive, try login, see 402 | +| 4 | Token issue | same | Generates access and refresh tokens embedding `user_id` and `account_id` in JWT, and returns them with expiry timestamps | Decode JWT and check `account_id` == Account.id | +| 5 | Tenant context per request | Any app request after login | `AccountContextMiddleware` reads JWT, fetches User and Account, sets `request.account` and blocks suspended / invalid accounts | Hit a random endpoint, log `request.account.id` and compare to user.account.id | + +If this flow passes, your base tenancy gate is sound. + +--- + +## Flow F3 - Normal app workflow and credits consumption + +Think of this as the day to day life of a tenant. + +### Subflow F3a - Site and sector inside an account + +| Step | UI | API | DB objects | Verify tenancy | +| ---- | ----------- | -------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | +| 1 | Sites page | `GET /api/v1/auth/sites/` | Returns `Site` filtered by `account` via `AccountModelViewSet` and `SiteSerializer` | Confirm only sites with `site.account_id = request.account.id` show | +| 2 | Create site | `POST /api/v1/auth/sites/` | Creates `Site` inheriting `account` from `request.account`, counts against `plan.max_sites` | Try creating more sites than `plan.max_sites` and confirm it fails as per your rule | +| 3 | Add sectors | `POST /api/v1/auth/sectors/` or site detail → add sector | Sector belongs to Site, and Site belongs to Account. `Site.get_max_sectors_limit()` uses `account.plan.max_industries` to enforce limit | Adding sectors beyond limit returns validation error; each sector.site.account_id matches account | + +This sets the container for Planner / Writer. + +--- + +### Subflow F3b - Planner credits usage + +Based on the Planner technical guide. + +#### Diagram + +```text +Keywords → Auto cluster → Ideas + +Each AI operation: + - Calculate credits needed + - Call CreditService to deduct from Account.credits + - Call AICore with account context + - Store results (clusters, ideas, tasks) +``` + +#### Table + +| Stage | Frontend action | Endpoint | Backend behavior | Credits check | +| ------------------ | ------------------------------- | --------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1. Attach keyword | Keywords page, "Add keyword" | `POST /v1/planner/keywords/` | Validates seed keyword, site, sector, then creates Keyword using `SiteSectorBaseModel` so `account` is set from site | No or minimal credits used, depending on your rule | +| 2. Auto cluster | Select keywords, "Auto Cluster" | `POST /v1/planner/keywords/auto_cluster/` | `auto_cluster` loads keywords, instantiates `ClusteringService`, calls `cluster_keywords(keyword_ids, account, sector_id)` | In `ClusteringService` step 2, it "Check credits - Calculate credits needed and deduct credits from account" . Validate that Account.credits decreases accordingly after clustering | +| 3. Idea generation | Clusters page, "Generate ideas" | Usually `POST /v1/planner/clusters/generate_ideas/` | Service collects cluster data, calls AI via `AICore(account=request.account)` and creates `ContentIdeas` | Confirm one AI call consumes the planned credits and created ideas are tied to same site/sector/account | + +To test: run each step for a known account and watch `Account.credits` in admin before and after. + +--- + +### Subflow F3c - Writer credits usage + +Pattern mirrors Planner. + +| Stage | Frontend | Endpoint | Backend behavior | Credits check | +| -------------------------- | -------------------------------------- | -------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------- | +| 1. Create task from idea | Ideas page, "Send to Writer" | `POST /v1/writer/tasks/` | Creates Writer Task linked to idea, site, sector, account | No credits yet or minimal | +| 2. Generate draft | Writer task detail, "Generate Article" | `POST /v1/writer/content/generate/` or similar | Calls AI via `AICore(account=request.account)`, passes structured prompt, receives content, stores `Content` row | Check that for each generation, Account.credits decreases by the expected credit cost per 1k tokens | +| 3. Regenerate sections etc | Same page | e.g. `POST /v1/writer/content/regenerate_section/` | Same AI pipeline, smaller requests | Verify smaller credit deductions and that usage is logged in any Usage / Billing tables if present | + +`AICore` always initialises with an account and logs cost per request based on tokens and `MODEL_RATES` . You can cross check AI cost vs credit deduction. + +--- + +## Flow F4 - Subscription renewal and monthly or annual cycle + +The renewal flow is Stripe driven, mirrored in your `Subscription` and `Account` rows. + +### Diagram + +```text +Stripe billing cycle + → Stripe renews subscription (card charged) + → Stripe sends webhook to backend + → Backend updates Subscription row + → Backend resets/adjusts Account.credits based on Plan.included_credits +``` + +### Step table + +| Step | Source | Backend effect | What to verify | | +| ---- | ------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------- | +| 1 | Stripe billing engine / bank transfer confirmation | Stripe (later) marks subscription renewed; today bank transfer confirmation is recorded manually against the subscription | For bank transfer, admin marks payment received and sets renewal date; Stripe dashboard flows remain future work | | +| 2 | Stripe webhook or admin update | Webhook (future) or admin update finds `Subscription` by payment method and updates `status`, `current_period_start`, `current_period_end`, `cancel_at_period_end` | Subscription dates/status match payment confirmation; Stripe webhook path is stubbed until integration | | +| 3 | Credits reset or top up | Same handler/job | Sets `Account.credits` to `plan.get_effective_credits_per_month()` or adds included credits, depending on your policy | At renewal/confirmation, credits jump to intended value and usage across the new period subtracts from that | +| 4 | Account status update | Same billing service | Ensures `Account.status` switches from `trial` or `past_due` back to `active` | After payment, login is allowed again and middleware does not block due to account status | + +You can simulate this by editing Subscription dates and status in admin if webhooks are not wired yet, then running the same code path as webhook. + +--- + +## Flow F5 - Cancellation and expiry + +Short but necessary to validate health. + +| Scenario | Backend state | Effects to validate | +| ---------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | +| User cancels at period end | `Subscription.cancel_at_period_end=True` while `status` remains `active` until `current_period_end` | App continues working, credits used until period end; no new credits after end date | +| Subscription ends or payment fails | `Subscription.status` becomes `canceled` or `past_due`. Billing job sets `Account.status` to `suspended` or `cancelled` | Login returns 402 or 403 according to your rule, Planner/Writer calls fail with proper error and do not consume credits | + +--- + +## How to actually use this + +You can turn this into a verification pass like: + +1. Pick a clean test user and Stripe test customer. +2. Walk F1 then F2 in sequence and confirm all DB fields line up. +3. Run through F3a, F3b, F3c and log credits before and after each AI-heavy action. +4. Simulate renewal and cancellation via Stripe or DB and walk F4 and F5. diff --git a/Tenancy_Audit_Report.md b/Tenancy_Audit_Report.md new file mode 100644 index 00000000..b213c46b --- /dev/null +++ b/Tenancy_Audit_Report.md @@ -0,0 +1,205 @@ +## 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. diff --git a/backend/igny8_core/api/base.py b/backend/igny8_core/api/base.py index 223cb99a..3303e49e 100644 --- a/backend/igny8_core/api/base.py +++ b/backend/igny8_core/api/base.py @@ -19,34 +19,21 @@ class AccountModelViewSet(viewsets.ModelViewSet): # Filter by account if model has account field if hasattr(queryset.model, 'account'): user = getattr(self.request, 'user', None) - - # ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Skip account filtering for: - # - Admins and developers (by role) - # - Users in system accounts (aws-admin, default-account) + if user and hasattr(user, 'is_authenticated') and user.is_authenticated: try: - # Check if user has admin/developer privileges - is_admin_or_dev = (hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) if user else False - is_system_user = (hasattr(user, 'is_system_account_user') and user.is_system_account_user()) if user else False - - if is_admin_or_dev or is_system_user: - # Skip account filtering - allow all accounts - pass + account = getattr(self.request, 'account', None) + if not account and hasattr(self.request, 'user') and self.request.user and hasattr(self.request.user, 'is_authenticated') and self.request.user.is_authenticated: + user_account = getattr(self.request.user, 'account', None) + if user_account: + account = user_account + + if account: + queryset = queryset.filter(account=account) else: - # Get account from request (set by middleware) - account = getattr(self.request, 'account', None) - if account: - queryset = queryset.filter(account=account) - elif hasattr(self.request, 'user') and self.request.user and hasattr(self.request.user, 'is_authenticated') and self.request.user.is_authenticated: - # Fallback to user's account - try: - user_account = getattr(self.request.user, 'account', None) - if user_account: - queryset = queryset.filter(account=user_account) - except (AttributeError, Exception): - # If account access fails (e.g., column mismatch), skip account filtering - pass - except (AttributeError, TypeError) as e: + # No account context -> block access + return queryset.none() + except (AttributeError, TypeError): # If there's an error accessing user attributes, return empty queryset return queryset.none() else: @@ -61,11 +48,11 @@ class AccountModelViewSet(viewsets.ModelViewSet): try: account = getattr(self.request.user, 'account', None) except (AttributeError, Exception): - # If account access fails (e.g., column mismatch), set to None account = None - - # If model has account field, set it - if account and hasattr(serializer.Meta.model, 'account'): + + if hasattr(serializer.Meta.model, 'account'): + if not account: + raise PermissionDenied("Account context is required to create this object.") serializer.save(account=account) else: serializer.save() @@ -253,24 +240,16 @@ class SiteSectorModelViewSet(AccountModelViewSet): # Check if user is authenticated and is a proper User instance (not AnonymousUser) if user and hasattr(user, 'is_authenticated') and user.is_authenticated and hasattr(user, 'get_accessible_sites'): try: - # ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Developers, admins, and system account users - # can see all data regardless of site/sector - if (hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) or \ - (hasattr(user, 'is_system_account_user') and user.is_system_account_user()): - # Skip site/sector filtering for admins, developers, and system account users - # But still respect optional query params if provided - pass + # Get user's accessible sites + accessible_sites = user.get_accessible_sites() + + # If no accessible sites, return empty queryset + if not accessible_sites.exists(): + queryset = queryset.none() else: - # Get user's accessible sites - accessible_sites = user.get_accessible_sites() - - # If no accessible sites, return empty queryset (unless admin/developer/system account) - if not accessible_sites.exists(): - queryset = queryset.none() - else: - # Filter by accessible sites - queryset = queryset.filter(site__in=accessible_sites) - except (AttributeError, TypeError) as e: + # Filter by accessible sites + queryset = queryset.filter(site__in=accessible_sites) + except (AttributeError, TypeError): # If there's an error accessing user attributes, return empty queryset queryset = queryset.none() else: @@ -295,21 +274,14 @@ class SiteSectorModelViewSet(AccountModelViewSet): # Convert site_id to int if it's a string site_id_int = int(site_id) if site_id else None if site_id_int: - # ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Admins, developers, and system account users - # can filter by any site, others must verify access if user and hasattr(user, 'is_authenticated') and user.is_authenticated and hasattr(user, 'get_accessible_sites'): try: - if (hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) or \ - (hasattr(user, 'is_system_account_user') and user.is_system_account_user()): - # Admin/Developer/System Account User can filter by any site + accessible_sites = user.get_accessible_sites() + if accessible_sites.filter(id=site_id_int).exists(): queryset = queryset.filter(site_id=site_id_int) else: - accessible_sites = user.get_accessible_sites() - if accessible_sites.filter(id=site_id_int).exists(): - queryset = queryset.filter(site_id=site_id_int) - else: - queryset = queryset.none() # Site not accessible - except (AttributeError, TypeError) as e: + queryset = queryset.none() # Site not accessible + except (AttributeError, TypeError): # If there's an error accessing user attributes, return empty queryset queryset = queryset.none() else: @@ -369,14 +341,10 @@ class SiteSectorModelViewSet(AccountModelViewSet): if user and hasattr(user, 'is_authenticated') and user.is_authenticated and site: try: - # ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Admins, developers, and system account users - # can create in any site, others must verify access - if not ((hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) or - (hasattr(user, 'is_system_account_user') and user.is_system_account_user())): - if hasattr(user, 'get_accessible_sites'): - accessible_sites = user.get_accessible_sites() - if not accessible_sites.filter(id=site.id).exists(): - raise PermissionDenied("You do not have access to this site") + if hasattr(user, 'get_accessible_sites'): + accessible_sites = user.get_accessible_sites() + if not accessible_sites.filter(id=site.id).exists(): + raise PermissionDenied("You do not have access to this site") # Verify sector belongs to site if sector and hasattr(sector, 'site') and sector.site != site: diff --git a/backend/igny8_core/api/permissions.py b/backend/igny8_core/api/permissions.py index be1a13e9..de1ce31d 100644 --- a/backend/igny8_core/api/permissions.py +++ b/backend/igny8_core/api/permissions.py @@ -41,19 +41,6 @@ class HasTenantAccess(permissions.BasePermission): except (AttributeError, Exception): pass - # Admin/Developer/System account users bypass tenant check - if request.user and hasattr(request.user, 'is_authenticated') and request.user.is_authenticated: - try: - is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and - request.user.is_admin_or_developer()) if request.user else False - is_system_user = (hasattr(request.user, 'is_system_account_user') and - request.user.is_system_account_user()) if request.user else False - - if is_admin_or_dev or is_system_user: - return True - except (AttributeError, TypeError): - pass - # Regular users must have account access if account: # Check if user belongs to this account @@ -76,18 +63,6 @@ class IsViewerOrAbove(permissions.BasePermission): if not request.user or not request.user.is_authenticated: return False - # Admin/Developer/System account users always have access - try: - is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and - request.user.is_admin_or_developer()) if request.user else False - is_system_user = (hasattr(request.user, 'is_system_account_user') and - request.user.is_system_account_user()) if request.user else False - - if is_admin_or_dev or is_system_user: - return True - except (AttributeError, TypeError): - pass - # Check user role if hasattr(request.user, 'role'): role = request.user.role @@ -107,18 +82,6 @@ class IsEditorOrAbove(permissions.BasePermission): if not request.user or not request.user.is_authenticated: return False - # Admin/Developer/System account users always have access - try: - is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and - request.user.is_admin_or_developer()) if request.user else False - is_system_user = (hasattr(request.user, 'is_system_account_user') and - request.user.is_system_account_user()) if request.user else False - - if is_admin_or_dev or is_system_user: - return True - except (AttributeError, TypeError): - pass - # Check user role if hasattr(request.user, 'role'): role = request.user.role @@ -138,18 +101,6 @@ class IsAdminOrOwner(permissions.BasePermission): if not request.user or not request.user.is_authenticated: return False - # Admin/Developer/System account users always have access - try: - is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and - request.user.is_admin_or_developer()) if request.user else False - is_system_user = (hasattr(request.user, 'is_system_account_user') and - request.user.is_system_account_user()) if request.user else False - - if is_admin_or_dev or is_system_user: - return True - except (AttributeError, TypeError): - pass - # Check user role if hasattr(request.user, 'role'): role = request.user.role diff --git a/backend/igny8_core/api/throttles.py b/backend/igny8_core/api/throttles.py index 3eb8e195..b67cb8bc 100644 --- a/backend/igny8_core/api/throttles.py +++ b/backend/igny8_core/api/throttles.py @@ -41,23 +41,12 @@ class DebugScopedRateThrottle(ScopedRateThrottle): if not request.user or not hasattr(request.user, 'is_authenticated') or not request.user.is_authenticated: public_blueprint_bypass = True - # Bypass for authenticated users (avoid user-facing 429s) and system accounts - system_account_bypass = False + # Bypass for authenticated users (avoid user-facing 429s) authenticated_bypass = False if hasattr(request, 'user') and request.user and hasattr(request.user, 'is_authenticated') and request.user.is_authenticated: authenticated_bypass = True # Do not throttle logged-in users - try: - # Check if user is in system account (aws-admin, default-account, default) - if hasattr(request.user, 'is_system_account_user') and request.user.is_system_account_user(): - system_account_bypass = True - # Also bypass for admin/developer roles - elif hasattr(request.user, 'is_admin_or_developer') and request.user.is_admin_or_developer(): - system_account_bypass = True - except (AttributeError, Exception): - # If checking fails, continue with normal throttling - pass - if debug_bypass or env_bypass or system_account_bypass or public_blueprint_bypass or authenticated_bypass: + if debug_bypass or env_bypass or public_blueprint_bypass or authenticated_bypass: # In debug mode or for system accounts, still set throttle headers but don't actually throttle # This allows testing throttle headers without blocking requests if hasattr(self, 'get_rate'): diff --git a/backend/igny8_core/auth/models.py b/backend/igny8_core/auth/models.py index 50cc3274..4d4fca3d 100644 --- a/backend/igny8_core/auth/models.py +++ b/backend/igny8_core/auth/models.py @@ -604,8 +604,7 @@ class User(AbstractUser): return self.role == 'developer' or self.is_superuser def is_admin_or_developer(self): - """Check if user is admin or developer with override privileges.""" - # ADMIN/DEV OVERRIDE: Both admin and developer roles bypass account/site/sector restrictions + """Check if user is admin or developer.""" return self.role in ['admin', 'developer'] or self.is_superuser def is_system_account_user(self): @@ -618,29 +617,17 @@ class User(AbstractUser): def get_accessible_sites(self): """Get all sites the user can access.""" - # System account users can access all sites across all accounts - if self.is_system_account_user(): - return Site.objects.filter(is_active=True).distinct() - - # Developers/super admins can access all sites across all accounts - # ADMIN/DEV OVERRIDE: Admins also bypass account restrictions (see is_admin_or_developer) - if self.is_developer(): - return Site.objects.filter(is_active=True).distinct() - try: if not self.account: return Site.objects.none() - # Owners and admins can access all sites in their account - if self.role in ['owner', 'admin']: - return Site.objects.filter(account=self.account, is_active=True) + base_sites = Site.objects.filter(account=self.account, is_active=True) + + if self.role in ['owner', 'admin', 'developer'] or self.is_superuser or self.is_system_account_user(): + return base_sites # Other users can only access sites explicitly granted via SiteUserAccess - return Site.objects.filter( - account=self.account, - is_active=True, - user_access__user=self - ).distinct() + return base_sites.filter(user_access__user=self).distinct() except (AttributeError, Exception): # If account access fails (e.g., column mismatch), return empty queryset return Site.objects.none() diff --git a/backend/igny8_core/auth/views.py b/backend/igny8_core/auth/views.py index 7a87aa0a..df276371 100644 --- a/backend/igny8_core/auth/views.py +++ b/backend/igny8_core/auth/views.py @@ -500,22 +500,14 @@ class SiteViewSet(AccountModelViewSet): user = self.request.user - # ADMIN/DEV OVERRIDE: Both admins and developers can see all sites - if user.is_admin_or_developer(): - return Site.objects.all().distinct() - - # Get account from user account = getattr(user, 'account', None) if not account: return Site.objects.none() - if user.role in ['owner', 'admin']: - return Site.objects.filter(account=account) + if hasattr(user, 'get_accessible_sites'): + return user.get_accessible_sites() - return Site.objects.filter( - account=account, - user_access__user=user - ).distinct() + return Site.objects.filter(account=account) def perform_create(self, serializer): """Create site with account.""" @@ -736,11 +728,6 @@ class SectorViewSet(AccountModelViewSet): user = self.request.user if not user or not user.is_authenticated: return Sector.objects.none() - - # ADMIN/DEV OVERRIDE: Both admins and developers can see all sectors across all sites - if user.is_admin_or_developer(): - return Sector.objects.all().distinct() - accessible_sites = user.get_accessible_sites() return Sector.objects.filter(site__in=accessible_sites) diff --git a/tenant/backend/igny8_core/api/base.py b/tenant/backend/igny8_core/api/base.py index 223cb99a..3303e49e 100644 --- a/tenant/backend/igny8_core/api/base.py +++ b/tenant/backend/igny8_core/api/base.py @@ -19,34 +19,21 @@ class AccountModelViewSet(viewsets.ModelViewSet): # Filter by account if model has account field if hasattr(queryset.model, 'account'): user = getattr(self.request, 'user', None) - - # ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Skip account filtering for: - # - Admins and developers (by role) - # - Users in system accounts (aws-admin, default-account) + if user and hasattr(user, 'is_authenticated') and user.is_authenticated: try: - # Check if user has admin/developer privileges - is_admin_or_dev = (hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) if user else False - is_system_user = (hasattr(user, 'is_system_account_user') and user.is_system_account_user()) if user else False - - if is_admin_or_dev or is_system_user: - # Skip account filtering - allow all accounts - pass + account = getattr(self.request, 'account', None) + if not account and hasattr(self.request, 'user') and self.request.user and hasattr(self.request.user, 'is_authenticated') and self.request.user.is_authenticated: + user_account = getattr(self.request.user, 'account', None) + if user_account: + account = user_account + + if account: + queryset = queryset.filter(account=account) else: - # Get account from request (set by middleware) - account = getattr(self.request, 'account', None) - if account: - queryset = queryset.filter(account=account) - elif hasattr(self.request, 'user') and self.request.user and hasattr(self.request.user, 'is_authenticated') and self.request.user.is_authenticated: - # Fallback to user's account - try: - user_account = getattr(self.request.user, 'account', None) - if user_account: - queryset = queryset.filter(account=user_account) - except (AttributeError, Exception): - # If account access fails (e.g., column mismatch), skip account filtering - pass - except (AttributeError, TypeError) as e: + # No account context -> block access + return queryset.none() + except (AttributeError, TypeError): # If there's an error accessing user attributes, return empty queryset return queryset.none() else: @@ -61,11 +48,11 @@ class AccountModelViewSet(viewsets.ModelViewSet): try: account = getattr(self.request.user, 'account', None) except (AttributeError, Exception): - # If account access fails (e.g., column mismatch), set to None account = None - - # If model has account field, set it - if account and hasattr(serializer.Meta.model, 'account'): + + if hasattr(serializer.Meta.model, 'account'): + if not account: + raise PermissionDenied("Account context is required to create this object.") serializer.save(account=account) else: serializer.save() @@ -253,24 +240,16 @@ class SiteSectorModelViewSet(AccountModelViewSet): # Check if user is authenticated and is a proper User instance (not AnonymousUser) if user and hasattr(user, 'is_authenticated') and user.is_authenticated and hasattr(user, 'get_accessible_sites'): try: - # ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Developers, admins, and system account users - # can see all data regardless of site/sector - if (hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) or \ - (hasattr(user, 'is_system_account_user') and user.is_system_account_user()): - # Skip site/sector filtering for admins, developers, and system account users - # But still respect optional query params if provided - pass + # Get user's accessible sites + accessible_sites = user.get_accessible_sites() + + # If no accessible sites, return empty queryset + if not accessible_sites.exists(): + queryset = queryset.none() else: - # Get user's accessible sites - accessible_sites = user.get_accessible_sites() - - # If no accessible sites, return empty queryset (unless admin/developer/system account) - if not accessible_sites.exists(): - queryset = queryset.none() - else: - # Filter by accessible sites - queryset = queryset.filter(site__in=accessible_sites) - except (AttributeError, TypeError) as e: + # Filter by accessible sites + queryset = queryset.filter(site__in=accessible_sites) + except (AttributeError, TypeError): # If there's an error accessing user attributes, return empty queryset queryset = queryset.none() else: @@ -295,21 +274,14 @@ class SiteSectorModelViewSet(AccountModelViewSet): # Convert site_id to int if it's a string site_id_int = int(site_id) if site_id else None if site_id_int: - # ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Admins, developers, and system account users - # can filter by any site, others must verify access if user and hasattr(user, 'is_authenticated') and user.is_authenticated and hasattr(user, 'get_accessible_sites'): try: - if (hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) or \ - (hasattr(user, 'is_system_account_user') and user.is_system_account_user()): - # Admin/Developer/System Account User can filter by any site + accessible_sites = user.get_accessible_sites() + if accessible_sites.filter(id=site_id_int).exists(): queryset = queryset.filter(site_id=site_id_int) else: - accessible_sites = user.get_accessible_sites() - if accessible_sites.filter(id=site_id_int).exists(): - queryset = queryset.filter(site_id=site_id_int) - else: - queryset = queryset.none() # Site not accessible - except (AttributeError, TypeError) as e: + queryset = queryset.none() # Site not accessible + except (AttributeError, TypeError): # If there's an error accessing user attributes, return empty queryset queryset = queryset.none() else: @@ -369,14 +341,10 @@ class SiteSectorModelViewSet(AccountModelViewSet): if user and hasattr(user, 'is_authenticated') and user.is_authenticated and site: try: - # ADMIN/DEV/SYSTEM ACCOUNT OVERRIDE: Admins, developers, and system account users - # can create in any site, others must verify access - if not ((hasattr(user, 'is_admin_or_developer') and user.is_admin_or_developer()) or - (hasattr(user, 'is_system_account_user') and user.is_system_account_user())): - if hasattr(user, 'get_accessible_sites'): - accessible_sites = user.get_accessible_sites() - if not accessible_sites.filter(id=site.id).exists(): - raise PermissionDenied("You do not have access to this site") + if hasattr(user, 'get_accessible_sites'): + accessible_sites = user.get_accessible_sites() + if not accessible_sites.filter(id=site.id).exists(): + raise PermissionDenied("You do not have access to this site") # Verify sector belongs to site if sector and hasattr(sector, 'site') and sector.site != site: diff --git a/tenant/backend/igny8_core/api/permissions.py b/tenant/backend/igny8_core/api/permissions.py index be1a13e9..de1ce31d 100644 --- a/tenant/backend/igny8_core/api/permissions.py +++ b/tenant/backend/igny8_core/api/permissions.py @@ -41,19 +41,6 @@ class HasTenantAccess(permissions.BasePermission): except (AttributeError, Exception): pass - # Admin/Developer/System account users bypass tenant check - if request.user and hasattr(request.user, 'is_authenticated') and request.user.is_authenticated: - try: - is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and - request.user.is_admin_or_developer()) if request.user else False - is_system_user = (hasattr(request.user, 'is_system_account_user') and - request.user.is_system_account_user()) if request.user else False - - if is_admin_or_dev or is_system_user: - return True - except (AttributeError, TypeError): - pass - # Regular users must have account access if account: # Check if user belongs to this account @@ -76,18 +63,6 @@ class IsViewerOrAbove(permissions.BasePermission): if not request.user or not request.user.is_authenticated: return False - # Admin/Developer/System account users always have access - try: - is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and - request.user.is_admin_or_developer()) if request.user else False - is_system_user = (hasattr(request.user, 'is_system_account_user') and - request.user.is_system_account_user()) if request.user else False - - if is_admin_or_dev or is_system_user: - return True - except (AttributeError, TypeError): - pass - # Check user role if hasattr(request.user, 'role'): role = request.user.role @@ -107,18 +82,6 @@ class IsEditorOrAbove(permissions.BasePermission): if not request.user or not request.user.is_authenticated: return False - # Admin/Developer/System account users always have access - try: - is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and - request.user.is_admin_or_developer()) if request.user else False - is_system_user = (hasattr(request.user, 'is_system_account_user') and - request.user.is_system_account_user()) if request.user else False - - if is_admin_or_dev or is_system_user: - return True - except (AttributeError, TypeError): - pass - # Check user role if hasattr(request.user, 'role'): role = request.user.role @@ -138,18 +101,6 @@ class IsAdminOrOwner(permissions.BasePermission): if not request.user or not request.user.is_authenticated: return False - # Admin/Developer/System account users always have access - try: - is_admin_or_dev = (hasattr(request.user, 'is_admin_or_developer') and - request.user.is_admin_or_developer()) if request.user else False - is_system_user = (hasattr(request.user, 'is_system_account_user') and - request.user.is_system_account_user()) if request.user else False - - if is_admin_or_dev or is_system_user: - return True - except (AttributeError, TypeError): - pass - # Check user role if hasattr(request.user, 'role'): role = request.user.role diff --git a/tenant/backend/igny8_core/api/throttles.py b/tenant/backend/igny8_core/api/throttles.py index 3eb8e195..b67cb8bc 100644 --- a/tenant/backend/igny8_core/api/throttles.py +++ b/tenant/backend/igny8_core/api/throttles.py @@ -41,23 +41,12 @@ class DebugScopedRateThrottle(ScopedRateThrottle): if not request.user or not hasattr(request.user, 'is_authenticated') or not request.user.is_authenticated: public_blueprint_bypass = True - # Bypass for authenticated users (avoid user-facing 429s) and system accounts - system_account_bypass = False + # Bypass for authenticated users (avoid user-facing 429s) authenticated_bypass = False if hasattr(request, 'user') and request.user and hasattr(request.user, 'is_authenticated') and request.user.is_authenticated: authenticated_bypass = True # Do not throttle logged-in users - try: - # Check if user is in system account (aws-admin, default-account, default) - if hasattr(request.user, 'is_system_account_user') and request.user.is_system_account_user(): - system_account_bypass = True - # Also bypass for admin/developer roles - elif hasattr(request.user, 'is_admin_or_developer') and request.user.is_admin_or_developer(): - system_account_bypass = True - except (AttributeError, Exception): - # If checking fails, continue with normal throttling - pass - if debug_bypass or env_bypass or system_account_bypass or public_blueprint_bypass or authenticated_bypass: + if debug_bypass or env_bypass or public_blueprint_bypass or authenticated_bypass: # In debug mode or for system accounts, still set throttle headers but don't actually throttle # This allows testing throttle headers without blocking requests if hasattr(self, 'get_rate'): diff --git a/tenant/backend/igny8_core/auth/models.py b/tenant/backend/igny8_core/auth/models.py index 50cc3274..4d4fca3d 100644 --- a/tenant/backend/igny8_core/auth/models.py +++ b/tenant/backend/igny8_core/auth/models.py @@ -604,8 +604,7 @@ class User(AbstractUser): return self.role == 'developer' or self.is_superuser def is_admin_or_developer(self): - """Check if user is admin or developer with override privileges.""" - # ADMIN/DEV OVERRIDE: Both admin and developer roles bypass account/site/sector restrictions + """Check if user is admin or developer.""" return self.role in ['admin', 'developer'] or self.is_superuser def is_system_account_user(self): @@ -618,29 +617,17 @@ class User(AbstractUser): def get_accessible_sites(self): """Get all sites the user can access.""" - # System account users can access all sites across all accounts - if self.is_system_account_user(): - return Site.objects.filter(is_active=True).distinct() - - # Developers/super admins can access all sites across all accounts - # ADMIN/DEV OVERRIDE: Admins also bypass account restrictions (see is_admin_or_developer) - if self.is_developer(): - return Site.objects.filter(is_active=True).distinct() - try: if not self.account: return Site.objects.none() - # Owners and admins can access all sites in their account - if self.role in ['owner', 'admin']: - return Site.objects.filter(account=self.account, is_active=True) + base_sites = Site.objects.filter(account=self.account, is_active=True) + + if self.role in ['owner', 'admin', 'developer'] or self.is_superuser or self.is_system_account_user(): + return base_sites # Other users can only access sites explicitly granted via SiteUserAccess - return Site.objects.filter( - account=self.account, - is_active=True, - user_access__user=self - ).distinct() + return base_sites.filter(user_access__user=self).distinct() except (AttributeError, Exception): # If account access fails (e.g., column mismatch), return empty queryset return Site.objects.none() diff --git a/tenant/backend/igny8_core/auth/views.py b/tenant/backend/igny8_core/auth/views.py index 7a87aa0a..df276371 100644 --- a/tenant/backend/igny8_core/auth/views.py +++ b/tenant/backend/igny8_core/auth/views.py @@ -500,22 +500,14 @@ class SiteViewSet(AccountModelViewSet): user = self.request.user - # ADMIN/DEV OVERRIDE: Both admins and developers can see all sites - if user.is_admin_or_developer(): - return Site.objects.all().distinct() - - # Get account from user account = getattr(user, 'account', None) if not account: return Site.objects.none() - if user.role in ['owner', 'admin']: - return Site.objects.filter(account=account) + if hasattr(user, 'get_accessible_sites'): + return user.get_accessible_sites() - return Site.objects.filter( - account=account, - user_access__user=user - ).distinct() + return Site.objects.filter(account=account) def perform_create(self, serializer): """Create site with account.""" @@ -736,11 +728,6 @@ class SectorViewSet(AccountModelViewSet): user = self.request.user if not user or not user.is_authenticated: return Sector.objects.none() - - # ADMIN/DEV OVERRIDE: Both admins and developers can see all sectors across all sites - if user.is_admin_or_developer(): - return Sector.objects.all().distinct() - accessible_sites = user.get_accessible_sites() return Sector.objects.filter(site__in=accessible_sites)