MemberJunction / MemberJunction/MJ

MS Graph: ReplyToMessage and ForwardMessage cannot resolve a mailbox from the request, so a stored service-principal credential still cannot drive them

Open
#4,322 0 comments 0 reactions 0 assignees View on GitHub
bug priority: medium
Dominant language
TSQL
Stars
29
Forks
6
Avg merge
2d 1h
Merged PRs (30d)
323

Description

**Found by** review of #4276 (`fix/msgraph-account-email`), 2026-09-09. Not caused by that PR — it is what remains after it, and #4276 is correct to have stopped where it did.

### What happens

#4276 makes `accountEmail` a per-operation default rather than a required credential field, so a credential created from the **Azure Service Principal** type can finally drive MS Graph. It resolves a mailbox through `MSGraphProvider.resolveMailbox(operation, creds, ...preferred)`, which takes the first mailbox the request named and falls back to `creds.accountEmail`.

Thirteen of the fifteen call sites pass a request-level candidate. Two pass none:

```ts
// packages/Communication/providers/MSGraph/src/MSGraphProvider.ts
463: const mailbox = this.resolveMailbox('ReplyToMessage', creds);
634: const mailbox = this.resolveMailbox('ForwardMessage', creds);
```

So those two can only resolve `creds.accountEmail`, and a schema-conformant credential cannot supply one:

- `metadata/credential-types/schemas/azure-service-principal.schema.json` declares exactly three properties (`tenantId`, `clientId`, `clientSecret`) and requires all three. It does not set `additionalProperties: false`, so it would not *reject* a stored `accountEmail` — but it declares no such property, so there is no field for an operator to fill in.
- No credential-type schema anywhere under `metadata/` mentions `accountEmail`, so there is no alternative type that carries a mailbox either.

That leaves `AZURE_ACCOUNT_EMAIL` as the only source — the environment fallback the Credentials engine exists to make unnecessary, and the one `disableEnvironmentFallback: true` switches off.

**Net effect: with a stored Azure Service Principal credential and the environment fallback disabled, `ReplyToMessage` and `ForwardMessage` refuse.** Not a regression (before #4276 they threw earlier, from `resolveCredentials`), but they are the two operations that PR's premise does not reach.

### Not covered by tests

#4276's five new tests cover the four resolution cases through `GetMessages`. Neither Reply nor Forward is exercised, which is why the gap is invisible from the suite. Whichever fix lands should carry a test per call site.

### Two fixes, different sizes

**1. `ReplyToMessage` — one argument.** `ReplyToMessageParams` already carries `ContextData`, the same field eleven sibling call sites read as `params.ContextData?.Email`:

```ts
const mailbox = this.resolveMailbox('ReplyToMessage', creds, params.ContextData?.Email as string);
```

That makes Reply consistent with `GetSingleMessage`, `DeleteMessage`, `MoveMessage`, `ListFolders`, `MarkAsRead`, `ArchiveMessage`, `SearchMessages`, `ListAttachments` and `DownloadAttachment`, and reachable on a conformant credential. Independently shippable.

**2. `ForwardMessage` — needs a params change.** `ForwardMessageParams` (`packages/Communication/base-types/src/BaseProvider.ts`) has `MessageID`, `Message`, `ToRecipients`, `CCRecipients`, `BCCRecipients` — no `ContextData`, no identifier. Forward cannot name a mailbox at all without adding one, which is a cross-package addition to the provider contract.

### The decision this actually settles

#4276 raises, and declines, adding an optional `accountEmail` to the credential schema, on the grounds that a default mailbox belongs to the connection rather than to a principal shared across services that have no mailbox. That reasoning is sound and I agree with it — but it is worth recording that Forward is the case which makes the decision consequential: with the schema left alone, adding `ContextData` (or an explicit mailbox field) to `ForwardMessageParams` is the only route by which Forward ever works on a stored credential.

Filed as one issue rather than two because it is one root cause and one decision; the Reply half can close first, but closing it alone would leave the user-facing fact — some operations still do not work with a stored credential — half-tracked.

Contributor guide

Open the contributing guide

Research direction

Start in packages/Communication/providers/MSGraph/src/MSGraphProvider.ts at the ReplyToMessage and ForwardMessage call sites, then inspect ForwardMessageParams in packages/Communication/base-types/src/BaseProvider.ts and the Azure service-principal schema. Compare the existing GetMessages resolution tests and sibling ContextData call sites; done means both operations resolve a mailbox from a stored credential path and each call site has coverage without relying on environment fallback.

Written by the indexing model from the issue text.

Assessment

Tech stack
azure, typescript
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.