Skip to content

Identity: Move actor JWT/cert minting into the main API - #1315

Merged
Taahir Ahmed (ahmedtd) merged 1 commit into
agent-substrate:mainfrom
ahmedtd:merge-broker
Sep 11, 2026
Merged

Taahir Ahmed (ahmedtd) merged 1 commit into
agent-substrate:mainfrom
ahmedtd:merge-broker

Conversation

@ahmedtd

@ahmedtd Taahir Ahmed (ahmedtd) commented Aug 29, 2026 •

Copy link
Copy Markdown
Collaborator

This was originally a separate service to make it easy to apply separate authentication and authorization interceptors.

It now seems clear that our authn/z framework will be strong enough to support atelet and external callers in one system (based on OpenFGA).

This change moves the MintJWT and MintCert RPCs into the control API, and removes some inline authz checks that will be handled by our unified authorizer framework.

@ahmedtd

Copy link
Copy Markdown
Collaborator Author

Whoops, this isn't ready to review yet.

@ahmedtd Taahir Ahmed (ahmedtd) changed the title Merge broker Identity: Move actor JWT/cert minting into the main API Sep 3, 2026
@ahmedtd
Taahir Ahmed (ahmedtd) force-pushed the merge-broker branch 2 times, most recently from 943838f to 10a0120 Compare September 4, 2026 20:16
@ahmedtd
Taahir Ahmed (ahmedtd) marked this pull request as ready for review September 4, 2026 20:17
@ahmedtd

Copy link
Copy Markdown
Collaborator Author

Ready for review now

ObjectRef tag = 1;
}

message MintActorJWTRequest {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] missing doc comment here and a few places below (looks like tests are complaining about this)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread cmd/ateom-gvisor/main.go
CredentialBundlePath: s.workerCredentialBundlePath,
TrustBundlePath: s.podIdentityTrustBundlePath,
ExpectedActorUID: actorUID,
ActorUID: actorUID,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, fixed. (Also in ateom-microvm)

Comment thread cmd/ateom-microvm/main.go Outdated
TrustBundlePath: s.podIdentityTrustBundlePath,
ExpectedActorUID: actorUID,

ActorUID: actorUID,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

if err != nil {
t.Fatalf("CreateActor failed: %v", err)
}
_, err = tc.client.MintActorJWT(t.Context(), &ateapipb.MintActorJWTRequest{

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check the error here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return errs
}

func (s *RPCService) MintActorJWT(ctx context.Context, req *ateapipb.MintActorJWTRequest) (*ateapipb.MintActorJWTResponse, error) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old RPC handlers called these auto-generated validation functions like so:


func validateMintJWTRequest(ctx context.Context, req *ateapipb.MintJWTRequest) field.ErrorList {
	// Call the generated validation.
	op := operation.Operation{Type: operation.Create}
	return controlapi.Validate_MintJWTRequest(ctx, op, nil, req, nil)
}

func validateMintCertRequest(ctx context.Context, req *ateapipb.MintCertRequest) field.ErrorList {
	// Call the generated validation.
	op := operation.Operation{Type: operation.Create}
	return controlapi.Validate_MintCertRequest(ctx, op, nil, req, nil)
}

I believe we need to copy that over to enforce the tag-based validation?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Done.

}

// We only issue tokens with audience bindings.
if len(req.GetAudience()) == 0 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old handler also validated the issuer (caller.Issuer != s.actorIdentityJWTIssuer), should we do that in the new handler as well?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That didn't make any sense -- you shouldn't have to authenticate with an Actor JWT in order to mint an Actor JWT. I would think typically, the thing calling this RPC would be the egress gateway, and it would be authenticating with a k8s client certificate.


// Verify that this actor exists in the store. It doesn't need to be
// running, since we may need to issue JWTs during actor boot / resume.
dbActor, err := s.impl.GetActor(ctx, resources.ActorRefFromObjectRef(req.GetActor()))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, should we check that the actor is in a RUNNING state (or in some set of valid states) before minting tokens or certs for it?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure. I think it's likely that components will need to issue credentials even if the actor is actively running (think about a frontend harness service that might need to access files, etc with actor identity, even if the LLM harness bits inside the actor proper are not currently running).

I think "not deleted" is the safest for now.

