fix: set default value for users.session.duration (argoproj#9962) (ar…

…goproj#10185) * fix: set default value for users.session.duration Signed-off-by: Takuya Yaginuma <biz.mkgoat.yagi+github@gmail.com> Signed-off-by: y-takuya <biz.mkgoat.yagi+github@gmail.com> * docs: add user.session.duration change Signed-off-by: y-takuya <biz.mkgoat.yagi+github@gmail.com> * docs: fix users.session.duration parameter name Signed-off-by: y-takuya <biz.mkgoat.yagi+github@gmail.com> * Retrigger CI pipeline Signed-off-by: y-takuya <biz.mkgoat.yagi+github@gmail.com> Co-authored-by: Michael Crenshaw <michael@crenshaw.dev>
ashutosh16 · Aug 11, 2022 · a0634e6 · a0634e6
1 parent d849884
commit a0634e6
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 9 deletions.
diff --git a/docs/operator-manual/upgrading/2.4-2.5.md b/docs/operator-manual/upgrading/2.4-2.5.md
@@ -7,6 +7,12 @@ In order to secure the communications between the dex server and the Argo CD API
 By default, without configuration, the dex server will generate a self-signed certificate upon startup. However, we recommend that users
 configure their own TLS certificate using the `argocd-dex-server-tls` secret. Please refer to the [TLS configuration guide](../tls.md#configuring-tls-to-argocd-dex-server) for more information.
 
+## Invalid users.session.duration values now fall back to 24h
+
+Before v2.5, an invalid `users.session.duration` value in argocd-cm would 1) log a warning and 2) result in user sessions having no duration limit.
+
+Starting with v2.5, invalid duration values will fall back to the default value of 24 hours with a warning.
+
 ## Out-of-bounds symlinks now blocked at fetch
 
 There have been several path traversal and identification vulnerabilities disclosed in the past related to symlinks. To help prevent any further vulnerabilities, we now scan all repositories and Helm charts for **out of bounds symlinks** at the time they are fetched and block further processing if they are found.

diff --git a/util/settings/settings.go b/util/settings/settings.go
@@ -1269,14 +1269,13 @@ func updateSettingsFromConfigMap(settings *ArgoCDSettings, argoCDCM *apiv1.Confi
 		log.Warnf("Failed to validate UI banner URL in configmap: %v", err)
 	}
 	settings.UiBannerURL = argoCDCM.Data[settingUiBannerURLKey]
