Add Outlook (Microsoft Graph) email provider support#3887
Draft
jbecke wants to merge 5 commits into
Draft
Conversation
Foundational layer for Outlook email support, mirroring the Gmail integration: - models_email: add UserProvider::Outlook and a new outlook module with Microsoft Graph resource models (message, mailFolder, attachment), delta-query models (the Outlook analogue of Gmail history), and change- notification/subscription models (the analogue of Gmail watch + Pub/Sub). - outlook_client: new crate wrapping the Microsoft Graph mail API, mirroring gmail_client's surface -- fetch/send messages, conversations (threads), delta sync, mail folders mapped to system labels, attachments, and webhook subscriptions. Includes a pure, unit-tested Graph->service message mapper. - email / email_db_client: thread UserProvider::Outlook through the domain and DB layers; add a forward-only migration adding OUTLOOK to the email_user_provider_enum Postgres enum. Verified: models_email + outlook_client compile with 15 passing unit tests; email and email_db_client compile.
Mirror the Gmail FusionAuth linking flow for Microsoft (Outlook):
- fusionauth: add a microsoft module (refresh/exchange/parse) and Microsoft
client credentials on FusionAuthClient via a with_microsoft_credentials
builder, so the three existing FusionAuthClient::new call sites are untouched.
- model + authentication_service_client: add MicrosoftAccessToken and
get_microsoft_access_token, mirroring the Google equivalents.
- authentication_service:
- POST /link/outlook + GET /link/outlook/status (Microsoft authorize URL).
- oauth2 /microsoft/callback wired into the shared callback handler; links
the Microsoft IdP to the user and stashes the linked email for /email/init.
- GET /internal/microsoft_access_token: looks up the user's microsoft_outlook
IdP link and refreshes the stored refresh token into a Graph access token.
- config: optional MICROSOFT_CLIENT_ID / MICROSOFT_CLIENT_SECRET_KEY
(default empty so the service still boots when Outlook is unconfigured).
Verified: fusionauth (with new microsoft test), model,
authentication_service_client, and authentication_service all compile.
- Add outlook_client to ApiContext and PubSubContext (and both construction sites); optional OUTLOOK_NOTIFICATION_URL / OUTLOOK_CLIENT_STATE config that default empty so the service boots when Outlook is unconfigured. - util/outlook/auth: fetch Microsoft access tokens via the auth service, with the same revoked-access -> enqueue-link-deletion behavior as Gmail. - api/outlook/webhook: Graph change-notification endpoint handling the validation-token handshake and per-notification clientState verification (the analogue of verifying Gmail's Google-signed JWT), nested at /outlook/webhook. - inbox_sync: new OutlookNotification operation that runs the read side of a delta sync end-to-end (token -> folders -> delta -> per-message fetch + convert to the provider-agnostic Message), wired into the inbox-sync process match. Follow-ons (deliberately deferred, marked with TODOs): persisting the subscription->link mapping so the webhook can enqueue, persisting the @odata.deltaLink for incremental sync, and reusing a provider-agnostic inbound upsert to store converted messages. Verified: email_service and the dependent email handlers compile.
Mirror the Gmail multi-inbox connect flow for Outlook in the settings UI: - service-auth client: initOutlookLink() hits POST /link/outlook and returns the Microsoft OAuth authorization URL (response shape inlined to avoid depending on a regenerated schema). - queries/auth: useInitOutlookLink mutation, mirroring useInitGmailLink. - settings/Account: an "Add Outlook inbox" button next to the Gmail one that starts the link flow and redirects to the authorization URL; the existing inbox-link callback then provisions the inbox (provider-agnostic). Verified: app package typechecks (tsc, 0 errors) and Biome passes on the changed files.
Add OUTLOOK_NOTIFICATION_URL and OUTLOOK_CLIENT_STATE to the email-service task environment via optional config (config.get, empty default) so deploys keep working before the Outlook integration is provisioned. The service treats empty values as "Outlook disabled". Provisioning follow-ups (separate, as discussed): the Azure app registration + FusionAuth Microsoft IdP, the MICROSOFT_CLIENT_ID / MICROSOFT_CLIENT_SECRET_KEY for the auth service (Doppler-managed there), and sourcing OUTLOOK_CLIENT_STATE from Secrets Manager.
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis pull request adds comprehensive Outlook (Microsoft Graph) email support to the email system. The changes enable users to link Microsoft accounts, authenticate via OAuth2, and synchronize Outlook emails with the system. The implementation spans data modeling, authentication infrastructure, a new OutlookClient for Graph API operations, email service integration, pub/sub-based inbox synchronization, and frontend UI for account linking. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. |
Contributor
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
rust/cloud-storage/outlook_client/Cargo.toml (1)
1-23:⚠️ Potential issue | 🟠 MajorAdd
#![deny(missing_docs)]tooutlook_clientcrate root
#![deny(missing_docs)]is not present inrust/cloud-storage/outlook_client/src/lib.rs; add it to enforce documentation requirements for public items in this crate.🤖 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 `@rust/cloud-storage/outlook_client/Cargo.toml` around lines 1 - 23, Add the crate-level deny for missing docs by opening outlook_client's crate root (src/lib.rs) and inserting the crate attribute #![deny(missing_docs)] at the top of the file (just under any existing crate-level doc comments) so the outlook_client crate enforces documentation on public items; ensure the attribute is placed before any module or item definitions so the linter applies to the entire crate.Source: Coding guidelines
rust/cloud-storage/authentication_service/src/main.rs (1)
147-172:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize Outlook credential gating to avoid partial-config runtime failures.
If only one of the two Microsoft config values is set, this keeps a non-empty
microsoft_client_idbut passes an empty secret, which pushes the error to OAuth runtime (invalid_client) instead of cleanly disabling Outlook or failing startup.Suggested fix
- let microsoft_client_secret = if config.microsoft_client_id.is_empty() - || config.microsoft_client_secret_key.is_empty() - { - String::new() - } else { - match config.environment { - Environment::Local => config.microsoft_client_secret_key.clone(), - _ => secretsmanager_client - .get_secret_value(&config.microsoft_client_secret_key) - .await - .context("unable to get microsoft client secret")? - .to_string(), - } - }; + let (microsoft_client_id, microsoft_client_secret) = if config.microsoft_client_id.is_empty() + || config.microsoft_client_secret_key.is_empty() + { + (String::new(), String::new()) + } else { + let secret = match config.environment { + Environment::Local => config.microsoft_client_secret_key.clone(), + _ => secretsmanager_client + .get_secret_value(&config.microsoft_client_secret_key) + .await + .context("unable to get microsoft client secret")? + .to_string(), + }; + (config.microsoft_client_id.clone(), secret) + }; @@ - .with_microsoft_credentials(config.microsoft_client_id.clone(), microsoft_client_secret); + .with_microsoft_credentials(microsoft_client_id, microsoft_client_secret);🤖 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 `@rust/cloud-storage/authentication_service/src/main.rs` around lines 147 - 172, The code currently allows a non-empty config.microsoft_client_id while microsoft_client_secret is empty, causing runtime OAuth errors; change the gating so Outlook creds are only enabled when BOTH config.microsoft_client_id and config.microsoft_client_secret_key are non-empty: compute a boolean like microsoft_enabled = !config.microsoft_client_id.is_empty() && !config.microsoft_client_secret_key.is_empty(), fetch the secret only when microsoft_enabled (using Environment::Local vs secretsmanager_client.get_secret_value), and either pass empty strings for both or skip calling with_microsoft_credentials unless microsoft_enabled; update usage around microsoft_client_secret, config.microsoft_client_id, secretsmanager_client.get_secret_value, and the auth_client.with_microsoft_credentials call accordingly.
🧹 Nitpick comments (5)
js/app/packages/app/component/settings/Account.tsx (1)
667-688: 💤 Low valueConsider consistent button styling between Gmail and Outlook.
The Gmail button uses only a
<PlusIcon>, while the Outlook button displays text "Outlook". For visual consistency, consider either giving both an icon-only style with differentiated tooltips, or both showing provider names.🤖 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 `@js/app/packages/app/component/settings/Account.tsx` around lines 667 - 688, The Gmail and Outlook add buttons differ in visual treatment; update the Button usages around handleAddInbox and handleAddOutlookInbox to use a consistent style (either both icon-only or both label+icon). For example, modify the Button wrapping PlusIcon (used with Tooltip and onClick=handleAddInbox) and the Button for Outlook (onClick=handleAddOutlookInbox) so they share the same props (variant, size, depth, aria-label) and visual content—either render a provider label plus an icon for both or render icons only (e.g., use PlusIcon for both and differentiate via Tooltip text and aria-label). Ensure Tooltip labels and aria-labels remain descriptive and adjust the Outlook Button content to match the chosen pattern.rust/cloud-storage/email_service/src/config.rs (1)
211-217: ⚡ Quick winFail fast on partial Outlook configuration.
OUTLOOK_NOTIFICATION_URLandOUTLOOK_CLIENT_STATEshould be validated as an all-or-none pair. Allowing only one value to be set can produce hard-to-debug runtime behavior.Suggested guard
let outlook_notification_url = std::env::var("OUTLOOK_NOTIFICATION_URL").unwrap_or_default(); let outlook_client_state = std::env::var("OUTLOOK_CLIENT_STATE").unwrap_or_default(); + if outlook_notification_url.is_empty() != outlook_client_state.is_empty() { + anyhow::bail!( + "OUTLOOK_NOTIFICATION_URL and OUTLOOK_CLIENT_STATE must either both be set or both be empty" + ); + }🤖 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 `@rust/cloud-storage/email_service/src/config.rs` around lines 211 - 217, Treat OUTLOOK_NOTIFICATION_URL and OUTLOOK_CLIENT_STATE as an all-or-none pair: after reading std::env::var into outlook_notification_url and outlook_client_state, add a validation that if exactly one of them is non-empty you return an error (or panic) from the surrounding config-loading function so the service fails fast on partial Outlook configuration; if both are empty continue as optional, and if both are present accept them. Use the variable names outlook_notification_url and outlook_client_state and the env var names OUTLOOK_NOTIFICATION_URL / OUTLOOK_CLIENT_STATE to locate where to add the check.infra/stacks/email-service/index.ts (1)
94-95: 🏗️ Heavy liftAvoid passing Graph
clientStateas a plain container env value.
OUTLOOK_CLIENT_STATEis the webhook authenticity secret, but it’s currently injected as a regular env var value. Prefer runtime secret resolution (Secrets Manager-backed flow) so this verifier secret isn’t broadly exposed via task/env inspection paths.Also applies to: 450-457
🤖 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 `@infra/stacks/email-service/index.ts` around lines 94 - 95, OUTLOOK_CLIENT_STATE is being loaded via config.get and exposed as a plain environment variable; change this to resolve at runtime from a secrets-backed source instead of config.get so the webhook clientState is not exposed in task/env inspection. Replace the plain constant initialization of OUTLOOK_CLIENT_STATE (and similar vars around the other occurrences) with a Secrets Manager-backed secret reference and wire it into the task/container using the platform’s secrets injection mechanism (e.g., ECS TaskDefinition container secrets or equivalent), updating any code that currently reads OUTLOOK_CLIENT_STATE to read from the injected secret at runtime.rust/cloud-storage/email_service/src/util/outlook/auth.rs (1)
16-16: ⚡ Quick winAdd
errto thetracing::instrumentattribute.This function returns
Result, so missingerrdrops structured error capture on failures.Suggested patch
-#[tracing::instrument(skip(auth_service_client, sqs_client))] +#[tracing::instrument(skip(auth_service_client, sqs_client), err)]As per coding guidelines, "Include
errwhen adding thetracing::instrumentattribute to functions that returnResult."🤖 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 `@rust/cloud-storage/email_service/src/util/outlook/auth.rs` at line 16, The tracing attribute on the function annotated with #[tracing::instrument(skip(auth_service_client, sqs_client))] should include error capture; update the attribute to #[tracing::instrument(skip(auth_service_client, sqs_client), err)] so that any Err returned from the function is recorded by tracing (keep the same skipped args and add the err flag on the function that uses auth_service_client and sqs_client).Source: Coding guidelines
rust/cloud-storage/email_service/src/pubsub/inbox_sync/operations/outlook_notification.rs (1)
26-26: ⚡ Quick winAdd
errto thetracing::instrumentattribute.This function returns
Result<(), ProcessingError>, soerrshould be included for structured error traces.Suggested patch
-#[tracing::instrument(skip(ctx))] +#[tracing::instrument(skip(ctx), err)]As per coding guidelines, "Include
errwhen adding thetracing::instrumentattribute to functions that returnResult."🤖 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 `@rust/cloud-storage/email_service/src/pubsub/inbox_sync/operations/outlook_notification.rs` at line 26, The tracing attribute on the function annotated with #[tracing::instrument(skip(ctx))] (the function that returns Result<(), ProcessingError>) must include error tracking; update the attribute to add err (i.e., #[tracing::instrument(skip(ctx), err)]) so returned Err variants are recorded in structured traces.Source: Coding guidelines
🤖 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
`@rust/cloud-storage/authentication_service/src/api/internal/microsoft_access_token.rs`:
- Around line 18-25: The span is currently capturing and Debug-formatting
MicrosoftAccessTokenParams (which contains PII) exposing user identifiers/email;
remove the Debug derive from MicrosoftAccessTokenParams and change the span
instrumentation (the code that records `params` around lines 29-45) to avoid
logging the whole struct — either log only non-PII indicators (e.g., a boolean
like `has_email_link` or `fusionauth_user_id_present`), log a redacted field
(e.g., email => "<redacted>"), or implement a custom Debug/formatter that
redacts sensitive fields before passing to the span. Keep serde::Deserialize on
MicrosoftAccessTokenParams so deserialization still works but ensure any
trace/log call uses the redacted or minimal data instead of the raw params.
In
`@rust/cloud-storage/email_service/src/pubsub/inbox_sync/operations/outlook_notification.rs`:
- Around line 101-134: The loop converts messages but never persists or applies
deletes; update the handler to (1) persist each converted message by calling the
provider-agnostic upsert used by the Gmail path (e.g. the existing
operations::upsert_message or the inbound upsert extracted for provider-agnostic
use) using ctx and link.id, await it and map errors into ProcessingError like
the existing get_message handling, and (2) iterate over
changes.message_ids_to_delete and call the corresponding delete/remove operation
(e.g. operations::delete_message or the unified inbound delete) to remove
messages from storage and trigger downstream work; after each successful
upsert/delete, trigger the same search notification and CRM/notification fan-out
the Gmail path uses so state and side-effects match (use link.id, the converted
message from map_message_resource_to_service, and ctx for client/DB access).
- Around line 32-43: The current map_err on fetch_token_or_delete_on_revocation
labels every failure as ProcessingError::NonRetryable which prevents retries on
transient auth-service outages; change the error mapping for
crate::util::outlook::auth::fetch_token_or_delete_on_revocation to inspect the
returned error and convert transient/auth-service failures into
ProcessingError::Retryable(DetailedError { reason:
FailureReason::AccessTokenFetchFailed, source: ... }) while only mapping genuine
token-revocation or irrecoverable errors to ProcessingError::NonRetryable;
implement this by matching the error (or using its is_transient()/classification
method) in the closure that currently builds DetailedError so
fetch_token_or_delete_on_revocation -> .await.map_err(|e| match e { transient =>
ProcessingError::Retryable(...), permanent => ProcessingError::NonRetryable(...)
})
In `@rust/cloud-storage/model/src/authentication/microsoft_token.rs`:
- Around line 4-8: The MicrosoftAccessToken struct currently derives Debug which
risks leaking bearer tokens; remove Debug from the derive list on pub struct
MicrosoftAccessToken so it only derives serde::Serialize and serde::Deserialize,
and search for any explicit uses of MicrosoftAccessToken::fmt or Debug
formatting (e.g., format!("{:?}", token) or log statements) and replace them
with safe/redacted logging if needed.
In `@rust/cloud-storage/outlook_client/src/lib.rs`:
- Line 1: The crate root (lib.rs) is missing the crate-level attribute to
enforce documentation; add the line #![deny(missing_docs)] at the top of lib.rs
(before the module/doc comments) so the compiler denies missing documentation
for this new crate and ensures all public items are documented.
- Line 65: OutlookClient::new currently uses reqwest::Client::new() with no
timeout; update the constructor to build the shared HTTP client with an explicit
request timeout (e.g., using
reqwest::Client::builder().timeout(Duration::from_secs(30)) or a configurable
value) instead of Client::new(), so slow/stalled Graph calls will time out;
modify the place that sets inner (the field initialized from
reqwest::Client::new()) to use the builder and ensure you import
std::time::Duration and handle any builder errors if needed.
In `@rust/cloud-storage/outlook_client/src/messages.rs`:
- Around line 21-26: The code currently interpolates opaque Graph IDs directly
into path segments (e.g., in messages.rs functions get_message and send_message
using message_id, parent_id, draft.id), which can contain reserved characters
like '/' and must be percent-encoded; update all ID-based URL path segment
usages (messages.rs, attachments.rs, delta.rs, subscriptions.rs) to
percent-encode the IDs before formatting the URL (only for path segments, not
query params) and use a proper percent-encoding routine (e.g., utf8
percent-encode with a path-segment safe encode set) so URLs like
"/me/messages/{encoded_id}" and "/me/messages/{encoded_id}/send" are built with
encoded IDs.
---
Outside diff comments:
In `@rust/cloud-storage/authentication_service/src/main.rs`:
- Around line 147-172: The code currently allows a non-empty
config.microsoft_client_id while microsoft_client_secret is empty, causing
runtime OAuth errors; change the gating so Outlook creds are only enabled when
BOTH config.microsoft_client_id and config.microsoft_client_secret_key are
non-empty: compute a boolean like microsoft_enabled =
!config.microsoft_client_id.is_empty() &&
!config.microsoft_client_secret_key.is_empty(), fetch the secret only when
microsoft_enabled (using Environment::Local vs
secretsmanager_client.get_secret_value), and either pass empty strings for both
or skip calling with_microsoft_credentials unless microsoft_enabled; update
usage around microsoft_client_secret, config.microsoft_client_id,
secretsmanager_client.get_secret_value, and the
auth_client.with_microsoft_credentials call accordingly.
In `@rust/cloud-storage/outlook_client/Cargo.toml`:
- Around line 1-23: Add the crate-level deny for missing docs by opening
outlook_client's crate root (src/lib.rs) and inserting the crate attribute
#![deny(missing_docs)] at the top of the file (just under any existing
crate-level doc comments) so the outlook_client crate enforces documentation on
public items; ensure the attribute is placed before any module or item
definitions so the linter applies to the entire crate.
---
Nitpick comments:
In `@infra/stacks/email-service/index.ts`:
- Around line 94-95: OUTLOOK_CLIENT_STATE is being loaded via config.get and
exposed as a plain environment variable; change this to resolve at runtime from
a secrets-backed source instead of config.get so the webhook clientState is not
exposed in task/env inspection. Replace the plain constant initialization of
OUTLOOK_CLIENT_STATE (and similar vars around the other occurrences) with a
Secrets Manager-backed secret reference and wire it into the task/container
using the platform’s secrets injection mechanism (e.g., ECS TaskDefinition
container secrets or equivalent), updating any code that currently reads
OUTLOOK_CLIENT_STATE to read from the injected secret at runtime.
In `@js/app/packages/app/component/settings/Account.tsx`:
- Around line 667-688: The Gmail and Outlook add buttons differ in visual
treatment; update the Button usages around handleAddInbox and
handleAddOutlookInbox to use a consistent style (either both icon-only or both
label+icon). For example, modify the Button wrapping PlusIcon (used with Tooltip
and onClick=handleAddInbox) and the Button for Outlook
(onClick=handleAddOutlookInbox) so they share the same props (variant, size,
depth, aria-label) and visual content—either render a provider label plus an
icon for both or render icons only (e.g., use PlusIcon for both and
differentiate via Tooltip text and aria-label). Ensure Tooltip labels and
aria-labels remain descriptive and adjust the Outlook Button content to match
the chosen pattern.
In `@rust/cloud-storage/email_service/src/config.rs`:
- Around line 211-217: Treat OUTLOOK_NOTIFICATION_URL and OUTLOOK_CLIENT_STATE
as an all-or-none pair: after reading std::env::var into
outlook_notification_url and outlook_client_state, add a validation that if
exactly one of them is non-empty you return an error (or panic) from the
surrounding config-loading function so the service fails fast on partial Outlook
configuration; if both are empty continue as optional, and if both are present
accept them. Use the variable names outlook_notification_url and
outlook_client_state and the env var names OUTLOOK_NOTIFICATION_URL /
OUTLOOK_CLIENT_STATE to locate where to add the check.
In
`@rust/cloud-storage/email_service/src/pubsub/inbox_sync/operations/outlook_notification.rs`:
- Line 26: The tracing attribute on the function annotated with
#[tracing::instrument(skip(ctx))] (the function that returns Result<(),
ProcessingError>) must include error tracking; update the attribute to add err
(i.e., #[tracing::instrument(skip(ctx), err)]) so returned Err variants are
recorded in structured traces.
In `@rust/cloud-storage/email_service/src/util/outlook/auth.rs`:
- Line 16: The tracing attribute on the function annotated with
#[tracing::instrument(skip(auth_service_client, sqs_client))] should include
error capture; update the attribute to
#[tracing::instrument(skip(auth_service_client, sqs_client), err)] so that any
Err returned from the function is recorded by tracing (keep the same skipped
args and add the err flag on the function that uses auth_service_client and
sqs_client).
🪄 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
Run ID: 68e79e35-7ab1-4a30-84d7-f0da1d750b31
⛔ Files ignored due to path filters (1)
rust/cloud-storage/Cargo.lockis excluded by!**/*.lock,!**/Cargo.lock
📒 Files selected for processing (61)
infra/stacks/email-service/index.tsjs/app/packages/app/component/settings/Account.tsxjs/app/packages/queries/auth/index.tsjs/app/packages/queries/auth/outlook-link.tsjs/app/packages/service-clients/service-auth/client.tsrust/cloud-storage/Cargo.tomlrust/cloud-storage/authentication_service/src/api/internal/microsoft_access_token.rsrust/cloud-storage/authentication_service/src/api/internal/mod.rsrust/cloud-storage/authentication_service/src/api/link/mod.rsrust/cloud-storage/authentication_service/src/api/link/outlook.rsrust/cloud-storage/authentication_service/src/api/oauth2/microsoft.rsrust/cloud-storage/authentication_service/src/api/oauth2/mod.rsrust/cloud-storage/authentication_service/src/config.rsrust/cloud-storage/authentication_service/src/main.rsrust/cloud-storage/authentication_service_client/src/lib.rsrust/cloud-storage/authentication_service_client/src/microsoft_access_token.rsrust/cloud-storage/email/src/domain/models/link.rsrust/cloud-storage/email/src/outbound/email_pg_repo/db_types.rsrust/cloud-storage/email/src/outbound/email_pg_repo/link.rsrust/cloud-storage/email_db_client/src/links/types.rsrust/cloud-storage/email_service/Cargo.tomlrust/cloud-storage/email_service/src/api/context.rsrust/cloud-storage/email_service/src/api/mod.rsrust/cloud-storage/email_service/src/api/outlook/mod.rsrust/cloud-storage/email_service/src/api/outlook/webhook.rsrust/cloud-storage/email_service/src/bin/pubsub_workers/pubsub_workers.rsrust/cloud-storage/email_service/src/config.rsrust/cloud-storage/email_service/src/main.rsrust/cloud-storage/email_service/src/pubsub/backfill/worker.rsrust/cloud-storage/email_service/src/pubsub/context.rsrust/cloud-storage/email_service/src/pubsub/inbox_sync/operations/mod.rsrust/cloud-storage/email_service/src/pubsub/inbox_sync/operations/outlook_notification.rsrust/cloud-storage/email_service/src/pubsub/inbox_sync/process.rsrust/cloud-storage/email_service/src/pubsub/inbox_sync/worker.rsrust/cloud-storage/email_service/src/util/mod.rsrust/cloud-storage/email_service/src/util/outlook/auth.rsrust/cloud-storage/email_service/src/util/outlook/mod.rsrust/cloud-storage/fusionauth/src/lib.rsrust/cloud-storage/fusionauth/src/microsoft/mod.rsrust/cloud-storage/fusionauth/src/microsoft/oauth.rsrust/cloud-storage/macro_db_client/migrations/20260609170618_add_outlook_user_provider.sqlrust/cloud-storage/model/src/authentication/microsoft_token.rsrust/cloud-storage/model/src/authentication/mod.rsrust/cloud-storage/models_email/src/email/api/link.rsrust/cloud-storage/models_email/src/email/service/link.rsrust/cloud-storage/models_email/src/gmail/inbox_sync.rsrust/cloud-storage/models_email/src/lib.rsrust/cloud-storage/models_email/src/outlook/delta.rsrust/cloud-storage/models_email/src/outlook/error.rsrust/cloud-storage/models_email/src/outlook/mod.rsrust/cloud-storage/models_email/src/outlook/subscription.rsrust/cloud-storage/outlook_client/Cargo.tomlrust/cloud-storage/outlook_client/src/attachments.rsrust/cloud-storage/outlook_client/src/convert.rsrust/cloud-storage/outlook_client/src/delta.rsrust/cloud-storage/outlook_client/src/folders.rsrust/cloud-storage/outlook_client/src/lib.rsrust/cloud-storage/outlook_client/src/messages.rsrust/cloud-storage/outlook_client/src/profile.rsrust/cloud-storage/outlook_client/src/subscriptions.rsrust/cloud-storage/outlook_client/src/threads.rs
Comment on lines
+18
to
+25
| #[derive(serde::Deserialize, Debug)] | ||
| pub struct MicrosoftAccessTokenParams { | ||
| fusionauth_user_id: String, | ||
| /// The linked Microsoft account's email — what FusionAuth stores as | ||
| /// `display_name` on the IdP link. Discriminates one Microsoft account from | ||
| /// another when the FA user has multiple Microsoft IdP links. | ||
| email: String, | ||
| } |
Contributor
There was a problem hiding this comment.
Stop tracing raw query params with user identifiers/email.
Current span instrumentation captures params (and Debug-formats it), which can leak PII in traces/log pipelines.
Suggested fix
-#[derive(serde::Deserialize, Debug)]
+#[derive(serde::Deserialize)]
pub struct MicrosoftAccessTokenParams {
@@
-#[tracing::instrument(skip(auth_client, _internal_access))]
+#[tracing::instrument(skip(auth_client, _internal_access, params))]
pub async fn handler(
@@
-#[tracing::instrument(skip(auth_client))]
+#[tracing::instrument(skip(auth_client, params))]
async fn get_access_token(Also applies to: 29-45
🤖 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
`@rust/cloud-storage/authentication_service/src/api/internal/microsoft_access_token.rs`
around lines 18 - 25, The span is currently capturing and Debug-formatting
MicrosoftAccessTokenParams (which contains PII) exposing user identifiers/email;
remove the Debug derive from MicrosoftAccessTokenParams and change the span
instrumentation (the code that records `params` around lines 29-45) to avoid
logging the whole struct — either log only non-PII indicators (e.g., a boolean
like `has_email_link` or `fusionauth_user_id_present`), log a redacted field
(e.g., email => "<redacted>"), or implement a custom Debug/formatter that
redacts sensitive fields before passing to the span. Keep serde::Deserialize on
MicrosoftAccessTokenParams so deserialization still works but ensure any
trace/log call uses the redacted or minimal data instead of the raw params.
Comment on lines
+32
to
+43
| let access_token = crate::util::outlook::auth::fetch_token_or_delete_on_revocation( | ||
| link, | ||
| &ctx.auth_service_client, | ||
| &ctx.sqs_client, | ||
| ) | ||
| .await | ||
| .map_err(|e| { | ||
| ProcessingError::NonRetryable(DetailedError { | ||
| reason: FailureReason::AccessTokenFetchFailed, | ||
| source: e.context("Failed to fetch Outlook access token"), | ||
| }) | ||
| })?; |
Contributor
There was a problem hiding this comment.
Do not mark all Outlook token-fetch failures as non-retryable.
Line 38-43 currently converts every token-fetch error into NonRetryable. That drops notifications on transient auth-service failures instead of retrying.
Suggested direction
- .map_err(|e| {
- ProcessingError::NonRetryable(DetailedError {
- reason: FailureReason::AccessTokenFetchFailed,
- source: e.context("Failed to fetch Outlook access token"),
- })
- })?;
+ .map_err(|e| {
+ // Keep revoked-access failures non-retryable, but retry transient failures.
+ // (Expose `is_forbidden_error` as `pub(crate)` or return a typed error from auth util.)
+ if crate::util::outlook::auth::is_forbidden_error(&e) {
+ ProcessingError::NonRetryable(DetailedError {
+ reason: FailureReason::AccessTokenFetchFailed,
+ source: e.context("Failed to fetch Outlook access token"),
+ })
+ } else {
+ ProcessingError::Retryable(DetailedError {
+ reason: FailureReason::AccessTokenFetchFailed,
+ source: e.context("Failed to fetch Outlook access token"),
+ })
+ }
+ })?;🤖 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
`@rust/cloud-storage/email_service/src/pubsub/inbox_sync/operations/outlook_notification.rs`
around lines 32 - 43, The current map_err on fetch_token_or_delete_on_revocation
labels every failure as ProcessingError::NonRetryable which prevents retries on
transient auth-service outages; change the error mapping for
crate::util::outlook::auth::fetch_token_or_delete_on_revocation to inspect the
returned error and convert transient/auth-service failures into
ProcessingError::Retryable(DetailedError { reason:
FailureReason::AccessTokenFetchFailed, source: ... }) while only mapping genuine
token-revocation or irrecoverable errors to ProcessingError::NonRetryable;
implement this by matching the error (or using its is_transient()/classification
method) in the closure that currently builds DetailedError so
fetch_token_or_delete_on_revocation -> .await.map_err(|e| match e { transient =>
ProcessingError::Retryable(...), permanent => ProcessingError::NonRetryable(...)
})
Comment on lines
+101
to
+134
| // Fetch + convert each changed message. This exercises the full client → | ||
| // convert path; persistence is the next increment. | ||
| for message_id in &changes.message_ids_to_upsert { | ||
| let Some(resource) = ctx | ||
| .outlook_client | ||
| .get_message(&access_token, message_id) | ||
| .await | ||
| .map_err(|e| { | ||
| ProcessingError::Retryable(DetailedError { | ||
| reason: FailureReason::GmailApiFailed, | ||
| source: anyhow::Error::new(e).context("Failed to fetch Outlook message"), | ||
| }) | ||
| })? | ||
| else { | ||
| continue; | ||
| }; | ||
|
|
||
| let folder_label = resource | ||
| .parent_folder_id | ||
| .as_deref() | ||
| .and_then(|fid| folder_labels.get(fid).copied()); | ||
|
|
||
| let _message = | ||
| outlook_client::convert::map_message_resource_to_service(resource, link.id, folder_label); | ||
|
|
||
| // TODO(outlook): persist `_message` via a provider-agnostic inbound | ||
| // upsert (extracted from `operations::upsert_message`), then notify | ||
| // search and run the CRM/notification fan-out as the Gmail path does. | ||
| tracing::debug!( | ||
| link_id = %link.id, | ||
| provider_message_id = %message_id, | ||
| "converted outlook message (persistence pending)" | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Delta results are computed but never applied to system state.
The handler fetches changes, converts messages, and returns Ok(()), but it neither persists upserts nor processes message_ids_to_delete. This acknowledges notifications without updating inbox state.
🤖 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
`@rust/cloud-storage/email_service/src/pubsub/inbox_sync/operations/outlook_notification.rs`
around lines 101 - 134, The loop converts messages but never persists or applies
deletes; update the handler to (1) persist each converted message by calling the
provider-agnostic upsert used by the Gmail path (e.g. the existing
operations::upsert_message or the inbound upsert extracted for provider-agnostic
use) using ctx and link.id, await it and map errors into ProcessingError like
the existing get_message handling, and (2) iterate over
changes.message_ids_to_delete and call the corresponding delete/remove operation
(e.g. operations::delete_message or the unified inbound delete) to remove
messages from storage and trigger downstream work; after each successful
upsert/delete, trigger the same search notification and CRM/notification fan-out
the Gmail path uses so state and side-effects match (use link.id, the converted
message from map_message_resource_to_service, and ctx for client/DB access).
Comment on lines
+4
to
+8
| #[derive(serde::Serialize, serde::Deserialize, Debug)] | ||
| pub struct MicrosoftAccessToken { | ||
| /// The user's Microsoft Graph access token. | ||
| pub access_token: String, | ||
| } |
Contributor
There was a problem hiding this comment.
Remove Debug from the bearer-token model.
MicrosoftAccessToken contains a live access token; deriving Debug makes accidental secret leakage to logs far easier.
Suggested fix
-#[derive(serde::Serialize, serde::Deserialize, Debug)]
+#[derive(serde::Serialize, serde::Deserialize)]
pub struct MicrosoftAccessToken {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[derive(serde::Serialize, serde::Deserialize, Debug)] | |
| pub struct MicrosoftAccessToken { | |
| /// The user's Microsoft Graph access token. | |
| pub access_token: String, | |
| } | |
| #[derive(serde::Serialize, serde::Deserialize)] | |
| pub struct MicrosoftAccessToken { | |
| /// The user's Microsoft Graph access token. | |
| pub access_token: String, | |
| } |
🤖 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 `@rust/cloud-storage/model/src/authentication/microsoft_token.rs` around lines
4 - 8, The MicrosoftAccessToken struct currently derives Debug which risks
leaking bearer tokens; remove Debug from the derive list on pub struct
MicrosoftAccessToken so it only derives serde::Serialize and serde::Deserialize,
and search for any explicit uses of MicrosoftAccessToken::fmt or Debug
formatting (e.g., format!("{:?}", token) or log statements) and replace them
with safe/redacted logging if needed.
| @@ -0,0 +1,317 @@ | |||
| //! A thin client over the [Microsoft Graph] mail API, used to integrate Outlook | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add crate-level missing-docs enforcement for this new crate.
Line 1 starts a new crate lib.rs, but it is missing #![deny(missing_docs)].
🛠️ Proposed fix
+#![deny(missing_docs)]
//! A thin client over the [Microsoft Graph] mail API, used to integrate Outlook
//! inboxes into the email service.As per coding guidelines, "New crates must have #![deny(missing_docs)] in lib.rs."
🤖 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 `@rust/cloud-storage/outlook_client/src/lib.rs` at line 1, The crate root
(lib.rs) is missing the crate-level attribute to enforce documentation; add the
line #![deny(missing_docs)] at the top of lib.rs (before the module/doc
comments) so the compiler denies missing documentation for this new crate and
ensures all public items are documented.
Source: Coding guidelines
| /// * `client_state` — a secret used to authenticate incoming notifications. | ||
| pub fn new(notification_url: String, client_state: String) -> Self { | ||
| Self { | ||
| inner: reqwest::Client::new(), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify timeout coverage in the new client crate.
rg -n 'reqwest::Client::new|Client::builder|timeout\(' rust/cloud-storage/outlook_client/srcRepository: macro-inc/macro
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section around the reported line
nl -ba rust/cloud-storage/outlook_client/src/lib.rs | sed -n '40,120p'
# Search within the crate for any timeout configuration
rg -n '\.timeout\(|timeout\(' rust/cloud-storage/outlook_client/srcRepository: macro-inc/macro
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect around the reported line (use cat -n since nl isn't available)
cat -n rust/cloud-storage/outlook_client/src/lib.rs | sed -n '40,120p'
# Search within the crate for any timeout configuration
rg -n '\.timeout\(|timeout\(' rust/cloud-storage/outlook_client/srcRepository: macro-inc/macro
Length of output: 3480
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Confirm no client/request-level timeouts exist in this crate
rg -n 'Client::new|Client::builder|tokio::time::timeout|\.timeout\(|\btimeout\b' rust/cloud-storage/outlook_client/src || true
# Show imports to see whether Duration is already used
cat -n rust/cloud-storage/outlook_client/src/lib.rs | sed -n '1,40p'Repository: macro-inc/macro
Length of output: 2215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether Duration is already used/imported anywhere in the crate
rg -n '\bDuration\b' rust/cloud-storage/outlook_client/src || true
# Show a larger import section to place the Duration import correctly (if needed)
cat -n rust/cloud-storage/outlook_client/src/lib.rs | sed -n '35,80p'Repository: macro-inc/macro
Length of output: 2286
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and show the reqwest dependency version for outlook_client
cat rust/cloud-storage/outlook_client/Cargo.tomlRepository: macro-inc/macro
Length of output: 600
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether callers wrap these requests in tokio::time::timeout
rg -n 'graph_get(_opt)?\(|tokio::time::timeout' rust/cloud-storage/outlook_client
# Check reqwest request timeout patterns in this crate
rg -n '\.timeout\(' rust/cloud-storage/outlook_client || trueRepository: macro-inc/macro
Length of output: 806
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether callers wrap these requests in tokio::time::timeout
rg -n 'tokio::time::timeout' rust/cloud-storage/outlook_client || true
# Check reqwest request timeout patterns in this crate
rg -n '\.timeout\(' rust/cloud-storage/outlook_client || trueRepository: macro-inc/macro
Length of output: 41
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the workspace reqwest dependency definition (version/type)
rg -n 'reqwest\s*=\s*\{' -S Cargo.toml rust -g'Cargo.toml'
rg -n 'reqwest\s*=' -S Cargo.toml rust -g'Cargo.toml' | head -n 50Repository: macro-inc/macro
Length of output: 2887
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the workspace Cargo.toml that defines the reqwest version
ls -la rust/cloud-storage | head
cat rust/cloud-storage/Cargo.toml | sed -n '120,190p'
# Extract the exact reqwest workspace dependency block
rg -n "reqwest\s*=\s*\{" rust/cloud-storage/Cargo.tomlRepository: macro-inc/macro
Length of output: 639
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the reqwest workspace dependency definition in rust/cloud-storage/Cargo.toml
rg -n "reqwest\s*=" rust/cloud-storage/Cargo.toml
# Print the relevant portion around the reqwest entry (replace range if needed)
sed -n '120,190p' rust/cloud-storage/Cargo.tomlRepository: macro-inc/macro
Length of output: 2251
Set an explicit request timeout on the shared Microsoft Graph HTTP client
OutlookClient::new creates reqwest::Client::new() with no request timeout, so a slow/stalled Graph call can keep awaiting tasks much longer than intended.
🛠️ Proposed fix
use serde::de::DeserializeOwned;
+use std::time::Duration;
@@
pub fn new(notification_url: String, client_state: String) -> Self {
Self {
- inner: reqwest::Client::new(),
+ inner: reqwest::Client::builder()
+ .timeout(Duration::from_secs(30))
+ .build()
+ .expect("failed to build OutlookClient HTTP client"),
base_url: DEFAULT_GRAPH_BASE_URL.to_string(),
notification_url,
client_state,
}
}🤖 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 `@rust/cloud-storage/outlook_client/src/lib.rs` at line 65, OutlookClient::new
currently uses reqwest::Client::new() with no timeout; update the constructor to
build the shared HTTP client with an explicit request timeout (e.g., using
reqwest::Client::builder().timeout(Duration::from_secs(30)) or a configurable
value) instead of Client::new(), so slow/stalled Graph calls will time out;
modify the place that sets inner (the field initialized from
reqwest::Client::new()) to use the builder and ensure you import
std::time::Duration and handle any builder errors if needed.
Comment on lines
+21
to
+26
| let url = format!( | ||
| "{}/me/messages/{}?$select={}", | ||
| client.base_url, | ||
| message_id, | ||
| MESSAGE_SELECT | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Microsoft Graph docs: when message/folder/subscription IDs are used in request URL paths, are they opaque identifiers that must be URL-encoded as path segments?
💡 Result:
In Microsoft Graph, identifiers such as message, folder, and subscription IDs are treated as segments of the URL path [1][2]. While these identifiers are often opaque and may contain characters (such as "/" or "+") that are not standard URL-safe characters, they must be properly percent-encoded when used as part of a request URL path to ensure the request is valid and unambiguous [3][4]. Microsoft Graph expects URLs to conform to RFC 3986 [3][4]. Because path segments are delimited by forward slashes (/), any identifier that contains characters outside the allowed set for a URL path—or that contains special characters that might be interpreted as segment delimiters—must be percent-encoded [3][4]. Key considerations for identifier encoding include: - Standard Percent Encoding: You should use standard percent encoding (e.g., replacing a space with %20 or a slash with %2F) for any characters that are not permitted in a URL path segment [3][4]. - Segment-Specific Encoding: You cannot simply encode an entire URL string in one operation because encoding rules differ by component [3][4]. You must ensure that each identifier used as a path segment is encoded individually before being appended to the URL [3][4]. - Case Sensitivity: While many parts of a URL (like resource names) are case-insensitive, resource IDs—including base64-encoded values—are case-sensitive and must be used exactly as they are returned by the API [1][2]. If you are working with path-based addressing (such as in OneDrive), extra care is required because user-provided content in the path may contain unsafe characters, making encoding mandatory for all segments [3][4]. Most modern development frameworks provide utility methods (e.g., URL encoding functions) that handle these conversions; it is recommended to use these to avoid common manual encoding errors [3][4].
Citations:
- 1: https://learn.microsoft.com/en-us/graph/traverse-the-graph
- 2: https://learn.microsoft.com/en-us/graph/call-api
- 3: https://learn.microsoft.com/en-us/graph/onedrive-addressing-driveitems
- 4: https://learn.microsoft.com/en-us/onedrive/developer/rest-api/concepts/addressing-driveitems?view=odsp-graph-online
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="rust/cloud-storage/outlook_client/src/messages.rs"
ls -l "$FILE"
sed -n '1,220p' "$FILE" | nl -ba | sed -n '1,220p'Repository: macro-inc/macro
Length of output: 219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="rust/cloud-storage/outlook_client/src/messages.rs"
awk '{print NR "\t" $0}' "$FILE" | sed -n '1,240p'Repository: macro-inc/macro
Length of output: 7117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "graph_get_opt|base_url" rust/cloud-storage/outlook_client/src -SRepository: macro-inc/macro
Length of output: 2531
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' rust/cloud-storage/outlook_client/src/lib.rs | awk '{print NR "\t" $0}' | sed -n '1,220p'Repository: macro-inc/macro
Length of output: 9180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "Url::parse|path_segments_mut|percent_encode|urlencode|form_urlencoded|encode_uri|query_pairs_mut" rust/cloud-storage/outlook_client/src -SRepository: macro-inc/macro
Length of output: 41
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "reqwest::Url|Url::parse|path_segments_mut|query_pairs_mut|urlencoding::|percent_encode|encode_component|form_urlencoded" rust/cloud-storage -SRepository: macro-inc/macro
Length of output: 13222
🌐 Web query:
Microsoft Graph mail message id used in URL path must be URL-encoded
💡 Result:
Yes, a Microsoft Graph mail message ID must be URL-encoded when used in a URL path if it contains reserved characters, most notably the forward slash (/) [1]. Because the message ID is part of the URL path structure, any unencoded reserved character can be misinterpreted by the API as a path segment delimiter, resulting in a 400 Bad Request or a "Resource not found" error [2][3][1]. Key considerations for handling message IDs: 1. Requirement for URL Encoding: If your message ID contains a slash (/), it must be percent-encoded (e.g., / becomes %2F) before being included in the request URL [1]. Microsoft Graph expects URLs to conform to RFC 3986 [4]. 2. Distinguish ID Types: Ensure you are using the actual Microsoft Graph message ID (returned by Microsoft Graph API calls like GET /messages) [2]. Attempting to use legacy EWS ItemIDs or other identifiers directly in the path will fail regardless of encoding, as Graph will not recognize them [2]. 3. Best Practices for Construction: Do not attempt to encode an entire URL at once, as the encoding rules differ between URL segments and query parameters [4]. Instead, build your URL programmatically using standard library tools like UriBuilder (for.NET) or equivalent functions in your language (e.g., percent-encoding the path segment specifically) to ensure only the necessary components are escaped [4]. 4. Case Sensitivity: While many parts of the Graph API URL are case-insensitive, resource IDs and other base64-encoded values are case-sensitive [5]. Ensure your encoding process maintains the integrity of the ID string [5]. If you continue to experience errors despite proper encoding, verify that the ID being used was retrieved directly from a Microsoft Graph API endpoint, as Graph IDs sometimes differ from those used in other Microsoft systems [2].
Citations:
- 1: https://learn.microsoft.com/en-us/answers/questions/1495697/how-to-get-message-with-microsoft-graph-api
- 2: https://learn.microsoft.com/en-us/answers/questions/5876774/failing-to-fetch-email-mime-content-for-itemid-usi
- 3: https://learn.microsoft.com/en-my/answers/questions/814458/(needhelp)-get-message-api-fails-when-the-resource
- 4: https://github.com/microsoftgraph/microsoft-graph-docs-contrib/blob/main/concepts/onedrive-addressing-driveitems.md
- 5: https://learn.microsoft.com/en-us/graph/call-api
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "get_message\\(|send_message\\(" rust/cloud-storage/outlook_client -SRepository: macro-inc/macro
Length of output: 660
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "\\.get_message\\(|get_message\\(" rust/cloud-storage -SRepository: macro-inc/macro
Length of output: 1056
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Outlook notification handler call site
sed -n '70,140p' rust/cloud-storage/email_service/src/pubsub/inbox_sync/operations/outlook_notification.rs | awk '{print NR "\t" $0}'
# Inspect upsert_message usage for provider ids
sed -n '1,120p' rust/cloud-storage/email_service/src/pubsub/inbox_sync/operations/upsert_message.rs | awk '{print NR "\t" $0}'Repository: macro-inc/macro
Length of output: 8146
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "message_ids_to_upsert|message_ids_to_delete|`@odata`\\.deltaLink|mailFolders|messages\\b|\"id\"\\s*:" rust/cloud-storage/outlook_client/src -SRepository: macro-inc/macro
Length of output: 4179
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find OutlookError definition
rg -n "enum OutlookError|struct OutlookError|OutlookError::HttpRequest" -S rust/cloud-storage | head -n 50Repository: macro-inc/macro
Length of output: 1117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' rust/cloud-storage/models_email/src/outlook/error.rs | awk '{print NR "\t" $0}'Repository: macro-inc/macro
Length of output: 2458
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' rust/cloud-storage/outlook_client/src/delta.rs | awk '{print NR "\t" $0}'
sed -n '1,120p' rust/cloud-storage/outlook_client/src/threads.rs | awk '{print NR "\t" $0}'
sed -n '1,120p' rust/cloud-storage/outlook_client/src/attachments.rs | awk '{print NR "\t" $0}'
sed -n '1,140p' rust/cloud-storage/outlook_client/src/subscriptions.rs | awk '{print NR "\t" $0}'Repository: macro-inc/macro
Length of output: 8864
Percent-encode Microsoft Graph IDs when used as URL path segments
In rust/cloud-storage/outlook_client/src/messages.rs, get_message interpolates message_id into /me/messages/{id} (lines 21-26), and send_message interpolates parent_id/draft.id into /me/messages/{id}/createReply (lines 118-119), /me/messages/{id} (line 132), and /me/messages/{id}/send (lines 157-158). Graph IDs are opaque and can contain reserved URI characters (e.g. /), so raw interpolation can change the intended path segment and cause “not found”/400 errors. The same raw path-segment interpolation pattern also appears in attachments.rs (message/attachment ids), delta.rs (folder id), and subscriptions.rs (subscription id).
🛠️ Proposed fix pattern (apply to all ID-based URLs in this module)
- let url = format!(
- "{}/me/messages/{}?$select={}",
- client.base_url,
- message_id,
- MESSAGE_SELECT
- );
- client.graph_get_opt(access_token, &url).await
+ let mut url = reqwest::Url::parse(&format!("{}/me/messages", client.base_url))
+ .map_err(|e| OutlookError::HttpRequest(e.to_string()))?;
+ url.path_segments_mut()
+ .map_err(|_| OutlookError::HttpRequest("invalid Graph base URL".to_string()))?
+ .push(message_id);
+ url.query_pairs_mut().append_pair("$select", MESSAGE_SELECT);
+ client.graph_get_opt(access_token, url.as_str()).await🤖 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 `@rust/cloud-storage/outlook_client/src/messages.rs` around lines 21 - 26, The
code currently interpolates opaque Graph IDs directly into path segments (e.g.,
in messages.rs functions get_message and send_message using message_id,
parent_id, draft.id), which can contain reserved characters like '/' and must be
percent-encoded; update all ID-based URL path segment usages (messages.rs,
attachments.rs, delta.rs, subscriptions.rs) to percent-encode the IDs before
formatting the URL (only for path segments, not query params) and use a proper
percent-encoding routine (e.g., utf8 percent-encode with a path-segment safe
encode set) so URLs like "/me/messages/{encoded_id}" and
"/me/messages/{encoded_id}/send" are built with encoded IDs.
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
3 participants
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.
Summary
This PR adds comprehensive support for linking and syncing Outlook/Microsoft 365 email accounts alongside the existing Gmail integration. It implements a complete Microsoft Graph client with OAuth authentication, incremental delta-sync, webhook notifications, and provider-agnostic service-layer mappings.
Key Changes
New Crates & Modules
outlook_client: New crate providing a thin HTTP client over Microsoft Graph mail API, mirroring thegmail_clientinterfacemessages.rs)folders.rs)delta.rs)subscriptions.rs)attachments.rs)profile.rs)convert.rs)models_email::outlook: New module with Microsoft Graph resource modelsGmailErrorAuthentication & OAuth
fusionauth::microsoft: Microsoft OAuth token refresh implementationauthentication_service:api/link/outlook.rs)api/oauth2/microsoft.rs)api/internal/microsoft_access_token.rs)authentication_service_client: Client method for fetching Microsoft access tokensEmail Service Integration
email_service/api/outlook/webhook.rs): Validates Graph subscription handshakes and enqueues delta-sync jobsemail_service/pubsub/inbox_sync/operations/outlook_notification.rs): Processes Outlook change notifications via delta queriesemail_service/util/outlook/auth.rs): Fetches tokens and handles revocationDatabase & Models
OUTLOOKuser provider enum variant across:models_email::email::service::link::UserProvidermodels_email::email::api::link::UserProvideremail_db_clientandemaildomain modelsOUTLOOKprovider supportMicrosoftAccessTokenmodel for token responsesConfiguration & Infrastructure
outlook_notification_urlandoutlook_client_statefor webhook setupmicrosoft_client_idandmicrosoft_client_secretoutlook_clientmemberFrontend
js/app/packages/queries/auth/outlook-link.ts)Notable Implementation Details
outlook_client::convertmaps Graph resources to service-layer structs, enabling uniform handling across Gmail and Outlook@odata.deltaLink) as the Outlook analogue to Gmail'shistoryIdclientStateverification)Optionand#[serde(default)]to accommodate variable$selectclausesThe implementation mirrors the Gmail integration pattern throughout, ensuring consistency and maintainability.
https://claude.ai/code/session_01PQLDbFK5RBnAtuZqtdqX98