return nil, fmt.Errorf("while retrieving actor: %w", err)
}
if dbActor.GetMetadata().GetUid() != req.GetActorUid() {
return nil, status.Error(codes.Aborted, "conflict; actor has been deleted and recreated")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like egress.renew() only stops retrying on FailedPrecondition or PermissionDenied:

if code == codes.FailedPrecondition || code == codes.PermissionDenied {

Should we add Aborted to the list of codes checked there, or adjust the code returned here, to stop it from retrying?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FailedPrecondition is like, the one single code you are not supposed to retry on. It means you need to fix something about your request or the state of the system.

I'll change that to Aborted.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, I read that backwards. As you point out, those are the codes it's not retrying on. I've added Aborted to that code list.

@ahmedtd
Taahir Ahmed (ahmedtd) force-pushed the merge-broker branch 2 times, most recently from f45aa7e to 28a47df Compare September 10, 2026 23:09
@ahmedtd
Taahir Ahmed (ahmedtd) force-pushed the merge-broker branch 2 times, most recently from 233cc21 to 2781b13 Compare September 11, 2026 05:12
This was originally a separate service to make it easy to apply separate
authentication and authorization interceptors.  It now seems clear that
they should be in controlapi, with the same authn/z handlers chosen
there.

* Remove checks that will be handled in the authorizer framework.
@ahmedtd
Taahir Ahmed (ahmedtd) merged commit dc675e8 into agent-substrate:main Sep 11, 2026
9 checks passed
pull Bot pushed a commit to warmchang/substrate that referenced this pull request Sep 25, 2026
…der (agent-substrate#1834)

Part of agent-substrate#1756.

- Adds `--actor-jwt-issuer`. Unset, it defaults to
`https://idp.<namespace>.svc`, the Service ate-idp-server will serve
discovery from. Before this, `iss` was hardcoded to
`https://api.ate-system.svc`, which points at ateapi's gRPC port.
- Adds `oidcdiscovery.ParseIssuer`, which requires a canonical https URL
with no query, fragment, or user info and strips trailing slashes.
ate-idp-server will use it too, so both emit the same bytes.
- Actor JWT headers now include `typ: JWT`.
- Drops the `actorIdentityJWTIssuer` plumbing into `RPCService`, unused
since agent-substrate#1315.

The default `iss` changes, but `MintActorJWT` has no production caller
yet.

Testing: new unit tests for the parser, the default, the header, and
flag resolution. `TestMintActorJWT_Success` now checks `typ`, `iss`, and
`sub`. It needs Docker, so it hasn't run locally.

- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR (docs
land later in the agent-substrate#1756 series)
jiayimeow pushed a commit to jiayimeow/substrate that referenced this pull request Oct 19, 2026
…entity docs (agent-substrate#2597)

Part of agent-substrate#1756.

**Why:** every install still has to set `actorIdentityJWTProvider`, and
`docs/authentication.md` says it decides who may mint actor JWTs. It no
longer decides anything:

- agent-substrate#757 added it when minting lived in a separate `ActorIdentity`
service. `MintJWT` refused any caller whose JWT came from a different
provider's issuer, which in practice limited minting to atelet's
Kubernetes service account tokens.
- agent-substrate#1315 moved minting into `ateapi.Control` and dropped that check,
leaving caller limits to the authz framework.
- agent-substrate#1834 removed the last code that read the provider's issuer.

So ate-api-server requires a field and fails to start if it names a
missing provider, yet nothing uses it, and the docs say it decides who
may mint, which authorization now decides. Section 7 of
`docs/api-guide.md` still describes the old service and its atelet-only
rules.

**What changes:**

- `actorIdentityJWTProvider` is gone from ate-api-server's
authentication config, and ate-setup stops writing it.
- `docs/authentication.md` drops it and keeps the pointer to
`authorization.md` for what a caller may do.
- Section 7 of `docs/api-guide.md` now describes `MintActorJwt` and
`MintActorCertificate` on `ateapi.Control`, who may call them, their
fields, and the JWT's claims, instead of the old service, its caller
checks, and a wrong SPIFFE path. The ADK paragraph, which pointed at the
same service, now describes the egress gateway attaching actor JWTs.

For reviewers: the config is parsed strictly and ate-setup only creates
`ate-api-authentication` when it's missing, so an existing install must
delete the `actorIdentityJWTProvider` line from that ConfigMap before
ate-api-server restarts on this version.

Testing: unit tests for the config loader and ate-setup's rendered
config. I didn't run e2e; CI's e2e installs a fresh kind cluster, so it
doesn't exercise the upgrade case.

- [ ] Tests pass
- [x] Appropriate changes to documentation are included in the PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants