fix: Auto rotate token should be between DefaultAutoRotateBeforeMinTTL and DefaultAutoRotateBeforeMaxTTL

Author: ilijamt

defs.go

@@ -15,10 +15,10 @@ var (
 
 const (
 	DefaultConfigFieldAccessTokenMaxTTL = 7 * 24 * time.Hour
-	DefaultConfigFieldAccessTokenRotate = 2 * 24 * time.Hour
+	DefaultConfigFieldAccessTokenRotate = DefaultAutoRotateBeforeMinTTL
 	DefaultRoleFieldAccessTokenMaxTTL   = 24 * time.Hour
 	DefaultAccessTokenMinTTL            = 24 * time.Hour
 	DefaultAccessTokenMaxPossibleTTL    = 365 * 24 * time.Hour
-	DefaultAutoRotateBeforeMinFraction  = 0.1
-	DefaultAutoRotateBeforeMaxFraction  = 0.5
+	DefaultAutoRotateBeforeMinTTL       = 24 * time.Hour
+	DefaultAutoRotateBeforeMaxTTL       = 730 * time.Hour
 )

path_config.go

@@ -49,14 +49,14 @@ var (
 		"revoke_auto_rotated_token": {
 			Type:        framework.TypeBool,
 			Default:     false,
-			Description: `Should we revoke the autorotated token after a new one has been generated?`,
+			Description: `Should we revoke the auto-rotated token after a new one has been generated?`,
 			DisplayAttrs: &framework.DisplayAttributes{
 				Name: "Revoke auto rotated token",
 			},
 		},
 		"auto_rotate_before": {
 			Type:        framework.TypeDurationSecond,
-			Description: `How much time should be remaining on the token validity before we should rotate it?`,
+			Description: `How much time should be remaining on the token validity before we should rotate it? Minimum can be set to 24h and maximum to 730h`,
 			Default:     DefaultConfigFieldAccessTokenRotate,
 			DisplayAttrs: &framework.DisplayAttributes{
 				Name: "Auto rotate before",
@@ -145,16 +145,16 @@ func (b *Backend) pathConfigWrite(ctx context.Context, req *logical.Request, dat
 
 	if autoTokenRotateTtlOk {
 		atr, _ := convertToInt(autoTokenRotateRaw)
-		if atr > int(config.MaxTTL.Seconds()*DefaultAutoRotateBeforeMaxFraction) {
-			err = multierror.Append(err, fmt.Errorf("auto_rotate_token can not be bigger than %d%% (max: %s) of %s: %w", int(DefaultAutoRotateBeforeMaxFraction*100), time.Duration(config.MaxTTL.Seconds()*DefaultAutoRotateBeforeMaxFraction)*time.Second, config.MaxTTL.String(), ErrInvalidValue))
-		} else if atr <= int(config.MaxTTL.Seconds()*DefaultAutoRotateBeforeMinFraction) {
-			err = multierror.Append(err, fmt.Errorf("auto_rotate_token can not be less than %d%% (max: %s) of %s: %w", int(DefaultAutoRotateBeforeMinFraction*100), time.Duration(config.MaxTTL.Seconds()*DefaultAutoRotateBeforeMinFraction)*time.Second, config.MaxTTL.String(), ErrInvalidValue))
+		if atr > int(DefaultAutoRotateBeforeMaxTTL.Seconds()) {
+			err = multierror.Append(err, fmt.Errorf("auto_rotate_token can not be bigger than %s: %w", DefaultAutoRotateBeforeMaxTTL, ErrInvalidValue))
+		} else if atr <= int(DefaultAutoRotateBeforeMinTTL.Seconds()) {
+			err = multierror.Append(err, fmt.Errorf("auto_rotate_token can not be less than %s: %w", DefaultAutoRotateBeforeMinTTL, ErrInvalidValue))
 		} else {
 			config.AutoRotateBefore = time.Duration(atr) * time.Second
 		}
 	} else {
-		config.AutoRotateBefore = time.Duration(config.MaxTTL.Seconds()*DefaultAutoRotateBeforeMinFraction) * time.Second
-		warnings = append(warnings, fmt.Sprintf("auto_rotate_token not specified setting to %v (%d%% of %s)", config.AutoRotateBefore.String(), int(DefaultAutoRotateBeforeMinFraction*100), config.MaxTTL.String()))
+		config.AutoRotateBefore = DefaultAutoRotateBeforeMinTTL
+		warnings = append(warnings, fmt.Sprintf("auto_rotate_token not specified setting to %s", DefaultAutoRotateBeforeMinTTL))
 	}
 
 	if err != nil {

path_config_token_autorotate_test.go

@@ -52,12 +52,44 @@ func TestPathConfig_AutoRotate(t *testing.T) {
 			Data: map[string]interface{}{
 				"token":              "super-secret-token",
 				"max_ttl":            "48h",
-				"auto_rotate_before": "48h",
+				"auto_rotate_before": (gitlab.DefaultAutoRotateBeforeMaxTTL + time.Hour).String(),
 			},
 		})
-		require.Error(t, err)
+		require.ErrorIs(t, err, gitlab.ErrInvalidValue)
 		require.Nil(t, resp)
+	})
+
+	t.Run("auto_rotate_before should be set to correct value", func(t *testing.T) {
+		b, l, err := getBackend()
+		require.NoError(t, err)
+		resp, err := b.HandleRequest(context.Background(), &logical.Request{
+			Operation: logical.UpdateOperation,
+			Path:      gitlab.PathConfigStorage, Storage: l,
+			Data: map[string]interface{}{
+				"token":              "super-secret-token",
+				"max_ttl":            "48h",
+				"auto_rotate_before": "48h",
+			},
+		})
+		require.NoError(t, err)
+		require.NotNil(t, resp)
+		require.EqualValues(t, "48h0m0s", resp.Data["auto_rotate_before"])
+	})
+
+	t.Run("auto_rotate_before should be more than the minimal limit", func(t *testing.T) {
+		b, l, err := getBackend()
+		require.NoError(t, err)
+		resp, err := b.HandleRequest(context.Background(), &logical.Request{
+			Operation: logical.UpdateOperation,
+			Path:      gitlab.PathConfigStorage, Storage: l,
+			Data: map[string]interface{}{
+				"token":              "super-secret-token",
+				"max_ttl":            "48h",
+				"auto_rotate_before": (gitlab.DefaultAutoRotateBeforeMinTTL - time.Hour).String(),
+			},
+		})
 		require.ErrorIs(t, err, gitlab.ErrInvalidValue)
+		require.Nil(t, resp)
 	})
 
 	t.Run("auto_rotate_before should be set to min if not specified", func(t *testing.T) {
@@ -88,10 +120,8 @@ func TestPathConfig_AutoRotate(t *testing.T) {
 				"auto_rotate_before": "10h",
 			},
 		})
-		require.NoError(t, err)
-		require.NotNil(t, resp)
-		assert.NotEmpty(t, resp.Data["auto_rotate_before"])
-		assert.EqualValues(t, "10h0m0s", resp.Data["auto_rotate_before"])
+		require.ErrorIs(t, err, gitlab.ErrInvalidValue)
+		require.Nil(t, resp)
 	})
 }