From 2989e338b25c18ab099839f0749a94280d6f97b7 Mon Sep 17 00:00:00 2001 From: Piotr Janus Date: Fri, 18 Sep 2026 15:57:36 +0200 Subject: [PATCH 1/5] feat: add keyrotation package with strict decode and cron validation Introduces the cac-owned schema for workspaces//key_rotation.yaml ({sig,enc} -> {enabled, cron, starting_from}) and the helpers the rest of the pipeline uses to carry it in a patch under the key_rotation key: Pop/Get (strict decode, unknown fields rejected), Validate (cron required for every present use and parsed with gorhill/cronexpr, the same library and version the server uses), and conversions to and from the admin AutomaticKeyRotation model. The read-only scheduled_at and the never-echoed starting_from are dropped when converting from the server. Adds utils.AsPatch for the recurring any -> patch map conversion. --- go.mod | 1 + go.sum | 2 + internal/cac/keyrotation/keyrotation.go | 167 +++++++++ internal/cac/keyrotation/keyrotation_test.go | 339 +++++++++++++++++++ internal/cac/utils/model.go | 13 + internal/cac/utils/model_test.go | 43 +++ 6 files changed, 565 insertions(+) create mode 100644 internal/cac/keyrotation/keyrotation.go create mode 100644 internal/cac/keyrotation/keyrotation_test.go diff --git a/go.mod b/go.mod index 96b5f60..cd7c794 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/go-openapi/strfmt v0.24.0 github.com/goccy/go-yaml v1.12.0 github.com/google/go-cmp v0.7.0 + github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75 github.com/imdario/mergo v0.3.16 github.com/mitchellh/mapstructure v1.5.0 github.com/pkg/errors v0.9.1 diff --git a/go.sum b/go.sum index a395216..7a7a870 100644 --- a/go.sum +++ b/go.sum @@ -92,6 +92,8 @@ github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75 h1:f0n1xnMSmBLzVfsMMvriDyA75NB/oBgILX2GcHXIQzY= +github.com/gorhill/cronexpr v0.0.0-20180427100037-88b0669f7d75/go.mod h1:g2644b03hfBX9Ov0ZBDgXXens4rxSxmqFBbhvKv2yVA= github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/huandu/xstrings v1.3.3/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE= diff --git a/internal/cac/keyrotation/keyrotation.go b/internal/cac/keyrotation/keyrotation.go new file mode 100644 index 0000000..f7b2204 --- /dev/null +++ b/internal/cac/keyrotation/keyrotation.go @@ -0,0 +1,167 @@ +// Package keyrotation holds the automatic key rotation configuration that rides in a workspace +// patch under the key_rotation key. It is not part of models.TreeServer, so it is popped out of +// the patch before every strict decode of the tree models and handled explicitly. +package keyrotation + +import ( + "maps" + + admodels "github.com/cloudentity/acp-client-go/clients/admin/models" + "github.com/cloudentity/acp-client-go/clients/hub/models" + "github.com/cloudentity/cac/internal/cac/utils" + "github.com/go-openapi/strfmt" + "github.com/gorhill/cronexpr" + "github.com/pkg/errors" +) + +// Key is the patch key (and the workspace file name) the configuration lives under. +const Key = "key_rotation" + +// UseSig and UseEnc are the only key uses ACP supports. +const ( + UseSig = "sig" + UseEnc = "enc" +) + +// Rotation is the on-disk and in-patch schema, owned by cac rather than reusing +// admodels.AutomaticKeyRotation: that model carries a read-only scheduled_at field users must not +// write, and its non-pointer date-times would serialize as 0001-01-01 whenever they are unset. +type Rotation struct { + Enabled bool `json:"enabled"` + Cron string `json:"cron"` + StartingFrom *strfmt.DateTime `json:"starting_from,omitempty"` +} + +type Config struct { + Sig *Rotation `json:"sig,omitempty"` + Enc *Rotation `json:"enc,omitempty"` +} + +// UseRotation pairs a key use with its rotation configuration. +type UseRotation struct { + Use string + Rotation *Rotation +} + +// Pop removes Key from patch and strict-decodes it. It returns (nil, nil) when the key is absent +// and does not validate the decoded configuration. +func Pop(patch models.Rfc7396PatchOperation) (*Config, error) { + var ( + raw any + ok bool + ) + + if raw, ok = patch[Key]; !ok { + return nil, nil + } + + delete(patch, Key) + + return decode(raw) +} + +// Get is the non-mutating variant of Pop. +func Get(patch models.Rfc7396PatchOperation) (*Config, error) { + var ( + raw any + ok bool + ) + + if raw, ok = patch[Key]; !ok { + return nil, nil + } + + return decode(raw) +} + +func decode(raw any) (*Config, error) { + var ( + sub models.Rfc7396PatchOperation + config *Config + ok bool + err error + ) + + if sub, ok = utils.AsPatch(raw); !ok { + return nil, errors.Errorf("failed to parse %s: expected an object, got %T", Key, raw) + } + + // FromPatchToModel cleans the map it is given, so decode a copy and leave the caller's alone. + patch := make(models.Rfc7396PatchOperation, len(sub)) + maps.Copy(patch, sub) + + if config, err = utils.FromPatchToModel[Config](patch); err != nil { + return nil, errors.Wrapf(err, "failed to parse %s", Key) + } + + // a configuration with no use configures nothing, so it is reported as absent and nothing is + // written or pushed for it + if config.Sig == nil && config.Enc == nil { + return nil, nil + } + + return config, nil +} + +// Uses returns the configured uses, sig first, skipping the ones that are not set. +func (c *Config) Uses() []UseRotation { + if c == nil { + return nil + } + + var out []UseRotation + + if c.Sig != nil { + out = append(out, UseRotation{Use: UseSig, Rotation: c.Sig}) + } + + if c.Enc != nil { + out = append(out, UseRotation{Use: UseEnc, Rotation: c.Enc}) + } + + return out +} + +// Validate checks that every configured use has a cron ACP will accept. It uses the same parser and +// version ACP does, so what passes here passes there. +func (c *Config) Validate() error { + for _, use := range c.Uses() { + if use.Rotation.Cron == "" { + return errors.Errorf("%s: cron is required for %s, ACP requires a valid cron even when enabled is false", Key, use.Use) + } + + if _, err := cronexpr.Parse(use.Rotation.Cron); err != nil { + return errors.Wrapf(err, "%s: invalid cron for %s", Key, use.Use) + } + } + + return nil +} + +// ToModel converts a Rotation to the API model. ScheduledAt is left zero: it is read-only. +func (r *Rotation) ToModel() *admodels.AutomaticKeyRotation { + out := &admodels.AutomaticKeyRotation{ + Cron: r.Cron, + Enabled: r.Enabled, + } + + if r.StartingFrom != nil { + out.StartingFrom = *r.StartingFrom + } + + return out +} + +// FromModel builds a Rotation out of a GET payload. It returns nil when the use was never +// configured, which ACP reports as an empty cron rather than a 404. StartingFrom and ScheduledAt +// are dropped on purpose: the server never echoes starting_from back, and scheduled_at is read-only. +func FromModel(m *admodels.AutomaticKeyRotation) *Rotation { + if m == nil || m.Cron == "" { + return nil + } + + return &Rotation{ + Enabled: m.Enabled, + Cron: m.Cron, + } +} diff --git a/internal/cac/keyrotation/keyrotation_test.go b/internal/cac/keyrotation/keyrotation_test.go new file mode 100644 index 0000000..d2f5565 --- /dev/null +++ b/internal/cac/keyrotation/keyrotation_test.go @@ -0,0 +1,339 @@ +package keyrotation_test + +import ( + "testing" + "time" + + admodels "github.com/cloudentity/acp-client-go/clients/admin/models" + "github.com/cloudentity/acp-client-go/clients/hub/models" + "github.com/cloudentity/cac/internal/cac/keyrotation" + "github.com/cloudentity/cac/internal/cac/utils" + "github.com/go-openapi/strfmt" + "github.com/stretchr/testify/require" +) + +func startingFrom(t *testing.T, value string) *strfmt.DateTime { + t.Helper() + + parsed, err := time.Parse(time.RFC3339, value) + require.NoError(t, err) + + out := strfmt.DateTime(parsed) + + return &out +} + +func TestPop(t *testing.T) { + t.Run("absent", func(t *testing.T) { + patch := models.Rfc7396PatchOperation{"name": "workspace1"} + + config, err := keyrotation.Pop(patch) + require.NoError(t, err) + require.Nil(t, config) + require.Equal(t, models.Rfc7396PatchOperation{"name": "workspace1"}, patch) + }) + + t.Run("present", func(t *testing.T) { + patch := models.Rfc7396PatchOperation{ + "name": "workspace1", + keyrotation.Key: map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + "starting_from": "2026-10-01T00:00:00.000Z", + }, + "enc": map[string]any{ + "enabled": false, + "cron": "@monthly", + }, + }, + } + + config, err := keyrotation.Pop(patch) + require.NoError(t, err) + require.Equal(t, &keyrotation.Config{ + Sig: &keyrotation.Rotation{ + Enabled: true, + Cron: "0 0 1 * *", + StartingFrom: startingFrom(t, "2026-10-01T00:00:00Z"), + }, + Enc: &keyrotation.Rotation{ + Enabled: false, + Cron: "@monthly", + }, + }, config) + require.Equal(t, models.Rfc7396PatchOperation{"name": "workspace1"}, patch) + }) + + t.Run("patch operation value", func(t *testing.T) { + patch := models.Rfc7396PatchOperation{ + keyrotation.Key: models.Rfc7396PatchOperation{ + "sig": map[string]any{"enabled": true, "cron": "@daily"}, + }, + } + + config, err := keyrotation.Pop(patch) + require.NoError(t, err) + require.Equal(t, &keyrotation.Config{ + Sig: &keyrotation.Rotation{Enabled: true, Cron: "@daily"}, + }, config) + }) + + t.Run("not an object", func(t *testing.T) { + patch := models.Rfc7396PatchOperation{keyrotation.Key: "@daily"} + + _, err := keyrotation.Pop(patch) + require.ErrorContains(t, err, keyrotation.Key) + }) +} + +func TestGetDoesNotMutatePatch(t *testing.T) { + sig := map[string]any{"enabled": true, "cron": "@monthly"} + patch := models.Rfc7396PatchOperation{ + keyrotation.Key: map[string]any{"sig": sig}, + } + + config, err := keyrotation.Get(patch) + require.NoError(t, err) + require.Equal(t, &keyrotation.Config{ + Sig: &keyrotation.Rotation{Enabled: true, Cron: "@monthly"}, + }, config) + require.Equal(t, models.Rfc7396PatchOperation{ + keyrotation.Key: map[string]any{ + "sig": map[string]any{"enabled": true, "cron": "@monthly"}, + }, + }, patch) + + t.Run("absent", func(t *testing.T) { + empty := models.Rfc7396PatchOperation{} + + config, err := keyrotation.Get(empty) + require.NoError(t, err) + require.Nil(t, config) + }) +} + +func TestStrictDecoding(t *testing.T) { + tcs := []struct { + name string + value map[string]any + }{ + { + name: "unknown field inside a use", + value: map[string]any{ + "sig": map[string]any{"enabled": true, "cron": "@monthly", "rotate": true}, + }, + }, + { + name: "unknown use", + value: map[string]any{ + "sgi": map[string]any{"enabled": true, "cron": "@monthly"}, + }, + }, + { + name: "read only scheduled_at", + value: map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "@monthly", + "scheduled_at": "2026-10-01T00:00:00.000Z", + }, + }, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + patch := models.Rfc7396PatchOperation{keyrotation.Key: tc.value} + + _, err := keyrotation.Pop(patch) + require.Error(t, err) + }) + } +} + +func TestUses(t *testing.T) { + var config *keyrotation.Config + require.Empty(t, config.Uses()) + + require.Empty(t, (&keyrotation.Config{}).Uses()) + + full := &keyrotation.Config{ + Sig: &keyrotation.Rotation{Cron: "@monthly"}, + Enc: &keyrotation.Rotation{Cron: "@daily"}, + } + require.Equal(t, []keyrotation.UseRotation{ + {Use: keyrotation.UseSig, Rotation: full.Sig}, + {Use: keyrotation.UseEnc, Rotation: full.Enc}, + }, full.Uses()) + + encOnly := &keyrotation.Config{Enc: &keyrotation.Rotation{Cron: "@daily"}} + require.Equal(t, []keyrotation.UseRotation{ + {Use: keyrotation.UseEnc, Rotation: encOnly.Enc}, + }, encOnly.Uses()) +} + +func TestValidate(t *testing.T) { + tcs := []struct { + name string + config *keyrotation.Config + errMsg string + }{ + { + name: "nil config", + config: nil, + }, + { + name: "no uses", + config: &keyrotation.Config{}, + }, + { + name: "missing cron", + config: &keyrotation.Config{Enc: &keyrotation.Rotation{Enabled: false}}, + errMsg: "enc", + }, + { + name: "garbage cron", + config: &keyrotation.Config{Sig: &keyrotation.Rotation{Enabled: true, Cron: "not a cron"}}, + errMsg: "sig", + }, + { + name: "every descriptor is not supported", + config: &keyrotation.Config{Sig: &keyrotation.Rotation{Enabled: true, Cron: "@every 5m"}}, + errMsg: "sig", + }, + { + name: "monthly descriptor", + config: &keyrotation.Config{Sig: &keyrotation.Rotation{Enabled: true, Cron: "@monthly"}}, + }, + { + name: "five fields", + config: &keyrotation.Config{Sig: &keyrotation.Rotation{Enabled: true, Cron: "0 0 1 * *"}}, + }, + { + name: "six fields with a year", + config: &keyrotation.Config{Sig: &keyrotation.Rotation{Enabled: true, Cron: "0 0 1 * * 2027"}}, + }, + { + name: "seven fields with seconds", + config: &keyrotation.Config{Sig: &keyrotation.Rotation{Enabled: true, Cron: "0 0 0 1 * * 2027"}}, + }, + { + name: "disabled with a valid cron", + config: &keyrotation.Config{Enc: &keyrotation.Rotation{Enabled: false, Cron: "0 0 1 * *"}}, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + err := tc.config.Validate() + + if tc.errMsg == "" { + require.NoError(t, err) + return + } + + require.ErrorContains(t, err, tc.errMsg) + }) + } +} + +func TestValidateMissingCronExplainsAcpRequirement(t *testing.T) { + config := &keyrotation.Config{Enc: &keyrotation.Rotation{Enabled: false}} + + err := config.Validate() + require.ErrorContains(t, err, "enc") + require.ErrorContains(t, err, "enabled") +} + +func TestToModel(t *testing.T) { + t.Run("starting from copied", func(t *testing.T) { + from := startingFrom(t, "2026-10-01T00:00:00Z") + rotation := &keyrotation.Rotation{Enabled: true, Cron: "@monthly", StartingFrom: from} + + require.Equal(t, &admodels.AutomaticKeyRotation{ + Cron: "@monthly", + Enabled: true, + StartingFrom: *from, + }, rotation.ToModel()) + }) + + t.Run("starting from unset leaves the zero time", func(t *testing.T) { + model := (&keyrotation.Rotation{Enabled: false, Cron: "@daily"}).ToModel() + + require.True(t, time.Time(model.StartingFrom).IsZero()) + require.True(t, time.Time(model.ScheduledAt).IsZero()) + }) +} + +func TestFromModel(t *testing.T) { + t.Run("nil", func(t *testing.T) { + require.Nil(t, keyrotation.FromModel(nil)) + }) + + t.Run("never configured", func(t *testing.T) { + require.Nil(t, keyrotation.FromModel(&admodels.AutomaticKeyRotation{Cron: ""})) + }) + + t.Run("drops server owned fields", func(t *testing.T) { + rotation := keyrotation.FromModel(&admodels.AutomaticKeyRotation{ + Cron: "0 0 1 * *", + Enabled: true, + ScheduledAt: *startingFrom(t, "2026-11-01T00:00:00Z"), + StartingFrom: *startingFrom(t, "2026-10-01T00:00:00Z"), + }) + + require.Equal(t, &keyrotation.Rotation{Enabled: true, Cron: "0 0 1 * *"}, rotation) + }) +} + +func TestToYamlOmitsUnsetStartingFrom(t *testing.T) { + config := &keyrotation.Config{ + Sig: &keyrotation.Rotation{Enabled: true, Cron: "0 0 1 * *"}, + } + + bts, err := utils.ToYaml(config) + require.NoError(t, err) + require.NotContains(t, string(bts), "starting_from") + require.NotContains(t, string(bts), "scheduled_at") + require.NotContains(t, string(bts), "enc") +} + +func TestPatchRoundTrip(t *testing.T) { + config := &keyrotation.Config{ + Sig: &keyrotation.Rotation{ + Enabled: true, + Cron: "0 0 1 * *", + StartingFrom: startingFrom(t, "2026-10-01T00:00:00Z"), + }, + Enc: &keyrotation.Rotation{Enabled: false, Cron: "@monthly"}, + } + + sub, err := utils.FromModelToPatch(config) + require.NoError(t, err) + + patch := models.Rfc7396PatchOperation{keyrotation.Key: sub} + + out, err := keyrotation.Pop(patch) + require.NoError(t, err) + require.Equal(t, config, out) + require.Empty(t, patch) +} + +func TestEmptyConfigIsAbsent(t *testing.T) { + // key_rotation: {} configures nothing, so it is treated as absent and no file is written for it + t.Run("pop", func(t *testing.T) { + patch := models.Rfc7396PatchOperation{"name": "workspace1", keyrotation.Key: map[string]any{}} + + config, err := keyrotation.Pop(patch) + require.NoError(t, err) + require.Nil(t, config) + require.Equal(t, models.Rfc7396PatchOperation{"name": "workspace1"}, patch) + }) + + t.Run("get", func(t *testing.T) { + config, err := keyrotation.Get(models.Rfc7396PatchOperation{keyrotation.Key: map[string]any{}}) + require.NoError(t, err) + require.Nil(t, config) + }) +} diff --git a/internal/cac/utils/model.go b/internal/cac/utils/model.go index 98fb849..023bdc8 100644 --- a/internal/cac/utils/model.go +++ b/internal/cac/utils/model.go @@ -66,6 +66,19 @@ func NormalizePatch(patch models.Rfc7396PatchOperation) (models.Rfc7396PatchOper return out, nil } +// AsPatch narrows a nested patch value to a patch of its own. A patch carries either shape +// depending on whether it was decoded from JSON or built in memory. +func AsPatch(v any) (models.Rfc7396PatchOperation, bool) { + switch value := v.(type) { + case models.Rfc7396PatchOperation: + return value, true + case map[string]any: + return value, true + default: + return nil, false + } +} + // CleanPatch cleans fields that are available in system model but not available in hub model func CleanPatch(patch models.Rfc7396PatchOperation) { delete(patch, "id") diff --git a/internal/cac/utils/model_test.go b/internal/cac/utils/model_test.go index 9604386..b66c010 100644 --- a/internal/cac/utils/model_test.go +++ b/internal/cac/utils/model_test.go @@ -155,3 +155,46 @@ func TestFilterPatch(t *testing.T) { }) } } + +func TestAsPatch(t *testing.T) { + tcs := []struct { + name string + value any + expected models.Rfc7396PatchOperation + ok bool + }{ + { + name: "patch operation", + value: models.Rfc7396PatchOperation{"name": "workspace1"}, + expected: models.Rfc7396PatchOperation{"name": "workspace1"}, + ok: true, + }, + { + name: "plain map", + value: map[string]any{"name": "workspace1"}, + expected: models.Rfc7396PatchOperation{"name": "workspace1"}, + ok: true, + }, + { + name: "string", + value: "workspace1", + }, + { + name: "nil", + value: nil, + }, + { + name: "map of another type", + value: map[string]string{"name": "workspace1"}, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + actual, ok := utils.AsPatch(tc.value) + + require.Equal(t, tc.ok, ok) + require.Equal(t, tc.expected, actual) + }) + } +} From c63abaa906342f86fbf4c195bc4094edfeb8549b Mon Sep 17 00:00:00 2001 From: Piotr Janus Date: Fri, 18 Sep 2026 15:57:36 +0200 Subject: [PATCH 2/5] feat(storage): store workspace key rotation in key_rotation.yaml Key rotation is not part of TreeServer, so ServerStorage pops it from the patch before the strict model conversion and writes it to its own file, and reads it back under the key_rotation key so filters, diff and merged sources see it like any other workspace resource. TenantStorage does the same per workspace. The file is optional on read. --- internal/cac/storage/dry_test.go | 67 ++++++++++ internal/cac/storage/server_storage.go | 15 +++ internal/cac/storage/server_storage_test.go | 137 ++++++++++++++++++++ internal/cac/storage/tenant_storage.go | 56 +++++++- internal/cac/storage/tenant_storage_test.go | 134 ++++++++++++++++++- 5 files changed, 401 insertions(+), 8 deletions(-) create mode 100644 internal/cac/storage/dry_test.go diff --git a/internal/cac/storage/dry_test.go b/internal/cac/storage/dry_test.go new file mode 100644 index 0000000..1e936a6 --- /dev/null +++ b/internal/cac/storage/dry_test.go @@ -0,0 +1,67 @@ +package storage_test + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/cloudentity/acp-client-go/clients/hub/models" + "github.com/cloudentity/cac/internal/cac/api" + "github.com/cloudentity/cac/internal/cac/keyrotation" + "github.com/cloudentity/cac/internal/cac/logging" + "github.com/cloudentity/cac/internal/cac/storage" + "github.com/stretchr/testify/require" +) + +// keyRotationPatch is what push hands the dry storage: key_rotation rides in the patch next to the +// workspace configuration. +func keyRotationPatch() models.Rfc7396PatchOperation { + return models.Rfc7396PatchOperation{ + "name": "demo workspace", + keyrotation.Key: map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + }, + }, + } +} + +func TestDryStorageKeyRotation(t *testing.T) { + require.NoError(t, logging.InitLogging(&logging.Configuration{Level: "debug"})) + + t.Run("file", func(t *testing.T) { + out := filepath.Join(t.TempDir(), "out.yaml") + + dry, err := storage.InitDryStorage(out, storage.InitServerStorage) + require.NoError(t, err) + + require.NoError(t, dry.Write(context.Background(), keyRotationPatch(), api.WithWorkspace("demo"))) + + bts, err := os.ReadFile(out) + require.NoError(t, err) + + require.YAMLEq(t, `name: demo workspace +key_rotation: + sig: + enabled: true + cron: "0 0 1 * *"`, string(bts)) + }) + + t.Run("directory", func(t *testing.T) { + out := t.TempDir() + + dry, err := storage.InitDryStorage(out, storage.InitServerStorage) + require.NoError(t, err) + + require.NoError(t, dry.Write(context.Background(), keyRotationPatch(), api.WithWorkspace("demo"))) + + bts, err := os.ReadFile(filepath.Join(out, "workspaces", "demo", "key_rotation.yaml")) + require.NoError(t, err) + + require.YAMLEq(t, `sig: + enabled: true + cron: "0 0 1 * *"`, string(bts)) + }) +} diff --git a/internal/cac/storage/server_storage.go b/internal/cac/storage/server_storage.go index 7e9f713..b25daf9 100644 --- a/internal/cac/storage/server_storage.go +++ b/internal/cac/storage/server_storage.go @@ -7,6 +7,7 @@ import ( "github.com/cloudentity/acp-client-go/clients/hub/models" smodels "github.com/cloudentity/acp-client-go/clients/system/models" "github.com/cloudentity/cac/internal/cac/api" + "github.com/cloudentity/cac/internal/cac/keyrotation" "github.com/cloudentity/cac/internal/cac/utils" "github.com/pkg/errors" "golang.org/x/exp/maps" @@ -40,6 +41,7 @@ func (s *ServerStorage) Write(ctx context.Context, input models.Rfc7396PatchOper workspacePath string workspace string data *models.TreeServer + rotation *keyrotation.Config options = &api.Options{} err error ) @@ -54,6 +56,11 @@ func (s *ServerStorage) Write(ctx context.Context, input models.Rfc7396PatchOper workspacePath = s.workspacePath(workspace) + // key_rotation is not part of the tree model, so it has to leave the patch before the strict decode + if rotation, err = keyrotation.Pop(input); err != nil { + return err + } + if data, err = utils.FromPatchToModel[models.TreeServer](input); err != nil { return errors.Wrap(err, "failed to convert patch to tree server") } @@ -150,6 +157,10 @@ func (s *ServerStorage) Write(ctx context.Context, input models.Rfc7396PatchOper return err } + if err = writeFile(rotation, filepath.Join(workspacePath, keyrotation.Key)); err != nil { + return err + } + slog.Info("Workspace configuration successfully stored", "workspace", workspace, "path", workspacePath) return nil @@ -257,6 +268,10 @@ func (s *ServerStorage) Read(ctx context.Context, opts ...api.SourceOpt) (models return nil, err } + if err = readFileToMap(server, keyrotation.Key, filepath.Join(path, keyrotation.Key)); err != nil { + return nil, err + } + if server, err = utils.FilterPatch(server, options.Filters, utils.ServerRootKeys); err != nil { return nil, err } diff --git a/internal/cac/storage/server_storage_test.go b/internal/cac/storage/server_storage_test.go index 997fca8..ffea12b 100644 --- a/internal/cac/storage/server_storage_test.go +++ b/internal/cac/storage/server_storage_test.go @@ -3,6 +3,7 @@ package storage_test import ( "context" "io/fs" + "maps" "os" "path/filepath" "slices" @@ -13,6 +14,7 @@ import ( "github.com/cloudentity/acp-client-go/clients/hub/models" "github.com/cloudentity/cac/internal/cac/api" "github.com/cloudentity/cac/internal/cac/diff" + "github.com/cloudentity/cac/internal/cac/keyrotation" "github.com/cloudentity/cac/internal/cac/logging" "github.com/cloudentity/cac/internal/cac/storage" "github.com/cloudentity/cac/internal/cac/utils" @@ -26,6 +28,8 @@ func TestStorage(t *testing.T) { tcs := []struct { desc string data *models.TreeServer + // extra holds patch keys that are not part of models.TreeServer, such as key_rotation + extra models.Rfc7396PatchOperation files []string filters []string assert func(t *testing.T, path string, bts []byte) @@ -607,6 +611,86 @@ system: false`, string(bts)) } }, }, + { + desc: "key rotation", + data: &models.TreeServer{ + Name: "demo workspace", + }, + extra: models.Rfc7396PatchOperation{ + keyrotation.Key: map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + "starting_from": "2026-10-01T00:00:00.000Z", + }, + "enc": map[string]any{ + "enabled": false, + "cron": "0 0 1 * *", + }, + }, + }, + files: []string{ + "workspaces/demo/key_rotation.yaml", + }, + assert: func(t *testing.T, path string, bts []byte) { + require.YAMLEq(t, `sig: + enabled: true + cron: "0 0 1 * *" + starting_from: 2026-10-01T00:00:00.000Z +enc: + enabled: false + cron: "0 0 1 * *"`, string(bts)) + // scheduled_at is read-only in ACP and must never be written out + require.NotContains(t, string(bts), "scheduled_at") + }, + }, + { + desc: "key rotation, sig only", + data: &models.TreeServer{ + Name: "demo workspace", + }, + extra: models.Rfc7396PatchOperation{ + keyrotation.Key: map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "@monthly", + }, + }, + }, + files: []string{ + "workspaces/demo/key_rotation.yaml", + }, + assert: func(t *testing.T, path string, bts []byte) { + require.YAMLEq(t, `sig: + enabled: true + cron: "@monthly"`, string(bts)) + require.NotContains(t, string(bts), "starting_from") + require.NotContains(t, string(bts), "enc") + }, + }, + { + desc: "key rotation, filtered", + data: &models.TreeServer{ + Idps: models.TreeIDPs{ + "some-idp": models.TreeIDP{ + Name: "Some IDP", + }, + }, + }, + extra: models.Rfc7396PatchOperation{ + keyrotation.Key: map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + }, + }, + }, + files: []string{ + "workspaces/demo/idps/Some_IDP.yaml", + "workspaces/demo/key_rotation.yaml", + }, + filters: []string{keyrotation.Key}, + }, } for _, tc := range tcs { @@ -626,9 +710,15 @@ system: false`, string(bts)) patchData, err := utils.FromModelToPatch(tc.data) require.NoError(t, err) + maps.Copy(patchData, tc.extra) + err = st.Write(context.Background(), patchData, api.WithWorkspace("demo")) require.NoError(t, err) + // Write pops the extension keys out of the patch it is handed, so put them back + // before the round trip comparison below + maps.Copy(patchData, tc.extra) + var files []string for _, dir := range st.Config.DirPath { @@ -674,9 +764,56 @@ system: false`, string(bts)) patchData, err = utils.FilterPatch(patchData, tc.filters, utils.ServerRootKeys) require.NoError(t, err) + // diff ignores key_rotation..starting_from, so this round trip alone cannot catch a + // dropped starting_from: the YAMLEq and keyrotation.Pop based tests cover that d, err := diff.Tree(patchData, readServer) require.NoError(t, err) require.Empty(t, d) }) } } + +func TestServerStorageKeyRotationAbsent(t *testing.T) { + require.NoError(t, logging.InitLogging(&logging.Configuration{Level: "debug"})) + + dir := t.TempDir() + st := storage.InitServerStorage(&storage.Configuration{DirPath: dir}) + + written, err := utils.FromModelToPatch(&models.TreeServer{Name: "demo workspace"}) + require.NoError(t, err) + + require.NoError(t, st.Write(context.Background(), written, api.WithWorkspace("demo"))) + + _, err = os.Stat(filepath.Join(dir, "workspaces", "demo", "key_rotation.yaml")) + require.True(t, os.IsNotExist(err), "key_rotation.yaml must not be written when the patch has no key_rotation") + + read, err := st.Read(context.Background(), api.WithWorkspace("demo")) + require.NoError(t, err) + require.NotContains(t, read, keyrotation.Key) +} + +func TestServerStorageKeyRotationFilters(t *testing.T) { + require.NoError(t, logging.InitLogging(&logging.Configuration{Level: "debug"})) + + st := storage.InitServerStorage(&storage.Configuration{DirPath: t.TempDir()}) + + written, err := utils.FromModelToPatch(&models.TreeServer{Name: "demo workspace"}) + require.NoError(t, err) + + rotation := map[string]any{ + "sig": map[string]any{"enabled": true, "cron": "0 0 1 * *"}, + } + written[keyrotation.Key] = rotation + + require.NoError(t, st.Write(context.Background(), written, api.WithWorkspace("demo"))) + + read, err := st.Read(context.Background(), api.WithWorkspace("demo"), api.WithFilters([]string{keyrotation.Key})) + require.NoError(t, err) + require.Equal(t, models.Rfc7396PatchOperation{keyrotation.Key: rotation}, read) + + // key_rotation lives in its own file, so it is not part of the root workspace configuration + read, err = st.Read(context.Background(), api.WithWorkspace("demo"), api.WithFilters([]string{utils.RootFilter})) + require.NoError(t, err) + require.NotContains(t, read, keyrotation.Key) + require.Equal(t, "demo workspace", read["name"]) +} diff --git a/internal/cac/storage/tenant_storage.go b/internal/cac/storage/tenant_storage.go index b94d9ca..4e8d352 100644 --- a/internal/cac/storage/tenant_storage.go +++ b/internal/cac/storage/tenant_storage.go @@ -6,7 +6,9 @@ import ( "github.com/cloudentity/acp-client-go/clients/hub/models" smodels "github.com/cloudentity/acp-client-go/clients/system/models" "github.com/cloudentity/cac/internal/cac/api" + "github.com/cloudentity/cac/internal/cac/keyrotation" "github.com/cloudentity/cac/internal/cac/utils" + "github.com/pkg/errors" "path/filepath" ) @@ -24,11 +26,17 @@ type TenantStorage struct { func (t *TenantStorage) Write(ctx context.Context, data models.Rfc7396PatchOperation, opts ...api.SourceOpt) error { var ( - path = t.Config.DirPath - model *models.TreeTenant - err error + path = t.Config.DirPath + model *models.TreeTenant + rotations map[string]*keyrotation.Config + err error ) + // key_rotation is not part of the tree model, so it has to leave the workspaces before the strict decode + if rotations, err = popKeyRotations(data); err != nil { + return err + } + if model, err = utils.FromPatchToModel[models.TreeTenant](data); err != nil { return err } @@ -87,6 +95,12 @@ func (t *TenantStorage) Write(ctx context.Context, data models.Rfc7396PatchOpera return err } + if rotation, ok := rotations[k]; ok { + if serverData[keyrotation.Key], err = utils.FromModelToPatch(rotation); err != nil { + return err + } + } + if err = t.ServerStorage.Write(ctx, serverData, opts...); err != nil { return err } @@ -208,6 +222,42 @@ func (t *TenantStorage) Read(ctx context.Context, opts ...api.SourceOpt) (models var _ Storage = &TenantStorage{} +// popKeyRotations removes key_rotation from every workspace of a tenant patch and returns the +// configurations keyed by workspace id, so they can be handed to the server storage afterwards. +func popKeyRotations(data models.Rfc7396PatchOperation) (map[string]*keyrotation.Config, error) { + var ( + servers models.Rfc7396PatchOperation + out = map[string]*keyrotation.Config{} + ok bool + ) + + if servers, ok = utils.AsPatch(data["servers"]); !ok { + return out, nil + } + + for wid, it := range servers { + var ( + server models.Rfc7396PatchOperation + config *keyrotation.Config + err error + ) + + if server, ok = utils.AsPatch(it); !ok { + continue + } + + if config, err = keyrotation.Pop(server); err != nil { + return nil, errors.Wrapf(err, "workspace %s", wid) + } + + if config != nil { + out[wid] = config + } + } + + return out, nil +} + func (t *TenantStorage) storeTenant(path string, data *models.TreeTenant) error { var ( tenant smodels.Tenant diff --git a/internal/cac/storage/tenant_storage_test.go b/internal/cac/storage/tenant_storage_test.go index 5a50846..6912111 100644 --- a/internal/cac/storage/tenant_storage_test.go +++ b/internal/cac/storage/tenant_storage_test.go @@ -5,12 +5,14 @@ import ( "github.com/cloudentity/acp-client-go/clients/hub/models" "github.com/cloudentity/cac/internal/cac/api" "github.com/cloudentity/cac/internal/cac/diff" + "github.com/cloudentity/cac/internal/cac/keyrotation" "github.com/cloudentity/cac/internal/cac/logging" "github.com/cloudentity/cac/internal/cac/storage" "github.com/cloudentity/cac/internal/cac/utils" "github.com/go-openapi/strfmt" "github.com/stretchr/testify/require" "io/fs" + "maps" "os" "path/filepath" "testing" @@ -19,11 +21,14 @@ import ( func TestTenantStorage(t *testing.T) { tcs := []struct { - desc string - data *models.TreeTenant - files []string - filters []string - assert func(t *testing.T, path string, bts []byte) + desc string + data *models.TreeTenant + // serverExtra holds, per workspace id, patch keys that are not part of + // models.TreeServer, such as key_rotation + serverExtra map[string]models.Rfc7396PatchOperation + files []string + filters []string + assert func(t *testing.T, path string, bts []byte) }{ { desc: "workspace and mfa_methods", @@ -297,6 +302,47 @@ updated_at: 0001-01-01T00:00:00.000Z } }, }, + { + desc: "key rotation for one of the workspaces", + data: &models.TreeTenant{ + Servers: models.TreeServers{ + "demo": models.TreeServer{Name: "demo workspace"}, + "other": models.TreeServer{Name: "other workspace"}, + }, + }, + serverExtra: map[string]models.Rfc7396PatchOperation{ + "demo": { + keyrotation.Key: map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + "starting_from": "2026-10-01T00:00:00.000Z", + }, + "enc": map[string]any{ + "enabled": false, + "cron": "0 0 1 * *", + }, + }, + }, + }, + files: []string{ + "workspaces/demo/server.yaml", + "workspaces/demo/key_rotation.yaml", + "workspaces/other/server.yaml", + }, + assert: func(t *testing.T, path string, bts []byte) { + if path == "workspaces/demo/key_rotation.yaml" { + require.YAMLEq(t, `sig: + enabled: true + cron: "0 0 1 * *" + starting_from: 2026-10-01T00:00:00.000Z +enc: + enabled: false + cron: "0 0 1 * *"`, string(bts)) + require.NotContains(t, string(bts), "scheduled_at") + } + }, + }, } for _, tc := range tcs { @@ -316,9 +362,15 @@ updated_at: 0001-01-01T00:00:00.000Z patchData, err := utils.FromModelToPatch(tc.data) require.NoError(t, err) + applyServerExtra(t, patchData, tc.serverExtra) + err = st.Write(context.Background(), patchData, api.WithWorkspace("demo")) require.NoError(t, err) + // Write pops the extension keys out of the patch it is handed, so put them back + // before the round trip comparison below + applyServerExtra(t, patchData, tc.serverExtra) + var files []string for _, dir := range st.Config.DirPath { @@ -407,3 +459,75 @@ func TestTenantStoragePhoneProviderConfigRoundTrip(t *testing.T) { require.Equal(t, "ACtest", back.PhoneProviderConfig.Providers[0].Twilio.Sid) require.Equal(t, "tok", back.PhoneProviderConfig.Providers[0].Twilio.AuthToken) } + +// applyServerExtra merges per workspace patch keys that models.TreeTenant does not carry into +// servers. of an already converted tenant patch. +func applyServerExtra(t *testing.T, patch models.Rfc7396PatchOperation, extra map[string]models.Rfc7396PatchOperation) { + t.Helper() + + if len(extra) == 0 { + return + } + + servers, ok := patch["servers"].(map[string]any) + require.True(t, ok, "patch has no servers to merge into") + + for wid, it := range extra { + server, ok := servers[wid].(map[string]any) + require.True(t, ok, "patch has no server %s to merge into", wid) + + maps.Copy(server, it) + } +} + +func TestTenantStorageKeyRotationRoundTrip(t *testing.T) { + require.NoError(t, logging.InitLogging(&logging.Configuration{Level: "debug"})) + + st, err := storage.InitMultiStorage(&storage.MultiStorageConfiguration{ + DirPath: []string{t.TempDir()}, + }, storage.InitTenantStorage) + require.NoError(t, err) + + written, err := utils.FromModelToPatch(&models.TreeTenant{ + Name: "Default", + Servers: models.TreeServers{ + "demo": models.TreeServer{Name: "demo workspace"}, + }, + }) + require.NoError(t, err) + + applyServerExtra(t, written, map[string]models.Rfc7396PatchOperation{ + "demo": { + keyrotation.Key: map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + "starting_from": "2026-10-01T00:00:00.000Z", + }, + }, + }, + }) + + require.NoError(t, st.Write(context.Background(), written, api.WithWorkspace("demo"))) + + read, err := st.Read(context.Background(), api.WithWorkspace("demo")) + require.NoError(t, err) + + servers, ok := read["servers"].(map[string]any) + require.True(t, ok, "servers did not survive the round trip") + + // the read path keeps every workspace as a patch of its own + server, ok := servers["demo"].(models.Rfc7396PatchOperation) + require.True(t, ok, "the demo workspace did not survive the round trip") + + config, err := keyrotation.Pop(server) + require.NoError(t, err) + require.NotNil(t, config, "key_rotation did not survive the round trip") + + startingFrom, err := strfmt.ParseDateTime("2026-10-01T00:00:00.000Z") + require.NoError(t, err) + + require.Equal(t, &keyrotation.Config{ + Sig: &keyrotation.Rotation{Enabled: true, Cron: "0 0 1 * *", StartingFrom: &startingFrom}, + }, config) +} From 60b839409940332c15deec3b053d4b662db66ff3 Mon Sep 17 00:00:00 2001 From: Piotr Janus Date: Fri, 18 Sep 2026 15:57:36 +0200 Subject: [PATCH 3/5] feat(client): pull and push automatic key rotation per workspace Client.Read fetches use=sig and use=enc from the admin keys endpoint and adds them to the patch, omitting a use the server reports as never configured (empty cron). Client.Write pops key_rotation before the hub import or patch call, skips that call when nothing else remains (push --filter key_rotation), and PUTs each present use afterwards. TenantClient does the same for every workspace in sorted order. --- internal/cac/client/client.go | 33 ++- internal/cac/client/client_test.go | 284 +++++++++++++++++++++++- internal/cac/client/key_rotation.go | 169 ++++++++++++++ internal/cac/client/mock_server_test.go | 275 +++++++++++++++++------ internal/cac/client/tenant_client.go | 36 ++- 5 files changed, 712 insertions(+), 85 deletions(-) create mode 100644 internal/cac/client/key_rotation.go diff --git a/internal/cac/client/client.go b/internal/cac/client/client.go index 343d214..d739861 100644 --- a/internal/cac/client/client.go +++ b/internal/cac/client/client.go @@ -8,6 +8,7 @@ import ( "github.com/cloudentity/acp-client-go/clients/hub/client/workspace_configuration" "github.com/cloudentity/acp-client-go/clients/hub/models" "github.com/cloudentity/cac/internal/cac/api" + "github.com/cloudentity/cac/internal/cac/keyrotation" "github.com/cloudentity/cac/internal/cac/utils" "github.com/pkg/errors" "golang.org/x/exp/slog" @@ -79,6 +80,20 @@ func (c *Client) Read(ctx context.Context, opts ...api.SourceOpt) (models.Rfc739 return nil, errors.Wrap(err, "failed to convert tree server to patch") } + if filterSelects(options.Filters, keyrotation.Key) { + var rotation *keyrotation.Config + + if rotation, err = readKeyRotation(ctx, c.acp, workspace); err != nil { + return nil, err + } + + if rotation != nil { + if data[keyrotation.Key], err = keyRotationToPatch(rotation); err != nil { + return nil, err + } + } + } + if data, err = utils.FilterPatch(data, options.Filters, utils.ServerRootKeys); err != nil { return nil, errors.Wrap(err, "failed to filter patch") } @@ -89,6 +104,7 @@ func (c *Client) Read(ctx context.Context, opts ...api.SourceOpt) (models.Rfc739 func (c *Client) Write(ctx context.Context, data models.Rfc7396PatchOperation, opts ...api.SourceOpt) error { var ( options = &api.Options{} + rotation *keyrotation.Config workspace string err error ) @@ -101,12 +117,21 @@ func (c *Client) Write(ctx context.Context, data models.Rfc7396PatchOperation, o return errors.New("workspace is required to write using server client") } - switch options.Method { - case "import": + // Key rotation has its own endpoint and is not part of the tree models, so it leaves the patch + // before either method sees it. + if rotation, err = keyrotation.Pop(data); err != nil { + return err + } + + switch { + case len(data) == 0: + // a push filtered to key rotation alone leaves the configuration api nothing to do + slog.Debug("No workspace configuration to push", "workspace", workspace) + case options.Method == "import": if err = c.Import(ctx, workspace, options.Mode, data); err != nil { return err } - case "patch": + case options.Method == "patch": if err = c.Patch(ctx, workspace, options.Mode, data); err != nil { return err } @@ -114,7 +139,7 @@ func (c *Client) Write(ctx context.Context, data models.Rfc7396PatchOperation, o return fmt.Errorf("unknown method: %v", options.Method) } - return nil + return writeKeyRotation(ctx, c.acp, workspace, rotation) } func (c *Client) Patch(ctx context.Context, workspace string, mode string, data models.Rfc7396PatchOperation) error { diff --git a/internal/cac/client/client_test.go b/internal/cac/client/client_test.go index 1b6e6d6..148d051 100644 --- a/internal/cac/client/client_test.go +++ b/internal/cac/client/client_test.go @@ -4,10 +4,13 @@ import ( "context" "fmt" acpclient "github.com/cloudentity/acp-client-go" + "github.com/cloudentity/acp-client-go/clients/hub/models" "github.com/cloudentity/cac/internal/cac/api" "github.com/cloudentity/cac/internal/cac/client" + "github.com/cloudentity/cac/internal/cac/keyrotation" "github.com/stretchr/testify/require" "net/url" + "strings" "testing" ) @@ -18,7 +21,7 @@ func TestClient(t *testing.T) { _, err := client.InitClient(&client.Configuration{ Config: acpclient.Config{ IssuerURL: issuer, - TenantID: "postmance", + TenantID: "postmance", ClientID: "fb346c287c4d4e378cbae39aa0c3fe52", ClientSecret: "-T1siRsUvmE58hB-2I_fWQZW1lLpk_gK76ZziR8Y9QY", }, @@ -33,7 +36,7 @@ func TestClient(t *testing.T) { _, err := client.InitClient(&client.Configuration{ Config: acpclient.Config{ IssuerURL: issuer, - TenantID: "postmance", + TenantID: "postmance", ClientID: "fb346c287c4d4e378cbae39aa0c3fe52", ClientSecret: "-T1siRsUvmE58hB-2I_fWQZW1lLpk_gK76ZziR8Y9QY", }, @@ -49,7 +52,7 @@ func TestClient(t *testing.T) { c, err := client.InitClient(&client.Configuration{ Config: acpclient.Config{ IssuerURL: issuer, - TenantID: "postmance", + TenantID: "postmance", ClientID: "fb346c287c4d4e378cbae39aa0c3fe52", ClientSecret: "invalid_secret", }, @@ -70,7 +73,7 @@ func TestClient(t *testing.T) { Insecure: true, Config: acpclient.Config{ IssuerURL: issuer, - TenantID: "postmance", + TenantID: "postmance", ClientID: "fb346c287c4d4e378cbae39aa0c3fe52", ClientSecret: "valid_secret", }, @@ -98,7 +101,7 @@ func TestClient(t *testing.T) { Insecure: true, Config: acpclient.Config{ IssuerURL: issuer, - TenantID: "postmance", + TenantID: "postmance", ClientID: "fb346c287c4d4e378cbae39aa0c3fe52", ClientSecret: "valid_secret", }, @@ -126,7 +129,7 @@ func TestClient(t *testing.T) { Insecure: true, Config: acpclient.Config{ IssuerURL: issuer, - TenantID: "postmance", + TenantID: "postmance", ClientID: "fb346c287c4d4e378cbae39aa0c3fe52", ClientSecret: "valid_secret", }, @@ -155,7 +158,7 @@ func TestClient(t *testing.T) { Insecure: true, Config: acpclient.Config{ IssuerURL: issuer, - TenantID: "postmance", + TenantID: "postmance", ClientID: "fb346c287c4d4e378cbae39aa0c3fe52", ClientSecret: "valid_secret", }, @@ -178,3 +181,270 @@ func TestClient(t *testing.T) { require.Equal(t, "secret", secret) }) } + +func keyRotationClient(t *testing.T, mock *MockServer) *client.Client { + t.Helper() + + issuer, err := url.Parse(fmt.Sprintf("%s/postmance/system", mock.URL)) + require.NoError(t, err) + + c, err := client.InitClient(&client.Configuration{ + Insecure: true, + Config: acpclient.Config{ + IssuerURL: issuer, + TenantID: "postmance", + ClientID: "fb346c287c4d4e378cbae39aa0c3fe52", + ClientSecret: "valid_secret", + }, + }) + require.NoError(t, err) + + return c +} + +// keyRotationPut returns the recorded PUT body for a use, failing when it was not sent. +func keyRotationPut(t *testing.T, calls []KeyRotationCall, workspace string, use string) map[string]any { + t.Helper() + + for _, call := range calls { + if call.Workspace == workspace && call.Use == use { + return call.Body + } + } + + require.Failf(t, "missing key rotation PUT", "workspace %s, use %s, got %v", workspace, use, calls) + + return nil +} + +// requireZeroTime asserts a date-time field carries the zero instant, which is how the read-only +// scheduled_at leaves cac: it is never set from configuration. +func requireZeroTime(t *testing.T, value any) { + t.Helper() + + if value == nil { + return + } + + require.Truef(t, strings.HasPrefix(fmt.Sprint(value), "0001-01-01"), "expected zero date-time, got %v", value) +} + +func TestClientKeyRotation(t *testing.T) { + ctx := context.Background() + + t.Run("pull keeps only the uses the server has configured", func(t *testing.T) { + mock := CreateMockServer(t) + c := keyRotationClient(t, mock) + + data, err := c.Read(ctx, api.WithWorkspace("admin"), api.WithSecrets(false)) + require.NoError(t, err) + + require.Equal(t, map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + }, + }, data[keyrotation.Key]) + + require.ElementsMatch(t, []KeyRotationCall{ + {Workspace: "admin", Use: "sig"}, + {Workspace: "admin", Use: "enc"}, + }, mock.KeyRotationGets()) + }) + + t.Run("pull with an unrelated filter does not read key rotation", func(t *testing.T) { + mock := CreateMockServer(t) + c := keyRotationClient(t, mock) + + data, err := c.Read(ctx, api.WithWorkspace("admin"), api.WithSecrets(false), api.WithFilters([]string{"clients"})) + require.NoError(t, err) + + require.NotContains(t, data, keyrotation.Key) + require.Empty(t, mock.KeyRotationGets()) + }) + + t.Run("pull filtered to key rotation keeps the key", func(t *testing.T) { + mock := CreateMockServer(t) + c := keyRotationClient(t, mock) + + data, err := c.Read(ctx, api.WithWorkspace("admin"), api.WithSecrets(false), api.WithFilters([]string{keyrotation.Key})) + require.NoError(t, err) + + require.Contains(t, data, keyrotation.Key) + require.NotContains(t, data, "clients") + require.Len(t, mock.KeyRotationGets(), 2) + }) + + for _, method := range []string{"patch", "import"} { + t.Run(fmt.Sprintf("push with method %s sends key rotation out of band", method), func(t *testing.T) { + mock := CreateMockServer(t) + c := keyRotationClient(t, mock) + + err := c.Write(ctx, models.Rfc7396PatchOperation{ + "name": "demo workspace", + keyrotation.Key: map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + "starting_from": "2026-10-01T00:00:00Z", + }, + "enc": map[string]any{ + "enabled": false, + "cron": "@monthly", + }, + }, + }, api.WithWorkspace("admin"), api.WithMethod(method), api.WithMode("update")) + require.NoError(t, err) + + writes := mock.ConfigWrites() + require.Len(t, writes, 1) + require.NotContains(t, writes[0].Body, keyrotation.Key) + require.Equal(t, "demo workspace", writes[0].Body["name"]) + + puts := mock.KeyRotationPuts() + require.Len(t, puts, 2) + + sig := keyRotationPut(t, puts, "admin", "sig") + require.Equal(t, true, sig["enabled"]) + require.Equal(t, "0 0 1 * *", sig["cron"]) + require.Contains(t, fmt.Sprint(sig["starting_from"]), "2026-10-01") + requireZeroTime(t, sig["scheduled_at"]) + + enc := keyRotationPut(t, puts, "admin", "enc") + require.Equal(t, false, enc["enabled"]) + require.Equal(t, "@monthly", enc["cron"]) + requireZeroTime(t, enc["starting_from"]) + requireZeroTime(t, enc["scheduled_at"]) + }) + } + + for _, method := range []string{"patch", "import"} { + t.Run(fmt.Sprintf("push of key rotation alone with method %s skips the configuration api", method), func(t *testing.T) { + mock := CreateMockServer(t) + c := keyRotationClient(t, mock) + + err := c.Write(ctx, models.Rfc7396PatchOperation{ + keyrotation.Key: map[string]any{ + "sig": map[string]any{"enabled": true, "cron": "0 0 1 * *"}, + }, + }, api.WithWorkspace("admin"), api.WithMethod(method), api.WithMode("update")) + require.NoError(t, err) + + require.Empty(t, mock.ConfigWrites()) + require.Len(t, mock.KeyRotationPuts(), 1) + }) + } + + t.Run("push without key rotation does not touch the keys api", func(t *testing.T) { + mock := CreateMockServer(t) + c := keyRotationClient(t, mock) + + err := c.Write(ctx, models.Rfc7396PatchOperation{ + "name": "demo workspace", + }, api.WithWorkspace("admin"), api.WithMethod("patch"), api.WithMode("update")) + require.NoError(t, err) + + require.Empty(t, mock.KeyRotationPuts()) + }) + + t.Run("tenant pull nests key rotation under the workspace", func(t *testing.T) { + mock := CreateMockServer(t) + c := keyRotationClient(t, mock) + + data, err := c.Tenant().Read(ctx, api.WithSecrets(false)) + require.NoError(t, err) + + servers, ok := data["servers"].(map[string]any) + require.True(t, ok) + + server, ok := servers["server1"].(map[string]any) + require.True(t, ok) + + require.Equal(t, map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + }, + }, server[keyrotation.Key]) + + require.ElementsMatch(t, []KeyRotationCall{ + {Workspace: "server1", Use: "sig"}, + {Workspace: "server1", Use: "enc"}, + }, mock.KeyRotationGets()) + }) + + t.Run("tenant push sends key rotation per workspace", func(t *testing.T) { + mock := CreateMockServer(t) + c := keyRotationClient(t, mock) + + err := c.Tenant().Write(ctx, models.Rfc7396PatchOperation{ + "name": "demo tenant", + "servers": map[string]any{ + "server1": map[string]any{ + "name": "demo workspace", + keyrotation.Key: map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + }, + }, + }, + }, + }, api.WithMethod("patch"), api.WithMode("update")) + require.NoError(t, err) + + writes := mock.ConfigWrites() + require.Len(t, writes, 1) + require.Equal(t, "/api/hub/postmance/promote/config-rfc7396", writes[0].Path) + + servers, ok := writes[0].Body["servers"].(map[string]any) + require.True(t, ok) + + server, ok := servers["server1"].(map[string]any) + require.True(t, ok) + require.NotContains(t, server, keyrotation.Key) + + puts := mock.KeyRotationPuts() + require.Len(t, puts, 1) + + sig := keyRotationPut(t, puts, "server1", "sig") + require.Equal(t, true, sig["enabled"]) + require.Equal(t, "0 0 1 * *", sig["cron"]) + }) + + t.Run("tenant push with method import sends key rotation per workspace", func(t *testing.T) { + mock := CreateMockServer(t) + c := keyRotationClient(t, mock) + + err := c.Tenant().Write(ctx, models.Rfc7396PatchOperation{ + "name": "demo tenant", + "servers": map[string]any{ + "server1": map[string]any{ + "name": "demo workspace", + keyrotation.Key: map[string]any{ + "sig": map[string]any{"enabled": true, "cron": "0 0 1 * *"}, + }, + }, + }, + }, api.WithMethod("import"), api.WithMode("update")) + require.NoError(t, err) + + writes := mock.ConfigWrites() + require.Len(t, writes, 1) + require.Equal(t, "/api/hub/postmance/promote/config", writes[0].Path) + + servers, ok := writes[0].Body["servers"].(map[string]any) + require.True(t, ok) + + server, ok := servers["server1"].(map[string]any) + require.True(t, ok) + require.NotContains(t, server, keyrotation.Key) + + puts := mock.KeyRotationPuts() + require.Len(t, puts, 1) + + sig := keyRotationPut(t, puts, "server1", "sig") + require.Equal(t, true, sig["enabled"]) + require.Equal(t, "0 0 1 * *", sig["cron"]) + }) +} diff --git a/internal/cac/client/key_rotation.go b/internal/cac/client/key_rotation.go new file mode 100644 index 0000000..e94a8f4 --- /dev/null +++ b/internal/cac/client/key_rotation.go @@ -0,0 +1,169 @@ +package client + +import ( + "context" + "slices" + + acpclient "github.com/cloudentity/acp-client-go" + "github.com/cloudentity/acp-client-go/clients/admin/client/keys" + "github.com/cloudentity/acp-client-go/clients/hub/models" + "github.com/cloudentity/cac/internal/cac/keyrotation" + "github.com/cloudentity/cac/internal/cac/utils" + "github.com/pkg/errors" + "golang.org/x/exp/slog" +) + +// filterSelects reports whether a key survives the requested filters. +func filterSelects(filters []string, key string) bool { + return len(filters) == 0 || slices.Contains(filters, key) +} + +// readKeyRotation fetches the automatic key rotation configuration of a workspace. It returns nil +// when neither use is configured. +func readKeyRotation(ctx context.Context, acp *acpclient.Client, workspace string) (*keyrotation.Config, error) { + var ( + config keyrotation.Config + err error + ) + + if config.Sig, err = readKeyRotationUse(ctx, acp, workspace, keyrotation.UseSig); err != nil { + return nil, err + } + + if config.Enc, err = readKeyRotationUse(ctx, acp, workspace, keyrotation.UseEnc); err != nil { + return nil, err + } + + if config.Sig == nil && config.Enc == nil { + return nil, nil + } + + return &config, nil +} + +// readKeyRotationUse fetches one use. ACP has no default for the use query parameter, so it is +// always sent; a use that was never configured answers 200 with an empty cron, which FromModel +// reports as nil rather than an error. +func readKeyRotationUse(ctx context.Context, acp *acpclient.Client, workspace string, use string) (*keyrotation.Rotation, error) { + var ( + ok *keys.GetAutomaticKeyRotationOK + err error + ) + + if ok, err = acp.Admin.Keys.GetAutomaticKeyRotation(keys. + NewGetAutomaticKeyRotationParams(). + WithContext(ctx). + WithWid(workspace). + WithUse(&use), nil); err != nil { + return nil, errors.Wrapf(err, "failed to get %s for workspace %s, use %s", keyrotation.Key, workspace, use) + } + + return keyrotation.FromModel(ok.Payload), nil +} + +// writeKeyRotation pushes every configured use of a workspace. It is a no-op for a nil config. +func writeKeyRotation(ctx context.Context, acp *acpclient.Client, workspace string, config *keyrotation.Config) error { + for _, use := range config.Uses() { + slog.Info("Pushing key rotation configuration", "workspace", workspace, "use", use.Use) + + if _, err := acp.Admin.Keys.SetAutomaticKeyRotation(keys. + NewSetAutomaticKeyRotationParams(). + WithContext(ctx). + WithWid(workspace). + WithUse(&use.Use). + WithAutomaticKeyRotation(use.Rotation.ToModel()), nil); err != nil { + return errors.Wrapf(err, "failed to set %s for workspace %s, use %s", keyrotation.Key, workspace, use.Use) + } + } + + return nil +} + +// keyRotationToPatch renders a configuration as a plain map, the shape everything else in a patch +// has, so mergo, diff and yaml treat it uniformly. +func keyRotationToPatch(config *keyrotation.Config) (map[string]any, error) { + var ( + patch models.Rfc7396PatchOperation + err error + ) + + if patch, err = utils.FromModelToPatch(config); err != nil { + return nil, errors.Wrapf(err, "failed to convert %s to patch", keyrotation.Key) + } + + return patch, nil +} + +// readServersKeyRotation fills in the key rotation of every workspace of a tenant patch. +func readServersKeyRotation(ctx context.Context, acp *acpclient.Client, data models.Rfc7396PatchOperation) error { + var ( + servers models.Rfc7396PatchOperation + ok bool + ) + + if servers, ok = utils.AsPatch(data["servers"]); !ok { + return nil + } + + for workspace, raw := range servers { + var ( + server models.Rfc7396PatchOperation + rotation *keyrotation.Config + err error + ) + + if server, ok = utils.AsPatch(raw); !ok { + continue + } + + if rotation, err = readKeyRotation(ctx, acp, workspace); err != nil { + return err + } + + if rotation == nil { + continue + } + + if server[keyrotation.Key], err = keyRotationToPatch(rotation); err != nil { + return err + } + } + + return nil +} + +// popServersKeyRotation removes the key rotation of every workspace of a tenant patch and returns +// what was removed, keyed by workspace. +func popServersKeyRotation(data models.Rfc7396PatchOperation) (map[string]*keyrotation.Config, error) { + var ( + servers models.Rfc7396PatchOperation + rotations = map[string]*keyrotation.Config{} + ok bool + ) + + if servers, ok = utils.AsPatch(data["servers"]); !ok { + return rotations, nil + } + + for workspace, raw := range servers { + var ( + server models.Rfc7396PatchOperation + rotation *keyrotation.Config + err error + ) + + if server, ok = utils.AsPatch(raw); !ok { + continue + } + + if rotation, err = keyrotation.Pop(server); err != nil { + return nil, err + } + + if rotation != nil { + rotations[workspace] = rotation + } + } + + return rotations, nil +} diff --git a/internal/cac/client/mock_server_test.go b/internal/cac/client/mock_server_test.go index 6458057..0a621a0 100644 --- a/internal/cac/client/mock_server_test.go +++ b/internal/cac/client/mock_server_test.go @@ -1,105 +1,242 @@ package client_test import ( - "github.com/cloudentity/acp-client-go/clients/hub/models" - "github.com/go-json-experiment/json" - "github.com/go-openapi/strfmt" - "github.com/stretchr/testify/require" + "io" "net/http" "net/http/httptest" + "strings" + "sync" "testing" "time" + + "github.com/cloudentity/acp-client-go/clients/hub/models" + "github.com/go-json-experiment/json" + "github.com/go-openapi/strfmt" + "github.com/stretchr/testify/require" +) + +const ( + keyRotationPrefix = "/api/admin/postmance/servers/" + keyRotationSuffix = "/keys/automatic-key-rotation" ) -func CreateMockServer(t *testing.T) *httptest.Server { - testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { +// keyRotationSigPayload is what ACP returns for a configured use. scheduled_at is read-only and +// starting_from is never echoed back, so both come back as the server really sends them. +const keyRotationSigPayload = `{"enabled":true,"cron":"0 0 1 * *","scheduled_at":"2026-10-01T00:00:00Z","starting_from":"0001-01-01T00:00:00Z"}` + +// keyRotationEncPayload is the shape ACP returns for a use that was never configured: 200 with an +// empty cron rather than a 404. +const keyRotationEncPayload = `{"enabled":false,"cron":"","starting_from":"0001-01-01T00:00:00Z","scheduled_at":"0001-01-01T00:00:00Z"}` + +// KeyRotationCall records one automatic key rotation request the mock server handled. +type KeyRotationCall struct { + Workspace string + Use string + Body map[string]any +} + +// ConfigWrite records one workspace or tenant configuration write the mock server handled. +type ConfigWrite struct { + Path string + Body map[string]any +} + +// MockServer is an httptest.Server that additionally records the requests tests assert on. +type MockServer struct { + *httptest.Server + + mu sync.Mutex + keyRotationGets []KeyRotationCall + keyRotationPuts []KeyRotationCall + configWrites []ConfigWrite +} + +func (m *MockServer) KeyRotationGets() []KeyRotationCall { + m.mu.Lock() + defer m.mu.Unlock() + + return append([]KeyRotationCall(nil), m.keyRotationGets...) +} + +func (m *MockServer) KeyRotationPuts() []KeyRotationCall { + m.mu.Lock() + defer m.mu.Unlock() + + return append([]KeyRotationCall(nil), m.keyRotationPuts...) +} - if req.URL.Path == "/postmance/system/.well-known/openid-configuration" { - js := []byte(`{ +func (m *MockServer) ConfigWrites() []ConfigWrite { + m.mu.Lock() + defer m.mu.Unlock() + + return append([]ConfigWrite(nil), m.configWrites...) +} + +func (m *MockServer) record(call KeyRotationCall, put bool) { + m.mu.Lock() + defer m.mu.Unlock() + + if put { + m.keyRotationPuts = append(m.keyRotationPuts, call) + } else { + m.keyRotationGets = append(m.keyRotationGets, call) + } +} + +func (m *MockServer) recordConfigWrite(write ConfigWrite) { + m.mu.Lock() + defer m.mu.Unlock() + + m.configWrites = append(m.configWrites, write) +} + +func CreateMockServer(t *testing.T) *MockServer { + mock := &MockServer{} + + mock.Server = httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + if req.URL.Path == "/postmance/system/.well-known/openid-configuration" { + js := []byte(`{ "issuer": "https://demo.eu.authz.cloudentity.io/demo/system", "authorization_endpoint": "https://postmance.eu.authz.cloudentity.io/demo/system/oauth2/auth", "token_endpoint": "https://postmance.eu.authz.cloudentity.io/demo/system/oauth2/token" }`) - res.WriteHeader(http.StatusOK) - _, err := res.Write(js) - require.NoError(t, err) + res.WriteHeader(http.StatusOK) + _, err := res.Write(js) + require.NoError(t, err) - return - } + return + } - if req.URL.Path == "/postmance/system/oauth2/token" { - js := []byte(`{ + if req.URL.Path == "/postmance/system/oauth2/token" { + js := []byte(`{ "token_type": "Bearer", "scope": "openid", "access_token": "MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3", "expires_in": 3600 }`) - res.Header().Set("Content-Type", "application/json") - res.WriteHeader(http.StatusOK) - _, err := res.Write(js) - require.NoError(t, err) + res.Header().Set("Content-Type", "application/json") + res.WriteHeader(http.StatusOK) + _, err := res.Write(js) + require.NoError(t, err) - return - } + return + } - if req.URL.Path == "/api/hub/postmance/promote/config" { - res.Header().Set("Content-Type", "application/json") - res.WriteHeader(http.StatusOK) - tt := models.TreeTenant{ - Name: "demo tenant", - Servers: models.TreeServers{ - "server1": models.TreeServer{ - Name: "demo workspace", - Clients: models.TreeClients{ - "cid1": models.TreeClient{ - ClientName: "client1", + if strings.HasPrefix(req.URL.Path, keyRotationPrefix) && strings.HasSuffix(req.URL.Path, keyRotationSuffix) { + workspace := strings.TrimSuffix(strings.TrimPrefix(req.URL.Path, keyRotationPrefix), keyRotationSuffix) + use := req.URL.Query().Get("use") + + res.Header().Set("Content-Type", "application/json") + + if req.Method == http.MethodPut { + raw, err := io.ReadAll(req.Body) + require.NoError(t, err) + + var body map[string]any + require.NoError(t, json.Unmarshal(raw, &body)) + + mock.record(KeyRotationCall{Workspace: workspace, Use: use, Body: body}, true) + + js, err := json.Marshal(body) + require.NoError(t, err) + + res.WriteHeader(http.StatusOK) + _, err = res.Write(js) + require.NoError(t, err) + + return + } + + mock.record(KeyRotationCall{Workspace: workspace, Use: use}, false) + + res.WriteHeader(http.StatusOK) + + var err error + + if use == "sig" { + _, err = res.Write([]byte(keyRotationSigPayload)) + } else { + _, err = res.Write([]byte(keyRotationEncPayload)) + } + + require.NoError(t, err) + + return + } + + if req.Method != http.MethodGet && strings.HasPrefix(req.URL.Path, "/api/hub/postmance/") { + raw, err := io.ReadAll(req.Body) + require.NoError(t, err) + + var body map[string]any + require.NoError(t, json.Unmarshal(raw, &body)) + + mock.recordConfigWrite(ConfigWrite{Path: req.URL.Path, Body: body}) + + res.WriteHeader(http.StatusNoContent) + + return + } + + if req.URL.Path == "/api/hub/postmance/promote/config" { + res.Header().Set("Content-Type", "application/json") + res.WriteHeader(http.StatusOK) + tt := models.TreeTenant{ + Name: "demo tenant", + Servers: models.TreeServers{ + "server1": models.TreeServer{ + Name: "demo workspace", + Clients: models.TreeClients{ + "cid1": models.TreeClient{ + ClientName: "client1", + }, }, }, }, - }, - MfaMethods: models.TreeMFAMethods{ - "sms": models.TreeMFAMethod{ - Enabled: true, + MfaMethods: models.TreeMFAMethods{ + "sms": models.TreeMFAMethod{ + Enabled: true, + }, }, - }, - } + } - if req.URL.Query().Get("with_credentials") != "" { - s := tt.Servers["server1"] - c :=s.Clients["cid1"] - c.ClientSecret = "secret" - s.Clients["cid1"] = c - } + if req.URL.Query().Get("with_credentials") != "" { + s := tt.Servers["server1"] + c := s.Clients["cid1"] + c.ClientSecret = "secret" + s.Clients["cid1"] = c + } - js, err := json.Marshal(tt) - require.NoError(t, err) + js, err := json.Marshal(tt) + require.NoError(t, err) - _, err = res.Write(js) - require.NoError(t, err) + _, err = res.Write(js) + require.NoError(t, err) - return - } + return + } - res.Header().Set("Content-Type", "application/json") - res.WriteHeader(http.StatusOK) - js, err := json.Marshal(models.TreeServer{ - Name: "demo workspace", - AccessTokenTTL: strfmt.Duration(10 * time.Minute), - Clients: models.TreeClients{ - "client1": models.TreeClient{ - ClientName: "client1", + res.Header().Set("Content-Type", "application/json") + res.WriteHeader(http.StatusOK) + js, err := json.Marshal(models.TreeServer{ + Name: "demo workspace", + AccessTokenTTL: strfmt.Duration(10 * time.Minute), + Clients: models.TreeClients{ + "client1": models.TreeClient{ + ClientName: "client1", + }, }, - }, - Idps: models.TreeIDPs{ - "idp1": models.TreeIDP{ - Name: "idp1", + Idps: models.TreeIDPs{ + "idp1": models.TreeIDP{ + Name: "idp1", + }, }, - }, - }) - require.NoError(t, err) + }) + require.NoError(t, err) - _, err = res.Write(js) - require.NoError(t, err) + _, err = res.Write(js) + require.NoError(t, err) })) -return testServer -} \ No newline at end of file + + return mock +} diff --git a/internal/cac/client/tenant_client.go b/internal/cac/client/tenant_client.go index c846f19..7800e25 100644 --- a/internal/cac/client/tenant_client.go +++ b/internal/cac/client/tenant_client.go @@ -3,11 +3,14 @@ package client import ( "context" "fmt" + "maps" + "slices" acpclient "github.com/cloudentity/acp-client-go" "github.com/cloudentity/acp-client-go/clients/hub/client/tenant_configuration" "github.com/cloudentity/acp-client-go/clients/hub/models" "github.com/cloudentity/cac/internal/cac/api" + "github.com/cloudentity/cac/internal/cac/keyrotation" "github.com/cloudentity/cac/internal/cac/utils" "golang.org/x/exp/slog" ) @@ -41,6 +44,12 @@ func (t *TenantClient) Read(ctx context.Context, opts ...api.SourceOpt) (models. return nil, err } + if filterSelects(options.Filters, "servers") { + if err = readServersKeyRotation(ctx, t.acp, data); err != nil { + return nil, err + } + } + if data, err = utils.FilterPatch(data, options.Filters, utils.TenantRootKeys); err != nil { return nil, err } @@ -50,20 +59,30 @@ func (t *TenantClient) Read(ctx context.Context, opts ...api.SourceOpt) (models. func (t *TenantClient) Write(ctx context.Context, data models.Rfc7396PatchOperation, opts ...api.SourceOpt) error { var ( - options = &api.Options{} - err error + options = &api.Options{} + rotations map[string]*keyrotation.Config + err error ) for _, opt := range opts { opt(options) } - switch options.Method { - case "import": + // Key rotation has its own endpoint and is not part of the tree models, so it leaves the patch + // before either method sees it. + if rotations, err = popServersKeyRotation(data); err != nil { + return err + } + + switch { + case len(data) == 0: + // a push filtered to key rotation alone leaves the configuration api nothing to do + slog.Debug("No tenant configuration to push") + case options.Method == "import": if err = t.Import(ctx, options.Mode, data); err != nil { return err } - case "patch": + case options.Method == "patch": if err = t.Patch(ctx, options.Mode, data); err != nil { return err } @@ -71,6 +90,13 @@ func (t *TenantClient) Write(ctx context.Context, data models.Rfc7396PatchOperat return fmt.Errorf("unknown method: %v", options.Method) } + // sorted so logs and errors do not depend on the map iteration order + for _, workspace := range slices.Sorted(maps.Keys(rotations)) { + if err = writeKeyRotation(ctx, t.acp, workspace, rotations[workspace]); err != nil { + return err + } + } + return nil } From cb6a78aa4c3b14b789f06cae84198d863ee96446 Mon Sep 17 00:00:00 2001 From: Piotr Janus Date: Fri, 18 Sep 2026 15:57:36 +0200 Subject: [PATCH 4/5] feat: validate key rotation on push and ignore starting_from in diff Validators strict-decode and validate key_rotation, then run the tree validation on a copy of the patch without it. The caller's patch is still cleaned of id and tenant_id so push sends the same body as before. The server never returns starting_from, so diff ignores key_rotation..starting_from unconditionally; otherwise every diff would report it as an addition. --- .../script_execution_points_validation.go | 4 +- internal/cac/data/server_validator.go | 35 +++- internal/cac/data/tenant_validator.go | 66 ++++++- internal/cac/data/validator_test.go | 163 +++++++++++++++++- internal/cac/diff/diff.go | 11 +- internal/cac/diff/diff_test.go | 104 +++++++++++ 6 files changed, 374 insertions(+), 9 deletions(-) create mode 100644 internal/cac/diff/diff_test.go diff --git a/internal/cac/data/script_execution_points_validation.go b/internal/cac/data/script_execution_points_validation.go index 8743f6c..348494a 100644 --- a/internal/cac/data/script_execution_points_validation.go +++ b/internal/cac/data/script_execution_points_validation.go @@ -14,11 +14,11 @@ func allowToDeleteScriptExecutionPoints(ts *models.TreeServer) { for scriptID, script := range typee { // empty scriptID means that script is being deleted if script.ScriptID == "" { - // scriptID is required, so we need to set it to some value to pass validation + // scriptID is required, so we need to set it to some value to pass validation script.ScriptID = "[DELETED]" typee[scriptID] = script } } ts.ScriptExecutionPoints[typeID] = typee } -} \ No newline at end of file +} diff --git a/internal/cac/data/server_validator.go b/internal/cac/data/server_validator.go index 87316d0..ce04e93 100644 --- a/internal/cac/data/server_validator.go +++ b/internal/cac/data/server_validator.go @@ -1,7 +1,10 @@ package data import ( + "maps" + "github.com/cloudentity/acp-client-go/clients/hub/models" + "github.com/cloudentity/cac/internal/cac/keyrotation" "github.com/cloudentity/cac/internal/cac/utils" "github.com/go-openapi/strfmt" ) @@ -12,10 +15,24 @@ var _ ValidatorApi = &ServerValidator{} func (sv *ServerValidator) Validate(data *models.Rfc7396PatchOperation) error { var ( - err error - serv *models.TreeServer + err error + rotation *keyrotation.Config + serv *models.TreeServer ) - if serv, err = utils.FromPatchToModel[models.TreeServer](*data); err != nil { + + // FromPatchToModel is handed a copy below, so the caller's patch is cleaned here instead: push + // still uses it afterwards and the hub rejects id and tenant_id in a body. + utils.CleanPatch(*data) + + if rotation, err = keyrotation.Get(*data); err != nil { + return err + } + + if err = rotation.Validate(); err != nil { + return err + } + + if serv, err = utils.FromPatchToModel[models.TreeServer](withoutKeyRotation(*data)); err != nil { return err } @@ -27,3 +44,15 @@ func (sv *ServerValidator) Validate(data *models.Rfc7396PatchOperation) error { return nil } + +// withoutKeyRotation returns a shallow copy of the patch without the key rotation configuration, +// which is not part of the tree models. The caller's map is left alone: push still needs the key +// after validation. +func withoutKeyRotation(patch models.Rfc7396PatchOperation) models.Rfc7396PatchOperation { + out := make(models.Rfc7396PatchOperation, len(patch)) + + maps.Copy(out, patch) + delete(out, keyrotation.Key) + + return out +} diff --git a/internal/cac/data/tenant_validator.go b/internal/cac/data/tenant_validator.go index 654086c..05ef62d 100644 --- a/internal/cac/data/tenant_validator.go +++ b/internal/cac/data/tenant_validator.go @@ -1,9 +1,13 @@ package data import ( + "maps" + "github.com/cloudentity/acp-client-go/clients/hub/models" + "github.com/cloudentity/cac/internal/cac/keyrotation" "github.com/cloudentity/cac/internal/cac/utils" "github.com/go-openapi/strfmt" + "github.com/pkg/errors" ) type TenantValidator struct{} @@ -13,9 +17,19 @@ var _ ValidatorApi = &TenantValidator{} func (sv *TenantValidator) Validate(data *models.Rfc7396PatchOperation) error { var ( err error + patch models.Rfc7396PatchOperation tenant *models.TreeTenant ) - if tenant, err = utils.FromPatchToModel[models.TreeTenant](*data); err != nil { + + // FromPatchToModel is handed a copy below, so the caller's patch is cleaned here instead: push + // still uses it afterwards and the hub rejects id and tenant_id in a body. + utils.CleanPatch(*data) + + if patch, err = validateKeyRotations(*data); err != nil { + return err + } + + if tenant, err = utils.FromPatchToModel[models.TreeTenant](patch); err != nil { return err } @@ -29,3 +43,53 @@ func (sv *TenantValidator) Validate(data *models.Rfc7396PatchOperation) error { return nil } + +// validateKeyRotations validates the key rotation configuration of every workspace in a tenant +// patch and returns a shallow copy of the patch with those configurations removed, leaving the +// caller's maps untouched. +func validateKeyRotations(patch models.Rfc7396PatchOperation) (models.Rfc7396PatchOperation, error) { + var ( + out = make(models.Rfc7396PatchOperation, len(patch)) + servers models.Rfc7396PatchOperation + ok bool + ) + + maps.Copy(out, patch) + + if servers, ok = utils.AsPatch(out["servers"]); !ok { + return out, nil + } + + cleaned := make(models.Rfc7396PatchOperation, len(servers)) + maps.Copy(cleaned, servers) + + for wid, raw := range servers { + var ( + server models.Rfc7396PatchOperation + rotation *keyrotation.Config + err error + ) + + if server, ok = utils.AsPatch(raw); !ok { + continue + } + + if _, ok = server[keyrotation.Key]; !ok { + continue + } + + if rotation, err = keyrotation.Get(server); err != nil { + return nil, errors.Wrapf(err, "invalid configuration of workspace %s", wid) + } + + if err = rotation.Validate(); err != nil { + return nil, errors.Wrapf(err, "invalid configuration of workspace %s", wid) + } + + cleaned[wid] = withoutKeyRotation(server) + } + + out["servers"] = cleaned + + return out, nil +} diff --git a/internal/cac/data/validator_test.go b/internal/cac/data/validator_test.go index fedec02..2308518 100644 --- a/internal/cac/data/validator_test.go +++ b/internal/cac/data/validator_test.go @@ -11,7 +11,7 @@ import ( func TestServerValidator(t *testing.T) { validator := &data.TenantValidator{} - + t.Run("allow_script_exec_point_deletion", func(t *testing.T) { patch := models.Rfc7396PatchOperation{ "servers": map[string]any{ @@ -31,4 +31,163 @@ func TestServerValidator(t *testing.T) { require.NoError(t, err) }) -} \ No newline at end of file +} + +func TestServerValidatorKeyRotation(t *testing.T) { + validator := &data.ServerValidator{} + + tcs := []struct { + name string + rotation map[string]any + err string + }{ + { + name: "valid", + rotation: map[string]any{ + "sig": map[string]any{"enabled": true, "cron": "0 0 1 * *", "starting_from": "2026-10-01T00:00:00Z"}, + "enc": map[string]any{"enabled": false, "cron": "@monthly"}, + }, + }, + { + name: "disabled with a valid cron", + rotation: map[string]any{"sig": map[string]any{"enabled": false, "cron": "0 0 1 * *"}}, + }, + { + name: "unknown field", + rotation: map[string]any{"sig": map[string]any{"enabled": true, "cron": "0 0 1 * *", "foo": 1}}, + err: "key_rotation", + }, + { + name: "read only scheduled_at", + rotation: map[string]any{"sig": map[string]any{"enabled": true, "cron": "0 0 1 * *", "scheduled_at": "2026-10-01T00:00:00Z"}}, + err: "scheduled_at", + }, + { + name: "missing cron", + rotation: map[string]any{"sig": map[string]any{"enabled": true}}, + err: "cron is required for sig", + }, + { + name: "invalid cron", + rotation: map[string]any{"enc": map[string]any{"enabled": true, "cron": "@every 1h"}}, + err: "invalid cron for enc", + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + patch := models.Rfc7396PatchOperation{ + "name": "demo", + "key_rotation": tc.rotation, + } + + err := validator.Validate(&patch) + + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + } else { + require.NoError(t, err) + } + + // push still needs the key after validation, so the caller's patch must keep it + require.Contains(t, patch, "key_rotation") + }) + } +} + +func TestTenantValidatorKeyRotation(t *testing.T) { + validator := &data.TenantValidator{} + + tcs := []struct { + name string + rotation map[string]any + err string + }{ + { + name: "valid", + rotation: map[string]any{ + "sig": map[string]any{"enabled": true, "cron": "0 0 1 * *", "starting_from": "2026-10-01T00:00:00Z"}, + "enc": map[string]any{"enabled": false, "cron": "@monthly"}, + }, + }, + { + name: "unknown field", + rotation: map[string]any{"sig": map[string]any{"enabled": true, "cron": "0 0 1 * *", "foo": 1}}, + err: "key_rotation", + }, + { + name: "read only scheduled_at", + rotation: map[string]any{"sig": map[string]any{"enabled": true, "cron": "0 0 1 * *", "scheduled_at": "2026-10-01T00:00:00Z"}}, + err: "scheduled_at", + }, + { + name: "missing cron", + rotation: map[string]any{"enc": map[string]any{"enabled": false}}, + err: "cron is required for enc", + }, + { + name: "invalid cron", + rotation: map[string]any{"sig": map[string]any{"enabled": true, "cron": "nope"}}, + err: "invalid cron for sig", + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + server := map[string]any{ + "name": "demo", + "key_rotation": tc.rotation, + } + patch := models.Rfc7396PatchOperation{ + "servers": map[string]any{"demo": server}, + } + + err := validator.Validate(&patch) + + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + } else { + require.NoError(t, err) + } + + require.Contains(t, server, "key_rotation") + }) + } +} + +func TestValidatorsCleanCallerPatch(t *testing.T) { + // the hub rejects id and tenant_id in a push body and validation is what strips them, so it has + // to keep cleaning the caller's map even though key_rotation is now decoded from a copy of it + rotation := map[string]any{"sig": map[string]any{"enabled": true, "cron": "0 0 1 * *"}} + + t.Run("server", func(t *testing.T) { + patch := models.Rfc7396PatchOperation{ + "id": "demo", + "tenant_id": "postmance", + "name": "demo workspace", + "key_rotation": rotation, + } + + require.NoError(t, (&data.ServerValidator{}).Validate(&patch)) + + require.NotContains(t, patch, "id") + require.NotContains(t, patch, "tenant_id") + require.Contains(t, patch, "key_rotation") + }) + + t.Run("tenant", func(t *testing.T) { + server := map[string]any{"name": "demo workspace", "key_rotation": rotation} + patch := models.Rfc7396PatchOperation{ + "id": "postmance", + "tenant_id": "postmance", + "name": "demo tenant", + "servers": map[string]any{"demo": server}, + } + + require.NoError(t, (&data.TenantValidator{}).Validate(&patch)) + + require.NotContains(t, patch, "id") + require.NotContains(t, patch, "tenant_id") + require.Contains(t, server, "key_rotation") + }) +} diff --git a/internal/cac/diff/diff.go b/internal/cac/diff/diff.go index c90778b..ea381d0 100644 --- a/internal/cac/diff/diff.go +++ b/internal/cac/diff/diff.go @@ -66,6 +66,12 @@ var volatileFields = []string{ "last_active", } +// writeOnlyFields are ignored in every diff: ACP never echoes key_rotation starting_from back, so +// comparing it would report the local value as an addition on every single run. +var writeOnlyFields = []string{ + `\["key_rotation"\].*\["starting_from"\]`, +} + var fieldsFilter = func(fields []string) cmp.Option { return cmp.FilterPath(func(p cmp.Path) bool { for _, vf := range fields { @@ -88,6 +94,7 @@ var fieldsFilter = func(fields []string) cmp.Option { var filerVolatileFields = fieldsFilter(volatileFields) var filterSecretFields = fieldsFilter(secretFields) +var filterWriteOnlyFields = fieldsFilter(writeOnlyFields) func Diff(ctx context.Context, source api.Source, target api.Source, workspace string, opts ...Option) (string, error) { var ( @@ -156,6 +163,8 @@ func Tree(source models.Rfc7396PatchOperation, target models.Rfc7396PatchOperati } } + diffOpts = append(diffOpts, filterWriteOnlyFields) + if options.FilterVolatile { diffOpts = append(diffOpts, filerVolatileFields) } @@ -168,7 +177,7 @@ func Tree(source models.Rfc7396PatchOperation, target models.Rfc7396PatchOperati diffOpts = append(diffOpts, cmpopts.SortSlices(func(a, b string) bool { return a < b })) - + out := cmp.Diff(target, source, diffOpts) if options.Color { diff --git a/internal/cac/diff/diff_test.go b/internal/cac/diff/diff_test.go new file mode 100644 index 0000000..7ada8c9 --- /dev/null +++ b/internal/cac/diff/diff_test.go @@ -0,0 +1,104 @@ +package diff_test + +import ( + "testing" + + "github.com/cloudentity/acp-client-go/clients/hub/models" + "github.com/cloudentity/cac/internal/cac/diff" + "github.com/stretchr/testify/require" +) + +func TestTreeKeyRotation(t *testing.T) { + t.Run("ignores starting_from", func(t *testing.T) { + source := models.Rfc7396PatchOperation{ + "key_rotation": map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + "starting_from": "2026-10-01T00:00:00Z", + }, + }, + } + target := models.Rfc7396PatchOperation{ + "key_rotation": map[string]any{ + "sig": map[string]any{ + "enabled": true, + "cron": "0 0 1 * *", + }, + }, + } + + out, err := diff.Tree(source, target) + require.NoError(t, err) + require.Empty(t, out) + }) + + t.Run("ignores starting_from nested under servers", func(t *testing.T) { + source := models.Rfc7396PatchOperation{ + "servers": map[string]any{ + "demo": map[string]any{ + "key_rotation": map[string]any{ + "enc": map[string]any{ + "enabled": false, + "cron": "@monthly", + "starting_from": "2026-10-01T00:00:00Z", + }, + }, + }, + }, + } + target := models.Rfc7396PatchOperation{ + "servers": map[string]any{ + "demo": map[string]any{ + "key_rotation": map[string]any{ + "enc": map[string]any{ + "enabled": false, + "cron": "@monthly", + }, + }, + }, + }, + } + + out, err := diff.Tree(source, target) + require.NoError(t, err) + require.Empty(t, out) + }) + + t.Run("reports a cron change", func(t *testing.T) { + source := models.Rfc7396PatchOperation{ + "key_rotation": map[string]any{ + "sig": map[string]any{"enabled": true, "cron": "0 0 1 * *"}, + }, + } + target := models.Rfc7396PatchOperation{ + "key_rotation": map[string]any{ + "sig": map[string]any{"enabled": true, "cron": "@monthly"}, + }, + } + + out, err := diff.Tree(source, target) + require.NoError(t, err) + require.NotEmpty(t, out) + require.Contains(t, out, "0 0 1 * *") + require.Contains(t, out, "@monthly") + }) + + t.Run("does not ignore starting_from outside key_rotation", func(t *testing.T) { + source := models.Rfc7396PatchOperation{ + "clients": map[string]any{ + "demo": map[string]any{"starting_from": "2026-10-01T00:00:00Z"}, + }, + } + target := models.Rfc7396PatchOperation{ + "clients": map[string]any{ + "demo": map[string]any{"starting_from": "2027-10-01T00:00:00Z"}, + }, + } + + out, err := diff.Tree(source, target) + require.NoError(t, err) + require.NotEmpty(t, out) + require.Contains(t, out, "2026-10-01T00:00:00Z") + }) +} From 052dc2bb02e32132d5061755fb5efe49aae20800 Mon Sep 17 00:00:00 2001 From: Piotr Janus Date: Fri, 18 Sep 2026 15:57:36 +0200 Subject: [PATCH 5/5] docs: document key_rotation.yaml and the key_rotation filter --- README.md | 21 +++++++++++++++++++++ cmd/diff.go | 2 +- cmd/pull.go | 2 +- cmd/push.go | 12 ++++++------ 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 10998be..0319d16 100644 --- a/README.md +++ b/README.md @@ -239,6 +239,27 @@ map[string]any{ - "ciba_authentication_service": map[string]any{"type": string("mock")}, ``` +## Key rotation + +Automatic key rotation is configured per workspace in `workspaces//key_rotation.yaml`: + +```yaml +sig: + enabled: true + cron: "0 0 1 * *" + starting_from: "2026-10-01T00:00:00Z" # optional, write-only +enc: + enabled: false + cron: "0 0 1 * *" # required even when disabled +``` + +- `sig` (signing keys) and `enc` (encryption keys) are both optional. A use that is absent from the file is left untouched on the server, and `pull` omits a use that SecureAuth reports as never configured. +- `enabled` and `cron` are required for every use present in the file. SecureAuth validates the cron expression even when `enabled` is `false`, so disabling rotation still requires a valid one. +- `cron` uses [gorhill/cronexpr](https://github.com/gorhill/cronexpr) syntax: five fields (minute, hour, day of month, month, day of week), an optional sixth field for the year, or a seven-field form with seconds first. The descriptors `@yearly`, `@annually`, `@monthly`, `@weekly`, `@daily` and `@hourly` are supported, as are `L`, `W` and `#`. `@every` is not. +- `starting_from` is optional and write-only. SecureAuth never returns it, so `pull` never writes it and `diff` ignores it. It is honored only when it is in the future; a past value is ignored. +- `scheduled_at` is computed by SecureAuth, is read-only, and is rejected if present in the file. +- `--filter key_rotation` restricts a `push` or a `diff` to this file. It only applies in workspace mode: in tenant mode the whole workspace configuration, key rotation included, is selected with `--filter servers`. + ## Templates Templates are used to generate configuration files. They are using [Go template language](https://golang.org/pkg/text/template/). diff --git a/cmd/diff.go b/cmd/diff.go index a0ad937..cc7edb7 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -131,7 +131,7 @@ Example: --only-present`) diffCmd.PersistentFlags().StringSliceVar(&diffConfig.Filters, "filter", []string{}, `Restrict the comparison to selected top-level resources (comma-separated or repeated). Workspace resources: clients, idps, claims, custom_apps, gateways, policies, policy_execution_points, pools, scopes (alias of scopes_without_service), scripts, script_execution_points, - server_consent, servers_bindings, services, theme_binding, webhooks, + server_consent, servers_bindings, services, theme_binding, key_rotation, webhooks, ciba (alias of ciba_authentication_service) Tenant resources: pools, schemas, mfa_methods, themes, servers Reserved: root (only root-level tenant/workspace config, excluding nested resources) diff --git a/cmd/pull.go b/cmd/pull.go index 6bde4f7..14c1ec4 100644 --- a/cmd/pull.go +++ b/cmd/pull.go @@ -73,7 +73,7 @@ Example: --with-secrets`) pullCmd.PersistentFlags().StringSliceVar(&pullConfig.Filters, "filter", []string{}, `Restrict the pull to selected top-level resources (comma-separated or repeated). Workspace resources: clients, idps, claims, custom_apps, gateways, policies, policy_execution_points, pools, scopes (alias of scopes_without_service), scripts, script_execution_points, - server_consent, servers_bindings, services, theme_binding, webhooks, + server_consent, servers_bindings, services, theme_binding, key_rotation, webhooks, ciba (alias of ciba_authentication_service) Tenant resources: pools, schemas, mfa_methods, themes, servers Reserved: root (only root-level tenant/workspace config, excluding nested resources) diff --git a/cmd/push.go b/cmd/push.go index 32e4e36..a0b6601 100644 --- a/cmd/push.go +++ b/cmd/push.go @@ -99,11 +99,11 @@ Examples: }, } pushConfig struct { - DryRun bool - Out string - Mode string - Method string - Filters []string + DryRun bool + Out string + Mode string + Method string + Filters []string NoLocalValidate bool } ) @@ -135,7 +135,7 @@ Example: --no-validate`) pushCmd.PersistentFlags().StringSliceVar(&pushConfig.Filters, "filter", []string{}, `Restrict the push to selected top-level resources (comma-separated or repeated). Workspace resources: clients, idps, claims, custom_apps, gateways, policies, policy_execution_points, pools, scopes (alias of scopes_without_service), scripts, script_execution_points, - server_consent, servers_bindings, services, theme_binding, webhooks, + server_consent, servers_bindings, services, theme_binding, key_rotation, webhooks, ciba (alias of ciba_authentication_service) Tenant resources: pools, schemas, mfa_methods, themes, servers Reserved: root (only root-level tenant/workspace config, excluding nested resources)