From 953c7fbecab84cf550fc656d6e09ccc80acd9958 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Mon, 23 Apr 2018 10:51:55 -0400
Subject: [PATCH 01/19] local secret IDs

---
 builtin/credential/approle/backend.go         |  5 +
 builtin/credential/approle/path_role.go       | 92 +++++++++++++++++--
 .../credential/approle/path_tidy_user_id.go   | 14 ++-
 builtin/credential/approle/validation.go      | 47 +++++-----
 4 files changed, 125 insertions(+), 33 deletions(-)

diff --git a/builtin/credential/approle/backend.go b/builtin/credential/approle/backend.go
index 6b914685ef55..e286f68a04f8 100644
--- a/builtin/credential/approle/backend.go
+++ b/builtin/credential/approle/backend.go
@@ -11,6 +11,11 @@ import (
 	"github.com/hashicorp/vault/logical/framework"
 )
 
+const (
+	secretIDPrefix      = "secret_id/"
+	secretIDLocalPrefix = "local_secret_id/"
+)
+
 type backend struct {
 	*framework.Backend
 
diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index b0d1e8314702..d2b3e94c34f5 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -67,6 +67,10 @@ type roleStorageEntry struct {
 	// LowerCaseRoleName enforces the lower casing of role names for all the
 	// roles that get created since this field was introduced.
 	LowerCaseRoleName bool `json:"lower_case_role_name" mapstructure:"lower_case_role_name" structs:"lower_case_role_name"`
+
+	// SecretIDPrefix is the storage prefix under which all the secret IDs will
+	// be persisted.
+	SecretIDPrefix string `json:"-"`
 }
 
 // roleIDStorageEntry represents the reverse mapping from RoleID to Role
@@ -163,6 +167,13 @@ TTL will be set to the value of this parameter.`,
 					Type:        framework.TypeString,
 					Description: "Identifier of the role. Defaults to a UUID.",
 				},
+				"local_secret_ids": &framework.FieldSchema{
+					Type: framework.TypeBool,
+					Description: `
+If set, indicates that the secret IDs generated using this role should be
+cluster local. This can only be set during role creation and once set, it can't
+be reset later.`,
+				},
 			},
 			ExistenceCheck: b.pathRoleExistenceCheck,
 			Callbacks: map[logical.Operation]framework.OperationFunc{
@@ -174,6 +185,27 @@ TTL will be set to the value of this parameter.`,
 			HelpSynopsis:    strings.TrimSpace(roleHelp["role"][0]),
 			HelpDescription: strings.TrimSpace(roleHelp["role"][1]),
 		},
+		&framework.Path{
+			Pattern: "role/" + framework.GenericNameRegex("role_name"+"/local_secret_ids"),
+			Fields: map[string]*framework.FieldSchema{
+				"role_name": &framework.FieldSchema{
+					Type:        framework.TypeString,
+					Description: "Name of the role.",
+				},
+				"local_secret_ids": &framework.FieldSchema{
+					Type: framework.TypeBool,
+					Description: `
+If set, indicates that the secret IDs generated using this role should be
+cluster local. This can only be set during role creation and once set, it can't
+be reset later.`,
+				},
+			},
+			Callbacks: map[logical.Operation]framework.OperationFunc{
+				logical.ReadOperation: b.pathRoleLocalSecretIDsRead,
+			},
+			HelpSynopsis:    strings.TrimSpace(roleHelp["role-local-secret-ids"][0]),
+			HelpDescription: strings.TrimSpace(roleHelp["role-local-secret-ids"][1]),
+		},
 		&framework.Path{
 			Pattern: "role/" + framework.GenericNameRegex("role_name") + "/policies$",
 			Fields: map[string]*framework.FieldSchema{
@@ -585,7 +617,7 @@ func (b *backend) pathRoleSecretIDList(ctx context.Context, req *logical.Request
 
 	// Listing works one level at a time. Get the first level of data
 	// which could then be used to get the actual SecretID storage entries.
-	secretIDHMACs, err := req.Storage.List(ctx, fmt.Sprintf("secret_id/%s/", roleNameHMAC))
+	secretIDHMACs, err := req.Storage.List(ctx, fmt.Sprintf("%s%s/", role.SecretIDPrefix, roleNameHMAC))
 	if err != nil {
 		return nil, err
 	}
@@ -598,7 +630,7 @@ func (b *backend) pathRoleSecretIDList(ctx context.Context, req *logical.Request
 		}
 
 		// Prepare the full index of the SecretIDs.
-		entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC)
+		entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC)
 
 		// SecretID locks are not indexed by SecretIDs itself.
 		// This is because SecretIDs are not stored in plaintext
@@ -779,6 +811,17 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request
 		return logical.ErrorResponse(fmt.Sprintf("invalid role name")), nil
 	}
 
+	localSecretIDsRaw, ok := data.GetOk("local_secret_ids")
+	if ok && req.Operation == logical.CreateOperation {
+		localSecretIDs := localSecretIDsRaw.(bool)
+		if localSecretIDs {
+			role.SecretIDPrefix = secretIDLocalPrefix
+		}
+	}
+	if role.SecretIDPrefix == "" {
+		role.SecretIDPrefix = secretIDPrefix
+	}
+
 	previousRoleID := role.RoleID
 	if roleIDRaw, ok := data.GetOk("role_id"); ok {
 		role.RoleID = roleIDRaw.(string)
@@ -985,7 +1028,7 @@ func (b *backend) pathRoleDelete(ctx context.Context, req *logical.Request, data
 	}
 
 	// Just before the role is deleted, remove all the SecretIDs issued as part of the role.
-	if err = b.flushRoleSecrets(ctx, req.Storage, roleName, role.HMACKey); err != nil {
+	if err = b.flushRoleSecrets(ctx, req.Storage, roleName, role.HMACKey, role.SecretIDPrefix); err != nil {
 		return nil, errwrap.Wrapf(fmt.Sprintf("failed to invalidate the secrets belonging to role %q: {{err}}", roleName), err)
 	}
 
@@ -1044,7 +1087,7 @@ func (b *backend) pathRoleSecretIDLookupUpdate(ctx context.Context, req *logical
 	}
 
 	// Create the index at which the secret_id would've been stored
-	entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC)
+	entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC)
 
 	return b.secretIDCommon(ctx, req.Storage, entryIndex, secretIDHMAC)
 }
@@ -1119,7 +1162,7 @@ func (b *backend) pathRoleSecretIDDestroyUpdateDelete(ctx context.Context, req *
 		return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err)
 	}
 
-	entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC)
+	entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC)
 
 	lock := b.secretIDLock(secretIDHMAC)
 	lock.Lock()
@@ -1189,7 +1232,7 @@ func (b *backend) pathRoleSecretIDAccessorLookupUpdate(ctx context.Context, req
 		return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err)
 	}
 
-	entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, accessorEntry.SecretIDHMAC)
+	entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, accessorEntry.SecretIDHMAC)
 
 	return b.secretIDCommon(ctx, req.Storage, entryIndex, accessorEntry.SecretIDHMAC)
 }
@@ -1230,7 +1273,7 @@ func (b *backend) pathRoleSecretIDAccessorDestroyUpdateDelete(ctx context.Contex
 		return nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err)
 	}
 
-	entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, accessorEntry.SecretIDHMAC)
+	entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, accessorEntry.SecretIDHMAC)
 
 	lock := b.secretIDLock(accessorEntry.SecretIDHMAC)
 	lock.Lock()
@@ -1404,6 +1447,33 @@ func (b *backend) pathRoleBindSecretIDDelete(ctx context.Context, req *logical.R
 	return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "")
 }
 
+func (b *backend) pathRoleLocalSecretIDsRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
+	roleName := data.Get("role_name").(string)
+	if roleName == "" {
+		return logical.ErrorResponse("missing role_name"), nil
+	}
+
+	lock := b.roleLock(roleName)
+	lock.RLock()
+	defer lock.RUnlock()
+
+	if role, err := b.roleEntry(ctx, req.Storage, strings.ToLower(roleName)); err != nil {
+		return nil, err
+	} else if role == nil {
+		return nil, nil
+	} else {
+		localSecretIDs := false
+		if role.SecretIDPrefix == secretIDLocalPrefix {
+			localSecretIDs = true
+		}
+		return &logical.Response{
+			Data: map[string]interface{}{
+				"local_secret_ids": localSecretIDs,
+			},
+		}, nil
+	}
+}
+
 func (b *backend) pathRolePoliciesUpdate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
 	roleName := data.Get("role_name").(string)
 	if roleName == "" {
@@ -2047,7 +2117,7 @@ func (b *backend) handleRoleSecretIDCommon(ctx context.Context, req *logical.Req
 		roleName = strings.ToLower(roleName)
 	}
 
-	if secretIDStorage, err = b.registerSecretIDEntry(ctx, req.Storage, roleName, secretID, role.HMACKey, secretIDStorage); err != nil {
+	if secretIDStorage, err = b.registerSecretIDEntry(ctx, req.Storage, roleName, secretID, role.HMACKey, role.SecretIDPrefix, secretIDStorage); err != nil {
 		return nil, errwrap.Wrapf("failed to store secret_id: {{err}}", err)
 	}
 
@@ -2265,4 +2335,10 @@ duration specified by this value. The renewal duration will
 be fixed. If the Period in the role is modified, the token
 will pick up the new value during its next renewal.`,
 	},
+	"role-local-secret-ids": {
+		"Sets the cluster local setting for secret IDs",
+		`If set, indicates that the secret IDs generated using this role should be
+cluster local. This can only be set during role creation and once set, it can't
+be reset later.`,
+	},
 }
diff --git a/builtin/credential/approle/path_tidy_user_id.go b/builtin/credential/approle/path_tidy_user_id.go
index 27e0d0300ab5..7ac28a9ccb11 100644
--- a/builtin/credential/approle/path_tidy_user_id.go
+++ b/builtin/credential/approle/path_tidy_user_id.go
@@ -34,10 +34,18 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error {
 		return fmt.Errorf("SecretID tidy operation already running")
 	}
 
-	roleNameHMACs, err := s.List(ctx, "secret_id/")
+	roleSecretIDPrefix := secretIDPrefix
+	roleNameHMACs, err := s.List(ctx, roleSecretIDPrefix)
 	if err != nil {
 		return err
 	}
+	if len(roleNameHMACs) == 0 {
+		roleSecretIDPrefix = secretIDLocalPrefix
+		roleNameHMACs, err = s.List(ctx, roleSecretIDPrefix)
+		if err != nil {
+			return err
+		}
+	}
 
 	// List all the accessors and add them all to a map
 	accessorHashes, err := s.List(ctx, "accessor/")
@@ -52,7 +60,7 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error {
 	var result error
 	for _, roleNameHMAC := range roleNameHMACs {
 		// roleNameHMAC will already have a '/' suffix. Don't append another one.
-		secretIDHMACs, err := s.List(ctx, fmt.Sprintf("secret_id/%s", roleNameHMAC))
+		secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s", roleSecretIDPrefix, roleNameHMAC))
 		if err != nil {
 			return err
 		}
@@ -62,7 +70,7 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error {
 			lock := b.secretIDLock(secretIDHMAC)
 			lock.Lock()
 			// roleNameHMAC will already have a '/' suffix. Don't append another one.
-			entryIndex := fmt.Sprintf("secret_id/%s%s", roleNameHMAC, secretIDHMAC)
+			entryIndex := fmt.Sprintf("%s%s%s", roleSecretIDPrefix, roleNameHMAC, secretIDHMAC)
 			secretIDEntry, err := s.Get(ctx, entryIndex)
 			if err != nil {
 				lock.Unlock()
diff --git a/builtin/credential/approle/validation.go b/builtin/credential/approle/validation.go
index 955eae81ec60..6fa5805cec87 100644
--- a/builtin/credential/approle/validation.go
+++ b/builtin/credential/approle/validation.go
@@ -128,7 +128,7 @@ func (b *backend) validateCredentials(ctx context.Context, req *logical.Request,
 		// Check if the SecretID supplied is valid. If use limit was specified
 		// on the SecretID, it will be decremented in this call.
 		var valid bool
-		valid, metadata, err = b.validateBindSecretID(ctx, req, roleName, secretID, role.HMACKey, role.BoundCIDRList)
+		valid, metadata, err = b.validateBindSecretID(ctx, req, roleName, secretID, role)
 		if err != nil {
 			return nil, "", metadata, "", nil, err
 		}
@@ -156,19 +156,19 @@ func (b *backend) validateCredentials(ctx context.Context, req *logical.Request,
 }
 
 // validateBindSecretID is used to determine if the given SecretID is a valid one.
-func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request, roleName, secretID,
-	hmacKey string, roleBoundCIDRList []string) (bool, map[string]string, error) {
-	secretIDHMAC, err := createHMAC(hmacKey, secretID)
+func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request, roleName, secretID string,
+	role *roleStorageEntry) (bool, map[string]string, error) {
+	secretIDHMAC, err := createHMAC(role.HMACKey, secretID)
 	if err != nil {
 		return false, nil, errwrap.Wrapf("failed to create HMAC of secret_id: {{err}}", err)
 	}
 
-	roleNameHMAC, err := createHMAC(hmacKey, roleName)
+	roleNameHMAC, err := createHMAC(role.HMACKey, roleName)
 	if err != nil {
 		return false, nil, errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err)
 	}
 
-	entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC)
+	entryIndex := fmt.Sprintf("%s%s/%s", role.SecretIDPrefix, roleNameHMAC, secretIDHMAC)
 
 	// SecretID locks are always index based on secretIDHMACs. This helps
 	// acquiring the locks when the SecretIDs are listed. This allows grabbing
@@ -176,7 +176,7 @@ func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request
 	lock := b.secretIDLock(secretIDHMAC)
 	lock.RLock()
 
-	result, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, roleNameHMAC, secretIDHMAC)
+	result, err := b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC)
 	if err != nil {
 		lock.RUnlock()
 		return false, nil, err
@@ -192,7 +192,7 @@ func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request
 		// Ensure that the CIDRs on the secret ID are still a subset of that of
 		// role's
 		if err := verifyCIDRRoleSecretIDSubset(result.CIDRList,
-			roleBoundCIDRList); err != nil {
+			role.BoundCIDRList); err != nil {
 			return false, nil, err
 		}
 
@@ -221,7 +221,7 @@ func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request
 	defer lock.Unlock()
 
 	// Lock switching may change the data. Refresh the contents.
-	result, err = b.nonLockedSecretIDStorageEntry(ctx, req.Storage, roleNameHMAC, secretIDHMAC)
+	result, err = b.nonLockedSecretIDStorageEntry(ctx, req.Storage, role.SecretIDPrefix, roleNameHMAC, secretIDHMAC)
 	if err != nil {
 		return false, nil, err
 	}
@@ -254,7 +254,7 @@ func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request
 	// Ensure that the CIDRs on the secret ID are still a subset of that of
 	// role's
 	if err := verifyCIDRRoleSecretIDSubset(result.CIDRList,
-		roleBoundCIDRList); err != nil {
+		role.BoundCIDRList); err != nil {
 		return false, nil, err
 	}
 
@@ -313,7 +313,7 @@ func (b *backend) secretIDAccessorLock(secretIDAccessor string) *locksutil.LockE
 // storage. The entry will be indexed based on the given HMACs of both role
 // name and the secret ID. This method will not acquire secret ID lock to fetch
 // the storage entry. Locks need to be acquired before calling this method.
-func (b *backend) nonLockedSecretIDStorageEntry(ctx context.Context, s logical.Storage, roleNameHMAC, secretIDHMAC string) (*secretIDStorageEntry, error) {
+func (b *backend) nonLockedSecretIDStorageEntry(ctx context.Context, s logical.Storage, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC string) (*secretIDStorageEntry, error) {
 	if secretIDHMAC == "" {
 		return nil, fmt.Errorf("missing secret ID HMAC")
 	}
@@ -323,7 +323,7 @@ func (b *backend) nonLockedSecretIDStorageEntry(ctx context.Context, s logical.S
 	}
 
 	// Prepare the storage index at which the secret ID will be stored
-	entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC)
+	entryIndex := fmt.Sprintf("%s%s/%s", roleSecretIDPrefix, roleNameHMAC, secretIDHMAC)
 
 	entry, err := s.Get(ctx, entryIndex)
 	if err != nil {
@@ -353,7 +353,7 @@ func (b *backend) nonLockedSecretIDStorageEntry(ctx context.Context, s logical.S
 	}
 
 	if persistNeeded {
-		if err := b.nonLockedSetSecretIDStorageEntry(ctx, s, roleNameHMAC, secretIDHMAC, &result); err != nil {
+		if err := b.nonLockedSetSecretIDStorageEntry(ctx, s, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC, &result); err != nil {
 			return nil, errwrap.Wrapf("failed to upgrade role storage entry {{err}}", err)
 		}
 	}
@@ -366,7 +366,10 @@ func (b *backend) nonLockedSecretIDStorageEntry(ctx context.Context, s logical.S
 // role name and the secret ID. This method will not acquire secret ID lock to
 // create/update the storage entry. Locks need to be acquired before calling
 // this method.
-func (b *backend) nonLockedSetSecretIDStorageEntry(ctx context.Context, s logical.Storage, roleNameHMAC, secretIDHMAC string, secretEntry *secretIDStorageEntry) error {
+func (b *backend) nonLockedSetSecretIDStorageEntry(ctx context.Context, s logical.Storage, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC string, secretEntry *secretIDStorageEntry) error {
+	if roleSecretIDPrefix == "" {
+		return fmt.Errorf("missing secret ID prefix")
+	}
 	if secretIDHMAC == "" {
 		return fmt.Errorf("missing secret ID HMAC")
 	}
@@ -379,7 +382,7 @@ func (b *backend) nonLockedSetSecretIDStorageEntry(ctx context.Context, s logica
 		return fmt.Errorf("nil secret entry")
 	}
 
-	entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC)
+	entryIndex := fmt.Sprintf("%s%s/%s", roleSecretIDPrefix, roleNameHMAC, secretIDHMAC)
 
 	if entry, err := logical.StorageEntryJSON(entryIndex, secretEntry); err != nil {
 		return err
@@ -391,7 +394,7 @@ func (b *backend) nonLockedSetSecretIDStorageEntry(ctx context.Context, s logica
 }
 
 // registerSecretIDEntry creates a new storage entry for the given SecretID.
-func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage, roleName, secretID, hmacKey string, secretEntry *secretIDStorageEntry) (*secretIDStorageEntry, error) {
+func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage, roleName, secretID, hmacKey, roleSecretIDPrefix string, secretEntry *secretIDStorageEntry) (*secretIDStorageEntry, error) {
 	secretIDHMAC, err := createHMAC(hmacKey, secretID)
 	if err != nil {
 		return nil, errwrap.Wrapf("failed to create HMAC of secret ID: {{err}}", err)
@@ -404,7 +407,7 @@ func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage,
 	lock := b.secretIDLock(secretIDHMAC)
 	lock.RLock()
 
-	entry, err := b.nonLockedSecretIDStorageEntry(ctx, s, roleNameHMAC, secretIDHMAC)
+	entry, err := b.nonLockedSecretIDStorageEntry(ctx, s, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC)
 	if err != nil {
 		lock.RUnlock()
 		return nil, err
@@ -421,7 +424,7 @@ func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage,
 	defer lock.Unlock()
 
 	// But before saving a new entry, check if the secretID entry was created during the lock switch.
-	entry, err = b.nonLockedSecretIDStorageEntry(ctx, s, roleNameHMAC, secretIDHMAC)
+	entry, err = b.nonLockedSecretIDStorageEntry(ctx, s, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC)
 	if err != nil {
 		return nil, err
 	}
@@ -454,7 +457,7 @@ func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage,
 		return nil, err
 	}
 
-	if err := b.nonLockedSetSecretIDStorageEntry(ctx, s, roleNameHMAC, secretIDHMAC, secretEntry); err != nil {
+	if err := b.nonLockedSetSecretIDStorageEntry(ctx, s, roleSecretIDPrefix, roleNameHMAC, secretIDHMAC, secretEntry); err != nil {
 		return nil, err
 	}
 
@@ -547,7 +550,7 @@ func (b *backend) deleteSecretIDAccessorEntry(ctx context.Context, s logical.Sto
 
 // flushRoleSecrets deletes all the SecretIDs that belong to the given
 // RoleID.
-func (b *backend) flushRoleSecrets(ctx context.Context, s logical.Storage, roleName, hmacKey string) error {
+func (b *backend) flushRoleSecrets(ctx context.Context, s logical.Storage, roleName, hmacKey, roleSecretIDPrefix string) error {
 	roleNameHMAC, err := createHMAC(hmacKey, roleName)
 	if err != nil {
 		return errwrap.Wrapf("failed to create HMAC of role_name: {{err}}", err)
@@ -557,7 +560,7 @@ func (b *backend) flushRoleSecrets(ctx context.Context, s logical.Storage, roleN
 	b.secretIDListingLock.RLock()
 	defer b.secretIDListingLock.RUnlock()
 
-	secretIDHMACs, err := s.List(ctx, fmt.Sprintf("secret_id/%s/", roleNameHMAC))
+	secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s/", roleSecretIDPrefix, roleNameHMAC))
 	if err != nil {
 		return err
 	}
@@ -565,7 +568,7 @@ func (b *backend) flushRoleSecrets(ctx context.Context, s logical.Storage, roleN
 		// Acquire the lock belonging to the SecretID
 		lock := b.secretIDLock(secretIDHMAC)
 		lock.Lock()
-		entryIndex := fmt.Sprintf("secret_id/%s/%s", roleNameHMAC, secretIDHMAC)
+		entryIndex := fmt.Sprintf("%s%s/%s", roleSecretIDPrefix, roleNameHMAC, secretIDHMAC)
 		if err := s.Delete(ctx, entryIndex); err != nil {
 			lock.Unlock()
 			return errwrap.Wrapf(fmt.Sprintf("error deleting SecretID %q from storage: {{err}}", secretIDHMAC), err)

From f8055c8e0609b32c0bbb4b56e009ac2c71f54415 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Mon, 23 Apr 2018 11:20:05 -0400
Subject: [PATCH 02/19] add prefix to LocalStorage

---
 builtin/credential/approle/backend.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/builtin/credential/approle/backend.go b/builtin/credential/approle/backend.go
index e286f68a04f8..4fc079d93930 100644
--- a/builtin/credential/approle/backend.go
+++ b/builtin/credential/approle/backend.go
@@ -95,6 +95,9 @@ func Backend(conf *logical.BackendConfig) (*backend, error) {
 			Unauthenticated: []string{
 				"login",
 			},
+			LocalStorage: []string{
+				secretIDLocalPrefix,
+			},
 		},
 		Paths: framework.PathAppend(
 			rolePaths(b),

From 4ee66b5958f2fd8040966013de56da1361b77127 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Mon, 23 Apr 2018 11:59:37 -0400
Subject: [PATCH 03/19] fix path regex and role storage

---
 builtin/credential/approle/path_role.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index d2b3e94c34f5..54935cb0f186 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -70,7 +70,7 @@ type roleStorageEntry struct {
 
 	// SecretIDPrefix is the storage prefix under which all the secret IDs will
 	// be persisted.
-	SecretIDPrefix string `json:"-"`
+	SecretIDPrefix string `json:"secret_id_prefix" mapstructure:"secret_id_prefix" structs:"secret_id_prefix"`
 }
 
 // roleIDStorageEntry represents the reverse mapping from RoleID to Role
@@ -186,7 +186,7 @@ be reset later.`,
 			HelpDescription: strings.TrimSpace(roleHelp["role"][1]),
 		},
 		&framework.Path{
-			Pattern: "role/" + framework.GenericNameRegex("role_name"+"/local_secret_ids"),
+			Pattern: "role/" + framework.GenericNameRegex("role_name") + "/local_secret_ids$",
 			Fields: map[string]*framework.FieldSchema{
 				"role_name": &framework.FieldSchema{
 					Type:        framework.TypeString,

From 52efa5e608a09d7e7de30288b3d8996cd02fd888 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Mon, 23 Apr 2018 16:02:55 -0400
Subject: [PATCH 04/19] Fix the tidy operation to consider both local and
 non-local secretID cleanups

---
 builtin/credential/approle/backend.go         |   6 +-
 .../credential/approle/path_tidy_user_id.go   | 152 +++++++++---------
 builtin/credential/approle/validation.go      |   7 +-
 3 files changed, 87 insertions(+), 78 deletions(-)

diff --git a/builtin/credential/approle/backend.go b/builtin/credential/approle/backend.go
index 4fc079d93930..51f0f3c3fdd9 100644
--- a/builtin/credential/approle/backend.go
+++ b/builtin/credential/approle/backend.go
@@ -12,8 +12,10 @@ import (
 )
 
 const (
-	secretIDPrefix      = "secret_id/"
-	secretIDLocalPrefix = "local_secret_id/"
+	secretIDPrefix              = "secret_id/"
+	secretIDLocalPrefix         = "local_secret_id/"
+	secretIDAccessorPrefix      = "accessor/"
+	secretIDAccessorLocalPrefix = "local_accessor/"
 )
 
 type backend struct {
diff --git a/builtin/credential/approle/path_tidy_user_id.go b/builtin/credential/approle/path_tidy_user_id.go
index 7ac28a9ccb11..82d9e40c3fe9 100644
--- a/builtin/credential/approle/path_tidy_user_id.go
+++ b/builtin/credential/approle/path_tidy_user_id.go
@@ -34,106 +34,112 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error {
 		return fmt.Errorf("SecretID tidy operation already running")
 	}
 
-	roleSecretIDPrefix := secretIDPrefix
-	roleNameHMACs, err := s.List(ctx, roleSecretIDPrefix)
-	if err != nil {
-		return err
-	}
-	if len(roleNameHMACs) == 0 {
-		roleSecretIDPrefix = secretIDLocalPrefix
-		roleNameHMACs, err = s.List(ctx, roleSecretIDPrefix)
+	var result error
+
+	tidyFunc := func(secretIDPrefixToUse, accessorIDPrefixToUse string) error {
+		roleNameHMACs, err := s.List(ctx, secretIDPrefixToUse)
 		if err != nil {
 			return err
 		}
-	}
 
-	// List all the accessors and add them all to a map
-	accessorHashes, err := s.List(ctx, "accessor/")
-	if err != nil {
-		return err
-	}
-	accessorMap := make(map[string]bool, len(accessorHashes))
-	for _, accessorHash := range accessorHashes {
-		accessorMap[accessorHash] = true
-	}
-
-	var result error
-	for _, roleNameHMAC := range roleNameHMACs {
-		// roleNameHMAC will already have a '/' suffix. Don't append another one.
-		secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s", roleSecretIDPrefix, roleNameHMAC))
+		// List all the accessors and add them all to a map
+		accessorHashes, err := s.List(ctx, accessorIDPrefixToUse)
 		if err != nil {
 			return err
 		}
-		for _, secretIDHMAC := range secretIDHMACs {
-			// In order to avoid lock swroleing in case there is need to delete,
-			// grab the write lock.
-			lock := b.secretIDLock(secretIDHMAC)
-			lock.Lock()
+		accessorMap := make(map[string]bool, len(accessorHashes))
+		for _, accessorHash := range accessorHashes {
+			accessorMap[accessorHash] = true
+		}
+
+		for _, roleNameHMAC := range roleNameHMACs {
 			// roleNameHMAC will already have a '/' suffix. Don't append another one.
-			entryIndex := fmt.Sprintf("%s%s%s", roleSecretIDPrefix, roleNameHMAC, secretIDHMAC)
-			secretIDEntry, err := s.Get(ctx, entryIndex)
+			secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s", secretIDPrefixToUse, roleNameHMAC))
 			if err != nil {
-				lock.Unlock()
-				return errwrap.Wrapf(fmt.Sprintf("error fetching SecretID %q: {{err}}", secretIDHMAC), err)
-			}
-
-			if secretIDEntry == nil {
-				result = multierror.Append(result, fmt.Errorf("entry for SecretID %q is nil", secretIDHMAC))
-				lock.Unlock()
-				continue
+				return err
 			}
+			for _, secretIDHMAC := range secretIDHMACs {
+				// In order to avoid lock swroleing in case there is need to delete,
+				// grab the write lock.
+				lock := b.secretIDLock(secretIDHMAC)
+				lock.Lock()
+				// roleNameHMAC will already have a '/' suffix. Don't append another one.
+				entryIndex := fmt.Sprintf("%s%s%s", secretIDPrefixToUse, roleNameHMAC, secretIDHMAC)
+				secretIDEntry, err := s.Get(ctx, entryIndex)
+				if err != nil {
+					lock.Unlock()
+					return errwrap.Wrapf(fmt.Sprintf("error fetching SecretID %q: {{err}}", secretIDHMAC), err)
+				}
 
-			if secretIDEntry.Value == nil || len(secretIDEntry.Value) == 0 {
-				lock.Unlock()
-				return fmt.Errorf("found entry for SecretID %q but actual SecretID is empty", secretIDHMAC)
-			}
+				if secretIDEntry == nil {
+					result = multierror.Append(result, fmt.Errorf("entry for SecretID %q is nil", secretIDHMAC))
+					lock.Unlock()
+					continue
+				}
 
-			var result secretIDStorageEntry
-			if err := secretIDEntry.DecodeJSON(&result); err != nil {
-				lock.Unlock()
-				return err
-			}
+				if secretIDEntry.Value == nil || len(secretIDEntry.Value) == 0 {
+					lock.Unlock()
+					return fmt.Errorf("found entry for SecretID %q but actual SecretID is empty", secretIDHMAC)
+				}
 
-			// ExpirationTime not being set indicates non-expiring SecretIDs
-			if !result.ExpirationTime.IsZero() && time.Now().After(result.ExpirationTime) {
-				// Clean up the accessor of the secret ID first
-				err = b.deleteSecretIDAccessorEntry(ctx, s, result.SecretIDAccessor)
-				if err != nil {
+				var result secretIDStorageEntry
+				if err := secretIDEntry.DecodeJSON(&result); err != nil {
 					lock.Unlock()
 					return err
 				}
 
-				if err := s.Delete(ctx, entryIndex); err != nil {
+				// ExpirationTime not being set indicates non-expiring SecretIDs
+				if !result.ExpirationTime.IsZero() && time.Now().After(result.ExpirationTime) {
+					// Clean up the accessor of the secret ID first
+					err = b.deleteSecretIDAccessorEntry(ctx, s, result.SecretIDAccessor)
+					if err != nil {
+						lock.Unlock()
+						return err
+					}
+
+					if err := s.Delete(ctx, entryIndex); err != nil {
+						lock.Unlock()
+						return errwrap.Wrapf(fmt.Sprintf("error deleting SecretID %q from storage: {{err}}", secretIDHMAC), err)
+					}
+				}
+
+				// At this point, the secret ID is not expired and is valid. Delete
+				// the corresponding accessor from the accessorMap. This will leave
+				// only the dangling accessors in the map which can then be cleaned
+				// up later.
+				salt, err := b.Salt(ctx)
+				if err != nil {
 					lock.Unlock()
-					return errwrap.Wrapf(fmt.Sprintf("error deleting SecretID %q from storage: {{err}}", secretIDHMAC), err)
+					return err
 				}
+				delete(accessorMap, salt.SaltID(result.SecretIDAccessor))
+
+				lock.Unlock()
 			}
+		}
 
-			// At this point, the secret ID is not expired and is valid. Delete
-			// the corresponding accessor from the accessorMap. This will leave
-			// only the dangling accessors in the map which can then be cleaned
-			// up later.
-			salt, err := b.Salt(ctx)
+		// Accessor indexes were not getting cleaned up until 0.9.3. This is a fix
+		// to clean up the dangling accessor entries.
+		for accessorHash, _ := range accessorMap {
+			// Ideally, locking should be performed here. But for that, accessors
+			// are required in plaintext, which are not available. Hence performing
+			// a racy cleanup.
+			err = s.Delete(ctx, secretIDAccessorPrefix+accessorHash)
 			if err != nil {
-				lock.Unlock()
 				return err
 			}
-			delete(accessorMap, salt.SaltID(result.SecretIDAccessor))
-
-			lock.Unlock()
 		}
+
+		return nil
 	}
 
-	// Accessor indexes were not getting cleaned up until 0.9.3. This is a fix
-	// to clean up the dangling accessor entries.
-	for accessorHash, _ := range accessorMap {
-		// Ideally, locking should be performed here. But for that, accessors
-		// are required in plaintext, which are not available. Hence performing
-		// a racy cleanup.
-		err = s.Delete(ctx, "accessor/"+accessorHash)
-		if err != nil {
-			return err
-		}
+	err := tidyFunc(secretIDPrefix, secretIDAccessorPrefix)
+	if err != nil {
+		return err
+	}
+	err = tidyFunc(secretIDLocalPrefix, secretIDAccessorLocalPrefix)
+	if err != nil {
+		return err
 	}
 
 	return result
diff --git a/builtin/credential/approle/validation.go b/builtin/credential/approle/validation.go
index 6fa5805cec87..f12f73a22343 100644
--- a/builtin/credential/approle/validation.go
+++ b/builtin/credential/approle/validation.go
@@ -383,6 +383,7 @@ func (b *backend) nonLockedSetSecretIDStorageEntry(ctx context.Context, s logica
 	}
 
 	entryIndex := fmt.Sprintf("%s%s/%s", roleSecretIDPrefix, roleNameHMAC, secretIDHMAC)
+	fmt.Printf("secret ID being stored at %q\n", entryIndex)
 
 	if entry, err := logical.StorageEntryJSON(entryIndex, secretEntry); err != nil {
 		return err
@@ -478,7 +479,7 @@ func (b *backend) secretIDAccessorEntry(ctx context.Context, s logical.Storage,
 	if err != nil {
 		return nil, err
 	}
-	entryIndex := "accessor/" + salt.SaltID(secretIDAccessor)
+	entryIndex := secretIDAccessorPrefix + salt.SaltID(secretIDAccessor)
 
 	accessorLock := b.secretIDAccessorLock(secretIDAccessor)
 	accessorLock.RLock()
@@ -511,7 +512,7 @@ func (b *backend) createSecretIDAccessorEntry(ctx context.Context, s logical.Sto
 	if err != nil {
 		return err
 	}
-	entryIndex := "accessor/" + salt.SaltID(entry.SecretIDAccessor)
+	entryIndex := secretIDAccessorPrefix + salt.SaltID(entry.SecretIDAccessor)
 
 	accessorLock := b.secretIDAccessorLock(accessorUUID)
 	accessorLock.Lock()
@@ -534,7 +535,7 @@ func (b *backend) deleteSecretIDAccessorEntry(ctx context.Context, s logical.Sto
 	if err != nil {
 		return err
 	}
-	accessorEntryIndex := "accessor/" + salt.SaltID(secretIDAccessor)
+	accessorEntryIndex := secretIDAccessorPrefix + salt.SaltID(secretIDAccessor)
 
 	accessorLock := b.secretIDAccessorLock(secretIDAccessor)
 	accessorLock.Lock()

From 3d7e704f3f6a9723eae17e49159720b15d1f1379 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Mon, 23 Apr 2018 16:19:05 -0400
Subject: [PATCH 05/19] segregate local and non-local accessor entries

---
 builtin/credential/approle/backend.go         |  1 +
 builtin/credential/approle/path_role.go       |  8 ++---
 .../credential/approle/path_tidy_user_id.go   |  2 +-
 builtin/credential/approle/validation.go      | 33 +++++++++++++------
 4 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/builtin/credential/approle/backend.go b/builtin/credential/approle/backend.go
index 51f0f3c3fdd9..1804390835d6 100644
--- a/builtin/credential/approle/backend.go
+++ b/builtin/credential/approle/backend.go
@@ -99,6 +99,7 @@ func Backend(conf *logical.BackendConfig) (*backend, error) {
 			},
 			LocalStorage: []string{
 				secretIDLocalPrefix,
+				secretIDAccessorLocalPrefix,
 			},
 		},
 		Paths: framework.PathAppend(
diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index 54935cb0f186..02413aa78e22 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -1178,7 +1178,7 @@ func (b *backend) pathRoleSecretIDDestroyUpdateDelete(ctx context.Context, req *
 	}
 
 	// Delete the accessor of the SecretID first
-	if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, result.SecretIDAccessor); err != nil {
+	if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, result.SecretIDAccessor, role.SecretIDPrefix); err != nil {
 		return nil, err
 	}
 
@@ -1219,7 +1219,7 @@ func (b *backend) pathRoleSecretIDAccessorLookupUpdate(ctx context.Context, req
 		return nil, fmt.Errorf("role %q does not exist", roleName)
 	}
 
-	accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor)
+	accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor, role.SecretIDPrefix)
 	if err != nil {
 		return nil, err
 	}
@@ -1260,7 +1260,7 @@ func (b *backend) pathRoleSecretIDAccessorDestroyUpdateDelete(ctx context.Contex
 		return nil, fmt.Errorf("role %q does not exist", roleName)
 	}
 
-	accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor)
+	accessorEntry, err := b.secretIDAccessorEntry(ctx, req.Storage, secretIDAccessor, role.SecretIDPrefix)
 	if err != nil {
 		return nil, err
 	}
@@ -1280,7 +1280,7 @@ func (b *backend) pathRoleSecretIDAccessorDestroyUpdateDelete(ctx context.Contex
 	defer lock.Unlock()
 
 	// Delete the accessor of the SecretID first
-	if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, secretIDAccessor); err != nil {
+	if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, secretIDAccessor, role.SecretIDPrefix); err != nil {
 		return nil, err
 	}
 
diff --git a/builtin/credential/approle/path_tidy_user_id.go b/builtin/credential/approle/path_tidy_user_id.go
index 82d9e40c3fe9..7fefb0dbdc4c 100644
--- a/builtin/credential/approle/path_tidy_user_id.go
+++ b/builtin/credential/approle/path_tidy_user_id.go
@@ -91,7 +91,7 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error {
 				// ExpirationTime not being set indicates non-expiring SecretIDs
 				if !result.ExpirationTime.IsZero() && time.Now().After(result.ExpirationTime) {
 					// Clean up the accessor of the secret ID first
-					err = b.deleteSecretIDAccessorEntry(ctx, s, result.SecretIDAccessor)
+					err = b.deleteSecretIDAccessorEntry(ctx, s, result.SecretIDAccessor, secretIDPrefixToUse)
 					if err != nil {
 						lock.Unlock()
 						return err
diff --git a/builtin/credential/approle/validation.go b/builtin/credential/approle/validation.go
index f12f73a22343..d42d68fd0c39 100644
--- a/builtin/credential/approle/validation.go
+++ b/builtin/credential/approle/validation.go
@@ -234,7 +234,7 @@ func (b *backend) validateBindSecretID(ctx context.Context, req *logical.Request
 	// requests to use the same SecretID will fail.
 	if result.SecretIDNumUses == 1 {
 		// Delete the secret IDs accessor first
-		if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, result.SecretIDAccessor); err != nil {
+		if err := b.deleteSecretIDAccessorEntry(ctx, req.Storage, result.SecretIDAccessor, role.SecretIDPrefix); err != nil {
 			return false, nil, err
 		}
 		if err := req.Storage.Delete(ctx, entryIndex); err != nil {
@@ -383,7 +383,6 @@ func (b *backend) nonLockedSetSecretIDStorageEntry(ctx context.Context, s logica
 	}
 
 	entryIndex := fmt.Sprintf("%s%s/%s", roleSecretIDPrefix, roleNameHMAC, secretIDHMAC)
-	fmt.Printf("secret ID being stored at %q\n", entryIndex)
 
 	if entry, err := logical.StorageEntryJSON(entryIndex, secretEntry); err != nil {
 		return err
@@ -454,7 +453,7 @@ func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage,
 	}
 
 	// Before storing the SecretID, store its accessor.
-	if err := b.createSecretIDAccessorEntry(ctx, s, secretEntry, secretIDHMAC); err != nil {
+	if err := b.createSecretIDAccessorEntry(ctx, s, secretEntry, secretIDHMAC, roleSecretIDPrefix); err != nil {
 		return nil, err
 	}
 
@@ -467,7 +466,7 @@ func (b *backend) registerSecretIDEntry(ctx context.Context, s logical.Storage,
 
 // secretIDAccessorEntry is used to read the storage entry that maps an
 // accessor to a secret_id.
-func (b *backend) secretIDAccessorEntry(ctx context.Context, s logical.Storage, secretIDAccessor string) (*secretIDAccessorStorageEntry, error) {
+func (b *backend) secretIDAccessorEntry(ctx context.Context, s logical.Storage, secretIDAccessor, roleSecretIDPrefix string) (*secretIDAccessorStorageEntry, error) {
 	if secretIDAccessor == "" {
 		return nil, fmt.Errorf("missing secretIDAccessor")
 	}
@@ -479,7 +478,11 @@ func (b *backend) secretIDAccessorEntry(ctx context.Context, s logical.Storage,
 	if err != nil {
 		return nil, err
 	}
-	entryIndex := secretIDAccessorPrefix + salt.SaltID(secretIDAccessor)
+	accessorPrefix := secretIDAccessorPrefix
+	if roleSecretIDPrefix == secretIDLocalPrefix {
+		accessorPrefix = secretIDAccessorLocalPrefix
+	}
+	entryIndex := accessorPrefix + salt.SaltID(secretIDAccessor)
 
 	accessorLock := b.secretIDAccessorLock(secretIDAccessor)
 	accessorLock.RLock()
@@ -499,7 +502,7 @@ func (b *backend) secretIDAccessorEntry(ctx context.Context, s logical.Storage,
 // createSecretIDAccessorEntry creates an identifier for the SecretID. A storage index,
 // mapping the accessor to the SecretID is also created. This method should
 // be called when the lock for the corresponding SecretID is held.
-func (b *backend) createSecretIDAccessorEntry(ctx context.Context, s logical.Storage, entry *secretIDStorageEntry, secretIDHMAC string) error {
+func (b *backend) createSecretIDAccessorEntry(ctx context.Context, s logical.Storage, entry *secretIDStorageEntry, secretIDHMAC, roleSecretIDPrefix string) error {
 	// Create a random accessor
 	accessorUUID, err := uuid.GenerateUUID()
 	if err != nil {
@@ -512,7 +515,12 @@ func (b *backend) createSecretIDAccessorEntry(ctx context.Context, s logical.Sto
 	if err != nil {
 		return err
 	}
-	entryIndex := secretIDAccessorPrefix + salt.SaltID(entry.SecretIDAccessor)
+
+	accessorPrefix := secretIDAccessorPrefix
+	if roleSecretIDPrefix == secretIDLocalPrefix {
+		accessorPrefix = secretIDAccessorLocalPrefix
+	}
+	entryIndex := accessorPrefix + salt.SaltID(entry.SecretIDAccessor)
 
 	accessorLock := b.secretIDAccessorLock(accessorUUID)
 	accessorLock.Lock()
@@ -530,19 +538,24 @@ func (b *backend) createSecretIDAccessorEntry(ctx context.Context, s logical.Sto
 }
 
 // deleteSecretIDAccessorEntry deletes the storage index mapping the accessor to a SecretID.
-func (b *backend) deleteSecretIDAccessorEntry(ctx context.Context, s logical.Storage, secretIDAccessor string) error {
+func (b *backend) deleteSecretIDAccessorEntry(ctx context.Context, s logical.Storage, secretIDAccessor, roleSecretIDPrefix string) error {
 	salt, err := b.Salt(ctx)
 	if err != nil {
 		return err
 	}
-	accessorEntryIndex := secretIDAccessorPrefix + salt.SaltID(secretIDAccessor)
+
+	accessorPrefix := secretIDAccessorPrefix
+	if roleSecretIDPrefix == secretIDLocalPrefix {
+		accessorPrefix = secretIDAccessorLocalPrefix
+	}
+	entryIndex := accessorPrefix + salt.SaltID(secretIDAccessor)
 
 	accessorLock := b.secretIDAccessorLock(secretIDAccessor)
 	accessorLock.Lock()
 	defer accessorLock.Unlock()
 
 	// Delete the accessor of the SecretID first
-	if err := s.Delete(ctx, accessorEntryIndex); err != nil {
+	if err := s.Delete(ctx, entryIndex); err != nil {
 		return errwrap.Wrapf("failed to delete accessor storage entry: {{err}}", err)
 	}
 

From 184dac8cfcaea38519ccec1f4c7799edd640bb2a Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Mon, 23 Apr 2018 16:31:47 -0400
Subject: [PATCH 06/19] Upgrade secret ID prefix and fix tests

---
 builtin/credential/approle/path_role.go      |  5 +++++
 builtin/credential/approle/path_role_test.go | 10 ++++++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index 02413aa78e22..73d3c6170f0e 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -763,6 +763,11 @@ func (b *backend) roleEntry(ctx context.Context, s logical.Storage, roleName str
 		needsUpgrade = true
 	}
 
+	if role.SecretIDPrefix == "" {
+		role.SecretIDPrefix = secretIDPrefix
+		needsUpgrade = true
+	}
+
 	if needsUpgrade && (b.System().LocalMount() || !b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary)) {
 		entry, err := logical.StorageEntryJSON("role/"+strings.ToLower(roleName), &role)
 		if err != nil {
diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go
index 1e07c32484fe..96ef49b53772 100644
--- a/builtin/credential/approle/path_role_test.go
+++ b/builtin/credential/approle/path_role_test.go
@@ -59,6 +59,7 @@ func TestApprole_UpgradeBoundCIDRList(t *testing.T) {
 		Policies:         []string{"default"},
 		BindSecretID:     true,
 		BoundCIDRListOld: "127.0.0.1/18,192.178.1.2/24",
+		SecretIDPrefix:   secretIDPrefix,
 	}
 	err = b.setRoleEntry(context.Background(), storage, "testrole", role, "")
 	if err != nil {
@@ -120,10 +121,11 @@ func TestApprole_RoleNameLowerCasing(t *testing.T) {
 
 	// Save a role with out LowerCaseRoleName set
 	role := &roleStorageEntry{
-		RoleID:       "testroleid",
-		HMACKey:      "testhmackey",
-		Policies:     []string{"default"},
-		BindSecretID: true,
+		RoleID:         "testroleid",
+		HMACKey:        "testhmackey",
+		Policies:       []string{"default"},
+		BindSecretID:   true,
+		SecretIDPrefix: secretIDPrefix,
 	}
 	err = b.setRoleEntry(context.Background(), storage, "testRoleName", role, "")
 	if err != nil {

From b929187362b6778c07064b450b03c107e07f6b42 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Mon, 23 Apr 2018 16:52:09 -0400
Subject: [PATCH 07/19] naming changes

---
 builtin/credential/approle/backend.go   |  4 ++--
 builtin/credential/approle/path_role.go | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/credential/approle/backend.go b/builtin/credential/approle/backend.go
index 1804390835d6..01911441297b 100644
--- a/builtin/credential/approle/backend.go
+++ b/builtin/credential/approle/backend.go
@@ -13,9 +13,9 @@ import (
 
 const (
 	secretIDPrefix              = "secret_id/"
-	secretIDLocalPrefix         = "local_secret_id/"
+	secretIDLocalPrefix         = "secret_id_local/"
 	secretIDAccessorPrefix      = "accessor/"
-	secretIDAccessorLocalPrefix = "local_accessor/"
+	secretIDAccessorLocalPrefix = "accessor_local/"
 )
 
 type backend struct {
diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index 73d3c6170f0e..8004b1177aee 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -167,7 +167,7 @@ TTL will be set to the value of this parameter.`,
 					Type:        framework.TypeString,
 					Description: "Identifier of the role. Defaults to a UUID.",
 				},
-				"local_secret_ids": &framework.FieldSchema{
+				"enable_local_secret_ids": &framework.FieldSchema{
 					Type: framework.TypeBool,
 					Description: `
 If set, indicates that the secret IDs generated using this role should be
@@ -186,13 +186,13 @@ be reset later.`,
 			HelpDescription: strings.TrimSpace(roleHelp["role"][1]),
 		},
 		&framework.Path{
-			Pattern: "role/" + framework.GenericNameRegex("role_name") + "/local_secret_ids$",
+			Pattern: "role/" + framework.GenericNameRegex("role_name") + "/enable_local_secret_ids$",
 			Fields: map[string]*framework.FieldSchema{
 				"role_name": &framework.FieldSchema{
 					Type:        framework.TypeString,
 					Description: "Name of the role.",
 				},
-				"local_secret_ids": &framework.FieldSchema{
+				"enable_local_secret_ids": &framework.FieldSchema{
 					Type: framework.TypeBool,
 					Description: `
 If set, indicates that the secret IDs generated using this role should be
@@ -816,7 +816,7 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request
 		return logical.ErrorResponse(fmt.Sprintf("invalid role name")), nil
 	}
 
-	localSecretIDsRaw, ok := data.GetOk("local_secret_ids")
+	localSecretIDsRaw, ok := data.GetOk("enable_local_secret_ids")
 	if ok && req.Operation == logical.CreateOperation {
 		localSecretIDs := localSecretIDsRaw.(bool)
 		if localSecretIDs {
@@ -1473,7 +1473,7 @@ func (b *backend) pathRoleLocalSecretIDsRead(ctx context.Context, req *logical.R
 		}
 		return &logical.Response{
 			Data: map[string]interface{}{
-				"local_secret_ids": localSecretIDs,
+				"enable_local_secret_ids": localSecretIDs,
 			},
 		}, nil
 	}
@@ -2341,7 +2341,7 @@ be fixed. If the Period in the role is modified, the token
 will pick up the new value during its next renewal.`,
 	},
 	"role-local-secret-ids": {
-		"Sets the cluster local setting for secret IDs",
+		"Enables cluster local secret IDs",
 		`If set, indicates that the secret IDs generated using this role should be
 cluster local. This can only be set during role creation and once set, it can't
 be reset later.`,

From b4f6b6fd31092c790244bde144dcfc1c0a10319e Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Mon, 23 Apr 2018 16:54:23 -0400
Subject: [PATCH 08/19] update docs

---
 website/source/api/auth/approle/index.html.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/website/source/api/auth/approle/index.html.md b/website/source/api/auth/approle/index.html.md
index 2665a4ec4e76..7f71c0b0e409 100644
--- a/website/source/api/auth/approle/index.html.md
+++ b/website/source/api/auth/approle/index.html.md
@@ -94,6 +94,9 @@ enabled while creating or updating a role.
   but the TTL set on the token at each renewal is fixed to the value specified
   here. If this value is modified, the token will pick up the new value at its
   next renewal.
+- `enable_local_secret_ids` `(bool: false)` - If set, indicates that the secret
+  IDs generated using this role should be cluster local. This can only be set
+  during role creation and once set, it can't be reset later.
 
 ### Sample Payload
 

From 20c7f20265f46c1e394f08230e6c18d822ae1c3f Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Mon, 23 Apr 2018 17:05:53 -0400
Subject: [PATCH 09/19] error on enable_local_secret_ids update after role
 creation

---
 builtin/credential/approle/path_role.go       | 26 ++++++++++---------
 website/source/api/auth/approle/index.html.md |  6 ++---
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index 8004b1177aee..3dbebbaca8bb 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -68,8 +68,8 @@ type roleStorageEntry struct {
 	// roles that get created since this field was introduced.
 	LowerCaseRoleName bool `json:"lower_case_role_name" mapstructure:"lower_case_role_name" structs:"lower_case_role_name"`
 
-	// SecretIDPrefix is the storage prefix under which all the secret IDs will
-	// be persisted.
+	// SecretIDPrefix is the storage prefix for persisting secret IDs. This
+	// differs based on whether the secret IDs are cluster local or not.
 	SecretIDPrefix string `json:"secret_id_prefix" mapstructure:"secret_id_prefix" structs:"secret_id_prefix"`
 }
 
@@ -170,9 +170,8 @@ TTL will be set to the value of this parameter.`,
 				"enable_local_secret_ids": &framework.FieldSchema{
 					Type: framework.TypeBool,
 					Description: `
-If set, indicates that the secret IDs generated using this role should be
-cluster local. This can only be set during role creation and once set, it can't
-be reset later.`,
+If set, the secret IDs generated using this role will be cluster local. This
+can only be set during role creation and once set, it can't be reset later.`,
 				},
 			},
 			ExistenceCheck: b.pathRoleExistenceCheck,
@@ -195,9 +194,8 @@ be reset later.`,
 				"enable_local_secret_ids": &framework.FieldSchema{
 					Type: framework.TypeBool,
 					Description: `
-If set, indicates that the secret IDs generated using this role should be
-cluster local. This can only be set during role creation and once set, it can't
-be reset later.`,
+If set, the secret IDs generated using this role will be cluster local. This
+can only be set during role creation and once set, it can't be reset later.`,
 				},
 			},
 			Callbacks: map[logical.Operation]framework.OperationFunc{
@@ -817,10 +815,14 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request
 	}
 
 	localSecretIDsRaw, ok := data.GetOk("enable_local_secret_ids")
-	if ok && req.Operation == logical.CreateOperation {
-		localSecretIDs := localSecretIDsRaw.(bool)
-		if localSecretIDs {
-			role.SecretIDPrefix = secretIDLocalPrefix
+	if ok {
+		if req.Operation == logical.CreateOperation {
+			localSecretIDs := localSecretIDsRaw.(bool)
+			if localSecretIDs {
+				role.SecretIDPrefix = secretIDLocalPrefix
+			}
+		} else {
+			return logical.ErrorResponse("enable_local_secret_ids can only be modified during role creation"), nil
 		}
 	}
 	if role.SecretIDPrefix == "" {
diff --git a/website/source/api/auth/approle/index.html.md b/website/source/api/auth/approle/index.html.md
index 7f71c0b0e409..e36a7f4271f7 100644
--- a/website/source/api/auth/approle/index.html.md
+++ b/website/source/api/auth/approle/index.html.md
@@ -94,9 +94,9 @@ enabled while creating or updating a role.
   but the TTL set on the token at each renewal is fixed to the value specified
   here. If this value is modified, the token will pick up the new value at its
   next renewal.
-- `enable_local_secret_ids` `(bool: false)` - If set, indicates that the secret
-  IDs generated using this role should be cluster local. This can only be set
-  during role creation and once set, it can't be reset later.
+- `enable_local_secret_ids` `(bool: false)` - If set, the secret IDs generated
+  using this role will be cluster local. This can only be set during role
+  creation and once set, it can't be reset later.
 
 ### Sample Payload
 

From 83aabbba053af5e3f6423746f61bdcd8927a9735 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Tue, 24 Apr 2018 09:53:36 -0400
Subject: [PATCH 10/19] Add enable_local_secret_ids to role read response

---
 builtin/credential/approle/path_role.go      | 23 +++++++++------
 builtin/credential/approle/path_role_test.go | 30 ++++++++++++++++++++
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index 3dbebbaca8bb..3989a637f224 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -957,15 +957,20 @@ func (b *backend) pathRoleRead(ctx context.Context, req *logical.Request, data *
 	}
 
 	respData := map[string]interface{}{
-		"bind_secret_id":     role.BindSecretID,
-		"bound_cidr_list":    role.BoundCIDRList,
-		"period":             role.Period / time.Second,
-		"policies":           role.Policies,
-		"secret_id_num_uses": role.SecretIDNumUses,
-		"secret_id_ttl":      role.SecretIDTTL / time.Second,
-		"token_max_ttl":      role.TokenMaxTTL / time.Second,
-		"token_num_uses":     role.TokenNumUses,
-		"token_ttl":          role.TokenTTL / time.Second,
+		"bind_secret_id":          role.BindSecretID,
+		"bound_cidr_list":         role.BoundCIDRList,
+		"period":                  role.Period / time.Second,
+		"policies":                role.Policies,
+		"secret_id_num_uses":      role.SecretIDNumUses,
+		"secret_id_ttl":           role.SecretIDTTL / time.Second,
+		"token_max_ttl":           role.TokenMaxTTL / time.Second,
+		"token_num_uses":          role.TokenNumUses,
+		"token_ttl":               role.TokenTTL / time.Second,
+		"enable_local_secret_ids": false,
+	}
+
+	if role.SecretIDPrefix == secretIDLocalPrefix {
+		respData["enable_local_secret_ids"] = true
 	}
 
 	resp := &logical.Response{
diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go
index 96ef49b53772..0d4b829bdcc4 100644
--- a/builtin/credential/approle/path_role_test.go
+++ b/builtin/credential/approle/path_role_test.go
@@ -2,6 +2,7 @@ package approle
 
 import (
 	"context"
+	"fmt"
 	"reflect"
 	"strings"
 	"testing"
@@ -12,6 +13,35 @@ import (
 	"github.com/mitchellh/mapstructure"
 )
 
+func TestApprole_UpgradeSecretIDPrefix(t *testing.T) {
+	var resp *logical.Response
+	var err error
+
+	b, storage := createBackendWithStorage(t)
+
+	role := &roleStorageEntry{
+		RoleID:           "testroleid",
+		HMACKey:          "testhmackey",
+		Policies:         []string{"default"},
+		BindSecretID:     true,
+		BoundCIDRListOld: "127.0.0.1/18,192.178.1.2/24",
+	}
+	err = b.setRoleEntry(context.Background(), storage, "testrole", role, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	resp, err = b.HandleRequest(context.Background(), &logical.Request{
+		Path:      "role/testrole",
+		Operation: logical.ReadOperation,
+		Storage:   storage,
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: err: %v\n resp: %#v", err, resp)
+	}
+	fmt.Printf("resp: %#v\n", resp)
+}
+
 func TestApprole_UpgradeBoundCIDRList(t *testing.T) {
 	var resp *logical.Response
 	var err error

From f39f4052a11df4501ad6f2e5100d3dc0eba6b8b9 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Tue, 24 Apr 2018 10:05:17 -0400
Subject: [PATCH 11/19] Add immutability test

---
 builtin/credential/approle/path_role.go      |  2 +-
 builtin/credential/approle/path_role_test.go | 40 +++++++++++++++++++-
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index 3989a637f224..3af3f08b0fe2 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -811,7 +811,7 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request
 			LowerCaseRoleName: true,
 		}
 	} else if role == nil {
-		return logical.ErrorResponse(fmt.Sprintf("invalid role name")), nil
+		return logical.ErrorResponse(fmt.Sprintf("role name %q doesn't exist", roleName)), nil
 	}
 
 	localSecretIDsRaw, ok := data.GetOk("enable_local_secret_ids")
diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go
index 0d4b829bdcc4..9aca3a7e8cf4 100644
--- a/builtin/credential/approle/path_role_test.go
+++ b/builtin/credential/approle/path_role_test.go
@@ -2,7 +2,6 @@ package approle
 
 import (
 	"context"
-	"fmt"
 	"reflect"
 	"strings"
 	"testing"
@@ -39,7 +38,44 @@ func TestApprole_UpgradeSecretIDPrefix(t *testing.T) {
 	if err != nil || (resp != nil && resp.IsError()) {
 		t.Fatalf("bad: err: %v\n resp: %#v", err, resp)
 	}
-	fmt.Printf("resp: %#v\n", resp)
+	_, ok := resp.Data["enable_local_secret_ids"]
+	if !ok {
+		t.Fatalf("expected enable_local_secret_ids to be present in the response")
+	}
+}
+
+func TestApprole_LocalSecretIDImmutability(t *testing.T) {
+	var resp *logical.Response
+	var err error
+
+	b, storage := createBackendWithStorage(t)
+
+	roleData := map[string]interface{}{
+		"policies":                []string{"default"},
+		"bind_secret_id":          true,
+		"bound_cidr_list":         []string{"127.0.0.1/18", "192.178.1.2/24"},
+		"enable_local_secret_ids": true,
+	}
+
+	resp, err = b.HandleRequest(context.Background(), &logical.Request{
+		Path:      "role/testrole",
+		Operation: logical.CreateOperation,
+		Storage:   storage,
+		Data:      roleData,
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: err: %v\nresp: %#v", err, resp)
+	}
+
+	resp, err = b.HandleRequest(context.Background(), &logical.Request{
+		Path:      "role/testrole",
+		Operation: logical.UpdateOperation,
+		Storage:   storage,
+		Data:      roleData,
+	})
+	if err == nil {
+		t.Fatalf("expected an error since enable_local_secret_ids can't be overwritten")
+	}
 }
 
 func TestApprole_UpgradeBoundCIDRList(t *testing.T) {

From 42e95d4630d3aba51e17cbaaba14df50ae6df5a8 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Tue, 24 Apr 2018 11:02:11 -0400
Subject: [PATCH 12/19] Add tests

---
 builtin/credential/approle/path_role_test.go | 104 ++++++++++++++++++-
 1 file changed, 101 insertions(+), 3 deletions(-)

diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go
index 9aca3a7e8cf4..71586762df8b 100644
--- a/builtin/credential/approle/path_role_test.go
+++ b/builtin/credential/approle/path_role_test.go
@@ -12,24 +12,120 @@ import (
 	"github.com/mitchellh/mapstructure"
 )
 
+func TestApprole_LocalNonLocalSecretIDs(t *testing.T) {
+	var resp *logical.Response
+	var err error
+
+	b, storage := createBackendWithStorage(t)
+
+	// Create a role with enable_local_secret_ids set
+	resp, err = b.HandleRequest(context.Background(), &logical.Request{
+		Path:      "role/testrole1",
+		Operation: logical.CreateOperation,
+		Storage:   storage,
+		Data: map[string]interface{}{
+			"policies":                []string{"default", "role1policy"},
+			"bind_secret_id":          true,
+			"enable_local_secret_ids": true,
+		},
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: err: %v\n resp: %#v", err, resp)
+	}
+
+	// Create another role without setting enable_local_secret_ids
+	resp, err = b.HandleRequest(context.Background(), &logical.Request{
+		Path:      "role/testrole2",
+		Operation: logical.CreateOperation,
+		Storage:   storage,
+		Data: map[string]interface{}{
+			"policies":       []string{"default", "role1policy"},
+			"bind_secret_id": true,
+		},
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: err: %v\n resp: %#v", err, resp)
+	}
+
+	count := 10
+	// Create secret IDs on testrole1
+	for i := 0; i < count; i++ {
+		resp, err = b.HandleRequest(context.Background(), &logical.Request{
+			Path:      "role/testrole1/secret-id",
+			Operation: logical.UpdateOperation,
+			Storage:   storage,
+		})
+		if err != nil || (resp != nil && resp.IsError()) {
+			t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+		}
+	}
+
+	// Check the number of secret IDs generated
+	resp, err = b.HandleRequest(context.Background(), &logical.Request{
+		Path:      "role/testrole1/secret-id",
+		Operation: logical.ListOperation,
+		Storage:   storage,
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+	}
+	if len(resp.Data["keys"].([]string)) != count {
+		t.Fatalf("failed to list secret IDs")
+	}
+
+	// Create secret IDs on testrole1
+	for i := 0; i < count; i++ {
+		resp, err = b.HandleRequest(context.Background(), &logical.Request{
+			Path:      "role/testrole2/secret-id",
+			Operation: logical.UpdateOperation,
+			Storage:   storage,
+		})
+		if err != nil || (resp != nil && resp.IsError()) {
+			t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+		}
+	}
+
+	resp, err = b.HandleRequest(context.Background(), &logical.Request{
+		Path:      "role/testrole2/secret-id",
+		Operation: logical.ListOperation,
+		Storage:   storage,
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("bad: resp: %#v\nerr: %v", resp, err)
+	}
+	if len(resp.Data["keys"].([]string)) != count {
+		t.Fatalf("failed to list secret IDs")
+	}
+}
+
 func TestApprole_UpgradeSecretIDPrefix(t *testing.T) {
 	var resp *logical.Response
 	var err error
 
 	b, storage := createBackendWithStorage(t)
 
-	role := &roleStorageEntry{
+	// Create a role entry directly in storage without SecretIDPrefix
+	err = b.setRoleEntry(context.Background(), storage, "testrole", &roleStorageEntry{
 		RoleID:           "testroleid",
 		HMACKey:          "testhmackey",
 		Policies:         []string{"default"},
 		BindSecretID:     true,
 		BoundCIDRListOld: "127.0.0.1/18,192.178.1.2/24",
+	}, "")
+	if err != nil {
+		t.Fatal(err)
 	}
-	err = b.setRoleEntry(context.Background(), storage, "testrole", role, "")
+
+	// Reading the role entry should upgrade it to contain SecretIDPrefix
+	role, err := b.roleEntry(context.Background(), storage, "testrole")
 	if err != nil {
 		t.Fatal(err)
 	}
+	if role.SecretIDPrefix == "" {
+		t.Fatalf("expected SecretIDPrefix to be set")
+	}
 
+	// Ensure that the API response contains enable_local_secret_ids
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{
 		Path:      "role/testrole",
 		Operation: logical.ReadOperation,
@@ -57,6 +153,7 @@ func TestApprole_LocalSecretIDImmutability(t *testing.T) {
 		"enable_local_secret_ids": true,
 	}
 
+	// Create a role with enable_local_secret_ids set
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{
 		Path:      "role/testrole",
 		Operation: logical.CreateOperation,
@@ -67,13 +164,14 @@ func TestApprole_LocalSecretIDImmutability(t *testing.T) {
 		t.Fatalf("bad: err: %v\nresp: %#v", err, resp)
 	}
 
+	// Attempt to modify enable_local_secret_ids should fail
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{
 		Path:      "role/testrole",
 		Operation: logical.UpdateOperation,
 		Storage:   storage,
 		Data:      roleData,
 	})
-	if err == nil {
+	if resp == nil || !resp.IsError() {
 		t.Fatalf("expected an error since enable_local_secret_ids can't be overwritten")
 	}
 }

From 0962457bc85aff063092a0ebb5a84f8922acb8b1 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Tue, 24 Apr 2018 14:28:03 -0400
Subject: [PATCH 13/19] Fix api path for reading the field

---
 builtin/credential/approle/path_role.go | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index 3af3f08b0fe2..9c3b3b6ff3fc 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -185,21 +185,15 @@ can only be set during role creation and once set, it can't be reset later.`,
 			HelpDescription: strings.TrimSpace(roleHelp["role"][1]),
 		},
 		&framework.Path{
-			Pattern: "role/" + framework.GenericNameRegex("role_name") + "/enable_local_secret_ids$",
+			Pattern: "role/" + framework.GenericNameRegex("role_name") + "/enable-local-secret-ids$",
 			Fields: map[string]*framework.FieldSchema{
 				"role_name": &framework.FieldSchema{
 					Type:        framework.TypeString,
 					Description: "Name of the role.",
 				},
-				"enable_local_secret_ids": &framework.FieldSchema{
-					Type: framework.TypeBool,
-					Description: `
-If set, the secret IDs generated using this role will be cluster local. This
-can only be set during role creation and once set, it can't be reset later.`,
-				},
 			},
 			Callbacks: map[logical.Operation]framework.OperationFunc{
-				logical.ReadOperation: b.pathRoleLocalSecretIDsRead,
+				logical.ReadOperation: b.pathRoleEnableLocalSecretIDsRead,
 			},
 			HelpSynopsis:    strings.TrimSpace(roleHelp["role-local-secret-ids"][0]),
 			HelpDescription: strings.TrimSpace(roleHelp["role-local-secret-ids"][1]),
@@ -1459,7 +1453,7 @@ func (b *backend) pathRoleBindSecretIDDelete(ctx context.Context, req *logical.R
 	return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "")
 }
 
-func (b *backend) pathRoleLocalSecretIDsRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
+func (b *backend) pathRoleEnableLocalSecretIDsRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
 	roleName := data.Get("role_name").(string)
 	if roleName == "" {
 		return logical.ErrorResponse("missing role_name"), nil

From 33256ab446b5789b2ee4b8f489d4570c9ce1146e Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Tue, 24 Apr 2018 15:48:07 -0400
Subject: [PATCH 14/19] Add field read test

---
 builtin/credential/approle/path_role_test.go | 33 ++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go
index 71586762df8b..760ca1d872f7 100644
--- a/builtin/credential/approle/path_role_test.go
+++ b/builtin/credential/approle/path_role_test.go
@@ -12,6 +12,39 @@ import (
 	"github.com/mitchellh/mapstructure"
 )
 
+func TestAppRole_EnableLocalSecretIDsRead(t *testing.T) {
+	var resp *logical.Response
+	var err error
+	b, storage := createBackendWithStorage(t)
+
+	roleData := map[string]interface{}{
+		"enable_local_secret_ids": true,
+		"bind_secret_id":          true,
+	}
+
+	resp, err = b.HandleRequest(context.Background(), &logical.Request{
+		Operation: logical.CreateOperation,
+		Path:      "role/testrole",
+		Storage:   storage,
+		Data:      roleData,
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+
+	resp, err = b.HandleRequest(context.Background(), &logical.Request{
+		Operation: logical.ReadOperation,
+		Storage:   storage,
+		Path:      "role/testrole/enable-local-secret-ids",
+	})
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+	if !resp.Data["enable_local_secret_ids"].(bool) {
+		t.Fatalf("expected enable_local_secret_ids to be returned")
+	}
+}
+
 func TestApprole_LocalNonLocalSecretIDs(t *testing.T) {
 	var resp *logical.Response
 	var err error

From 3f92d9c8cae8204dddbdf5c5282af4108c531c92 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Tue, 24 Apr 2018 15:55:40 -0400
Subject: [PATCH 15/19] remove unneeded setting of secret ID prefix

---
 builtin/credential/approle/path_role.go | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index 9c3b3b6ff3fc..45a1f6f9f9ff 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -169,8 +169,7 @@ TTL will be set to the value of this parameter.`,
 				},
 				"enable_local_secret_ids": &framework.FieldSchema{
 					Type: framework.TypeBool,
-					Description: `
-If set, the secret IDs generated using this role will be cluster local. This
+					Description: `If set, the secret IDs generated using this role will be cluster local. This
 can only be set during role creation and once set, it can't be reset later.`,
 				},
 			},
@@ -810,18 +809,16 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request
 
 	localSecretIDsRaw, ok := data.GetOk("enable_local_secret_ids")
 	if ok {
-		if req.Operation == logical.CreateOperation {
+		switch {
+		case req.Operation == logical.CreateOperation:
 			localSecretIDs := localSecretIDsRaw.(bool)
 			if localSecretIDs {
 				role.SecretIDPrefix = secretIDLocalPrefix
 			}
-		} else {
+		default:
 			return logical.ErrorResponse("enable_local_secret_ids can only be modified during role creation"), nil
 		}
 	}
-	if role.SecretIDPrefix == "" {
-		role.SecretIDPrefix = secretIDPrefix
-	}
 
 	previousRoleID := role.RoleID
 	if roleIDRaw, ok := data.GetOk("role_id"); ok {
@@ -2343,8 +2340,8 @@ will pick up the new value during its next renewal.`,
 	},
 	"role-local-secret-ids": {
 		"Enables cluster local secret IDs",
-		`If set, indicates that the secret IDs generated using this role should be
-cluster local. This can only be set during role creation and once set, it can't
-be reset later.`,
+		`If set, the secret IDs generated using this role will be cluster local.
+This can only be set during role creation and once set, it can't be
+reset later.`,
 	},
 }

From 417b004697e69f9a358f973158ed572f46b99905 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Tue, 24 Apr 2018 16:03:18 -0400
Subject: [PATCH 16/19] fix typo

---
 builtin/credential/approle/path_tidy_user_id.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/credential/approle/path_tidy_user_id.go b/builtin/credential/approle/path_tidy_user_id.go
index 7fefb0dbdc4c..87d524ee485c 100644
--- a/builtin/credential/approle/path_tidy_user_id.go
+++ b/builtin/credential/approle/path_tidy_user_id.go
@@ -59,7 +59,7 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error {
 				return err
 			}
 			for _, secretIDHMAC := range secretIDHMACs {
-				// In order to avoid lock swroleing in case there is need to delete,
+				// In order to avoid lock swapping in case there is need to delete,
 				// grab the write lock.
 				lock := b.secretIDLock(secretIDHMAC)
 				lock.Lock()

From 419e70c1e2fc590177074b575ca79aba96a91f38 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Tue, 24 Apr 2018 16:17:24 -0400
Subject: [PATCH 17/19] refactor to be able to defer lock.Unlock()

---
 .../credential/approle/path_tidy_user_id.go   | 100 +++++++++---------
 1 file changed, 51 insertions(+), 49 deletions(-)

diff --git a/builtin/credential/approle/path_tidy_user_id.go b/builtin/credential/approle/path_tidy_user_id.go
index 87d524ee485c..a5f7e20c856a 100644
--- a/builtin/credential/approle/path_tidy_user_id.go
+++ b/builtin/credential/approle/path_tidy_user_id.go
@@ -52,69 +52,71 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error {
 			accessorMap[accessorHash] = true
 		}
 
-		for _, roleNameHMAC := range roleNameHMACs {
+		secretIDCleanupFunc := func(secretIDHMAC, roleNameHMAC, secretIDPrefixToUse string) error {
+			// In order to avoid lock swapping in case there is need to delete,
+			// grab the write lock.
+			lock := b.secretIDLock(secretIDHMAC)
+			lock.Lock()
+			defer lock.Unlock()
+
 			// roleNameHMAC will already have a '/' suffix. Don't append another one.
-			secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s", secretIDPrefixToUse, roleNameHMAC))
+			entryIndex := fmt.Sprintf("%s%s%s", secretIDPrefixToUse, roleNameHMAC, secretIDHMAC)
+			secretIDEntry, err := s.Get(ctx, entryIndex)
 			if err != nil {
-				return err
+				return errwrap.Wrapf(fmt.Sprintf("error fetching SecretID %q: {{err}}", secretIDHMAC), err)
 			}
-			for _, secretIDHMAC := range secretIDHMACs {
-				// In order to avoid lock swapping in case there is need to delete,
-				// grab the write lock.
-				lock := b.secretIDLock(secretIDHMAC)
-				lock.Lock()
-				// roleNameHMAC will already have a '/' suffix. Don't append another one.
-				entryIndex := fmt.Sprintf("%s%s%s", secretIDPrefixToUse, roleNameHMAC, secretIDHMAC)
-				secretIDEntry, err := s.Get(ctx, entryIndex)
-				if err != nil {
-					lock.Unlock()
-					return errwrap.Wrapf(fmt.Sprintf("error fetching SecretID %q: {{err}}", secretIDHMAC), err)
-				}
 
-				if secretIDEntry == nil {
-					result = multierror.Append(result, fmt.Errorf("entry for SecretID %q is nil", secretIDHMAC))
-					lock.Unlock()
-					continue
-				}
+			if secretIDEntry == nil {
+				result = multierror.Append(result, fmt.Errorf("entry for SecretID %q is nil", secretIDHMAC))
+				return nil
+			}
 
-				if secretIDEntry.Value == nil || len(secretIDEntry.Value) == 0 {
-					lock.Unlock()
-					return fmt.Errorf("found entry for SecretID %q but actual SecretID is empty", secretIDHMAC)
-				}
+			if secretIDEntry.Value == nil || len(secretIDEntry.Value) == 0 {
+				return fmt.Errorf("found entry for SecretID %q but actual SecretID is empty", secretIDHMAC)
+			}
 
-				var result secretIDStorageEntry
-				if err := secretIDEntry.DecodeJSON(&result); err != nil {
-					lock.Unlock()
+			var result secretIDStorageEntry
+			if err := secretIDEntry.DecodeJSON(&result); err != nil {
+				return err
+			}
+
+			// ExpirationTime not being set indicates non-expiring SecretIDs
+			if !result.ExpirationTime.IsZero() && time.Now().After(result.ExpirationTime) {
+				// Clean up the accessor of the secret ID first
+				err = b.deleteSecretIDAccessorEntry(ctx, s, result.SecretIDAccessor, secretIDPrefixToUse)
+				if err != nil {
 					return err
 				}
 
-				// ExpirationTime not being set indicates non-expiring SecretIDs
-				if !result.ExpirationTime.IsZero() && time.Now().After(result.ExpirationTime) {
-					// Clean up the accessor of the secret ID first
-					err = b.deleteSecretIDAccessorEntry(ctx, s, result.SecretIDAccessor, secretIDPrefixToUse)
-					if err != nil {
-						lock.Unlock()
-						return err
-					}
-
-					if err := s.Delete(ctx, entryIndex); err != nil {
-						lock.Unlock()
-						return errwrap.Wrapf(fmt.Sprintf("error deleting SecretID %q from storage: {{err}}", secretIDHMAC), err)
-					}
+				if err := s.Delete(ctx, entryIndex); err != nil {
+					return errwrap.Wrapf(fmt.Sprintf("error deleting SecretID %q from storage: {{err}}", secretIDHMAC), err)
 				}
+			}
+
+			// At this point, the secret ID is not expired and is valid. Delete
+			// the corresponding accessor from the accessorMap. This will leave
+			// only the dangling accessors in the map which can then be cleaned
+			// up later.
+			salt, err := b.Salt(ctx)
+			if err != nil {
+				return err
+			}
+			delete(accessorMap, salt.SaltID(result.SecretIDAccessor))
+
+			return nil
+		}
 
-				// At this point, the secret ID is not expired and is valid. Delete
-				// the corresponding accessor from the accessorMap. This will leave
-				// only the dangling accessors in the map which can then be cleaned
-				// up later.
-				salt, err := b.Salt(ctx)
+		for _, roleNameHMAC := range roleNameHMACs {
+			// roleNameHMAC will already have a '/' suffix. Don't append another one.
+			secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s", secretIDPrefixToUse, roleNameHMAC))
+			if err != nil {
+				return err
+			}
+			for _, secretIDHMAC := range secretIDHMACs {
+				err = secretIDCleanupFunc(secretIDHMAC, roleNameHMAC, secretIDPrefixToUse)
 				if err != nil {
-					lock.Unlock()
 					return err
 				}
-				delete(accessorMap, salt.SaltID(result.SecretIDAccessor))
-
-				lock.Unlock()
 			}
 		}
 

From 3c49d7b480d887266293c107fb335f238629da71 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Tue, 24 Apr 2018 16:28:25 -0400
Subject: [PATCH 18/19] remove unneeded comments

---
 builtin/credential/approle/path_tidy_user_id.go | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/builtin/credential/approle/path_tidy_user_id.go b/builtin/credential/approle/path_tidy_user_id.go
index a5f7e20c856a..9cab00d3daa5 100644
--- a/builtin/credential/approle/path_tidy_user_id.go
+++ b/builtin/credential/approle/path_tidy_user_id.go
@@ -53,13 +53,10 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error {
 		}
 
 		secretIDCleanupFunc := func(secretIDHMAC, roleNameHMAC, secretIDPrefixToUse string) error {
-			// In order to avoid lock swapping in case there is need to delete,
-			// grab the write lock.
 			lock := b.secretIDLock(secretIDHMAC)
 			lock.Lock()
 			defer lock.Unlock()
 
-			// roleNameHMAC will already have a '/' suffix. Don't append another one.
 			entryIndex := fmt.Sprintf("%s%s%s", secretIDPrefixToUse, roleNameHMAC, secretIDHMAC)
 			secretIDEntry, err := s.Get(ctx, entryIndex)
 			if err != nil {
@@ -107,7 +104,6 @@ func (b *backend) tidySecretID(ctx context.Context, s logical.Storage) error {
 		}
 
 		for _, roleNameHMAC := range roleNameHMACs {
-			// roleNameHMAC will already have a '/' suffix. Don't append another one.
 			secretIDHMACs, err := s.List(ctx, fmt.Sprintf("%s%s", secretIDPrefixToUse, roleNameHMAC))
 			if err != nil {
 				return err

From a030db2af8f7cbcb7e6b56559f20c886531fbd51 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Tue, 24 Apr 2018 17:52:42 -0400
Subject: [PATCH 19/19] s/enable_local_secret_ids/local_secret_ids

---
 builtin/credential/approle/path_role.go      | 36 ++++++++---------
 builtin/credential/approle/path_role_test.go | 42 ++++++++++----------
 2 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index 45a1f6f9f9ff..4c057b6d3561 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -167,7 +167,7 @@ TTL will be set to the value of this parameter.`,
 					Type:        framework.TypeString,
 					Description: "Identifier of the role. Defaults to a UUID.",
 				},
-				"enable_local_secret_ids": &framework.FieldSchema{
+				"local_secret_ids": &framework.FieldSchema{
 					Type: framework.TypeBool,
 					Description: `If set, the secret IDs generated using this role will be cluster local. This
 can only be set during role creation and once set, it can't be reset later.`,
@@ -184,7 +184,7 @@ can only be set during role creation and once set, it can't be reset later.`,
 			HelpDescription: strings.TrimSpace(roleHelp["role"][1]),
 		},
 		&framework.Path{
-			Pattern: "role/" + framework.GenericNameRegex("role_name") + "/enable-local-secret-ids$",
+			Pattern: "role/" + framework.GenericNameRegex("role_name") + "/local-secret-ids$",
 			Fields: map[string]*framework.FieldSchema{
 				"role_name": &framework.FieldSchema{
 					Type:        framework.TypeString,
@@ -192,7 +192,7 @@ can only be set during role creation and once set, it can't be reset later.`,
 				},
 			},
 			Callbacks: map[logical.Operation]framework.OperationFunc{
-				logical.ReadOperation: b.pathRoleEnableLocalSecretIDsRead,
+				logical.ReadOperation: b.pathRoleLocalSecretIDsRead,
 			},
 			HelpSynopsis:    strings.TrimSpace(roleHelp["role-local-secret-ids"][0]),
 			HelpDescription: strings.TrimSpace(roleHelp["role-local-secret-ids"][1]),
@@ -807,7 +807,7 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request
 		return logical.ErrorResponse(fmt.Sprintf("role name %q doesn't exist", roleName)), nil
 	}
 
-	localSecretIDsRaw, ok := data.GetOk("enable_local_secret_ids")
+	localSecretIDsRaw, ok := data.GetOk("local_secret_ids")
 	if ok {
 		switch {
 		case req.Operation == logical.CreateOperation:
@@ -816,7 +816,7 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request
 				role.SecretIDPrefix = secretIDLocalPrefix
 			}
 		default:
-			return logical.ErrorResponse("enable_local_secret_ids can only be modified during role creation"), nil
+			return logical.ErrorResponse("local_secret_ids can only be modified during role creation"), nil
 		}
 	}
 
@@ -948,20 +948,20 @@ func (b *backend) pathRoleRead(ctx context.Context, req *logical.Request, data *
 	}
 
 	respData := map[string]interface{}{
-		"bind_secret_id":          role.BindSecretID,
-		"bound_cidr_list":         role.BoundCIDRList,
-		"period":                  role.Period / time.Second,
-		"policies":                role.Policies,
-		"secret_id_num_uses":      role.SecretIDNumUses,
-		"secret_id_ttl":           role.SecretIDTTL / time.Second,
-		"token_max_ttl":           role.TokenMaxTTL / time.Second,
-		"token_num_uses":          role.TokenNumUses,
-		"token_ttl":               role.TokenTTL / time.Second,
-		"enable_local_secret_ids": false,
+		"bind_secret_id":     role.BindSecretID,
+		"bound_cidr_list":    role.BoundCIDRList,
+		"period":             role.Period / time.Second,
+		"policies":           role.Policies,
+		"secret_id_num_uses": role.SecretIDNumUses,
+		"secret_id_ttl":      role.SecretIDTTL / time.Second,
+		"token_max_ttl":      role.TokenMaxTTL / time.Second,
+		"token_num_uses":     role.TokenNumUses,
+		"token_ttl":          role.TokenTTL / time.Second,
+		"local_secret_ids":   false,
 	}
 
 	if role.SecretIDPrefix == secretIDLocalPrefix {
-		respData["enable_local_secret_ids"] = true
+		respData["local_secret_ids"] = true
 	}
 
 	resp := &logical.Response{
@@ -1450,7 +1450,7 @@ func (b *backend) pathRoleBindSecretIDDelete(ctx context.Context, req *logical.R
 	return nil, b.setRoleEntry(ctx, req.Storage, roleName, role, "")
 }
 
-func (b *backend) pathRoleEnableLocalSecretIDsRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
+func (b *backend) pathRoleLocalSecretIDsRead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
 	roleName := data.Get("role_name").(string)
 	if roleName == "" {
 		return logical.ErrorResponse("missing role_name"), nil
@@ -1471,7 +1471,7 @@ func (b *backend) pathRoleEnableLocalSecretIDsRead(ctx context.Context, req *log
 		}
 		return &logical.Response{
 			Data: map[string]interface{}{
-				"enable_local_secret_ids": localSecretIDs,
+				"local_secret_ids": localSecretIDs,
 			},
 		}, nil
 	}
diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go
index 760ca1d872f7..e3a757f74661 100644
--- a/builtin/credential/approle/path_role_test.go
+++ b/builtin/credential/approle/path_role_test.go
@@ -12,14 +12,14 @@ import (
 	"github.com/mitchellh/mapstructure"
 )
 
-func TestAppRole_EnableLocalSecretIDsRead(t *testing.T) {
+func TestAppRole_LocalSecretIDsRead(t *testing.T) {
 	var resp *logical.Response
 	var err error
 	b, storage := createBackendWithStorage(t)
 
 	roleData := map[string]interface{}{
-		"enable_local_secret_ids": true,
-		"bind_secret_id":          true,
+		"local_secret_ids": true,
+		"bind_secret_id":   true,
 	}
 
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{
@@ -35,13 +35,13 @@ func TestAppRole_EnableLocalSecretIDsRead(t *testing.T) {
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{
 		Operation: logical.ReadOperation,
 		Storage:   storage,
-		Path:      "role/testrole/enable-local-secret-ids",
+		Path:      "role/testrole/local-secret-ids",
 	})
 	if err != nil || (resp != nil && resp.IsError()) {
 		t.Fatalf("err:%v resp:%#v", err, resp)
 	}
-	if !resp.Data["enable_local_secret_ids"].(bool) {
-		t.Fatalf("expected enable_local_secret_ids to be returned")
+	if !resp.Data["local_secret_ids"].(bool) {
+		t.Fatalf("expected local_secret_ids to be returned")
 	}
 }
 
@@ -51,22 +51,22 @@ func TestApprole_LocalNonLocalSecretIDs(t *testing.T) {
 
 	b, storage := createBackendWithStorage(t)
 
-	// Create a role with enable_local_secret_ids set
+	// Create a role with local_secret_ids set
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{
 		Path:      "role/testrole1",
 		Operation: logical.CreateOperation,
 		Storage:   storage,
 		Data: map[string]interface{}{
-			"policies":                []string{"default", "role1policy"},
-			"bind_secret_id":          true,
-			"enable_local_secret_ids": true,
+			"policies":         []string{"default", "role1policy"},
+			"bind_secret_id":   true,
+			"local_secret_ids": true,
 		},
 	})
 	if err != nil || (resp != nil && resp.IsError()) {
 		t.Fatalf("bad: err: %v\n resp: %#v", err, resp)
 	}
 
-	// Create another role without setting enable_local_secret_ids
+	// Create another role without setting local_secret_ids
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{
 		Path:      "role/testrole2",
 		Operation: logical.CreateOperation,
@@ -158,7 +158,7 @@ func TestApprole_UpgradeSecretIDPrefix(t *testing.T) {
 		t.Fatalf("expected SecretIDPrefix to be set")
 	}
 
-	// Ensure that the API response contains enable_local_secret_ids
+	// Ensure that the API response contains local_secret_ids
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{
 		Path:      "role/testrole",
 		Operation: logical.ReadOperation,
@@ -167,9 +167,9 @@ func TestApprole_UpgradeSecretIDPrefix(t *testing.T) {
 	if err != nil || (resp != nil && resp.IsError()) {
 		t.Fatalf("bad: err: %v\n resp: %#v", err, resp)
 	}
-	_, ok := resp.Data["enable_local_secret_ids"]
+	_, ok := resp.Data["local_secret_ids"]
 	if !ok {
-		t.Fatalf("expected enable_local_secret_ids to be present in the response")
+		t.Fatalf("expected local_secret_ids to be present in the response")
 	}
 }
 
@@ -180,13 +180,13 @@ func TestApprole_LocalSecretIDImmutability(t *testing.T) {
 	b, storage := createBackendWithStorage(t)
 
 	roleData := map[string]interface{}{
-		"policies":                []string{"default"},
-		"bind_secret_id":          true,
-		"bound_cidr_list":         []string{"127.0.0.1/18", "192.178.1.2/24"},
-		"enable_local_secret_ids": true,
+		"policies":         []string{"default"},
+		"bind_secret_id":   true,
+		"bound_cidr_list":  []string{"127.0.0.1/18", "192.178.1.2/24"},
+		"local_secret_ids": true,
 	}
 
-	// Create a role with enable_local_secret_ids set
+	// Create a role with local_secret_ids set
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{
 		Path:      "role/testrole",
 		Operation: logical.CreateOperation,
@@ -197,7 +197,7 @@ func TestApprole_LocalSecretIDImmutability(t *testing.T) {
 		t.Fatalf("bad: err: %v\nresp: %#v", err, resp)
 	}
 
-	// Attempt to modify enable_local_secret_ids should fail
+	// Attempt to modify local_secret_ids should fail
 	resp, err = b.HandleRequest(context.Background(), &logical.Request{
 		Path:      "role/testrole",
 		Operation: logical.UpdateOperation,
@@ -205,7 +205,7 @@ func TestApprole_LocalSecretIDImmutability(t *testing.T) {
 		Data:      roleData,
 	})
 	if resp == nil || !resp.IsError() {
-		t.Fatalf("expected an error since enable_local_secret_ids can't be overwritten")
+		t.Fatalf("expected an error since local_secret_ids can't be overwritten")
 	}
 }