config: set default length only if password policy is missing (#85)

* config: set default length only if password policy is missing

* make fmt

* update test comment

* add changelog, remove unneeded ok check

CHANGELOG.md

@@ -0,0 +1,5 @@
+## Unreleased
+
+### IMPROVEMENTS:
+
+* config: set default length only if password policy is missing [GH-85](https://github.com/hashicorp/vault-plugin-secrets-ad/pull/85)

go.mod

@@ -14,5 +14,6 @@ require (
 	github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d // indirect
 	github.com/mitchellh/mapstructure v1.4.2
 	github.com/patrickmn/go-cache v2.1.0+incompatible
+	github.com/stretchr/testify v1.7.0
 	golang.org/x/text v0.3.7
 )

plugin/path_config.go

@@ -114,10 +114,19 @@ func (b *backend) configUpdateOperation(ctx context.Context, req *logical.Reques
 	maxTTL := fieldData.Get("max_ttl").(int)
 	lastRotationTolerance := fieldData.Get("last_rotation_tolerance").(int)
 
-	length := fieldData.Get("length").(int)
-	formatter := fieldData.Get("formatter").(string)
 	passwordPolicy := fieldData.Get("password_policy").(string)
 
+	var length int
+	if lengthRaw, ok := fieldData.GetOk("length"); ok {
+		length = lengthRaw.(int)
+	} else if passwordPolicy == "" {
+		// If neither the length nor a password policy was provided, fall back
+		// to the length's field data default value.
+		length = fieldData.Get("length").(int)
+	}
+
+	formatter := fieldData.Get("formatter").(string)
+
 	if pre111Val, ok := fieldData.GetOk("use_pre111_group_cn_behavior"); ok {
 		activeDirectoryConf.UsePre111GroupCNBehavior = new(bool)
 		*activeDirectoryConf.UsePre111GroupCNBehavior = pre111Val.(bool)

plugin/path_config_test.go

@@ -2,6 +2,8 @@ package plugin
 
 import (
 	"context"
+	"github.com/mitchellh/mapstructure"
+	"github.com/stretchr/testify/assert"
 	"testing"
 
 	"github.com/hashicorp/vault/sdk/framework"
@@ -88,3 +90,100 @@ func TestCacheReader(t *testing.T) {
 		t.Fatal("config should be nil")
 	}
 }
+
+func TestConfig_PasswordLength(t *testing.T) {
+
+	// we should start with no config
+	config, err := readConfig(ctx, storage)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if config != nil {
+		t.Fatal("config should be nil")
+	}
+
+	req := &logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      configPath,
+		Storage:   storage,
+	}
+
+	tests := []struct {
+		name         string
+		rawFieldData map[string]interface{}
+		wantErr      bool
+	}{
+		{
+			"length provided",
+			map[string]interface{}{
+				"length": 32,
+			},
+			false,
+		},
+		{
+			"password policy provided",
+			map[string]interface{}{
+				"password_policy": "foo",
+			},
+			false,
+		},
+		{
+			"no length or password policy provided",
+			nil,
+			false,
+		},
+		{
+			"both length and password policy provided",
+			map[string]interface{}{
+				"password_policy": "foo",
+				"length":          32,
+			},
+			true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Start common config fields and append what we need to test against
+			fieldData := &framework.FieldData{
+				Schema: testBackend.pathConfig().Fields,
+				Raw: map[string]interface{}{
+					"binddn":   "tester",
+					"password": "pa$$w0rd",
+					"urls":     "ldap://138.91.247.105",
+					"userdn":   "example,com",
+				},
+			}
+
+			for k, v := range tt.rawFieldData {
+				fieldData.Raw[k] = v
+			}
+
+			_, err = testBackend.configUpdateOperation(ctx, req, fieldData)
+			assert.Equal(t, tt.wantErr, err != nil)
+
+			if tt.wantErr && err != nil {
+				return
+			}
+
+			config, err := readConfig(ctx, storage)
+			assert.NoError(t, err)
+
+			var actual map[string]interface{}
+
+			cfg := &mapstructure.DecoderConfig{
+				Result:  &actual,
+				TagName: "json",
+			}
+			decoder, err := mapstructure.NewDecoder(cfg)
+			assert.NoError(t, err)
+			err = decoder.Decode(config.PasswordConf)
+			assert.NoError(t, err)
+
+			for k, v := range tt.rawFieldData {
+				assert.Contains(t, actual, k)
+				assert.Equal(t, actual[k], v)
+			}
+		})
+	}
+}