Bump version to 2.9.9 (#17558)

Signed-off-by: GitHub <noreply@github.com>
Co-authored-by: crenshaw-dev <crenshaw-dev@users.noreply.github.com>

VERSION

@@ -1 +1 @@
-2.9.6
+2.9.9

cmd/argocd-server/commands/argocd_server.go

@@ -232,7 +232,7 @@ func NewCommand() *cobra.Command {
 	command.Flags().StringVar(&repoServerAddress, "repo-server", env.StringFromEnv("ARGOCD_SERVER_REPO_SERVER", common.DefaultRepoServerAddr), "Repo server address")
 	command.Flags().StringVar(&dexServerAddress, "dex-server", env.StringFromEnv("ARGOCD_SERVER_DEX_SERVER", common.DefaultDexServerAddr), "Dex server address")
 	command.Flags().BoolVar(&disableAuth, "disable-auth", env.ParseBoolFromEnv("ARGOCD_SERVER_DISABLE_AUTH", false), "Disable client authentication")
-	command.Flags().StringVar(&contentTypes, "api-content-types", env.StringFromEnv("ARGOCD_API_CONTENT_TYPES", "application/json"), "Semicolon separated list of allowed content types for non GET api requests. Any content type is allowed if empty.")
+	command.Flags().StringVar(&contentTypes, "api-content-types", env.StringFromEnv("ARGOCD_API_CONTENT_TYPES", "application/json", env.StringFromEnvOpts{AllowEmpty: true}), "Semicolon separated list of allowed content types for non GET api requests. Any content type is allowed if empty.")
 	command.Flags().BoolVar(&enableGZip, "enable-gzip", env.ParseBoolFromEnv("ARGOCD_SERVER_ENABLE_GZIP", true), "Enable GZIP compression")
 	command.AddCommand(cli.NewVersionCmd(cliName))
 	command.Flags().StringVar(&listenHost, "address", env.StringFromEnv("ARGOCD_SERVER_LISTEN_ADDRESS", common.DefaultAddressAPIServer), "Listen on given address")

cmd/argocd/commands/app.go

@@ -1031,6 +1031,7 @@ func NewApplicationDiffCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
 					defer argoio.Close(conn)
 					cluster, err := clusterIf.Get(ctx, &clusterpkg.ClusterQuery{Name: app.Spec.Destination.Name, Server: app.Spec.Destination.Server})
 					errors.CheckError(err)
+
 					diffOption.local = local
 					diffOption.localRepoRoot = localRepoRoot
 					diffOption.cluster = cluster

manifests/base/kustomization.yaml

@@ -5,7 +5,7 @@ kind: Kustomization
 images:
 - name: quay.io/argoproj/argocd
   newName: quay.io/argoproj/argocd
-  newTag: v2.9.6
+  newTag: v2.9.9
 resources:
 - ./application-controller
 - ./dex

manifests/core-install.yaml

@@ -20750,7 +20750,7 @@ spec:
               key: applicationsetcontroller.allowed.scm.providers
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: argocd-applicationset-controller
         ports:
@@ -21050,7 +21050,7 @@ spec:
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -21102,7 +21102,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         name: copyutil
         securityContext:
           allowPrivilegeEscalation: false
@@ -21321,7 +21321,7 @@ spec:
               key: controller.kubectl.parallelism.limit
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: argocd-application-controller
         ports:

manifests/core-install/kustomization.yaml

@@ -12,4 +12,4 @@ resources:
 images:
 - name: quay.io/argoproj/argocd
   newName: quay.io/argoproj/argocd
-  newTag: v2.9.6
+  newTag: v2.9.9

manifests/ha/base/kustomization.yaml

@@ -12,7 +12,7 @@ patches:
 images:
 - name: quay.io/argoproj/argocd
   newName: quay.io/argoproj/argocd
-  newTag: v2.9.6
+  newTag: v2.9.9
 resources:
 - ../../base/application-controller
 - ../../base/applicationset-controller

manifests/ha/install.yaml

@@ -22007,7 +22007,7 @@ spec:
               key: applicationsetcontroller.allowed.scm.providers
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: argocd-applicationset-controller
         ports:
@@ -22130,7 +22130,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: copyutil
         securityContext:
@@ -22206,7 +22206,7 @@ spec:
               key: application.namespaces
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -22537,7 +22537,7 @@ spec:
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -22589,7 +22589,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         name: copyutil
         securityContext:
           allowPrivilegeEscalation: false
@@ -22884,7 +22884,7 @@ spec:
               key: server.api.content.types
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -23130,7 +23130,7 @@ spec:
               key: controller.kubectl.parallelism.limit
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: argocd-application-controller
         ports:

manifests/ha/namespace-install.yaml

@@ -1662,7 +1662,7 @@ spec:
               key: applicationsetcontroller.allowed.scm.providers
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: argocd-applicationset-controller
         ports:
@@ -1785,7 +1785,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: copyutil
         securityContext:
@@ -1861,7 +1861,7 @@ spec:
               key: application.namespaces
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -2192,7 +2192,7 @@ spec:
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -2244,7 +2244,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         name: copyutil
         securityContext:
           allowPrivilegeEscalation: false
@@ -2539,7 +2539,7 @@ spec:
               key: server.api.content.types
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -2785,7 +2785,7 @@ spec:
               key: controller.kubectl.parallelism.limit
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: argocd-application-controller
         ports:

manifests/install.yaml

@@ -21102,7 +21102,7 @@ spec:
               key: applicationsetcontroller.allowed.scm.providers
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: argocd-applicationset-controller
         ports:
@@ -21225,7 +21225,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: copyutil
         securityContext:
@@ -21301,7 +21301,7 @@ spec:
               key: application.namespaces
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -21583,7 +21583,7 @@ spec:
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -21635,7 +21635,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         name: copyutil
         securityContext:
           allowPrivilegeEscalation: false
@@ -21928,7 +21928,7 @@ spec:
               key: server.api.content.types
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -22174,7 +22174,7 @@ spec:
               key: controller.kubectl.parallelism.limit
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: argocd-application-controller
         ports:

manifests/namespace-install.yaml

@@ -757,7 +757,7 @@ spec:
               key: applicationsetcontroller.allowed.scm.providers
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: argocd-applicationset-controller
         ports:
@@ -880,7 +880,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: copyutil
         securityContext:
@@ -956,7 +956,7 @@ spec:
               key: application.namespaces
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           tcpSocket:
@@ -1238,7 +1238,7 @@ spec:
           value: /helm-working-dir
         - name: HELM_DATA_HOME
           value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           failureThreshold: 3
@@ -1290,7 +1290,7 @@ spec:
         - -n
         - /usr/local/bin/argocd
         - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         name: copyutil
         securityContext:
           allowPrivilegeEscalation: false
@@ -1583,7 +1583,7 @@ spec:
               key: server.api.content.types
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         livenessProbe:
           httpGet:
@@ -1829,7 +1829,7 @@ spec:
               key: controller.kubectl.parallelism.limit
               name: argocd-cmd-params-cm
               optional: true
-        image: quay.io/argoproj/argocd:v2.9.6
+        image: quay.io/argoproj/argocd:v2.9.9
         imagePullPolicy: Always
         name: argocd-application-controller
         ports:

reposerver/repository/repository.go

@@ -1378,7 +1378,7 @@ func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string,
 		if q.KustomizeOptions != nil {
 			kustomizeBinary = q.KustomizeOptions.BinaryPath
 		}
-		k := kustomize.NewKustomizeApp(appPath, q.Repo.GetGitCreds(gitCredsStore), repoURL, kustomizeBinary)
+		k := kustomize.NewKustomizeApp(repoRoot, appPath, q.Repo.GetGitCreds(gitCredsStore), repoURL, kustomizeBinary)
 		targetObjs, _, err = k.Build(q.ApplicationSource.Kustomize, q.KustomizeOptions, env)
 	case v1alpha1.ApplicationSourceTypePlugin:
 		pluginName := ""
@@ -1965,7 +1965,7 @@ func (s *Service) GetAppDetails(ctx context.Context, q *apiclient.RepoServerAppD
 				return err
 			}
 		case v1alpha1.ApplicationSourceTypeKustomize:
-			if err := populateKustomizeAppDetails(res, q, opContext.appPath, commitSHA, s.gitCredsStore); err != nil {
+			if err := populateKustomizeAppDetails(res, q, repoRoot, opContext.appPath, commitSHA, s.gitCredsStore); err != nil {
 				return err
 			}
 		case v1alpha1.ApplicationSourceTypePlugin:
@@ -2106,13 +2106,13 @@ func findHelmValueFilesInPath(path string) ([]string, error) {
 	return result, nil
 }
 
-func populateKustomizeAppDetails(res *apiclient.RepoAppDetailsResponse, q *apiclient.RepoServerAppDetailsQuery, appPath string, reversion string, credsStore git.CredsStore) error {
+func populateKustomizeAppDetails(res *apiclient.RepoAppDetailsResponse, q *apiclient.RepoServerAppDetailsQuery, repoRoot string, appPath string, reversion string, credsStore git.CredsStore) error {
 	res.Kustomize = &apiclient.KustomizeAppSpec{}
 	kustomizeBinary := ""
 	if q.KustomizeOptions != nil {
 		kustomizeBinary = q.KustomizeOptions.BinaryPath
 	}
-	k := kustomize.NewKustomizeApp(appPath, q.Repo.GetGitCreds(credsStore), q.Repo.Repo, kustomizeBinary)
+	k := kustomize.NewKustomizeApp(repoRoot, appPath, q.Repo.GetGitCreds(credsStore), q.Repo.Repo, kustomizeBinary)
 	fakeManifestRequest := apiclient.ManifestRequest{
 		AppName:           q.AppName,
 		Namespace:         "", // FIXME: omit it for now

server/application/application.go

@@ -325,6 +325,15 @@ func (s *Server) Create(ctx context.Context, q *application.ApplicationCreateReq
 		return nil, security.NamespaceNotPermittedError(appNs)
 	}
 
+	// Don't let the app creator set the operation explicitly. Those requests should always go through the Sync API.
+	if a.Operation != nil {
+		log.WithFields(log.Fields{
+			"application":            a.Name,
+			argocommon.SecurityField: argocommon.SecurityLow,
+		}).Warn("User attempted to set operation on application creation. This could have allowed them to bypass branch protection rules by setting manifests directly. Ignoring the set operation.")
+		a.Operation = nil
+	}
+
 	created, err := s.appclientset.ArgoprojV1alpha1().Applications(appNs).Create(ctx, a, metav1.CreateOptions{})
 	if err == nil {
 		s.logAppEvent(created, ctx, argo.EventReasonResourceCreated, "created application")

server/application/application_test.go

@@ -1439,6 +1439,27 @@ func TestCreateAppWithDestName(t *testing.T) {
 	assert.Equal(t, app.Spec.Destination.Server, "https://cluster-api.com")
 }
 
+// TestCreateAppWithOperation tests that an application created with an operation is created with the operation removed.
+// Avoids regressions of https://github.com/argoproj/argo-cd/security/advisories/GHSA-g623-jcgg-mhmm
+func TestCreateAppWithOperation(t *testing.T) {
+	appServer := newTestAppServer(t)
+	testApp := newTestAppWithDestName()
+	testApp.Operation = &appsv1.Operation{
+		Sync: &appsv1.SyncOperation{
+			Manifests: []string{
+				"test",
+			},
+		},
+	}
+	createReq := application.ApplicationCreateRequest{
+		Application: testApp,
+	}
+	app, err := appServer.Create(context.Background(), &createReq)
+	require.NoError(t, err)
+	require.NotNil(t, app)
+	assert.Nil(t, app.Operation)
+}
+
 func TestUpdateApp(t *testing.T) {
 	testApp := newTestApp()
 	appServer := newTestAppServer(t, testApp)

server/application/terminal.go

@@ -38,12 +38,12 @@ type terminalHandler struct {
 	allowedShells     []string
 	namespace         string
 	enabledNamespaces []string
-	sessionManager    util_session.SessionManager
+	sessionManager    *util_session.SessionManager
 }
 
 // NewHandler returns a new terminal handler.
 func NewHandler(appLister applisters.ApplicationLister, namespace string, enabledNamespaces []string, db db.ArgoDB, enf *rbac.Enforcer, cache *servercache.Cache,
-	appResourceTree AppResourceTreeFn, allowedShells []string, sessionManager util_session.SessionManager) *terminalHandler {
+	appResourceTree AppResourceTreeFn, allowedShells []string, sessionManager *util_session.SessionManager) *terminalHandler {
 	return &terminalHandler{
 		appLister:         appLister,
 		db:                db,

server/application/websocket.go

@@ -37,7 +37,7 @@ type terminalSession struct {
 	tty            bool
 	readLock       sync.Mutex
 	writeLock      sync.Mutex
-	sessionManager util_session.SessionManager
+	sessionManager *util_session.SessionManager
 	token          *string
 }
 
@@ -48,7 +48,7 @@ func getToken(r *http.Request) (string, error) {
 }
 
 // newTerminalSession create terminalSession
-func newTerminalSession(w http.ResponseWriter, r *http.Request, responseHeader http.Header, sessionManager util_session.SessionManager) (*terminalSession, error) {
+func newTerminalSession(w http.ResponseWriter, r *http.Request, responseHeader http.Header, sessionManager *util_session.SessionManager) (*terminalSession, error) {
 	token, err := getToken(r)
 	if err != nil {
 		return nil, err

server/server.go

@@ -997,7 +997,7 @@ func (a *ArgoCDServer) newHTTPServer(ctx context.Context, port int, grpcWebHandl
 	}
 	mux.Handle("/api/", handler)
 
-	terminal := application.NewHandler(a.appLister, a.Namespace, a.ApplicationNamespaces, a.db, a.enf, a.Cache, appResourceTreeFn, a.settings.ExecShells, *a.sessionMgr).
+	terminal := application.NewHandler(a.appLister, a.Namespace, a.ApplicationNamespaces, a.db, a.enf, a.Cache, appResourceTreeFn, a.settings.ExecShells, a.sessionMgr).
 		WithFeatureFlagMiddleware(a.settingsMgr.GetSettings)
 	th := util_session.WithAuthMiddleware(a.DisableAuth, a.sessionMgr, terminal)
 	mux.Handle("/terminal", th)

test/e2e/app_management_ns_test.go

@@ -748,7 +748,7 @@ func TestNamespacedResourceDiffing(t *testing.T) {
 		Then().
 		Expect(SyncStatusIs(SyncStatusCodeOutOfSync)).
 		And(func(app *Application) {
-			diffOutput, err := RunCli("app", "diff", ctx.AppQualifiedName(), "--local", "testdata/guestbook")
+			diffOutput, err := RunCli("app", "diff", ctx.AppQualifiedName(), "--local-repo-root", ".", "--local", "testdata/guestbook")
 			assert.Error(t, err)
 			assert.Contains(t, diffOutput, fmt.Sprintf("===== apps/Deployment %s/guestbook-ui ======", DeploymentNamespace()))
 		}).
@@ -761,7 +761,7 @@ func TestNamespacedResourceDiffing(t *testing.T) {
 		Then().
 		Expect(SyncStatusIs(SyncStatusCodeSynced)).
 		And(func(app *Application) {
-			diffOutput, err := RunCli("app", "diff", ctx.AppQualifiedName(), "--local", "testdata/guestbook")
+			diffOutput, err := RunCli("app", "diff", ctx.AppQualifiedName(), "--local-repo-root", ".", "--local", "testdata/guestbook")
 			assert.NoError(t, err)
 			assert.Empty(t, diffOutput)
 		}).
@@ -897,7 +897,7 @@ func testNSEdgeCasesApplicationResources(t *testing.T, appPath string, statusCod
 	expect.
 		Expect(HealthIs(statusCode)).
 		And(func(app *Application) {
-			diffOutput, err := RunCli("app", "diff", ctx.AppQualifiedName(), "--local", path.Join("testdata", appPath))
+			diffOutput, err := RunCli("app", "diff", ctx.AppQualifiedName(), "--local-repo-root", ".", "--local", path.Join("testdata", appPath))
 			assert.Empty(t, diffOutput)
 			assert.NoError(t, err)
 		})
@@ -998,7 +998,7 @@ func TestNamespacedLocalManifestSync(t *testing.T) {
 		Given().
 		LocalPath(guestbookPathLocal).
 		When().
-		Sync().
+		Sync("--local-repo-root", ".").
 		Then().
 		Expect(SyncStatusIs(SyncStatusCodeSynced)).
 		And(func(app *Application) {
@@ -1066,7 +1066,7 @@ func TestNamespacedLocalSyncDryRunWithASEnabled(t *testing.T) {
 			assert.NoError(t, err)
 
 			appBefore := app.DeepCopy()
-			_, err = RunCli("app", "sync", app.QualifiedName(), "--dry-run", "--local", guestbookPathLocal)
+			_, err = RunCli("app", "sync", app.QualifiedName(), "--dry-run", "--local-repo-root", ".", "--local", guestbookPathLocal)
 			assert.NoError(t, err)
 
 			appAfter := app.DeepCopy()

test/e2e/app_management_test.go

@@ -1324,7 +1324,7 @@ func TestLocalManifestSync(t *testing.T) {
 		Given().
 		LocalPath(guestbookPathLocal).
 		When().
-		Sync().
+		Sync("--local-repo-root", ".").
 		Then().
 		Expect(SyncStatusIs(SyncStatusCodeSynced)).
 		And(func(app *Application) {
@@ -1385,7 +1385,7 @@ func TestLocalSyncDryRunWithAutosyncEnabled(t *testing.T) {
 			assert.NoError(t, err)
 
 			appBefore := app.DeepCopy()
-			_, err = RunCli("app", "sync", app.Name, "--dry-run", "--local", guestbookPathLocal)
+			_, err = RunCli("app", "sync", app.Name, "--dry-run", "--local-repo-root", ".", "--local", guestbookPathLocal)
 			assert.NoError(t, err)
 
 			appAfter := app.DeepCopy()

ui/src/app/applications/components/application-summary/application-summary.tsx

@@ -30,6 +30,7 @@ import {EditAnnotations} from './edit-annotations';
 
 import './application-summary.scss';
 import {DeepLinks} from '../../../shared/components/deep-links';
+import {ExternalLinks} from '../application-urls';
 
 function swap(array: any[], a: number, b: number) {
     array = array.slice();
@@ -326,20 +327,19 @@ export const ApplicationSummary = (props: ApplicationSummaryProps) => {
             )
         }
     ];
-
-    const urls = app.status.summary.externalURLs || [];
+    const urls = ExternalLinks(app.status.summary.externalURLs);
     if (urls.length > 0) {
         attributes.push({
             title: 'URLs',
             view: (
                 <React.Fragment>
-                    {urls
-                        .map(item => item.split('|'))
-                        .map((parts, i) => (
-                            <a key={i} href={parts.length > 1 ? parts[1] : parts[0]} target='__blank'>
-                                {parts[0]} &nbsp;
+                    {urls.map((url, i) => {
+                        return (
+                            <a key={i} href={url.ref} target='__blank'>
+                                {url.title} &nbsp;
                             </a>
-                        ))}
+                        );
+                    })}
                 </React.Fragment>
             )
         });

ui/src/app/applications/components/application-urls.test.ts

@@ -1,4 +1,4 @@
-import {ExternalLink, InvalidExternalLinkError} from './application-urls';
+import { ExternalLink, ExternalLinks, InvalidExternalLinkError } from './application-urls';
 
 test('rejects malicious URLs', () => {
     expect(() => {
@@ -7,6 +7,16 @@ test('rejects malicious URLs', () => {
     expect(() => {
         const _ = new ExternalLink('data:text/html;<h1>hi</h1>');
     }).toThrowError(InvalidExternalLinkError);
+    expect(() => {
+        const _ = new ExternalLink('title|data:text/html;<h1>hi</h1>');
+    }).toThrowError(InvalidExternalLinkError);
+    expect(() => {
+        const _ = new ExternalLink('data:title|data:text/html;<h1>hi</h1>');
+    }).toThrowError(InvalidExternalLinkError);
+
+    expect(() => {
+        const _ = new ExternalLink('data:title|https://localhost:8080/applications');
+    }).not.toThrowError(InvalidExternalLinkError);
 });
 
 test('allows absolute URLs', () => {
@@ -18,3 +28,59 @@ test('allows relative URLs', () => {
     window.location = new URL('https://localhost:8080/applications');
     expect(new ExternalLink('/applications').ref).toEqual('/applications');
 });
+
+
+test('URLs format', () => {
+    expect(new ExternalLink('https://localhost:8080/applications')).toEqual({
+        ref: 'https://localhost:8080/applications',
+        title: 'https://localhost:8080/applications',
+    })
+    expect(new ExternalLink('title|https://localhost:8080/applications')).toEqual({
+        ref: 'https://localhost:8080/applications',
+        title: 'title',
+    })
+});
+
+
+test('malicious URLs from list to be removed', () => {
+    const urls: string[] = [
+        'javascript:alert("hi")',
+        'https://localhost:8080/applications',
+    ]
+    const links = ExternalLinks(urls);
+
+    expect(links).toHaveLength(1);
+    expect(links).toContainEqual({
+        ref: 'https://localhost:8080/applications',
+        title: 'https://localhost:8080/applications',
+    });
+});
+
+
+test('list to be sorted', () => {
+    const urls: string[] = [
+        'https://a',
+        'https://b',
+        'a|https://c',
+        'z|https://c',
+        'x|https://d',
+        'x|https://c',
+    ]
+    const links = ExternalLinks(urls);
+
+    // 'a|https://c',
+    // 'x|https://c',
+    // 'x|https://d',
+    // 'z|https://c',
+    // 'https://a',
+    // 'https://b',
+    expect(links).toHaveLength(6);
+    expect(links[0].title).toEqual('a')
+    expect(links[1].title).toEqual('x')
+    expect(links[1].ref).toEqual('https://c')
+    expect(links[2].title).toEqual('x')
+    expect(links[2].ref).toEqual('https://d')
+    expect(links[3].title).toEqual('z')
+    expect(links[4].title).toEqual('https://a')
+    expect(links[5].title).toEqual('https://b')
+});

ui/src/app/applications/components/application-urls.tsx

@@ -29,7 +29,7 @@ export class ExternalLink {
     }
 }
 
-export const ApplicationURLs = ({urls}: {urls: string[]}) => {
+export const ExternalLinks = (urls?: string[]) => {
     const externalLinks: ExternalLink[] = [];
     for (const url of urls || []) {
         try {
@@ -42,16 +42,26 @@ export const ApplicationURLs = ({urls}: {urls: string[]}) => {
 
     // sorted alphabetically & links with titles first
     externalLinks.sort((a, b) => {
-        if (a.title !== '' && b.title !== '') {
+        const hasTitle = (x: ExternalLink): boolean => {
+            return x.title !== x.ref && x.title !== '';
+        };
+
+        if (hasTitle(a) && hasTitle(b) && a.title !== b.title) {
             return a.title > b.title ? 1 : -1;
-        } else if (a.title === '') {
+        } else if (hasTitle(b) && !hasTitle(a)) {
             return 1;
-        } else if (b.title === '') {
+        } else if (hasTitle(a) && !hasTitle(b)) {
             return -1;
         }
         return a.ref > b.ref ? 1 : -1;
     });
 
+    return externalLinks;
+};
+
+export const ApplicationURLs = ({urls}: {urls: string[]}) => {
+    const externalLinks: ExternalLink[] = ExternalLinks(urls);
+
     return (
         ((externalLinks || []).length > 0 && (
             <div className='applications-list__external-links-icon-container'>

util/env/env.go

@@ -124,8 +124,17 @@ func ParseDurationFromEnv(env string, defaultValue, min, max time.Duration) time
 	return dur
 }
 
-func StringFromEnv(env string, defaultValue string) string {
-	if str := os.Getenv(env); str != "" {
+type StringFromEnvOpts struct {
+	// AllowEmpty allows the value to be empty as long as the environment variable is set.
+	AllowEmpty bool
+}
+
+func StringFromEnv(env string, defaultValue string, opts ...StringFromEnvOpts) string {
+	opt := StringFromEnvOpts{}
+	for _, o := range opts {
+		opt.AllowEmpty = opt.AllowEmpty || o.AllowEmpty
+	}
+	if str, ok := os.LookupEnv(env); opt.AllowEmpty && ok || str != "" {
 		return str
 	}
 	return defaultValue

util/env/env_test.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"k8s.io/utils/pointer"
 )
 
 func TestParseNumFromEnv(t *testing.T) {
@@ -167,19 +168,25 @@ func TestStringFromEnv(t *testing.T) {
 
 	testCases := []struct {
 		name     string
-		env      string
+		env      *string
 		expected string
 		def      string
+		opts     []StringFromEnvOpts
 	}{
-		{"Some string", "true", "true", def},
-		{"Empty string with default", "", def, def},
-		{"Empty string without default", "", "", ""},
+		{"Some string", pointer.String("true"), "true", def, nil},
+		{"Empty string with default", pointer.String(""), def, def, nil},
+		{"Empty string without default", pointer.String(""), "", "", nil},
+		{"No env variable with default allow empty", nil, "default", "default", []StringFromEnvOpts{{AllowEmpty: true}}},
+		{"Some variable with default allow empty", pointer.String("true"), "true", "default", []StringFromEnvOpts{{AllowEmpty: true}}},
+		{"Empty variable with default allow empty", pointer.String(""), "", "default", []StringFromEnvOpts{{AllowEmpty: true}}},
 	}
 
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {
-			t.Setenv(envKey, tt.env)
-			b := StringFromEnv(envKey, tt.def)
+			if tt.env != nil {
+				t.Setenv(envKey, *tt.env)
+			}
+			b := StringFromEnv(envKey, tt.def, tt.opts...)
 			assert.Equal(t, tt.expected, b)
 		})
 	}

util/helm/client.go

@@ -8,7 +8,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	executil "github.com/argoproj/argo-cd/v2/util/exec"
 	"io"
 	"net/http"
 	"net/url"
@@ -19,6 +18,8 @@ import (
 	"strings"
 	"time"
 
+	executil "github.com/argoproj/argo-cd/v2/util/exec"
+
 	"github.com/argoproj/pkg/sync"
 	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v2"
@@ -34,6 +35,8 @@ import (
 var (
 	globalLock = sync.NewKeyLock()
 	indexLock  = sync.NewKeyLock()
+
+	OCINotEnabledErr = errors.New("could not perform the action when oci is not enabled")
 )
 
 type Creds struct {
@@ -401,6 +404,10 @@ func getIndexURL(rawURL string) (string, error) {
 }
 
 func (c *nativeHelmChart) GetTags(chart string, noCache bool) (*TagsList, error) {
+	if !c.enableOci {
+		return nil, OCINotEnabledErr
+	}
+
 	tagsURL := strings.Replace(fmt.Sprintf("%s/%s", c.repoURL, chart), "https://", "", 1)
 	indexLock.Lock(tagsURL)
 	defer indexLock.Unlock(tagsURL)
@@ -428,10 +435,12 @@ func (c *nativeHelmChart) GetTags(chart string, noCache bool) (*TagsList, error)
 			TLSClientConfig:   tlsConf,
 			DisableKeepAlives: true,
 		}}
+
+		repoHost, _, _ := strings.Cut(tagsURL, "/")
 		repo.Client = &auth.Client{
 			Client: client,
 			Cache:  nil,
-			Credential: auth.StaticCredential(c.repoURL, auth.Credential{
+			Credential: auth.StaticCredential(repoHost, auth.Credential{
 				Username: c.creds.Username,
 				Password: c.creds.Password,
 			}),

util/helm/client_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"math"
+	"net/url"
 	"os"
 	"strings"
 	"testing"
@@ -159,41 +160,129 @@ func TestGetIndexURL(t *testing.T) {
 }
 
 func TestGetTagsFromUrl(t *testing.T) {
+	t.Run("should return tags correctly while following the link header", func(t *testing.T) {
+		server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			t.Logf("called %s", r.URL.Path)
+			responseTags := TagsList{}
+			w.Header().Set("Content-Type", "application/json")
+			if !strings.Contains(r.URL.String(), "token") {
+				w.Header().Set("Link", fmt.Sprintf("<https://%s%s?token=next-token>; rel=next", r.Host, r.URL.Path))
+				responseTags.Tags = []string{"first"}
+			} else {
+				responseTags.Tags = []string{
+					"second",
+					"2.8.0",
+					"2.8.0-prerelease",
+					"2.8.0_build",
+					"2.8.0-prerelease_build",
+					"2.8.0-prerelease.1_build.1234",
+				}
+			}
+			w.WriteHeader(http.StatusOK)
+			err := json.NewEncoder(w).Encode(responseTags)
+			if err != nil {
+				t.Fatal(err)
+			}
+		}))
+
+		client := NewClient(server.URL, Creds{InsecureSkipVerify: true}, true, "")
+
+		tags, err := client.GetTags("mychart", true)
+		assert.NoError(t, err)
+		assert.ElementsMatch(t, tags.Tags, []string{
+			"first",
+			"second",
+			"2.8.0",
+			"2.8.0-prerelease",
+			"2.8.0+build",
+			"2.8.0-prerelease+build",
+			"2.8.0-prerelease.1+build.1234",
+		})
+	})
+
+	t.Run("should return an error not when oci is not enabled", func(t *testing.T) {
+		client := NewClient("example.com", Creds{}, false, "")
+
+		_, err := client.GetTags("my-chart", true)
+		assert.ErrorIs(t, OCINotEnabledErr, err)
+	})
+}
+
+func TestGetTagsFromURLPrivateRepoAuthentication(t *testing.T) {
 	server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		t.Logf("called %s", r.URL.Path)
-		responseTags := TagsList{}
-		w.Header().Set("Content-Type", "application/json")
-		if !strings.Contains(r.URL.String(), "token") {
-			w.Header().Set("Link", fmt.Sprintf("<https://%s%s?token=next-token>; rel=next", r.Host, r.URL.Path))
-			responseTags.Tags = []string{"first"}
-		} else {
-			responseTags.Tags = []string{
-				"second",
+
+		authorization := r.Header.Get("Authorization")
+		if authorization == "" {
+			w.Header().Set("WWW-Authenticate", `Basic realm="helm repo to get tags"`)
+			w.WriteHeader(http.StatusUnauthorized)
+			return
+		}
+
+		t.Logf("authorization received %s", authorization)
+
+		responseTags := TagsList{
+			Tags: []string{
 				"2.8.0",
 				"2.8.0-prerelease",
 				"2.8.0_build",
 				"2.8.0-prerelease_build",
 				"2.8.0-prerelease.1_build.1234",
-			}
+			},
 		}
+
+		w.Header().Set("Content-Type", "application/json")
 		w.WriteHeader(http.StatusOK)
 		err := json.NewEncoder(w).Encode(responseTags)
 		if err != nil {
 			t.Fatal(err)
 		}
 	}))
+	t.Cleanup(server.Close)
 
-	client := NewClient(server.URL, Creds{InsecureSkipVerify: true}, true, "")
-
-	tags, err := client.GetTags("mychart", true)
+	serverURL, err := url.Parse(server.URL)
 	assert.NoError(t, err)
-	assert.ElementsMatch(t, tags.Tags, []string{
-		"first",
-		"second",
-		"2.8.0",
-		"2.8.0-prerelease",
-		"2.8.0+build",
-		"2.8.0-prerelease+build",
-		"2.8.0-prerelease.1+build.1234",
-	})
+
+	testCases := []struct {
+		name    string
+		repoURL string
+	}{
+		{
+			name:    "should login correctly when the repo path is in the server root with http scheme",
+			repoURL: server.URL,
+		},
+		{
+			name:    "should login correctly when the repo path is not in the server root with http scheme",
+			repoURL: fmt.Sprintf("%s/my-repo", server.URL),
+		},
+		{
+			name:    "should login correctly when the repo path is in the server root without http scheme",
+			repoURL: serverURL.Host,
+		},
+		{
+			name:    "should login correctly when the repo path is not in the server root without http scheme",
+			repoURL: fmt.Sprintf("%s/my-repo", serverURL.Host),
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			client := NewClient(testCase.repoURL, Creds{
+				InsecureSkipVerify: true,
+				Username:           "my-username",
+				Password:           "my-password",
+			}, true, "")
+
+			tags, err := client.GetTags("mychart", true)
+
+			assert.NoError(t, err)
+			assert.ElementsMatch(t, tags.Tags, []string{
+				"2.8.0",
+				"2.8.0-prerelease",
+				"2.8.0+build",
+				"2.8.0-prerelease+build",
+				"2.8.0-prerelease.1+build.1234",
+			})
+		})
+	}
 }

util/helm/cmd.go

@@ -91,6 +91,28 @@ func (c *Cmd) RegistryLogin(repo string, creds Creds) (string, error) {
 		args = append(args, "--password", creds.Password)
 	}
 
+	if creds.CAPath != "" {
+		args = append(args, "--ca-file", creds.CAPath)
+	}
+
+	if len(creds.CertData) > 0 {
+		filePath, closer, err := writeToTmp(creds.CertData)
+		if err != nil {
+			return "", err
+		}
+		defer argoio.Close(closer)
+		args = append(args, "--cert-file", filePath)
+	}
+
+	if len(creds.KeyData) > 0 {
+		filePath, closer, err := writeToTmp(creds.KeyData)
+		if err != nil {
+			return "", err
+		}
+		defer argoio.Close(closer)
+		args = append(args, "--key-file", filePath)
+	}
+
 	if creds.InsecureSkipVerify {
 		args = append(args, "--insecure")
 	}
@@ -238,6 +260,25 @@ func (c *Cmd) PullOCI(repo string, chart string, version string, destination str
 	if creds.CAPath != "" {
 		args = append(args, "--ca-file", creds.CAPath)
 	}
+
+	if len(creds.CertData) > 0 {
+		filePath, closer, err := writeToTmp(creds.CertData)
+		if err != nil {
+			return "", err
+		}
+		defer argoio.Close(closer)
+		args = append(args, "--cert-file", filePath)
+	}
+
+	if len(creds.KeyData) > 0 {
+		filePath, closer, err := writeToTmp(creds.KeyData)
+		if err != nil {
+			return "", err
+		}
+		defer argoio.Close(closer)
+		args = append(args, "--key-file", filePath)
+	}
+
 	if creds.InsecureSkipVerify && c.insecureSkipVerifySupported {
 		args = append(args, "--insecure-skip-tls-verify")
 	}

util/kustomize/kustomize.go

@@ -35,8 +35,9 @@ type Kustomize interface {
 }
 
 // NewKustomizeApp create a new wrapper to run commands on the `kustomize` command-line tool.
-func NewKustomizeApp(path string, creds git.Creds, fromRepo string, binaryPath string) Kustomize {
+func NewKustomizeApp(repoRoot string, path string, creds git.Creds, fromRepo string, binaryPath string) Kustomize {
 	return &kustomize{
+		repoRoot:   repoRoot,
 		path:       path,
 		creds:      creds,
 		repo:       fromRepo,
@@ -45,6 +46,8 @@ func NewKustomizeApp(path string, creds git.Creds, fromRepo string, binaryPath s
 }
 
 type kustomize struct {
+	// path to the Git repository root
+	repoRoot string
 	// path inside the checked out tree
 	path string
 	// creds structure
@@ -281,6 +284,7 @@ func (k *kustomize) Build(opts *v1alpha1.ApplicationSourceKustomize, kustomizeOp
 	}
 
 	cmd.Env = append(cmd.Env, environ...)
+	cmd.Dir = k.repoRoot
 	out, err := executil.Run(cmd)
 	if err != nil {
 		return nil, nil, err

util/kustomize/kustomize_test.go

@@ -39,7 +39,7 @@ func TestKustomizeBuild(t *testing.T) {
 	namePrefix := "namePrefix-"
 	nameSuffix := "-nameSuffix"
 	namespace := "custom-namespace"
-	kustomize := NewKustomizeApp(appPath, git.NopCreds{}, "", "")
+	kustomize := NewKustomizeApp(appPath, appPath, git.NopCreds{}, "", "")
 	env := &v1alpha1.Env{
 		&v1alpha1.EnvEntry{Name: "ARGOCD_APP_NAME", Value: "argo-cd-tests"},
 	}
@@ -122,7 +122,7 @@ func TestKustomizeBuild(t *testing.T) {
 func TestFailKustomizeBuild(t *testing.T) {
 	appPath, err := testDataDir(t, kustomization1)
 	assert.Nil(t, err)
-	kustomize := NewKustomizeApp(appPath, git.NopCreds{}, "", "")
+	kustomize := NewKustomizeApp(appPath, appPath, git.NopCreds{}, "", "")
 	kustomizeSource := v1alpha1.ApplicationSourceKustomize{
 		Replicas: []v1alpha1.KustomizeReplica{
 			{
@@ -221,7 +221,7 @@ func TestKustomizeBuildForceCommonLabels(t *testing.T) {
 	for _, tc := range testCases {
 		appPath, err := testDataDir(t, tc.TestData)
 		assert.Nil(t, err)
-		kustomize := NewKustomizeApp(appPath, git.NopCreds{}, "", "")
+		kustomize := NewKustomizeApp(appPath, appPath, git.NopCreds{}, "", "")
 		objs, _, err := kustomize.Build(&tc.KustomizeSource, nil, tc.Env)
 		switch tc.ExpectErr {
 		case true:
@@ -313,7 +313,7 @@ func TestKustomizeBuildForceCommonAnnotations(t *testing.T) {
 	for _, tc := range testCases {
 		appPath, err := testDataDir(t, tc.TestData)
 		assert.Nil(t, err)
-		kustomize := NewKustomizeApp(appPath, git.NopCreds{}, "", "")
+		kustomize := NewKustomizeApp(appPath, appPath, git.NopCreds{}, "", "")
 		objs, _, err := kustomize.Build(&tc.KustomizeSource, nil, tc.Env)
 		switch tc.ExpectErr {
 		case true:
@@ -333,7 +333,7 @@ func TestKustomizeCustomVersion(t *testing.T) {
 	kustomizePath, err := testDataDir(t, kustomization4)
 	assert.Nil(t, err)
 	envOutputFile := kustomizePath + "/env_output"
-	kustomize := NewKustomizeApp(appPath, git.NopCreds{}, "", kustomizePath+"/kustomize.special")
+	kustomize := NewKustomizeApp(appPath, appPath, git.NopCreds{}, "", kustomizePath+"/kustomize.special")
 	kustomizeSource := v1alpha1.ApplicationSourceKustomize{
 		Version: "special",
 	}
@@ -355,7 +355,7 @@ func TestKustomizeCustomVersion(t *testing.T) {
 func TestKustomizeBuildPatches(t *testing.T) {
 	appPath, err := testDataDir(t, kustomization5)
 	assert.Nil(t, err)
-	kustomize := NewKustomizeApp(appPath, git.NopCreds{}, "", "")
+	kustomize := NewKustomizeApp(appPath, appPath, git.NopCreds{}, "", "")
 
 	kustomizeSource := v1alpha1.ApplicationSourceKustomize{
 		Patches: []v1alpha1.KustomizePatch{

util/session/sessionmanager.go

@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"os"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/coreos/go-oidc/v3/oidc"
@@ -41,6 +42,7 @@ type SessionManager struct {
 	storage                       UserStateStorage
 	sleep                         func(d time.Duration)
 	verificationDelayNoiseEnabled bool
+	failedLock                    sync.RWMutex
 }
 
 // LoginAttempts is a timestamped counter for failed login attempts
@@ -69,7 +71,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
@@ -284,7 +286,7 @@ func (mgr *SessionManager) Parse(tokenString string) (jwt.Claims, string, error)
 	return token.Claims, newToken, nil
 }
 
-// GetLoginFailures retrieves the login failure information from the cache
+// GetLoginFailures retrieves the login failure information from the cache. Any modifications to the LoginAttemps map must be done in a thread-safe manner.
 func (mgr *SessionManager) GetLoginFailures() map[string]LoginAttempts {
 	// Get failures from the cache
 	var failures map[string]LoginAttempts
@@ -299,25 +301,43 @@ func (mgr *SessionManager) GetLoginFailures() map[string]LoginAttempts {
 	return failures
 }
 
-func expireOldFailedAttempts(maxAge time.Duration, failures *map[string]LoginAttempts) int {
+func expireOldFailedAttempts(maxAge time.Duration, failures map[string]LoginAttempts) int {
 	expiredCount := 0
-	for key, attempt := range *failures {
+	for key, attempt := range failures {
 		if time.Since(attempt.LastFailed) > maxAge*time.Second {
 			expiredCount += 1
-			delete(*failures, key)
+			delete(failures, key)
 		}
 	}
 	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) {
+	mgr.failedLock.Lock()
+	defer mgr.failedLock.Unlock()
 
 	failures := mgr.GetLoginFailures()
 
 	// Expire old entries in the cache if we have a failure window defined.
 	if window := getLoginFailureWindow(); window > 0 {
-		count := expireOldFailedAttempts(window, &failures)
+		count := expireOldFailedAttempts(window, failures)
 		if count > 0 {
 			log.Infof("Expired %d entries from session cache due to max age reached", count)
 		}
@@ -327,23 +347,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]
@@ -374,6 +384,8 @@ func (mgr *SessionManager) updateFailureCount(username string, failed bool) {
 
 // Get the current login failure attempts for given username
 func (mgr *SessionManager) getFailureCount(username string) LoginAttempts {
+	mgr.failedLock.RLock()
+	defer mgr.failedLock.RUnlock()
 	failures := mgr.GetLoginFailures()
 	attempt, ok := failures[username]
 	if !ok {

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)
+		}
+	})
+}