fix(notification): missing use of timeout (#4652)

internal/commands/storage_run.go

@@ -983,7 +983,7 @@ func (ctx *CmdCtx) StorageUserTOTPExportPNGRunE(cmd *cobra.Command, _ []string)
 	}
 
 	if dir == "" {
-		dir = utils.RandomString(8, utils.CharSetAlphaNumeric, false)
+		dir = utils.RandomString(8, utils.CharSetAlphaNumeric)
 	}
 
 	if _, err = os.Stat(dir); !os.IsNotExist(err) {

internal/commands/util.go

@@ -109,7 +109,7 @@ func flagsGetRandomCharacters(flags *pflag.FlagSet, flagNameLength, flagNameChar
 		}
 	}
 
-	return utils.RandomString(n, charset, true), nil
+	return utils.RandomString(n, charset), nil
 }
 
 func termReadConfirmation(flags *pflag.FlagSet, name, prompt, confirmation string) (confirmed bool, err error) {

internal/notification/smtp_notifier.go

@@ -6,6 +6,7 @@ import (
 	"crypto/x509"
 	"fmt"
 	"net/mail"
+	"os"
 	"strings"
 
 	"github.com/sirupsen/logrus"
@@ -19,13 +20,29 @@ import (
 
 // NewSMTPNotifier creates a SMTPNotifier using the notifier configuration.
 func NewSMTPNotifier(config *schema.SMTPNotifierConfiguration, certPool *x509.CertPool) *SMTPNotifier {
+	var tlsconfig *tls.Config
+
+	if config.TLS != nil {
+		tlsconfig = utils.NewTLSConfig(config.TLS, certPool)
+	}
+
 	opts := []gomail.Option{
 		gomail.WithPort(config.Port),
-		gomail.WithTLSConfig(utils.NewTLSConfig(config.TLS, certPool)),
+		gomail.WithTLSConfig(tlsconfig),
 		gomail.WithHELO(config.Identifier),
+		gomail.WithTimeout(config.Timeout),
+		gomail.WithoutNoop(),
+	}
+
+	ssl := config.Port == smtpPortSUBMISSIONS
+
+	if ssl {
+		opts = append(opts, gomail.WithSSL())
 	}
 
 	switch {
+	case ssl:
+		break
 	case config.DisableStartTLS:
 		opts = append(opts, gomail.WithTLSPolicy(gomail.NoTLS))
 	case config.DisableRequireTLS:
@@ -34,10 +51,6 @@ func NewSMTPNotifier(config *schema.SMTPNotifierConfiguration, certPool *x509.Ce
 		opts = append(opts, gomail.WithTLSPolicy(gomail.TLSMandatory))
 	}
 
-	if config.Port == smtpPortSUBMISSIONS {
-		opts = append(opts, gomail.WithSSL())
-	}
-
 	var domain string
 
 	at := strings.LastIndex(config.Sender.Address, "@")
@@ -89,9 +102,11 @@ func (n *SMTPNotifier) StartupCheck() (err error) {
 func (n *SMTPNotifier) Send(ctx context.Context, recipient mail.Address, subject string, et *templates.EmailTemplate, data any) (err error) {
 	msg := gomail.NewMsg(
 		gomail.WithMIMEVersion(gomail.Mime10),
-		gomail.WithBoundary(utils.RandomString(30, utils.CharSetAlphaNumeric, true)),
+		gomail.WithBoundary(utils.RandomString(30, utils.CharSetAlphaNumeric)),
 	)
 
+	setMessageID(msg, n.domain)
+
 	if err = msg.From(n.config.Sender.String()); err != nil {
 		return fmt.Errorf("notifier: smtp: failed to set from address: %w", err)
 	}
@@ -143,3 +158,12 @@ func (n *SMTPNotifier) Send(ctx context.Context, recipient mail.Address, subject
 
 	return nil
 }
+
+func setMessageID(msg *gomail.Msg, domain string) {
+	rn, _ := utils.RandomInt(100000000)
+	rm, _ := utils.RandomInt(10000)
+	rs := utils.RandomString(17, utils.CharSetAlphaNumeric)
+	pid := os.Getpid() + rm
+
+	msg.SetMessageIDWithValue(fmt.Sprintf("%d.%d%d.%s@%s", pid, rn, rm, rs, domain))
+}

internal/server/template.go

@@ -59,7 +59,7 @@ func ServeTemplatedFile(publicDir, file string, opts *TemplatedFileOptions) midd
 			ctx.SetContentTypeTextPlain()
 		}
 
-		nonce := utils.RandomString(32, utils.CharSetAlphaNumeric, true)
+		nonce := utils.RandomString(32, utils.CharSetAlphaNumeric)
 
 		switch {
 		case publicDir == assetsSwagger:

internal/utils/crypto.go

@@ -573,3 +573,50 @@ loop:
 
 	return extKeyUsage
 }
+
+// RandomString returns a random string with a given length with values from the provided characters. When crypto is set
+// to false we use math/rand and when it's set to true we use crypto/rand. The crypto option should always be set to true
+// excluding when the task is time sensitive and would not benefit from extra randomness.
+func RandomString(n int, characters string) (randomString string) {
+	return string(RandomBytes(n, characters))
+}
+
+// RandomBytes returns a random []byte with a given length with values from the provided characters. When crypto is set
+// to false we use math/rand and when it's set to true we use crypto/rand. The crypto option should always be set to true
+// excluding when the task is time sensitive and would not benefit from extra randomness.
+func RandomBytes(n int, characters string) (bytes []byte) {
+	bytes = make([]byte, n)
+
+	_, _ = rand.Read(bytes)
+
+	for i, b := range bytes {
+		bytes[i] = characters[b%byte(len(characters))]
+	}
+
+	return bytes
+}
+
+func RandomInt(n int) (int, error) {
+	if n <= 0 {
+		return 0, fmt.Errorf("n must be more than 0")
+	}
+
+	max := big.NewInt(int64(n))
+
+	if max.IsUint64() {
+		return 0, fmt.Errorf("generated max is uint64")
+	}
+
+	value, err := rand.Int(rand.Reader, max)
+	if err != nil {
+		return 0, err
+	}
+
+	output := int(value.Int64())
+
+	if output < 0 {
+		return 0, fmt.Errorf("generated number is too big for int")
+	}
+
+	return output, nil
+}

internal/utils/hashing_test.go

@@ -22,7 +22,7 @@ func TestShouldHashString(t *testing.T) {
 	assert.Equal(t, "ae448ac86c4e8e4dec645729708ef41873ae79c6dff84eff73360989487f08e5", anotherSum)
 	assert.NotEqual(t, sum, anotherSum)
 
-	randomInput := RandomString(40, CharSetAlphaNumeric, false)
+	randomInput := RandomString(40, CharSetAlphaNumeric)
 	randomSum := HashSHA256FromString(randomInput)
 
 	assert.NotEqual(t, randomSum, sum)
@@ -38,7 +38,7 @@ func TestShouldHashPath(t *testing.T) {
 	err = os.WriteFile(filepath.Join(dir, "anotherfile"), []byte("another\n"), 0600)
 	assert.NoError(t, err)
 
-	err = os.WriteFile(filepath.Join(dir, "randomfile"), []byte(RandomString(40, CharSetAlphaNumeric, true)+"\n"), 0600)
+	err = os.WriteFile(filepath.Join(dir, "randomfile"), []byte(RandomString(40, CharSetAlphaNumeric)+"\n"), 0600)
 	assert.NoError(t, err)
 
 	sum, err := HashSHA256FromPath(filepath.Join(dir, "myfile"))

internal/utils/strings.go

@@ -1,13 +1,10 @@
 package utils
 
 import (
-	crand "crypto/rand"
 	"fmt"
-	"math/rand"
 	"net/url"
 	"strconv"
 	"strings"
-	"time"
 	"unicode"
 
 	"github.com/valyala/fasthttp"
@@ -211,32 +208,6 @@ func StringSlicesDelta(before, after []string) (added, removed []string) {
 	return added, removed
 }
 
-// RandomString returns a random string with a given length with values from the provided characters. When crypto is set
-// to false we use math/rand and when it's set to true we use crypto/rand. The crypto option should always be set to true
-// excluding when the task is time sensitive and would not benefit from extra randomness.
-func RandomString(n int, characters string, crypto bool) (randomString string) {
-	return string(RandomBytes(n, characters, crypto))
-}
-
-// RandomBytes returns a random []byte with a given length with values from the provided characters. When crypto is set
-// to false we use math/rand and when it's set to true we use crypto/rand. The crypto option should always be set to true
-// excluding when the task is time sensitive and would not benefit from extra randomness.
-func RandomBytes(n int, characters string, crypto bool) (bytes []byte) {
-	bytes = make([]byte, n)
-
-	if crypto {
-		_, _ = crand.Read(bytes)
-	} else {
-		_, _ = rand.Read(bytes) //nolint:gosec // As this is an option when using this function it's not necessary to be concerned about this.
-	}
-
-	for i, b := range bytes {
-		bytes[i] = characters[b%byte(len(characters))]
-	}
-
-	return bytes
-}
-
 // StringHTMLEscape escapes chars for a HTML body.
 func StringHTMLEscape(input string) (output string) {
 	return htmlEscaper.Replace(input)
@@ -307,7 +278,3 @@ func IsURLHostComponentWithPort(u url.URL) (isHostComponentWithPort bool) {
 
 	return false
 }
-
-func init() {
-	rand.Seed(time.Now().UnixNano())
-}

internal/utils/strings_test.go

@@ -54,14 +54,10 @@ func TestStringJoinDelimitedEscaped(t *testing.T) {
 }
 
 func TestShouldNotGenerateSameRandomString(t *testing.T) {
-	randomStringOne := RandomString(10, CharSetAlphaNumeric, false)
+	randomStringOne := RandomString(10, CharSetAlphaNumeric)
-	randomStringTwo := RandomString(10, CharSetAlphaNumeric, false)
+	randomStringTwo := RandomString(10, CharSetAlphaNumeric)
-
-	randomCryptoStringOne := RandomString(10, CharSetAlphaNumeric, true)
-	randomCryptoStringTwo := RandomString(10, CharSetAlphaNumeric, true)
 
 	assert.NotEqual(t, randomStringOne, randomStringTwo)
-	assert.NotEqual(t, randomCryptoStringOne, randomCryptoStringTwo)
 }
 
 func TestShouldDetectAlphaNumericString(t *testing.T) {