From 9a0c5440929f04ee3af8f0c69c43d04e01af6a61 Mon Sep 17 00:00:00 2001
From: Joel Thompson <jathomps08@gmail.com>
Date: Mon, 17 Jul 2017 11:08:57 -0400
Subject: [PATCH] Properly store iam_server_id_header_value (#3014)

In auth/aws/config/client, when only the iam_server_id_header_value was
being updated on an existing config, it wouldn't get stored because I
was trying to avoid unnecessarily flushing the cache of AWS clients, and
the flag to not flush the cache also meant that the updated entry didn't
get written back to the storage. This now adds a new flag for when
other changes occur that don't require flushing the cache but do require
getting written to the storage. It also adds a test for this explicitly.

Fixes #3004
---
 builtin/credential/aws/path_config_client.go  |  7 +++-
 .../credential/aws/path_config_client_test.go | 33 +++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/builtin/credential/aws/path_config_client.go b/builtin/credential/aws/path_config_client.go
index df1a6234d611..9242ebd14716 100644
--- a/builtin/credential/aws/path_config_client.go
+++ b/builtin/credential/aws/path_config_client.go
@@ -150,7 +150,11 @@ func (b *backend) pathConfigClientCreateUpdate(
 		configEntry = &clientConfig{}
 	}
 
+	// changedCreds is whether we need to flush the cached AWS clients and store in the backend
 	changedCreds := false
+	// changedOtherConfig is whether other config has changed that requires storing in the backend
+	// but does not require flushing the cached clients
+	changedOtherConfig := false
 
 	accessKeyStr, ok := data.GetOk("access_key")
 	if ok {
@@ -213,6 +217,7 @@ func (b *backend) pathConfigClientCreateUpdate(
 		if configEntry.IAMServerIdHeaderValue != headerValStr.(string) {
 			// NOT setting changedCreds here, since this isn't really cached
 			configEntry.IAMServerIdHeaderValue = headerValStr.(string)
+			changedOtherConfig = true
 		}
 	} else if req.Operation == logical.CreateOperation {
 		configEntry.IAMServerIdHeaderValue = data.Get("iam_server_id_header_value").(string)
@@ -228,7 +233,7 @@ func (b *backend) pathConfigClientCreateUpdate(
 		return nil, err
 	}
 
-	if changedCreds || req.Operation == logical.CreateOperation {
+	if changedCreds || changedOtherConfig || req.Operation == logical.CreateOperation {
 		if err := req.Storage.Put(entry); err != nil {
 			return nil, err
 		}
diff --git a/builtin/credential/aws/path_config_client_test.go b/builtin/credential/aws/path_config_client_test.go
index 268571024041..215cee75e7e7 100644
--- a/builtin/credential/aws/path_config_client_test.go
+++ b/builtin/credential/aws/path_config_client_test.go
@@ -73,4 +73,37 @@ func TestBackend_pathConfigClient(t *testing.T) {
 		t.Fatalf("expected iam_server_id_header_value: '%#v'; returned iam_server_id_header_value: '%#v'",
 			data["iam_server_id_header_value"], resp.Data["iam_server_id_header_value"])
 	}
+
+	data = map[string]interface{}{
+		"iam_server_id_header_value": "vault_server_identification_2718281",
+	}
+	resp, err = b.HandleRequest(&logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      "config/client",
+		Data:      data,
+		Storage:   storage,
+	})
+
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp != nil && resp.IsError() {
+		t.Fatal("failed to update the client config entry")
+	}
+
+	resp, err = b.HandleRequest(&logical.Request{
+		Operation: logical.ReadOperation,
+		Path:      "config/client",
+		Storage:   storage,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp == nil || resp.IsError() {
+		t.Fatal("failed to read the client config entry")
+	}
+	if resp.Data["iam_server_id_header_value"] != data["iam_server_id_header_value"] {
+		t.Fatalf("expected iam_server_id_header_value: '%#v'; returned iam_server_id_header_value: '%#v'",
+			data["iam_server_id_header_value"], resp.Data["iam_server_id_header_value"])
+	}
 }