Jean-Gabriel Gill-Couture
e9522464af
All checks were successful
Run Check Script / check (pull_request) Successful in 2m21s
Removes the brittle pieces that landed in the original role-enforcement
commit: `extract_roles(value, path)` walking `serde_json::Value` with
a `path.contains("urn:")`-aware splitter, an env-driven
`FLEET_AUTH_ROLES_CLAIM`, and `VerifiedSession.roles: Vec<String>`
compared with `iter().any(|r| r == "fleet-admin")` in middleware.
This is a security-relevant code path; runtime string heuristics inside
it hide injection-shaped bugs (a misconfigured env or a custom Zitadel
mapper shifts the lookup to a path the attacker controls).
Replaced with end-to-end typed serde decoding:
* `Role` — closed enum (one variant today: `FleetAdmin`). Adding a
variant is a deliberate code change to the security path. Unknown
role names emitted by the IdP cannot be represented and therefore
cannot grant access.
* `RoleClaims` — wire-side struct, flattened into the JWT `Claims`.
The two well-known role-claim locations are matched verbatim by
`#[serde(rename = "urn:zitadel:iam:org:project:roles")]` and
`#[serde(rename = "roles")]`. No dotted-path navigation. No env
string. If a future issuer adds a third location it is an additive
`#[serde(rename = ...)]` field inside the security boundary.
* `Roles` — domain value on `VerifiedSession`. Construction is
restricted to `RoleClaims::into_roles` plus a typed
`FromIterator<Role>` for test fixtures. `Roles::has(Role::FleetAdmin)`
is the only check the middleware needs; no string comparison exists
anywhere downstream.
* Malformed shapes (scalar at the URN path, mixed-type array) now
ERROR at decode rather than degrading to an empty-vec "closed door".
Fail-loud is the security-correct default when the IdP misbehaves
— the user re-logs in, the operator notices.
Callout side: reverted the shared-extract_roles delegation. The
callout retains its own, unchanged role-extraction logic. We do NOT
need cross-crate sharing here, and the shared extractor was the entry
point we were trying to delete — the callout's own behaviour was the
status quo and is preserved verbatim.
Dropped exports: `DEFAULT_ADMIN_ROLE`, `DEFAULT_ROLES_CLAIM`,
`extract_roles`, `ROLES_CLAIM_ENV`, `ZitadelAuthConfig.roles_claim`.
None had external consumers in the workspace.
Tests:
* 12 tests on `RoleClaims` deserialization: both shapes resolve,
Zitadel URN wins precedence, unknown roles dropped, malformed
scalar/mixed-type arrays error at decode, missing claim → empty,
empty object/array → empty, extra unrelated claims are ignored,
display matches wire spelling.
* 4 middleware tests on the typed `require_role(Role::FleetAdmin, …)`
path. Dropped the redundant "admin-only vs admin+other" test —
with a single-variant enum it duplicated the positive case.