fix(slack): handle file redirects
Co-authored-by: Glucksberg <markuscontasul@gmail.com>
This commit is contained in:
Peter Steinberger
278
src/slack/monitor/media.test.ts
Normal file
278
src/slack/monitor/media.test.ts
Normal file
@@ -0,0 +1,278 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
// Store original fetch
|
||||
const originalFetch = globalThis.fetch;
|
||||
let mockFetch: ReturnType<typeof vi.fn>;
|
||||
|
||||
describe("fetchWithSlackAuth", () => {
|
||||
beforeEach(() => {
|
||||
// Create a new mock for each test
|
||||
mockFetch = vi.fn();
|
||||
globalThis.fetch = mockFetch as typeof fetch;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Restore original fetch
|
||||
globalThis.fetch = originalFetch;
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
it("sends Authorization header on initial request with manual redirect", async () => {
|
||||
// Import after mocking fetch
|
||||
const { fetchWithSlackAuth } = await import("./media.js");
|
||||
|
||||
// Simulate direct 200 response (no redirect)
|
||||
const mockResponse = new Response(Buffer.from("image data"), {
|
||||
status: 200,
|
||||
headers: { "content-type": "image/jpeg" },
|
||||
});
|
||||
mockFetch.mockResolvedValueOnce(mockResponse);
|
||||
|
||||
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
|
||||
|
||||
expect(result).toBe(mockResponse);
|
||||
|
||||
// Verify fetch was called with correct params
|
||||
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||
expect(mockFetch).toHaveBeenCalledWith("https://files.slack.com/test.jpg", {
|
||||
headers: { Authorization: "Bearer xoxb-test-token" },
|
||||
redirect: "manual",
|
||||
});
|
||||
});
|
||||
|
||||
it("follows redirects without Authorization header", async () => {
|
||||
const { fetchWithSlackAuth } = await import("./media.js");
|
||||
|
||||
// First call: redirect response from Slack
|
||||
const redirectResponse = new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: "https://cdn.slack-edge.com/presigned-url?sig=abc123" },
|
||||
});
|
||||
|
||||
// Second call: actual file content from CDN
|
||||
const fileResponse = new Response(Buffer.from("actual image data"), {
|
||||
status: 200,
|
||||
headers: { "content-type": "image/jpeg" },
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse);
|
||||
|
||||
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
|
||||
|
||||
expect(result).toBe(fileResponse);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(2);
|
||||
|
||||
// First call should have Authorization header and manual redirect
|
||||
expect(mockFetch).toHaveBeenNthCalledWith(1, "https://files.slack.com/test.jpg", {
|
||||
headers: { Authorization: "Bearer xoxb-test-token" },
|
||||
redirect: "manual",
|
||||
});
|
||||
|
||||
// Second call should follow the redirect without Authorization
|
||||
expect(mockFetch).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
"https://cdn.slack-edge.com/presigned-url?sig=abc123",
|
||||
{ redirect: "follow" },
|
||||
);
|
||||
});
|
||||
|
||||
it("handles relative redirect URLs", async () => {
|
||||
const { fetchWithSlackAuth } = await import("./media.js");
|
||||
|
||||
// Redirect with relative URL
|
||||
const redirectResponse = new Response(null, {
|
||||
status: 302,
|
||||
headers: { location: "/files/redirect-target" },
|
||||
});
|
||||
|
||||
const fileResponse = new Response(Buffer.from("image data"), {
|
||||
status: 200,
|
||||
headers: { "content-type": "image/jpeg" },
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse);
|
||||
|
||||
await fetchWithSlackAuth("https://files.slack.com/original.jpg", "xoxb-test-token");
|
||||
|
||||
// Second call should resolve the relative URL against the original
|
||||
expect(mockFetch).toHaveBeenNthCalledWith(2, "https://files.slack.com/files/redirect-target", {
|
||||
redirect: "follow",
|
||||
});
|
||||
});
|
||||
|
||||
it("returns redirect response when no location header is provided", async () => {
|
||||
const { fetchWithSlackAuth } = await import("./media.js");
|
||||
|
||||
// Redirect without location header
|
||||
const redirectResponse = new Response(null, {
|
||||
status: 302,
|
||||
// No location header
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(redirectResponse);
|
||||
|
||||
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
|
||||
|
||||
// Should return the redirect response directly
|
||||
expect(result).toBe(redirectResponse);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("returns 4xx/5xx responses directly without following", async () => {
|
||||
const { fetchWithSlackAuth } = await import("./media.js");
|
||||
|
||||
const errorResponse = new Response("Not Found", {
|
||||
status: 404,
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(errorResponse);
|
||||
|
||||
const result = await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
|
||||
|
||||
expect(result).toBe(errorResponse);
|
||||
expect(mockFetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("handles 301 permanent redirects", async () => {
|
||||
const { fetchWithSlackAuth } = await import("./media.js");
|
||||
|
||||
const redirectResponse = new Response(null, {
|
||||
status: 301,
|
||||
headers: { location: "https://cdn.slack.com/new-url" },
|
||||
});
|
||||
|
||||
const fileResponse = new Response(Buffer.from("image data"), {
|
||||
status: 200,
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(redirectResponse).mockResolvedValueOnce(fileResponse);
|
||||
|
||||
await fetchWithSlackAuth("https://files.slack.com/test.jpg", "xoxb-test-token");
|
||||
|
||||
expect(mockFetch).toHaveBeenCalledTimes(2);
|
||||
expect(mockFetch).toHaveBeenNthCalledWith(2, "https://cdn.slack.com/new-url", {
|
||||
redirect: "follow",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveSlackMedia", () => {
|
||||
beforeEach(() => {
|
||||
mockFetch = vi.fn();
|
||||
globalThis.fetch = mockFetch as typeof fetch;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
globalThis.fetch = originalFetch;
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
it("prefers url_private_download over url_private", async () => {
|
||||
// Mock the store module
|
||||
vi.doMock("../../media/store.js", () => ({
|
||||
saveMediaBuffer: vi.fn().mockResolvedValue({
|
||||
path: "/tmp/test.jpg",
|
||||
contentType: "image/jpeg",
|
||||
}),
|
||||
}));
|
||||
|
||||
const { resolveSlackMedia } = await import("./media.js");
|
||||
|
||||
const mockResponse = new Response(Buffer.from("image data"), {
|
||||
status: 200,
|
||||
headers: { "content-type": "image/jpeg" },
|
||||
});
|
||||
mockFetch.mockResolvedValueOnce(mockResponse);
|
||||
|
||||
await resolveSlackMedia({
|
||||
files: [
|
||||
{
|
||||
url_private: "https://files.slack.com/private.jpg",
|
||||
url_private_download: "https://files.slack.com/download.jpg",
|
||||
name: "test.jpg",
|
||||
},
|
||||
],
|
||||
token: "xoxb-test-token",
|
||||
maxBytes: 1024 * 1024,
|
||||
});
|
||||
|
||||
expect(mockFetch).toHaveBeenCalledWith(
|
||||
"https://files.slack.com/download.jpg",
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns null when download fails", async () => {
|
||||
const { resolveSlackMedia } = await import("./media.js");
|
||||
|
||||
// Simulate a network error
|
||||
mockFetch.mockRejectedValueOnce(new Error("Network error"));
|
||||
|
||||
const result = await resolveSlackMedia({
|
||||
files: [{ url_private: "https://files.slack.com/test.jpg", name: "test.jpg" }],
|
||||
token: "xoxb-test-token",
|
||||
maxBytes: 1024 * 1024,
|
||||
});
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when no files are provided", async () => {
|
||||
const { resolveSlackMedia } = await import("./media.js");
|
||||
|
||||
const result = await resolveSlackMedia({
|
||||
files: [],
|
||||
token: "xoxb-test-token",
|
||||
maxBytes: 1024 * 1024,
|
||||
});
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
it("skips files without url_private", async () => {
|
||||
const { resolveSlackMedia } = await import("./media.js");
|
||||
|
||||
const result = await resolveSlackMedia({
|
||||
files: [{ name: "test.jpg" }], // No url_private
|
||||
token: "xoxb-test-token",
|
||||
maxBytes: 1024 * 1024,
|
||||
});
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(mockFetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("falls through to next file when first file returns error", async () => {
|
||||
// Mock the store module
|
||||
vi.doMock("../../media/store.js", () => ({
|
||||
saveMediaBuffer: vi.fn().mockResolvedValue({
|
||||
path: "/tmp/test.jpg",
|
||||
contentType: "image/jpeg",
|
||||
}),
|
||||
}));
|
||||
|
||||
const { resolveSlackMedia } = await import("./media.js");
|
||||
|
||||
// First file: 404
|
||||
const errorResponse = new Response("Not Found", { status: 404 });
|
||||
// Second file: success
|
||||
const successResponse = new Response(Buffer.from("image data"), {
|
||||
status: 200,
|
||||
headers: { "content-type": "image/jpeg" },
|
||||
});
|
||||
|
||||
mockFetch.mockResolvedValueOnce(errorResponse).mockResolvedValueOnce(successResponse);
|
||||
|
||||
const result = await resolveSlackMedia({
|
||||
files: [
|
||||
{ url_private: "https://files.slack.com/first.jpg", name: "first.jpg" },
|
||||
{ url_private: "https://files.slack.com/second.jpg", name: "second.jpg" },
|
||||
],
|
||||
token: "xoxb-test-token",
|
||||
maxBytes: 1024 * 1024,
|
||||
});
|
||||
|
||||
expect(result).not.toBeNull();
|
||||
expect(mockFetch).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
@@ -5,6 +5,38 @@ import { fetchRemoteMedia } from "../../media/fetch.js";
|
||||
import { saveMediaBuffer } from "../../media/store.js";
|
||||
import type { SlackFile } from "../types.js";
|
||||
|
||||
/**
|
||||
* Fetches a URL with Authorization header, handling cross-origin redirects.
|
||||
* Node.js fetch strips Authorization headers on cross-origin redirects for security.
|
||||
* Slack's files.slack.com URLs redirect to CDN domains with pre-signed URLs that
|
||||
* don't need the Authorization header, so we handle the initial auth request manually.
|
||||
*/
|
||||
export async function fetchWithSlackAuth(url: string, token: string): Promise<Response> {
|
||||
// Initial request with auth and manual redirect handling
|
||||
const initialRes = await fetch(url, {
|
||||
headers: { Authorization: `Bearer ${token}` },
|
||||
redirect: "manual",
|
||||
});
|
||||
|
||||
// If not a redirect, return the response directly
|
||||
if (initialRes.status < 300 || initialRes.status >= 400) {
|
||||
return initialRes;
|
||||
}
|
||||
|
||||
// Handle redirect - the redirected URL should be pre-signed and not need auth
|
||||
const redirectUrl = initialRes.headers.get("location");
|
||||
if (!redirectUrl) {
|
||||
return initialRes;
|
||||
}
|
||||
|
||||
// Resolve relative URLs against the original
|
||||
const resolvedUrl = new URL(redirectUrl, url).toString();
|
||||
|
||||
// Follow the redirect without the Authorization header
|
||||
// (Slack's CDN URLs are pre-signed and don't need it)
|
||||
return fetch(resolvedUrl, { redirect: "follow" });
|
||||
}
|
||||
|
||||
export async function resolveSlackMedia(params: {
|
||||
files?: SlackFile[];
|
||||
token: string;
|
||||
@@ -19,10 +51,12 @@ export async function resolveSlackMedia(params: {
|
||||
const url = file.url_private_download ?? file.url_private;
|
||||
if (!url) continue;
|
||||
try {
|
||||
const fetchImpl: FetchLike = (input, init) => {
|
||||
const headers = new Headers(init?.headers);
|
||||
headers.set("Authorization", `Bearer ${params.token}`);
|
||||
return fetch(input, { ...init, headers });
|
||||
// Note: We ignore init options because fetchWithSlackAuth handles
|
||||
// redirect behavior specially. fetchRemoteMedia only passes the URL.
|
||||
const fetchImpl: FetchLike = (input) => {
|
||||
const inputUrl =
|
||||
typeof input === "string" ? input : input instanceof URL ? input.href : input.url;
|
||||
return fetchWithSlackAuth(inputUrl, params.token);
|
||||
};
|
||||
const fetched = await fetchRemoteMedia({
|
||||
url,
|
||||
|
||||
Reference in New Issue
Block a user