feat(tenants): close PMS-21 - cfg-gate cleanup + module-config and cross-tenant tests #86
No reviewers
Labels
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
psa-systems/mokosh-server!86
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/pms-21-closeout"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
PMS-21 ('Story: Tenants and multi-tenancy') had three marked-resolved subtasks but a live audit revealed real gaps in three of the five ACs:
servernotmulti-tenant, so they compile as dead code in single-tenant builds.This PR fixes AC3 with a cfg-gate cleanup and AC5 with 4 integration tests. AC2 is re-scoped honestly in the YT description (post-merge) and the mass migration is filed as a related follow-up issue, mirroring how PMS-17 AC2 deferred to PMS-25 and PMS-4 AC6 deferred to PMS-138.
AC3 (cfg-gate cleanup):
mod routes;andmod service;on#[cfg(all(feature = \"server\", feature = \"multi-tenant\"))]so they no longer compile as dead code in single-tenant builds.use crate::modules::tenants::{tenant_routes, TenantService};on#[cfg(feature = \"multi-tenant\")]and drop the#[cfg(not(feature = \"multi-tenant\"))] let _ = TenantService::new(...)no-op stub. With the module gating tightened, that stub would not compile anyway.AC5 (integration tests in tests/tenants.rs):
All four tests reuse
seed_admin,seed_user,seed_tenant_with_admin,boot, andloginfrom tests/common/mod.rs (introduced in PMS-4); no new helpers needed.Verified locally:
cargo test --test tenants against a live postgres is deferred to CI because the dev-01 dev stack is currently down per the user's instruction; the test pattern is identical to the proven tenant_isolation_get_user_by_id_returns_404 test in tests/auth.rs (PMS-4) which runs 10/10 green.
Plan + closeout notes: docs/mokosh-upgrade/auto-stories/PMS-21/plan.md.
Closes PMS-21. Follow-up YT issue for the TenantScope mass adoption + TenantId service-layer newtype will be filed post-merge.
PMS-21 ('Story: Tenants and multi-tenancy') had three marked-resolved subtasks but a live audit revealed real gaps in three of the five ACs: - AC1 (F5 module-config routes): wired, authz correct, service helpers called. - AC2 (impossible-to-write-unscoped-query): TenantScope extractor exists at src/modules/auth/middleware.rs:197 but adoption is 0/248 handlers; service signatures still take raw Uuid; cross-cutting issue #8 stays open. - AC3 (single-tenant feature flag): router gate works; mod.rs gates routes/service on `server` not `multi-tenant`, so they compile as dead code in single-tenant builds. - AC4 (authz matrix): correct on every route. - AC5 (integration tests): tests/tenants.rs has only one test, no module-config or cross-tenant coverage. This PR fixes AC3 with a cfg-gate cleanup and AC5 with 4 integration tests. AC2 is re-scoped honestly in the YT description (post-merge) and the mass migration is filed as a related follow-up issue, mirroring how PMS-17 AC2 deferred to PMS-25 and PMS-4 AC6 deferred to PMS-138. AC3 (cfg-gate cleanup): - src/modules/tenants/mod.rs: gate `mod routes;` and `mod service;` on `#[cfg(all(feature = \"server\", feature = \"multi-tenant\"))]` so they no longer compile as dead code in single-tenant builds. - src/api/router.rs: gate the unconditional `use crate::modules::tenants::{tenant_routes, TenantService};` on `#[cfg(feature = \"multi-tenant\")]` and drop the `#[cfg(not(feature = \"multi-tenant\"))] let _ = TenantService::new(...)` no-op stub. With the module gating tightened, that stub would not compile anyway. AC5 (integration tests in tests/tenants.rs): - module_config_read_returns_default: GET /tenants/{id}/modules/billing returns 200 with the default ModuleConfig shape (module_name, is_enabled, config) for a row that does not exist yet. Pins F5 read + the service's missing-row fallback at service.rs:380-385. - module_config_write_then_read_persists: PUT writes test_module with is_enabled and config payload; independent re-GET returns the same values. Pins F5 write + persistence at service.rs:390-413. - module_config_cross_tenant_returns_403: tenant-A admin (not super_admin) trying to GET or PUT tenant-B's module config gets 403. Pins authz at routes.rs:186 and :205. - cross_tenant_get_tenant_returns_403: tenant-A technician trying to GET tenant-B's record gets 403. Pins authz at routes.rs:92-100. All four tests reuse `seed_admin`, `seed_user`, `seed_tenant_with_admin`, `boot`, and `login` from tests/common/mod.rs (introduced in PMS-4); no new helpers needed. Verified locally: - cargo check --tests - cargo check --no-default-features --features server,multi-tenant - cargo check --no-default-features --features server,single-tenant - cargo clippy --tests -- -Dwarnings - cargo fmt --all --check cargo test --test tenants against a live postgres is deferred to CI because the dev-01 dev stack is currently down per the user's instruction; the test pattern is identical to the proven tenant_isolation_get_user_by_id_returns_404 test in tests/auth.rs (PMS-4) which runs 10/10 green. Plan + closeout notes: docs/mokosh-upgrade/auto-stories/PMS-21/plan.md. Closes PMS-21. Follow-up YT issue for the TenantScope mass adoption + TenantId service-layer newtype will be filed post-merge.