Graph work/school: request explicit mail scopes at add-time so consent prompts (#391)

[CityDweller]

Fixes #391.

Adding a work/school M365 account on a tenant that hasn't consented to QuickMail completes sign-in with no consent prompt, then every Graph mail call 403s (Authorization_RequestDenied). Reported by a tenant admin adding a first account on a brand-new tenant.

Cause: work/school mail requests only .default, which returns a token for what's already granted and doesn't drive an interactive consent prompt on a fresh tenant — so it silently yields a partial token. (Personal accounts already use explicit scopes for exactly this reason; contacts/calendar use explicit scopes and do prompt, which is why those worked and mail didn't.)

Fix: request explicit mail scopes at first connect for work/school (GraphMailScopesWork = Mail.ReadWrite, Mail.Send, User.Read) via new FirstConnectScopesFor, so adding the account prompts for consent — mirroring the personal-account path. Silent refreshes still use .default (DefaultScopesFor unchanged), so the requested-vs-declared match from #208 is preserved. MailboxSettings.ReadWrite (admin-restricted, server rules) is intentionally not requested at add-time so ordinary users aren't blocked by an admin-approval wall; .default picks it up after admin consent.

Tests: work/school first-connect returns the explicit set (not .default), personal returns the personal set, IMAP returns IMAP scopes, and refresh still uses .default. Full scope-selection suite green.

Small and self-contained; no change to the refresh path or the #208 fix.

[kellylford]

Context first: #393 (account setup) reworks how accounts are created, and one change there makes this PR considerably more load-bearing than it looks today. Work-or-school Microsoft accounts on a custom domain now default to the Graph backend, because the IMAP path requests outlook.office.com/IMAP.AccessAsUser.All + SMTP.Send, which most tenants have never consented to — sign-in ended at "your administrator needs to make a change" on a real tenant. So the Graph first-connect path this PR changes goes from a rare case to the ordinary one for business accounts.

That means this is worth getting exactly right, and I would like to work through it with you rather than merge it as-is. Findings below, roughly by severity. Some of these I am confident about; where I am not, I have said so.

1. The description and the diff disagree — this one is definite

The body and #391 both say:

GraphMailScopesWork = Mail.ReadWrite, Mail.Send, User.Read … MailboxSettings.ReadWrite (admin-restricted, server rules) is intentionally not requested at add-time

The diff ships eight scopes, including MailboxSettings.ReadWrite, Calendars.ReadWrite, Contacts.Read, People.Read, and User.ReadBasic.All — and FirstConnect_WorkSchool_RequestsTheFullDeclaredScopeSet pins all eight.

I think the code is the better of the two, so the fix is to correct the description rather than the array. Which matters, because the description's plan would break server rules: it says .default "picks up MailboxSettings.ReadWrite after admin consent", and per Microsoft's Example 3 that is not so — once a grant exists, .default returns the old grant silently and never re-prompts for a permission added later. A user who consented without it would have no in-app path to acquire it, and GraphServerRuleService would 403 permanently.

2. The root-cause claim does not match Microsoft's docs

The stated cause is that .default does not drive consent on a fresh tenant. Scopes and permissions, under ".default when the user gives consent", says it prompts unless consent was already granted for any delegated permission between the client and the resource — and Example 2 is explicitly the fresh-tenant case, where the user does see a consent page.

The documented gap is Example 3 instead: once any grant exists, .default does no incremental consent. That is a real problem and it may well be what your reporter hit — QuickMail's own RequestContactsConsentAsync / RequestCalendarConsentAsync request explicit scopes and therefore create a grant, which would put the tenant into exactly that state.

If that is the actual mechanism, the documented remedy is WithPrompt(Prompt.Consent) on the add-account path while staying on .default — much smaller, and it preserves #208's requested-equals-declared invariant by construction rather than by maintenance.

Two things would settle it: does the failing tenant's log show the 403 on a mail call, or on a /users directory call? Authorization_RequestDenied / "Insufficient privileges" is the directory-API error shape — a mail call short of Mail.ReadWrite usually returns ErrorAccessDenied or MailboxNotEnabledForRESTAPI. And had that account ever signed in to QuickMail before, or had contacts/calendar enabled?

3. Sharpest concrete risk: personal MSA on a vanity domain

FirstConnectScopesFor falls back to IsPersonalMicrosoftDomain(account.Username) when IsPersonalMicrosoftAccount is null — and at add-account time it is always null, because the temp account is built before sign-in. So the domain heuristic decides, and OAuthService.cs documents that it misses "a personal account on a custom domain" (which is why #233 added the tenant-id signal).

Those accounts would get GraphMailScopesWork, which contains User.ReadBasic.All — and this codebase already says org-only permissions "don't apply to personal accounts and are omitted". Requesting a scope the consumer endpoint does not recognise is the #239 shape that #242 fixed: sign-in fails outright rather than under-delivering.

#393 makes this reachable, since it routes any Microsoft address on a non-consumer domain to Graph. I handled the mirror image of this problem there by correcting after sign-in, when the tenant id is known. Simplest fix here is probably to not select the work set when IsPersonalMicrosoftAccount is null, or to drop User.ReadBasic.All from the first-connect set.

4. Stale-grant tenants may go from degraded to blocked

A tenant whose admin consented before Calendars.ReadWrite / Contacts.Read / People.Read were declared currently gets a partial .default token: mail works, contacts and calendar dead-end. With an explicit dynamic request, AAD must match each scope against the grant, so the un-granted ones can produce "Need admin approval" at add-account time — the user cannot add the account at all. That is worse than partial functionality. Worth confirming against a tenant in that state before merging.

5. One interactive path still uses .default

OAuthService.GetAccessTokenAsync catches MsalUiRequiredException and falls back to SignInInteractiveAsync(account, scopes, ct) with scopes = DefaultScopesFor(account). Every re-auth after a cache loss, a refresh-token expiry, or a revoked grant goes interactive on .default. By this PR's own theory that is the same bug on a different entry point — so either the theory needs adjusting or the fix is incomplete. GraphClient.SendAsync passes scopes: null into the same path.

6. The tests do not cover the change

All four call FirstConnectScopesFor directly. The only line that alters runtime behaviour is the DefaultScopesFor → FirstConnectScopesFor swap at the interactive call site — revert that one line and all four still pass. FirstConnect_WorkSchool_RequestsTheFullDeclaredScopeSet is a change-detector on an array literal, and its contents are what contradict the description.

Testing it properly needs a seam that observes the scope array actually handed to AcquireTokenInteractive; the 1-arg IOAuthService method hides scopes, so the existing fake cannot see them. We hit the same shape on #393 — several tests there passed with their fix reverted, and reverting each fix to watch the test fail was the only thing that caught it. Worth doing here.

Suggested next steps

1. Reconcile the description and Work/school M365 mail: no consent prompt on a fresh tenant (.default doesn't prompt) → mail 403s #391 with the eight-scope diff.
2. Post the failing tenant's log line, and whether that account had signed in or enabled contacts/calendar before.
3. Say why WithPrompt(Prompt.Consent) on .default is not the smaller fix — if it is, that is a much safer change.
4. Handle the personal-MSA-on-custom-domain case.
5. Route the MsalUiRequiredException fallback consistently, or explain why it is exempt.
6. Add a test that observes the scopes actually passed.

On ordering: I would land #393 first so this can be tested against the new default rather than shipping essentially unexercised and then silently becoming the default path. They do not collide mechanically — #393 does not touch OAuthService.cs, and although both edit OAuthServiceScopeSelectionTests.cs they insert at different points and are semantically compatible.

Genuinely good catch on the underlying symptom — a work account that signs in cleanly and then 403s on every mail call is exactly the kind of thing that is miserable to diagnose from the outside. I would just like the fix aimed at the mechanism we can prove.