Address the caller's own tenant without an id (PMS-751) #506

Merged
longjacksonle merged 1 commit from fix/PMS-751-tenants-current into main 2026-08-09 20:27:08 +02:00

PMS-751. /settings/organization on staging shows "Could not load your organization", and the save would have failed the same way. The SPA half is mokosh-apps #501; it needs this.

The failing request

GET https://api.msp.a8n.systems/api/v1//tenants/00000000-0000-0000-0000-000000000000  ->  404

The page built that URL from auth.user.tenant_id, whose only source is the mokosh_tenant_id id_token claim. Bunyip mints that claim only for a client whose oauth_clients.tenant_claim_name is non-null, with a matching oauth_client_user_tenants assignment. Staging's client has neither, and both the SPA's login callback and its rehydrate path fall back to Uuid::nil() when the claim is absent. Nothing corrects it afterwards: /auth/me reconciles role, name, timezone and theme, and UserResponse carries no tenant at all.

The 404 rather than a 403 is itself a clue. get_tenant rejects a foreign tenant with 403, but a super_admin skips that guard, so the lookup ran for the nil uuid and correctly found nothing.

What this adds

GET and PUT /api/v1/tenants/current, resolving the tenant from the caller's session.

The server knows which tenant is calling on every request. Having the browser tell it, via a claim that is optional per client, was the defect. This makes the page work whether or not bunyip's client registration is ever corrected, which matters because that registration is not ours to fix from here.

Reading needs only a session: the tenant returned is the one the caller is already authenticated against, so it exposes nothing they could not read from any other tenant-scoped response. Renaming is admin-gated, matching PUT /{tenant_id}. The name is the "from" on every request-form and invitation email a client receives, so it is tenant-wide configuration rather than a personal preference.

The id-addressed routes are untouched and keep their super-admin cross-tenant guards, which is what they were for. current is a static segment, so axum matches it ahead of {tenant_id} regardless of declaration order and it can never be parsed as a uuid.

Testing

Four new integration tests in tests/tenants.rs, at HTTP level on purpose: the entire failure was that the id in the URL was wrong, which no unit test on a service can see.

  • read then rename, asserted against the tenants.name column the emails actually read
  • current is not swallowed by the uuid path param (a 400 there would mean it was)
  • a technician can read but not rename
  • renaming through current leaves a second tenant untouched

cargo fmt --all --check, cargo clippy --all-targets, and the tenants suite green (16 tests).

Not fixed here

The bunyip side. The same console shows GET https://api.a8n.systems/v1/auth/memberships returning 401, after which the SPA logs "memberships load failed, using synthetic fallback", so the org switcher is showing a synthesised entry rather than the user's real orgs. That plus the missing tenant claim both point at an incomplete client registration for b0000000-0000-4000-8000-000000000002 on staging. Worth a BUNYIP ticket. This change deliberately does not depend on it.

PMS-751. `/settings/organization` on staging shows "Could not load your organization", and the save would have failed the same way. The SPA half is mokosh-apps #501; it needs this. ## The failing request ``` GET https://api.msp.a8n.systems/api/v1//tenants/00000000-0000-0000-0000-000000000000 -> 404 ``` The page built that URL from `auth.user.tenant_id`, whose only source is the `mokosh_tenant_id` id_token claim. Bunyip mints that claim only for a client whose `oauth_clients.tenant_claim_name` is non-null, with a matching `oauth_client_user_tenants` assignment. Staging's client has neither, and both the SPA's login callback and its rehydrate path fall back to `Uuid::nil()` when the claim is absent. Nothing corrects it afterwards: `/auth/me` reconciles role, name, timezone and theme, and `UserResponse` carries no tenant at all. The 404 rather than a 403 is itself a clue. `get_tenant` rejects a foreign tenant with 403, but a super_admin skips that guard, so the lookup ran for the nil uuid and correctly found nothing. ## What this adds `GET` and `PUT /api/v1/tenants/current`, resolving the tenant from the caller's session. The server knows which tenant is calling on every request. Having the browser tell it, via a claim that is optional per client, was the defect. This makes the page work whether or not bunyip's client registration is ever corrected, which matters because that registration is not ours to fix from here. Reading needs only a session: the tenant returned is the one the caller is already authenticated against, so it exposes nothing they could not read from any other tenant-scoped response. Renaming is admin-gated, matching `PUT /{tenant_id}`. The name is the "from" on every request-form and invitation email a client receives, so it is tenant-wide configuration rather than a personal preference. The id-addressed routes are untouched and keep their super-admin cross-tenant guards, which is what they were for. `current` is a static segment, so axum matches it ahead of `{tenant_id}` regardless of declaration order and it can never be parsed as a uuid. ## Testing Four new integration tests in `tests/tenants.rs`, at HTTP level on purpose: the entire failure was that the id in the URL was wrong, which no unit test on a service can see. - read then rename, asserted against the `tenants.name` column the emails actually read - `current` is not swallowed by the uuid path param (a 400 there would mean it was) - a technician can read but not rename - renaming through `current` leaves a second tenant untouched `cargo fmt --all --check`, `cargo clippy --all-targets`, and the tenants suite green (16 tests). ## Not fixed here The bunyip side. The same console shows `GET https://api.a8n.systems/v1/auth/memberships` returning 401, after which the SPA logs "memberships load failed, using synthetic fallback", so the org switcher is showing a synthesised entry rather than the user's real orgs. That plus the missing tenant claim both point at an incomplete client registration for `b0000000-0000-4000-8000-000000000002` on staging. Worth a BUNYIP ticket. This change deliberately does not depend on it.
feat(tenants): address the caller's own tenant without an id
All checks were successful
E2E / Playwright against staging (pull_request) Successful in 2m10s
Check / fmt + clippy + build + tests (pull_request) Successful in 4m26s
Create release / Gate (release-branch merges only) (pull_request) Successful in 1s
Create release / Create release from merged PR (pull_request) Has been skipped
Integration / integration tests (pull_request) Successful in 5m31s
9126e433a3
PMS-751. The organization settings page could neither load nor save on staging: it built its URL from `auth.user.tenant_id`, whose only source is the `mokosh_tenant_id` id_token claim, and bunyip mints that claim only for a client configured with `tenant_claim_name` and a matching tenant assignment. Without it the SPA carried the nil uuid, so the page asked for tenant 00000000-0000-0000-0000-000000000000 and got a 404. Nothing corrected it either: `/auth/me` reconciles role, name and theme but not the tenant, and `UserResponse` does not carry one.

`GET` and `PUT /api/v1/tenants/current` resolve the tenant from the caller's session instead. The server knows which tenant is calling on every request; having the browser supply it, from a claim that is optional per client, was the defect.

Reading needs only a session, since the tenant returned is the one the caller is already authenticated against. Renaming is admin-gated, matching `PUT /{tenant_id}`: the name is the "from" on every request-form and invitation email a client receives, so it is tenant-wide configuration rather than a personal preference. The id-addressed routes keep their super-admin cross-tenant guards, which is what they were for.

Tested end to end rather than at the service, because the entire failure was that the id in the URL was wrong, which no unit test on a service can see: read-then-rename against the column the emails actually read, `current` not being swallowed by the uuid path param, a technician refused the rename, and a second tenant left untouched by it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E48b8YHWYwmaKjFBy5esXp
longjacksonle scheduled this pull request to auto merge when all checks succeed 2026-08-09 20:26:14 +02:00
longjacksonle deleted branch fix/PMS-751-tenants-current 2026-08-09 20:27:09 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!506
No description provided.