From d03cc5eeffc69ddf42761201682099dd2b29d0fb Mon Sep 17 00:00:00 2001 From: PaulDotterer Date: Sat, 4 Jul 2026 16:57:56 +0200 Subject: [PATCH 1/3] refactor(events): typed payloads for PII-bearing emit sites (manchtools/power-manage-server#507) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert every SCIM, idp, totp, and identity-link event emit site from map[string]any literals to the shared internal/eventtypes/payloads structs, so the emit site and the projector decoder share one wire shape (compile-time drift detection) and spec 19's pii:"true" tag mechanism has typed structs to reflect over. - New payload structs: SCIMGroupMapped/Unmapped/MappingUpdated, TOTPVerified/Disabled/BackupCodeUsed, UserDeleted/Disabled/Enabled, IdentityProviderDeleted/SCIMDisabled. Empty structs marshal to {}, byte-identical to the legacy map emits (pinned by test). - IdentityLinkLoginUpdated gains user_id — the DEK-owner spec 19's crypto-shred layer resolves; both emitters (SSO linker, SCIM sync) populate it. Pre-#507 events decode with UserID == "". - IdentityUnlinked emitters now populate user_id/provider_id instead of {} (audit value; the projector keys off stream_id regardless). - payloads.UserGroupUpdated.Description becomes *string omitempty, aligned with the projector's update-vs-preserve contract; the SCIM rename path relies on nil-preserve, the API handler sets the pointer. Decoders for UserGroupUpdated + SCIMGroup* now decode via the shared structs (projector-local duplicates removed). - New archtest TestEventPayloadsAreTyped: exact-count per-file ratchet over Data: map[string]any emit literals — new untyped sites fail the build, and converting a site forces the allowance down in the same commit. Remaining 14 allowlisted sites are the non-PII broader-F-05 backlog. Wire compatibility: emitted keys are unchanged on every path (empty structs -> {}, always-set pointers -> always-present keys); the only additive changes are user_id fields and UserGroupCreated's is_dynamic/ dynamic_query defaults, all ignored-or-tolerated by the decoders. --- internal/api/auth_handler.go | 2 +- internal/api/idp_handler.go | 8 +- internal/api/totp_handler.go | 12 +- internal/api/user_group_handler.go | 2 +- internal/api/user_handler.go | 6 +- internal/archtest/typed_payloads_test.go | 132 ++++++++++++++++++ internal/eventtypes/payloads/identity_link.go | 17 ++- internal/eventtypes/payloads/idp.go | 11 ++ internal/eventtypes/payloads/payloads_test.go | 129 ++++++++++++++++- internal/eventtypes/payloads/scim_group.go | 29 ++++ internal/eventtypes/payloads/totp.go | 21 +++ internal/eventtypes/payloads/user.go | 21 ++- internal/eventtypes/payloads/user_group.go | 12 +- internal/idp/linker.go | 3 +- internal/idp/linker_test.go | 13 ++ internal/projectors/scim_group_mapping.go | 19 +-- internal/projectors/user_group.go | 12 +- internal/scim/groups.go | 7 +- internal/scim/groups_create.go | 43 +++--- internal/scim/groups_helpers.go | 35 ++--- internal/scim/groups_mutate.go | 58 ++++---- internal/scim/users.go | 5 +- internal/scim/users_create.go | 53 +++---- internal/scim/users_helpers.go | 34 +++-- internal/scim/users_link_test.go | 34 +++++ internal/scim/users_mutate.go | 56 ++++---- 26 files changed, 586 insertions(+), 188 deletions(-) create mode 100644 internal/archtest/typed_payloads_test.go create mode 100644 internal/eventtypes/payloads/scim_group.go diff --git a/internal/api/auth_handler.go b/internal/api/auth_handler.go index a6ed0584..9fc4ab76 100644 --- a/internal/api/auth_handler.go +++ b/internal/api/auth_handler.go @@ -184,7 +184,7 @@ func (h *AuthHandler) Login(ctx context.Context, req *connect.Request[pm.LoginRe StreamType: "user", StreamID: user.ID, EventType: string(eventtypes.UserLoggedIn), - Data: map[string]any{}, + Data: payloads.UserLoggedIn{}, ActorType: "user", ActorID: user.ID, }); err != nil { diff --git a/internal/api/idp_handler.go b/internal/api/idp_handler.go index 03432a33..dd143b0f 100644 --- a/internal/api/idp_handler.go +++ b/internal/api/idp_handler.go @@ -264,7 +264,7 @@ func (h *IDPHandler) DeleteIdentityProvider(ctx context.Context, req *connect.Re StreamType: "identity_provider", StreamID: providerID, EventType: string(eventtypes.IdentityProviderDeleted), - Data: map[string]any{}, + Data: payloads.IdentityProviderDeleted{}, ActorType: "user", ActorID: userCtx.ID, }, "failed to delete provider"); err != nil { @@ -277,7 +277,7 @@ func (h *IDPHandler) DeleteIdentityProvider(ctx context.Context, req *connect.Re StreamType: "identity_provider", StreamID: link.ID, EventType: string(eventtypes.IdentityUnlinked), - Data: map[string]any{}, + Data: payloads.IdentityUnlinked{UserID: link.UserID, ProviderID: providerID}, ActorType: "user", ActorID: userCtx.ID, }); err != nil { @@ -312,7 +312,7 @@ func (h *IDPHandler) DeleteIdentityProvider(ctx context.Context, req *connect.Re StreamType: "user", StreamID: link.UserID, EventType: string(eventtypes.UserDeleted), - Data: map[string]any{}, + Data: payloads.UserDeleted{}, ActorType: "user", ActorID: userCtx.ID, }) @@ -409,7 +409,7 @@ func (h *IDPHandler) DisableSCIM(ctx context.Context, req *connect.Request[pm.Di StreamType: "identity_provider", StreamID: req.Msg.Id, EventType: string(eventtypes.IdentityProviderSCIMDisabled), - Data: map[string]any{}, + Data: payloads.IdentityProviderSCIMDisabled{}, ActorType: "user", ActorID: userCtx.ID, }, "failed to disable SCIM"); err != nil { diff --git a/internal/api/totp_handler.go b/internal/api/totp_handler.go index d6227b55..355afaf7 100644 --- a/internal/api/totp_handler.go +++ b/internal/api/totp_handler.go @@ -166,7 +166,7 @@ func (h *TOTPHandler) VerifyTOTP(ctx context.Context, req *connect.Request[pm.Ve StreamType: "totp", StreamID: userCtx.ID, EventType: string(eventtypes.TOTPVerified), - Data: map[string]any{}, + Data: payloads.TOTPVerified{}, ActorType: "user", ActorID: userCtx.ID, }, "failed to verify TOTP"); err != nil { @@ -210,7 +210,7 @@ func (h *TOTPHandler) DisableTOTP(ctx context.Context, req *connect.Request[pm.D StreamType: "totp", StreamID: userCtx.ID, EventType: string(eventtypes.TOTPDisabled), - Data: map[string]any{}, + Data: payloads.TOTPDisabled{}, ActorType: "user", ActorID: userCtx.ID, }, "failed to disable TOTP"); err != nil { @@ -251,7 +251,7 @@ func (h *TOTPHandler) AdminDisableUserTOTP(ctx context.Context, req *connect.Req StreamType: "totp", StreamID: targetUserID, EventType: string(eventtypes.TOTPDisabled), - Data: map[string]any{"admin": true}, + Data: payloads.TOTPDisabled{Admin: true}, ActorType: "user", ActorID: userCtx.ID, }, "failed to disable TOTP"); err != nil { @@ -421,7 +421,7 @@ func (h *TOTPHandler) VerifyLoginTOTP(ctx context.Context, req *connect.Request[ StreamType: "totp", StreamID: claims.UserID, EventType: string(eventtypes.TOTPBackupCodeUsed), - Data: map[string]any{"index": idx}, + Data: payloads.TOTPBackupCodeUsed{Index: idx}, ActorType: "user", ActorID: claims.UserID, }, expectedVersion) @@ -466,7 +466,7 @@ func (h *TOTPHandler) VerifyLoginTOTP(ctx context.Context, req *connect.Request[ StreamType: "totp", StreamID: claims.UserID, EventType: string(eventtypes.TOTPBackupCodeUsed), - Data: map[string]any{"index": retryIdx}, + Data: payloads.TOTPBackupCodeUsed{Index: retryIdx}, ActorType: "user", ActorID: claims.UserID, }, retryExpectedVersion) @@ -542,7 +542,7 @@ func (h *TOTPHandler) VerifyLoginTOTP(ctx context.Context, req *connect.Request[ StreamType: "user", StreamID: claims.UserID, EventType: string(eventtypes.UserLoggedIn), - Data: map[string]any{}, + Data: payloads.UserLoggedIn{}, ActorType: "user", ActorID: claims.UserID, }); err != nil { diff --git a/internal/api/user_group_handler.go b/internal/api/user_group_handler.go index c57cb081..698047a2 100644 --- a/internal/api/user_group_handler.go +++ b/internal/api/user_group_handler.go @@ -232,7 +232,7 @@ func (h *UserGroupHandler) UpdateUserGroup(ctx context.Context, req *connect.Req EventType: string(eventtypes.UserGroupUpdated), Data: payloads.UserGroupUpdated{ Name: req.Msg.Name, - Description: req.Msg.Description, + Description: &req.Msg.Description, }, ActorType: "user", ActorID: userCtx.ID, diff --git a/internal/api/user_handler.go b/internal/api/user_handler.go index 5b4955ef..a7c7457b 100644 --- a/internal/api/user_handler.go +++ b/internal/api/user_handler.go @@ -417,8 +417,10 @@ func (h *UserHandler) SetUserDisabled(ctx context.Context, req *connect.Request[ // Emit appropriate event eventType := string(eventtypes.UserEnabled) + var data any = payloads.UserEnabled{} if req.Msg.Disabled { eventType = string(eventtypes.UserDisabled) + data = payloads.UserDisabled{} } appendDisable := func() error { @@ -426,7 +428,7 @@ func (h *UserHandler) SetUserDisabled(ctx context.Context, req *connect.Request[ StreamType: "user", StreamID: req.Msg.Id, EventType: eventType, - Data: map[string]any{}, + Data: data, ActorType: "user", ActorID: userCtx.ID, }); err != nil { @@ -489,7 +491,7 @@ func (h *UserHandler) DeleteUser(ctx context.Context, req *connect.Request[pm.De StreamType: "user", StreamID: req.Msg.Id, EventType: string(eventtypes.UserDeleted), - Data: map[string]any{}, + Data: payloads.UserDeleted{}, ActorType: "user", ActorID: userCtx.ID, }); err != nil { diff --git a/internal/archtest/typed_payloads_test.go b/internal/archtest/typed_payloads_test.go new file mode 100644 index 00000000..99820214 --- /dev/null +++ b/internal/archtest/typed_payloads_test.go @@ -0,0 +1,132 @@ +package archtest + +import ( + "go/ast" + "sort" + "strings" + "testing" +) + +// TestEventPayloadsAreTyped ratchets the F-05 conversion (#507): event +// emit sites must pass a typed struct from internal/eventtypes/payloads +// as Data, not a map[string]any literal. One shared struct between the +// emit site and the projector decoder catches wire-schema drift at +// compile time; a map literal re-opens the renamed-key / typo'd-key / +// dropped-field class the payloads package exists to close. Spec 19's +// pii:"true" reflection also only works over typed structs — an +// untyped PII emit site is invisible to the crypto layer. +// +// Mechanics: any composite literal carrying BOTH an EventType: and a +// Data: key (store.Event, idp.EventInput — every event-append shape in +// the module) whose Data value is a map composite literal is a legacy +// site. The allowlist below holds the not-yet-converted remainder with +// EXACT per-file counts, so the number can only shrink: a NEW map +// emit anywhere fails, a count above the allowance fails, and a count +// below (someone converted a site) fails too until the entry here is +// shrunk in the same commit — keeping the ratchet honest. +// +// internal/testutil is excluded: factories seed legacy wire shapes on +// purpose and are not production emit sites (drift there fails tests, +// not projections). Assigning a map VARIABLE to Data evades the +// syntactic detector — accepted; review catches it, and the projector +// decode contract still holds. +func TestEventPayloadsAreTyped(t *testing.T) { + root := moduleRoot(t) + files := walkGoFiles(t, root, func(rel string) bool { + if strings.HasPrefix(rel, "internal/store/generated/") { + return false + } + if strings.HasPrefix(rel, "internal/testutil/") { + return false + } + return true + }) + if len(files) == 0 { + t.Fatal("matches-zero guard: walked zero production Go files — detector is mis-scoped") + } + + // The F-05 remainder (issue #507, "broader" checkbox): non-PII emit + // sites pending package-by-package conversion. Exact counts. + allowed := map[string]int{ + "internal/api/action_crud.go": 2, + "internal/api/action_dispatch.go": 1, + "internal/api/action_set_handler.go": 1, + "internal/api/compliance_policy_handler.go": 1, + "internal/api/definition_handler.go": 1, + "internal/api/device_group_handler.go": 1, + "internal/api/device_handler.go": 1, + "internal/api/role_handler.go": 2, + "internal/api/settings_handler.go": 1, + "internal/api/system_action_store.go": 1, + "internal/api/token_handler.go": 2, + "internal/api/user_group_handler.go": 2, + } + + sawEmitLiteral := 0 // liveness: every EventType+Data literal, typed or not + found := map[string][]int{} + for _, gf := range files { + ast.Inspect(gf.ast, func(n ast.Node) bool { + lit, ok := n.(*ast.CompositeLit) + if !ok { + return true + } + dataVal, isEmit := eventDataValue(lit) + if !isEmit { + return true + } + sawEmitLiteral++ + inner, ok := dataVal.(*ast.CompositeLit) + if !ok { + return true + } + if _, isMap := inner.Type.(*ast.MapType); isMap { + found[gf.rel] = append(found[gf.rel], gf.line(inner)) + } + return true + }) + } + if sawEmitLiteral == 0 { + t.Fatal("matches-zero guard: found no event-append composite literal anywhere — the detector is dead, the guard would pass vacuously") + } + + for rel, lines := range found { + sort.Ints(lines) + switch want := allowed[rel]; { + case want == 0: + t.Errorf("untyped event payload: %s lines %v pass a map[string]any literal as Data — use a typed struct from internal/eventtypes/payloads (shared with the projector decoder) instead", rel, lines) + case len(lines) > want: + t.Errorf("untyped event payloads grew in %s: %d map-literal Data sites (lines %v), allowance is %d — convert the new site to a typed payloads struct", rel, len(lines), lines, want) + case len(lines) < want: + t.Errorf("ratchet stale for %s: %d map-literal Data sites remain (lines %v) but the allowance is %d — shrink the entry in this test to %d", rel, len(lines), lines, want, len(lines)) + } + } + for rel, want := range allowed { + if len(found[rel]) == 0 { + t.Errorf("ratchet stale: %s is allowed %d map-literal Data sites but has none — delete the entry", rel, want) + } + } +} + +// eventDataValue reports whether lit is an event-append composite +// literal (it names both EventType: and Data: keys) and returns the +// Data value expression. +func eventDataValue(lit *ast.CompositeLit) (ast.Expr, bool) { + var dataVal ast.Expr + hasEventType := false + for _, elt := range lit.Elts { + kv, ok := elt.(*ast.KeyValueExpr) + if !ok { + continue + } + switch identName(kv.Key) { + case "EventType": + hasEventType = true + case "Data": + dataVal = kv.Value + } + } + if !hasEventType || dataVal == nil { + return nil, false + } + return dataVal, true +} diff --git a/internal/eventtypes/payloads/identity_link.go b/internal/eventtypes/payloads/identity_link.go index baad1a53..95288af9 100644 --- a/internal/eventtypes/payloads/identity_link.go +++ b/internal/eventtypes/payloads/identity_link.go @@ -14,14 +14,17 @@ type IdentityLinked struct { ExternalName string `json:"external_name,omitempty"` } -// IdentityLinkLoginUpdated is the wire shape for the SSO refresh path -// — the user already has a link, but the IdP's claims drifted (display -// name, email). The projector updates the existing row's denormalised -// external_email / external_name. user_id is intentionally omitted -// here (the link is keyed by the (provider_id, external_id) tuple -// alone — the user_id is loaded by the projector from the existing -// row). +// IdentityLinkLoginUpdated is the wire shape for the SSO/SCIM refresh +// path — the user already has a link, but the IdP's claims drifted +// (display name, email). The projector updates the existing row's +// denormalised external_email / external_name, keyed by the +// (provider_id, external_id) tuple. user_id names the owning user: +// the projector doesn't need it (the tuple keys the row), but spec 19 +// does — external_email/external_name are PII, and the crypto-shred +// layer resolves the DEK owner from this field (#507). Events emitted +// before the field existed decode with UserID == "". type IdentityLinkLoginUpdated struct { + UserID string `json:"user_id"` ProviderID string `json:"provider_id"` ExternalID string `json:"external_id"` ExternalEmail string `json:"external_email,omitempty"` diff --git a/internal/eventtypes/payloads/idp.go b/internal/eventtypes/payloads/idp.go index 5562cd31..68a5bb39 100644 --- a/internal/eventtypes/payloads/idp.go +++ b/internal/eventtypes/payloads/idp.go @@ -18,6 +18,17 @@ type IdentityProviderSCIMTokenRotated struct { ScimTokenHash string `json:"scim_token_hash"` } +// IdentityProviderDeleted is the wire shape for IdentityProviderDeleted. +// Deliberately empty — the projector soft-deletes off stream_id and +// cascades the scim_group_mapping DELETE; `{}` on the wire matches the +// legacy map payload. +type IdentityProviderDeleted struct{} + +// IdentityProviderSCIMDisabled is the wire shape for +// IdentityProviderSCIMDisabled. Deliberately empty — the projector +// clears the token hash + cascades off stream_id alone. +type IdentityProviderSCIMDisabled struct{} + // IdentityProviderCreated is the wire shape for the IdentityProviderCreated // event. ClientSecretEncrypted is the AES-GCM ciphertext (the raw // secret never enters the event store; the audit-log redactor also diff --git a/internal/eventtypes/payloads/payloads_test.go b/internal/eventtypes/payloads/payloads_test.go index cb587030..b620546d 100644 --- a/internal/eventtypes/payloads/payloads_test.go +++ b/internal/eventtypes/payloads/payloads_test.go @@ -772,7 +772,7 @@ func TestRoundtrip_UserGroupCreated(t *testing.T) { } func TestRoundtrip_UserGroupUpdated(t *testing.T) { - in := payloads.UserGroupUpdated{Name: "ops", Description: "ops team"} + in := payloads.UserGroupUpdated{Name: "ops", Description: ptr("ops team")} raw, err := json.Marshal(in) require.NoError(t, err) var out payloads.UserGroupUpdated @@ -962,3 +962,130 @@ func TestRoundtrip_DeviceGroupMaintenanceWindowSet(t *testing.T) { require.NoError(t, json.Unmarshal(raw, &out)) assert.Equal(t, in, out) } + +// UserGroupUpdated's nil Description must stay OFF the wire — the +// projector reads a missing key as "preserve the existing description" +// (the SCIM rename path depends on this; an accidental always-emit +// would wipe descriptions on every SCIM group rename). +func TestUserGroupUpdated_NilDescriptionOmitsKey(t *testing.T) { + raw, err := json.Marshal(payloads.UserGroupUpdated{Name: "ops"}) + require.NoError(t, err) + assert.JSONEq(t, `{"name":"ops"}`, string(raw)) +} + +func TestRoundtrip_IdentityLinked(t *testing.T) { + in := payloads.IdentityLinked{ + UserID: "01USER", + ProviderID: "01PROV", + ExternalID: "ext-1", + ExternalEmail: "a@b.com", + ExternalName: "Alice Example", + } + raw, err := json.Marshal(in) + require.NoError(t, err) + var out payloads.IdentityLinked + require.NoError(t, json.Unmarshal(raw, &out)) + assert.Equal(t, in, out) +} + +// user_id was added to IdentityLinkLoginUpdated for spec 19 (#507): +// external_email/external_name are PII and the crypto-shred layer +// resolves the DEK owner from this field. The roundtrip pins the key; +// events emitted before the field existed decode with UserID == "". +func TestRoundtrip_IdentityLinkLoginUpdated(t *testing.T) { + in := payloads.IdentityLinkLoginUpdated{ + UserID: "01USER", + ProviderID: "01PROV", + ExternalID: "ext-1", + ExternalEmail: "a@b.com", + ExternalName: "Alice Example", + } + raw, err := json.Marshal(in) + require.NoError(t, err) + assert.Contains(t, string(raw), `"user_id":"01USER"`) + var out payloads.IdentityLinkLoginUpdated + require.NoError(t, json.Unmarshal(raw, &out)) + assert.Equal(t, in, out) + + legacy := []byte(`{"provider_id":"01PROV","external_id":"ext-1"}`) + var old payloads.IdentityLinkLoginUpdated + require.NoError(t, json.Unmarshal(legacy, &old)) + assert.Empty(t, old.UserID, "pre-#507 events must decode with empty UserID") +} + +func TestRoundtrip_SCIMGroupMapped(t *testing.T) { + in := payloads.SCIMGroupMapped{ + ProviderID: "01PROV", + SCIMGroupID: "scim-g-1", + SCIMDisplayName: ptr("Ops"), + UserGroupID: "01GROUP", + } + raw, err := json.Marshal(in) + require.NoError(t, err) + var out payloads.SCIMGroupMapped + require.NoError(t, json.Unmarshal(raw, &out)) + assert.Equal(t, in, out) +} + +func TestRoundtrip_SCIMGroupUnmapped(t *testing.T) { + in := payloads.SCIMGroupUnmapped{ProviderID: "01PROV", SCIMGroupID: "scim-g-1"} + raw, err := json.Marshal(in) + require.NoError(t, err) + var out payloads.SCIMGroupUnmapped + require.NoError(t, json.Unmarshal(raw, &out)) + assert.Equal(t, in, out) +} + +func TestRoundtrip_SCIMGroupMappingUpdated(t *testing.T) { + in := payloads.SCIMGroupMappingUpdated{ + ProviderID: "01PROV", + SCIMGroupID: "scim-g-1", + SCIMDisplayName: ptr(""), + } + raw, err := json.Marshal(in) + require.NoError(t, err) + // Explicit "" stays ON the wire (pointer set) — matches the legacy + // always-present map key; nil would mean "preserve" to the decoder. + assert.Contains(t, string(raw), `"scim_display_name":""`) + var out payloads.SCIMGroupMappingUpdated + require.NoError(t, json.Unmarshal(raw, &out)) + assert.Equal(t, in, out) +} + +func TestRoundtrip_TOTPBackupCodeUsed(t *testing.T) { + in := payloads.TOTPBackupCodeUsed{Index: 0} + raw, err := json.Marshal(in) + require.NoError(t, err) + // Index 0 is a valid slot and must stay on the wire (no omitempty). + assert.JSONEq(t, `{"index":0}`, string(raw)) + var out payloads.TOTPBackupCodeUsed + require.NoError(t, json.Unmarshal(raw, &out)) + assert.Equal(t, in, out) +} + +// The empty lifecycle payloads must marshal to `{}`, byte-identical to +// the legacy map[string]any{} emits, so the wire format is unchanged +// by the #507 typed-struct conversion. +func TestEmptyPayloads_MarshalToEmptyObject(t *testing.T) { + for name, v := range map[string]any{ + "TOTPVerified": payloads.TOTPVerified{}, + "TOTPDisabled (self-service)": payloads.TOTPDisabled{}, + "UserDeleted": payloads.UserDeleted{}, + "UserDisabled": payloads.UserDisabled{}, + "UserEnabled": payloads.UserEnabled{}, + "IdentityProviderDeleted": payloads.IdentityProviderDeleted{}, + "IdentityProviderSCIMDisabled": payloads.IdentityProviderSCIMDisabled{}, + "UserLoggedIn (local login)": payloads.UserLoggedIn{}, + } { + raw, err := json.Marshal(v) + require.NoError(t, err, name) + assert.Equal(t, `{}`, string(raw), name) + } +} + +// The admin variant of TOTPDisabled carries the audit marker. +func TestTOTPDisabled_AdminVariant(t *testing.T) { + raw, err := json.Marshal(payloads.TOTPDisabled{Admin: true}) + require.NoError(t, err) + assert.JSONEq(t, `{"admin":true}`, string(raw)) +} diff --git a/internal/eventtypes/payloads/scim_group.go b/internal/eventtypes/payloads/scim_group.go new file mode 100644 index 00000000..99037e49 --- /dev/null +++ b/internal/eventtypes/payloads/scim_group.go @@ -0,0 +1,29 @@ +package payloads + +// SCIMGroupMapped is the wire shape for SCIMGroupMapped — a SCIM +// provider group bound to a local user group. The projector UPSERTs +// the mapping row keyed by (provider_id, scim_group_id). The pointer +// SCIMDisplayName preserves the decoder's absent-vs-empty distinction +// (absent defaults to ""); emit sites always set it. +type SCIMGroupMapped struct { + ProviderID string `json:"provider_id"` + SCIMGroupID string `json:"scim_group_id"` + SCIMDisplayName *string `json:"scim_display_name,omitempty"` + UserGroupID string `json:"user_group_id"` +} + +// SCIMGroupUnmapped is the wire shape for SCIMGroupUnmapped — the +// composite key of the mapping row the projector deletes. +type SCIMGroupUnmapped struct { + ProviderID string `json:"provider_id"` + SCIMGroupID string `json:"scim_group_id"` +} + +// SCIMGroupMappingUpdated is the wire shape for SCIMGroupMappingUpdated. +// Only the display name is updatable; the pointer signals update +// (non-nil, including "") vs preserve (nil → COALESCE keeps existing). +type SCIMGroupMappingUpdated struct { + ProviderID string `json:"provider_id"` + SCIMGroupID string `json:"scim_group_id"` + SCIMDisplayName *string `json:"scim_display_name,omitempty"` +} diff --git a/internal/eventtypes/payloads/totp.go b/internal/eventtypes/payloads/totp.go index 119f6302..b9f36102 100644 --- a/internal/eventtypes/payloads/totp.go +++ b/internal/eventtypes/payloads/totp.go @@ -16,3 +16,24 @@ type TOTPSetupInitiated struct { type TOTPBackupCodesRegenerated struct { BackupCodesHash []string `json:"backup_codes_hash"` } + +// TOTPVerified is the wire shape for TOTPVerified. Deliberately empty +// — the projector flips the enabled flag off the event's stream_id +// alone; the zero struct marshals to `{}`, byte-identical to the +// legacy map[string]any{} payload. +type TOTPVerified struct{} + +// TOTPDisabled is the wire shape for TOTPDisabled. admin marks the +// AdminDisableUserTOTP path (audit context: the actor disabled someone +// ELSE's TOTP); the self-service path emits the zero struct, which +// marshals to `{}` exactly like the legacy payload. +type TOTPDisabled struct { + Admin bool `json:"admin,omitempty"` +} + +// TOTPBackupCodeUsed is the wire shape for TOTPBackupCodeUsed. Index +// is the position of the consumed code in the backup_codes_hash slice +// — the projector NULLs that slot so the code is single-use. +type TOTPBackupCodeUsed struct { + Index int `json:"index"` +} diff --git a/internal/eventtypes/payloads/user.go b/internal/eventtypes/payloads/user.go index 96e4b4f3..e03ea5cf 100644 --- a/internal/eventtypes/payloads/user.go +++ b/internal/eventtypes/payloads/user.go @@ -81,14 +81,25 @@ type UserLinuxUsernameChanged struct { LinuxUsername *string `json:"linux_username,omitempty"` } -// UserLoggedIn is the wire shape for the UserLoggedIn audit event the -// SSO callback handler appends after a successful authentication. The -// projector reads only `provider` to denormalise "what IdP did this -// user last authenticate via" onto users_projection. +// UserLoggedIn is the wire shape for the UserLoggedIn audit event +// appended after a successful authentication. SSO logins carry the +// provider slug; local-credential logins (password, TOTP step-up) +// have no provider and emit the zero struct — omitempty keeps that +// byte-identical to the legacy `{}` payload. type UserLoggedIn struct { - Provider string `json:"provider"` + Provider string `json:"provider,omitempty"` } +// UserDeleted / UserDisabled / UserEnabled are the wire shapes for the +// user lifecycle toggles. Deliberately empty — the projector keys off +// the event's stream_id alone; the zero structs marshal to `{}`, +// byte-identical to the legacy map[string]any{} payloads. +type UserDeleted struct{} + +type UserDisabled struct{} + +type UserEnabled struct{} + // UserSystemActionLinked is the wire shape for UserSystemActionLinked. // Field selects which of the three system_*_action_id columns gets // the supplied action_id (see SystemActionField* constants in the diff --git a/internal/eventtypes/payloads/user_group.go b/internal/eventtypes/payloads/user_group.go index 08f88126..53c33610 100644 --- a/internal/eventtypes/payloads/user_group.go +++ b/internal/eventtypes/payloads/user_group.go @@ -13,11 +13,15 @@ type UserGroupCreated struct { DynamicQuery string `json:"dynamic_query"` } -// UserGroupUpdated is the wire shape for UserGroupUpdated. Same -// always-emit semantics as UserGroupCreated. +// UserGroupUpdated is the wire shape for UserGroupUpdated. The +// pointer Description mirrors the projector's update-vs-preserve +// contract: non-nil = write the value (including explicit ""), nil = +// key absent on the wire = preserve the existing description. The +// SCIM rename path relies on nil-preserve (it only carries the name); +// the API update path always sets the pointer. type UserGroupUpdated struct { - Name string `json:"name"` - Description string `json:"description"` + Name string `json:"name"` + Description *string `json:"description,omitempty"` } // UserGroupMaintenanceWindowSet is the wire shape for the diff --git a/internal/idp/linker.go b/internal/idp/linker.go index 6c0c6cda..3522efbf 100644 --- a/internal/idp/linker.go +++ b/internal/idp/linker.go @@ -110,7 +110,7 @@ func (l *Linker) LinkOrCreate(ctx context.Context, provider store.IdentityProvid StreamType: "identity_provider", StreamID: link.ID, EventType: string(eventtypes.IdentityUnlinked), - Data: map[string]any{}, + Data: payloads.IdentityUnlinked{UserID: link.UserID, ProviderID: link.ProviderID}, ActorType: "system", ActorID: "sso", }); err != nil { @@ -126,6 +126,7 @@ func (l *Linker) LinkOrCreate(ctx context.Context, provider store.IdentityProvid StreamID: link.ID, EventType: string(eventtypes.IdentityLinkLoginUpdated), Data: payloads.IdentityLinkLoginUpdated{ + UserID: link.UserID, ProviderID: provider.ID, ExternalID: claims.Subject, ExternalEmail: claims.Email, diff --git a/internal/idp/linker_test.go b/internal/idp/linker_test.go index 814b0464..564a93f6 100644 --- a/internal/idp/linker_test.go +++ b/internal/idp/linker_test.go @@ -285,6 +285,19 @@ func TestLinkOrCreate_ExistingLink_UpdatesLogin(t *testing.T) { "existing live link must update login, got: %v", eventTypes(appender)) assert.Equal(t, 0, countEventsOfType(appender, "IdentityLinked"), "must not emit a fresh IdentityLinked for an already-linked user") + + // #507 / spec 19: the login-update payload carries PII + // (external_email/external_name), so it must name the owning user — + // the crypto-shred layer resolves the DEK owner from user_id. + for _, e := range appender.events { + if e.EventType != "IdentityLinkLoginUpdated" { + continue + } + upd, ok := e.Data.(payloads.IdentityLinkLoginUpdated) + require.True(t, ok, "login update must emit the shared payloads struct, got %T", e.Data) + assert.Equal(t, "user-1", upd.UserID, + "IdentityLinkLoginUpdated must carry the DEK-owner user_id") + } } // TestLinkOrCreate_SoftDeletedLinkedUser_CleansUpAndFallsThrough pins WS5 #3: a diff --git a/internal/projectors/scim_group_mapping.go b/internal/projectors/scim_group_mapping.go index 1f3db9f7..ca72d309 100644 --- a/internal/projectors/scim_group_mapping.go +++ b/internal/projectors/scim_group_mapping.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/manchtools/power-manage/server/internal/eventtypes" + "github.com/manchtools/power-manage/server/internal/eventtypes/payloads" "github.com/manchtools/power-manage/server/internal/store" ) @@ -35,12 +36,7 @@ type SCIMGroupMappingUpdatedPayload struct { // SCIMGroupMappedFromEvent decodes SCIMGroupMapped. func SCIMGroupMappedFromEvent(e store.PersistedEvent) (SCIMGroupMappedPayload, error) { - raw, err := decodePayload[struct { - ProviderID string `json:"provider_id"` - SCIMGroupID string `json:"scim_group_id"` - SCIMDisplayName *string `json:"scim_display_name,omitempty"` - UserGroupID string `json:"user_group_id"` - }](e, "scim_group_mapping", eventtypes.SCIMGroupMapped) + raw, err := decodePayload[payloads.SCIMGroupMapped](e, "scim_group_mapping", eventtypes.SCIMGroupMapped) if err != nil { return SCIMGroupMappedPayload{}, err } @@ -66,10 +62,7 @@ func SCIMGroupMappedFromEvent(e store.PersistedEvent) (SCIMGroupMappedPayload, e // SCIMGroupUnmappedFromEvent decodes SCIMGroupUnmapped. func SCIMGroupUnmappedFromEvent(e store.PersistedEvent) (SCIMGroupUnmappedPayload, error) { - raw, err := decodePayload[struct { - ProviderID string `json:"provider_id"` - SCIMGroupID string `json:"scim_group_id"` - }](e, "scim_group_mapping", eventtypes.SCIMGroupUnmapped) + raw, err := decodePayload[payloads.SCIMGroupUnmapped](e, "scim_group_mapping", eventtypes.SCIMGroupUnmapped) if err != nil { return SCIMGroupUnmappedPayload{}, err } @@ -86,11 +79,7 @@ func SCIMGroupUnmappedFromEvent(e store.PersistedEvent) (SCIMGroupUnmappedPayloa // Only scim_display_name is updatable; pointer field preserves // "missing → no update" via COALESCE. func SCIMGroupMappingUpdatedFromEvent(e store.PersistedEvent) (SCIMGroupMappingUpdatedPayload, error) { - raw, err := decodePayload[struct { - ProviderID string `json:"provider_id"` - SCIMGroupID string `json:"scim_group_id"` - SCIMDisplayName *string `json:"scim_display_name,omitempty"` - }](e, "scim_group_mapping", eventtypes.SCIMGroupMappingUpdated) + raw, err := decodePayload[payloads.SCIMGroupMappingUpdated](e, "scim_group_mapping", eventtypes.SCIMGroupMappingUpdated) if err != nil { return SCIMGroupMappingUpdatedPayload{}, err } diff --git a/internal/projectors/user_group.go b/internal/projectors/user_group.go index 4f24b19b..bd52173e 100644 --- a/internal/projectors/user_group.go +++ b/internal/projectors/user_group.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/manchtools/power-manage/server/internal/eventtypes" + "github.com/manchtools/power-manage/server/internal/eventtypes/payloads" "github.com/manchtools/power-manage/server/internal/store" ) @@ -87,16 +88,13 @@ type UserGroupUpdatedPayload struct { Description *string } -type userGroupUpdatedRaw struct { - Name string `json:"name"` - Description *string `json:"description,omitempty"` -} - // UserGroupUpdatedFromEvent decodes UserGroupUpdated. The pointer // Description signals "update" vs "preserve": non-nil = update with -// the value (incl. empty string); nil = preserve. +// the value (incl. empty string); nil = preserve. Decodes via the +// shared payloads struct — the emit sites (API handler, SCIM rename) +// construct the same type, so a wire-key drift fails at compile time. func UserGroupUpdatedFromEvent(e store.PersistedEvent) (UserGroupUpdatedPayload, error) { - raw, err := decodePayload[userGroupUpdatedRaw](e, "user_group", eventtypes.UserGroupUpdated) + raw, err := decodePayload[payloads.UserGroupUpdated](e, "user_group", eventtypes.UserGroupUpdated) if err != nil { return UserGroupUpdatedPayload{}, err } diff --git a/internal/scim/groups.go b/internal/scim/groups.go index 44e371d6..00e441a1 100644 --- a/internal/scim/groups.go +++ b/internal/scim/groups.go @@ -6,6 +6,7 @@ import ( "strconv" "github.com/manchtools/power-manage/server/internal/eventtypes" + "github.com/manchtools/power-manage/server/internal/eventtypes/payloads" "github.com/manchtools/power-manage/server/internal/store" ) @@ -206,9 +207,9 @@ func (h *Handler) deleteGroup(w http.ResponseWriter, r *http.Request) { StreamType: "scim_group_mapping", StreamID: mapping.ID, EventType: string(eventtypes.SCIMGroupUnmapped), - Data: map[string]any{ - "provider_id": provider.ID, - "scim_group_id": mapping.SCIMGroupID, + Data: payloads.SCIMGroupUnmapped{ + ProviderID: provider.ID, + SCIMGroupID: mapping.SCIMGroupID, }, ActorType: "scim", ActorID: provider.ID, diff --git a/internal/scim/groups_create.go b/internal/scim/groups_create.go index 8a27d296..2d1552ee 100644 --- a/internal/scim/groups_create.go +++ b/internal/scim/groups_create.go @@ -12,6 +12,7 @@ import ( "net/http" "github.com/manchtools/power-manage/server/internal/eventtypes" + "github.com/manchtools/power-manage/server/internal/eventtypes/payloads" "github.com/manchtools/power-manage/server/internal/store" ) @@ -59,10 +60,10 @@ func (h *Handler) createGroup(w http.ResponseWriter, r *http.Request) { StreamType: "scim_group_mapping", StreamID: existing.ID, EventType: string(eventtypes.SCIMGroupMappingUpdated), - Data: map[string]any{ - "provider_id": provider.ID, - "scim_group_id": scimGroupID, - "scim_display_name": scimGroup.DisplayName, + Data: payloads.SCIMGroupMappingUpdated{ + ProviderID: provider.ID, + SCIMGroupID: scimGroupID, + SCIMDisplayName: &scimGroup.DisplayName, }, ActorType: "scim", ActorID: provider.ID, @@ -71,8 +72,10 @@ func (h *Handler) createGroup(w http.ResponseWriter, r *http.Request) { StreamType: "user_group", StreamID: existing.UserGroupID, EventType: string(eventtypes.UserGroupUpdated), - Data: map[string]any{ - "name": scimGroup.DisplayName, + // nil Description = preserve the existing one (SCIM + // only renames; the description is server-owned). + Data: payloads.UserGroupUpdated{ + Name: scimGroup.DisplayName, }, ActorType: "scim", ActorID: provider.ID, @@ -97,9 +100,9 @@ func (h *Handler) createGroup(w http.ResponseWriter, r *http.Request) { StreamType: "scim_group_mapping", StreamID: existing.ID, EventType: string(eventtypes.SCIMGroupUnmapped), - Data: map[string]any{ - "provider_id": provider.ID, - "scim_group_id": existing.SCIMGroupID, + Data: payloads.SCIMGroupUnmapped{ + ProviderID: provider.ID, + SCIMGroupID: existing.SCIMGroupID, }, ActorType: "scim", ActorID: provider.ID, @@ -123,9 +126,9 @@ func (h *Handler) createGroup(w http.ResponseWriter, r *http.Request) { StreamType: "user_group", StreamID: userGroupID, EventType: string(eventtypes.UserGroupCreated), - Data: map[string]any{ - "name": scimGroup.DisplayName, - "description": fmt.Sprintf("SCIM-provisioned group from %s", provider.Name), + Data: payloads.UserGroupCreated{ + Name: scimGroup.DisplayName, + Description: fmt.Sprintf("SCIM-provisioned group from %s", provider.Name), }, ActorType: "scim", ActorID: provider.ID, @@ -142,11 +145,11 @@ func (h *Handler) createGroup(w http.ResponseWriter, r *http.Request) { StreamType: "scim_group_mapping", StreamID: mappingID, EventType: string(eventtypes.SCIMGroupMapped), - Data: map[string]any{ - "provider_id": provider.ID, - "scim_group_id": scimGroupID, - "scim_display_name": scimGroup.DisplayName, - "user_group_id": userGroupID, + Data: payloads.SCIMGroupMapped{ + ProviderID: provider.ID, + SCIMGroupID: scimGroupID, + SCIMDisplayName: &scimGroup.DisplayName, + UserGroupID: userGroupID, }, ActorType: "scim", ActorID: provider.ID, @@ -170,9 +173,9 @@ func (h *Handler) createGroup(w http.ResponseWriter, r *http.Request) { StreamType: "user_group", StreamID: streamID, EventType: string(eventtypes.UserGroupMemberAdded), - Data: map[string]any{ - "group_id": userGroupID, - "user_id": member.Value, + Data: payloads.UserGroupMemberAdded{ + GroupID: userGroupID, + UserID: member.Value, }, ActorType: "scim", ActorID: provider.ID, diff --git a/internal/scim/groups_helpers.go b/internal/scim/groups_helpers.go index 24daaf6d..6c643da0 100644 --- a/internal/scim/groups_helpers.go +++ b/internal/scim/groups_helpers.go @@ -11,6 +11,7 @@ import ( "time" "github.com/manchtools/power-manage/server/internal/eventtypes" + "github.com/manchtools/power-manage/server/internal/eventtypes/payloads" "github.com/manchtools/power-manage/server/internal/store" ) @@ -69,9 +70,9 @@ func (h *Handler) reconcileGroupMembers(ctx context.Context, provider store.Iden StreamType: "user_group", StreamID: streamID, EventType: string(eventtypes.UserGroupMemberAdded), - Data: map[string]any{ - "group_id": groupID, - "user_id": userID, + Data: payloads.UserGroupMemberAdded{ + GroupID: groupID, + UserID: userID, }, ActorType: "scim", ActorID: provider.ID, @@ -88,9 +89,9 @@ func (h *Handler) reconcileGroupMembers(ctx context.Context, provider store.Iden StreamType: "user_group", StreamID: streamID, EventType: string(eventtypes.UserGroupMemberRemoved), - Data: map[string]any{ - "group_id": groupID, - "user_id": userID, + Data: payloads.UserGroupMemberRemoved{ + GroupID: groupID, + UserID: userID, }, ActorType: "scim", ActorID: provider.ID, @@ -163,9 +164,9 @@ func (h *Handler) restoreOrphanedGroup(ctx context.Context, providerID string, m StreamType: "user_group", StreamID: newGroupID, EventType: string(eventtypes.UserGroupCreated), - Data: map[string]any{ - "name": m.SCIMDisplayName, - "description": "SCIM-provisioned group (restored)", + Data: payloads.UserGroupCreated{ + Name: m.SCIMDisplayName, + Description: "SCIM-provisioned group (restored)", }, ActorType: "scim", ActorID: providerID, @@ -178,9 +179,9 @@ func (h *Handler) restoreOrphanedGroup(ctx context.Context, providerID string, m StreamType: "scim_group_mapping", StreamID: m.ID, EventType: string(eventtypes.SCIMGroupUnmapped), - Data: map[string]any{ - "provider_id": providerID, - "scim_group_id": m.SCIMGroupID, + Data: payloads.SCIMGroupUnmapped{ + ProviderID: providerID, + SCIMGroupID: m.SCIMGroupID, }, ActorType: "scim", ActorID: providerID, @@ -192,11 +193,11 @@ func (h *Handler) restoreOrphanedGroup(ctx context.Context, providerID string, m StreamType: "scim_group_mapping", StreamID: newMappingID, EventType: string(eventtypes.SCIMGroupMapped), - Data: map[string]any{ - "provider_id": providerID, - "scim_group_id": m.SCIMGroupID, - "scim_display_name": m.SCIMDisplayName, - "user_group_id": newGroupID, + Data: payloads.SCIMGroupMapped{ + ProviderID: providerID, + SCIMGroupID: m.SCIMGroupID, + SCIMDisplayName: &m.SCIMDisplayName, + UserGroupID: newGroupID, }, ActorType: "scim", ActorID: providerID, diff --git a/internal/scim/groups_mutate.go b/internal/scim/groups_mutate.go index 8009fed0..4b3d6219 100644 --- a/internal/scim/groups_mutate.go +++ b/internal/scim/groups_mutate.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/manchtools/power-manage/server/internal/eventtypes" + "github.com/manchtools/power-manage/server/internal/eventtypes/payloads" "github.com/manchtools/power-manage/server/internal/store" ) @@ -57,10 +58,10 @@ func (h *Handler) replaceGroup(w http.ResponseWriter, r *http.Request) { StreamType: "scim_group_mapping", StreamID: mapping.ID, EventType: string(eventtypes.SCIMGroupMappingUpdated), - Data: map[string]any{ - "provider_id": provider.ID, - "scim_group_id": mapping.SCIMGroupID, - "scim_display_name": scimGroup.DisplayName, + Data: payloads.SCIMGroupMappingUpdated{ + ProviderID: provider.ID, + SCIMGroupID: mapping.SCIMGroupID, + SCIMDisplayName: &scimGroup.DisplayName, }, ActorType: "scim", ActorID: provider.ID, @@ -70,8 +71,10 @@ func (h *Handler) replaceGroup(w http.ResponseWriter, r *http.Request) { StreamType: "user_group", StreamID: groupID, EventType: string(eventtypes.UserGroupUpdated), - Data: map[string]any{ - "name": scimGroup.DisplayName, + // nil Description = preserve the existing one (SCIM + // only renames; the description is server-owned). + Data: payloads.UserGroupUpdated{ + Name: scimGroup.DisplayName, }, ActorType: "scim", ActorID: provider.ID, @@ -195,9 +198,9 @@ func (h *Handler) handleGroupPatchAdd(ctx context.Context, provider store.Identi StreamType: "user_group", StreamID: streamID, EventType: string(eventtypes.UserGroupMemberAdded), - Data: map[string]any{ - "group_id": groupID, - "user_id": userID, + Data: payloads.UserGroupMemberAdded{ + GroupID: groupID, + UserID: userID, }, ActorType: "scim", ActorID: provider.ID, @@ -219,9 +222,9 @@ func (h *Handler) handleGroupPatchRemove(ctx context.Context, provider store.Ide StreamType: "user_group", StreamID: streamID, EventType: string(eventtypes.UserGroupMemberRemoved), - Data: map[string]any{ - "group_id": groupID, - "user_id": userID, + Data: payloads.UserGroupMemberRemoved{ + GroupID: groupID, + UserID: userID, }, ActorType: "scim", ActorID: provider.ID, @@ -239,9 +242,9 @@ func (h *Handler) handleGroupPatchRemove(ctx context.Context, provider store.Ide StreamType: "user_group", StreamID: streamID, EventType: string(eventtypes.UserGroupMemberRemoved), - Data: map[string]any{ - "group_id": groupID, - "user_id": userID, + Data: payloads.UserGroupMemberRemoved{ + GroupID: groupID, + UserID: userID, }, ActorType: "scim", ActorID: provider.ID, @@ -265,10 +268,10 @@ func (h *Handler) handleGroupPatchReplace(ctx context.Context, provider store.Id StreamType: "scim_group_mapping", StreamID: mapping.ID, EventType: string(eventtypes.SCIMGroupMappingUpdated), - Data: map[string]any{ - "provider_id": provider.ID, - "scim_group_id": mapping.SCIMGroupID, - "scim_display_name": name, + Data: payloads.SCIMGroupMappingUpdated{ + ProviderID: provider.ID, + SCIMGroupID: mapping.SCIMGroupID, + SCIMDisplayName: &name, }, ActorType: "scim", ActorID: provider.ID, @@ -278,8 +281,9 @@ func (h *Handler) handleGroupPatchReplace(ctx context.Context, provider store.Id StreamType: "user_group", StreamID: groupID, EventType: string(eventtypes.UserGroupUpdated), - Data: map[string]any{ - "name": name, + // nil Description = preserve (SCIM only renames). + Data: payloads.UserGroupUpdated{ + Name: name, }, ActorType: "scim", ActorID: provider.ID, @@ -316,9 +320,9 @@ func (h *Handler) handleGroupPatchReplace(ctx context.Context, provider store.Id StreamType: "user_group", StreamID: streamID, EventType: string(eventtypes.UserGroupMemberAdded), - Data: map[string]any{ - "group_id": groupID, - "user_id": userID, + Data: payloads.UserGroupMemberAdded{ + GroupID: groupID, + UserID: userID, }, ActorType: "scim", ActorID: provider.ID, @@ -335,9 +339,9 @@ func (h *Handler) handleGroupPatchReplace(ctx context.Context, provider store.Id StreamType: "user_group", StreamID: streamID, EventType: string(eventtypes.UserGroupMemberRemoved), - Data: map[string]any{ - "group_id": groupID, - "user_id": userID, + Data: payloads.UserGroupMemberRemoved{ + GroupID: groupID, + UserID: userID, }, ActorType: "scim", ActorID: provider.ID, diff --git a/internal/scim/users.go b/internal/scim/users.go index 5f41c369..1c7e534f 100644 --- a/internal/scim/users.go +++ b/internal/scim/users.go @@ -6,6 +6,7 @@ import ( "strconv" "github.com/manchtools/power-manage/server/internal/eventtypes" + "github.com/manchtools/power-manage/server/internal/eventtypes/payloads" "github.com/manchtools/power-manage/server/internal/store" db "github.com/manchtools/power-manage/server/internal/store/generated" ) @@ -229,7 +230,7 @@ func (h *Handler) deleteUser(w http.ResponseWriter, r *http.Request) { StreamType: "identity_provider", StreamID: link.ID, EventType: string(eventtypes.IdentityUnlinked), - Data: map[string]any{}, + Data: payloads.IdentityUnlinked{UserID: userID, ProviderID: provider.ID}, ActorType: "scim", ActorID: provider.ID, }); err != nil { @@ -262,7 +263,7 @@ func (h *Handler) deleteUser(w http.ResponseWriter, r *http.Request) { StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserDeleted), - Data: map[string]any{}, + Data: payloads.UserDeleted{}, ActorType: "scim", ActorID: provider.ID, }) diff --git a/internal/scim/users_create.go b/internal/scim/users_create.go index d7f326eb..82dd79c5 100644 --- a/internal/scim/users_create.go +++ b/internal/scim/users_create.go @@ -18,6 +18,7 @@ import ( "net/http" "github.com/manchtools/power-manage/server/internal/eventtypes" + "github.com/manchtools/power-manage/server/internal/eventtypes/payloads" "github.com/manchtools/power-manage/server/internal/store" db "github.com/manchtools/power-manage/server/internal/store/generated" ) @@ -122,12 +123,12 @@ func (h *Handler) createUser(w http.ResponseWriter, r *http.Request) { StreamType: "identity_provider", StreamID: linkID, EventType: string(eventtypes.IdentityLinked), - Data: map[string]any{ - "user_id": existingUser.ID, - "provider_id": provider.ID, - "external_id": externalID, - "external_email": email, - "external_name": formatExternalName(scimUser.Name), + Data: payloads.IdentityLinked{ + UserID: existingUser.ID, + ProviderID: provider.ID, + ExternalID: externalID, + ExternalEmail: email, + ExternalName: formatExternalName(scimUser.Name), }, ActorType: "scim", ActorID: provider.ID, @@ -178,14 +179,16 @@ func (h *Handler) createUser(w http.ResponseWriter, r *http.Request) { StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserCreatedWithRoles), - Data: map[string]any{ - "email": email, - "display_name": formatExternalName(scimUser.Name), - "given_name": safeNameField(scimUser.Name, "given"), - "family_name": safeNameField(scimUser.Name, "family"), - "linux_username": linuxUsername, - "linux_uid": linuxUID, - "role_ids": roleIDs, + // Pointers always set (possibly to ""): SCIM asserts every + // field, mirroring the legacy always-present map keys. + Data: payloads.UserCreatedWithRoles{ + Email: &email, + DisplayName: ptr(formatExternalName(scimUser.Name)), + GivenName: ptr(safeNameField(scimUser.Name, "given")), + FamilyName: ptr(safeNameField(scimUser.Name, "family")), + LinuxUsername: &linuxUsername, + LinuxUID: &linuxUID, + RoleIDs: roleIDs, }, ActorType: "scim", ActorID: provider.ID, @@ -202,12 +205,12 @@ func (h *Handler) createUser(w http.ResponseWriter, r *http.Request) { StreamType: "identity_provider", StreamID: linkID, EventType: string(eventtypes.IdentityLinked), - Data: map[string]any{ - "user_id": userID, - "provider_id": provider.ID, - "external_id": externalID, - "external_email": email, - "external_name": formatExternalName(scimUser.Name), + Data: payloads.IdentityLinked{ + UserID: userID, + ProviderID: provider.ID, + ExternalID: externalID, + ExternalEmail: email, + ExternalName: formatExternalName(scimUser.Name), }, ActorType: "scim", ActorID: provider.ID, @@ -225,7 +228,7 @@ func (h *Handler) createUser(w http.ResponseWriter, r *http.Request) { StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserProvisioningSettingsUpdated), - Data: map[string]any{"user_provisioning_enabled": true}, + Data: payloads.UserProvisioningSettingsUpdated{UserProvisioningEnabled: ptr(true)}, ActorType: "system", ActorID: "scim", }); err != nil { @@ -237,10 +240,10 @@ func (h *Handler) createUser(w http.ResponseWriter, r *http.Request) { StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserSshSettingsUpdated), - Data: map[string]any{ - "ssh_access_enabled": true, - "ssh_allow_pubkey": true, - "ssh_allow_password": false, + Data: payloads.UserSshSettingsUpdated{ + SshAccessEnabled: ptr(true), + SshAllowPubkey: ptr(true), + SshAllowPassword: ptr(false), }, ActorType: "system", ActorID: "scim", diff --git a/internal/scim/users_helpers.go b/internal/scim/users_helpers.go index e9627685..70b371a5 100644 --- a/internal/scim/users_helpers.go +++ b/internal/scim/users_helpers.go @@ -18,6 +18,7 @@ import ( "github.com/oklog/ulid/v2" "github.com/manchtools/power-manage/server/internal/eventtypes" + "github.com/manchtools/power-manage/server/internal/eventtypes/payloads" "github.com/manchtools/power-manage/server/internal/store" db "github.com/manchtools/power-manage/server/internal/store/generated" ) @@ -29,6 +30,11 @@ func newULID() string { return ulid.MustNew(ulid.Timestamp(time.Now()), entropy).String() } +// ptr lifts a value into a pointer for the payloads structs whose +// pointer fields mean "explicitly present on the wire" — SCIM is the +// source of truth and always emits the field, even when empty. +func ptr[T any](v T) *T { return &v } + // syncUserFromSCIM syncs email, active status, profile, and identity link data from SCIM. // SCIM is treated as the source of truth — any differences are overwritten. func (h *Handler) syncUserFromSCIM(ctx context.Context, provider store.IdentityProvider, userID, email string, active *bool, name *SCIMName) { @@ -44,7 +50,7 @@ func (h *Handler) syncUserFromSCIM(ctx context.Context, provider store.IdentityP StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserEmailChanged), - Data: map[string]any{"email": email}, + Data: payloads.UserEmailChanged{Email: &email}, ActorType: "scim", ActorID: provider.ID, }) @@ -57,7 +63,7 @@ func (h *Handler) syncUserFromSCIM(ctx context.Context, provider store.IdentityP StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserDisabled), - Data: map[string]any{}, + Data: payloads.UserDisabled{}, ActorType: "scim", ActorID: provider.ID, }) @@ -66,7 +72,7 @@ func (h *Handler) syncUserFromSCIM(ctx context.Context, provider store.IdentityP StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserEnabled), - Data: map[string]any{}, + Data: payloads.UserEnabled{}, ActorType: "scim", ActorID: provider.ID, }) @@ -81,10 +87,13 @@ func (h *Handler) syncUserFromSCIM(ctx context.Context, provider store.IdentityP StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserProfileUpdated), - Data: map[string]any{ - "display_name": newDisplayName, - "given_name": newGivenName, - "family_name": newFamilyName, + // Pointers always set: SCIM is the source of truth, so an + // empty field is an explicit "" on the wire (overwrite, + // matching the legacy map emit) — never nil (preserve). + Data: payloads.UserProfileUpdated{ + DisplayName: &newDisplayName, + GivenName: &newGivenName, + FamilyName: &newFamilyName, }, ActorType: "scim", ActorID: provider.ID, @@ -111,11 +120,12 @@ func (h *Handler) syncIdentityLink(ctx context.Context, provider store.IdentityP StreamType: "identity_provider", StreamID: link.ID, EventType: string(eventtypes.IdentityLinkLoginUpdated), - Data: map[string]any{ - "provider_id": provider.ID, - "external_id": link.ExternalID, - "external_email": email, - "external_name": formatExternalName(name), + Data: payloads.IdentityLinkLoginUpdated{ + UserID: userID, + ProviderID: provider.ID, + ExternalID: link.ExternalID, + ExternalEmail: email, + ExternalName: formatExternalName(name), }, ActorType: "scim", ActorID: provider.ID, diff --git a/internal/scim/users_link_test.go b/internal/scim/users_link_test.go index 6deb9ef9..e652b296 100644 --- a/internal/scim/users_link_test.go +++ b/internal/scim/users_link_test.go @@ -2,6 +2,7 @@ package scim_test import ( "context" + "encoding/json" "net/http" "net/http/httptest" "testing" @@ -127,3 +128,36 @@ func TestCreateUser_AutoLinkByEmail_RequiresVerifiedSignal(t *testing.T) { assert.Equal(t, http.StatusCreated, w.Code, "no matching local account → create new: %s", w.Body.String()) }) } + +// TestReplaceUser_LoginUpdatedCarriesUserID pins the #507 / spec 19 +// contract on the SCIM sync path: the IdentityLinkLoginUpdated event +// carries PII (external_email/external_name), so its payload must name +// the owning user — the crypto-shred layer resolves the DEK owner from +// data->>'user_id'. +func TestReplaceUser_LoginUpdatedCarriesUserID(t *testing.T) { + env := setupSCIM(t) + + email := "louu-" + testutil.NewID()[:6] + "@corp.com" + w := postSCIMUser(t, env, email) + require.Equal(t, http.StatusCreated, w.Code, "%s", w.Body.String()) + var created map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &created)) + userID := created["id"].(string) + + // PUT triggers syncIdentityLink → IdentityLinkLoginUpdated. + put := scimReq(t, env, env.slug, env.token, http.MethodPut, "/Users/"+userID, map[string]any{ + "schemas": []string{"urn:ietf:params:scim:schemas:core:2.0:User"}, + "userName": email, + "name": map[string]any{"givenName": "Lou", "familyName": "Update"}, + "active": true, + }) + require.Equal(t, http.StatusOK, put.Code, "%s", put.Body.String()) + + var gotUserID string + require.NoError(t, env.st.TestingPool().QueryRow(context.Background(), + `SELECT data->>'user_id' FROM events + WHERE event_type = 'IdentityLinkLoginUpdated' + ORDER BY sequence_num DESC LIMIT 1`).Scan(&gotUserID)) + assert.Equal(t, userID, gotUserID, + "IdentityLinkLoginUpdated must carry the DEK-owner user_id") +} diff --git a/internal/scim/users_mutate.go b/internal/scim/users_mutate.go index 73bc7fd5..a7722b1e 100644 --- a/internal/scim/users_mutate.go +++ b/internal/scim/users_mutate.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/manchtools/power-manage/server/internal/eventtypes" + "github.com/manchtools/power-manage/server/internal/eventtypes/payloads" "github.com/manchtools/power-manage/server/internal/store" db "github.com/manchtools/power-manage/server/internal/store/generated" ) @@ -68,11 +69,9 @@ func (h *Handler) replaceUser(w http.ResponseWriter, r *http.Request) { StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserEmailChanged), - Data: map[string]any{ - "email": newEmail, - }, - ActorType: "scim", - ActorID: provider.ID, + Data: payloads.UserEmailChanged{Email: &newEmail}, + ActorType: "scim", + ActorID: provider.ID, }) if err != nil { h.logger.Error("failed to update user email", "error", err) @@ -87,7 +86,7 @@ func (h *Handler) replaceUser(w http.ResponseWriter, r *http.Request) { StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserDisabled), - Data: map[string]any{}, + Data: payloads.UserDisabled{}, ActorType: "scim", ActorID: provider.ID, }); err != nil { @@ -100,7 +99,7 @@ func (h *Handler) replaceUser(w http.ResponseWriter, r *http.Request) { StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserEnabled), - Data: map[string]any{}, + Data: payloads.UserEnabled{}, ActorType: "scim", ActorID: provider.ID, }); err != nil { @@ -119,10 +118,13 @@ func (h *Handler) replaceUser(w http.ResponseWriter, r *http.Request) { StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserProfileUpdated), - Data: map[string]any{ - "display_name": newDisplayName, - "given_name": newGivenName, - "family_name": newFamilyName, + // Pointers always set: SCIM is the source of truth, so an + // empty field is an explicit "" on the wire (overwrite, + // matching the legacy map emit) — never nil (preserve). + Data: payloads.UserProfileUpdated{ + DisplayName: &newDisplayName, + GivenName: &newGivenName, + FamilyName: &newFamilyName, }, ActorType: "scim", ActorID: provider.ID, @@ -268,7 +270,7 @@ func (h *Handler) handleUserPatchReplace(ctx context.Context, provider store.Ide StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserDisabled), - Data: map[string]any{}, + Data: payloads.UserDisabled{}, ActorType: "scim", ActorID: provider.ID, }) @@ -277,7 +279,7 @@ func (h *Handler) handleUserPatchReplace(ctx context.Context, provider store.Ide StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserEnabled), - Data: map[string]any{}, + Data: payloads.UserEnabled{}, ActorType: "scim", ActorID: provider.ID, }) @@ -293,11 +295,9 @@ func (h *Handler) handleUserPatchReplace(ctx context.Context, provider store.Ide StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserEmailChanged), - Data: map[string]any{ - "email": email, - }, - ActorType: "scim", - ActorID: provider.ID, + Data: payloads.UserEmailChanged{Email: &email}, + ActorType: "scim", + ActorID: provider.ID, }) } @@ -320,11 +320,9 @@ func (h *Handler) handleUserPatchReplace(ctx context.Context, provider store.Ide StreamType: "user", StreamID: userID, EventType: string(eventtypes.UserEmailChanged), - Data: map[string]any{ - "email": email, - }, - ActorType: "scim", - ActorID: provider.ID, + Data: payloads.UserEmailChanged{Email: &email}, + ActorType: "scim", + ActorID: provider.ID, }) } @@ -334,17 +332,19 @@ func (h *Handler) handleUserPatchReplace(ctx context.Context, provider store.Ide if !ok { return fmt.Errorf("invalid name value") } - data := map[string]any{} + // Partial update: only the supplied fields get pointers, so + // omitted keys stay off the wire (projector preserves them). + var data payloads.UserProfileUpdated if gn, ok := nameMap["givenName"].(string); ok { - data["given_name"] = gn + data.GivenName = &gn } if fn, ok := nameMap["familyName"].(string); ok { - data["family_name"] = fn + data.FamilyName = &fn } if fm, ok := nameMap["formatted"].(string); ok { - data["display_name"] = fm + data.DisplayName = &fm } - if len(data) > 0 { + if data != (payloads.UserProfileUpdated{}) { return h.store.AppendEvent(ctx, store.Event{ StreamType: "user", StreamID: userID, From d57ea2f91a9c2b909e50fafcb99955f4c1d6c79e Mon Sep 17 00:00:00 2001 From: PaulDotterer Date: Sat, 4 Jul 2026 17:04:06 +0200 Subject: [PATCH 2/3] fix(scim): explicit empty name object clears the profile (manchtools/power-manage-server#507) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local CR finding on the typed-payload conversion: the profile-sync gate skipped the UserProfileUpdated emit whenever every computed name value was empty, so a SCIM PUT/sync that explicitly cleared the name could never propagate — contradicting the pointer semantics (present-but-empty = overwrite). Gate on the name object being asserted instead: omitted name preserves the profile, explicit empty name clears it. Regression test pins both directions. --- internal/scim/users_helpers.go | 6 +++- internal/scim/users_link_test.go | 53 ++++++++++++++++++++++++++++++++ internal/scim/users_mutate.go | 6 +++- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/internal/scim/users_helpers.go b/internal/scim/users_helpers.go index 70b371a5..ac1f9d9b 100644 --- a/internal/scim/users_helpers.go +++ b/internal/scim/users_helpers.go @@ -82,7 +82,11 @@ func (h *Handler) syncUserFromSCIM(ctx context.Context, provider store.IdentityP newDisplayName := formatExternalName(name) newGivenName := safeNameField(name, "given") newFamilyName := safeNameField(name, "family") - if newDisplayName != "" || newGivenName != "" || newFamilyName != "" { + // Gate on "name object asserted" rather than "any value non-empty": + // SCIM is the source of truth, so an explicitly empty name object + // clears the profile ("" overwrite), while an omitted one preserves + // it. The old any-non-empty gate made an explicit clear impossible. + if name != nil { h.appendEvent(ctx, store.Event{ StreamType: "user", StreamID: userID, diff --git a/internal/scim/users_link_test.go b/internal/scim/users_link_test.go index e652b296..bb1ce43d 100644 --- a/internal/scim/users_link_test.go +++ b/internal/scim/users_link_test.go @@ -161,3 +161,56 @@ func TestReplaceUser_LoginUpdatedCarriesUserID(t *testing.T) { assert.Equal(t, userID, gotUserID, "IdentityLinkLoginUpdated must carry the DEK-owner user_id") } + +// TestReplaceUser_ExplicitEmptyNameClearsProfile pins the SCIM +// source-of-truth contract on profile sync: a PUT that carries an +// explicit empty name object CLEARS the profile fields (pointer +// semantics: present-but-"" = overwrite), while a PUT that omits the +// name object entirely PRESERVES them (absent = not asserted). The +// old gate skipped the event whenever every computed value was empty, +// making an explicit clear impossible (local CR finding on #507). +func TestReplaceUser_ExplicitEmptyNameClearsProfile(t *testing.T) { + env := setupSCIM(t) + + email := "clear-" + testutil.NewID()[:6] + "@corp.com" + w := scimReq(t, env, env.slug, env.token, http.MethodPost, "/Users", map[string]any{ + "schemas": []string{"urn:ietf:params:scim:schemas:core:2.0:User"}, + "userName": email, + "externalId": "ext-" + testutil.NewID(), + "active": true, + "name": map[string]any{"givenName": "Lou", "familyName": "Update"}, + }) + require.Equal(t, http.StatusCreated, w.Code, "%s", w.Body.String()) + var created map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &created)) + userID := created["id"].(string) + + seeded, err := env.st.Queries().GetUserByID(context.Background(), userID) + require.NoError(t, err) + require.Equal(t, "Lou", seeded.GivenName, "seed sanity: given_name populated") + + // PUT with NO name object — profile must be preserved. + put := scimReq(t, env, env.slug, env.token, http.MethodPut, "/Users/"+userID, map[string]any{ + "schemas": []string{"urn:ietf:params:scim:schemas:core:2.0:User"}, + "userName": email, + "active": true, + }) + require.Equal(t, http.StatusOK, put.Code, "%s", put.Body.String()) + kept, err := env.st.Queries().GetUserByID(context.Background(), userID) + require.NoError(t, err) + assert.Equal(t, "Lou", kept.GivenName, "omitted name object must not touch the profile") + + // PUT with an explicit EMPTY name object — profile must clear. + put = scimReq(t, env, env.slug, env.token, http.MethodPut, "/Users/"+userID, map[string]any{ + "schemas": []string{"urn:ietf:params:scim:schemas:core:2.0:User"}, + "userName": email, + "active": true, + "name": map[string]any{}, + }) + require.Equal(t, http.StatusOK, put.Code, "%s", put.Body.String()) + cleared, err := env.st.Queries().GetUserByID(context.Background(), userID) + require.NoError(t, err) + assert.Empty(t, cleared.GivenName, "explicit empty name object must clear given_name") + assert.Empty(t, cleared.FamilyName, "explicit empty name object must clear family_name") + assert.Empty(t, cleared.DisplayName, "explicit empty name object must clear display_name") +} diff --git a/internal/scim/users_mutate.go b/internal/scim/users_mutate.go index a7722b1e..3e4fd41c 100644 --- a/internal/scim/users_mutate.go +++ b/internal/scim/users_mutate.go @@ -113,7 +113,11 @@ func (h *Handler) replaceUser(w http.ResponseWriter, r *http.Request) { newDisplayName := formatExternalName(scimUser.Name) newGivenName := safeNameField(scimUser.Name, "given") newFamilyName := safeNameField(scimUser.Name, "family") - if newDisplayName != "" || newGivenName != "" || newFamilyName != "" { + // Gate on "name object asserted" rather than "any value non-empty": + // SCIM is the source of truth, so an explicitly empty name object + // clears the profile ("" overwrite), while an omitted one preserves + // it. The old any-non-empty gate made an explicit clear impossible. + if scimUser.Name != nil { if err := h.store.AppendEvent(ctx, store.Event{ StreamType: "user", StreamID: userID, From 5d350b4035f1d9ad62f30dd484e33ba21c2ff785 Mon Sep 17 00:00:00 2001 From: PaulDotterer Date: Sat, 4 Jul 2026 17:15:58 +0200 Subject: [PATCH 3/3] =?UTF-8?q?scim:=20fail=20hard=20on=20event-append=20e?= =?UTF-8?q?rrors=20=E2=80=94=20unmap=20guard,=20profile=20update=20500=20(?= =?UTF-8?q?#507)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/scim/groups_helpers.go | 10 +++++++--- internal/scim/users_mutate.go | 7 ++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/internal/scim/groups_helpers.go b/internal/scim/groups_helpers.go index 6c643da0..524223fd 100644 --- a/internal/scim/groups_helpers.go +++ b/internal/scim/groups_helpers.go @@ -174,8 +174,10 @@ func (h *Handler) restoreOrphanedGroup(ctx context.Context, providerID string, m return m, fmt.Errorf("create user group: %w", err) } - // Remove old mapping - h.appendEvent(ctx, store.Event{ + // Remove old mapping. Fail hard: if the unmap append fails, creating + // the replacement below would leave the stale mapping active alongside + // the new one — two mappings for one SCIM group id. + if err := h.store.AppendEvent(ctx, store.Event{ StreamType: "scim_group_mapping", StreamID: m.ID, EventType: string(eventtypes.SCIMGroupUnmapped), @@ -185,7 +187,9 @@ func (h *Handler) restoreOrphanedGroup(ctx context.Context, providerID string, m }, ActorType: "scim", ActorID: providerID, - }) + }); err != nil { + return m, fmt.Errorf("unmap old SCIM group mapping: %w", err) + } // Create new mapping pointing to the new user group newMappingID := newULID() diff --git a/internal/scim/users_mutate.go b/internal/scim/users_mutate.go index 3e4fd41c..f604a71f 100644 --- a/internal/scim/users_mutate.go +++ b/internal/scim/users_mutate.go @@ -133,7 +133,12 @@ func (h *Handler) replaceUser(w http.ResponseWriter, r *http.Request) { ActorType: "scim", ActorID: provider.ID, }); err != nil { - h.logger.Warn("failed to update user profile via SCIM", "error", err) + // Fail the request like the email/status branches above: a + // 200 after a dropped source-of-truth profile update would + // silently desync PM from the IdP. + h.logger.Error("failed to update user profile via SCIM", "error", err) + writeError(w, http.StatusInternalServerError, "failed to update user profile") + return } }