Skip to content

feat(apps): resolve Agentic User file bytes through Graph /shares - #594

Open
Corina (corinagum) wants to merge 17 commits into
mainfrom
cg/files-graph-receive
Open

feat(apps): resolve Agentic User file bytes through Graph /shares#594
Corina (corinagum) wants to merge 17 commits into
mainfrom
cg/files-graph-receive

Conversation

@corinagum

Copy link
Copy Markdown
Contributor

What this does

Agentic User file attachments arrive with a contentUrl and no downloadUrl, so the pre-authorized ("tempauth") URL traditional bots rely on is absent. This resolves those bytes through Microsoft Graph, GET /shares/{u!encoded}/driveItem/content, using the Agentic User's own credential.

The route is chosen from the shape of the attachment, not from the identity: a contentUrl with no downloadUrl, at personal scope, resolves via graphShare. Everything else keeps the existing preauthUrl path unchanged.

What it deliberately does not do

An expired pre-authorized URL is terminal. It raises FileUrlExpiredError and makes no Graph call.

A developer can still call Graph with their own client, and keeps the locator because contentUrl is public API. The constraint is on what the SDK does automatically.

Auditing trap: the agentic token is itself delegated-shaped, carrying scp rather than roles. Follow the identity, not the word "delegated".

Why .default and not a named scope

A by-name request hard-fails on exact-string consent mismatch: a blueprint consented to Files.ReadWrite.All returns AADSTS65001 and no token at all for a Files.Read.All request. Graph's own /shares docs list Files.ReadWrite.All as least-privileged and list no read-only permission, so admins following Microsoft's guidance land exactly where a by-name request breaks.

The cost is a late, ambiguous 403 when a file permission is missing. We pay that in the SDK instead: the pre-flight rejects a token carrying permissions but none file-capable, which restores an early, legible failure. A Files.Read.All-then-.default ladder was rejected, because AADSTS65001 is also what "no file permission consented at all" produces, so the ladder makes the most common deployment error harder to diagnose, not easier.

For security review: under .default the token carries the blueprint's full consented set, including unrelated scopes. The mitigation is the grant, not the request string: do not consent unrelated scopes on a file-handling blueprint.

Testing

Unit and integration coverage is in the diff. All three scenarios were also run live against real Teams, tenant asserted twice per run (browser session state, and the SharePoint host in the bot's own log).

Scenario Result
Traditional bot, fresh file preauthUrl, byte-exact
Agentic User via Graph /shares/{u!}/driveItem/content 200, byte-exact, driving the shipped helpers
Expired URL FileUrlExpiredError, zero requests to graph.microsoft.com in the entire process log

The third row was checked by grepping the whole bot process log for an absence, not by reading the rendered result, and the expiry is real rather than simulated: the same UniqueId returns 200 early and a hard 401 later. And the pre-authorized URL lifetime is not a constant: three runs bounded the flip at (62.7, 72.4], (62.5, 68.4] and (63.6, 66.4] minutes. Poll rather than picking a wait time.


This language

9 commits, 25 files, +1650 / -49.

Follow storage redirects on the pre-authorized download path closes a real divergence rather than introducing one. httpx does not follow redirects by default, unlike the TS and .NET transports, so a storage 302 surfaced as a bare RuntimeError instead of bytes. Pinned by unit tests and transport parity; no storage 302 occurred during live testing, so it has no live confirmation.

Copilot AI lite review requested due to automatic review settings September 10, 2026 21:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical and moderate review findings remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds Microsoft Graph /shares retrieval for content-URL-only Agentic User attachments while preserving pre-authorized URL downloads.

Changes:

  • Adds Graph URL construction, credential selection, cloud routing, and typed errors.
  • Supports redirects and preserves expired-URL behavior.
  • Adds tests and updates API/example documentation.
File summaries
File Summary
packages/apps/tests/test_optional_graph_dependencies.py Tests Graph host derivation and optional dependencies.
packages/apps/tests/test_graph_share.py Tests share URL encoding and endpoint construction.
packages/apps/tests/test_files_errors.py Tests file retrieval error guidance.
packages/apps/tests/test_files_download_graph.py Tests Graph downloads, redirects, credentials, and failures.
packages/apps/tests/test_files_credential.py Tests credential selection.
packages/apps/tests/test_files_accessor.py Tests cleanup, compatibility, and pre-authorized downloads.
packages/apps/tests/test_files_accessor_agentic.py Tests Agentic attachment mapping and scope behavior.
packages/apps/tests/test_app.py Tests Agentic User token acquisition.
packages/apps/tests/test_app_process.py Tests credential propagation.
packages/apps/tests/test_app_oauth.py Updates processor construction tests.
packages/apps/src/microsoft_teams/apps/utils/graph.py Derives Graph host URLs.
packages/apps/src/microsoft_teams/apps/utils/__init__.py Exports Graph URL helpers.
packages/apps/src/microsoft_teams/apps/routing/activity_context.py Passes file credentials into contexts.
packages/apps/src/microsoft_teams/apps/files/incoming_file.py Carries content URLs and credentials.
packages/apps/src/microsoft_teams/apps/files/graph_share.py Builds Graph sharing endpoints.
packages/apps/src/microsoft_teams/apps/files/files_accessor.py Maps content-URL-only attachments.
packages/apps/src/microsoft_teams/apps/files/errors.py Defines retrieval errors and guidance.
packages/apps/src/microsoft_teams/apps/files/download.py Implements Graph and pre-authorized downloads.
packages/apps/src/microsoft_teams/apps/files/__init__.py Exports file error types.
packages/apps/src/microsoft_teams/apps/files_credential.py Selects file credentials.
packages/apps/src/microsoft_teams/apps/app.py Acquires Agentic User Graph tokens.
packages/apps/src/microsoft_teams/apps/app_process.py Wires credentials into activity contexts.
packages/api/src/microsoft_teams/api/models/file/file_download_info.py Clarifies file identifier semantics.
examples/ai-file-analysis/src/file_card.py Clarifies file source labeling.
examples/ai-file-analysis/README.md Documents retrieval routes and expiry behavior.
Review details

Suppressed comments (8)

examples/ai-file-analysis/README.md:84

  • The implementation only admits content-URL-only attachments in personal scope; groupChat and channel entries are skipped. This sentence says every missing download_url follows Graph, which overpromises support for the other scopes. State the personal-scope limitation here.
The route is determined by whether or not the pre-authorized URL exists. If it does, it is used. Otherwise, the Graph path is followed.

packages/apps/src/microsoft_teams/apps/app_process.py:172

  • The agentic token callback receives only activity.recipient.agentic_identity, so it ignores the tenant_id already extracted from the conversation/channel data. If an inbound identity omits tenantId (the field is optional), TokenManager.get_agentic_user_token has no default tenant and raises, or a configured single-tenant fallback can select the wrong directory. Pass the activity tenant as the fallback for agentic token acquisition, as the app-token callback does here.
            get_agentic_graph_token=self.get_agentic_graph_token,

packages/apps/src/microsoft_teams/apps/files/download.py:210

  • The Graph path has the same downgrade issue: a trusted HTTPS Graph endpoint can return an HTTP Location, which follow_redirects=True will follow and use to transfer the file over cleartext. Apply the same HTTPS-only redirect policy here as on the pre-authorized path.
        response = await http.send(request, stream=True, follow_redirects=True)

packages/apps/src/microsoft_teams/apps/files/download.py:114

  • This second new collections.abc.AsyncGenerator[OpenedFileStream] annotation has the same import-time typing error: the send type is missing. Use AsyncGenerator[OpenedFileStream, None] here as well.
) -> AsyncGenerator[OpenedFileStream]:

packages/apps/src/microsoft_teams/apps/files/download.py:185

  • This third new collections.abc.AsyncGenerator[OpenedFileStream] annotation likewise omits the required send type and can prevent the module from importing. Use AsyncGenerator[OpenedFileStream, None].
) -> AsyncGenerator[OpenedFileStream]:

packages/apps/src/microsoft_teams/apps/files/errors.py:19

  • This public reason contract says a missing drive item is covered by access_denied because Graph returns 403, but the implementation maps only 401/403 and the added missing-drive scenario returns RuntimeError for 404. Either map 404 consistently or document the current behavior so callers are not given an inaccurate exception contract.
- `access_denied`: the identity used was refused by the storage service. Covers an unconsented scope, a file the
  identity was never granted, and a drive item that does not exist, which are indistinguishable on the wire: Graph
  answers all three with `403`, because telling an unauthorized caller whether a resource exists would disclose it.

packages/apps/src/microsoft_teams/apps/files/errors.py:124

  • When token acquisition raises, download.py passes that exception string as details for no_graph_credential, but this constructor labels every detail as (service said: ...). Graph was never called in that branch, so AAD/provider failures are misattributed to Graph and make the diagnostic misleading.
        if details:
            message = f"{message} (service said: {details})"

packages/apps/tests/test_files_accessor.py:350

  • This ordinary in-package dependency is imported inside the test body, unlike the module-level imports used by this test and the source package. Move IncomingFile into the existing import block so import failures are detected during collection and imports remain consistent.
        from microsoft_teams.apps.files import IncomingFile
  • Files reviewed: 25/25 changed files
  • Comments generated: 7
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/apps/src/microsoft_teams/apps/files/download.py
Comment thread packages/apps/src/microsoft_teams/apps/files/download.py Outdated
Comment thread packages/apps/src/microsoft_teams/apps/files/incoming_file.py
Comment thread packages/apps/src/microsoft_teams/apps/files/download.py Outdated
Comment thread packages/apps/src/microsoft_teams/apps/files/download.py Outdated
Comment thread examples/ai-file-analysis/README.md Outdated
Comment thread packages/apps/src/microsoft_teams/apps/files/incoming_file.py
Comment thread examples/ai-file-analysis/README.md Outdated
Comment thread examples/ai-file-analysis/README.md Outdated
Comment thread packages/apps/src/microsoft_teams/apps/files/download.py Outdated
Comment thread packages/apps/src/microsoft_teams/apps/files/errors.py Outdated
Comment thread packages/apps/src/microsoft_teams/apps/files/errors.py Outdated
Comment thread packages/apps/src/microsoft_teams/apps/app_process.py Outdated
Comment thread packages/apps/src/microsoft_teams/apps/files/download.py Outdated
Port of the TypeScript change. FileRetrievalError and its reason discriminant are replaced by FileCredentialError and FileAccessError, the activity tenant is threaded through to the Graph token request, redirects are followed manually so a non-https hop is refused, and the actor is no longer defaulted.

The manual redirect walk is needed because the client follows redirects internally, which would otherwise carry the Authorization header across a downgrade before anything could inspect it.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7d326e8a-aeb4-4481-bc0a-58d2a012584e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants