fix(auth): infer actor type from preferred_username on JIT provision

[chadcrum]

Summary

• JIT-provisioned actors were hardcoded to ActorTypeHuman regardless of their actual type
• Keycloak service accounts use preferred_username=service-account-<client-id> - now we detect and type them as ActorTypeServiceAccount
• Added inferActorType() helper that checks for the service-account- prefix and returns the correct type
• Updated provisionActor() to use inferred type instead of hardcoding, and added actorType to structured logs

Verification

• Verified service account JIT provisioning produces type=service_account in the DB against a live Keycloak + DCM stack
• Unit tests: TestResolveActor_JITProvisionServiceAccount (end-to-end through mock store) and TestInferActorType (table-driven, 8 cases covering prefix match, case sensitivity, and edge cases)

[qodo-code-review]

PR Summary by Qodo

Fix JIT actor type inference for Keycloak service accounts

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Infer actor type during JIT provisioning using Keycloak service-account- username convention.
• Persist and return the inferred actor type, and include it in structured logs.
• Add unit tests for service-account provisioning and inferActorType() edge cases.

Diagram

graph TD
A["ResolveActor()"] --> B["provisionActor()"] --> C{"Has \"service-account-\" prefix?"}
C --> D["ActorTypeServiceAccount"] --> G["Return ActorInfo"]
C --> E["ActorTypeHuman"] --> G
B --> F[("Actor Store")]
G --> F

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Infer actor type from stronger IdP claims (client_id / azp / token roles)

• ➕ Less brittle than string prefix conventions
• ➕ Works even if usernames are renamed or normalized
• ➖ Requires plumbing additional token claims into context / service layer
• ➖ More Keycloak/OIDC-claim coupling and potentially more edge cases

2. Make service-account detection configurable (prefix/regex list)

• ➕ Avoids hard-coding Keycloak’s naming convention
• ➕ Supports multiple environments or IdPs with different patterns
• ➖ Adds configuration surface area and validation needs
• ➖ Still pattern-based; may hide misconfigurations until runtime

Recommendation: The PR’s approach is a good fit given the immediate bug and Keycloak’s well-known service-account- convention. Keep this implementation, but consider a follow-up to make the prefix configurable or migrate to claim-based typing if additional actor types or IdPs are expected.

Files changed (2) +65 / -3

Bug fix (1) +13 / -3

service.goInfer and persist actor type during JIT provisioning +13/-3

Infer and persist actor type during JIT provisioning

• Adds 'inferActorType()' using a 'service-account-' preferred_username prefix check. Updates 'provisionActor()' to persist the inferred type, return it in 'ActorInfo', and include 'actorType' in structured logs.

internal/auth/service/service.go

Tests (1) +52 / -0

service_test.goAdd tests for service-account JIT provisioning and actor type inference +52/-0

Add tests for service-account JIT provisioning and actor type inference

• Adds an end-to-end-ish unit test validating service-account actors are provisioned with 'ActorTypeServiceAccount'. Adds a table-driven test for 'inferActorType()' covering prefix match, case sensitivity, and edge cases.

internal/auth/service/service_test.go

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Type inferred from externalID ✓ Resolved 🐞 Bug ≡ Correctness

Description

In provisionActor(), actorType is inferred from username after username falls back to
externalID, so actor typing can be driven by the subject/externalID string when
preferred_username is absent. This can persist and return service_account for any newly
provisioned actor whose externalID starts with service-account-, even though no preferred username
was provided.

Code

internal/auth/service/service.go[R66-70]

	username := auth.PreferredUsernameFromContext(ctx)
	if username == "" {
		username = externalID
	}
+	actorType := inferActorType(username)

Evidence

provisionActor() falls back username to externalID and then passes that value to
inferActorType, so type inference runs on the externalID string when the preferred username is
missing. The externalID is sourced from either the JWT sub claim or the X-Forwarded-User header,
while preferred username may be empty, so this is a reachable path.

internal/auth/service/service.go[54-78]
internal/auth/middleware.go[38-68]
internal/auth/jwt.go[52-76]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`provisionActor()` currently infers `actorType` from `username` after `username` may have been replaced with `externalID` (fallback when preferred username is empty). This makes actor-type inference depend on `externalID` in the no-preferred-username path, which is inconsistent with the intended “infer from preferred_username” behavior.

## Issue Context
- `preferredUsername` can be empty (JWT claim missing or proxy header not set), while `externalID` is always required for authentication.
- The PR introduces `inferActorType()` and then calls it with the post-fallback `username`.

## Fix Focus Areas
- internal/auth/service/service.go[54-121]

### Suggested implementation direction
- Keep `username` fallback behavior for `Actor.Username` (preferred username else externalID).
- Compute `actorType` from the *original* preferred username only, e.g.:
 - `preferred := auth.PreferredUsernameFromContext(ctx)`
 - `username := preferred; if username == "" { username = externalID }`
 - `actorType := inferActorType(preferred)` (so empty preferred defaults to human)
- Consider renaming the `inferActorType` parameter to reflect what is expected (preferred username) to avoid future misuse.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[chadcrum]

Thanks for the review. Fixed the actor type inference issue in cbcb814 - all tests pass.