Skip to content

Cedar cannot see group claims that the upstream asserts only in its id_token #6049

Description

@JAORMX

Bug description

Cedar reads group and role claims from the pinned upstream's access token only.
Providers that assert group membership in the id_token but not in the access token
therefore produce an empty group set, so every policy of the form

permit(principal in THVGroup::"platform-eng", action == Action::"call_tool", resource);

denies for every user, with no diagnostic beyond the has-guard failing.

This is the same shape as #5916 — a claim the upstream does assert, in a token
ToolHive already holds, that the authorizer does not look at — but for
authorization-bearing claims rather than email. #6022 and #6036 fixed the
profile-claim case by supplementing name/email from that provider's id_token
and deliberately left groups out of scope, because widening the supplement newly
grants requests that deny today and so needed its own change.

The supplement allowlist is profileClaimsFromIDToken in
pkg/authz/authorizers/cedar/core.go, currently {name, email}, consumed by
supplementProfileClaims. Group extraction (extractGroups, plus
GroupClaimName/RoleClaimName and the well-known defaults) runs over the resolved
claim set, so it sees whatever that allowlist admits.

Note this is now purely a scope decision, not a trust problem: since #6036 the
supplement source is the same provider's id_token, so a group taken from it is
asserted by the same IdP that issued the access token. That is what makes this
tractable — the objection that killed the original merge-everything proposal in
#5916 was that claims must not come from the ToolHive-issued token, and they no
longer do.

Steps to reproduce

  1. Configure a workload with the embedded auth server and an OIDC upstream that puts
    group membership in the id_token only — common where groups are a profile-scope
    claim rather than an access-token claim.
  2. Attach a Cedar policy referencing groups, e.g.
    permit(principal in THVGroup::"platform-eng", action == Action::"call_tool", resource);
  3. Sign in as a user who is in platform-eng upstream, then call tools/call.

Expected behavior

The group the upstream asserted is visible to Cedar, and the policy permits.

Actual behavior

tools/list returns an empty list and tools/call returns 403, because the resolved
claim set has no groups key. The upstream asserted the group; ToolHive has the
token containing it; the authorizer does not read it.

Additional context

Why this needs care rather than a one-line allowlist addition:

  • It grants access that is currently denied. Every other change in this series
    was deny-preserving. This one changes deny to permit for anyone whose groups live
    in the id_token, which is the intended fix but must be a deliberate, released
    behaviour change rather than a side effect.
  • Precedence matters. supplementProfileClaims fills only keys the access token
    omits, so an access token asserting groups must keep winning. A provider that
    asserts a partial group list in the access token and the full list in the
    id_token would silently keep the partial one — worth deciding explicitly and
    documenting, rather than discovering later.
  • hd and custom namespaced claims are in the same category and should be
    considered in the same pass, not left as a third round.
  • The existing security boundary must survive: a group on the ToolHive-issued
    token must still never grant anything. That is pinned by
    authorization_bearing_claims_are_not_filled in core_test.go and by
    direct_groups_ignored_when_upstream_configured /
    toolhive_groups_ignored_when_upstream_configured — those tests are the invariant,
    and they should still pass unchanged. Only the id_token source is being opened up.
  • resolveClaims' doc comment currently states groups are left alone "for now" and
    says why; that reasoning needs replacing rather than deleting, so the next reader
    knows the decision was revisited deliberately.

Related: #5916, #6022, #6036, #5072 (group entity types), #6048.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingneeds-triageIssue needs initial triage by a maintainer

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions