Fix bug where updates to config would fail is password isn't provided (…

…#91) (#93)

* Fix bug where updates to config would fail is password isn't provided

* Make test clearer

plugin/backend_test.go

@@ -34,6 +34,7 @@ func TestBackend(t *testing.T) {
 	t.Run("write config", WriteConfig)
 	t.Run("read config", ReadConfig)
 	t.Run("delete config", DeleteConfig)
+	t.Run("update config", UpdateConfig)
 
 	// Plant a config for further testing.
 	t.Run("plant config", PlantConfig)
@@ -82,6 +83,45 @@ func WriteConfig(t *testing.T) {
 	}
 }
 
+func UpdateConfig(t *testing.T) {
+	req := &logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      configPath,
+		Storage:   testStorage,
+		Data: map[string]interface{}{
+			"binddn":                  "tester",
+			"password":                "pa$$w0rd",
+			"url":                     "ldap://138.91.247.105",
+			"userdn":                  "dc=example,dc=com",
+			"formatter":               "mycustom{{PASSWORD}}",
+			"last_rotation_tolerance": 10,
+		},
+	}
+	resp, err := testBackend.HandleRequest(testCtx, req)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatal(err)
+	}
+	if resp != nil {
+		t.Fatal("expected no response because Vault generally doesn't return it for posts")
+	}
+
+	req = &logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      configPath,
+		Storage:   testStorage,
+		Data: map[string]interface{}{
+			"certificate": validCertificate,
+		},
+	}
+	resp, err = testBackend.HandleRequest(testCtx, req)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatal(err)
+	}
+	if resp != nil {
+		t.Fatal("expected no response because Vault generally doesn't return it for posts")
+	}
+}
+
 func ReadConfig(t *testing.T) {
 	req := &logical.Request{
 		Operation: logical.ReadOperation,

plugin/checkout_handler.go

@@ -127,8 +127,8 @@ func (h *checkOutHandler) CheckIn(ctx context.Context, storage logical.Storage,
 }
 
 // LoadCheckOut returns either:
-//  - A *CheckOut and nil error if the serviceAccountName is currently managed by this engine.
-//  - A nil *Checkout and errNotFound if the serviceAccountName is not currently managed by this engine.
+//   - A *CheckOut and nil error if the serviceAccountName is currently managed by this engine.
+//   - A nil *Checkout and errNotFound if the serviceAccountName is not currently managed by this engine.
 func (h *checkOutHandler) LoadCheckOut(ctx context.Context, storage logical.Storage, serviceAccountName string) (*CheckOut, error) {
 	if ctx == nil {
 		return nil, errors.New("ctx must be provided")
@@ -174,9 +174,9 @@ func (h *checkOutHandler) Delete(ctx context.Context, storage logical.Storage, s
 
 // retrievePassword is a utility function for grabbing a service account's password from storage.
 // retrievePassword will return:
-//  - "password", nil if it was successfully able to retrieve the password.
-//  - errNotFound if there's no password presently.
-//  - Some other err if it was unable to complete successfully.
+//   - "password", nil if it was successfully able to retrieve the password.
+//   - errNotFound if there's no password presently.
+//   - Some other err if it was unable to complete successfully.
 func retrievePassword(ctx context.Context, storage logical.Storage, serviceAccountName string) (string, error) {
 	entry, err := storage.Get(ctx, passwordStoragePrefix+serviceAccountName)
 	if err != nil {

plugin/client/fieldregistry.go

@@ -9,14 +9,13 @@ import (
 //
 // Example: Accessing constants
 //
-//   FieldRegistry.AccountExpires
-//   FieldRegistry.BadPasswordCount
+//	FieldRegistry.AccountExpires
+//	FieldRegistry.BadPasswordCount
 //
 // Example: Utility methods
 //
-//   FieldRegistry.List()
-//   FieldRegistry.Parse("givenName")
-//
+//	FieldRegistry.List()
+//	FieldRegistry.Parse("givenName")
 var FieldRegistry = newFieldRegistry()
 
 // newFieldRegistry iterates through all the fields in the registry,

plugin/client/time.go

@@ -27,11 +27,11 @@ func ParseTicks(ticks string) (time.Time, error) {
 // TicksToTime converts an ActiveDirectory time in ticks to a time.
 // This algorithm is summarized as:
 //
-// 		Many dates are saved in Active Directory as Large Integer values.
-// 		These attributes represent dates as the number of 100-nanosecond intervals since 12:00 AM January 1, 1601.
-//		100-nanosecond intervals, equal to 0.0000001 seconds, are also called ticks.
-//		Dates in Active Directory are always saved in Coordinated Universal Time, or UTC.
-//		More: https://social.technet.microsoft.com/wiki/contents/articles/31135.active-directory-large-integer-attributes.aspx
+//	Many dates are saved in Active Directory as Large Integer values.
+//	These attributes represent dates as the number of 100-nanosecond intervals since 12:00 AM January 1, 1601.
+//	100-nanosecond intervals, equal to 0.0000001 seconds, are also called ticks.
+//	Dates in Active Directory are always saved in Coordinated Universal Time, or UTC.
+//	More: https://social.technet.microsoft.com/wiki/contents/articles/31135.active-directory-large-integer-attributes.aspx
 //
 // If we directly follow the above algorithm we encounter time.Duration limits of 290 years and int overflow issues.
 // Thus below, we carefully sidestep those.

plugin/path_config.go

@@ -100,11 +100,29 @@ func (b *backend) configFields() map[string]*framework.FieldSchema {
 }
 
 func (b *backend) configUpdateOperation(ctx context.Context, req *logical.Request, fieldData *framework.FieldData) (*logical.Response, error) {
+
+	conf, err := readConfig(ctx, req.Storage)
+	if err != nil {
+		return nil, err
+	}
+
+	if conf == nil {
+		conf = new(configuration)
+		conf.ADConf = new(client.ADConf)
+	}
+
+	// Use the existing ldap client config if it is set
+	var existing *ldaputil.ConfigEntry
+	if conf.ADConf != nil && conf.ADConf.ConfigEntry != nil {
+		existing = conf.ADConf.ConfigEntry
+	}
+
 	// Build and validate the ldap conf.
-	activeDirectoryConf, err := ldaputil.NewConfigEntry(nil, fieldData)
+	activeDirectoryConf, err := ldaputil.NewConfigEntry(existing, fieldData)
 	if err != nil {
 		return nil, err
 	}
+
 	if err := activeDirectoryConf.Validate(); err != nil {
 		return nil, err
 	}

plugin/path_config_test.go

@@ -2,9 +2,10 @@ package plugin
 
 import (
 	"context"
+	"testing"
+
 	"github.com/mitchellh/mapstructure"
 	"github.com/stretchr/testify/assert"
-	"testing"
 
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/logical"