fix regression on Core.pluginDirectory when backend is builtin (#15027)

* fix dev-plugin-dir when backend is builtin

* use builtinRegistry.Contains

* revert aa76337

* use correct plugin type for logical backend after revert

* fix factory func default setting after revert

* add ut coverage for builtin plugin with plugin directory set

* add coverage for secrets plugin type

* use totp in tests to avoid test import cycle in ssh package

* use nomad in tests to avoid test import cycle

* remove secrets mount tests due to unavoidable test import cycle

vault/auth.go

@@ -915,7 +915,18 @@ func (c *Core) newCredentialBackend(ctx context.Context, entry *MountEntry, sysV
 
 	f, ok := c.credentialBackends[t]
 	if !ok {
-		f = wrapFactoryCheckPerms(c, plugin.Factory)
+		plug, err := c.pluginCatalog.Get(ctx, entry.Type, consts.PluginTypeCredential)
+		if err != nil {
+			return nil, err
+		}
+		if plug == nil {
+			return nil, fmt.Errorf("%w: %s", ErrPluginNotFound, entry.Type)
+		}
+
+		f = plugin.Factory
+		if !plug.Builtin {
+			f = wrapFactoryCheckPerms(c, plugin.Factory)
+		}
 	}
 
 	// Set up conf to pass in plugin_name

vault/auth_test.go

@@ -141,6 +141,28 @@ func TestCore_DefaultAuthTable(t *testing.T) {
 	}
 }
 
+func TestCore_BuiltinRegistry(t *testing.T) {
+	conf := &CoreConfig{
+		// set PluginDirectory and ensure that vault doesn't expect approle to
+		// be there when we are mounting the builtin approle
+		PluginDirectory: "/Users/foo",
+
+		DisableMlock:    true,
+		BuiltinRegistry: NewMockBuiltinRegistry(),
+	}
+	c, _, _ := TestCoreUnsealedWithConfig(t, conf)
+
+	me := &MountEntry{
+		Table: credentialTableType,
+		Path:  "approle/",
+		Type:  "approle",
+	}
+	err := c.enableCredential(namespace.RootContext(nil), me)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+}
+
 func TestCore_EnableCredential(t *testing.T) {
 	c, keys, _ := TestCoreUnsealed(t)
 	c.credentialBackends["noop"] = func(context.Context, *logical.BackendConfig) (logical.Backend, error) {

vault/mount.go

@@ -1395,7 +1395,18 @@ func (c *Core) newLogicalBackend(ctx context.Context, entry *MountEntry, sysView
 
 	f, ok := c.logicalBackends[t]
 	if !ok {
-		f = wrapFactoryCheckPerms(c, plugin.Factory)
+		plug, err := c.pluginCatalog.Get(ctx, entry.Type, consts.PluginTypeSecrets)
+		if err != nil {
+			return nil, err
+		}
+		if plug == nil {
+			return nil, fmt.Errorf("%w: %s", ErrPluginNotFound, entry.Type)
+		}
+
+		f = plugin.Factory
+		if !plug.Builtin {
+			f = wrapFactoryCheckPerms(c, plugin.Factory)
+		}
 	}
 
 	// Set up conf to pass in plugin_name

vault/testing.go

@@ -33,6 +33,7 @@ import (
 	raftlib "github.com/hashicorp/raft"
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/audit"
+	"github.com/hashicorp/vault/builtin/credential/approle"
 	"github.com/hashicorp/vault/command/server"
 	"github.com/hashicorp/vault/helper/metricsutil"
 	"github.com/hashicorp/vault/helper/namespace"
@@ -201,6 +202,7 @@ func TestCoreWithSealAndUINoCleanup(t testing.T, opts *CoreConfig) *Core {
 	conf.RawConfig = opts.RawConfig
 	conf.EnableResponseHeaderHostname = opts.EnableResponseHeaderHostname
 	conf.DisableSSCTokens = opts.DisableSSCTokens
+	conf.PluginDirectory = opts.PluginDirectory
 
 	if opts.Logger != nil {
 		conf.Logger = opts.Logger
@@ -1920,7 +1922,8 @@ func (testCluster *TestCluster) newCore(t testing.T, idx int, coreConfig *CoreCo
 
 func (testCluster *TestCluster) setupClusterListener(
 	t testing.T, idx int, core *Core, coreConfig *CoreConfig,
-	opts *TestClusterOptions, listeners []*TestListener, handler http.Handler) {
+	opts *TestClusterOptions, listeners []*TestListener, handler http.Handler,
+) {
 	if coreConfig.ClusterAddr == "" {
 		return
 	}
@@ -2106,7 +2109,8 @@ func (tc *TestCluster) initCores(t testing.T, opts *TestClusterOptions, addAudit
 
 func (testCluster *TestCluster) getAPIClient(
 	t testing.T, opts *TestClusterOptions,
-	port int, tlsConfig *tls.Config) *api.Client {
+	port int, tlsConfig *tls.Config,
+) *api.Client {
 	transport := cleanhttp.DefaultPooledTransport()
 	transport.TLSClientConfig = tlsConfig.Clone()
 	if err := http2.ConfigureTransport(transport); err != nil {
@@ -2136,11 +2140,18 @@ func (testCluster *TestCluster) getAPIClient(
 	return apiClient
 }
 
+func toFunc(f logical.Factory) func() (interface{}, error) {
+	return func() (interface{}, error) {
+		return f, nil
+	}
+}
+
 func NewMockBuiltinRegistry() *mockBuiltinRegistry {
 	return &mockBuiltinRegistry{
 		forTesting: map[string]consts.PluginType{
 			"mysql-database-plugin":      consts.PluginTypeDatabase,
 			"postgresql-database-plugin": consts.PluginTypeDatabase,
+			"approle":                    consts.PluginTypeCredential,
 		},
 	}
 }
@@ -2149,6 +2160,7 @@ type mockBuiltinRegistry struct {
 	forTesting map[string]consts.PluginType
 }
 
+// Get only supports getting database plugins, and approle
 func (m *mockBuiltinRegistry) Get(name string, pluginType consts.PluginType) (func() (interface{}, error), bool) {
 	testPluginType, ok := m.forTesting[name]
 	if !ok {
@@ -2157,39 +2169,49 @@ func (m *mockBuiltinRegistry) Get(name string, pluginType consts.PluginType) (fu
 	if pluginType != testPluginType {
 		return nil, false
 	}
+
+	if name == "approle" {
+		return toFunc(approle.Factory), true
+	}
+
 	if name == "postgresql-database-plugin" {
 		return dbPostgres.New, true
 	}
 	return dbMysql.New(dbMysql.DefaultUserNameTemplate), true
 }
 
-// Keys only supports getting a realistic list of the keys for database plugins.
+// Keys only supports getting a realistic list of the keys for database plugins,
+// and approle
 func (m *mockBuiltinRegistry) Keys(pluginType consts.PluginType) []string {
-	if pluginType != consts.PluginTypeDatabase {
-		return []string{}
-	}
-	/*
-		This is a hard-coded reproduction of the db plugin keys in helper/builtinplugins/registry.go.
-		The registry isn't directly used because it causes import cycles.
-	*/
-	return []string{
-		"mysql-database-plugin",
-		"mysql-aurora-database-plugin",
-		"mysql-rds-database-plugin",
-		"mysql-legacy-database-plugin",
+	switch pluginType {
+	case consts.PluginTypeDatabase:
+		// This is a hard-coded reproduction of the db plugin keys in
+		// helper/builtinplugins/registry.go. The registry isn't directly used
+		// because it causes import cycles.
+		return []string{
+			"mysql-database-plugin",
+			"mysql-aurora-database-plugin",
+			"mysql-rds-database-plugin",
+			"mysql-legacy-database-plugin",
 
-		"cassandra-database-plugin",
-		"couchbase-database-plugin",
-		"elasticsearch-database-plugin",
-		"hana-database-plugin",
-		"influxdb-database-plugin",
-		"mongodb-database-plugin",
-		"mongodbatlas-database-plugin",
-		"mssql-database-plugin",
-		"postgresql-database-plugin",
-		"redshift-database-plugin",
-		"snowflake-database-plugin",
+			"cassandra-database-plugin",
+			"couchbase-database-plugin",
+			"elasticsearch-database-plugin",
+			"hana-database-plugin",
+			"influxdb-database-plugin",
+			"mongodb-database-plugin",
+			"mongodbatlas-database-plugin",
+			"mssql-database-plugin",
+			"postgresql-database-plugin",
+			"redshift-database-plugin",
+			"snowflake-database-plugin",
+		}
+	case consts.PluginTypeCredential:
+		return []string{
+			"approle",
+		}
 	}
+	return []string{}
 }
 
 func (m *mockBuiltinRegistry) Contains(name string, pluginType consts.PluginType) bool {