move logger start function call closer to lockout entry creation

vault/core.go

@@ -3662,7 +3662,10 @@ func (c *Core) setupCachedMFAResponseAuth() {
 	return
 }
 
-func (c *Core) startLockoutLogger() {
+// startLockoutLogger starts a background goroutine to emit a log while a user lockout
+// exists anywhere in Vault. locked should be set to true if we need to hold the
+// userFailedLoginInfoLock when getting the locked user count.
+func (c *Core) startLockoutLogger(locked bool) {
 	// Are we already running a logger
 	if c.lockoutLoggerCancel.Load() != nil {
 		return
@@ -3672,7 +3675,7 @@ func (c *Core) startLockoutLogger() {
 	c.lockoutLoggerCancel.Store(&cancelFunc)
 
 	// Perform first check for lockout entries
-	lockedUserCount := c.getUserFailedLoginCount(ctx)
+	lockedUserCount := c.getUserFailedLoginCount(locked, ctx)
 
 	if lockedUserCount > 0 {
 		c.Logger().Warn("user lockout(s) in effect; review by using /sys/locked-users endpoint")
@@ -3687,7 +3690,7 @@ func (c *Core) startLockoutLogger() {
 			select {
 			case <-ticker.C:
 				// Check for lockout entries
-				lockedUserCount := c.getUserFailedLoginCount(ctx)
+				lockedUserCount := c.getUserFailedLoginCount(true, ctx)
 
 				if lockedUserCount > 0 {
 					c.Logger().Warn("user lockout(s) in effect; review by using /sys/locked-users endpoint")
@@ -3736,9 +3739,11 @@ func (c *Core) updateLockedUserEntries() {
 	}()
 }
 
-func (c *Core) getUserFailedLoginCount(ctx context.Context) int {
-	c.userFailedLoginInfoLock.Lock()
-	defer c.userFailedLoginInfoLock.Unlock()
+func (c *Core) getUserFailedLoginCount(locked bool, ctx context.Context) int {
+	if locked {
+		c.userFailedLoginInfoLock.Lock()
+		defer c.userFailedLoginInfoLock.Unlock()
+	}
 
 	return len(c.userFailedLoginInfo)
 }

vault/external_tests/identity/userlockouts_test.go

@@ -6,6 +6,11 @@ package identity
 import (
 	"bufio"
 	"bytes"
+	"os"
+	"strings"
+	"testing"
+	"time"
+
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/builtin/credential/userpass"
@@ -14,10 +19,6 @@ import (
 	"github.com/hashicorp/vault/sdk/helper/logging"
 	"github.com/hashicorp/vault/sdk/logical"
 	"github.com/hashicorp/vault/vault"
-	"os"
-	"strings"
-	"testing"
-	"time"
 )
 
 const (
@@ -286,7 +287,6 @@ func TestUserLockoutLogger_ManualUnlockTest(t *testing.T) {
 	if !(strings.Count(result, expected) > 1) {
 		t.Fatalf("expected log to contain %s, got %s", expected, result)
 	}
-
 }
 
 // TestIdentityStore_DisableUserLockoutTest tests that user login will

vault/logical_system_user_lockout.go

@@ -56,13 +56,12 @@ func unlockUser(ctx context.Context, core *Core, mountAccessor string, aliasName
 	numLockedUsers := len(core.userFailedLoginInfo)
 	core.userFailedLoginInfoLock.RUnlock()
 
-	if numLockedUsers == 0 {
-		core.Logger().Info("user lockout(s) cleared")
-		if core.lockoutLoggerCancel.Load() == nil {
-			return nil
+	if core.lockoutLoggerCancel.Load() != nil {
+		if numLockedUsers == 0 {
+			core.Logger().Info("user lockout(s) cleared")
+			cancelFunc := *core.lockoutLoggerCancel.Load()
+			cancelFunc()
 		}
-		cancelFunc := *core.lockoutLoggerCancel.Load()
-		cancelFunc()
 	}
 
 	return nil

vault/request_handling.go

@@ -1501,7 +1501,6 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
 			return nil, nil, err
 		}
 		if isLoginUserLocked {
-			c.startLockoutLogger()
 			return nil, nil, logical.ErrPermissionDenied
 		}
 	}
@@ -2472,6 +2471,9 @@ func (c *Core) LocalUpdateUserFailedLoginInfo(ctx context.Context, userKey Faile
 				return err
 			}
 
+			// Start lockout logger
+			// We run this as locked false since we already hold the userFailedLoginInfoLock here
+			c.startLockoutLogger(false)
 		}
 
 	default: