Fix SSL error and improve authentication failure#495
Conversation
- Add STARTTLS negotiation for port 143 with use_ssl=True - Create IMAPSecurityError exception for security violations - Refuse to continue unencrypted when SSL is required - Check server STARTTLS capability before attempting upgrade - Fail fast to prevent credential leakage on security downgrade Connection behavior: - Port 993 + use_ssl=True → SSL direct - Port 143 + use_ssl=True → STARTTLS (mandatory) - Port 143 + use_ssl=False → unencrypted (explicit choice) Fixes: SSL WRONG_VERSION_NUMBER error on port 143
Backend improvements: - Catch SSL handshake failures during IMAP4_SSL connection - Provide clear error message suggesting port 143 with STARTTLS - Add authentication failure logging with username context - Wrap login() in try-except to log auth errors before propagating Frontend improvements: - Detect "IMAP authentication failed" message in error responses - Display user-friendly credential error message in UI This helps users understand connection failures and authentication issues with clearer, actionable error messages.
📝 WalkthroughWalkthroughAdds a public Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant IMAPClient as IMAP Client
participant IMAPServer as IMAP Server
App->>IMAPClient: connect(host, port, use_ssl)
IMAPClient->>IMAPServer: TCP connect / greeting
alt use_ssl == true and port == 143 (STARTTLS path)
rect rgba(100, 150, 200, 0.5)
IMAPClient->>IMAPServer: CAPABILITY
IMAPServer-->>IMAPClient: capabilities
IMAPClient->>IMAPServer: STARTTLS
IMAPServer-->>IMAPClient: OK / NO
IMAPClient->>IMAPServer: TLS handshake (if OK)
IMAPServer-->>IMAPClient: TLS established
end
else SSL-direct (SSL on connect)
rect rgba(200, 100, 100, 0.5)
IMAPClient->>IMAPServer: SSL socket handshake
IMAPServer-->>IMAPClient: handshake OK / failure
end
end
rect rgba(100, 200, 150, 0.5)
IMAPClient->>IMAPServer: LOGIN username/password
IMAPServer-->>IMAPClient: OK / NO
end
alt security or handshake failure
IMAPClient->>App: raise IMAPSecurityError (logout attempted)
else auth failure
IMAPClient->>App: propagate auth error (logout attempted)
else success
IMAPClient->>App: return authenticated connection (UTF-8 enforced)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/backend/core/services/importer/imap.py`:
- Around line 128-133: The current exception handler logs the PII-containing
username (self.username) when self.connection.login raises imaplib.IMAP4.error;
remove the username from the log and instead log the mail server/port (e.g., use
self.host and self.port or connection.server attributes) along with the caught
exception object (e) in the logger.error call inside the except block that wraps
self.connection.login, and ensure no references to self.username or other PII
remain in that error message.
- Around line 101-121: The capability() and starttls() calls can raise
exceptions and currently are not exception-wrapped, so if they raise (e.g.,
imaplib.IMAP4.error or ssl.SSLError) the connection may never be logged out;
wrap the capability() call and the starttls() call in try/except blocks that on
any Exception call self.connection.logout() and re-raise an IMAPSecurityError
(or the original exception) with a descriptive message; also when parsing
capabilities, aggregate all response lines from data instead of only data[0]
(join/decode all items) to robustly detect "STARTTLS"; mirror the existing
login() cleanup pattern (use the same exception handling and logout behavior)
and reference the same symbols: self.connection.capability(),
self.connection.starttls(), self.connection.logout(), and IMAPSecurityError.
| typ, data = self.connection.capability() | ||
| capabilities = data[0].decode().upper() if data and data[0] else "" | ||
| if typ != "OK" or "STARTTLS" not in capabilities: | ||
| error_msg = ( | ||
| f"Server {self.server}:{self.port} does not support STARTTLS. " | ||
| "Encrypted connection required." | ||
| ) | ||
| logger.error(error_msg) | ||
| self.connection.logout() | ||
| raise IMAPSecurityError(error_msg) | ||
|
|
||
| # Attempt STARTTLS | ||
| status, response = self.connection.starttls() | ||
| if status != "OK": | ||
| error_msg = ( | ||
| f"STARTTLS failed for {self.server}:{self.port}: {response}. " | ||
| "Encrypted connection required." | ||
| ) | ||
| logger.error(error_msg) | ||
| self.connection.logout() | ||
| raise IMAPSecurityError(error_msg) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/backend/core/services/importer/imap.py | head -130 | tail -50Repository: suitenumerique/messages
Length of output: 2950
🏁 Script executed:
wc -l src/backend/core/services/importer/imap.pyRepository: suitenumerique/messages
Length of output: 113
🌐 Web query:
imaplib.IMAP4 capability() return value structure Python documentation
💡 Result:
In imaplib, most IMAP4 commands (including CAPABILITY) return a 2‑tuple:
typ: usually'OK'or'NO'data: a Pythonlistof response items, where each item is typicallybytes(sometimes a(header, literal)tuple for commands that return literals)【1】.
So, in practice, IMAP4.capability() returns something shaped like:
typ, data = M.capability()
# typ: 'OK' / 'NO'
# data: [b'IMAP4rev1 ...'] # a single bytes line with space-separated capabilitiesTo turn it into a list of capability tokens, you typically parse the first element, e.g. data[0].split() (after decoding if you want str).
Also note imaplib stores parsed capability tokens in the instance variable M.capabilities (used e.g. for AUTH=... mechanisms)【1】.
Sources: Python imaplib docs (response tuple format; capabilities attribute usage)【1】.
[1] https://docs.python.org/3/library/imaplib.html
🌐 Web query:
imaplib.IMAP4 starttls() exception handling Python
💡 Result:
imaplib.IMAP4.starttls() can fail in two main ways: imaplib-level protocol checks (raising IMAP4.error / IMAP4.abort) and TLS handshake/socket errors (raising ssl.SSLError, OSError, etc.).
What starttls() raises (in practice)
From the stdlib docs, the relevant exception hierarchy is: IMAP4.error (base), with IMAP4.abort as a subclass for server/protocol-level abort conditions. [1]
From the imaplib implementation, starttls() specifically raises: [2]
IMAP4.error('SSL support missing')if Python was built without SSL support.IMAP4.abort('TLS session already established')if you call it twice.IMAP4.abort('TLS not supported by server')if the server doesn’t advertiseSTARTTLSin capabilities.IMAP4.error("Couldn't establish TLS session")if the server responds non-OK to theSTARTTLScommand.- Additionally, the actual TLS wrap/handshake (
ssl_context.wrap_socket(...)) may raisessl.SSLErrororOSErrorduring negotiation. [2]
A typical exception-handling pattern
import imaplib, ssl
M = imaplib.IMAP4(host, 143)
try:
M.starttls(ssl.create_default_context())
except imaplib.IMAP4.abort as e:
# e.g. "TLS not supported by server" / "TLS session already established"
raise
except imaplib.IMAP4.error as e:
# e.g. "SSL support missing" / "Couldn't establish TLS session"
raise
except (ssl.SSLError, OSError) as e:
# handshake / socket problems
raiseIf you require TLS, you can also pre-check b"STARTTLS" in M.capabilities (or just rely on the IMAP4.abort('TLS not supported by server') you’ll get). [2]
Sources: Python imaplib docs for exception types [1]; imaplib.starttls() implementation showing the exact raised conditions [2].
🏁 Script executed:
cat -n src/backend/core/services/importer/imap.py | head -145 | tail -70Repository: suitenumerique/messages
Length of output: 3822
🏁 Script executed:
rg "def.*__init__|def.*connect" src/backend/core/services/importer/imap.py -A 2Repository: suitenumerique/messages
Length of output: 456
Wrap capability() and starttls() in try-except to ensure connection cleanup.
Both capability() and starttls() can raise unexpected exceptions (e.g., imaplib.IMAP4.error, ssl.SSLError). If either raises an exception not caught by the current explicit status checks, the connection won't be logged out, leaving it open.
Also, for robustness, aggregate all capability lines instead of checking only data[0], in case a server returns multiple response items.
The pattern matches the login() handling at line 128, which properly wraps the call with exception cleanup.
♻️ Suggested fix
- typ, data = self.connection.capability()
- capabilities = data[0].decode().upper() if data and data[0] else ""
+ try:
+ typ, data = self.connection.capability()
+ capabilities = " ".join(
+ part.decode().upper()
+ for part in (data or [])
+ if isinstance(part, (bytes, bytearray))
+ )
+ except Exception:
+ self.connection.logout()
+ raise
@@
- status, response = self.connection.starttls()
+ try:
+ status, response = self.connection.starttls()
+ except Exception:
+ self.connection.logout()
+ raise🤖 Prompt for AI Agents
In `@src/backend/core/services/importer/imap.py` around lines 101 - 121, The
capability() and starttls() calls can raise exceptions and currently are not
exception-wrapped, so if they raise (e.g., imaplib.IMAP4.error or ssl.SSLError)
the connection may never be logged out; wrap the capability() call and the
starttls() call in try/except blocks that on any Exception call
self.connection.logout() and re-raise an IMAPSecurityError (or the original
exception) with a descriptive message; also when parsing capabilities, aggregate
all response lines from data instead of only data[0] (join/decode all items) to
robustly detect "STARTTLS"; mirror the existing login() cleanup pattern (use the
same exception handling and logout behavior) and reference the same symbols:
self.connection.capability(), self.connection.starttls(),
self.connection.logout(), and IMAPSecurityError.
2 participants
Fix https://sentry.incubateur.anct.gouv.fr/organizations/sentry/issues/14719/events/f5f190afa2544d0db3904ffd50896845/
Improve authentication failure handling
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.