feat: Microsoft 365 OAuth2 IMAP support

[YourEconProf]

I do still need to make edits to README.md for the organizational O365 account. Personal hotmail account is easy.

Summary

  - Microsoft 365 IMAP support: New add-o365 command sets up OAuth2 authentication for Microsoft accounts,
  then syncs mail over IMAP using the XOAUTH2 SASL mechanism
  - XOAUTH2 auth layer: Added AuthMethod config field and XOAUTH2 SASL client so IMAP connections can
  authenticate with OAuth2 bearer tokens instead of passwords
  - Microsoft OAuth2 manager: Full browser-based OAuth2 flow with PKCE, token persistence, automatic refresh,
   and email validation via ID token claims
  - Personal vs. org account support: Automatically detects personal Microsoft domains (hotmail.com,
  outlook.com, live.com) and uses the correct IMAP scope (outlook.office.com vs outlook.office365.com)
  - Account lookup by email: sync-full and sync now resolve IMAP accounts by display name when the bare email
   is passed (e.g., sync-full matt@5.life finds imaps://matt@5.life@outlook.office365.com:993)
  - Config: New [microsoft] section in config.toml for Azure AD client_id and tenant_id

  Tested with

  - Organizational O365 account (custom domain)
  - Personal Hotmail account
  - Gmail account (no regression)
  - Full sync of ~10K messages over IMAP/XOAUTH2

  Test plan

  - msgvault add-o365 user@org.com --tenant — org account auth flow
  - msgvault add-o365 user@hotmail.com — personal account auth flow (auto-detects scope)
  - msgvault sync-full user@org.com --limit 100 — IMAP sync with XOAUTH2
  - msgvault sync-full user@hotmail.com --limit 100 — personal account sync
  - msgvault sync-full user@gmail.com --limit 100 — Gmail still works
  - Unit tests pass (make test)

[roborev-ci]

roborev: Combined Review (e1748d1)

Verdict: Changes are not clean; 1 high-severity regression and 1 medium-severity robustness issue need attention.

High

• internal/microsoft/oauth.go:243: resolveTokenEmail now treats the ID token as authoritative for mailbox identity, but extractEmailFromIDToken falls back to preferred_username when email is absent. In Azure AD, preferred_username is often the sign-in UPN rather than the primary SMTP address, so org accounts where those differ can now fail with TokenMismatchError. This is a regression relative to the prior /me.mail behavior.

Medium

• internal/microsoft/oauth.go:30: scopesForEmail selects personal vs org IMAP scopes using a short hard-coded domain allowlist. Consumer Microsoft accounts using other Outlook/Hotmail aliases or regional domains can still be misclassified as org accounts and receive the wrong IMAP scope, so the auth failure this change is intended to fix can still occur.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (6b128fe)

Verdict: Changes are not ready to merge due to 1 high-severity and 2 medium-severity issues in Microsoft OAuth account verification.

High

• internal/microsoft/oauth.go:312 (also reported at internal/microsoft/oauth.go:305)
  resolveTokenEmail can accept the user-supplied email even when the ID token lacks an email claim and preferred_username does not match. Because login_hint is only advisory, a user can complete the browser flow with a different Microsoft account, and the code may then bind/store that token under the requested mailbox. This is an account-binding/authentication bypass relative to the prior verification path.
  Recommended fix: Require an authoritative identifier match before saving the token. If email is missing and preferred_username does not case-insensitively match the requested address, fail authorization or perform a trusted follow-up identity lookup.

Medium

• internal/microsoft/oauth.go:150
  The new scope-correction flow attempts to switch from the O365 IMAP resource to the personal-account IMAP resource by using the refresh token after the initial browser flow. That does not establish consent for the new resource, so custom-domain consumer accounts can still fail authorization despite this change intending to fix that case.
  Recommended fix: When tid shows the initial scope guess was wrong, restart the interactive authorization flow with the corrected IMAP scope instead of trying to retarget the grant via refresh.

• internal/microsoft/oauth.go:328
extractIDTokenClaims decodes the JWT payload without validating signature, issuer, audience, expiry, or nonce, but those claims are then used for security-sensitive decisions including account verification and tenant/scope selection. Trusting unvalidated ID token claims is unsafe.
  Recommended fix: Validate the ID token against Microsoft OIDC metadata/JWKS and check at least signature, iss, aud, exp, and nonce; include a nonce in the auth request and verify it on return.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (3839fbc)

Summary verdict: Changes are generally solid, but there are 3 medium-severity regressions that should be addressed before merge.

Medium

• internal/microsoft/oauth.go:262
resolveTokenEmail now hard-fails when the ID token has no email claim and preferred_username does not match the user-entered SMTP address. This breaks legitimate Entra/O365 setups where the mailbox address differs from the sign-in UPN, which the previous Graph-based lookup handled.
  Fix: fall back to a trusted mailbox-address source when email is absent (for example Graph /me), or bind to a stable directory identity and store the mailbox address separately instead of treating UPN mismatch as fatal.

• internal/microsoft/oauth.go:191
TokenSource reuses whatever scopes are already persisted in the token file. Accounts authorized before this change will keep stale organizational IMAP scopes, so existing personal/custom-domain Microsoft accounts will not benefit from the new scope-correction logic unless they manually reauthorize.
  Fix: add a migration or scope revalidation path when loading/refreshing stored Microsoft tokens, and force reauthorization when saved scopes are stale or mismatched.

• cmd/msgvault/cmd/sync.go:103, cmd/msgvault/cmd/syncfull.go:98, internal/store/sources.go:38
  The new CLI fallback assumes display_name is unique, but GetSourcesByDisplayName returns all matches. With the updated add-o365 guidance to run sync-full <email>, duplicate display names can cause one command to target multiple sources unintentionally.
  Fix: treat multiple display-name matches as an ambiguity error and require the full identifier, or enforce uniqueness for display names used as CLI selectors.

Notes

• The security review did not identify any new security issues in the diff.
• No other medium-or-higher findings were reported.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[wesm]

I'm closing my PR and will let this one be the definitive branch until it gets landed into main

[roborev-ci]

roborev: Combined Review (aeed1b7)

Verdict: Changes are close, but there is 1 high-severity regression risk and 1 medium-severity concurrency issue that should be addressed before merge.

High

• Tenant override is not persisted across refresh/sync flows
  Location: cmd/msgvault/cmd/addo365.go:37, internal/microsoft/oauth.go:184
  The add-o365 --tenant ... override is only applied during the initial interactive auth flow and is not stored per account. Later operations reconstruct the Microsoft manager from global config (cfg.Microsoft.EffectiveTenantID()), so refreshes can fall back to common instead of the tenant-specific authority required by some single-tenant app registrations. Accounts added with --tenant can therefore stop working after the initial access token expires.
  Suggested fix: Persist the authority tenant alongside the account/token and use that persisted tenant when building the token endpoint for TokenSource and subsequent sync/remove flows. Add a regression test that authorizes with a tenant override, then validates refresh behavior from a fresh manager instance.

Medium

• TokenSource ignores caller context and updates shared token state unsafely
  Location: internal/microsoft/oauth.go:214-226
  The closure returned by Manager.TokenSource uses the outer ctx instead of the per-call callCtx, and it mutates tf.Token without synchronization. If multiple syncs or connections refresh concurrently, this can produce a data race and inconsistent token state.
  Suggested fix: Guard shared token updates with a sync.Mutex, and call cfg.TokenSource(callCtx, &tf.Token).Token() inside the closure so refreshes respect the caller’s context.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (7ca2071)

Verdict: Changes are not safe to merge yet due to one high-severity schema migration regression.

High

• Schema migration missing for oauth_app column
  • Location: internal/store/schema.sql:22, internal/store/sources.go:43, internal/store/sources.go:46
  • Problem: oauth_app was added only in the CREATE TABLE IF NOT EXISTS definition, which does not update existing SQLite databases. The new read path now selects oauth_app, so older databases will fail with no such column: oauth_app when code paths like display-name source lookup run.
  • Fix: Add an explicit migration during schema initialization, such as ALTER TABLE sources ADD COLUMN oauth_app TEXT when the column is missing, and add a migration test covering upgrade from a pre-change database.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (7c13655)

Verdict: Changes are not ready to merge due to 1 High and 2 Medium findings.

High

• internal/microsoft/oauth.go
  • Problem: The OAuth callback handler and ListenAndServe goroutine can block forever when sending to errChan or codeChan if more than one request reaches the callback endpoint. With a buffer size of 1, a second send can hang the handler, which then causes server.Shutdown(ctx) to wait indefinitely and freeze the CLI process.
  • Fix: Make channel sends non-blocking, for example with select { case ch <- v: default: }.

Medium

• Location: cmd/msgvault/cmd/sync.go:140, cmd/msgvault/cmd/syncfull.go:127

  • Problem: XOAUTH2 IMAP sources still consider a legacy password credential file sufficient because credential presence is checked before the source auth method. If an Office 365 source was migrated from password auth, a stale password file can make it appear usable even when the Microsoft token is missing, so sync proceeds and fails later instead of skipping or prompting for re-auth.
  • Fix: Parse sync_config first for IMAP sources and, when EffectiveAuthMethod() is AuthXOAuth2, ignore HasCredentials(...) and require msMgr.HasToken(imapCfg.Username).

• Location: cmd/msgvault/cmd/sync.go:85, cmd/msgvault/cmd/syncfull.go:79

  • Problem: Display-name lookup only runs when GetSourcesByIdentifier(args[0]) returns no rows. If another source already uses that email as its identifier, an O365 IMAP source with the same email in display_name is excluded silently, making msgvault sync[-full] <email> unreliable in mixed setups.
  • Fix: Merge identifier and display-name matches, deduplicate by source ID, then sync the single resolved source or raise an explicit ambiguity error.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (bd91b3b)

Verdict: Changes look mostly solid, but there are 2 issues that should be addressed before merge.

High

• cmd/msgvault/cmd/build_cache.go:423
  If the Parquet verification query fails, countQueryOK is set to false and the zero-row guard is skipped, which allows _last_sync.json to still be written even though the export was not successfully verified. This can recreate the poisoned-state failure the guard is intended to prevent.
  Recommended fix: treat verification-query failure as a hard stop for state updates, and do not write the state file when the Parquet count/readback step fails.

Medium

• cmd/msgvault/cmd/sync.go:152
• cmd/msgvault/cmd/syncfull.go:139
  The IMAP auth precheck ignores ConfigFromJSON errors. If sync_config is malformed, the source is silently treated as having missing credentials and skipped, which masks configuration corruption behind a misleading recovery message.
  Recommended fix: handle parseErr explicitly and surface a configuration/parse error for that source instead of falling through to the missing-credentials path.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)