fix(web): drop the Outlook presets — Microsoft refuses app passwords - #175
Merged
Conversation
Every preset was checked against its provider's own published documentation. All hosts, ports and TLS values were correct. The finding was who is listed, not what they were configured with. **Microsoft cannot work at all.** Exchange Online states plainly that "Basic authentication is now disabled in all tenants… no one (you or Microsoft support) can re-enable" it, and that the deprecation "also prevents the use of app passwords". Consumer Outlook.com likewise requires Modern Auth/OAuth2 for POP, IMAP and SMTP. No host/port combination makes an app password work, so offering `outlook.com`/`hotmail.com`/`live.com` presets handed the operator a credential their provider will refuse and an opaque auth failure to debug. specs/mail/mailbox-connection.md §3 already recorded Microsoft 365 business as **No** and consumer Outlook.com as *unresolved — test before claiming support*. The presets contradicted the spec that governs them. That row is now resolved to **No** with the citation, so the two agree. Typing such an address now says why: Outlook does not allow app passwords for IMAP or SMTP — it requires OAuth. Connecting this address here will not work. Also fixes a defect introduced by that notice: typing gmail.com and then outlook.com left Gmail's hosts in the form underneath the Outlook warning — one provider's settings beneath another provider's warning. An OAuth-only domain now clears the connection fields. Found by driving the form; `web/` has no tests (#172), so nothing else would have caught it. Verified live in the browser: gmail.com applies its preset, outlook.com shows the notice with empty fields, an unrecognized domain still opens Advanced. Unresolved and NOT guessed at: Apple publishes different usernames per leg for iCloud — the local part for IMAP, the full address for SMTP — and `ImapConnectInput` carries one `username` for both. Same "one field, two independent connections" shape as the TLS flag; recorded on #173. Gates: typecheck, web typecheck, web build, lint all exit 0; 1742 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe mailbox connection form removes Microsoft app-password presets, detects recognized OAuth-only domains, clears incompatible connection settings, and displays a warning. Documentation describes OAuth requirements, advisory detection, and unsupported custom business domains. ChangesOAuth-only mailbox support
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant ConnectInboxForm
participant OAUTH_ONLY_DOMAINS
participant ConnectionFields
Operator->>ConnectInboxForm: Enter mailbox domain
ConnectInboxForm->>ConnectInboxForm: Normalize domain
ConnectInboxForm->>OAUTH_ONLY_DOMAINS: Check OAuth-only domain
OAUTH_ONLY_DOMAINS-->>ConnectInboxForm: Return domain match
ConnectInboxForm->>ConnectionFields: Clear IMAP/SMTP settings
ConnectInboxForm-->>Operator: Display OAuth-only warning
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Adversarial Codex pass, run in place of CodeRabbit (rate limited, zero review objects on this head). Three findings, all real. **The list cannot detect Microsoft, and the previous doc let a reader think it could.** A Microsoft 365 business tenant uses its own domain — `ops@contoso. example` contains no Microsoft-shaped string — so it matches nothing, gets no notice, and the operator is invited to enter an app password Exchange Online will refuse. Subdomains miss for the same reason. Real detection needs MX or Autodiscover discovery at type-time, which this screen does not do. The list is now documented as an exact-match hint for well-known CONSUMER domains, with the gap stated plainly: adding domains narrows it and never closes it. Widened to the consumer variants a real operator would type: hotmail.co.uk/de/ fr/it/es, live.co.uk/ca/com.au, outlook.co.uk, msn.com, passport.com. **A trailing DNS dot bypassed both maps.** `user@outlook.com.` is a legal fully-qualified name resolving to the same host, but matched neither the preset map nor the OAuth-only map — no preset AND no warning. Domain normalization now strips it. **Spec §4's "disabled with an explanation" is only partially satisfied**, and that is now recorded in the spec rather than left to drift: the notice is advisory rather than a disabled control (so a diagnostic attempt stays possible), and a business tenant on its own domain is not recognised at all. Both deviations are deliberate and neither is silent. Codex cleared the state machine: gmail.com → outlook.com → gmail.com correctly re-applies the Gmail preset, because the OAuth-only branch resets `appliedPresetDomain`. Gates: typecheck, web typecheck, web build, lint all exit 0; 1742 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@specs/mail/mailbox-connection.md`:
- Around line 116-118: Update the Outlook.com resolution section in the
mailbox-connection specification to include direct Microsoft Learn or Microsoft
Support citations for both claims: Exchange Online disabling Basic
authentication and consumer Outlook.com requiring OAuth2 for POP, IMAP, and
SMTP. Place the links beside the relevant resolution text without changing the
documented behavior or claims.
In `@web/src/components/ConnectInboxForm.tsx`:
- Around line 309-320: Update the locked-address initialization in
ConnectInboxForm so OAuth-only domains from initialConfig also clear IMAP/SMTP
hosts and ports and cannot retain stored password-auth settings; preserve the
existing handleAddressChange behavior for editable addresses. Adjust the OAuth
advisory rendering near the existing notice condition so it remains visible when
lockAddress is true for an OAuth-only reconnect.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8095fc36-3b92-4b5d-bf70-6b1a4d5e9b9e
📒 Files selected for processing (2)
specs/mail/mailbox-connection.mdweb/src/components/ConnectInboxForm.tsx
Comment on lines
+116
to
+118
| | Outlook.com consumer | **No** | Resolved 2026-07-31: Microsoft requires OAuth2 for POP/IMAP/SMTP on consumer accounts too; app passwords are refused | | ||
|
|
||
| **Resolution of the Outlook.com row (2026-07-31).** The row above read *unresolved — test before claiming support* while the connect screen shipped `outlook.com`/`hotmail.com`/`live.com` presets regardless. Microsoft's own documentation settles it in both directions: Exchange Online states "Basic authentication is now disabled in all tenants… no one (you or Microsoft support) can re-enable" it, and notes the deprecation "also prevents the use of app passwords"; the consumer Outlook.com guidance likewise requires Modern Auth/OAuth2 for POP, IMAP and SMTP. Those presets are removed, and typing such an address now shows an explicit "requires OAuth" notice instead of offering settings that cannot authenticate. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -i -C 2 \
'learn\.microsoft\.com|support\.microsoft\.com|Basic authentication is now disabled|Modern Auth|OAuth2' \
specs/mail/mailbox-connection.mdRepository: Helpthread/helpthread
Length of output: 1250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- references/links in file ---\n'
rg -n -i '\[(Basic|Modern Auth|OAuth|OAuth2|POP|IMAP|SMTP|app password) [^\]]+\]\(|https?://|learn\.microsoft\.com|support\.microsoft\.com' specs/mail/mailbox-connection.md || true
printf '\n--- nearby sections ---\n'
sed -n '1-180p' specs/mail/mailbox-connection.mdRepository: Helpthread/helpthread
Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- references/links in file ---'
rg -n -i '\[.*\]\(|https?://|learn\.microsoft\.com|support\.microsoft\.com' specs/mail/mailbox-connection.md || true
printf '%s\n' ''
printf '%s\n' '--- nearby sections ---'
sed -n '1,180p' specs/mail/mailbox-connection.mdRepository: Helpthread/helpthread
Length of output: 18395
Add direct Microsoft citations to the Outlook.com resolution.
This section quotes Microsoft policy and claims both Exchange Online and consumer-outreach guidance, but specs/mail/mailbox-connection.md contains no learn.microsoft.com or support.microsoft.com links for these claims. Add direct references beside the resolution so Agents can validate the auth-block rationale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specs/mail/mailbox-connection.md` around lines 116 - 118, Update the
Outlook.com resolution section in the mailbox-connection specification to
include direct Microsoft Learn or Microsoft Support citations for both claims:
Exchange Online disabling Basic authentication and consumer Outlook.com
requiring OAuth2 for POP, IMAP, and SMTP. Place the links beside the relevant
resolution text without changing the documented behavior or claims.
Comment on lines
+309
to
+320
| // An OAuth-only domain clears whatever a previous preset left behind. | ||
| // Without this, typing gmail.com and then outlook.com leaves Gmail's hosts | ||
| // sitting underneath an "Outlook requires OAuth" warning — the form would | ||
| // be showing one provider's settings while warning about another's. | ||
| if (nextDomain !== null && OAUTH_ONLY_DOMAINS[nextDomain] !== undefined) { | ||
| setImapHost('') | ||
| setImapPort('') | ||
| setSmtpHost('') | ||
| setSmtpPort('') | ||
| setAppliedPresetDomain(null) | ||
| return | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle locked OAuth-only reconnects.
handleAddressChange does not run when lockAddress is true. An existing Outlook mailbox from initialConfig retains its stored IMAP/SMTP settings. Line 479 also hides the OAuth notice in this path. After password entry, the reconnect form enables an unsupported app-password attempt.
Initialize OAuth-only locked addresses with cleared connection fields. Render the advisory for locked reconnects too.
Proposed fix
+ const initialDomain = domainFromAddress(initialConfig?.address ?? '')
+ const initialIsOauthOnly =
+ initialDomain !== null && OAUTH_ONLY_DOMAINS[initialDomain] !== undefined
+
const [address, setAddress] = useState(initialConfig?.address ?? '')
const [password, setPassword] = useState('')
- const [imapHost, setImapHost] = useState(initialConfig?.imapHost ?? '')
+ const [imapHost, setImapHost] = useState(initialIsOauthOnly ? '' : initialConfig?.imapHost ?? '')
const [imapPort, setImapPort] = useState(
- initialConfig !== undefined ? String(initialConfig.imapPort) : '',
+ initialConfig !== undefined && !initialIsOauthOnly ? String(initialConfig.imapPort) : '',
)
- const [smtpHost, setSmtpHost] = useState(initialConfig?.smtpHost ?? '')
+ const [smtpHost, setSmtpHost] = useState(initialIsOauthOnly ? '' : initialConfig?.smtpHost ?? '')
const [smtpPort, setSmtpPort] = useState(
- initialConfig !== undefined ? String(initialConfig.smtpPort) : '',
+ initialConfig !== undefined && !initialIsOauthOnly ? String(initialConfig.smtpPort) : '',
)
...
- {oauthOnlyLabel !== undefined && !lockAddress && (
+ {oauthOnlyLabel !== undefined && (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/ConnectInboxForm.tsx` around lines 309 - 320, Update the
locked-address initialization in ConnectInboxForm so OAuth-only domains from
initialConfig also clear IMAP/SMTP hosts and ports and cannot retain stored
password-auth settings; preserve the existing handleAddressChange behavior for
editable addresses. Adjust the OAuth advisory rendering near the existing notice
condition so it remains visible when lockAddress is true for an OAuth-only
reconnect.
CodeRabbit reviewed this head (its earlier silence was a rate limit, not a verdict). Two findings, both real. **The reconnect path hid the reason.** The notice was gated on `!lockAddress`, copied from the preset hint above it — but a reconnect screen is precisely where an operator is most likely to retry a stored Outlook mailbox that has never been able to authenticate, and hiding the explanation there is backwards. It now renders on both paths, with wording matched to each. The stored host/port values are deliberately NOT cleared on the locked path. That screen exists to review an existing configuration; blanking it would destroy the context it is for. Clearing stays scoped to the case where the operator is actively typing a new address. **The spec asserted Microsoft's policy without citing it.** It quoted the documentation but linked nothing, so the claim could not be checked by whoever reads it next. Direct links added for both Exchange Online and consumer Outlook.com, plus the five provider pages the rest of §3 was verified against. Gates: typecheck, web typecheck, web build, lint all exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🟢 SAFE TO MERGE
Gates green. Decisions approved by the maintainer. Review adjudicated.
CodeRabbit: 2 findings, 2 real, both fixed. It was rate-limited when this PR opened and produced nothing; it reviewed on a later push. Its earlier green check was a rate limit, not a verdict.
Codex (adversarial, run while CodeRabbit was unavailable): 3 findings, 3 real, all addressed.
CodeRabbit's findings
lockAddress), so a stored Outlook mailbox could be retried with no explanationWhat the substitute pass found
ops@contoso.examplematches nothing and the operator is invited to enter a credential Exchange Online refusesuser@outlook.com.(trailing DNS dot) bypassed both maps — no preset and no warningCodex explicitly cleared the state machine:
gmail.com → outlook.com → gmail.comcorrectly re-applies the Gmail preset.The honest limit of this change: a fixed domain list narrows the gap and can never close it. A business tenant on its own domain is unrecognisable without an MX or Autodiscover lookup at type-time, which this screen does not do. Said so in the code and the spec instead of letting a longer list imply coverage it does not have.
Every connect-screen preset was checked against that provider's own published documentation. All hosts, ports and TLS values were correct. The finding is about who is listed, not how they were configured.
Decision provenance
What was wrong
Microsoft refuses app passwords for IMAP/POP/SMTP. From Exchange Online's deprecation doc (updated 2026-07-16):
and:
Consumer Outlook.com is the same — Microsoft Support requires Modern Auth/OAuth2 for POP, IMAP and SMTP.
No host/port combination makes this work. The preset handed an operator a credential their provider will not issue for this purpose, then failed with an opaque auth error.
specs/mail/mailbox-connection.md§3 already said so — Microsoft 365 business listed as No, consumer Outlook.com as "unresolved — test before claiming support" — and the presets shipped regardless. The presets contradicted the spec that governs them. That row now reads No with the citation.Verified, and correct as configured
A defect this change introduced, and fixed
Typing
@gmail.comand then@outlook.comleftimap.gmail.comin the form underneath the Outlook warning — one provider's settings beneath another provider's warning. An OAuth-only domain now clears the connection fields.Found by driving the form in a browser.
web/has no test infrastructure (#172), so nothing automated would have caught it.Verification
npm run typechecknpm run -w web typechecknpm run -w web buildnpm run lintnpm testLive browser check, against a dev server:
someone@gmail.comsomeone@outlook.comUnresolved — recorded, not guessed at
Apple publishes different usernames per leg for iCloud: the local part (
johnappleseed) for IMAP, the full address for SMTP.ImapConnectInputcarries oneusernamefor both — the same "one field, two independent connections" flaw as the TLS flag. Apple's guidance is to try the full address if the short form fails, so the preset may work as written, but it is untested and the data model cannot express the split. Added to #173.🤖 Generated with Claude Code
Summary by CodeRabbit