feat(fleet-auth): unify Zitadel role extraction + request roles via scope (Ch1) #327

johnride · 2026-06-05T01:32:17Z

johnride commented

2026-06-05 01:32:17 +00:00

Role-gate follow-ups from v0.3 plan Ch1:

New leaf crate harmony_zitadel_roles: one authoritative parser for
Zitadel/OIDC project roles (array + object-map + project-scoped URN
variant), shared by the two backend trust boundaries that gate on roles.
The dashboard's extract_zitadel_roles (object-map only) and the callout's
ZitadelValidator::extract_roles (array + object-map) now both delegate;
their duplicated logic and tests are deleted.
Request roles in-band: build_login_attempt forces the
urn:zitadel:iam:org:project:roles scope, so the gate no longer depends on
Zitadel's out-of-band "Assert Roles on Authentication" checkbox (which broke
it once). Idempotent if the scope is already present.
docs/guides/operator-dashboard-sso.md step 1b + config reference: drop the
wrong checkbox instruction, document the in-band scope.

Item 4 (lift require_role to a composable layer) intentionally skipped as
YAGNI — only fleet-admin exists; revisit at the second role.

Role-gate follow-ups from v0.3 plan Ch1: - New leaf crate `harmony_zitadel_roles`: one authoritative parser for Zitadel/OIDC project roles (array + object-map + project-scoped URN variant), shared by the two backend trust boundaries that gate on roles. The dashboard's `extract_zitadel_roles` (object-map only) and the callout's `ZitadelValidator::extract_roles` (array + object-map) now both delegate; their duplicated logic and tests are deleted. - Request roles in-band: `build_login_attempt` forces the `urn:zitadel:iam:org:project:roles` scope, so the gate no longer depends on Zitadel's out-of-band "Assert Roles on Authentication" checkbox (which broke it once). Idempotent if the scope is already present. - docs/guides/operator-dashboard-sso.md step 1b + config reference: drop the wrong checkbox instruction, document the in-band scope. Item 4 (lift `require_role` to a composable layer) intentionally skipped as YAGNI — only `fleet-admin` exists; revisit at the second role.

johnride added 1 commit 2026-06-05 01:32:17 +00:00

feat(fleet-auth): unify Zitadel role extraction + request roles via scope (Ch1)

Run Check Script / check (pull_request) Failing after 1m4s

Details

9dd33f2671

Role-gate follow-ups from v0.3 plan Ch1:

- New leaf crate `harmony_zitadel_roles`: one authoritative parser for
  Zitadel/OIDC project roles (array + object-map + project-scoped URN
  variant), shared by the two backend trust boundaries that gate on roles.
  The dashboard's `extract_zitadel_roles` (object-map only) and the callout's
  `ZitadelValidator::extract_roles` (array + object-map) now both delegate;
  their duplicated logic and tests are deleted.
- Request roles in-band: `build_login_attempt` forces the
  `urn:zitadel:iam:org:project:roles` scope, so the gate no longer depends on
  Zitadel's out-of-band "Assert Roles on Authentication" checkbox (which broke
  it once). Idempotent if the scope is already present.
- docs/guides/operator-dashboard-sso.md step 1b + config reference: drop the
  wrong checkbox instruction, document the in-band scope.

Item 4 (lift `require_role` to a composable layer) intentionally skipped as
YAGNI — only `fleet-admin` exists; revisit at the second role.

johnride reviewed 2026-06-05 11:00:04 +00:00

johnride left a comment

this feels like a lot for such a small feature, and that is bound to zitadel. What we actually want is handling jwt tokens and oidc features, not becoming hard-bound to zitadel.

The only detail that is zitadel specific is the string for project roles claim as I saw it.

Also the code is extremely bloated with comments which turns what should be a utility function into a crate and a 400 lines p-r.

I think there is value in working towards clear separation of concerns for oidc / jwt handling in harmony but this is too much too early. Extracting the core abstractions/concepts from the harmony_zitadel_auth crate or better, renaming the crate to harmony_auth or harmony_oidc and having a small zitadel module inside it would make for something easier to reason about, much less comment bloated and more scalable as we add other idp than zitadel.

this feels like a lot for such a small feature, and that is bound to zitadel. What we actually want is handling jwt tokens and oidc features, not becoming hard-bound to zitadel. The only detail that is zitadel specific is the string for project roles claim as I saw it. Also the code is extremely bloated with comments which turns what should be a utility function into a crate and a 400 lines p-r. I think there is value in working towards clear separation of concerns for oidc / jwt handling in harmony but this is too much too early. Extracting the core abstractions/concepts from the harmony_zitadel_auth crate or better, renaming the crate to harmony_auth or harmony_oidc and having a small zitadel module inside it would make for something easier to reason about, much less comment bloated and more scalable as we add other idp than zitadel.

johnride force-pushed feat/fleet-ch1-role-gate-followups from 9dd33f2671 to f316bd629b

2026-06-05 19:48:55 +00:00

Compare

johnride merged commit 76351d90d5 into feat/fleet-device-exec-logs

2026-06-09 19:44:32 +00:00

johnride deleted branch feat/fleet-ch1-role-gate-followups

2026-06-09 19:44:33 +00:00

johnride referenced this issue from a commit

2026-06-09 19:44:33 +00:00

Merge pull request 'feat(fleet-auth): unify Zitadel role extraction + request roles via scope (Ch1)' (#327) from feat/fleet-ch1-role-gate-followups into feat/fleet-device-exec-logs

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: NationTech/harmony#327