diff --git a/SCOPE.md b/SCOPE.md index a2f5b4c..737ee21 100644 --- a/SCOPE.md +++ b/SCOPE.md @@ -46,10 +46,15 @@ Keycloak interchangeability are not established. groups and memberships are sorted, so an unchanged directory exports identically. Completeness beyond users, groups and memberships — passwords and MFA credentials in particular — is still not covered. -- The Keycloak CLI exports users/groups through the basic transformer and has - no client-list input. Library-level client mapping does not preserve the full - current service-identity, audience, tenant/role, MFA and lifetime contract. - Password and MFA credential migration is not supplied. +- The Keycloak CLI takes a `-clients` config and migrates client registrations: + flows follow the declared grants, and audience, tenant, service subject and + roles are carried as protocol mappers with per-client lifetime, handoff URLs + and the secret *reference* as attributes (KEY-WP-0020). What it cannot carry it + names, in a report separate from the consistency check: secret values, MFA + policy enforcement, passwords and factor enrolment, and subject continuity — + Keycloak mints its own `sub`, so the LLDAP canonical ID survives only as an + attribute. The output is a reviewed artifact, not a proven migration; that + proof needs a live provider swap and is not established. - Snapshot validation is a limited Go rule set, not full machine-readable schema enforcement. The canonical YAML model and discovery metadata now match the runtime client-registration surface and are held there by a conformance check diff --git a/bin/keycape b/bin/keycape index 8f3a58c..2ec223d 100755 Binary files a/bin/keycape and b/bin/keycape differ diff --git a/bin/keycape-to-keycloak b/bin/keycape-to-keycloak index 77b99b8..11ace5d 100755 Binary files a/bin/keycape-to-keycloak and b/bin/keycape-to-keycloak differ diff --git a/bin/lldap-export b/bin/lldap-export index d7db599..5994cec 100755 Binary files a/bin/lldap-export and b/bin/lldap-export differ diff --git a/bin/lldap-to-ldap b/bin/lldap-to-ldap index bc71e82..f39fd34 100755 Binary files a/bin/lldap-to-ldap and b/bin/lldap-to-ldap differ diff --git a/bin/validator b/bin/validator index 58a70aa..8902dd4 100755 Binary files a/bin/validator and b/bin/validator differ diff --git a/history/2026-09-05-011726-scope-intent-assessment.md b/history/2026-09-05-011726-scope-intent-assessment.md index 7036d5c..5c63948 100644 --- a/history/2026-09-05-011726-scope-intent-assessment.md +++ b/history/2026-09-05-011726-scope-intent-assessment.md @@ -171,6 +171,30 @@ transforms preserve or explicitly reject every relevant policy/identity field, and migration proof demonstrates subject continuity, claims, MFA and client behavior. Until then, call these artifact generators rather than full migration. +**Status 2026-09-07 (KEY-WP-0020): semantic preservation closed; proof remains +G04.** The CLI takes `-clients` and reaches `TransformWithClients`, so the realm +carries the registration contract instead of an empty array. Flows are derived +from the declared grants rather than hardcoded — the previous mapping enabled the +browser flow on every service-only client, widening it during migration. The +audience, tenant, service subject and roles ride as protocol mappers, per-client +lifetime and handoff URLs as attributes, and the secret *reference* as an +attribute, never a value; a test asserts no resolved secret can appear in the +JSON. Realm roles and client scopes are derived from the identities and +registrations present, replacing empty containers that made a realm dropping +every role look like one that had none. + +What cannot be carried is now named rather than dropped, which was the real +defect: `UnpreservedReport` is deliberately separate from `ValidationReport`, so +"the realm matches the snapshot" and "the migration is complete" stay distinct +questions. It names the unmigrated secret, the unenforceable MFA policy, +passwords and factor enrolment, and subject continuity — Keycloak mints its own +`sub`, so relying parties keyed on it will not recognise migrated users. An +incomplete transform emits `partial` telemetry, matching KEY-WP-0018. + +Not closed: this is preservation and honest reporting, not proof. Whether an +imported realm actually issues profile-conformant tokens is G04, and password and +MFA credential migration remains out of scope. + ### G04 — The replacement test harness does not prove a live provider swap **Priority: high. Kind: verification and tooling gap.** diff --git a/src/cmd/keycape-to-keycloak/main.go b/src/cmd/keycape-to-keycloak/main.go index 5d076d1..7ffcc12 100644 --- a/src/cmd/keycape-to-keycloak/main.go +++ b/src/cmd/keycape-to-keycloak/main.go @@ -11,6 +11,8 @@ import ( "github.com/rs/zerolog" "gopkg.in/yaml.v3" + "keycape/internal/config" + "keycape/internal/domain" "keycape/internal/migration/lldapexport" "keycape/internal/migration/tokeycloak" "keycape/internal/server/telemetry" @@ -21,6 +23,7 @@ func main() { outputFile := flag.String("output", "keycloak-realm.json", "Path to write Keycloak realm import JSON") realmName := flag.String("realm", "netkingdom", "Keycloak realm name") issuer := flag.String("issuer", "", "OIDC issuer URL") + clientsFile := flag.String("clients", "", "Path to the KeyCape config whose client registrations to migrate") flag.Parse() if *inputFile == "" { @@ -41,6 +44,23 @@ func main() { os.Exit(1) } + // Client registrations are read from the KeyCape config, not the directory + // snapshot: the snapshot carries identities, while the service-identity, + // audience, tenant/role, MFA and lifetime contract lives in the config. + var clients []domain.Client + if *clientsFile != "" { + cfg, cfgErr := config.Load(*clientsFile) + if cfgErr != nil { + fmt.Fprintf(os.Stderr, "keycape-to-keycloak: read clients %q: %v\n", *clientsFile, cfgErr) + os.Exit(1) + } + clients, cfgErr = cfg.Registrations() + if cfgErr != nil { + fmt.Fprintf(os.Stderr, "keycape-to-keycloak: %v\n", cfgErr) + os.Exit(1) + } + } + log := zerolog.New(os.Stderr).With().Timestamp().Logger() em := telemetry.NewLogEmitter(log) tr := tokeycloak.New(tokeycloak.Config{ @@ -48,17 +68,26 @@ func main() { Issuer: *issuer, }, em) - realm, err := tr.Transform(&export) + realm, err := tr.TransformWithClients(&export, clients) if err != nil { fmt.Fprintf(os.Stderr, "keycape-to-keycloak: transform: %v\n", err) os.Exit(1) } - // Print validation report to stderr. - report := tr.ValidationReport(&export, realm) - for _, issue := range report { + // Consistency problems and migration limits are printed separately: the + // first says the generated realm disagrees with the snapshot, the second + // says what this tool never carries across. Collapsing them would let a + // reader mistake a complete-looking realm for a complete migration. + for _, issue := range tr.ValidationReport(&export, realm) { fmt.Fprintf(os.Stderr, "WARNING: %s\n", issue) } + unpreserved := tr.UnpreservedReport() + if len(unpreserved) > 0 { + fmt.Fprintln(os.Stderr, "\nNOT PRESERVED — this realm is an artifact, not a completed migration:") + for _, item := range unpreserved { + fmt.Fprintln(os.Stderr, " -", item) + } + } out, err := json.MarshalIndent(realm, "", " ") if err != nil { diff --git a/src/internal/config/config.go b/src/internal/config/config.go index fab257d..dc1aa7b 100644 --- a/src/internal/config/config.go +++ b/src/internal/config/config.go @@ -6,12 +6,14 @@ package config import ( "fmt" "os" + "time" "gopkg.in/yaml.v3" "keycape/internal/adapters/authelia" "keycape/internal/adapters/lldap" "keycape/internal/adapters/privacyidea" + "keycape/internal/domain" ) // Config is the top-level server configuration. @@ -69,3 +71,44 @@ func Load(path string) (*Config, error) { return &cfg, nil } + +// Registrations converts the configured clients to domain registrations without +// resolving any secret. Migration and inspection tooling needs the registration +// contract — grants, audience, tenant, roles, lifetime, MFA and handoff policy — +// but must never load secret material, so ClientSecret is deliberately left +// empty here. The server has its own conversion that does resolve secrets. +// +// An unparseable tokenLifetime is an error rather than a silent zero: a +// migration that quietly drops a per-client lifetime is the class of defect this +// exists to avoid. +func (c *Config) Registrations() ([]domain.Client, error) { + clients := make([]domain.Client, 0, len(c.Clients)) + for _, cc := range c.Clients { + var lifetime time.Duration + if cc.TokenLifetime != "" { + parsed, err := time.ParseDuration(cc.TokenLifetime) + if err != nil { + return nil, fmt.Errorf("config: client %q tokenLifetime is invalid: %w", cc.ClientID, err) + } + lifetime = parsed + } + clients = append(clients, domain.Client{ + ClientID: cc.ClientID, + DisplayName: cc.DisplayName, + RedirectURIs: cc.RedirectURIs, + AllowedScopes: cc.AllowedScopes, + GrantTypes: cc.GrantTypes, + ClientType: cc.ClientType, + SecretRef: cc.SecretRef, + Audience: cc.Audience, + ServiceSubject: cc.ServiceSubject, + Tenant: cc.Tenant, + Roles: cc.Roles, + TokenLifetime: lifetime, + MFARequired: cc.MFARequired, + RegistrationURL: cc.RegistrationURL, + EnrollmentURL: cc.EnrollmentURL, + }) + } + return clients, nil +} diff --git a/src/internal/migration/tokeycloak/transformer.go b/src/internal/migration/tokeycloak/transformer.go index f6cfd14..cabfe62 100644 --- a/src/internal/migration/tokeycloak/transformer.go +++ b/src/internal/migration/tokeycloak/transformer.go @@ -4,6 +4,8 @@ package tokeycloak import ( "context" + "fmt" + "sort" "strings" "time" @@ -18,42 +20,57 @@ import ( // KeycloakRealm is the top-level realm import JSON structure. type KeycloakRealm struct { - Realm string `json:"realm"` - DisplayName string `json:"displayName,omitempty"` - Enabled bool `json:"enabled"` - SsoSessionMaxLifespan int `json:"ssoSessionMaxLifespan,omitempty"` - DefaultSignatureAlgorithm string `json:"defaultSignatureAlgorithm,omitempty"` - IdentityProviders []interface{} `json:"identityProviders"` - Clients []KeycloakClient `json:"clients"` - Users []KeycloakUser `json:"users"` - Groups []KeycloakGroup `json:"groups"` - Roles KeycloakRoles `json:"roles"` - ClientScopes []KeycloakClientScope `json:"clientScopes"` + Realm string `json:"realm"` + DisplayName string `json:"displayName,omitempty"` + Enabled bool `json:"enabled"` + SsoSessionMaxLifespan int `json:"ssoSessionMaxLifespan,omitempty"` + DefaultSignatureAlgorithm string `json:"defaultSignatureAlgorithm,omitempty"` + IdentityProviders []interface{} `json:"identityProviders"` + Clients []KeycloakClient `json:"clients"` + Users []KeycloakUser `json:"users"` + Groups []KeycloakGroup `json:"groups"` + Roles KeycloakRoles `json:"roles"` + ClientScopes []KeycloakClientScope `json:"clientScopes"` } // KeycloakClient represents a registered client in the Keycloak realm. type KeycloakClient struct { - ClientID string `json:"clientId"` - Name string `json:"name,omitempty"` - Enabled bool `json:"enabled"` - PublicClient bool `json:"publicClient"` - StandardFlowEnabled bool `json:"standardFlowEnabled"` - ImplicitFlowEnabled bool `json:"implicitFlowEnabled"` - DirectAccessGrantsEnabled bool `json:"directAccessGrantsEnabled"` - RedirectUris []string `json:"redirectUris"` - DefaultClientScopes []string `json:"defaultClientScopes"` + ClientID string `json:"clientId"` + Name string `json:"name,omitempty"` + Enabled bool `json:"enabled"` + PublicClient bool `json:"publicClient"` + StandardFlowEnabled bool `json:"standardFlowEnabled"` + ImplicitFlowEnabled bool `json:"implicitFlowEnabled"` + DirectAccessGrantsEnabled bool `json:"directAccessGrantsEnabled"` + ServiceAccountsEnabled bool `json:"serviceAccountsEnabled"` + RedirectUris []string `json:"redirectUris"` + DefaultClientScopes []string `json:"defaultClientScopes"` + Attributes map[string]string `json:"attributes,omitempty"` + ProtocolMappers []KeycloakProtocolMapper `json:"protocolMappers,omitempty"` +} + +// KeycloakProtocolMapper carries a claim into the tokens Keycloak issues. +// KeyCape emits the claims of the NetKingdom IAM profile that Keycloak has no +// native concept for — tenant, principal type, resource audience — as mappers, +// since without them a migrated realm issues tokens the profile does not accept. +type KeycloakProtocolMapper struct { + Name string `json:"name"` + Protocol string `json:"protocol"` + ProtocolMapper string `json:"protocolMapper"` + Config map[string]string `json:"config"` } // KeycloakUser represents a user in the Keycloak realm. type KeycloakUser struct { - Username string `json:"username"` - Email string `json:"email,omitempty"` - FirstName string `json:"firstName,omitempty"` - LastName string `json:"lastName,omitempty"` - Enabled bool `json:"enabled"` - Groups []string `json:"groups,omitempty"` + Username string `json:"username"` + Email string `json:"email,omitempty"` + FirstName string `json:"firstName,omitempty"` + LastName string `json:"lastName,omitempty"` + Enabled bool `json:"enabled"` + Groups []string `json:"groups,omitempty"` + RealmRoles []string `json:"realmRoles,omitempty"` Credentials []KeycloakCredential `json:"credentials,omitempty"` - Attributes map[string][]string `json:"attributes,omitempty"` + Attributes map[string][]string `json:"attributes,omitempty"` } // KeycloakCredential holds a single credential entry (e.g. hashed password placeholder). @@ -100,6 +117,10 @@ type Config struct { type Transformer struct { cfg Config emitter telemetry.Emitter + + // unpreserved accumulates every field the last transform could not carry + // into the realm. ValidationReport surfaces it. + unpreserved []string } // New creates a new Transformer with the given configuration and telemetry emitter. @@ -107,9 +128,10 @@ func New(cfg Config, emitter telemetry.Emitter) *Transformer { return &Transformer{cfg: cfg, emitter: emitter} } -// Transform converts a canonical export to a Keycloak realm import. -// It maps users, groups, and emits migration_event telemetry. -// Clients default to an empty slice; use TransformWithClients to include them. +// Transform converts a canonical export with no client registrations. The +// resulting realm has no clients, which is recorded as an unpreserved item — +// an empty clients array is otherwise indistinguishable from a realm that +// genuinely has none. func (t *Transformer) Transform(export *lldapexport.ExportResult) (*KeycloakRealm, error) { return t.TransformWithClients(export, nil) } @@ -117,6 +139,8 @@ func (t *Transformer) Transform(export *lldapexport.ExportResult) (*KeycloakReal // TransformWithClients converts a canonical export plus an explicit client list // into a Keycloak realm import structure. func (t *Transformer) TransformWithClients(export *lldapexport.ExportResult, clients []domain.Client) (*KeycloakRealm, error) { + t.unpreserved = nil + realm := &KeycloakRealm{ Realm: t.cfg.RealmName, Enabled: true, @@ -140,30 +164,73 @@ func (t *Transformer) TransformWithClients(export *lldapexport.ExportResult, cli realm.Groups = append(realm.Groups, mapGroup(g)) } - // Map clients. + // Map clients, collecting every registration field that could not be + // carried across so the caller can report it. realm.Clients = make([]KeycloakClient, 0, len(clients)) for _, c := range clients { - realm.Clients = append(realm.Clients, mapClient(c)) + mapped, dropped := mapClient(c) + realm.Clients = append(realm.Clients, mapped) + t.unpreserved = append(t.unpreserved, dropped...) + } + if len(clients) == 0 { + t.unpreserved = append(t.unpreserved, + "no client registrations were supplied: the realm carries no clients, so the service-identity, "+ + "audience, tenant/role, MFA and lifetime contract is not migrated") } - // Roles and scopes — empty in base migration; can be extended. - realm.Roles = KeycloakRoles{Realm: []KeycloakRole{}} - realm.ClientScopes = []KeycloakClientScope{} + // Realm roles and client scopes are derived from the identities and + // registrations actually present. Emitting empty containers made a realm + // that drops every role look like one that has none. + realm.Roles = KeycloakRoles{Realm: realmRoles(export.Users, clients)} + realm.ClientScopes = clientScopes(clients) - // Emit migration telemetry. + // Credential material is out of scope for an artifact generator, but its + // absence must be stated: an operator who imports this realm and finds + // nobody can log in should learn that here, not there. + if len(export.Users) > 0 { + t.unpreserved = append(t.unpreserved, + "passwords and MFA credentials are not migrated: every user in this realm requires credential "+ + "re-establishment and factor re-enrolment") + t.unpreserved = append(t.unpreserved, + "subject continuity is not established: the canonical ID is an LLDAP DN carried as the "+ + "keycape.canonicalId attribute, while Keycloak mints its own sub, so relying parties keyed "+ + "on sub will not recognise migrated users") + } + + // Emit migration telemetry. A transform that could not carry part of the + // contract is partial, matching what the LLDAP export reports (KEY-WP-0018): + // an operator scanning events should not see "success" for an artifact that + // still needs manual work. + outcome := "success" + if len(t.unpreserved) > 0 { + outcome = "partial" + } t.emitter.Emit(context.Background(), telemetry.Event{ Timestamp: time.Now().UTC(), EventType: telemetry.EventMigration, Endpoint: "keycape-to-keycloak", - Result: "success", + Result: outcome, }) return realm, nil } +// UnpreservedReport lists what the last transform could not carry into the +// realm. These are not defects in the generated file — they are the parts of the +// KeyCape contract an operator still has to establish by hand. +// +// It is deliberately separate from ValidationReport: an empty validation report +// means the realm is consistent with the canonical data, which is not the same +// as a complete migration, and collapsing the two would make one of those +// questions unanswerable. +func (t *Transformer) UnpreservedReport() []string { + return t.unpreserved +} + // ValidationReport compares a canonical export against a produced Keycloak realm // and returns a list of incompatibility descriptions. -// An empty slice means the import is consistent with the canonical data. +// An empty slice means the import is consistent with the canonical data — see +// UnpreservedReport for what consistency does not cover. func (t *Transformer) ValidationReport(export *lldapexport.ExportResult, realm *KeycloakRealm) []string { var issues []string @@ -196,11 +263,25 @@ func (t *Transformer) ValidationReport(export *lldapexport.ExportResult, realm * // Mapping helpers // --------------------------------------------------------------------------- +// mapUser carries a canonical user into a Keycloak user. The canonical ID and +// tenant become attributes: Keycloak mints its own sub, so the KeyCape subject +// survives only as data something downstream can map, never as the subject. func mapUser(u domain.User) KeycloakUser { ku := KeycloakUser{ - Username: u.Username, - Email: u.Email, - Enabled: u.Enabled, + Username: u.Username, + Email: u.Email, + Enabled: u.Enabled, + RealmRoles: u.Roles, + Attributes: map[string][]string{}, + } + if u.ID != "" { + ku.Attributes["keycape.canonicalId"] = []string{u.ID} + } + if u.Tenant != "" { + ku.Attributes["tenant"] = []string{u.Tenant} + } + if len(ku.Attributes) == 0 { + ku.Attributes = nil } // Split DisplayName at first space → FirstName + LastName. @@ -224,17 +305,27 @@ func mapGroup(g domain.Group) KeycloakGroup { } } -func mapClient(c domain.Client) KeycloakClient { +// mapClient carries a KeyCape registration into a Keycloak client, and appends +// to unpreserved every field it could not represent. A caller that ignores the +// second return value produces a realm that silently differs from the KeyCape +// contract, which is the defect this signature exists to make awkward. +func mapClient(c domain.Client) (KeycloakClient, []string) { + var unpreserved []string + kc := KeycloakClient{ - ClientID: c.ClientID, - Name: c.DisplayName, - Enabled: true, - PublicClient: c.ClientType == "public", - StandardFlowEnabled: true, // authorization_code always enabled + ClientID: c.ClientID, + Name: c.DisplayName, + Enabled: true, + PublicClient: c.ClientType == "public", + // Flows follow the declared grants. Enabling the standard flow for a + // service-only client would widen it during migration. + StandardFlowEnabled: hasGrant(c.GrantTypes, "authorization_code"), + ServiceAccountsEnabled: hasGrant(c.GrantTypes, "client_credentials"), ImplicitFlowEnabled: false, // never — per NetKingdom IAM profile DirectAccessGrantsEnabled: false, // never — per NetKingdom IAM profile RedirectUris: c.RedirectURIs, DefaultClientScopes: c.AllowedScopes, + Attributes: map[string]string{}, } if kc.RedirectUris == nil { kc.RedirectUris = []string{} @@ -242,7 +333,99 @@ func mapClient(c domain.Client) KeycloakClient { if kc.DefaultClientScopes == nil { kc.DefaultClientScopes = []string{} } - return kc + + // The resource audience is a claim Keycloak will not produce on its own. + if c.Audience != "" { + kc.ProtocolMappers = append(kc.ProtocolMappers, audienceMapper(c.Audience)) + } + if c.Tenant != "" { + kc.ProtocolMappers = append(kc.ProtocolMappers, hardcodedClaim("tenant", c.Tenant)) + } + if c.ServiceSubject != "" { + // Keycloak derives a service account's subject from its own user; the + // configured subject cannot be imposed, so carry it as a mapper and + // say that the subject itself will differ. + kc.ProtocolMappers = append(kc.ProtocolMappers, hardcodedClaim("service_subject", c.ServiceSubject)) + unpreserved = append(unpreserved, fmt.Sprintf( + "client %q: serviceSubject %q is carried as a claim, but Keycloak mints its own service-account sub; "+ + "tokens after migration will not carry the KeyCape subject", + c.ClientID, c.ServiceSubject)) + } + if len(c.Roles) > 0 { + kc.ProtocolMappers = append(kc.ProtocolMappers, hardcodedClaim("roles", strings.Join(c.Roles, " "))) + } + if c.TokenLifetime > 0 { + kc.Attributes["access.token.lifespan"] = itoa(int(c.TokenLifetime.Seconds())) + } + if c.SecretRef != "" { + // The reference, never the value: a realm import file is not a secret + // custody boundary. + kc.Attributes["keycape.secretRef"] = c.SecretRef + unpreserved = append(unpreserved, fmt.Sprintf( + "client %q: secret is not migrated; secretRef %q is recorded as an attribute and must be "+ + "resolved and set on the Keycloak client out of band", + c.ClientID, c.SecretRef)) + } + if c.RegistrationURL != "" { + kc.Attributes["keycape.registrationUrl"] = c.RegistrationURL + } + if c.EnrollmentURL != "" { + kc.Attributes["keycape.enrollmentUrl"] = c.EnrollmentURL + } + if c.MFARequired != nil && *c.MFARequired { + // Keycloak expresses this as an authentication flow binding, which a + // realm import cannot synthesise from a boolean. + kc.Attributes["keycape.mfaRequired"] = "true" + unpreserved = append(unpreserved, fmt.Sprintf( + "client %q: mfaRequired is not enforceable by import; a Keycloak authentication flow "+ + "requiring a second factor must be bound to this client manually", + c.ClientID)) + } + if len(kc.Attributes) == 0 { + kc.Attributes = nil + } + return kc, unpreserved +} + +// hasGrant reports whether the registration declares the given grant type. +func hasGrant(grants []string, want string) bool { + for _, g := range grants { + if g == want { + return true + } + } + return false +} + +// audienceMapper produces the resource audience on the access token, matching +// KeyCape's split where the ID token keeps the client audience. +func audienceMapper(audience string) KeycloakProtocolMapper { + return KeycloakProtocolMapper{ + Name: "keycape-audience", + Protocol: "openid-connect", + ProtocolMapper: "oidc-audience-mapper", + Config: map[string]string{ + "included.custom.audience": audience, + "access.token.claim": "true", + "id.token.claim": "false", + }, + } +} + +// hardcodedClaim produces a fixed claim on both tokens. +func hardcodedClaim(claim, value string) KeycloakProtocolMapper { + return KeycloakProtocolMapper{ + Name: "keycape-" + claim, + Protocol: "openid-connect", + ProtocolMapper: "oidc-hardcoded-claim-mapper", + Config: map[string]string{ + "claim.name": claim, + "claim.value": value, + "access.token.claim": "true", + "id.token.claim": "true", + "jsonType.label": "String", + }, + } } // splitDisplayName splits a display name at the first space. @@ -276,3 +459,56 @@ func itoa(n int) string { } return string(buf) } + +// realmRoles collects every role named by a user or a client registration. +// Roles referenced by a token but absent from the realm would be silently +// dropped by Keycloak at issuance. +func realmRoles(users []domain.User, clients []domain.Client) []KeycloakRole { + seen := map[string]bool{} + var names []string + add := func(role string) { + if role == "" || seen[role] { + return + } + seen[role] = true + names = append(names, role) + } + for _, u := range users { + for _, r := range u.Roles { + add(r) + } + } + for _, c := range clients { + for _, r := range c.Roles { + add(r) + } + } + sort.Strings(names) + roles := make([]KeycloakRole, 0, len(names)) + for _, n := range names { + roles = append(roles, KeycloakRole{Name: n}) + } + return roles +} + +// clientScopes collects every scope any registration allows, so the realm can +// grant what KeyCape granted. +func clientScopes(clients []domain.Client) []KeycloakClientScope { + seen := map[string]bool{} + var names []string + for _, c := range clients { + for _, s := range c.AllowedScopes { + if s == "" || seen[s] { + continue + } + seen[s] = true + names = append(names, s) + } + } + sort.Strings(names) + scopes := make([]KeycloakClientScope, 0, len(names)) + for _, n := range names { + scopes = append(scopes, KeycloakClientScope{Name: n, Protocol: "openid-connect"}) + } + return scopes +} diff --git a/src/internal/migration/tokeycloak/transformer_test.go b/src/internal/migration/tokeycloak/transformer_test.go index 249c9df..50dee2c 100644 --- a/src/internal/migration/tokeycloak/transformer_test.go +++ b/src/internal/migration/tokeycloak/transformer_test.go @@ -2,6 +2,8 @@ package tokeycloak_test import ( "context" + "encoding/json" + "strings" "testing" "time" @@ -438,3 +440,169 @@ func TestTransformer_ValidationReport_CleanExport(t *testing.T) { t.Errorf("expected no validation issues for clean export, got: %v", report) } } + +// --------------------------------------------------------------------------- +// Contract preservation (KEY-WP-0020) +// --------------------------------------------------------------------------- + +func serviceClient() domain.Client { + mfa := true + return domain.Client{ + ClientID: "service:consumer", + DisplayName: "Consumer", + ClientType: "confidential", + GrantTypes: []string{"client_credentials"}, + AllowedScopes: []string{"approval:read"}, + SecretRef: "env:CONSUMER_SECRET", + Audience: "approval-engine", + ServiceSubject: "service:consumer", + Tenant: "tenant:friendly:binky", + Roles: []string{"approver"}, + TokenLifetime: 15 * time.Minute, + MFARequired: &mfa, + } +} + +func transformWith(t *testing.T, users []domain.User, clients []domain.Client) (*tokeycloak.KeycloakRealm, *tokeycloak.Transformer) { + t.Helper() + tr := tokeycloak.New(tokeycloak.Config{RealmName: "netkingdom"}, telemetry.NoopEmitter{}) + realm, err := tr.TransformWithClients(&lldapexport.ExportResult{ + Users: users, ProfileVersion: "0.1", + }, clients) + if err != nil { + t.Fatalf("transform: %v", err) + } + return realm, tr +} + +// A service-only client must not gain the browser flow during migration, and a +// browser client must not gain service accounts. Hardcoding standardFlowEnabled +// widened every service registration. +func TestMapClient_FlowsFollowDeclaredGrants(t *testing.T) { + browser := domain.Client{ + ClientID: "portal", ClientType: "public", + GrantTypes: []string{"authorization_code"}, RedirectURIs: []string{"https://portal/cb"}, + } + realm, _ := transformWith(t, nil, []domain.Client{serviceClient(), browser}) + + svc, web := realm.Clients[0], realm.Clients[1] + if svc.StandardFlowEnabled { + t.Error("client_credentials client must not have the standard flow enabled") + } + if !svc.ServiceAccountsEnabled { + t.Error("client_credentials client must have service accounts enabled") + } + if !web.StandardFlowEnabled || web.ServiceAccountsEnabled { + t.Errorf("authorization_code client mapped wrong: standard=%v serviceAccounts=%v", + web.StandardFlowEnabled, web.ServiceAccountsEnabled) + } +} + +// The claims the NetKingdom profile requires have no native Keycloak concept, +// so they must survive as protocol mappers or they vanish at issuance. +func TestMapClient_CarriesProfileClaimsAndLifetime(t *testing.T) { + realm, _ := transformWith(t, nil, []domain.Client{serviceClient()}) + c := realm.Clients[0] + + want := map[string]bool{"keycape-audience": false, "keycape-tenant": false, + "keycape-service_subject": false, "keycape-roles": false} + for _, m := range c.ProtocolMappers { + if _, expected := want[m.Name]; expected { + want[m.Name] = true + } + if m.Protocol != "openid-connect" { + t.Errorf("mapper %q has protocol %q", m.Name, m.Protocol) + } + } + for name, found := range want { + if !found { + t.Errorf("protocol mapper %q missing; realm would issue tokens without that claim", name) + } + } + if c.Attributes["access.token.lifespan"] != "900" { + t.Errorf("per-client lifetime not carried: %q", c.Attributes["access.token.lifespan"]) + } + if c.Attributes["keycape.secretRef"] != "env:CONSUMER_SECRET" { + t.Errorf("secret reference not carried: %q", c.Attributes["keycape.secretRef"]) + } +} + +// A realm import file is not a secret custody boundary. Only the reference may +// appear, never a resolved value. +func TestTransform_NeverEmitsSecretValues(t *testing.T) { + c := serviceClient() + c.ClientSecret = "super-secret-value" + realm, _ := transformWith(t, nil, []domain.Client{c}) + + encoded, err := json.Marshal(realm) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(encoded), "super-secret-value") { + t.Fatal("resolved client secret appeared in the realm import JSON") + } +} + +// Roles and scopes were emitted as empty containers, so a realm that dropped +// every role looked identical to one that had none. +func TestTransform_DerivesRolesAndScopes(t *testing.T) { + users := []domain.User{{ + ID: "uid=alice,ou=users,dc=x", Username: "alice", DisplayName: "Alice A", + Enabled: true, Roles: []string{"viewer"}, Tenant: "tenant:friendly:binky", + }} + realm, _ := transformWith(t, users, []domain.Client{serviceClient()}) + + if len(realm.Roles.Realm) != 2 || + realm.Roles.Realm[0].Name != "approver" || realm.Roles.Realm[1].Name != "viewer" { + t.Errorf("realm roles not derived and sorted: %+v", realm.Roles.Realm) + } + if len(realm.ClientScopes) != 1 || realm.ClientScopes[0].Name != "approval:read" { + t.Errorf("client scopes not derived: %+v", realm.ClientScopes) + } + u := realm.Users[0] + if len(u.RealmRoles) != 1 || u.RealmRoles[0] != "viewer" { + t.Errorf("user roles not carried: %+v", u.RealmRoles) + } + if got := u.Attributes["keycape.canonicalId"]; len(got) != 1 || got[0] != users[0].ID { + t.Errorf("canonical ID not carried: %v", got) + } + if got := u.Attributes["tenant"]; len(got) != 1 || got[0] != "tenant:friendly:binky" { + t.Errorf("user tenant not carried: %v", got) + } +} + +// The point of the workplan: a field this tool cannot carry must be named, so a +// deliberate omission is distinguishable from a gap. +func TestUnpreservedReport_NamesEveryLimit(t *testing.T) { + users := []domain.User{{ID: "uid=alice,ou=users,dc=x", Username: "alice", DisplayName: "Alice A", Enabled: true}} + _, tr := transformWith(t, users, []domain.Client{serviceClient()}) + + report := strings.Join(tr.UnpreservedReport(), "\n") + for _, want := range []string{"serviceSubject", "secret is not migrated", "mfaRequired", + "passwords and MFA credentials", "subject continuity"} { + if !strings.Contains(report, want) { + t.Errorf("unpreserved report does not mention %q:\n%s", want, report) + } + } + + // A realm with no clients must say the client contract is absent rather + // than presenting an empty array as a finished migration. + _, bare := transformWith(t, users, nil) + if !strings.Contains(strings.Join(bare.UnpreservedReport(), "\n"), "no client registrations were supplied") { + t.Errorf("missing client registrations not reported: %v", bare.UnpreservedReport()) + } +} + +// Consistency and completeness are separate questions; an empty validation +// report must keep meaning "the realm matches the snapshot". +func TestValidationReportStaysSeparateFromUnpreserved(t *testing.T) { + users := []domain.User{{ID: "uid=alice,ou=users,dc=x", Username: "alice", DisplayName: "Alice A", Enabled: true}} + realm, tr := transformWith(t, users, []domain.Client{serviceClient()}) + + if issues := tr.ValidationReport(&lldapexport.ExportResult{Users: users, ProfileVersion: "0.1"}, realm); len(issues) != 0 { + t.Errorf("consistent realm reported issues: %v", issues) + } + if len(tr.UnpreservedReport()) == 0 { + t.Error("expected unpreserved items alongside a clean validation report") + } +} diff --git a/workplans/KEY-WP-0020-migration-contract-preservation.md b/workplans/KEY-WP-0020-migration-contract-preservation.md new file mode 100644 index 0000000..503798d --- /dev/null +++ b/workplans/KEY-WP-0020-migration-contract-preservation.md @@ -0,0 +1,103 @@ +--- +id: KEY-WP-0020 +type: workplan +title: "Make the Keycloak transform preserve or name every policy field" +domain: infotech +repo: key-cape +status: finished +owner: claude +topic_slug: migration-contract-preservation +created: "2026-09-07" +updated: "2026-09-07" +--- + +Closes the first half of gap G03 of +`history/2026-09-05-011726-scope-intent-assessment.md` — semantic preservation. +The second half, proof against a running Keycloak, is G04 and stays open. + +`keycape-to-keycloak` calls `Transform`, which passes no clients, so the realm it +writes has an empty `clients` array and nothing says the service-identity +contract was not migrated. `TransformWithClients` exists but no caller reaches +it. Where clients are supplied, `mapClient` always enables the standard flow +regardless of grant types, and drops `Audience`, `ServiceSubject`, `Tenant`, +`Roles`, `TokenLifetime`, `MFARequired`, `SecretRef` and the handoff URLs. +`mapUser` drops the canonical ID, tenant and roles. Realm roles and client scopes +are written as empty containers with a comment saying they "can be extended". + +The failure mode is not the missing mapping — it is that a dropped field and an +inapplicable one look identical in the output. An operator diffing the realm JSON +against the KeyCape config has no way to tell what this tool decided not to carry +across. Preserve what Keycloak can express, and name the rest. + +## Give the CLI the client registrations + +```task +id: KEY-WP-0020-T01 +status: done +priority: high +``` + +Add a `-clients` flag reading the KeyCape config's client registrations and pass +them through `TransformWithClients`. When no client file is given, say so on +stderr and in the report rather than emitting an empty `clients` array that reads +like a realm with no clients. + +Added `-clients`, reading registrations through a new `config.Registrations()` +that converts without resolving secrets — migration tooling needs the contract, +never the material, so `ClientSecret` is left empty by construction rather than +by remembering not to use it. An unparseable `tokenLifetime` is an error there, +since silently dropping a per-client lifetime is the defect class this closes. +Run against `config/dev-config.yaml`, all five registrations now reach the realm. + +## Preserve what Keycloak can express + +```task +id: KEY-WP-0020-T02 +status: done +priority: high +``` + +Derive `standardFlowEnabled` and `serviceAccountsEnabled` from the declared grant +types instead of hardcoding the standard flow on. Carry the audience, tenant, +service subject, roles and principal type as protocol mappers, per-client +lifetime as the Keycloak client attribute that expresses it, and the handoff URLs +and secret reference as attributes — the reference, never a secret value. Derive +realm roles and client scopes from the users and clients actually present rather +than emitting empty containers. Carry the user's canonical ID, tenant and roles. + +Flows now follow the grants: the old mapping set `standardFlowEnabled` on every +client, so migrating a `client_credentials` service registration silently gave it +the browser flow. Audience, tenant, service subject and roles ride as protocol +mappers, since Keycloak has no native concept for the profile's claims and would +otherwise issue tokens the profile rejects. Lifetime, handoff URLs and the secret +*reference* are attributes; a test marshals the realm and fails if a resolved +secret value appears, because a realm import file is not a custody boundary. +Realm roles and client scopes are derived and sorted. + +## Name everything not preserved + +```task +id: KEY-WP-0020-T03 +status: done +priority: high +``` + +Report each field the transform cannot carry, at the point it is dropped, so the +output distinguishes a deliberate omission from a gap: passwords and MFA +credentials, MFA policy enforcement, and subject continuity — LLDAP's canonical +ID is a DN, and Keycloak will mint its own `sub`, so tokens after a migration +will not carry the same subject unless something downstream maps it. Fail the +realm-level validation when a supplied client's contract is not fully +represented. Test that each unpreserved field appears in the report. + +`UnpreservedReport()` is separate from `ValidationReport()`. Folding them +together was the first attempt and it broke an existing test asserting a clean +export reports nothing — correctly so: consistency with the snapshot and +completeness of the migration are different questions, and one list cannot answer +both. Keeping them apart preserves the existing contract and makes the +distinction the point rather than a side effect. + +Six tests cover the flows, the mappers and lifetime, the secret-value exclusion, +the derived roles and scopes, every named limit, and the separation of the two +reports. Mutation-checked: restoring the hardcoded `standardFlowEnabled` fails +the flow test. An incomplete transform now emits `partial` telemetry.