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) 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/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 } 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") + }) +} 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/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) +} 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) + }) + } +}