Normalize plugin_name option for mount and enable-auth (#3202)

api/sys_auth.go

@@ -82,10 +82,15 @@ func (c *Sys) DisableAuth(path string) error {
 // documentation. Please refer to that documentation for more details.
 
 type EnableAuthOptions struct {
-	Type        string `json:"type" structs:"type"`
-	Description string `json:"description" structs:"description"`
-	Local       bool   `json:"local" structs:"local"`
-	PluginName  string `json:"plugin_name,omitempty" structs:"plugin_name,omitempty" mapstructure:"plugin_name"`
+	Type        string          `json:"type" structs:"type"`
+	Description string          `json:"description" structs:"description"`
+	Config      AuthConfigInput `json:"config" structs:"config"`
+	Local       bool            `json:"local" structs:"local"`
+	PluginName  string          `json:"plugin_name,omitempty" structs:"plugin_name,omitempty"`
+}
+
+type AuthConfigInput struct {
+	PluginName string `json:"plugin_name,omitempty" structs:"plugin_name,omitempty" mapstructure:"plugin_name"`
 }
 
 type AuthMount struct {

api/sys_mounts.go

@@ -124,6 +124,7 @@ type MountInput struct {
 	Description string           `json:"description" structs:"description"`
 	Config      MountConfigInput `json:"config" structs:"config"`
 	Local       bool             `json:"local" structs:"local"`
+	PluginName  string           `json:"plugin_name,omitempty" structs:"plugin_name"`
 }
 
 type MountConfigInput struct {

command/auth_enable.go

@@ -57,8 +57,10 @@ func (c *AuthEnableCommand) Run(args []string) int {
 	if err := client.Sys().EnableAuthWithOptions(path, &api.EnableAuthOptions{
 		Type:        authType,
 		Description: description,
-		PluginName:  pluginName,
-		Local:       local,
+		Config: api.AuthConfigInput{
+			PluginName: pluginName,
+		},
+		Local: local,
 	}); err != nil {
 		c.Ui.Error(fmt.Sprintf(
 			"Error: %s", err))

vault/logical_system.go

@@ -291,6 +291,10 @@ func NewSystemBackend(core *Core) *SystemBackend {
 						Default:     false,
 						Description: strings.TrimSpace(sysHelp["mount_local"][0]),
 					},
+					"plugin_name": &framework.FieldSchema{
+						Type:        framework.TypeString,
+						Description: strings.TrimSpace(sysHelp["mount_plugin_name"][0]),
+					},
 				},
 
 				Callbacks: map[logical.Operation]framework.OperationFunc{
@@ -493,15 +497,19 @@ func NewSystemBackend(core *Core) *SystemBackend {
 						Type:        framework.TypeString,
 						Description: strings.TrimSpace(sysHelp["auth_desc"][0]),
 					},
-					"plugin_name": &framework.FieldSchema{
-						Type:        framework.TypeString,
-						Description: strings.TrimSpace(sysHelp["auth_plugin"][0]),
+					"config": &framework.FieldSchema{
+						Type:        framework.TypeMap,
+						Description: strings.TrimSpace(sysHelp["auth_config"][0]),
 					},
 					"local": &framework.FieldSchema{
 						Type:        framework.TypeBool,
 						Default:     false,
 						Description: strings.TrimSpace(sysHelp["mount_local"][0]),
 					},
+					"plugin_name": &framework.FieldSchema{
+						Type:        framework.TypeString,
+						Description: strings.TrimSpace(sysHelp["auth_plugin"][0]),
+					},
 				},
 
 				Callbacks: map[logical.Operation]framework.OperationFunc{
@@ -1256,6 +1264,7 @@ func (b *SystemBackend) handleMount(
 	path := data.Get("path").(string)
 	logicalType := data.Get("type").(string)
 	description := data.Get("description").(string)
+	pluginName := data.Get("plugin_name").(string)
 
 	path = sanitizeMountPath(path)
 
@@ -1310,9 +1319,19 @@ func (b *SystemBackend) handleMount(
 			logical.ErrInvalidRequest
 	}
 
-	// Only set plugin-name if mount is of type plugin
-	if logicalType == "plugin" && apiConfig.PluginName != "" {
-		config.PluginName = apiConfig.PluginName
+	// Only set plugin-name if mount is of type plugin, with apiConfig.PluginName
+	// option taking precedence.
+	if logicalType == "plugin" {
+		switch {
+		case apiConfig.PluginName != "":
+			config.PluginName = apiConfig.PluginName
+		case pluginName != "":
+			config.PluginName = pluginName
+		default:
+			return logical.ErrorResponse(
+					"plugin_name must be provided for plugin backend"),
+				logical.ErrInvalidRequest
+		}
 	}
 
 	// Copy over the force no cache if set
@@ -1754,10 +1773,31 @@ func (b *SystemBackend) handleEnableAuth(
 	pluginName := data.Get("plugin_name").(string)
 
 	var config MountConfig
+	var apiConfig APIMountConfig
 
-	// Only set plugin name if mount is of type plugin
-	if logicalType == "plugin" && pluginName != "" {
-		config.PluginName = pluginName
+	configMap := data.Get("config").(map[string]interface{})
+	if configMap != nil && len(configMap) != 0 {
+		err := mapstructure.Decode(configMap, &apiConfig)
+		if err != nil {
+			return logical.ErrorResponse(
+					"unable to convert given auth config information"),
+				logical.ErrInvalidRequest
+		}
+	}
+
+	// Only set plugin name if mount is of type plugin, with apiConfig.PluginName
+	// option taking precedence.
+	if logicalType == "plugin" {
+		switch {
+		case apiConfig.PluginName != "":
+			config.PluginName = apiConfig.PluginName
+		case pluginName != "":
+			config.PluginName = pluginName
+		default:
+			return logical.ErrorResponse(
+					"plugin_name must be provided for plugin backend"),
+				logical.ErrInvalidRequest
+		}
 	}
 
 	if logicalType == "" {
@@ -2541,6 +2581,11 @@ and max_lease_ttl.`,
 and is unaffected by replication.`,
 	},
 
+	"mount_plugin_name": {
+		`Name of the plugin to mount based from the name registered 
+in the plugin catalog.`,
+	},
+
 	"tune_default_lease_ttl": {
 		`The default lease TTL for this mount.`,
 	},
@@ -2677,6 +2722,10 @@ Example: you might have an OAuth backend for GitHub, and one for Google Apps.
 		"",
 	},
 
+	"auth_config": {
+		`Configuration for this mount, such as plugin_name.`,
+	},
+
 	"auth_plugin": {
 		`Name of the auth plugin to use based from the name in the plugin catalog.`,
 		"",

vault/logical_system_integ_test.go

@@ -15,12 +15,12 @@ import (
 )
 
 func TestSystemBackend_Plugin_secret(t *testing.T) {
-	cluster := testSystemBackendMock(t, 1, logical.TypeLogical)
+	cluster := testSystemBackendMock(t, 2, logical.TypeLogical)
 	defer cluster.Cleanup()
 }
 
 func TestSystemBackend_Plugin_auth(t *testing.T) {
-	cluster := testSystemBackendMock(t, 1, logical.TypeCredential)
+	cluster := testSystemBackendMock(t, 2, logical.TypeCredential)
 	defer cluster.Cleanup()
 }
 
@@ -148,12 +148,18 @@ func testSystemBackendMock(t *testing.T, numMounts int, backendType logical.Back
 	case logical.TypeLogical:
 		vault.TestAddTestPlugin(t, core.Core, "mock-plugin", "TestBackend_PluginMainLogical")
 		for i := 0; i < numMounts; i++ {
-			resp, err := client.Logical().Write(fmt.Sprintf("sys/mounts/mock-%d", i), map[string]interface{}{
+			// Alternate input styles for plugin_name on every other mount
+			options := map[string]interface{}{
 				"type": "plugin",
-				"config": map[string]interface{}{
+			}
+			if (i+1)%2 == 0 {
+				options["config"] = map[string]interface{}{
 					"plugin_name": "mock-plugin",
-				},
-			})
+				}
+			} else {
+				options["plugin_name"] = "mock-plugin"
+			}
+			resp, err := client.Logical().Write(fmt.Sprintf("sys/mounts/mock-%d", i), options)
 			if err != nil {
 				t.Fatalf("err: %v", err)
 			}
@@ -164,10 +170,18 @@ func testSystemBackendMock(t *testing.T, numMounts int, backendType logical.Back
 	case logical.TypeCredential:
 		vault.TestAddTestPlugin(t, core.Core, "mock-plugin", "TestBackend_PluginMainCredentials")
 		for i := 0; i < numMounts; i++ {
-			resp, err := client.Logical().Write(fmt.Sprintf("sys/auth/mock-%d", i), map[string]interface{}{
-				"type":        "plugin",
-				"plugin_name": "mock-plugin",
-			})
+			// Alternate input styles for plugin_name on every other mount
+			options := map[string]interface{}{
+				"type": "plugin",
+			}
+			if (i+1)%2 == 0 {
+				options["config"] = map[string]interface{}{
+					"plugin_name": "mock-plugin",
+				}
+			} else {
+				options["plugin_name"] = "mock-plugin"
+			}
+			resp, err := client.Logical().Write(fmt.Sprintf("sys/auth/mock-%d", i), options)
 			if err != nil {
 				t.Fatalf("err: %v", err)
 			}

website/source/api/system/auth.html.md

@@ -74,16 +74,25 @@ For example, mounting the "foo" auth backend will make it accessible at
 - `type` `(string: <required>)` – Specifies the name of the authentication
   backend type, such as "github" or "token".
 
+- `config` `(map<string|string>: nil)` – Specifies configuration options for
+  this mount. These are the possible values:
+
+    - `plugin_name`
+
+    The plugin_name can be provided in the config map or as a top-level option, 
+    with the former taking precedence.
+
+- `plugin_name` `(string: "")` – Specifies the name of the auth plugin to
+  use based from the name in the plugin catalog. Applies only to plugin
+  backends.
+
 Additionally, the following options are allowed in Vault open-source, but
 relevant functionality is only supported in Vault Enterprise:
 
 - `local` `(bool: false)` – Specifies if the auth backend is a local mount
   only. Local mounts are not replicated nor (if a secondary) removed by
   replication.
 
-- `plugin_name` `(string: "")` – Specifies the name of the auth plugin to
-  use based from the name in the plugin catalog.
-
 ### Sample Payload
 
 ```json

website/source/api/system/mounts.html.md

@@ -74,16 +74,22 @@ This endpoint mounts a new secret backend at the given path.
   mount.
 
 - `config` `(map<string|string>: nil)` – Specifies configuration options for
-  this mount. This is an object with three possible values:
+  this mount. This is an object with four possible values:
 
     - `default_lease_ttl`
     - `max_lease_ttl`
     - `force_no_cache`
     - `plugin_name`
 
-    These control the default and maximum lease time-to-live, and force
-    disabling backend caching respectively. If set on a specific mount, this
-    overrides the global defaults.
+    These control the default and maximum lease time-to-live, force
+    disabling backend caching, and option plugin name for plugin backends 
+    respectively. The first three options override the global defaults if
+    set on a specific mount. The plugin_name can be provided in the config
+    map or as a top-level option, with the former taking precedence.
+
+- `plugin_name` `(string: "")` – Specifies the name of the plugin to
+  use based from the name in the plugin catalog. Applies only to plugin
+  backends.
 
 Additionally, the following options are allowed in Vault open-source, but 
 relevant functionality is only supported in Vault Enterprise: