fix: align id_token claims, surface mutation errors, drop dead user() (MAPPS-380) #440

Merged
nrupard merged 1 commit from fix/mapps-380-review-cleanup into main 2026-07-24 17:52:44 +02:00
Owner

What

Three small, independent cleanups surfaced by a codebase and cross-app review, grouped into one PR (MAPPS-380).

Why

  • The id_token claim struct read mokosh_role and mokosh_active_tenant, but post-cutover bunyip-as-OP never emits either (it uses bunyip_role and omits the active-tenant claim), so both always deserialized to None. The reads were dead weight and the surrounding comments misdescribed where role/tenant come from; /api/v1/auth/me is the source of truth for both (PMS-158).
  • Four mutation results were discarded with let _ =, so failures were silent: revoke-invitation, grant/revoke portal access, and mark-notification-read. A user whose action failed got no feedback.
  • AuthContext::user() was a panic-only helper (.expect("User not authenticated")) with zero call sites.

Changes

  • src/modules/oidc/tokens.rs: removed the role (mokosh_role) and active_tenant_id (mokosh_active_tenant) fields from IdTokenClaims and documented on the struct why role/active-tenant are not read from the id_token. The tenant_id (mokosh_tenant_id) read is unchanged.
  • src/hooks/auth.rs and src/pages/auth_callback.rs: role now seeds UserRole::default() (Technician) and active-tenant seeds the home tenant, both reconciled by the post-login/rehydrate /api/v1/auth/me fetch; updated the adjacent comments and the AuthContext.active_tenant_id doc that referenced the removed claim.
  • src/pages/team.rs: the revoke-invitation DELETE now matches the Result and sets the page error signal on Err (matching the invite-send path).
  • src/pages/contacts.rs: the grant and revoke portal-access PUTs now surface failures via an error toast instead of proceeding silently.
  • src/components/layout.rs: the fire-and-forget mark-notification-read POST now logs on Err via tracing::warn!.
  • src/hooks/auth.rs: removed the dead panic-only AuthContext::user() helper.

Tests

  • just pre-commit in the CI-matching rust-builder-glibc image: cargo fmt --all --check passed, cargo clippy --all-targets -- -D warnings passed, cargo check --target wasm32-unknown-unknown passed, cargo test --lib = 223 passed / 0 failed. Overall exit 0.
## What Three small, independent cleanups surfaced by a codebase and cross-app review, grouped into one PR (MAPPS-380). ## Why - The id_token claim struct read `mokosh_role` and `mokosh_active_tenant`, but post-cutover bunyip-as-OP never emits either (it uses `bunyip_role` and omits the active-tenant claim), so both always deserialized to None. The reads were dead weight and the surrounding comments misdescribed where role/tenant come from; `/api/v1/auth/me` is the source of truth for both (PMS-158). - Four mutation results were discarded with `let _ =`, so failures were silent: revoke-invitation, grant/revoke portal access, and mark-notification-read. A user whose action failed got no feedback. - `AuthContext::user()` was a panic-only helper (`.expect("User not authenticated")`) with zero call sites. ## Changes - `src/modules/oidc/tokens.rs`: removed the `role` (`mokosh_role`) and `active_tenant_id` (`mokosh_active_tenant`) fields from `IdTokenClaims` and documented on the struct why role/active-tenant are not read from the id_token. The `tenant_id` (`mokosh_tenant_id`) read is unchanged. - `src/hooks/auth.rs` and `src/pages/auth_callback.rs`: role now seeds `UserRole::default()` (Technician) and active-tenant seeds the home tenant, both reconciled by the post-login/rehydrate `/api/v1/auth/me` fetch; updated the adjacent comments and the `AuthContext.active_tenant_id` doc that referenced the removed claim. - `src/pages/team.rs`: the revoke-invitation `DELETE` now matches the `Result` and sets the page error signal on `Err` (matching the invite-send path). - `src/pages/contacts.rs`: the grant and revoke portal-access `PUT`s now surface failures via an error toast instead of proceeding silently. - `src/components/layout.rs`: the fire-and-forget mark-notification-read `POST` now logs on `Err` via `tracing::warn!`. - `src/hooks/auth.rs`: removed the dead panic-only `AuthContext::user()` helper. ## Tests - `just pre-commit` in the CI-matching `rust-builder-glibc` image: `cargo fmt --all --check` passed, `cargo clippy --all-targets -- -D warnings` passed, `cargo check --target wasm32-unknown-unknown` passed, `cargo test --lib` = 223 passed / 0 failed. Overall exit 0.
fix: align id_token claims, surface mutation errors, drop dead user()
All checks were successful
Check / fmt + clippy + tests (pull_request) Successful in 10m26s
Create release / Create release from merged PR (pull_request) Has been skipped
389a34cea2
Codebase and cross-app review cleanup grouped into one PR (MAPPS-380).

- Stale id_token claims: IdTokenClaims no longer reads mokosh_role or mokosh_active_tenant. Post-cutover bunyip-as-OP emits neither (it uses bunyip_role and omits the active-tenant claim), so both always parsed to None; role and active-tenant now seed their defaults (Technician / home tenant) and /api/v1/auth/me remains the source of truth per PMS-158. The mokosh_tenant_id read is unchanged.
- Swallowed mutation errors: the revoke-invitation DELETE (team.rs) and the grant/revoke portal-access PUTs (contacts.rs) now match the Result and surface the failure via the page error signal or an error toast instead of discarding it with `let _ =`; the fire-and-forget mark-notification-read POST (layout.rs) now logs on Err.
- Dead helper: removed the panic-only AuthContext::user() (zero call sites).

#MAPPS-380

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nrupard deleted branch fix/mapps-380-review-cleanup 2026-07-24 17:52:44 +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-apps!440
No description provided.