From 8eeab93d20391688e4bd20839a54782ed7b48209 Mon Sep 17 00:00:00 2001 From: Matt Jenkinson <75292329+mattdjenkinson@users.noreply.github.com> Date: Fri, 1 May 2026 17:09:44 +0100 Subject: [PATCH] feat(activity): enrich activities with iam User display names Resolve actors and User-typed link targets to human-readable display names so the UI can show names instead of raw emails / UIDs. - ActivityActor gets a new omitempty DisplayName field; ActivityLink gets DisplayName + Email (populated only when the link's resource is an iam User). - New UserResolver interface in internal/processor/userlookup.go with NoopUserResolver and a CachedUserResolver wrapper (TTL, single-flight, negative caching). - ResolveActorWithResolver populates the actor's DisplayName from the iam User's spec.givenName + spec.familyName via the resolver. - ActivityBuilder gains a UserResolver and rewrites the summary at build time: substitutes actor.Name (typically email) with actor.DisplayName, upgrades existing actor link() entries in place (or appends a synthetic actor link), and hydrates User-typed link targets via the resolver. - *WithResolver overloads on Evaluate{Audit,Event}Rules and EvaluateCompiledAuditRules let callers opt into enrichment without changing the existing public API. - IAMUserResolver in internal/activityprocessor uses a controller-runtime client to fetch iam.miloapis.com/v1alpha1/User CRs as Unstructured (no milo Go-types dependency). - The processor wires NewCachedUserResolver(NewIAMUserResolver(...)) at startup; resolver errors are silently ignored, so the activity is still emitted with raw values when lookup fails. All new optional fields are backwards compatible: consumers that don't know about DisplayName continue to work, and once they recognise it they get richer data automatically. Tests: - internal/processor/userlookup_test.go: cache hit, neg-cache, TTL expiry, single-flight collapse, error-not-cached, empty-key short-circuit. - internal/processor/enrichment_test.go: actor replacement, link upgrade in place, actor-not-in-summary, user link hydration, non-user link untouched, ResolveActorWithResolver paths. --- internal/activityprocessor/policycache.go | 21 +- internal/activityprocessor/processor.go | 10 +- internal/activityprocessor/userresolver.go | 101 ++++++++ internal/processor/activity.go | 116 +++++++++- internal/processor/classifier.go | 18 ++ internal/processor/enrichment_test.go | 232 +++++++++++++++++++ internal/processor/evaluate.go | 38 ++- internal/processor/userlookup.go | 177 ++++++++++++++ internal/processor/userlookup_test.go | 185 +++++++++++++++ pkg/apis/activity/v1alpha1/types_activity.go | 26 +++ 10 files changed, 916 insertions(+), 8 deletions(-) create mode 100644 internal/activityprocessor/userresolver.go create mode 100644 internal/processor/enrichment_test.go create mode 100644 internal/processor/userlookup.go create mode 100644 internal/processor/userlookup_test.go diff --git a/internal/activityprocessor/policycache.go b/internal/activityprocessor/policycache.go index 8de25fd1..67b9b208 100644 --- a/internal/activityprocessor/policycache.go +++ b/internal/activityprocessor/policycache.go @@ -464,11 +464,27 @@ func (r *CompiledRule) EvaluateEventMatch(eventMap map[string]any) (bool, error) // EvaluateCompiledAuditRules evaluates pre-compiled audit rules against an audit event. // Returns the generated Activity, the matching rule index, and any error. // Returns (nil, -1, nil) if no rule matched. +// +// Use EvaluateCompiledAuditRulesWithResolver to also enrich the resulting +// activity with user display names; this convenience wrapper passes nil. func EvaluateCompiledAuditRules( policy *CompiledPolicy, auditMap map[string]any, audit *auditv1.Event, resolveKind processor.KindResolver, +) (*v1alpha1.Activity, int, error) { + return EvaluateCompiledAuditRulesWithResolver(policy, auditMap, audit, resolveKind, nil) +} + +// EvaluateCompiledAuditRulesWithResolver is like EvaluateCompiledAuditRules +// but enriches the activity actor and any User-typed link targets with +// display names looked up via resolver. +func EvaluateCompiledAuditRulesWithResolver( + policy *CompiledPolicy, + auditMap map[string]any, + audit *auditv1.Event, + resolveKind processor.KindResolver, + resolver processor.UserResolver, ) (*v1alpha1.Activity, int, error) { for i := range policy.AuditRules { rule := &policy.AuditRules[i] @@ -491,8 +507,9 @@ func EvaluateCompiledAuditRules( } builder := &processor.ActivityBuilder{ - APIGroup: policy.APIGroup, - Kind: policy.Kind, + APIGroup: policy.APIGroup, + Kind: policy.Kind, + UserResolver: resolver, } activity, err := builder.BuildFromAudit(audit, summary, links, resolveKind) if err != nil { diff --git a/internal/activityprocessor/processor.go b/internal/activityprocessor/processor.go index 22a3a154..59ee1ee8 100644 --- a/internal/activityprocessor/processor.go +++ b/internal/activityprocessor/processor.go @@ -294,6 +294,10 @@ type Processor struct { // dlqRetryController handles automatic retry of DLQ events. dlqRetryController *DLQRetryController + // userResolver enriches activities with iam User display names. nil + // disables enrichment; activities are emitted with raw emails/IDs. + userResolver processor.UserResolver + wg sync.WaitGroup ctx context.Context cancel context.CancelFunc @@ -393,6 +397,10 @@ func (p *Processor) Start(ctx context.Context) error { // Create event emitter for health reporting p.eventEmitter = NewEventEmitter(k8sClient, recorder) + // Wire a cached iam User resolver so activities are enriched with + // human-readable display names. Failures fall back to email/UID. + p.userResolver = processor.NewCachedUserResolver(NewIAMUserResolver(k8sClient), 0, 0) + // Build NATS connection options natsOpts := []nats.Option{ nats.Name("activity-processor"), @@ -1034,7 +1042,7 @@ func (p *Processor) processMessage(msg *nats.Msg) error { // evaluateCompiledAuditRules evaluates audit rules using pre-compiled CEL programs. func (p *Processor) evaluateCompiledAuditRules(policy *CompiledPolicy, auditMap map[string]any, audit *auditv1.Event) (*v1alpha1.Activity, int, error) { - return EvaluateCompiledAuditRules(policy, auditMap, audit, p.resourceToKind) + return EvaluateCompiledAuditRulesWithResolver(policy, auditMap, audit, p.resourceToKind, p.userResolver) } // auditToMap converts an audit event to a map for CEL evaluation. diff --git a/internal/activityprocessor/userresolver.go b/internal/activityprocessor/userresolver.go new file mode 100644 index 00000000..e1980730 --- /dev/null +++ b/internal/activityprocessor/userresolver.go @@ -0,0 +1,101 @@ +package activityprocessor + +import ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + + "go.miloapis.com/activity/internal/processor" +) + +// userGVK identifies the iam User custom resource we resolve display names +// from. The activity processor never serves these objects, so we query them +// as Unstructured to avoid pulling in the milo iam Go types. +var userGVK = schema.GroupVersionKind{ + Group: "iam.miloapis.com", + Version: "v1alpha1", + Kind: "User", +} + +// IAMUserResolver implements processor.UserResolver against an iam User CR +// store reached through a controller-runtime client. It is safe for +// concurrent use; wrap with processor.NewCachedUserResolver to add caching +// and per-key single-flight. +type IAMUserResolver struct { + Client client.Client +} + +// NewIAMUserResolver returns a resolver that fetches iam Users via c. +func NewIAMUserResolver(c client.Client) *IAMUserResolver { + return &IAMUserResolver{Client: c} +} + +// LookupByEmail finds the first iam User whose spec.email matches the given +// address. Returns ok=false when no match is found or email is empty. +func (r *IAMUserResolver) LookupByEmail(ctx context.Context, email string) (processor.UserInfo, bool, error) { + if email == "" || r == nil || r.Client == nil { + return processor.UserInfo{}, false, nil + } + + list := &unstructured.UnstructuredList{} + list.SetGroupVersionKind(schema.GroupVersionKind{ + Group: userGVK.Group, + Version: userGVK.Version, + Kind: userGVK.Kind + "List", + }) + + // Most clusters do not index spec.email server-side; list all and filter + // in process. The cached wrapper amortizes this across calls; if scale + // becomes a concern, register a field indexer for spec.email in the + // manager's cache. + if err := r.Client.List(ctx, list); err != nil { + return processor.UserInfo{}, false, fmt.Errorf("list iam users: %w", err) + } + + for i := range list.Items { + item := &list.Items[i] + got, _, _ := unstructured.NestedString(item.Object, "spec", "email") + if got == email { + return userInfoFromUnstructured(item), true, nil + } + } + + return processor.UserInfo{}, false, nil +} + +// LookupByName fetches an iam User by metadata.name and returns its display +// fields. Returns ok=false on NotFound. +func (r *IAMUserResolver) LookupByName(ctx context.Context, name string) (processor.UserInfo, bool, error) { + if name == "" || r == nil || r.Client == nil { + return processor.UserInfo{}, false, nil + } + + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(userGVK) + + if err := r.Client.Get(ctx, client.ObjectKey{Name: name}, obj); err != nil { + if apierrors.IsNotFound(err) { + return processor.UserInfo{}, false, nil + } + return processor.UserInfo{}, false, fmt.Errorf("get iam user %q: %w", name, err) + } + + return userInfoFromUnstructured(obj), true, nil +} + +func userInfoFromUnstructured(obj *unstructured.Unstructured) processor.UserInfo { + given, _, _ := unstructured.NestedString(obj.Object, "spec", "givenName") + family, _, _ := unstructured.NestedString(obj.Object, "spec", "familyName") + email, _, _ := unstructured.NestedString(obj.Object, "spec", "email") + return processor.UserInfo{ + Name: obj.GetName(), + GivenName: given, + FamilyName: family, + Email: email, + UID: string(obj.GetUID()), + } +} diff --git a/internal/processor/activity.go b/internal/processor/activity.go index 84ded593..48b1073c 100644 --- a/internal/processor/activity.go +++ b/internal/processor/activity.go @@ -1,10 +1,12 @@ package processor import ( + "context" "crypto/sha256" "encoding/hex" "encoding/json" "fmt" + "strings" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -15,6 +17,10 @@ import ( "go.miloapis.com/activity/pkg/apis/activity/v1alpha1" ) +// iamGroup is the API group for Milo IAM resources we enrich with user +// display names. +const iamGroup = "iam.miloapis.com" + // activityName generates a deterministic activity name from the origin event // identifier and the policy's resource target. The same input always produces // the same name, enabling NATS message deduplication on retries. @@ -35,6 +41,10 @@ type ActivityBuilder struct { // Resource information from the policy APIGroup string Kind string + + // UserResolver is consulted (when non-nil) to enrich the actor and any + // User-typed link targets with human-readable display names. + UserResolver UserResolver } // BuildFromAudit constructs an Activity from an audit event. @@ -64,8 +74,9 @@ func (b *ActivityBuilder) BuildFromAudit( resourceUID := extractResponseUID(audit.ResponseObject) // Classify change source and resolve actor + ctx := context.Background() changeSource := ClassifyChangeSource(audit.User) - actor := ResolveActor(audit.User) + actor := ResolveActorWithResolver(ctx, audit.User, b.UserResolver) tenant := ExtractTenant(audit.User) // Generate activity name @@ -77,6 +88,10 @@ func (b *ActivityBuilder) BuildFromAudit( return nil, fmt.Errorf("%w: %v", ErrActivityBuild, err) } + // Enrich: replace actor email with display name in summary, attach actor + // link, and hydrate any User-typed link targets with display names. + summary, activityLinks = enrichSummaryWithDisplayNames(ctx, summary, actor, activityLinks, b.UserResolver) + return &v1alpha1.Activity{ TypeMeta: metav1.TypeMeta{ APIVersion: v1alpha1.SchemeGroupVersion.String(), @@ -115,6 +130,101 @@ func (b *ActivityBuilder) BuildFromAudit( }, nil } +// enrichSummaryWithDisplayNames rewrites the summary to use human-readable +// display names for the actor and any User-typed link targets, and appends +// link metadata so the UI can render an email/UID tooltip. +// +// Behavior: +// - When the actor has a DisplayName, the first occurrence of the actor's +// Name (typically an email) in the summary is replaced with the +// DisplayName, and a synthetic actor link is appended carrying the +// DisplayName, Email, and UID. +// - For each existing link whose resource is an iam User, the resolver is +// queried by the link's resource name; on hit, the link's Marker is +// replaced in the summary with the user's DisplayName and the link's +// DisplayName/Email fields are populated. +// +// Returns the rewritten summary and links. When resolver is nil or no +// matches occur, the inputs are returned unchanged. +func enrichSummaryWithDisplayNames( + ctx context.Context, + summary string, + actor v1alpha1.ActivityActor, + links []v1alpha1.ActivityLink, + resolver UserResolver, +) (string, []v1alpha1.ActivityLink) { + // Actor: if we have a display name distinct from the name, swap it into + // the summary. If the policy template already wrapped the actor with + // link() (so a link entry exists with marker == actor.Name), upgrade + // that entry in place; otherwise append a synthetic actor link so the + // UI can render the hover tooltip. + if actor.DisplayName != "" && actor.DisplayName != actor.Name && actor.Name != "" { + summaryHadActor := strings.Contains(summary, actor.Name) + if summaryHadActor { + summary = strings.Replace(summary, actor.Name, actor.DisplayName, 1) + } + + upgraded := false + for i := range links { + if links[i].Marker == actor.Name { + links[i].Marker = actor.DisplayName + links[i].DisplayName = actor.DisplayName + if links[i].Email == "" { + links[i].Email = actor.Email + } + upgraded = true + break + } + } + if !upgraded && summaryHadActor { + links = append(links, v1alpha1.ActivityLink{ + Marker: actor.DisplayName, + Resource: v1alpha1.ActivityResource{ + APIGroup: iamGroup, + Kind: "User", + UID: actor.UID, + }, + DisplayName: actor.DisplayName, + Email: actor.Email, + }) + } + } + + // User-typed link targets: hydrate via resolver and rewrite the summary. + if resolver != nil { + for i := range links { + link := &links[i] + if !isUserLink(link.Resource) { + continue + } + if link.Resource.Name == "" || link.DisplayName != "" { + continue + } + info, ok, err := resolver.LookupByName(ctx, link.Resource.Name) + if err != nil || !ok { + continue + } + displayName := info.DisplayName() + if displayName == "" { + continue + } + if link.Marker != "" && link.Marker != displayName { + summary = strings.Replace(summary, link.Marker, displayName, 1) + link.Marker = displayName + } + link.DisplayName = displayName + link.Email = info.Email + } + } + + return summary, links +} + +// isUserLink reports whether the resource targets an iam User CR. +func isUserLink(r v1alpha1.ActivityResource) bool { + return r.APIGroup == iamGroup && r.Kind == "User" +} + // extractResponseUID extracts the UID from an audit response object's metadata. func extractResponseUID(responseObject *runtime.Unknown) string { if responseObject == nil || len(responseObject.Raw) == 0 { @@ -202,6 +312,10 @@ func (b *ActivityBuilder) BuildFromEvent( return nil, fmt.Errorf("%w: %v", ErrActivityBuild, err) } + // Hydrate User-typed links with display names (event actors are system + // components, so no actor enrichment is needed). + summary, activityLinks = enrichSummaryWithDisplayNames(context.Background(), summary, actor, activityLinks, b.UserResolver) + return &v1alpha1.Activity{ TypeMeta: metav1.TypeMeta{ APIVersion: v1alpha1.SchemeGroupVersion.String(), diff --git a/internal/processor/classifier.go b/internal/processor/classifier.go index 84e01ac1..14109d2b 100644 --- a/internal/processor/classifier.go +++ b/internal/processor/classifier.go @@ -1,6 +1,7 @@ package processor import ( + "context" "strings" authnv1 "k8s.io/api/authentication/v1" @@ -38,6 +39,14 @@ const ( // - user: Human users authenticated via OIDC or other providers // - system: Kubernetes controllers, service accounts, and other system components func ResolveActor(user authnv1.UserInfo) v1alpha1.ActivityActor { + return ResolveActorWithResolver(context.Background(), user, nil) +} + +// ResolveActorWithResolver behaves like ResolveActor but additionally +// populates ActivityActor.DisplayName for human users when resolver is non-nil +// and a matching User record is found. Resolver errors are silently ignored: +// the activity is still emitted with whatever data is available. +func ResolveActorWithResolver(ctx context.Context, user authnv1.UserInfo, resolver UserResolver) v1alpha1.ActivityActor { actor := v1alpha1.ActivityActor{ UID: user.UID, } @@ -62,6 +71,15 @@ func ResolveActor(user authnv1.UserInfo) v1alpha1.ActivityActor { actor.Name = "unknown" } + // Enrich with display name for human users when a resolver is available. + if resolver != nil && actor.Type == ActorTypeUser && actor.Email != "" { + if info, ok, err := resolver.LookupByEmail(ctx, actor.Email); err == nil && ok { + if dn := info.DisplayName(); dn != "" { + actor.DisplayName = dn + } + } + } + return actor } diff --git a/internal/processor/enrichment_test.go b/internal/processor/enrichment_test.go new file mode 100644 index 00000000..dd5422ee --- /dev/null +++ b/internal/processor/enrichment_test.go @@ -0,0 +1,232 @@ +package processor + +import ( + "context" + "testing" + + authnv1 "k8s.io/api/authentication/v1" + + "go.miloapis.com/activity/pkg/apis/activity/v1alpha1" +) + +// userInfoFixture builds a minimal authentication.UserInfo for tests. +func userInfoFixture(username, uid string) authnv1.UserInfo { + return authnv1.UserInfo{Username: username, UID: uid} +} + +func TestEnrichSummaryWithDisplayNames_ActorReplacement(t *testing.T) { + cases := []struct { + name string + summary string + actor v1alpha1.ActivityActor + links []v1alpha1.ActivityLink + wantSummary string + wantLinks int + wantMarker string + }{ + { + name: "no display name leaves summary untouched", + summary: "smith@datum.net created machine account ma-1", + actor: v1alpha1.ActivityActor{ + Type: ActorTypeUser, + Name: "smith@datum.net", + Email: "smith@datum.net", + UID: "uid-1", + }, + wantSummary: "smith@datum.net created machine account ma-1", + wantLinks: 0, + }, + { + name: "display name replaces email and synthetic link is appended", + summary: "smith@datum.net created machine account ma-1", + actor: v1alpha1.ActivityActor{ + Type: ActorTypeUser, + Name: "smith@datum.net", + Email: "smith@datum.net", + UID: "uid-1", + DisplayName: "Smith Nelson", + }, + wantSummary: "Smith Nelson created machine account ma-1", + wantLinks: 1, + wantMarker: "Smith Nelson", + }, + { + name: "existing actor link is upgraded in place", + summary: "smith@datum.net created machine account ma-1", + actor: v1alpha1.ActivityActor{ + Type: ActorTypeUser, + Name: "smith@datum.net", + Email: "smith@datum.net", + UID: "uid-1", + DisplayName: "Smith Nelson", + }, + links: []v1alpha1.ActivityLink{ + { + Marker: "smith@datum.net", + Resource: v1alpha1.ActivityResource{ + APIGroup: iamGroup, + Kind: "User", + Name: "smith", + }, + }, + }, + wantSummary: "Smith Nelson created machine account ma-1", + wantLinks: 1, + wantMarker: "Smith Nelson", + }, + { + name: "actor not in summary leaves summary alone and skips link", + summary: "system updated machine account ma-1", + actor: v1alpha1.ActivityActor{ + Type: ActorTypeUser, + Name: "smith@datum.net", + Email: "smith@datum.net", + UID: "uid-1", + DisplayName: "Smith Nelson", + }, + wantSummary: "system updated machine account ma-1", + wantLinks: 0, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + gotSummary, gotLinks := enrichSummaryWithDisplayNames( + context.Background(), tc.summary, tc.actor, tc.links, nil, + ) + if gotSummary != tc.wantSummary { + t.Fatalf("summary = %q, want %q", gotSummary, tc.wantSummary) + } + if len(gotLinks) != tc.wantLinks { + t.Fatalf("links = %d, want %d (links=%+v)", len(gotLinks), tc.wantLinks, gotLinks) + } + if tc.wantMarker != "" { + found := false + for _, l := range gotLinks { + if l.Marker == tc.wantMarker { + found = true + if l.DisplayName != tc.actor.DisplayName { + t.Errorf("link.DisplayName = %q, want %q", l.DisplayName, tc.actor.DisplayName) + } + if l.Email != tc.actor.Email { + t.Errorf("link.Email = %q, want %q", l.Email, tc.actor.Email) + } + } + } + if !found { + t.Errorf("no link with marker %q", tc.wantMarker) + } + } + }) + } +} + +func TestEnrichSummaryWithDisplayNames_UserLinkHydration(t *testing.T) { + resolver := &fakeResolver{ + names: map[string]UserInfo{ + "340583683847098197": { + Name: "340583683847098197", + GivenName: "Dean", + FamilyName: "Gaghan", + Email: "dgaghan@datum.net", + }, + }, + } + + summary := "Smith Nelson updated user 340583683847098197" + links := []v1alpha1.ActivityLink{ + { + Marker: "340583683847098197", + Resource: v1alpha1.ActivityResource{ + APIGroup: iamGroup, + Kind: "User", + Name: "340583683847098197", + }, + }, + } + actor := v1alpha1.ActivityActor{Type: ActorTypeUser, Name: "Smith Nelson", DisplayName: "Smith Nelson"} + + gotSummary, gotLinks := enrichSummaryWithDisplayNames(context.Background(), summary, actor, links, resolver) + + wantSummary := "Smith Nelson updated user Dean Gaghan" + if gotSummary != wantSummary { + t.Fatalf("summary = %q, want %q", gotSummary, wantSummary) + } + if len(gotLinks) != 1 { + t.Fatalf("links = %d, want 1", len(gotLinks)) + } + got := gotLinks[0] + if got.Marker != "Dean Gaghan" { + t.Errorf("Marker = %q, want %q", got.Marker, "Dean Gaghan") + } + if got.DisplayName != "Dean Gaghan" { + t.Errorf("DisplayName = %q, want %q", got.DisplayName, "Dean Gaghan") + } + if got.Email != "dgaghan@datum.net" { + t.Errorf("Email = %q", got.Email) + } +} + +func TestEnrichSummaryWithDisplayNames_NonUserLinkUntouched(t *testing.T) { + resolver := &fakeResolver{names: map[string]UserInfo{ + "some-resource": {GivenName: "Should not", FamilyName: "Be Used"}, + }} + + summary := "Smith Nelson created machine account ma-1" + links := []v1alpha1.ActivityLink{ + { + Marker: "ma-1", + Resource: v1alpha1.ActivityResource{ + APIGroup: iamGroup, + Kind: "MachineAccount", + Name: "ma-1", + }, + }, + } + actor := v1alpha1.ActivityActor{Type: ActorTypeUser, Name: "Smith Nelson"} + + gotSummary, gotLinks := enrichSummaryWithDisplayNames(context.Background(), summary, actor, links, resolver) + if gotSummary != summary { + t.Fatalf("summary changed: %q", gotSummary) + } + if gotLinks[0].DisplayName != "" || gotLinks[0].Email != "" { + t.Fatalf("MachineAccount link should not be hydrated: %+v", gotLinks[0]) + } + if resolver.nameCalls.Load() != 0 { + t.Fatalf("resolver should not be called for non-User links, got %d calls", resolver.nameCalls.Load()) + } +} + +func TestResolveActorWithResolver_PopulatesDisplayName(t *testing.T) { + resolver := &fakeResolver{ + emails: map[string]UserInfo{ + "smith@datum.net": {GivenName: "Smith", FamilyName: "Nelson", Email: "smith@datum.net"}, + }, + } + + actor := ResolveActorWithResolver(context.Background(), userInfoFixture("smith@datum.net", "uid-1"), resolver) + + if actor.DisplayName != "Smith Nelson" { + t.Fatalf("DisplayName = %q, want %q", actor.DisplayName, "Smith Nelson") + } + if actor.Email != "smith@datum.net" { + t.Errorf("Email = %q", actor.Email) + } +} + +func TestResolveActorWithResolver_NilResolverNoDisplayName(t *testing.T) { + actor := ResolveActorWithResolver(context.Background(), userInfoFixture("smith@datum.net", "uid-1"), nil) + if actor.DisplayName != "" { + t.Fatalf("DisplayName should be empty without resolver, got %q", actor.DisplayName) + } +} + +func TestResolveActorWithResolver_SystemActorSkipsLookup(t *testing.T) { + resolver := &fakeResolver{} + actor := ResolveActorWithResolver(context.Background(), userInfoFixture("system:admin", "uid-system"), resolver) + if actor.Type != ActorTypeSystem { + t.Fatalf("Type = %q, want %q", actor.Type, ActorTypeSystem) + } + if resolver.emailCalls.Load() != 0 { + t.Fatalf("resolver should not be called for system actors, got %d calls", resolver.emailCalls.Load()) + } +} diff --git a/internal/processor/evaluate.go b/internal/processor/evaluate.go index 28ae8f26..815faa94 100644 --- a/internal/processor/evaluate.go +++ b/internal/processor/evaluate.go @@ -28,10 +28,24 @@ type EvaluationResult struct { // EvaluateAuditRules evaluates audit rules against an audit log input. // Returns the generated Activity if a rule matches, or nil if no rule matched. // If resolveKind is provided, it will be used to resolve resource names to Kind in links. +// +// Use EvaluateAuditRulesWithResolver to also enrich activities with user +// display names; this convenience wrapper forwards a nil resolver. func EvaluateAuditRules( spec *v1alpha1.ActivityPolicySpec, audit *auditv1.Event, resolveKind KindResolver, +) (*EvaluationResult, error) { + return EvaluateAuditRulesWithResolver(spec, audit, resolveKind, nil) +} + +// EvaluateAuditRulesWithResolver is like EvaluateAuditRules but additionally +// enriches the resulting Activity with display names looked up via resolver. +func EvaluateAuditRulesWithResolver( + spec *v1alpha1.ActivityPolicySpec, + audit *auditv1.Event, + resolveKind KindResolver, + resolver UserResolver, ) (*EvaluationResult, error) { // Convert to map for CEL evaluation auditMap, err := toMap(audit) @@ -41,8 +55,9 @@ func EvaluateAuditRules( // Create activity builder builder := &ActivityBuilder{ - APIGroup: spec.Resource.APIGroup, - Kind: spec.Resource.Kind, + APIGroup: spec.Resource.APIGroup, + Kind: spec.Resource.Kind, + UserResolver: resolver, } // Try each audit rule in order @@ -83,10 +98,24 @@ func EvaluateAuditRules( // EvaluateEventRules evaluates event rules against a Kubernetes event input. // Returns the generated Activity if a rule matches, or nil if no rule matched. // If resolveKind is provided, it will be used to resolve resource names to Kind in links. +// +// Use EvaluateEventRulesWithResolver to also enrich activities with user +// display names; this convenience wrapper forwards a nil resolver. func EvaluateEventRules( spec *v1alpha1.ActivityPolicySpec, eventData interface{}, resolveKind KindResolver, +) (*EvaluationResult, error) { + return EvaluateEventRulesWithResolver(spec, eventData, resolveKind, nil) +} + +// EvaluateEventRulesWithResolver is like EvaluateEventRules but additionally +// enriches User-typed link targets with display names looked up via resolver. +func EvaluateEventRulesWithResolver( + spec *v1alpha1.ActivityPolicySpec, + eventData interface{}, + resolveKind KindResolver, + resolver UserResolver, ) (*EvaluationResult, error) { // Convert event data to map if needed eventMap, err := toMap(eventData) @@ -96,8 +125,9 @@ func EvaluateEventRules( // Create activity builder builder := &ActivityBuilder{ - APIGroup: spec.Resource.APIGroup, - Kind: spec.Resource.Kind, + APIGroup: spec.Resource.APIGroup, + Kind: spec.Resource.Kind, + UserResolver: resolver, } // Try each event rule in order diff --git a/internal/processor/userlookup.go b/internal/processor/userlookup.go new file mode 100644 index 00000000..a4f3242a --- /dev/null +++ b/internal/processor/userlookup.go @@ -0,0 +1,177 @@ +package processor + +import ( + "context" + "sync" + "time" +) + +// UserInfo is the subset of iam User fields needed to enrich activities with +// human-readable display names. Resolvers MUST populate at least one of +// GivenName/FamilyName/Email when returning ok=true; a fully-empty result +// should return ok=false so callers can treat it as a miss. +type UserInfo struct { + // Name is the User CR's metadata.name. Used to construct a + // resource reference in the activity Link. + Name string + GivenName string + FamilyName string + Email string + UID string +} + +// DisplayName returns a human-readable name from given/family, falling back +// to whichever component is populated, then to the email. Returns "" when +// nothing is available. +func (u UserInfo) DisplayName() string { + switch { + case u.GivenName != "" && u.FamilyName != "": + return u.GivenName + " " + u.FamilyName + case u.GivenName != "": + return u.GivenName + case u.FamilyName != "": + return u.FamilyName + default: + return u.Email + } +} + +// UserResolver looks up User records to enrich activities with display names. +// +// Implementations MUST be safe for concurrent use. When the resolver cannot +// find a matching user (or hits a transient error), it SHOULD return +// (UserInfo{}, false, nil). A non-nil error indicates a non-cacheable failure +// the caller should log; the activity is still emitted without enrichment. +type UserResolver interface { + // LookupByEmail resolves a user by their email address (typically the + // audit username for OIDC-authenticated requests). + LookupByEmail(ctx context.Context, email string) (UserInfo, bool, error) + + // LookupByName resolves a user by the User CR's metadata.name. Used to + // hydrate user-typed link targets, where audit.objectRef.name is the User + // resource name. + LookupByName(ctx context.Context, name string) (UserInfo, bool, error) +} + +// NoopUserResolver is a UserResolver that always returns a miss. Used as the +// default when no real resolver is wired (e.g., in unit tests). +type NoopUserResolver struct{} + +func (NoopUserResolver) LookupByEmail(context.Context, string) (UserInfo, bool, error) { + return UserInfo{}, false, nil +} + +func (NoopUserResolver) LookupByName(context.Context, string) (UserInfo, bool, error) { + return UserInfo{}, false, nil +} + +// CachedUserResolver wraps an underlying resolver with a TTL cache and +// per-key single-flight to collapse concurrent lookups for the same user. +// Negative results are cached briefly so a missing user doesn't translate to +// a request storm. +type CachedUserResolver struct { + inner UserResolver + posTTL time.Duration + negTTL time.Duration + now func() time.Time + mu sync.Mutex + emailCache map[string]cachedUser + nameCache map[string]cachedUser + emailFlight map[string]*flight + nameFlight map[string]*flight +} + +type cachedUser struct { + info UserInfo + ok bool + expires time.Time +} + +type flight struct { + done chan struct{} + info UserInfo + ok bool + err error +} + +// NewCachedUserResolver wraps inner with a TTL cache. posTTL applies to hits; +// negTTL to misses (kept short so newly-created users become resolvable +// quickly). When inner is nil, lookups always miss. +func NewCachedUserResolver(inner UserResolver, posTTL, negTTL time.Duration) *CachedUserResolver { + if inner == nil { + inner = NoopUserResolver{} + } + if posTTL <= 0 { + posTTL = 5 * time.Minute + } + if negTTL <= 0 { + negTTL = 30 * time.Second + } + return &CachedUserResolver{ + inner: inner, + posTTL: posTTL, + negTTL: negTTL, + now: time.Now, + emailCache: make(map[string]cachedUser), + nameCache: make(map[string]cachedUser), + emailFlight: make(map[string]*flight), + nameFlight: make(map[string]*flight), + } +} + +func (c *CachedUserResolver) LookupByEmail(ctx context.Context, email string) (UserInfo, bool, error) { + if email == "" { + return UserInfo{}, false, nil + } + return c.lookup(ctx, email, c.emailCache, c.emailFlight, c.inner.LookupByEmail) +} + +func (c *CachedUserResolver) LookupByName(ctx context.Context, name string) (UserInfo, bool, error) { + if name == "" { + return UserInfo{}, false, nil + } + return c.lookup(ctx, name, c.nameCache, c.nameFlight, c.inner.LookupByName) +} + +type lookupFn func(context.Context, string) (UserInfo, bool, error) + +func (c *CachedUserResolver) lookup( + ctx context.Context, + key string, + cache map[string]cachedUser, + flights map[string]*flight, + fetch lookupFn, +) (UserInfo, bool, error) { + c.mu.Lock() + if entry, found := cache[key]; found && c.now().Before(entry.expires) { + c.mu.Unlock() + return entry.info, entry.ok, nil + } + + if f, inflight := flights[key]; inflight { + c.mu.Unlock() + <-f.done + return f.info, f.ok, f.err + } + + f := &flight{done: make(chan struct{})} + flights[key] = f + c.mu.Unlock() + + info, ok, err := fetch(ctx, key) + + c.mu.Lock() + delete(flights, key) + if err == nil { + ttl := c.negTTL + if ok { + ttl = c.posTTL + } + cache[key] = cachedUser{info: info, ok: ok, expires: c.now().Add(ttl)} + } + c.mu.Unlock() + + f.info, f.ok, f.err = info, ok, err + close(f.done) + return info, ok, err +} diff --git a/internal/processor/userlookup_test.go b/internal/processor/userlookup_test.go new file mode 100644 index 00000000..d058e4dc --- /dev/null +++ b/internal/processor/userlookup_test.go @@ -0,0 +1,185 @@ +package processor + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "testing" + "time" +) + +// fakeResolver is a UserResolver whose responses are configured per-test. +type fakeResolver struct { + emailCalls atomic.Int64 + nameCalls atomic.Int64 + emails map[string]UserInfo + names map[string]UserInfo + wait chan struct{} + emailErr error + nameErr error +} + +func (f *fakeResolver) LookupByEmail(ctx context.Context, email string) (UserInfo, bool, error) { + f.emailCalls.Add(1) + if f.wait != nil { + <-f.wait + } + if f.emailErr != nil { + return UserInfo{}, false, f.emailErr + } + info, ok := f.emails[email] + return info, ok, nil +} + +func (f *fakeResolver) LookupByName(ctx context.Context, name string) (UserInfo, bool, error) { + f.nameCalls.Add(1) + if f.wait != nil { + <-f.wait + } + if f.nameErr != nil { + return UserInfo{}, false, f.nameErr + } + info, ok := f.names[name] + return info, ok, nil +} + +func TestUserInfo_DisplayName(t *testing.T) { + cases := []struct { + name string + in UserInfo + want string + }{ + {"both", UserInfo{GivenName: "Smith", FamilyName: "Nelson"}, "Smith Nelson"}, + {"given only", UserInfo{GivenName: "Smith"}, "Smith"}, + {"family only", UserInfo{FamilyName: "Nelson"}, "Nelson"}, + {"email fallback", UserInfo{Email: "smith@datum.net"}, "smith@datum.net"}, + {"empty", UserInfo{}, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := tc.in.DisplayName(); got != tc.want { + t.Fatalf("DisplayName() = %q, want %q", got, tc.want) + } + }) + } +} + +func TestCachedUserResolver_PositiveHitCached(t *testing.T) { + inner := &fakeResolver{emails: map[string]UserInfo{ + "smith@datum.net": {GivenName: "Smith", FamilyName: "Nelson", Email: "smith@datum.net"}, + }} + c := NewCachedUserResolver(inner, time.Minute, time.Minute) + + for i := 0; i < 5; i++ { + info, ok, err := c.LookupByEmail(context.Background(), "smith@datum.net") + if err != nil || !ok || info.DisplayName() != "Smith Nelson" { + t.Fatalf("iteration %d: got info=%+v ok=%v err=%v", i, info, ok, err) + } + } + if got := inner.emailCalls.Load(); got != 1 { + t.Fatalf("inner LookupByEmail called %d times, want 1", got) + } +} + +func TestCachedUserResolver_NegativeHitCached(t *testing.T) { + inner := &fakeResolver{emails: map[string]UserInfo{}} + c := NewCachedUserResolver(inner, time.Minute, time.Minute) + + for i := 0; i < 3; i++ { + _, ok, err := c.LookupByEmail(context.Background(), "missing@datum.net") + if err != nil || ok { + t.Fatalf("iteration %d: ok=%v err=%v", i, ok, err) + } + } + if got := inner.emailCalls.Load(); got != 1 { + t.Fatalf("inner LookupByEmail called %d times, want 1 (negative cache miss)", got) + } +} + +func TestCachedUserResolver_TTLExpiry(t *testing.T) { + inner := &fakeResolver{emails: map[string]UserInfo{ + "smith@datum.net": {GivenName: "Smith", Email: "smith@datum.net"}, + }} + c := NewCachedUserResolver(inner, 50*time.Millisecond, 50*time.Millisecond) + + if _, ok, _ := c.LookupByEmail(context.Background(), "smith@datum.net"); !ok { + t.Fatal("first lookup must hit") + } + now := time.Now() + c.now = func() time.Time { return now.Add(time.Hour) } + if _, ok, _ := c.LookupByEmail(context.Background(), "smith@datum.net"); !ok { + t.Fatal("post-TTL lookup must still resolve via inner") + } + if got := inner.emailCalls.Load(); got != 2 { + t.Fatalf("inner LookupByEmail called %d times, want 2", got) + } +} + +func TestCachedUserResolver_SingleFlight(t *testing.T) { + inner := &fakeResolver{ + emails: map[string]UserInfo{"smith@datum.net": {GivenName: "Smith", Email: "smith@datum.net"}}, + wait: make(chan struct{}), + } + c := NewCachedUserResolver(inner, time.Minute, time.Minute) + + const concurrent = 20 + var wg sync.WaitGroup + for i := 0; i < concurrent; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _, _, _ = c.LookupByEmail(context.Background(), "smith@datum.net") + }() + } + + // Give all goroutines a moment to enqueue behind the in-flight lookup. + time.Sleep(20 * time.Millisecond) + close(inner.wait) + wg.Wait() + + if got := inner.emailCalls.Load(); got != 1 { + t.Fatalf("inner LookupByEmail called %d times, want 1 (single-flight)", got) + } +} + +func TestCachedUserResolver_ErrorNotCached(t *testing.T) { + sentinel := errors.New("boom") + inner := &fakeResolver{emailErr: sentinel} + c := NewCachedUserResolver(inner, time.Minute, time.Minute) + + for i := 0; i < 3; i++ { + _, _, err := c.LookupByEmail(context.Background(), "smith@datum.net") + if !errors.Is(err, sentinel) { + t.Fatalf("iteration %d: err=%v, want %v", i, err, sentinel) + } + } + if got := inner.emailCalls.Load(); got != 3 { + t.Fatalf("inner LookupByEmail called %d times, want 3 (errors are not cached)", got) + } +} + +func TestCachedUserResolver_EmptyKeysShortCircuit(t *testing.T) { + inner := &fakeResolver{} + c := NewCachedUserResolver(inner, time.Minute, time.Minute) + + if _, ok, err := c.LookupByEmail(context.Background(), ""); ok || err != nil { + t.Fatalf("empty email should miss without err; ok=%v err=%v", ok, err) + } + if _, ok, err := c.LookupByName(context.Background(), ""); ok || err != nil { + t.Fatalf("empty name should miss without err; ok=%v err=%v", ok, err) + } + if got := inner.emailCalls.Load() + inner.nameCalls.Load(); got != 0 { + t.Fatalf("inner called %d times, want 0", got) + } +} + +func TestNoopUserResolver(t *testing.T) { + r := NoopUserResolver{} + if _, ok, err := r.LookupByEmail(context.Background(), "x"); ok || err != nil { + t.Fatalf("noop email lookup ok=%v err=%v", ok, err) + } + if _, ok, err := r.LookupByName(context.Background(), "x"); ok || err != nil { + t.Fatalf("noop name lookup ok=%v err=%v", ok, err) + } +} diff --git a/pkg/apis/activity/v1alpha1/types_activity.go b/pkg/apis/activity/v1alpha1/types_activity.go index df15deed..cca25898 100644 --- a/pkg/apis/activity/v1alpha1/types_activity.go +++ b/pkg/apis/activity/v1alpha1/types_activity.go @@ -128,6 +128,17 @@ type ActivityActor struct { // // +optional Email string `json:"email,omitempty"` + + // DisplayName is the actor's human-readable name (e.g., "Smith Nelson"). + // For user actors, populated from the iam User's spec.givenName and + // spec.familyName when available. Empty if no User record is found or the + // actor is not a human user. + // + // UIs SHOULD prefer DisplayName for visible text and use Name/Email/UID + // only when DisplayName is empty. + // + // +optional + DisplayName string `json:"displayName,omitempty"` } // ActivityResource identifies the Kubernetes resource affected by an activity. @@ -179,6 +190,21 @@ type ActivityLink struct { // // +required Resource ActivityResource `json:"resource"` + + // DisplayName is the human-readable name for the linked entity, when one + // is available. Populated server-side for User-typed links from the iam + // User's givenName + familyName so the UI can render names instead of + // raw UIDs in the summary. + // + // +optional + DisplayName string `json:"displayName,omitempty"` + + // Email is the email address for the linked entity, when one is + // available. Populated server-side for User-typed links so the UI can + // surface the email on hover. + // + // +optional + Email string `json:"email,omitempty"` } // ActivityTenant identifies the scope for multi-tenant isolation.