mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-07-09 16:00:59 +00:00
fix(email): eliminate data race on provider username resolution
sendViaProviderWithAddresses mutated the shared e.config.Username for provider-specific defaults (SendGrid, Postmark, SparkPost, Resend). If concurrent goroutines sent email simultaneously, this was a data race on the config struct. Move the resolution into negotiateAuth via a local variable (resolveProviderUsername helper) so the shared config is never mutated.
This commit is contained in:
parent
c07fce0c33
commit
22ed97f562
2 changed files with 37 additions and 45 deletions
|
|
@ -237,32 +237,45 @@ func buildMultipartEmailMessage(addresses resolvedEmailAddresses, subject, htmlB
|
|||
// and returns the best smtp.Auth to use. Prefers PLAIN, falls back to LOGIN.
|
||||
// Returns nil if auth is not configured.
|
||||
func (e *EnhancedEmailManager) negotiateAuth(client *smtp.Client) (smtp.Auth, error) {
|
||||
if !e.config.AuthRequired || e.config.Username == "" || e.config.Password == "" {
|
||||
username := resolveProviderUsername(e.config)
|
||||
if !e.config.AuthRequired || username == "" || e.config.Password == "" {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// Check what the server advertises after EHLO.
|
||||
// Extension returns (ok bool, params string).
|
||||
hasAuth, mechanismsRaw := client.Extension("AUTH")
|
||||
mechanisms := strings.ToUpper(mechanismsRaw)
|
||||
|
||||
if strings.Contains(mechanisms, "PLAIN") {
|
||||
return &plainAuth{"", e.config.Username, e.config.Password}, nil
|
||||
return &plainAuth{"", username, e.config.Password}, nil
|
||||
}
|
||||
if strings.Contains(mechanisms, "LOGIN") {
|
||||
return LoginAuth(e.config.Username, e.config.Password), nil
|
||||
return LoginAuth(username, e.config.Password), nil
|
||||
}
|
||||
|
||||
// Server didn't advertise AUTH at all — try PLAIN as a default since
|
||||
// it's the most widely supported. This handles servers that don't
|
||||
// properly advertise their capabilities.
|
||||
if !hasAuth || mechanisms == "" {
|
||||
return &plainAuth{"", e.config.Username, e.config.Password}, nil
|
||||
return &plainAuth{"", username, e.config.Password}, nil
|
||||
}
|
||||
|
||||
return nil, fmt.Errorf("server advertises AUTH mechanisms [%s] but none are supported (PLAIN, LOGIN)", mechanisms)
|
||||
}
|
||||
|
||||
func resolveProviderUsername(config EmailProviderConfig) string {
|
||||
if config.Username != "" {
|
||||
return config.Username
|
||||
}
|
||||
switch config.Provider {
|
||||
case "SendGrid":
|
||||
return "apikey"
|
||||
case "Postmark":
|
||||
return config.Password
|
||||
case "SparkPost":
|
||||
return "SMTP_Injection"
|
||||
case "Resend":
|
||||
return "resend"
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// EnhancedEmailManager extends email functionality with provider support
|
||||
type EnhancedEmailManager struct {
|
||||
config EmailProviderConfig
|
||||
|
|
@ -391,29 +404,8 @@ func (e *EnhancedEmailManager) sendViaProvider(msg []byte) error {
|
|||
func (e *EnhancedEmailManager) sendViaProviderWithAddresses(msg []byte, addresses resolvedEmailAddresses) error {
|
||||
addr := net.JoinHostPort(e.config.SMTPHost, strconv.Itoa(e.config.SMTPPort))
|
||||
|
||||
// Special handling for specific providers
|
||||
switch e.config.Provider {
|
||||
case "SendGrid":
|
||||
// SendGrid uses "apikey" as username
|
||||
if e.config.Username == "" {
|
||||
e.config.Username = "apikey"
|
||||
}
|
||||
case "Postmark":
|
||||
// Postmark uses API token for both username and password
|
||||
if e.config.Password != "" && e.config.Username == "" {
|
||||
e.config.Username = e.config.Password
|
||||
}
|
||||
case "SparkPost":
|
||||
// SparkPost uses specific username
|
||||
if e.config.Username == "" {
|
||||
e.config.Username = "SMTP_Injection"
|
||||
}
|
||||
case "Resend":
|
||||
// Resend uses "resend" as username
|
||||
if e.config.Username == "" {
|
||||
e.config.Username = "resend"
|
||||
}
|
||||
}
|
||||
// Provider-specific usernames are resolved in negotiateAuth without
|
||||
// mutating the shared config struct.
|
||||
|
||||
// Send with TLS configuration — auth is negotiated after connection
|
||||
if e.config.TLS || e.config.SMTPPort == 465 {
|
||||
|
|
|
|||
|
|
@ -138,7 +138,7 @@ func TestSendViaProvider_ProviderUsernameDefaults(t *testing.T) {
|
|||
t.Run(tt.name, func(t *testing.T) {
|
||||
config := EmailProviderConfig{
|
||||
EmailConfig: EmailConfig{
|
||||
SMTPHost: "invalid.host.test", // Will fail to connect
|
||||
SMTPHost: "invalid.host.test",
|
||||
SMTPPort: 587,
|
||||
Username: tt.initialUsername,
|
||||
Password: "test",
|
||||
|
|
@ -149,12 +149,12 @@ func TestSendViaProvider_ProviderUsernameDefaults(t *testing.T) {
|
|||
AuthRequired: true,
|
||||
}
|
||||
|
||||
manager := NewEnhancedEmailManager(config)
|
||||
// Call sendViaProvider - it will fail on connection, but will have set username
|
||||
_ = manager.sendViaProvider([]byte("test"))
|
||||
|
||||
if manager.config.Username != tt.expectedUsername {
|
||||
t.Errorf("expected username %q, got %q", tt.expectedUsername, manager.config.Username)
|
||||
got := resolveProviderUsername(config)
|
||||
if got != tt.expectedUsername {
|
||||
t.Errorf("expected username %q, got %q", tt.expectedUsername, got)
|
||||
}
|
||||
if config.Username != tt.initialUsername {
|
||||
t.Errorf("config was mutated: expected %q, got %q", tt.initialUsername, config.Username)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
@ -174,12 +174,12 @@ func TestSendViaProvider_PostmarkUsernameFromPassword(t *testing.T) {
|
|||
AuthRequired: true,
|
||||
}
|
||||
|
||||
manager := NewEnhancedEmailManager(config)
|
||||
_ = manager.sendViaProvider([]byte("test"))
|
||||
|
||||
// Postmark copies password to username when username is empty
|
||||
if manager.config.Username != "postmark-api-token" {
|
||||
t.Errorf("expected username to be set from password, got %q", manager.config.Username)
|
||||
got := resolveProviderUsername(config)
|
||||
if got != "postmark-api-token" {
|
||||
t.Errorf("expected username to be set from password, got %q", got)
|
||||
}
|
||||
if config.Username != "" {
|
||||
t.Errorf("config was mutated: expected empty, got %q", config.Username)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue