chore(otp): set a clamp to prevent invalid values for otp expiration

Author: annabkr

internal/conf/configuration.go

@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"math"
 	"net/url"
 	"regexp"
 	"strings"
@@ -25,6 +26,12 @@ const defaultChallengeExpiryDuration float64 = 300
 const defaultFactorExpiryDuration time.Duration = 300 * time.Second
 const defaultFlowStateExpiryDuration time.Duration = 300 * time.Second
 
+// maxOtpExp is the largest OTP validity window, in seconds, that survives
+// conversion to a time.Duration, which counts nanoseconds in an int64. That is
+// roughly 292 years. ApplyDefaults clamps OtpExp to it so the conversion cannot
+// overflow into a negative duration, which would expire tokens at creation.
+const maxOtpExp uint = math.MaxInt64 / uint(time.Second)
+
 // See: https://www.postgresql.org/docs/7.0/syntax525.htm
 var postgresNamesRegexp = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]{0,62}$`)
 
@@ -696,7 +703,8 @@ type MailerConfiguration struct {
 }
 
 func (c *MailerConfiguration) OtpExpAsDuration() time.Duration {
-	return time.Duration(c.OtpExp) * time.Second // #nosec G115 -- OtpExp comes from trusted config, not user input
+	// OtpExp is clamped to maxOtpExp in ApplyDefaults, so this cannot overflow.
+	return time.Duration(c.OtpExp) * time.Second // #nosec G115
 }
 
 func (c *MailerConfiguration) Validate() error {
@@ -789,7 +797,8 @@ type SmsProviderConfiguration struct {
 }
 
 func (c *SmsProviderConfiguration) OtpExpAsDuration() time.Duration {
-	return time.Duration(c.OtpExp) * time.Second // #nosec G115 -- OtpExp comes from trusted config, not user input
+	// OtpExp is clamped to maxOtpExp in ApplyDefaults, so this cannot overflow.
+	return time.Duration(c.OtpExp) * time.Second // #nosec G115
 }
 
 func (c *SmsProviderConfiguration) GetTestOTP(phone string, now time.Time) (string, bool) {
@@ -1211,6 +1220,10 @@ func (config *GlobalConfiguration) ApplyDefaults() error {
 		config.Mailer.OtpExp = 86400 // 1 day
 	}
 
+	if config.Mailer.OtpExp > maxOtpExp {
+		config.Mailer.OtpExp = maxOtpExp
+	}
+
 	if config.Mailer.OtpLength == 0 || config.Mailer.OtpLength < 6 || config.Mailer.OtpLength > 10 {
 		// 6-digit otp by default
 		config.Mailer.OtpLength = 6
@@ -1228,6 +1241,10 @@ func (config *GlobalConfiguration) ApplyDefaults() error {
 		config.Sms.OtpExp = 60
 	}
 
+	if config.Sms.OtpExp > maxOtpExp {
+		config.Sms.OtpExp = maxOtpExp
+	}
+
 	if config.Sms.OtpLength == 0 || config.Sms.OtpLength < 6 || config.Sms.OtpLength > 10 {
 		// 6-digit otp by default
 		config.Sms.OtpLength = 6

internal/conf/configuration_test.go

@@ -3,6 +3,7 @@ package conf
 import (
 	"encoding/base64"
 	"errors"
+	"math"
 	"os"
 	"sort"
 	"strings"
@@ -1251,3 +1252,65 @@ func TestProviderLinkingDomainsDecode(t *testing.T) {
 		require.Error(t, d.Decode("custom:github="))
 	}
 }
+
+func TestApplyDefaultsClampsOtpExp(t *testing.T) {
+	baseConfig := func() *GlobalConfiguration {
+		c := &GlobalConfiguration{}
+		c.JWT.Secret = "secret"
+		return c
+	}
+
+	cases := []struct {
+		desc       string
+		mailerExp  uint
+		smsExp     uint
+		wantMailer uint
+		wantSms    uint
+	}{
+		{
+			desc:       "zero takes the per-channel default",
+			mailerExp:  0,
+			smsExp:     0,
+			wantMailer: 86400,
+			wantSms:    60,
+		},
+		{
+			desc:       "in-range values pass through untouched",
+			mailerExp:  3600,
+			smsExp:     120,
+			wantMailer: 3600,
+			wantSms:    120,
+		},
+		{
+			desc:       "the ceiling itself is not clamped",
+			mailerExp:  maxOtpExp,
+			smsExp:     maxOtpExp,
+			wantMailer: maxOtpExp,
+			wantSms:    maxOtpExp,
+		},
+		{
+			desc:       "values above the ceiling clamp down to it",
+			mailerExp:  maxOtpExp + 1,
+			smsExp:     math.MaxUint,
+			wantMailer: maxOtpExp,
+			wantSms:    maxOtpExp,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			cfg := baseConfig()
+			cfg.Mailer.OtpExp = c.mailerExp
+			cfg.Sms.OtpExp = c.smsExp
+			require.NoError(t, cfg.ApplyDefaults())
+
+			require.Equal(t, c.wantMailer, cfg.Mailer.OtpExp)
+			require.Equal(t, c.wantSms, cfg.Sms.OtpExp)
+
+			// the clamp exists so that this conversion cannot overflow into a
+			// negative duration, which would expire every token at creation
+			require.Positive(t, cfg.Mailer.OtpExpAsDuration())
+			require.Positive(t, cfg.Sms.OtpExpAsDuration())
+		})
+	}
+}