From ab0b0c96ca9a76444b944c35cd1a9a3708a628f1 Mon Sep 17 00:00:00 2001
From: Tom Proctor <tomhjp@users.noreply.github.com>
Date: Tue, 17 May 2022 17:41:26 +0100
Subject: [PATCH] api: make ListPlugins parse only known plugin types (#15434)

---
 api/sys_plugins.go      | 17 ++++-----
 api/sys_plugins_test.go | 79 +++++++++++++++++++++++++++++++++++++++++
 changelog/15434.txt     |  3 ++
 3 files changed, 91 insertions(+), 8 deletions(-)
 create mode 100644 api/sys_plugins_test.go
 create mode 100644 changelog/15434.txt

diff --git a/api/sys_plugins.go b/api/sys_plugins.go
index 920af4c3c..004ee222b 100644
--- a/api/sys_plugins.go
+++ b/api/sys_plugins.go
@@ -100,23 +100,24 @@ func (c *Sys) ListPluginsWithContext(ctx context.Context, i *ListPluginsInput) (
 		PluginsByType: make(map[consts.PluginType][]string),
 	}
 	if i.Type == consts.PluginTypeUnknown {
-		for pluginTypeStr, pluginsRaw := range secret.Data {
-			pluginType, err := consts.ParsePluginType(pluginTypeStr)
-			if err != nil {
-				return nil, err
+		for _, pluginType := range consts.PluginTypes {
+			pluginsRaw, ok := secret.Data[pluginType.String()]
+			if !ok {
+				continue
 			}
 
 			pluginsIfc, ok := pluginsRaw.([]interface{})
 			if !ok {
-				return nil, fmt.Errorf("unable to parse plugins for %q type", pluginTypeStr)
+				return nil, fmt.Errorf("unable to parse plugins for %q type", pluginType.String())
 			}
 
-			plugins := make([]string, len(pluginsIfc))
-			for i, nameIfc := range pluginsIfc {
+			plugins := make([]string, 0, len(pluginsIfc))
+			for _, nameIfc := range pluginsIfc {
 				name, ok := nameIfc.(string)
 				if !ok {
+					continue
 				}
-				plugins[i] = name
+				plugins = append(plugins, name)
 			}
 			result.PluginsByType[pluginType] = plugins
 		}
diff --git a/api/sys_plugins_test.go b/api/sys_plugins_test.go
new file mode 100644
index 000000000..24295b6f2
--- /dev/null
+++ b/api/sys_plugins_test.go
@@ -0,0 +1,79 @@
+package api
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/hashicorp/vault/sdk/helper/consts"
+)
+
+func TestListPlugins(t *testing.T) {
+	mockVaultServer := httptest.NewServer(http.HandlerFunc(mockVaultHandler))
+	defer mockVaultServer.Close()
+
+	cfg := DefaultConfig()
+	cfg.Address = mockVaultServer.URL
+	client, err := NewClient(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	resp, err := client.Sys().ListPluginsWithContext(context.Background(), &ListPluginsInput{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expectedPlugins := map[consts.PluginType][]string{
+		consts.PluginTypeCredential: {"alicloud"},
+		consts.PluginTypeDatabase:   {"cassandra-database-plugin"},
+		consts.PluginTypeSecrets:    {"ad", "alicloud"},
+	}
+
+	for pluginType, expected := range expectedPlugins {
+		actualPlugins := resp.PluginsByType[pluginType]
+		if len(expected) != len(actualPlugins) {
+			t.Fatal("Wrong number of plugins", expected, actualPlugins)
+		}
+		for i := range actualPlugins {
+			if expected[i] != actualPlugins[i] {
+				t.Fatalf("Expected %q but got %q", expected[i], actualPlugins[i])
+			}
+		}
+	}
+}
+
+func mockVaultHandler(w http.ResponseWriter, _ *http.Request) {
+	_, _ = w.Write([]byte(listUntypedResponse))
+}
+
+const listUntypedResponse = `{
+  "request_id": "82601a91-cd7a-718f-feca-f573449cc1bb",
+  "lease_id": "",
+  "renewable": false,
+  "lease_duration": 0,
+  "data": {
+    "auth": [
+      "alicloud"
+    ],
+    "database": [
+      "cassandra-database-plugin"
+    ],
+    "secret": [
+      "ad",
+      "alicloud"
+    ],
+    "some_other_unexpected_key": [
+      {
+        "objectKey": "objectValue"
+      },
+      {
+        "arbitraryData": 7
+      }
+    ]
+  },
+  "wrap_info": null,
+  "warnings": null,
+  "auth": null
+}`
diff --git a/changelog/15434.txt b/changelog/15434.txt
new file mode 100644
index 000000000..565fb9e74
--- /dev/null
+++ b/changelog/15434.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+api: make ListPlugins parse only known plugin types
+```
\ No newline at end of file