Skip to content

fix: respect SMTP encryption setting for email notifications#8655

Open
sharkcreep87 wants to merge 1 commit intocoollabsio:nextfrom
sharkcreep87:fix/smtp-test-email-encryption-6442
Open

fix: respect SMTP encryption setting for email notifications#8655
sharkcreep87 wants to merge 1 commit intocoollabsio:nextfrom
sharkcreep87:fix/smtp-test-email-encryption-6442

Conversation

@sharkcreep87
Copy link
Contributor

Summary

  • Fixes the SMTP encryption setting being ignored when sending emails (test emails and all notifications)
  • Symfony's EsmtpTransport constructor expects bool|null for the TLS parameter, but the code was passing incorrect values:
    • 'none' mapped to null (auto-detect) which still attempted STARTTLS — now correctly maps to false
    • 'tls' mapped to the string 'tls' instead of boolean true — now correctly maps to true
Encryption Setting Before (broken) After (fixed) Symfony behavior
starttls null null (unchanged) Try STARTTLS if supported
tls 'tls' (string) true (bool) Force implicit TLS
none null (auto-detect) false (disabled) No encryption at all

Fixes #6442

Changes

File Change
app/Notifications/Channels/EmailChannel.php Fix match expression to return correct bool|null values for EsmtpTransport

Test plan

  • Configure SMTP with Encryption set to "None" (port 25) → Send Test Email → verify it succeeds without attempting STARTTLS
  • Configure SMTP with Encryption set to "TLS" (port 465) → Send Test Email → verify it connects with implicit TLS
  • Configure SMTP with Encryption set to "STARTTLS" (port 587) → Send Test Email → verify STARTTLS upgrade works
  • Verify team invitation emails also respect the encryption setting

The EsmtpTransport constructor expects bool|null for the tls parameter:
- null: auto-detect (try STARTTLS if supported)
- true: force implicit TLS (port 465)
- false: no encryption

Previously 'none' mapped to null which still attempted STARTTLS,
and 'tls' mapped to the string 'tls' instead of boolean true.

Fixes coollabsio#6442
Copy link
Contributor Author

@sharkcreep87 sharkcreep87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review — the fix aligns with Symfony's EsmtpTransport constructor signature (bool|null $tls):

Setting Before (broken) After (fixed) Symfony behavior
starttls null null (no change) Opportunistic STARTTLS
tls 'tls' (string) true (bool) Force implicit TLS
none null (auto-detect) false (disabled) No encryption

The 'none' => null mapping was the direct cause — null tells Symfony to auto-detect and attempt STARTTLS, which fails on servers that don't support it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant