User Lockout Perf Standby Error oss (#20783)

* adding changes from ent

* add changelog

* removing new line

* rename changelog file name

changelog/20783.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+core: Fix writes to readonly storage on performance standbys when user lockout feature is enabled.
+```

vault/core.go

@@ -3546,18 +3546,19 @@ func (c *Core) runLockedUserEntryUpdatesForMountAccessor(ctx context.Context, mo
 		lockoutDurationFromConfiguration := userLockoutConfiguration.LockoutDuration
 
 		// get the entry for the locked user from userFailedLoginInfo map
-		failedLoginInfoFromMap := c.GetUserFailedLoginInfo(ctx, loginUserInfoKey)
+		failedLoginInfoFromMap := c.LocalGetUserFailedLoginInfo(ctx, loginUserInfoKey)
 
 		// check if the storage entry for locked user is stale
 		if time.Now().After(lastFailedLoginTimeFromStorageEntry.Add(lockoutDurationFromConfiguration)) {
 			// stale entry, remove from storage
+			// leaving this as it is as this happens on the active node
+			// also handles case where namespace is deleted
 			if err := c.barrier.Delete(ctx, path+alias); err != nil {
 				return 0, err
 			}
-
 			// remove entry for this user from userFailedLoginInfo map if present as the user is not locked
 			if failedLoginInfoFromMap != nil {
-				if err = c.UpdateUserFailedLoginInfo(ctx, loginUserInfoKey, nil, true); err != nil {
+				if err = updateUserFailedLoginInfo(ctx, c, loginUserInfoKey, nil, true); err != nil {
 					return 0, err
 				}
 			}
@@ -3574,7 +3575,7 @@ func (c *Core) runLockedUserEntryUpdatesForMountAccessor(ctx context.Context, mo
 
 		if failedLoginInfoFromMap != &actualFailedLoginInfo {
 			// entry is invalid, updating the entry in userFailedLoginMap with correct information
-			if err = c.UpdateUserFailedLoginInfo(ctx, loginUserInfoKey, &actualFailedLoginInfo, false); err != nil {
+			if err = updateUserFailedLoginInfo(ctx, c, loginUserInfoKey, &actualFailedLoginInfo, false); err != nil {
 				return 0, err
 			}
 		}

vault/core_test.go

@@ -482,7 +482,7 @@ func TestCore_RunLockedUserUpdatesForValidEntry(t *testing.T) {
 		mountAccessor: "mountAccessor1",
 	}
 
-	failedLoginInfoFromMap := core.GetUserFailedLoginInfo(context.Background(), loginUserInfoKey)
+	failedLoginInfoFromMap := core.LocalGetUserFailedLoginInfo(context.Background(), loginUserInfoKey)
 	if failedLoginInfoFromMap == nil {
 		t.Fatalf("err: entry must exist for locked user in userFailedLoginInfo map")
 	}

vault/external_tests/identity/userlockouts_test.go

@@ -4,9 +4,9 @@
 package identity
 
 import (
+	"os"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/builtin/credential/userpass"
@@ -15,94 +15,21 @@ import (
 	"github.com/hashicorp/vault/vault"
 )
 
-// TestIdentityStore_UserLockoutTest tests that the user gets locked after
+const (
-// more than 1 failed login request than the number specified for
+	UserLockoutThresholdDefault = 5
-// lockout threshold field in user lockout configuration. It also
+)
-// tests that the user gets unlocked after the duration specified
-// for lockout duration field has passed
-func TestIdentityStore_UserLockoutTest(t *testing.T) {
-	coreConfig := &vault.CoreConfig{
-		CredentialBackends: map[string]logical.Factory{
-			"userpass": userpass.Factory,
-		},
-	}
-	cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
-		HandlerFunc: vaulthttp.Handler,
-	})
-	cluster.Start()
-	defer cluster.Cleanup()
-	active := cluster.Cores[0].Client
-	standby := cluster.Cores[1].Client
 
-	err := active.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{
+// TestIdentityStore_DisableUserLockoutTest tests that user login will
-		Type: "userpass",
+// fail when supplied with wrong credentials. If the user is locked,
-	})
+// it returns permission denied. Otherwise, it returns invalid user
-	if err != nil {
+// credentials error if the user lockout feature is disabled.
-		t.Fatal(err)
+// It tests disabling the feature using env variable VAULT_DISABLE_USER_LOCKOUT
-	}
+// and also using auth tune. Also, tests that env var has more precedence over
+// settings in auth tune.
+func TestIdentityStore_DisableUserLockoutTest(t *testing.T) {
+	// reset to false before exiting
+	defer os.Unsetenv("VAULT_DISABLE_USER_LOCKOUT")
 
-	// tune auth mount
-	userlockoutConfig := &api.UserLockoutConfigInput{
-		LockoutThreshold:            "3",
-		LockoutDuration:             "5s",
-		LockoutCounterResetDuration: "5s",
-	}
-	err = active.Sys().TuneMount("auth/userpass", api.MountConfigInput{
-		UserLockoutConfig: userlockoutConfig,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// create a user for userpass
-	_, err = standby.Logical().Write("auth/userpass/users/bsmith", map[string]interface{}{
-		"password": "training",
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// login failure count 1
-	standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-
-	// login failure count 2
-	standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-
-	// login failure count 3
-	active.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-
-	// login : permission denied as user locked out
-	_, err = standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "training",
-	})
-	if err == nil {
-		t.Fatal("expected login to fail as user locked out")
-	}
-	if !strings.Contains(err.Error(), logical.ErrPermissionDenied.Error()) {
-		t.Fatalf("expected to see permission denied error as user locked out, got %v", err)
-	}
-
-	time.Sleep(5 * time.Second)
-
-	// login with right password and wait for user to get unlocked
-	_, err = standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "training",
-	})
-	if err != nil {
-		t.Fatal("expected login to succeed as user is unlocked")
-	}
-}
-
-// TestIdentityStore_UserFailedLoginMapResetOnSuccess tests that
-// the user lockout feature is reset for a user after one successfull attempt
-// after multiple failed login attempts (within lockout threshold)
-func TestIdentityStore_UserFailedLoginMapResetOnSuccess(t *testing.T) {
 	coreConfig := &vault.CoreConfig{
 		CredentialBackends: map[string]logical.Factory{
 			"userpass": userpass.Factory,
@@ -114,8 +41,10 @@ func TestIdentityStore_UserFailedLoginMapResetOnSuccess(t *testing.T) {
 	cluster.Start()
 	defer cluster.Cleanup()
 
-	client := cluster.Cores[0].Client
+	// standby client
+	client := cluster.Cores[1].Client
 
+	// enable userpass
 	err := client.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{
 		Type: "userpass",
 	})
@@ -123,20 +52,7 @@ func TestIdentityStore_UserFailedLoginMapResetOnSuccess(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// tune auth mount
+	// create a userpass user
-	userlockoutConfig := &api.UserLockoutConfigInput{
-		LockoutThreshold:            "3",
-		LockoutDuration:             "5s",
-		LockoutCounterResetDuration: "5s",
-	}
-	err = client.Sys().TuneMount("auth/userpass", api.MountConfigInput{
-		UserLockoutConfig: userlockoutConfig,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// create a user for userpass
 	_, err = client.Logical().Write("auth/userpass/users/bsmith", map[string]interface{}{
 		"password": "training",
 	})
@@ -144,308 +60,119 @@ func TestIdentityStore_UserFailedLoginMapResetOnSuccess(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// login failure count 1
-	client.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-
-	// login failure count 2
-	client.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-
-	// login with right credentials - successful login
-	// entry for this user is removed from userFailedLoginInfo map
-	_, err = client.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "training",
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// login failure count 3, is now count 1 after successful login
-	client.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-
-	// login failure count 4, is now count 2 after successful login
-	// error should not be permission denied as user not locked out
-	_, err = client.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-	if err == nil {
-		t.Fatal("expected login to fail due to wrong credentials")
-	}
-	if !strings.Contains(err.Error(), "invalid username or password") {
-		t.Fatalf("expected to see invalid username or password error as user is not locked out, got %v", err)
-	}
-}
-
-// TestIdentityStore_DisableUserLockoutTest tests that user login will
-// fail when supplied with wrong credentials. If the user is locked,
-// it returns permission denied. In this case, it returns invalid user
-// credentials error as the user lockout feature is disabled and the
-// user did not get locked after multiple failed login attempts
-func TestIdentityStore_DisableUserLockoutTest(t *testing.T) {
-	coreConfig := &vault.CoreConfig{
-		CredentialBackends: map[string]logical.Factory{
-			"userpass": userpass.Factory,
-		},
-	}
-	cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
-		HandlerFunc: vaulthttp.Handler,
-	})
-	cluster.Start()
-	defer cluster.Cleanup()
-
-	active := cluster.Cores[0].Client
-	standby := cluster.Cores[1].Client
-
-	err := active.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{
-		Type: "userpass",
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// tune auth mount
-	disableLockout := true
-	userlockoutConfig := &api.UserLockoutConfigInput{
-		LockoutThreshold: "3",
-		DisableLockout:   &disableLockout,
-	}
-	err = active.Sys().TuneMount("auth/userpass", api.MountConfigInput{
-		UserLockoutConfig: userlockoutConfig,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// create a userpass user
-	_, err = standby.Logical().Write("auth/userpass/users/bsmith", map[string]interface{}{
-		"password": "training",
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// login failure count 1
-	standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-
-	// login failure count 2
-	standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-
-	// login failure count 3
-	active.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-
-	// login failure count 4
-	_, err = standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-	if err == nil {
-		t.Fatal("expected login to fail due to wrong credentials")
-	}
-	if !strings.Contains(err.Error(), "invalid username or password") {
-		t.Fatalf("expected to see invalid username or password error as user is not locked out, got %v", err)
-	}
-}
-
-// TestIdentityStore_LockoutCounterResetTest tests that the user lockout counter
-// for a user is reset after no failed login attempts for a duration
-// as specified for lockout counter reset field in user lockout configuration
-func TestIdentityStore_LockoutCounterResetTest(t *testing.T) {
-	coreConfig := &vault.CoreConfig{
-		CredentialBackends: map[string]logical.Factory{
-			"userpass": userpass.Factory,
-		},
-	}
-	cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
-		HandlerFunc: vaulthttp.Handler,
-	})
-	cluster.Start()
-	defer cluster.Cleanup()
-	active := cluster.Cores[0].Client
-	standby := cluster.Cores[1].Client
-
-	err := active.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{
-		Type: "userpass",
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// tune auth mount
-	userlockoutConfig := &api.UserLockoutConfigInput{
-		LockoutThreshold:            "3",
-		LockoutCounterResetDuration: "5s",
-	}
-	err = active.Sys().TuneMount("auth/userpass", api.MountConfigInput{
-		UserLockoutConfig: userlockoutConfig,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// create a user for userpass
-	_, err = standby.Logical().Write("auth/userpass/users/bsmith", map[string]interface{}{
-		"password": "training",
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// login failure count 1
-	standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-	// login failure count 2
-	standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-
-	// set sleep timer to reset login counter
-	time.Sleep(5 * time.Second)
-
-	// login failure 3, count should be reset, this will be treated as failed count 1
-	active.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-	// login failure 4, this will be treated as failed count 2
-	_, err = standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-	if err == nil {
-		t.Fatal("expected login to fail due to wrong credentials")
-	}
-	if !strings.Contains(err.Error(), "invalid username or password") {
-		t.Fatalf("expected to see invalid username or password error as user is not locked out, got %v", err)
-	}
-}
-
-// TestIdentityStore_UnlockUserTest tests the user is
-// unlocked if locked  using
-// sys/locked-users/[mount_accessor]/unlock/[alias-identifier]
-func TestIdentityStore_UnlockUserTest(t *testing.T) {
-	coreConfig := &vault.CoreConfig{
-		CredentialBackends: map[string]logical.Factory{
-			"userpass": userpass.Factory,
-		},
-	}
-	cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{
-		HandlerFunc: vaulthttp.Handler,
-	})
-	cluster.Start()
-	defer cluster.Cleanup()
-	active := cluster.Cores[0].Client
-	standby := cluster.Cores[1].Client
-
-	// enable userpass auth method on path userpass
-	if err := active.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{
-		Type: "userpass",
-	}); err != nil {
-		t.Fatal(err)
-	}
-
 	// get mount accessor for userpass mount
-	secret, err := standby.Logical().Read("sys/auth/userpass")
+	secret, err := client.Logical().Read("sys/auth/userpass")
 	if err != nil || secret == nil {
 		t.Fatal(err)
 	}
 	mountAccessor := secret.Data["accessor"].(string)
 
-	// tune auth mount
+	// variables for auth tune
-	userlockoutConfig := &api.UserLockoutConfigInput{
+	disableLockout := true
-		LockoutThreshold: "2",
+	enableLockout := false
-		LockoutDuration:  "5m",
-	}
-	if err = active.Sys().TuneMount("auth/userpass", api.MountConfigInput{
-		UserLockoutConfig: userlockoutConfig,
-	}); err != nil {
-		t.Fatal(err)
-	}
 
-	// create a user for userpass
+	tests := []struct {
-	if _, err = standby.Logical().Write("auth/userpass/users/bsmith", map[string]interface{}{
+		name                        string
-		"password": "training",
+		setDisableUserLockoutEnvVar string
-	}); err != nil {
+		// default is false
-		t.Fatal(err)
+		setDisableLockoutAuthTune bool
+		expectedUserLocked        bool
+	}{
+		{
+			name:                        "Both unset, uses default behaviour i.e; user lockout feature enabled",
+			setDisableUserLockoutEnvVar: "",
+			setDisableLockoutAuthTune:   false,
+			expectedUserLocked:          true,
+		},
+		{
+			name:                        "User lockout feature is disabled using auth tune",
+			setDisableUserLockoutEnvVar: "",
+			setDisableLockoutAuthTune:   true,
+			expectedUserLocked:          false,
+		},
+		{
+			name:                        "User Lockout feature is disabled using env var VAULT_DISABLE_USER_LOCKOUT",
+			setDisableUserLockoutEnvVar: "true",
+			setDisableLockoutAuthTune:   false,
+			expectedUserLocked:          false,
+		},
+		{
+			name:                        "User lockout feature is enabled using env variable, disabled using auth tune",
+			setDisableUserLockoutEnvVar: "false",
+			setDisableLockoutAuthTune:   true,
+			expectedUserLocked:          true,
+		},
+		{
+			name:                        "User lockout feature is disabled using auth tune and env variable",
+			setDisableUserLockoutEnvVar: "true",
+			setDisableLockoutAuthTune:   true,
+			expectedUserLocked:          false,
+		},
 	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.setDisableUserLockoutEnvVar != "" {
+				os.Setenv("VAULT_DISABLE_USER_LOCKOUT", tt.setDisableUserLockoutEnvVar)
+			} else {
+				os.Unsetenv("VAULT_DISABLE_USER_LOCKOUT")
+			}
 
-	// create another user for userpass with a different case
+			var disableLockoutAuthTune *bool
-	if _, err = standby.Logical().Write("auth/userpass/users/bSmith", map[string]interface{}{
-		"password": "training",
-	}); err != nil {
-		t.Fatal(err)
-	}
 
-	// login failure count 1
+			// default for disable lockout is false
-	standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
+			disableLockoutAuthTune = &enableLockout
-		"password": "wrongPassword",
-	})
-	// login failure count 2
-	standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "wrongPassword",
-	})
-	// login : permission denied as user locked out
-	if _, err = standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		"password": "training",
-	}); err == nil {
-		t.Fatal("expected login to fail as user locked out")
-	}
-	if !strings.Contains(err.Error(), logical.ErrPermissionDenied.Error()) {
-		t.Fatalf("expected to see permission denied error as user locked out, got %v", err)
-	}
 
-	// unlock user
+			if tt.setDisableLockoutAuthTune == true {
-	if _, err = standby.Logical().Write("sys/locked-users/"+mountAccessor+"/unlock/bsmith", nil); err != nil {
+				disableLockoutAuthTune = &disableLockout
-		t.Fatal(err)
+			}
-	}
 
-	// login: should be successful as user unlocked
+			// tune auth mount
-	if _, err = standby.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
+			userlockoutConfig := &api.UserLockoutConfigInput{
-		"password": "training",
+				DisableLockout: disableLockoutAuthTune,
-	}); err != nil {
+			}
-		t.Fatal("expected login to succeed as user is unlocked")
+			err := client.Sys().TuneMount("auth/userpass", api.MountConfigInput{
-	}
+				UserLockoutConfig: userlockoutConfig,
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
 
-	// login failure count 1 for user bSmith
+			// login for default lockout threshold times with wrong credentials
-	standby.Logical().Write("auth/userpass/login/bSmith", map[string]interface{}{
+			for i := 0; i < UserLockoutThresholdDefault; i++ {
-		"password": "wrongPassword",
+				_, err = client.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-	})
+					"password": "wrongPassword",
-	// login failure count 2 for user bSmith
+				})
-	standby.Logical().Write("auth/userpass/login/bSmith", map[string]interface{}{
+				if err == nil {
-		"password": "wrongPassword",
+					t.Fatal("expected login to fail due to wrong credentials")
-	})
+				}
-	// login : permission denied as user locked out for user bSmith
+				if !strings.Contains(err.Error(), "invalid username or password") {
-	if _, err = standby.Logical().Write("auth/userpass/login/bSmith", map[string]interface{}{
+					t.Fatal(err)
-		"password": "training",
+				}
-	}); err == nil {
+			}
-		t.Fatal("expected login to fail as user locked out")
-	}
-	if !strings.Contains(err.Error(), logical.ErrPermissionDenied.Error()) {
-		t.Fatalf("expected to see permission denied error as user locked out, got %v", err)
-	}
 
-	// unlock user bSmith
+			// login to check if user locked
-	if _, err = standby.Logical().Write("sys/locked-users/"+mountAccessor+"/unlock/bSmith", nil); err != nil {
+			_, err = client.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{
-		t.Fatal(err)
+				"password": "wrongPassword",
-	}
+			})
+			if err == nil {
+				t.Fatal("expected login to fail due to wrong credentials")
+			}
 
-	// login: should be successful as user bSmith unlocked
+			switch tt.expectedUserLocked {
-	if _, err = standby.Logical().Write("auth/userpass/login/bSmith", map[string]interface{}{
+			case true:
-		"password": "training",
+				if !strings.Contains(err.Error(), logical.ErrPermissionDenied.Error()) {
-	}); err != nil {
+					t.Fatalf("expected user to get locked but got %v", err)
-		t.Fatal("expected login to succeed as user is unlocked")
+				}
-	}
+				// user locked, unlock user to perform next test iteration
+				if _, err = client.Logical().Write("sys/locked-users/"+mountAccessor+"/unlock/bsmith", nil); err != nil {
+					t.Fatal(err)
+				}
 
-	// unlock unlocked user
+			default:
-	if _, err = active.Logical().Write("sys/locked-users/mountAccessor/unlock/bsmith", nil); err != nil {
+				if !strings.Contains(err.Error(), "invalid username or password") {
-		t.Fatal(err)
+					t.Fatalf("expected user to be unlocked but locked, got  %v", err)
+				}
+			}
+		})
 	}
 }

vault/logical_system_user_lockout.go

@@ -35,6 +35,8 @@ func unlockUser(ctx context.Context, core *Core, mountAccessor string, aliasName
 	lockedUserStoragePath := coreLockedUsersPath + ns.ID + "/" + mountAccessor + "/" + aliasName
 
 	// remove entry for locked user from storage
+	// if read only error, the error is handled by handleError in logical_system.go
+	// this will be forwarded to the active node
 	if err := core.barrier.Delete(ctx, lockedUserStoragePath); err != nil {
 		return err
 	}
@@ -44,7 +46,7 @@ func unlockUser(ctx context.Context, core *Core, mountAccessor string, aliasName
 		mountAccessor: mountAccessor,
 	}
 
-	// remove entry for locked user from userFailedLoginInfo map
+	// remove entry for locked user from userFailedLoginInfo map and storage
 	if err := updateUserFailedLoginInfo(ctx, core, loginUserInfoKey, nil, true); err != nil {
 		return err
 	}

vault/request_handling.go

@@ -1402,9 +1402,11 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
 
 	// if routeErr has invalid credentials error, update the userFailedLoginMap
 	if routeErr != nil && routeErr == logical.ErrInvalidCredentials {
-		err := c.failedUserLoginProcess(ctx, entry, req)
+		if !isUserLockoutDisabled {
-		if err != nil {
+			err := c.failedUserLoginProcess(ctx, entry, req)
-			return nil, nil, err
+			if err != nil {
+				return nil, nil, err
+			}
 		}
 		return nil, nil, resp.Error()
 	}
@@ -1689,6 +1691,10 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
 			aliasName:     auth.Alias.Name,
 			mountAccessor: auth.Alias.MountAccessor,
 		}
+
+		// We don't need to try to delete the lockedUsers storage entry, since we're
+		// processing a login request. If a login attempt is allowed, it means the user is
+		// unlocked and we only add storage entry when the user gets locked.
 		err = updateUserFailedLoginInfo(ctx, c, loginUserInfoKey, nil, true)
 		if err != nil {
 			return nil, nil, err
@@ -1855,39 +1861,12 @@ func (c *Core) failedUserLoginProcess(ctx context.Context, mountEntry *MountEntr
 		}
 	}
 
-	// update the userFailedLoginInfo map with the updated/new entry
+	// update the userFailedLoginInfo map (and/or storage) with the updated/new entry
 	err = updateUserFailedLoginInfo(ctx, c, loginUserInfoKey, &failedLoginInfo, false)
 	if err != nil {
 		return err
 	}
 
-	// if failed login count has reached threshold, create a storage entry as the user got locked
-	if failedLoginInfo.count >= uint(userLockoutConfiguration.LockoutThreshold) {
-		// user locked
-		ns, err := namespace.FromContext(ctx)
-		if err != nil {
-			return fmt.Errorf("could not parse namespace from http context: %w", err)
-		}
-		storageUserLockoutPath := fmt.Sprintf(coreLockedUsersPath+"%s/%s/%s", ns.ID, loginUserInfoKey.mountAccessor, loginUserInfoKey.aliasName)
-		compressedBytes, err := jsonutil.EncodeJSONAndCompress(failedLoginInfo.lastFailedLoginTime, nil)
-		if err != nil {
-			c.logger.Error("failed to encode or compress failed login user entry", "error", err)
-			return err
-		}
-
-		// Create an entry
-		entry := &logical.StorageEntry{
-			Key:   storageUserLockoutPath,
-			Value: compressedBytes,
-		}
-
-		// Write to the physical backend
-		if err := c.barrier.Put(ctx, entry); err != nil {
-			c.logger.Error("failed to persist failed login user entry", "error", err)
-			return err
-		}
-
-	}
 	return nil
 }
 
@@ -1916,16 +1895,16 @@ func (c *Core) isUserLockoutDisabled(mountEntry *MountEntry) (bool, error) {
 	}
 
 	// check environment variable
-	var disableUserLockout bool
 	if disableUserLockoutEnv := os.Getenv(consts.VaultDisableUserLockout); disableUserLockoutEnv != "" {
 		var err error
-		disableUserLockout, err = strconv.ParseBool(disableUserLockoutEnv)
+		disableUserLockout, err := strconv.ParseBool(disableUserLockoutEnv)
 		if err != nil {
 			return false, errors.New("Error parsing the environment variable VAULT_DISABLE_USER_LOCKOUT")
 		}
-	}
+		if disableUserLockout {
-	if disableUserLockout {
+			return true, nil
-		return true, nil
+		}
+		return false, nil
 	}
 
 	// read auth tune for mount entry
@@ -1986,13 +1965,11 @@ func (c *Core) isUserLocked(ctx context.Context, mountEntry *MountEntry, req *lo
 		if time.Now().Unix()-int64(lastLoginTime) < int64(userLockoutConfiguration.LockoutDuration.Seconds()) {
 			// user locked
 			return true, nil
-		} else {
-			// user is not locked. Entry is stale, remove this from storage
-			if err := c.barrier.Delete(ctx, storageUserLockoutPath); err != nil {
-				c.logger.Error("failed to cleanup storage entry for user", "path", storageUserLockoutPath, "error", err)
-			}
 		}
 
+		// else user is not locked. Entry is stale, this will be removed from storage during cleanup
+		// by the background thread
+
 	default:
 		// entry found in userFailedLoginInfo map, check if the user is locked
 		isCountOverLockoutThreshold := userFailedLoginInfo.count >= uint(userLockoutConfiguration.LockoutThreshold)
@@ -2185,7 +2162,11 @@ func (c *Core) RegisterAuth(ctx context.Context, tokenTTL time.Duration, path st
 				aliasName:     auth.Alias.Name,
 				mountAccessor: auth.Alias.MountAccessor,
 			}
-			err = c.UpdateUserFailedLoginInfo(ctx, loginUserInfoKey, nil, true)
+
+			// We don't need to try to delete the lockedUsers storage entry, since we're
+			// processing a login request. If a login attempt is allowed, it means the user is
+			// unlocked and we only add storage entry when the user gets locked.
+			err = updateUserFailedLoginInfo(ctx, c, loginUserInfoKey, nil, true)
 			if err != nil {
 				return err
 			}
@@ -2194,8 +2175,8 @@ func (c *Core) RegisterAuth(ctx context.Context, tokenTTL time.Duration, path st
 	return nil
 }
 
-// GetUserFailedLoginInfo gets the failed login information for a user based on alias name and mountAccessor
+// LocalGetUserFailedLoginInfo gets the failed login information for a user based on alias name and mountAccessor
-func (c *Core) GetUserFailedLoginInfo(ctx context.Context, userKey FailedLoginUser) *FailedLoginInfo {
+func (c *Core) LocalGetUserFailedLoginInfo(ctx context.Context, userKey FailedLoginUser) *FailedLoginInfo {
 	c.userFailedLoginInfoLock.Lock()
 	value, exists := c.userFailedLoginInfo[userKey]
 	c.userFailedLoginInfoLock.Unlock()
@@ -2205,23 +2186,52 @@ func (c *Core) GetUserFailedLoginInfo(ctx context.Context, userKey FailedLoginUs
 	return nil
 }
 
-// UpdateUserFailedLoginInfo updates the failed login information for a user based on alias name and mountAccessor
+// LocalUpdateUserFailedLoginInfo updates the failed login information for a user based on alias name and mountAccessor
-func (c *Core) UpdateUserFailedLoginInfo(ctx context.Context, userKey FailedLoginUser, failedLoginInfo *FailedLoginInfo, deleteEntry bool) error {
+func (c *Core) LocalUpdateUserFailedLoginInfo(ctx context.Context, userKey FailedLoginUser, failedLoginInfo *FailedLoginInfo, deleteEntry bool) error {
 	c.userFailedLoginInfoLock.Lock()
 	switch deleteEntry {
 	case false:
-		// create or update entry in the map
+		// update entry in the map
 		c.userFailedLoginInfo[userKey] = failedLoginInfo
+
+		// get the user lockout configuration for the user
+		mountEntry := c.router.MatchingMountByAccessor(userKey.mountAccessor)
+		if mountEntry == nil {
+			mountEntry = &MountEntry{}
+			mountEntry.NamespaceID = namespace.RootNamespaceID
+		}
+		userLockoutConfiguration := c.getUserLockoutConfiguration(mountEntry)
+
+		// if failed login count has reached threshold, create a storage entry as the user got locked
+		if failedLoginInfo.count >= uint(userLockoutConfiguration.LockoutThreshold) {
+			// user locked
+			storageUserLockoutPath := fmt.Sprintf(coreLockedUsersPath+"%s/%s/%s", mountEntry.NamespaceID, userKey.mountAccessor, userKey.aliasName)
+
+			compressedBytes, err := jsonutil.EncodeJSONAndCompress(failedLoginInfo.lastFailedLoginTime, nil)
+			if err != nil {
+				c.logger.Error("failed to encode or compress failed login user entry", "error", err)
+				return err
+			}
+
+			// Create an entry
+			entry := &logical.StorageEntry{
+				Key:   storageUserLockoutPath,
+				Value: compressedBytes,
+			}
+
+			// Write to the physical backend
+			if err := c.barrier.Put(ctx, entry); err != nil {
+				c.logger.Error("failed to persist failed login user entry", "error", err)
+				return err
+			}
+
+		}
+
 	default:
-		// delete the entry from the map
+		// delete the entry from the map, if no key exists it is no-op
 		delete(c.userFailedLoginInfo, userKey)
 	}
 	c.userFailedLoginInfoLock.Unlock()
-	// check if the update worked
-	failedLoginResp := c.GetUserFailedLoginInfo(ctx, userKey)
-	if (failedLoginResp == nil && !deleteEntry) || (failedLoginResp != nil && deleteEntry) {
-		return fmt.Errorf("failed to update entry in userFailedLoginInfo map")
-	}
 	return nil
 }
 

vault/request_handling_util.go

@@ -55,11 +55,11 @@ func getAuthRegisterFunc(c *Core) (RegisterAuthFunc, error) {
 }
 
 func getUserFailedLoginInfo(ctx context.Context, c *Core, userInfo FailedLoginUser) (*FailedLoginInfo, error) {
-	return c.GetUserFailedLoginInfo(ctx, userInfo), nil
+	return c.LocalGetUserFailedLoginInfo(ctx, userInfo), nil
 }
 
 func updateUserFailedLoginInfo(ctx context.Context, c *Core, userInfo FailedLoginUser, failedLoginInfo *FailedLoginInfo, deleteEntry bool) error {
-	return c.UpdateUserFailedLoginInfo(ctx, userInfo, failedLoginInfo, deleteEntry)
+	return c.LocalUpdateUserFailedLoginInfo(ctx, userInfo, failedLoginInfo, deleteEntry)
 }
 
 func possiblyForwardAliasCreation(ctx context.Context, c *Core, inErr error, auth *logical.Auth, entity *identity.Entity) (*identity.Entity, bool, error) {