Skip to content

Share chat attachments with the chat's members after upload - #93

Merged
sionsmith merged 3 commits into
osodevops:mainfrom
philipwilsonTHG:chat-attach-share
Sep 6, 2026
Merged

Share chat attachments with the chat's members after upload#93
sionsmith merged 3 commits into
osodevops:mainfrom
philipwilsonTHG:chat-attach-share

Conversation

@philipwilsonTHG

@philipwilsonTHG philipwilsonTHG commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Problem

teams message send --chat … --attach FILE uploads the file into the sender's OneDrive (Microsoft Teams Chat Files) and links the driveItem from the message, but nobody else in the chat can open it: the Teams client grants every chat member read permission when it attaches a file, and the CLI did not. Recipients saw "you don't have permission".

Fix

After each chat upload the CLI now lists the chat's members once and grants them read access through the driveItem's invite action (roles: ["read"], requireSignIn: true, sendInvitation: false — no notification email). Members are addressed by Entra object ID where the membership carries one (covers accounts without a mail attribute), else by email; the sender and members with neither are skipped, and duplicates collapse.

The grant is best-effort: if it fails (for example a token that cannot list chat members) the upload and message still go through and a warning on stderr says to share the file from OneDrive by hand. Channel attachments are unchanged — they live in the team's SharePoint library, which channel members already read.

Changes

  • AttachDestination::Chat carries the chat id so the media layer can look the members up.
  • New endpoints::me_drive_item_invite, DriveInviteRequest / DriveRecipient / DrivePermission models, files::grant_read_access.
  • Unit tests for recipient selection and the invite request's JSON shape.
  • Docs: command reference, attachments spec (new step 3 under Graph mechanics), CHANGELOG.

Verified

Live in a four-member chat: the debug log reports the file shared with 3 members and Graph lists a read permission for each of them, with the sender as owner.

🤖 Generated with Claude Code

philipwilsonTHG and others added 3 commits September 6, 2026 07:10
`message send --chat … --attach FILE` uploads into the sender's OneDrive
("Microsoft Teams Chat Files") and links the driveItem from the message, but
nobody else could open it: the Teams client grants every chat member read
permission when it attaches a file, and the CLI did not. Recipients saw
"you don't have permission" (observed live, 2026-09-03).

After each chat upload the CLI now lists the chat's members once and grants
them read access through the driveItem's `invite` action, with
requireSignIn=true and no notification email. Members are addressed by Entra
object ID where the membership carries one (covers accounts without a mail
attribute), else by email; the sender and members with neither are skipped,
and duplicates collapse. The grant is best-effort: if it fails the upload and
message still go through and a warning on stderr says to share the file from
OneDrive by hand. Channel attachments are unchanged — they live in the team's
SharePoint library, which channel members already read.

`AttachDestination::Chat` now carries the chat id so the media layer can look
the members up. New: `endpoints::me_drive_item_invite`, `DriveInviteRequest`
/ `DriveRecipient` / `DrivePermission` models, `files::grant_read_access`.
Unit tests cover recipient selection and the invite request's JSON shape;
docs and CHANGELOG updated.

Verified live in a four-member meeting chat: the debug log reports the file
shared with 3 members and Graph lists a read permission for each.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The best-effort follow-up shortened the entry to the mechanism and lost
the symptom it fixes. Keep the symptom, the invite mechanics, the tenant
rule and the best-effort behaviour in one entry.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WG7vFLjFZHqAUMRS1zkWRE
@sionsmith

Copy link
Copy Markdown
Contributor

Reviewed and verified locally:

  • fmt, clippy (-D warnings) and the full suite pass; the invite request's JSON shape and recipient selection are pinned by tests.
  • Microsoft Learn agrees with the approach: driveItem: invite takes driveRecipients by objectId or email, sendInvitation: false suppresses the mail, and the same Files.ReadWrite scope the upload needs covers it. It also states that a chat's membership "may include users from different tenants", which matters below. I could not exercise it live: the sandbox token has no Files.ReadWrite.

Two things the original diff did not do that its description promised, now fixed on the branch via @aberoham's follow-up commit:

  1. Best-effort really is best-effort. chat_recipients(...).await? propagated a failed /me or member lookup and aborted before the upload, so a token that could send but not list members lost the whole message rather than just the sharing step. Lookup failures now warn and continue, with mocked tests for a failing /me, a failing members call, and a failing second page.
  2. Object IDs only mean something in the sender's directory. A member from another tenant was addressed by an object ID the sender's OneDrive cannot resolve. The roster's tenantId now gates that: same tenant → object ID, otherwise email, and members with neither are counted in a stderr warning.