+	settings.UserSessionDuration = time.Hour * 24
 	if userSessionDurationStr, ok := argoCDCM.Data[userSessionDurationKey]; ok {
 		if val, err := timeutil.ParseDuration(userSessionDurationStr); err != nil {
 			log.Warnf("Failed to parse '%s' key: %v", userSessionDurationKey, err)
 		} else {
 			settings.UserSessionDuration = *val
 		}
-	} else {
-		settings.UserSessionDuration = time.Hour * 24
 	}
 	settings.PasswordPattern = argoCDCM.Data[settingsPasswordPatternKey]
 	if settings.PasswordPattern == "" {

diff --git a/util/settings/settings_test.go b/util/settings/settings_test.go
@@ -8,6 +8,7 @@ import (
 	"sort"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
@@ -668,7 +669,7 @@ func TestSettingsManager_GetHelp(t *testing.T) {
 	t.Run("GetBinaryUrls", func(t *testing.T) {
 		_, settingsManager := fixtures(map[string]string{
 			"help.download.darwin-amd64": "amd64-path",
-			"help.download.linux-s390x": "s390x-path",
+			"help.download.linux-s390x":  "s390x-path",
 			"help.download.unsupported":  "nowhere",
 		})
 		h, err := settingsManager.GetHelp()
@@ -677,6 +678,103 @@ func TestSettingsManager_GetHelp(t *testing.T) {
 	})
 }
 
+func TestSettingsManager_GetSettings(t *testing.T) {
+	t.Run("UserSessionDurationNotProvided", func(t *testing.T) {
+		kubeClient := fake.NewSimpleClientset(
+			&v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      common.ArgoCDConfigMapName,
+					Namespace: "default",
+					Labels: map[string]string{
+						"app.kubernetes.io/part-of": "argocd",
+					},
+				},
+				Data: nil,
+			},
+			&v1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      common.ArgoCDSecretName,
+					Namespace: "default",
+					Labels: map[string]string{
+						"app.kubernetes.io/part-of": "argocd",
+					},
+				},
+				Data: map[string][]byte{
+					"server.secretkey": nil,
+				},
+			},
+		)
+		settingsManager := NewSettingsManager(context.Background(), kubeClient, "default")
+		s, err := settingsManager.GetSettings()
+		assert.NoError(t, err)
+		assert.Equal(t, time.Hour*24, s.UserSessionDuration)
+	})
+	t.Run("UserSessionDurationInvalidFormat", func(t *testing.T) {
+		kubeClient := fake.NewSimpleClientset(
+			&v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      common.ArgoCDConfigMapName,
+					Namespace: "default",
+					Labels: map[string]string{
+						"app.kubernetes.io/part-of": "argocd",
+					},
+				},
+				Data: map[string]string{
+					"users.session.duration": "10hh",
+				},
+			},
+			&v1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      common.ArgoCDSecretName,
+					Namespace: "default",
+					Labels: map[string]string{
+						"app.kubernetes.io/part-of": "argocd",
+					},
+				},
+				Data: map[string][]byte{
+					"server.secretkey": nil,
+				},
+			},
+		)
+		settingsManager := NewSettingsManager(context.Background(), kubeClient, "default")
+		s, err := settingsManager.GetSettings()
+		assert.NoError(t, err)
+		assert.Equal(t, time.Hour*24, s.UserSessionDuration)
+	})
+	t.Run("UserSessionDurationProvided", func(t *testing.T) {
+		kubeClient := fake.NewSimpleClientset(
+			&v1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      common.ArgoCDConfigMapName,
+					Namespace: "default",
+					Labels: map[string]string{
+						"app.kubernetes.io/part-of": "argocd",
+					},
+				},
+				Data: map[string]string{
+					"users.session.duration": "10h",
+				},
+			},
+			&v1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      common.ArgoCDSecretName,
+					Namespace: "default",
+					Labels: map[string]string{
+						"app.kubernetes.io/part-of": "argocd",
+					},
+				},
+				Data: map[string][]byte{
+					"server.secretkey": nil,
+				},
+			},
+		)
+		settingsManager := NewSettingsManager(context.Background(), kubeClient, "default")
+		s, err := settingsManager.GetSettings()
+		assert.NoError(t, err)
+		assert.Equal(t, time.Hour*10, s.UserSessionDuration)
+	})
+}
+
 func TestGetOIDCConfig(t *testing.T) {
 	kubeClient := fake.NewSimpleClientset(
 		&v1.ConfigMap{
@@ -1184,9 +1282,9 @@ func TestArgoCDSettings_OIDCTLSConfig_OIDCTLSInsecureSkipVerify(t *testing.T) {
 	certParsed, err := tls.X509KeyPair(test.Cert, test.PrivateKey)
 	require.NoError(t, err)
 
-	testCases := []struct{
-		name string
-		settings *ArgoCDSettings
+	testCases := []struct {
+		name               string
+		settings           *ArgoCDSettings
 		expectNilTLSConfig bool
 	}{
 		{
@@ -1219,12 +1317,12 @@ requestedScopes: ["oidc"]
 rootCA: "invalid"`},
 		},
 		{
-			name: "OIDC not configured, no cert configured",
-			settings: &ArgoCDSettings{},
+			name:               "OIDC not configured, no cert configured",
+			settings:           &ArgoCDSettings{},
 			expectNilTLSConfig: true,
 		},
 		{
-			name: "OIDC not configured, cert configured",
+			name:     "OIDC not configured, cert configured",
 			settings: &ArgoCDSettings{Certificate: &certParsed},
 		},
 	}