Allow WIF and rotation parameters to be set independently (#14414) (#14713)

* allowing WIF and rotation parameters to be set independently

* adding CL entry

* VAULT-42211 allowing independently setting of parameter for client/config endpoint

* introducing logic for identity token and rotation parameter detection

* moving the detectection change logic to corresponding packages

* sdk: add rotation and wif helpers

* changelog

* changelog updates

---------

Co-authored-by: John-Michael Faircloth <fairclothjm@users.noreply.github.com>
Co-authored-by: Martin Hristov <mhristov@hashicorp.com>

builtin/credential/aws/path_config_client.go

@@ -368,14 +368,26 @@ func (b *backend) pathConfigClientCreateUpdate(ctx context.Context, req *logical
 		configEntry.RoleARN = data.Get("role_arn").(string)
 	}
 
+	// Checking if identity_token_ttl is actually changed, no need to flush the cache if it is not
+	previousIdentityParams := configEntry.PluginIdentityTokenParams
 	if err := configEntry.ParsePluginIdentityTokenFields(data); err != nil {
 		return logical.ErrorResponse(err.Error()), nil
 	}
 
+	if !previousIdentityParams.Equals(configEntry.PluginIdentityTokenParams) {
+		changedCreds = true
+	}
+
+	// Checking if any for the rotation parameters has been modified, if yes, we set "changedOtherConfig" to true
+	previousRotationParams := configEntry.AutomatedRotationParams
 	if err := configEntry.ParseAutomatedRotationFields(data); err != nil {
 		return logical.ErrorResponse(err.Error()), nil
 	}
 
+	if !previousRotationParams.Equals(configEntry.AutomatedRotationParams) {
+		changedOtherConfig = true
+	}
+
 	// handle mutual exclusivity
 	if configEntry.IdentityTokenAudience != "" && configEntry.AccessKey != "" {
 		return logical.ErrorResponse("only one of 'access_key' or 'identity_token_audience' can be set"), nil

builtin/credential/aws/path_config_client_test.go

@@ -187,3 +187,39 @@ func (d testSystemView) RegisterRotationJob(_ context.Context, _ *rotation.Rotat
 func (d testSystemView) DeregisterRotationJob(_ context.Context, _ *rotation.RotationJobDeregisterRequest) error {
 	return nil
 }
+
+// TestBackend_PathConfigClient_RotationParameters tests that configuration
+// of root creds rotation returns an immediate error.
+func TestBackend_PathConfigClient_RotationParameters(t *testing.T) {
+	config := logical.TestBackendConfig()
+	config.StorageView = &logical.InmemStorage{}
+	config.System = &testSystemView{}
+
+	b, err := Backend(config)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = b.Setup(context.Background(), config)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	configData := map[string]interface{}{
+		"disable_automated_rotation": "false",
+		"rotation_schedule":          "0 2 1-7 * TUE",
+		"rotation_window":            "1h",
+	}
+
+	configReq := &logical.Request{
+		Operation: logical.UpdateOperation,
+		Storage:   config.StorageView,
+		Path:      "config/client",
+		Data:      configData,
+	}
+
+	resp, err := b.HandleRequest(context.Background(), configReq)
+	assert.NoError(t, err)
+	assert.NotNil(t, resp)
+	assert.ErrorContains(t, resp.Error(), automatedrotationutil.ErrRotationManagerUnsupported.Error())
+}

changelog/_14414.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+auth/aws: fix bug where rotation and wif config updates were not persisted to storage
+```
+```release-note:improvement
+sdk: add WIF and rotation helpers for checking if params were updated to allow
+the consumer to know when changes need to be persisted to storage
+```

sdk/helper/automatedrotationutil/fields.go

@@ -250,3 +250,9 @@ func AddAutomatedRotationFieldsWithGroup(m map[string]*framework.FieldSchema, gr
 func AddAutomatedRotationFields(m map[string]*framework.FieldSchema) {
 	AddAutomatedRotationFieldsWithGroup(m, "default")
 }
+
+// Equals returns true if the automated rotation parameters match the other instance.
+// Useful for detecting configuration changes after parsing new field data.
+func (p *AutomatedRotationParams) Equals(other AutomatedRotationParams) bool {
+	return *p == other
+}

sdk/helper/automatedrotationutil/fields_test.go

@@ -525,3 +525,222 @@ func TestAddAutomatedRotationFields(t *testing.T) {
 		})
 	}
 }
+
+func TestAutomatedRotationParams_Equals(t *testing.T) {
+	testcases := []struct {
+		name     string
+		p1       AutomatedRotationParams
+		p2       AutomatedRotationParams
+		expected bool
+	}{
+		{
+			name: "equal-all-fields",
+			p1: AutomatedRotationParams{
+				RotationSchedule:         "*/15 * * * *",
+				RotationWindow:           60 * time.Second,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "policy1",
+			},
+			p2: AutomatedRotationParams{
+				RotationSchedule:         "*/15 * * * *",
+				RotationWindow:           60 * time.Second,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "policy1",
+			},
+			expected: true,
+		},
+		{
+			name: "equal-zero-values",
+			p1: AutomatedRotationParams{
+				RotationSchedule:         "",
+				RotationWindow:           0,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "",
+			},
+			p2: AutomatedRotationParams{
+				RotationSchedule:         "",
+				RotationWindow:           0,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "",
+			},
+			expected: true,
+		},
+		{
+			name: "different-schedule",
+			p1: AutomatedRotationParams{
+				RotationSchedule:         "*/15 * * * *",
+				RotationWindow:           60 * time.Second,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "policy1",
+			},
+			p2: AutomatedRotationParams{
+				RotationSchedule:         "*/30 * * * *",
+				RotationWindow:           60 * time.Second,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "policy1",
+			},
+			expected: false,
+		},
+		{
+			name: "different-window",
+			p1: AutomatedRotationParams{
+				RotationSchedule:         "*/15 * * * *",
+				RotationWindow:           60 * time.Second,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "policy1",
+			},
+			p2: AutomatedRotationParams{
+				RotationSchedule:         "*/15 * * * *",
+				RotationWindow:           120 * time.Second,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "policy1",
+			},
+			expected: false,
+		},
+		{
+			name: "different-period",
+			p1: AutomatedRotationParams{
+				RotationSchedule:         "",
+				RotationWindow:           0,
+				RotationPeriod:           10 * time.Second,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "policy1",
+			},
+			p2: AutomatedRotationParams{
+				RotationSchedule:         "",
+				RotationWindow:           0,
+				RotationPeriod:           20 * time.Second,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "policy1",
+			},
+			expected: false,
+		},
+		{
+			name: "different-disable-flag",
+			p1: AutomatedRotationParams{
+				RotationSchedule:         "*/15 * * * *",
+				RotationWindow:           60 * time.Second,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "policy1",
+			},
+			p2: AutomatedRotationParams{
+				RotationSchedule:         "*/15 * * * *",
+				RotationWindow:           60 * time.Second,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: true,
+				RotationPolicy:           "policy1",
+			},
+			expected: false,
+		},
+		{
+			name: "different-policy",
+			p1: AutomatedRotationParams{
+				RotationSchedule:         "*/15 * * * *",
+				RotationWindow:           60 * time.Second,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "policy1",
+			},
+			p2: AutomatedRotationParams{
+				RotationSchedule:         "*/15 * * * *",
+				RotationWindow:           60 * time.Second,
+				RotationPeriod:           0,
+				DisableAutomatedRotation: false,
+				RotationPolicy:           "policy2",
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range testcases {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.p1.Equals(tt.p2)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestAutomatedRotationParams_Equals_Embedded(t *testing.T) {
+	// Test with embedded struct
+	type ConfigWithRotationParams struct {
+		Name string
+		AutomatedRotationParams
+	}
+
+	testcases := []struct {
+		name     string
+		c1       ConfigWithRotationParams
+		c2       ConfigWithRotationParams
+		expected bool
+	}{
+		{
+			name: "embedded-equal",
+			c1: ConfigWithRotationParams{
+				Name: "config1",
+				AutomatedRotationParams: AutomatedRotationParams{
+					RotationSchedule:         "*/15 * * * *",
+					RotationWindow:           60 * time.Second,
+					RotationPeriod:           0,
+					DisableAutomatedRotation: false,
+					RotationPolicy:           "policy1",
+				},
+			},
+			c2: ConfigWithRotationParams{
+				Name: "config2", // Different name, but we only compare AutomatedRotationParams
+				AutomatedRotationParams: AutomatedRotationParams{
+					RotationSchedule:         "*/15 * * * *",
+					RotationWindow:           60 * time.Second,
+					RotationPeriod:           0,
+					DisableAutomatedRotation: false,
+					RotationPolicy:           "policy1",
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "embedded-different",
+			c1: ConfigWithRotationParams{
+				Name: "config1",
+				AutomatedRotationParams: AutomatedRotationParams{
+					RotationSchedule:         "*/15 * * * *",
+					RotationWindow:           60 * time.Second,
+					RotationPeriod:           0,
+					DisableAutomatedRotation: false,
+					RotationPolicy:           "policy1",
+				},
+			},
+			c2: ConfigWithRotationParams{
+				Name: "config1",
+				AutomatedRotationParams: AutomatedRotationParams{
+					RotationSchedule:         "*/30 * * * *",
+					RotationWindow:           60 * time.Second,
+					RotationPeriod:           0,
+					DisableAutomatedRotation: false,
+					RotationPolicy:           "policy1",
+				},
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range testcases {
+		t.Run(tt.name, func(t *testing.T) {
+			// Test comparing the embedded fields directly
+			result := tt.c1.AutomatedRotationParams.Equals(tt.c2.AutomatedRotationParams)
+			assert.Equal(t, tt.expected, result)
+
+			// Test using method promotion
+			result2 := tt.c1.Equals(tt.c2.AutomatedRotationParams)
+			assert.Equal(t, tt.expected, result2)
+		})
+	}
+}

sdk/helper/pluginidentityutil/fields.go

@@ -75,3 +75,9 @@ func AddPluginIdentityTokenFieldsWithGroup(m map[string]*framework.FieldSchema,
 func AddPluginIdentityTokenFields(m map[string]*framework.FieldSchema) {
 	AddPluginIdentityTokenFieldsWithGroup(m, "default")
 }
+
+// Equals returns true if the plugin identity token parameters match the other instance.
+// Useful for detecting configuration changes after parsing new field data.
+func (p *PluginIdentityTokenParams) Equals(other PluginIdentityTokenParams) bool {
+	return *p == other
+}

sdk/helper/pluginidentityutil/fields_test.go

@@ -232,3 +232,144 @@ func TestAddPluginIdentityTokenFields(t *testing.T) {
 		})
 	}
 }
+
+func TestPluginIdentityTokenParams_Equals(t *testing.T) {
+	testcases := []struct {
+		name     string
+		p1       PluginIdentityTokenParams
+		p2       PluginIdentityTokenParams
+		expected bool
+	}{
+		{
+			name: "equal-all-fields",
+			p1: PluginIdentityTokenParams{
+				IdentityTokenTTL:      10 * time.Second,
+				IdentityTokenAudience: "test-aud",
+			},
+			p2: PluginIdentityTokenParams{
+				IdentityTokenTTL:      10 * time.Second,
+				IdentityTokenAudience: "test-aud",
+			},
+			expected: true,
+		},
+		{
+			name: "equal-zero-values",
+			p1: PluginIdentityTokenParams{
+				IdentityTokenTTL:      0,
+				IdentityTokenAudience: "",
+			},
+			p2: PluginIdentityTokenParams{
+				IdentityTokenTTL:      0,
+				IdentityTokenAudience: "",
+			},
+			expected: true,
+		},
+		{
+			name: "different-ttl",
+			p1: PluginIdentityTokenParams{
+				IdentityTokenTTL:      10 * time.Second,
+				IdentityTokenAudience: "test-aud",
+			},
+			p2: PluginIdentityTokenParams{
+				IdentityTokenTTL:      20 * time.Second,
+				IdentityTokenAudience: "test-aud",
+			},
+			expected: false,
+		},
+		{
+			name: "different-audience",
+			p1: PluginIdentityTokenParams{
+				IdentityTokenTTL:      10 * time.Second,
+				IdentityTokenAudience: "test-aud",
+			},
+			p2: PluginIdentityTokenParams{
+				IdentityTokenTTL:      10 * time.Second,
+				IdentityTokenAudience: "different-aud",
+			},
+			expected: false,
+		},
+		{
+			name: "different-both",
+			p1: PluginIdentityTokenParams{
+				IdentityTokenTTL:      10 * time.Second,
+				IdentityTokenAudience: "test-aud",
+			},
+			p2: PluginIdentityTokenParams{
+				IdentityTokenTTL:      20 * time.Second,
+				IdentityTokenAudience: "different-aud",
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range testcases {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.p1.Equals(tt.p2)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestPluginIdentityTokenParams_Equals_Embedded(t *testing.T) {
+	// Test with embedded struct
+	type ConfigWithIdentityParams struct {
+		Name string
+		PluginIdentityTokenParams
+	}
+
+	testcases := []struct {
+		name     string
+		c1       ConfigWithIdentityParams
+		c2       ConfigWithIdentityParams
+		expected bool
+	}{
+		{
+			name: "embedded-equal",
+			c1: ConfigWithIdentityParams{
+				Name: "config1",
+				PluginIdentityTokenParams: PluginIdentityTokenParams{
+					IdentityTokenTTL:      10 * time.Second,
+					IdentityTokenAudience: "test-aud",
+				},
+			},
+			c2: ConfigWithIdentityParams{
+				Name: "config2", // Different name, but we only compare PluginIdentityTokenParams
+				PluginIdentityTokenParams: PluginIdentityTokenParams{
+					IdentityTokenTTL:      10 * time.Second,
+					IdentityTokenAudience: "test-aud",
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "embedded-different",
+			c1: ConfigWithIdentityParams{
+				Name: "config1",
+				PluginIdentityTokenParams: PluginIdentityTokenParams{
+					IdentityTokenTTL:      10 * time.Second,
+					IdentityTokenAudience: "test-aud",
+				},
+			},
+			c2: ConfigWithIdentityParams{
+				Name: "config1",
+				PluginIdentityTokenParams: PluginIdentityTokenParams{
+					IdentityTokenTTL:      20 * time.Second,
+					IdentityTokenAudience: "test-aud",
+				},
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range testcases {
+		t.Run(tt.name, func(t *testing.T) {
+			// Test comparing the embedded fields directly
+			result := tt.c1.PluginIdentityTokenParams.Equals(tt.c2.PluginIdentityTokenParams)
+			assert.Equal(t, tt.expected, result)
+
+			// Test using method promotion
+			result2 := tt.c1.Equals(tt.c2.PluginIdentityTokenParams)
+			assert.Equal(t, tt.expected, result2)
+		})
+	}
+}