From 4fc5016f8fd2abb37fa482c282bc11d24979d66a Mon Sep 17 00:00:00 2001 From: sudie-codes Date: Thu, 9 Apr 2026 20:08:49 -0700 Subject: [PATCH] fix(msteams): fetch OneDrive/SharePoint shared media via Graph shares endpoint (#55383) (#63942) * fix(msteams): fetch OneDrive/SharePoint media via Graph shares endpoint (#55383) * fix(msteams): rewrite shared links before allowlist check * test(msteams): fix typed fetch call assertions --------- Co-authored-by: Brad Groux --- extensions/msteams/src/attachments.test.ts | 100 ++++++++++++++++++ .../msteams/src/attachments/download.ts | 16 ++- extensions/msteams/src/attachments/graph.ts | 11 +- .../msteams/src/attachments/shared.test.ts | 72 +++++++++++++ extensions/msteams/src/attachments/shared.ts | 61 +++++++++++ 5 files changed, 254 insertions(+), 6 deletions(-) diff --git a/extensions/msteams/src/attachments.test.ts b/extensions/msteams/src/attachments.test.ts index 67fd81c400a..2a855fa3fa1 100644 --- a/extensions/msteams/src/attachments.test.ts +++ b/extensions/msteams/src/attachments.test.ts @@ -550,5 +550,105 @@ describe("msteams attachments", () => { expectAttachmentMediaLength(media, 0); expect(fetchMock).toHaveBeenCalledTimes(1); }); + + describe("OneDrive/SharePoint shared links", () => { + const GRAPH_SHARES_URL_PREFIX = `https://${GRAPH_HOST}/v1.0/shares/`; + const DEFAULT_GRAPH_ALLOW_HOSTS = [GRAPH_HOST]; + const PDF_PAYLOAD = Buffer.from("pdf-bytes"); + + const createGraphSharesFetchMock = () => + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input.toString(); + const auth = new Headers(init?.headers).get("Authorization"); + if (url.startsWith(GRAPH_SHARES_URL_PREFIX)) { + if (!auth) { + return createTextResponse("unauthorized", 401); + } + return createBufferResponse(PDF_PAYLOAD, CONTENT_TYPE_APPLICATION_PDF); + } + return createNotFoundResponse(); + }); + + it.each([ + { + label: "SharePoint URL", + contentUrl: "https://contoso.sharepoint.com/personal/user/Documents/report.pdf", + }, + { + label: "OneDrive 1drv.ms URL", + contentUrl: "https://1drv.ms/b/s!AkxYabcdefg", + }, + { + label: "OneDrive onedrive.live.com URL", + contentUrl: "https://onedrive.live.com/share/file", + }, + ])("routes $label through Graph shares endpoint", async ({ contentUrl }) => { + const tokenProvider = createTokenProvider(); + const fetchMock = createGraphSharesFetchMock(); + detectMimeMock.mockResolvedValueOnce(CONTENT_TYPE_APPLICATION_PDF); + saveMediaBufferMock.mockResolvedValueOnce({ + id: "saved.pdf", + path: SAVED_PDF_PATH, + size: Buffer.byteLength(PDF_PAYLOAD), + contentType: CONTENT_TYPE_APPLICATION_PDF, + }); + + const media = await downloadMSTeamsAttachments( + buildDownloadParams( + [ + { + contentType: "reference", + contentUrl, + name: "report.pdf", + }, + ], + { + tokenProvider, + allowHosts: DEFAULT_GRAPH_ALLOW_HOSTS, + authAllowHosts: DEFAULT_GRAPH_ALLOW_HOSTS, + fetchFn: asFetchFn(fetchMock), + }, + ), + ); + + expectAttachmentMediaLength(media, 1); + expect(media[0]?.path).toBe(SAVED_PDF_PATH); + // The only host that should be fetched is graph.microsoft.com. + const calledUrls = fetchMock.mock.calls.map(([input]) => + typeof input === "string" ? input : String(input), + ); + expect(calledUrls.length).toBeGreaterThan(0); + for (const url of calledUrls) { + expect(url.startsWith(GRAPH_SHARES_URL_PREFIX)).toBe(true); + } + // Graph scope token was acquired for the shares fetch. + expect(tokenProvider.getAccessToken).toHaveBeenCalled(); + }); + + it("falls through to direct fetch for non-shared-link URLs", async () => { + const directUrl = createTestUrl("direct.pdf"); + const fetchMock = createOkFetchMock(CONTENT_TYPE_APPLICATION_PDF, "pdf"); + detectMimeMock.mockResolvedValueOnce(CONTENT_TYPE_APPLICATION_PDF); + saveMediaBufferMock.mockResolvedValueOnce({ + id: "saved.pdf", + path: SAVED_PDF_PATH, + size: Buffer.byteLength(PDF_BUFFER), + contentType: CONTENT_TYPE_APPLICATION_PDF, + }); + + const media = await downloadAttachmentsWithFetch( + createPdfAttachments(directUrl), + fetchMock, + ); + + expectAttachmentMediaLength(media, 1); + const calledUrls = fetchMock.mock.calls.map(([input]) => + typeof input === "string" ? input : String(input), + ); + // Should have hit the original host, NOT graph shares. + expect(calledUrls.some((url) => url === directUrl)).toBe(true); + expect(calledUrls.some((url) => url.startsWith(GRAPH_SHARES_URL_PREFIX))).toBe(false); + }); + }); }); }); diff --git a/extensions/msteams/src/attachments/download.ts b/extensions/msteams/src/attachments/download.ts index 4a599dd6c46..66da1887f1a 100644 --- a/extensions/msteams/src/attachments/download.ts +++ b/extensions/msteams/src/attachments/download.ts @@ -16,6 +16,7 @@ import { resolveAttachmentFetchPolicy, resolveRequestUrl, safeFetchWithPolicy, + tryBuildGraphSharesUrlForSharedLink, } from "./shared.js"; import type { MSTeamsAccessTokenProvider, @@ -65,10 +66,21 @@ function resolveDownloadCandidate(att: MSTeamsAttachmentLike): DownloadCandidate return null; } + // OneDrive/SharePoint shared links (delivered in 1:1 DMs when the user + // picks "Attach > OneDrive") cannot be fetched directly — the URL returns + // an HTML landing page rather than the file bytes. Rewrite them to the + // Graph shares endpoint so the auth fallback attaches a Graph-scoped token + // and the response is the real file content. + const sharesUrl = tryBuildGraphSharesUrlForSharedLink(contentUrl); + const resolvedUrl = sharesUrl ?? contentUrl; + // Graph shares returns raw bytes without a declared content type we can + // trust for routing — let the downloader infer MIME from the buffer. + const resolvedContentTypeHint = sharesUrl ? undefined : contentType; + return { - url: contentUrl, + url: resolvedUrl, fileHint: name || undefined, - contentTypeHint: contentType, + contentTypeHint: resolvedContentTypeHint, placeholder: inferPlaceholder({ contentType, fileName: name }), }; } diff --git a/extensions/msteams/src/attachments/graph.ts b/extensions/msteams/src/attachments/graph.ts index ec3275b5fb8..add3c1fad99 100644 --- a/extensions/msteams/src/attachments/graph.ts +++ b/extensions/msteams/src/attachments/graph.ts @@ -10,6 +10,7 @@ import { downloadMSTeamsAttachments } from "./download.js"; import { downloadAndStoreMSTeamsRemoteMedia } from "./remote-media.js"; import { applyAuthorizationHeaderForUrl, + encodeGraphShareId, GRAPH_ROOT, estimateBase64DecodedBytes, inferPlaceholder, @@ -322,13 +323,15 @@ export async function downloadMSTeamsGraphMedia(params: { const name = att.name ?? "file"; try { - // SharePoint URLs need to be accessed via Graph shares API + // SharePoint URLs need to be accessed via Graph shares API. Validate the + // rewritten Graph URL, not the original SharePoint host, so the existing + // Graph allowlist path can fetch shared files without separately allowing + // arbitrary SharePoint hosts. const shareUrl = att.contentUrl!; - if (!isUrlAllowed(shareUrl, policy.allowHosts)) { + const sharesUrl = `${GRAPH_ROOT}/shares/${encodeGraphShareId(shareUrl)}/driveItem/content`; + if (!isUrlAllowed(sharesUrl, policy.allowHosts)) { continue; } - const encodedUrl = Buffer.from(shareUrl).toString("base64url"); - const sharesUrl = `${GRAPH_ROOT}/shares/u!${encodedUrl}/driveItem/content`; const media = await downloadAndStoreMSTeamsRemoteMedia({ url: sharesUrl, diff --git a/extensions/msteams/src/attachments/shared.test.ts b/extensions/msteams/src/attachments/shared.test.ts index e541d88f005..7e83b7d983f 100644 --- a/extensions/msteams/src/attachments/shared.test.ts +++ b/extensions/msteams/src/attachments/shared.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it, vi } from "vitest"; import { applyAuthorizationHeaderForUrl, + encodeGraphShareId, extractInlineImageCandidates, + isGraphSharedLinkUrl, isPrivateOrReservedIP, isUrlAllowed, resolveAndValidateIP, @@ -11,6 +13,7 @@ import { resolveMediaSsrfPolicy, safeFetch, safeFetchWithPolicy, + tryBuildGraphSharesUrlForSharedLink, } from "./shared.js"; const publicResolve = async () => ({ address: "13.107.136.10" }); @@ -395,6 +398,75 @@ describe("attachment fetch auth helpers", () => { }); }); +describe("Graph shared-link helpers", () => { + it.each([ + ["https://contoso.sharepoint.com/personal/user/Documents/report.pdf", true], + ["https://contoso.sharepoint.us/sites/team/file.docx", true], + ["https://contoso.sharepoint.cn/file", true], + ["https://tenant-my.sharepoint.com/:b:/g/personal/file", true], + ["https://1drv.ms/b/s!AkxYabc", true], + ["https://onedrive.live.com/view.aspx?resid=ABC", true], + ["https://onedrive.com/share/abc", true], + ["https://graph.microsoft.com/v1.0/me", false], + ["https://smba.trafficmanager.net/amer/v3", false], + ["https://example.com/file.pdf", false], + ["not-a-url", false], + ])("isGraphSharedLinkUrl(%s) === %s", (url, expected) => { + expect(isGraphSharedLinkUrl(url)).toBe(expected); + }); + + it("encodeGraphShareId uses u! + base64url without padding", () => { + // Graph docs example: encoding "https://onedrive.live.com/redir?resid=..." + // should yield u!aHR0cHM6... (base64url, no '+', '/', or trailing '='). + const url = "https://contoso.sharepoint.com/sites/a/Shared Documents/file.pdf"; + const shareId = encodeGraphShareId(url); + expect(shareId.startsWith("u!")).toBe(true); + const encoded = shareId.slice(2); + // base64url alphabet is A-Z, a-z, 0-9, '-', '_' (no padding). + expect(encoded).toMatch(/^[A-Za-z0-9_-]+$/); + // Round-trip check: decoding yields the original URL. + const decoded = Buffer.from(encoded, "base64url").toString("utf8"); + expect(decoded).toBe(url); + }); + + it("encodeGraphShareId swaps '+' and '/' for '-' and '_'", () => { + // A URL whose standard base64 contains '+' and '/' chars. + // Choose an input that base64 encodes with those characters. + const url = "https://host.sharepoint.com/sites/path?x=???"; + const shareId = encodeGraphShareId(url); + const encoded = shareId.slice(2); + expect(encoded).not.toContain("+"); + expect(encoded).not.toContain("/"); + expect(encoded).not.toContain("="); + }); + + it("tryBuildGraphSharesUrlForSharedLink rewrites SharePoint URLs", () => { + const url = "https://contoso.sharepoint.com/personal/user/Documents/report.pdf"; + const result = tryBuildGraphSharesUrlForSharedLink(url); + expect(result).toBeDefined(); + expect(result).toMatch( + /^https:\/\/graph\.microsoft\.com\/v1\.0\/shares\/u![A-Za-z0-9_-]+\/driveItem\/content$/, + ); + }); + + it("tryBuildGraphSharesUrlForSharedLink rewrites OneDrive URLs", () => { + const url = "https://1drv.ms/b/s!AkxYabcdefg"; + const result = tryBuildGraphSharesUrlForSharedLink(url); + expect(result).toBeDefined(); + expect(result).toMatch( + /^https:\/\/graph\.microsoft\.com\/v1\.0\/shares\/u![A-Za-z0-9_-]+\/driveItem\/content$/, + ); + }); + + it("tryBuildGraphSharesUrlForSharedLink returns undefined for non-shared URLs", () => { + expect( + tryBuildGraphSharesUrlForSharedLink("https://graph.microsoft.com/v1.0/me"), + ).toBeUndefined(); + expect(tryBuildGraphSharesUrlForSharedLink("https://example.com/file.pdf")).toBeUndefined(); + expect(tryBuildGraphSharesUrlForSharedLink("not-a-url")).toBeUndefined(); + }); +}); + describe("msteams inline image limits", () => { const smallPngDataUrl = "data:image/png;base64,aGVsbG8="; // "hello" (5 bytes) diff --git a/extensions/msteams/src/attachments/shared.ts b/extensions/msteams/src/attachments/shared.ts index a40699717a5..1724dae4a8c 100644 --- a/extensions/msteams/src/attachments/shared.ts +++ b/extensions/msteams/src/attachments/shared.ts @@ -84,6 +84,67 @@ export const DEFAULT_MEDIA_AUTH_HOST_ALLOWLIST = [ export const GRAPH_ROOT = "https://graph.microsoft.com/v1.0"; export { isRecord }; +/** + * Host suffixes for SharePoint/OneDrive shared links that must be fetched via + * the Graph `/shares/{shareId}/driveItem/content` endpoint instead of directly. + * + * Direct fetches of SharePoint/OneDrive shared URLs return empty/HTML landing + * pages unless encoded as a Graph share id. See + * https://learn.microsoft.com/en-us/graph/api/shares-get for the encoding. + */ +const GRAPH_SHARED_LINK_HOST_SUFFIXES = [ + ".sharepoint.com", + ".sharepoint.us", + ".sharepoint.de", + ".sharepoint.cn", + ".sharepoint-df.com", + "1drv.ms", + "onedrive.live.com", + "onedrive.com", +] as const; + +/** + * Returns true when the URL points at a SharePoint or OneDrive host whose + * shared-link content must be fetched through the Graph shares API rather + * than directly. + */ +export function isGraphSharedLinkUrl(url: string): boolean { + let host: string; + try { + host = normalizeLowercaseStringOrEmpty(new URL(url).hostname); + } catch { + return false; + } + if (!host) { + return false; + } + return GRAPH_SHARED_LINK_HOST_SUFFIXES.some((suffix) => host === suffix || host.endsWith(suffix)); +} + +/** + * Encode a SharePoint/OneDrive URL as a Graph shareId using the documented + * `u!` + base64url (no padding) scheme: + * https://learn.microsoft.com/en-us/graph/api/shares-get#encoding-sharing-urls + */ +export function encodeGraphShareId(url: string): string { + // Buffer.from(...).toString("base64url") already returns base64url without + // padding, matching the Graph spec exactly. + return `u!${Buffer.from(url, "utf8").toString("base64url")}`; +} + +/** + * When `url` is a SharePoint/OneDrive shared link, return the matching + * `GET /shares/{shareId}/driveItem/content` URL that actually yields the file + * bytes. Returns `undefined` for non-shared-link URLs so callers can fall + * through to the existing fetch path. + */ +export function tryBuildGraphSharesUrlForSharedLink(url: string): string | undefined { + if (!isGraphSharedLinkUrl(url)) { + return undefined; + } + return `${GRAPH_ROOT}/shares/${encodeGraphShareId(url)}/driveItem/content`; +} + export function readNestedString(value: unknown, keys: Array): string | undefined { let current: unknown = value; for (const key of keys) {