security: keep machine identity on delegated tokens and the required_relations gate (GHSA-vq29-8q3c-3hrm)

internal/http_handlers/token_exchange.go

@@ -256,13 +256,19 @@ func (h *httpProvider) handleTokenExchangeGrant(gc *gin.Context, agent *schemas.
 	}
 
 	delegated, err := h.TokenProvider.CreateDelegatedAccessToken(&token.DelegationTokenConfig{
-		Subject:   subject,
-		Actor:     act,
-		Audience:  resource,
-		Scope:     effective,
-		ClientID:  agent.ClientID,
-		HostName:  hostname,
-		SessionID: sessionID,
+		Subject:  subject,
+		Actor:    act,
+		Audience: resource,
+		Scope:    effective,
+		ClientID: agent.ClientID,
+		HostName: hostname,
+		// Carry a machine subject's identity onto the minted token. Without
+		// this the token drops login_method entirely and service.fga
+		// re-classifies the service account as a human user
+		// (GHSA-vq29-8q3c-3hrm). onBehalfOfType was derived above from the
+		// subject_token's OWN login_method, not from anything the caller sent.
+		ServiceAccountSubject: onBehalfOfType == "agent",
+		SessionID:             sessionID,
 	})
 	if err != nil {
 		log.Debug().Err(err).Msg("failed to mint delegated token")

internal/integration_tests/fga_service_account_test.go

@@ -220,9 +220,11 @@ func TestFGAServiceAccountSubject(t *testing.T) {
 
 	// (c)+(d) A user/session token still resolves to user:<sub> and NEVER inherits
 	// the service account's grants — even though a service_account with tuples
-	// exists. A real RFC 8693 delegated token carries a user sub + an act chain
-	// and no service_account login_method, so it is classified here exactly like
-	// this user token: the guard is structural (see callerOwnSubject).
+	// exists. A USER-SUBJECT RFC 8693 delegated token carries a user sub + an act
+	// chain and no login_method, so it is classified here exactly like this user
+	// token. (A delegated token whose SUBJECT is a service account is a different
+	// case: it carries login_method=service_account and resolves to
+	// service_account:<client_id> — see TestDelegatedTokenKeepsMachineIdentity.)
 	t.Run("user token stays user:<sub> and does not inherit service_account grants", func(t *testing.T) {
 		clearCookies(ts)
 		ts.GinContext.Request.Header.Del("Authorization")

internal/integration_tests/token_exchange_session_liveness_test.go

@@ -101,8 +101,9 @@ func TestTokenExchangeServiceAccountSubjectIsExemptFromSessionCheck(t *testing.T
 // TestTokenExchangeChainedHopFollowsTheSubjectSession pins that the check reads
 // the `sid` a chained exchange carries, not just a first-hop `nonce`.
 //
-// A delegated token deliberately carries no login_method or nonce claim, so hop 2
-// resolves its session through the `sid` hop 1 stamped. If the check only ever
+// A delegated token carries no nonce claim (and, for the user subject used here,
+// no login_method either), so hop 2 resolves its session through the `sid` hop 1
+// stamped. If the check only ever
 // looked at `nonce`, hop 2 would silently skip it and a logout would stop the
 // first hop while leaving every subsequent one working.
 func TestTokenExchangeChainedHopFollowsTheSubjectSession(t *testing.T) {

internal/service/fga.go

@@ -135,21 +135,29 @@ func (p *provider) resolveFgaSubject(ctx context.Context, meta RequestMetadata,
 //
 // MACHINE vs USER vs DELEGATED — the classification keys ONLY on the token's
 // login_method claim:
-//   - login_method == constants.AuthRecipeMethodServiceAccount is stamped
-//     EXCLUSIVELY on client_credentials machine tokens
-//     (token.createMachineAccessToken). Those tokens have no resource-owner user
-//     (sub is the service account's surrogate id) and never carry an RFC 8693
-//     `act` delegation claim. Such a caller resolves to
-//     "service_account:<client_id>".
-//   - every other login_method (human recipes, sso) resolves to "user:<sub>".
+//   - login_method == constants.AuthRecipeMethodServiceAccount marks a MACHINE
+//     subject. It is stamped on client_credentials tokens
+//     (token.createMachineAccessToken) and on a delegated token whose SUBJECT is
+//     a service account (token.DelegationTokenConfig.ServiceAccountSubject).
+//     Either way `sub` is the service account's surrogate id and the caller
+//     resolves to "service_account:<client_id>".
+//   - every other login_method — INCLUDING its absence — resolves to
+//     "user:<sub>".
 //
-// This makes the delegation guard structural, not a runtime check: an RFC 8693
-// delegated token (token.CreateDelegatedAccessToken) is stateless, carries a
-// user `sub` plus an `act` chain, and carries NO login_method claim — so it can
-// never be classified as a machine subject and always resolves to "user:<sub>".
-// The security-critical rule (delegated and user tokens stay user subjects; only
-// autonomous machine tokens become service_account subjects) holds by
-// construction.
+// The absence rule is why the claim must be stamped. An earlier version relied
+// on a delegated token carrying NO login_method and concluded that a delegated
+// token "always resolves to user:<sub> by construction". That was correct for a
+// user subject and wrong for a machine one: a service account exchanging a
+// token got a credential with no login_method, so this function classified an
+// autonomous machine as a human user, flipping OpenFGA decisions from deny to
+// allow (GHSA-vq29-8q3c-3hrm). Classification follows the SUBJECT's real
+// identity, which the mint now records explicitly rather than leaving to be
+// inferred from a missing claim.
+//
+// Delegation is orthogonal to this split and is carried by actorID, not by
+// login_method: BOTH a user-subject and a machine-subject delegated token
+// resolve with a non-empty actorID, so delegationSubjects applies the
+// perms(agent) ∩ perms(subject) intersection to each identically.
 //
 // The actor is read from the same source as the subject, never from a second
 // lookup: authctx.Principal is populated ONLY by the gRPC interceptor, so a
@@ -173,17 +181,50 @@ func (p *provider) resolveFgaCaller(ctx context.Context, meta RequestMetadata) (
 	if callerID == "" {
 		return fgaCaller{}, nil
 	}
+	// actorID is carried on BOTH branches, machine included.
+	//
+	// It used to be dropped for a machine subject, on the reasoning that a
+	// machine token never carries an `act` chain so a service_account subject
+	// is never delegated. That is true of a client_credentials token — which
+	// has no `act`, so actorID is "" there anyway — but NOT of a delegated
+	// token whose SUBJECT is a service account (the multi-hop agent chain).
+	// Since such a token now correctly carries login_method=service_account it
+	// classifies as a machine subject WITH an actor, and dropping the actor
+	// would collapse authority from perms(agent) ∩ perms(subject) to
+	// perms(subject) alone — trading identity laundering for privilege
+	// widening.
+	subject, err := p.fgaSubjectFor(ctx, callerID, loginMethod)
+	if err != nil {
+		return fgaCaller{}, err
+	}
+	return fgaCaller{subject: subject, actorID: actorID}, nil
+}
+
+// fgaSubjectFor maps a (subject id, login_method) pair to its canonical OpenFGA
+// subject. It is the SINGLE source of truth for that classification, shared by
+// resolveFgaCaller (which classifies the CALLER) and enforceRequiredRelations
+// (which classifies the identity a presented token represents).
+//
+// It exists because those two used to classify differently.
+// enforceRequiredRelations hardcoded "user:<id>", so a machine identity was
+// evaluated as a human user on that surface — the same laundering primitive as
+// GHSA-vq29-8q3c-3hrm, reachable without any delegation at all: a plain
+// client_credentials token presented to validate_jwt_token with
+// required_relations was answered against "user:<service-account-row-id>",
+// so a tuple written for a user-shaped principal satisfied a gate for an
+// autonomous machine. check_permissions denied the very same token, which is
+// precisely the "two answers to one authority question" this surface's own doc
+// comment warns about.
+//
+// Fail-closed: machineFgaSubject denies on any lookup failure, a client whose
+// kind is not service_account, an inactive client, or separator smuggling in
+// the client_id. A machine subject is therefore never silently downgraded to a
+// user subject.
+func (p *provider) fgaSubjectFor(ctx context.Context, subjectID, loginMethod string) (string, error) {
 	if loginMethod == constants.AuthRecipeMethodServiceAccount {
-		subject, err := p.machineFgaSubject(ctx, callerID)
-		if err != nil {
-			return fgaCaller{}, err
-		}
-		// A machine token never carries an `act` chain (see above), so a
-		// service_account subject is never delegated. Dropping any actorID here
-		// keeps that invariant enforced rather than merely documented.
-		return fgaCaller{subject: subject}, nil
+		return p.machineFgaSubject(ctx, subjectID)
 	}
-	return fgaCaller{subject: "user:" + callerID, actorID: actorID}, nil
+	return "user:" + subjectID, nil
 }
 
 // machineFgaSubject maps an authenticated client_credentials caller — whose
@@ -299,7 +340,7 @@ func toContextualTuples(in []*model.FgaTupleInput) ([]engine.ContextualTuple, er
 // relation, same object. Two answers to one authority question is worse than
 // either answer: a gateway gating on required_relations would admit a request
 // the permission API refuses.
-func (p *provider) enforceRequiredRelations(ctx context.Context, meta RequestMetadata, log zerolog.Logger, userID string, required []*model.FgaRelationInput) error {
+func (p *provider) enforceRequiredRelations(ctx context.Context, meta RequestMetadata, log zerolog.Logger, userID, subjectLoginMethod string, required []*model.FgaRelationInput) error {
 	if len(required) == 0 {
 		return nil
 	}
@@ -316,7 +357,17 @@ func (p *provider) enforceRequiredRelations(ctx context.Context, meta RequestMet
 	if err != nil {
 		return PermissionDenied("unauthorized")
 	}
-	subjects, err := p.delegationSubjects(ctx, caller, "user:"+userID, metrics.FgaOpRequiredRelations)
+	// Classify the presented token's OWN identity, exactly as the permission
+	// APIs classify theirs. Hardcoding "user:"+userID here is what let a
+	// machine token be answered as a human user on this surface — see
+	// fgaSubjectFor.
+	subject, err := p.fgaSubjectFor(ctx, userID, subjectLoginMethod)
+	if err != nil {
+		metrics.RecordFgaCheck(metrics.FgaOpRequiredRelations, metrics.FgaResultError)
+		log.Debug().Err(err).Msg("required relations: failed to classify the token subject; denying")
+		return PermissionDenied("unauthorized")
+	}
+	subjects, err := p.delegationSubjects(ctx, caller, subject, metrics.FgaOpRequiredRelations)
 	if err != nil {
 		metrics.RecordFgaCheck(metrics.FgaOpRequiredRelations, metrics.FgaResultError)
 		log.Debug().Err(err).Msg("required relations: failed to resolve delegation subjects; denying")

internal/service/session.go

@@ -61,7 +61,7 @@ func (p *provider) Session(ctx context.Context, meta RequestMetadata, params *mo
 
 	// Fine-grained authorization gate (AND semantics, fail-closed).
 	if params != nil && len(params.RequiredRelations) > 0 {
-		if err := p.enforceRequiredRelations(ctx, meta, log, userID, params.RequiredRelations); err != nil {
+		if err := p.enforceRequiredRelations(ctx, meta, log, userID, "", params.RequiredRelations); err != nil {
 			log.Debug().Err(err).Msg("Required relations not satisfied")
 			return nil, nil, err
 		}

internal/service/validate_jwt_token.go

@@ -40,6 +40,10 @@ func (p *provider) ValidateJwtToken(ctx context.Context, meta RequestMetadata, p
 		return nil, nil, Unauthenticated("invalid token")
 	}
 	userID = sub
+	// The PRESENTED token's own login_method. It classifies the subject for the
+	// required_relations gate below, so a machine token is answered as
+	// service_account:<client_id> rather than user:<surrogate-id>.
+	subjectLoginMethod, _ := claims["login_method"].(string)
 
 	if tokenType == constants.TokenTypeAccessToken || tokenType == constants.TokenTypeRefreshToken {
 		nonceVal, ok := claims["nonce"].(string)
@@ -116,7 +120,7 @@ func (p *provider) ValidateJwtToken(ctx context.Context, meta RequestMetadata, p
 	}
 	// Fine-grained authorization gate (AND semantics, fail-closed).
 	if len(params.RequiredRelations) > 0 {
-		if err := p.enforceRequiredRelations(ctx, meta, log, userID, params.RequiredRelations); err != nil {
+		if err := p.enforceRequiredRelations(ctx, meta, log, userID, subjectLoginMethod, params.RequiredRelations); err != nil {
 			log.Debug().Err(err).Msg("Required relations not satisfied")
 			return nil, nil, err
 		}

internal/service/validate_session.go

@@ -64,7 +64,7 @@ func (p *provider) ValidateSession(ctx context.Context, meta RequestMetadata, pa
 	}
 	// Fine-grained authorization gate (AND semantics, fail-closed).
 	if params != nil && len(params.RequiredRelations) > 0 {
-		if err := p.enforceRequiredRelations(ctx, meta, log, userID, params.RequiredRelations); err != nil {
+		if err := p.enforceRequiredRelations(ctx, meta, log, userID, "", params.RequiredRelations); err != nil {
 			log.Debug().Err(err).Msg("Required relations not satisfied")
 			return nil, nil, err
 		}

internal/token/delegation_token.go

@@ -74,6 +74,21 @@ type DelegationTokenConfig struct {
 	// produce one) but a token without it can never authenticate HERE; it
 	// remains usable at the downstream resource server it was bound to.
 	SessionID string
+	// ServiceAccountSubject stamps login_method=service_account on tokens whose
+	// SUBJECT is a service account (the multi-hop agent chain, where agent A
+	// exchanges agent B's machine token — and the degenerate case where they
+	// are the same account).
+	//
+	// Without it such a token carries no login_method at all, and
+	// service.resolveFgaCaller classifies "no login_method" as a human user.
+	// A machine identity therefore laundered itself into "user:<sub>",
+	// flipping OpenFGA decisions from deny to allow and slipping past every
+	// login_method-keyed guard (GHSA-vq29-8q3c-3hrm).
+	//
+	// It is set ONLY for a service-account subject. A user-subject delegation
+	// still carries no login_method and still resolves to "user:<sub>", which
+	// is what keeps every existing delegation working unchanged.
+	ServiceAccountSubject bool
 }
 
 // DelegationSessionID encodes the memory-store coordinates of the session a
@@ -96,10 +111,18 @@ type DelegationTokenConfig struct {
 // The format mirrors ValidateAccessToken's session-key derivation
 // ("<login_method>:<user_id>|<nonce>", the login_method half omitted when the
 // token carries none) so both paths address the same entry. It is deliberately
-// NOT stamped as separate `nonce` and `login_method` claims: a `login_method`
-// claim on a delegated token would make service/fga.go classify the caller as a
-// service_account subject, silently breaking the invariant that a delegated
-// token always resolves to "user:<sub>".
+// NOT stamped as a separate `nonce` claim, which would make the stateless token
+// look addressable by the stateful validator.
+//
+// An earlier version of this comment also claimed a `login_method` claim must
+// never appear on a delegated token, because service/fga.go would then classify
+// the caller as a service_account subject and break "a delegated token always
+// resolves to user:<sub>". That invariant was itself the bug
+// (GHSA-vq29-8q3c-3hrm): it is correct for a USER subject and wrong for a
+// SERVICE-ACCOUNT one, where resolving to "user:<sub>" launders a machine
+// identity into a human one. login_method is now stamped for exactly the
+// service-account case — see DelegationTokenConfig.ServiceAccountSubject — and
+// still omitted for a user subject.
 //
 // NOTE the OIDC Back-Channel Logout token (backchannel_logout.go) also carries
 // a `sid`, and sends the BARE NONCE. The two are deliberately not identical —
@@ -162,6 +185,13 @@ func (p *provider) CreateDelegatedAccessToken(cfg *DelegationTokenConfig) (*JWTT
 	if cfg.SessionID != "" {
 		claims["sid"] = cfg.SessionID
 	}
+	// Carry a machine subject's identity forward. See ServiceAccountSubject:
+	// omitting this let a service account's delegated token be classified as a
+	// human user. Deliberately NOT set for a user subject, whose absence of the
+	// claim is what keeps it resolving to "user:<sub>".
+	if cfg.ServiceAccountSubject {
+		claims["login_method"] = constants.AuthRecipeMethodServiceAccount
+	}
 	signed, err := p.signJWTToken(claims, accessTokenJWTType)
 	if err != nil {
 		return nil, err