[Design Discussion] Make auth observability events principal-aware #5018
lashinijay
started this conversation in
Design
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Related Feature Issue
#4142
Problem Summary
High-Level Approach
The proposal is to enrich the existing event payloads rather than introduce new event types or a new pipeline. Every auth event already carries a free-form Data map that flows through the console, file and OpenTelemetry sinks untouched, so the work is about deciding which identity facts to put in that map, and plumbing the few that are not already available at the publish site.
Architecture Overview
This is an enrichment to already publishing diagnostic logs, plus two new data paths that carry identifiers across request boundaries. The single structural addition is an existing provider being injected into the token builder.
Concretely, the additions are:
act_type,sub_type,sub_id,act_sub,is_delegated,correlation_id)system/observability/eventApplication.EntityCategoryjson:"-" yaml:"-")pkg/thunderidengine/providersTokenDTO.ActorSub,TokenDTO.SubjectID,TokenDTO.SubjectCategory,TokenResponseDTO.CorrelationIDoauth/oauth2/modelAuthorizationCode.CorrelationID,AuthorizationCode.SubjectID,AuthorizationCode.SubjectCategoryoauth/oauth2/authzcorrelation_id,sub_idandsub_typeassertion claimsoauth/oauth2/constantsAccessTokenBuildContext.SubjectEntityIDoauth/oauth2/tokenserviceActorProviderdependencyoauth/oauth2/tokenserviceEvents report who the subject is (
sub_id) and what kind of principal they are (sub_type). Both are needed: the identity answers "what happened for this principal", and the category answers "was this a user login or an agent-for-agent exchange", which is the primary requirement.The value published is the subject's entity resource ID, which is not always the same as the
subclaim on the issued token.Application.SubjectAttributeis an existing per-user-type mapping that sets the tokensubto any schema attribute that is unique, required, and string-typed, so on a deployment mappingemailorusernamethe token subject is a directly identifying attribute. Publishing that onto events would put personal data into the log and trace sinks under a configuration this change does not control.The resource ID avoids that and is better on three further counts:
subin two applications, so the token subject is not a usable join key; the resource ID is.ActorProvider.GetActoris keyed on, so resolvingsub_typeworks on every deployment rather than silently failing wherever a subject mapping is configured.Naming follows the token vocabulary rather than inventing a parallel one for events:
sub_idandsub_typefor the subject,act_subandact_typefor the actor. One set of names across claims and events, and it matches the axis symmetry #5042 anticipates.One consequence to document for consumers: on a deployment that configures
Application.SubjectAttribute, an event'ssub_idand the corresponding token'ssubclaim hold different values - the resource ID and the mapped attribute respectively. They coincide on every other deployment. Anything correlating events against token contents must join on the resource ID rather than on the claim.The pre-existing
user_idon flow events is retired in favor ofsub_id, as a clean break with no deprecation period. The subject may be an agent - one authenticating through a flow, or one standing as the subject of a token exchange — so auser_-prefixed key encodes exactly the assumption this change exists to remove; it is populated from an entity reference that is already an agent's ID whenever an agent authenticates, so the name is only correct by accident today.The user_id key is write-only: it is published in
flowexec/engine.goand read nowhere else in the backend, the Console, or the tests, so nothing inside ThunderID depends on it and the only possible consumers are external. Observability is disabled by default, so the population of deployments with any consumer at all is limited to those that opted in. And the key is unreliable today — it is gated onctx.AuthenticatedUser.IsAuthenticated, and #4142 reports the authenticated subject missing from live AUTHENTICATION flow events, which is why this proposal reads the subject from the entity reference instead. Publishing both keys during a deprecation window would mean steering consumers toward the less reliable of the two while it remains the documented one.This gives a symmetric vocabulary across the two axes:
act_subact_typesub_idsub_typeAlignment with the
sub_typeclaim (#5042)#5042 adds a
sub_typeclaim toclient_credentialstokens with valuesapplicationandagent, derived from the sameOAuthClient.EntityCategorythis proposal reads. Its naming was researched against the IANA JWT registry and the relevant IETF drafts and argued in #3578.This proposal adopts that vocabulary rather than inventing a parallel one.
act_typeandsub_typepublishuser,agent, andapplication.sub_typedirectly. That is a usable source forsub_typeon exactly the agent-for-agent case this proposal must not misreport as a user.Integration with the existing observability pipeline
The pipeline is untouched. Events are still constructed with
event.NewEvent(...), published throughObservabilityProvider.PublishEvent, and fanned out to the console, file and OpenTelemetry subscribers, none of which need to know about the new fields - they serialize theDatamap generically. The OTel subscriber already turns every data key into a span attribute, so the new fields become queryable Jaeger tags with no subscriber change.Two publish sites are enriched, each reading from a source that already exists in its layer:
oauth/oauth2/token(TOKEN_ISSUANCE_STARTED,TOKEN_ISSUED,TOKEN_ISSUANCE_FAILED)act_type,app_id,sub_id,sub_type,act_sub,is_delegated,correlation_idOAuthClient.EntityCategory(already populated byinboundclientwhen the client is loaded byclient_id); the access-token DTO, which now carries the subject's resource ID and category alongside the token subject it already heldflow/flowexec(FLOW_*,FLOW_NODE_EXECUTION_*)act_type,client_id,sub_id,sub_type,correlation_idApplication.EntityCategory(set byactorproviderfrom the entity it already loads);AuthUser.EntityReference(set by the authn providers)servicemanager.gois unchanged - there is no newInitializeto register. Neither publish site gains a dependency; the only injection isActorProviderinto the token builder, described next.Resolving the subject once, where the token is built
The subject's identity and category are recorded on the token DTO in
BuildAccessToken, which every grant passes through, next to where the actor is already recorded fromActorClaims. Flow events need no equivalent step:AuthUser.EntityReferencealready carries both the entity ID and its category, set by the authn providers.client_credentials)OAuthClient.IDOAuthClient.EntityCategory— short-circuit, no lookupauthorization_codesub_idassertion claim, via the authorization codesub_typeassertion claim — no lookuprefresh_tokenActorProvider.GetActor(sub)sub_type(#5042) when present, elseActorProvider.GetActor(sub)The builder resolves the category only when the grant handler has not already supplied one. Where an upstream layer knows the answer for free the lookup is skipped, and where it does not the builder resolves it - so the login path costs nothing and no grant can end up with the field unset by accident.
That last property is the reason resolution lives here at all. Every grant must pass through
BuildAccessTokento mint an access token, so a grant added later inherits the behavior without its author knowing this document exists. The alternative considered was to make each component that establishes a subject responsible for carrying the category forward, with no resolution in the builder at all. That spreads one fact across four independent plumbing points and makes omission silent, since an absent field is indistinguishable from an unresolvable one. Treating the upstream value as a fast path over a guaranteed fallback keeps both properties rather than trading one for the other.Entity reads are served by the cache-backed entity store, so the fallback lookup is normally an in-memory hit.
The two data paths
An authentication spans several HTTP requests, so no request-scoped value can join them. The correlation id and the subject's resource ID therefore travel on carriers that already cross those boundaries, following the route the token family id (
tfid) already takes:sub_idis a new claim because the assertion does not currently carry the resource ID. Itssubclaim holds the mapped token subject, and the field nameduserIDin the parsed claims is populated from that samesub- so despite the name, no resource ID survives the assertion today.sub_typerides the same carrier.AuthUser.EntityReferencealready holds the subject's category at the point the assertion is minted, so stamping it costs nothing and spares the login path an entity read it would otherwise pay on every issuance. The builder still resolves the category when no claim arrives, so this is a fast path rather than the only path.Both carriers are schemaless with respect to this change: the assertion is a JWT (a new claim needs no migration) and the authorization code is a JSON document in the runtime store keyed by
NamespaceAuthzCode(a new field needs no migration). No schema change and no data migration are required.Grants with no originating flow (
client_credentials, token exchange) fall back to the request trace id for correlation, so that field is uniformly populated without a special case at the consumer.Extension-surface consideration
Application.EntityCategorywill be added topkg/thunderidengine/providers, which is the engine's SPI rather than an internal package. It is markedjson:"-" yaml:"-"so it never appears in the application API or in declarative resources, and it is re-derived on every flow-context load rather than persisted. A third-partyActorProviderimplementation that does not set it simply leaves it empty, in which caseact_typeis omitted from flow events and everything else behaves as before — the degradation is graceful rather than an error.Operational impact
sub_idandact_subare opaque identifiers, the latter only on delegated issuance. For the OTel sink these are span attributes rather than new spans, so trace counts are unchanged.sub_idis a per-principal value, so it is the one attribute here with meaningful cardinality on the OTel sink. It is bounded by the number of distinct principals authenticating, not by request volume.client_credentialsreads the category off the client it already loaded, andauthorization_codereads it off the flow assertion, so neither adds a lookup. A cache-backed entity read remains as the fallback for grants whose subject arrives without a category - refresh where the family did not carry one, and the exchange grants - and it happens during token construction rather than event publication, so it is on the response path when it does occur.FLOW_STARTEDandFLOW_FAILEDmove from the request trace into their siblings' flow trace.user_idis retired from flow events in favor ofsub_id. Together with the trace-grouping change, these are the only two changes visible to an existing consumer.PII considerations
Observability sinks have a different profile from the token itself: they are retained longer, shipped off-host, and readable by operators who are not token holders. Anything published onto an event should be assessed against that, not against what the client already receives.
The codebase already takes a position on this. Application logs record user identifiers through
log.MaskedString, which keeps only the first and last character of the value, and always log the resource ID rather than a human-readable attribute. The event pipeline has no equivalent masking — it serializes theDatamap verbatim — so what is put on an event must be safe unmasked.Every identifier this change publishes is opaque or categorical:
act_typeuser|agent|applicationsub_typeuser|agent|applicationis_delegatedsub_idact_suboauthApp.IDor the refresh token'sact.subapp_id/client_idcorrelation_idThe field that would have carried personal data is the token's
subclaim, and it is not published. Where a deployment mapsemailorusernamethroughApplication.SubjectAttribute, that value is the token subject — so publishingsubwould have put a directly identifying attribute into the sinks, conditional on a configuration this change does not control. Publishing the resource ID keeps the mapping unreachable from the event path entirely, rather than safe-by-default and exposed under a configuration a deployment may reasonably choose.The stream therefore stays free of directly identifying data regardless of how subject attributes are configured, while still supporting per-subject correlation through a stable opaque key.
Security considerations
The change publishes more context onto events that already exist and adds two identifiers that cross request boundaries.
Data published to the sinks. The PII assessment above is the primary control: everything published is an opaque resource identifier or a low-cardinality enum, and no directly identifying value becomes publishable under any configuration. The change publishes no token values, no client secrets, no assertions, and no authentication factors. Error descriptions were already published by both sites and are unchanged.
The correlation identifier is a handle for a live flow. The value carried is the flow execution id, which is also the continuation handle a client presents to resume an in-progress authentication — the flow service resolves it and rejects an unknown one with
ErrorInvalidExecutionID. Publishing it holds for three reasons:execution_idbefore this change;correlation_idduplicates a value the same event carried.TOKEN_ISSUEDfires, the id no longer resolves to a resumable context. What reaches the token sinks is a spent handle.TokenResponseDTO.CorrelationIDis an internal field, not serialized on the token response, and the authorization code it arrives on is a server-side document in the runtime store.The residual risk is an operator with log access replaying a still-live execution id observed on a flow event — unchanged from before this change, since
execution_idwas already there.The new assertion claims are not trust inputs.
correlation_idandsub_idare added to the internal flow assertion, which is signed, so both inherit the assertion's integrity and a tampered value fails verification like any other. Neither is used to authorize, to gate a decision, or to select a code path:correlation_idis only stamped onto events, andsub_idis used to report the subject and to key a category lookup whose failure mode is an omitted field. A wrong value produces a mis-stitched trace or an absent category, not a wrong authorization outcome. In particular,sub_iddoes not replace the token'ssubclaim and does not participate in issuing or validating tokens.Signal integrity for the fields that could mislead.
sub_typeis resolved from the subject's own entity record rather than inferred from the request, specifically so an agent acting as a token subject is not reported as a user. Where it cannot be resolved it is omitted rather than defaulted, so a consumer never sees a confident wrong answer. The same applies toact_type. This matters because these fields will be read as evidence about who did what — a field that guesses is worse than a field that is absent.No new external surface.
Application.EntityCategoryis added to the engine SPI but markedjson:"-" yaml:"-", so it appears on no API response and in no declarative resource, and it is re-derived rather than persisted. No schema change, no migration, no new endpoint, no new configuration toggle.Impacted Areas
Alternatives Considered
Leave the join to the consumer. Keep publishing
client_idand let whoever reads the logs resolve it against the application or agent API to learn what kind of principal it was. Rejected because it pushes a join onto every consumer, requires each of them to hold API credentials, and cannot be done retroactively — once an agent is deleted, its historical events become uninterpretable. Enriching at publish time makes the event self-describing for as long as it is retained.is_agentboolean instead ofact_type. Rejected because a boolean collapsesuser,agentandapplicationinto two states and cannot express the subject side at all, so a second boolean would be needed almost immediately. The enum costs the same to publish and extends without a schema decision each time a principal kind is added.Publish the token's
subclaim as the subject. The literal reading of "Add subject" in #4142. Rejected becausesubis deployment-configurable throughApplication.SubjectAttributeand may be an email address or username, which would put personal data into the sinks; because it varies per application for the same principal, making it useless as a join key; and because it is not whatGetActoris keyed on, so it cannot resolve the subject's category. Publishing the entity resource ID satisfies the requirement without any of the three problems.Publish nothing for the subject's identity, only its category. An earlier draft of this proposal, on the grounds that identity serves audit rather than diagnostics. Rejected once it was clear that the resource ID carries no personal data: the scope argument was really an argument against publishing
subspecifically, and it does not extend to an opaque identifier. Omitting identity altogether would have left "what happened for this principal" unanswerable and left the category lookup broken on subject-mapped deployments, for no privacy gain.Publish a hashed or pseudonymized subject. A middle option that would preserve per-subject correlation without a plaintext identifier. Rejected because a hash of a low-entropy value such as an email address is recoverable by dictionary, so it is pseudonymization rather than anonymization and remains personal data — and because the resource ID already provides correlation with no reversibility question at all.
Name the field
user_id, matching the existing flow-event key. Rejected because the subject may be an agent, so the name would assert a principal class the change exists to stop assuming. The existinguser_idis retired for the same reason rather than carried forward as an alias, since shipping both would guarantee dashboards built on the wrong one.Mint a dedicated correlation identifier. Rather than reusing the flow execution id, generate a fresh id at the start of an authentication and carry it. Rejected because it adds a second identifier that means almost the same thing as one the system already has, and every consumer would then need to know which of the two to pivot on. The execution id already exists, is already published on flow events, and already survives the request boundaries that matter.
Reuse the token family id (
tfid) as the correlation key. It travels the exact route this change needs, so it was the obvious candidate. Rejected because it does not exist early enough: flow events are published before any token is minted, so the flow half of the trace could not carry it. It is also scoped to a token family for revocation purposes, which is a different lifetime from one authentication.Resolve the subject's category only in the builder, with no upstream fast path. The simplest version of the choke point:
BuildAccessTokenlooks the category up for every grant exceptclient_credentials. Rejected because it charges the login path a lookup for a fact the flow already had in hand, and the flow assertion is already being extended forsub_idandcorrelation_id, so carrying the category costs one more claim on a carrier that is being touched anyway.Carry the category forward from each component, with no resolution in the builder. The mirror image: every component that establishes a subject sets the category, and the builder assumes it is present. Rejected because it spreads one fact across four independent plumbing points and makes omission silent — a grant added later would emit no category, and an absent field is indistinguishable from an unresolvable one, so nothing would surface the mistake. The adopted design takes the upstream value when it exists and resolves it otherwise, which keeps the no-lookup property of this option and the cannot-be-forgotten property of the previous one.
Keep
FLOW_STARTEDandFLOW_FAILEDon the request trace. Addingcorrelation_idalone would stitch the flow together without moving any event between traces, avoiding one of the two consumer-visible behavior changes here. Rejected because those two events would remain separated from their own children in any trace-native view, which is where an operator actually looks first; a correlation field that only works in log search solves half the problem the issue describes.All reactions