Also restored the changelog entry's account of the symptom (the follow-up had shortened it), and rebased onto main so the changelog, README, man page and tests merge with #90, #91 and #92. Merging once CI is green.

@sionsmith
sionsmith merged commit 88978ce into osodevops:main Sep 6, 2026
6 checks passed
@sionsmith

Copy link
Copy Markdown
Contributor

@philipwilsonTHG a few questions so we can close the verification gap on our side (our sandbox token has no Files.ReadWrite, and the only self-only chat has no second member to invite):

  1. In the four-member chat you tested against, were any members guests or from another tenant? And did every conversationMember from GET /chats/{id}/members carry both userId and tenantId? The merged version addresses a member by object ID only when the roster's tenantId matches the sender's, and by email otherwise, so I want to be sure that rule does not change behaviour for the setup you validated.
  2. Could you re-run the merged main against that chat and paste the read-back from GET /me/drive/items/{item-id}/permissions (IDs redacted), so we have roles: ["read"] per member on record? The invite response alone does not show whether the grant stuck.
  3. Did the recipients open the attachment from the message card without a permission prompt, and did it appear in the chat's Files tab?
  4. Which scope set did the sending token carry: the defaults plus Files.ReadWrite only? That confirms member lookup works with Chat.ReadWrite and needs no extra scope.

Thanks for the fix, it is in main and will ship in the next release.

@sionsmith sionsmith mentioned this pull request Sep 6, 2026
5 tasks
@philipwilsonTHG

Copy link
Copy Markdown
Contributor Author

Thanks for the review and the two fixes — both are real gaps in what I sent.

Re-ran today against merged main (b4b1fdf, the v0.7.0 release) in the same four-member chat. Build note: our machine keeps tokens in a file rather than the keychain (unattended cron use), so the binary was main plus my not-yet-upstreamed file-token-store patch applied on top; that touches token storage only, none of the attachment or sharing code.

  1. Roster. No guests and no external members — all four are in the sender's tenant. Every conversationMember from GET /chats/{id}/members carried userId, tenantId, email and displayName (plus id, roles, visibleHistoryStartDateTime), so under the merged rule every member goes down the object-ID path — the same path the original validation used. Behaviour unchanged for this setup; I have not been able to test a cross-tenant member.

  2. Read-back. After teams message send --chat … --attach pr93_share_check.txt, the debug log reported Shared 'pr93_share_check.txt' with 3 chat member(s); roles granted: read,read,read, and GET /me/drive/items/{item-id}/permissions returned four entries:

[
  {"id": "<redacted>", "roles": ["read"],  "grantedToV2": {"user": {"displayName": "<member 1>", "id": "<redacted>", "email": "<redacted>"}}},
  {"id": "<redacted>", "roles": ["read"],  "grantedToV2": {"user": {"displayName": "<member 2>", "id": "<redacted>", "email": "<redacted>"}}},
  {"id": "<redacted>", "roles": ["read"],  "grantedToV2": {"user": {"displayName": "<member 3>", "id": "<redacted>", "email": "<redacted>"}}},
  {"id": "<redacted>", "roles": ["owner"], "grantedToV2": {"user": {"displayName": "<sender>",   "id": "<redacted>", "email": "<redacted>"}}}
]

No link on any entry (direct grants, not sharing links). The same shape is on nine earlier attachments sent from the pre-merge branch build, so the grant sticks.

  1. Recipient experience. A recipient confirmed today that the attachments open straight from the message card with no permission prompt, and the files appear in the chat's Files tab. Earlier, before the fix, the same recipients got "you don't have permission" on files sent this way; since the fix nobody has reported that on any of today's attachments.

  2. Scopes. The sending token carried the defaults plus Files.ReadWrite.All (not the narrower Files.ReadWrite): Channel.ReadBasic.All ChannelMessage.Read.All ChannelMessage.Send Chat.Read Chat.ReadBasic Chat.ReadWrite ChatMessage.Read ChatMessage.Send Files.ReadWrite.All OnlineMeetings.Read People.Read Presence.Read.All Presence.ReadWrite Team.ReadBasic.All TeamMember.Read.All User.Read User.ReadBasic.All profile openid email. Member lookup worked with Chat.ReadWrite; nothing extra was consented for it.

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