From 6e181d72b31522f886a2afa029d5b26d7912ec7b Mon Sep 17 00:00:00 2001
From: pasha-codefresh <pavel@codefresh.io>
Date: Mon, 18 Mar 2024 09:58:18 +0200
Subject: [PATCH] Merge pull request from GHSA-2vgg-9h6w-m454

* feat: pick random user and exclude admin user and current user from deletion candidates

Signed-off-by: pashakostohrys <pavel@codefresh.io>

* feat: increase default max cache size

Signed-off-by: pashakostohrys <pavel@codefresh.io>

* add nil protection

Signed-off-by: pashakostohrys <pavel@codefresh.io>

* Update util/session/sessionmanager.go

Signed-off-by: Dan Garfield <dan@codefresh.io>

Signed-off-by: Dan Garfield <dan@codefresh.io>

* chore: fix linter issues

Signed-off-by: pashakostohrys <pavel@codefresh.io>

---------

Signed-off-by: pashakostohrys <pavel@codefresh.io>
Signed-off-by: Dan Garfield <dan@codefresh.io>
Co-authored-by: Dan Garfield <dan@codefresh.io>
---
 util/session/sessionmanager.go      | 36 +++++++++++++++-----------
 util/session/sessionmanager_test.go | 39 +++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/util/session/sessionmanager.go b/util/session/sessionmanager.go
index 8999009b9bdd7..d11c96c6cf5aa 100644
--- a/util/session/sessionmanager.go
+++ b/util/session/sessionmanager.go
@@ -69,7 +69,7 @@ const (
 	// Maximum length of username, too keep the cache's memory signature low
 	maxUsernameLength = 32
 	// The default maximum session cache size
-	defaultMaxCacheSize = 1000
+	defaultMaxCacheSize = 10000
 	// The default number of maximum login failures before delay kicks in
 	defaultMaxLoginFailures = 5
 	// The default time in seconds for the failure window
@@ -310,6 +310,22 @@ func expireOldFailedAttempts(maxAge time.Duration, failures *map[string]LoginAtt
 	return expiredCount
 }
 
+// Protect admin user from login attempt reset caused by attempts to overflow cache in a brute force attack. Instead remove random non-admin to make room in cache. 
+func pickRandomNonAdminLoginFailure(failures map[string]LoginAttempts, username string) *string {
+	idx := rand.Intn(len(failures) - 1)
+	i := 0
+	for key := range failures {
+		if i == idx {
+			if key == common.ArgoCDAdminUsername || key == username {
+				return pickRandomNonAdminLoginFailure(failures, username)
+			}
+			return &key
+		}
+		i++
+	}
+	return nil
+}
+
 // Updates the failure count for a given username. If failed is true, increases the counter. Otherwise, sets counter back to 0.
 func (mgr *SessionManager) updateFailureCount(username string, failed bool) {
 
@@ -327,23 +343,13 @@ func (mgr *SessionManager) updateFailureCount(username string, failed bool) {
 	// prevent overbloating the cache with fake entries, as this could lead to
 	// memory exhaustion and ultimately in a DoS. We remove a single entry to
 	// replace it with the new one.
-	//
-	// Chances are that we remove the one that is under active attack, but this
-	// chance is low (1:cache_size)
 	if failed && len(failures) >= getMaximumCacheSize() {
 		log.Warnf("Session cache size exceeds %d entries, removing random entry", getMaximumCacheSize())
-		idx := rand.Intn(len(failures) - 1)
-		var rmUser string
-		i := 0
-		for key := range failures {
-			if i == idx {
-				rmUser = key
-				delete(failures, key)
-				break
-			}
-			i++
+		rmUser := pickRandomNonAdminLoginFailure(failures, username)
+		if rmUser != nil {
+			delete(failures, *rmUser)
+			log.Infof("Deleted entry for user %s from cache", *rmUser)
 		}
-		log.Infof("Deleted entry for user %s from cache", rmUser)
 	}
 
 	attempt, ok := failures[username]
diff --git a/util/session/sessionmanager_test.go b/util/session/sessionmanager_test.go
index d01ba3ef5f32d..817966376daa3 100644
--- a/util/session/sessionmanager_test.go
+++ b/util/session/sessionmanager_test.go
@@ -1173,3 +1173,42 @@ requestedScopes: ["oidc"]`, oidcTestServer.URL),
 		assert.ErrorIs(t, err, common.TokenVerificationErr)
 	})
 }
+
+func Test_PickFailureAttemptWhenOverflowed(t *testing.T) {
+	t.Run("Not pick admin user from the queue", func(t *testing.T) {
+		failures := map[string]LoginAttempts{
+			"admin": {
+				FailCount: 1,
+			},
+			"test2": {
+				FailCount: 1,
+			},
+		}
+
+		// inside pickRandomNonAdminLoginFailure, it uses random, so we need to test it multiple times
+		for i := 0; i < 1000; i++ {
+			user := pickRandomNonAdminLoginFailure(failures, "test")
+			assert.Equal(t, "test2", *user)
+		}
+	})
+
+	t.Run("Not pick admin user and current user from the queue", func(t *testing.T) {
+		failures := map[string]LoginAttempts{
+			"test": {
+				FailCount: 1,
+			},
+			"admin": {
+				FailCount: 1,
+			},
+			"test2": {
+				FailCount: 1,
+			},
+		}
+
+		// inside pickRandomNonAdminLoginFailure, it uses random, so we need to test it multiple times
+		for i := 0; i < 1000; i++ {
+			user := pickRandomNonAdminLoginFailure(failures, "test")
+			assert.Equal(t, "test2", *user)
+		}
+	})
+}