Skip to content

Commit b4c1f88

Browse files
tommoorclaude
andauthored
feat: Allow document unfurling with shareId (outline#12007)
* feat: Allow document unfurling with shareId * fix: Handle collection shares, share-scoped URLs, and unauthenticated unfurls - Return 204 instead of 404 for collection shares without a document - Use share-scoped URL in unfurl response so hover previews stay within the share context - Add test coverage for unauthenticated share URL unfurling Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * perf: Only resolve team from context for non-UUID share identifiers loadPublicShare only requires teamId when the share identifier is a slug (urlId), not a UUID. Skip the getTeamFromContext DB lookup on the common UUID path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b650a0f commit b4c1f88

File tree

7 files changed

+251
-13
lines changed

7 files changed

+251
-13
lines changed

app/components/HoverPreview/HoverPreviewDocument.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const HoverPreviewDocument = React.forwardRef(function HoverPreviewDocument_(
2626
<ErrorBoundary showTitle={false} reloadOnChunkMissing={false}>
2727
<Flex column gap={2}>
2828
<Title>{title}</Title>
29-
<Info>{lastActivityByViewer}</Info>
29+
{lastActivityByViewer && <Info>{lastActivityByViewer}</Info>}
3030
<Description as="div">
3131
<React.Suspense fallback={<div />}>
3232
<Editor

server/presenters/unfurl.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,14 +92,17 @@ const presentDocument = (
9292
data: Record<string, any>
9393
): UnfurlResponse[UnfurlResourceType.Document] => {
9494
const document: Document = data.document;
95-
const viewer: User = data.viewer;
95+
const viewer: User | undefined = data.viewer;
96+
const url: string | undefined = data.url;
9697
return {
97-
url: document.url,
98+
url: url ?? document.url,
9899
type: UnfurlResourceType.Document,
99100
id: document.id,
100101
title: document.titleWithDefault,
101102
summary: document.getSummary(),
102-
lastActivityByViewer: presentLastActivityInfoFor(document, viewer),
103+
lastActivityByViewer: viewer
104+
? presentLastActivityInfoFor(document, viewer)
105+
: undefined,
103106
};
104107
};
105108

server/routes/api/urls/urls.test.ts

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
import { UnfurlResourceType } from "@shared/types";
22
import env from "@server/env";
33
import type { User } from "@server/models";
4-
import { buildDocument, buildUser } from "@server/test/factories";
4+
import {
5+
buildCollection,
6+
buildDocument,
7+
buildShare,
8+
buildUser,
9+
} from "@server/test/factories";
510
import { getTestServer } from "@server/test/support";
611
import Iframely from "plugins/iframely/server/iframely";
712

@@ -157,6 +162,120 @@ describe("#urls.unfurl", () => {
157162
expect(body.id).toEqual(document.id);
158163
});
159164

165+
it("should succeed with status 200 ok when valid share url is supplied", async () => {
166+
const document = await buildDocument({
167+
teamId: user.teamId,
168+
});
169+
const share = await buildShare({
170+
teamId: user.teamId,
171+
userId: user.id,
172+
documentId: document.id,
173+
published: true,
174+
});
175+
176+
const res = await server.post("/api/urls.unfurl", {
177+
body: {
178+
token: user.getJwtToken(),
179+
url: `${env.URL}/s/${share.id}/doc/${document.urlId}`,
180+
},
181+
});
182+
const body = await res.json();
183+
expect(res.status).toEqual(200);
184+
expect(body.type).toEqual(UnfurlResourceType.Document);
185+
expect(body.title).toEqual(document.titleWithDefault);
186+
expect(body.id).toEqual(document.id);
187+
});
188+
189+
it("should succeed with status 200 ok when share url with urlId is supplied", async () => {
190+
const document = await buildDocument({
191+
teamId: user.teamId,
192+
});
193+
const share = await buildShare({
194+
teamId: user.teamId,
195+
userId: user.id,
196+
documentId: document.id,
197+
urlId: "test-share",
198+
published: true,
199+
});
200+
201+
const res = await server.post("/api/urls.unfurl", {
202+
body: {
203+
token: user.getJwtToken(),
204+
url: `${env.URL}/s/${share.urlId}/doc/${document.urlId}`,
205+
},
206+
});
207+
const body = await res.json();
208+
expect(res.status).toEqual(200);
209+
expect(body.type).toEqual(UnfurlResourceType.Document);
210+
expect(body.title).toEqual(document.titleWithDefault);
211+
expect(body.id).toEqual(document.id);
212+
});
213+
214+
it("should succeed with status 200 ok for share url without authentication", async () => {
215+
const document = await buildDocument({
216+
teamId: user.teamId,
217+
});
218+
const share = await buildShare({
219+
teamId: user.teamId,
220+
userId: user.id,
221+
documentId: document.id,
222+
published: true,
223+
});
224+
225+
const res = await server.post("/api/urls.unfurl", {
226+
body: {
227+
url: `${env.URL}/s/${share.id}/doc/${document.urlId}`,
228+
},
229+
});
230+
const body = await res.json();
231+
expect(res.status).toEqual(200);
232+
expect(body.type).toEqual(UnfurlResourceType.Document);
233+
expect(body.title).toEqual(document.titleWithDefault);
234+
expect(body.id).toEqual(document.id);
235+
});
236+
237+
it("should return share-scoped url in unfurl response", async () => {
238+
const document = await buildDocument({
239+
teamId: user.teamId,
240+
});
241+
const share = await buildShare({
242+
teamId: user.teamId,
243+
userId: user.id,
244+
documentId: document.id,
245+
published: true,
246+
});
247+
248+
const res = await server.post("/api/urls.unfurl", {
249+
body: {
250+
token: user.getJwtToken(),
251+
url: `${env.URL}/s/${share.id}/doc/${document.urlId}`,
252+
},
253+
});
254+
const body = await res.json();
255+
expect(res.status).toEqual(200);
256+
expect(body.url).toContain(`/s/${share.id}/doc/`);
257+
});
258+
259+
it("should return 204 for collection share url without document", async () => {
260+
const collection = await buildCollection({
261+
teamId: user.teamId,
262+
});
263+
const share = await buildShare({
264+
teamId: user.teamId,
265+
userId: user.id,
266+
collectionId: collection.id,
267+
published: true,
268+
});
269+
270+
const res = await server.post("/api/urls.unfurl", {
271+
body: {
272+
token: user.getJwtToken(),
273+
url: `${env.URL}/s/${share.id}`,
274+
},
275+
});
276+
expect(res.status).toEqual(204);
277+
});
278+
160279
it("should succeed with status 200 ok for a valid external url", async () => {
161280
(Iframely.requestResource as jest.Mock).mockResolvedValue(
162281
Promise.resolve({

server/routes/api/urls/urls.ts

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
11
import dns from "node:dns";
22
import Router from "koa-router";
33
import { traceFunction } from "@server/logging/tracing";
4+
import isUUID from "validator/lib/isUUID";
45
import { MentionType, UnfurlResourceType } from "@shared/types";
56
import { getBaseDomain, parseDomain } from "@shared/utils/domains";
67
import parseDocumentSlug from "@shared/utils/parseDocumentSlug";
78
import parseMentionUrl from "@shared/utils/parseMentionUrl";
8-
import { isInternalUrl } from "@shared/utils/urls";
9-
import { NotFoundError, ValidationError } from "@server/errors";
9+
import { isInternalUrl, parseShareIdFromUrl } from "@shared/utils/urls";
10+
import {
11+
AuthenticationError,
12+
NotFoundError,
13+
ValidationError,
14+
} from "@server/errors";
1015
import auth from "@server/middlewares/authentication";
1116
import { rateLimiter } from "@server/middlewares/rateLimiter";
1217
import validate from "@server/middlewares/validate";
1318
import { Document, Share, Team, User, Group, GroupUser } from "@server/models";
1419
import { authorize, can } from "@server/policies";
20+
import { loadPublicShare } from "@server/commands/shareLoader";
1521
import presentUnfurl from "@server/presenters/unfurl";
1622
import type { APIContext, Unfurl } from "@server/types";
1723
import { CacheHelper, type CacheResult } from "@server/utils/CacheHelper";
@@ -22,6 +28,7 @@ import {
2228
checkEmbeddability,
2329
type EmbedCheckResult,
2430
} from "@server/utils/embeds";
31+
import { getTeamFromContext } from "@server/utils/passport";
2532
import * as T from "./schema";
2633
import { MAX_AVATAR_DISPLAY } from "@shared/constants";
2734
import { Day } from "@shared/utils/time";
@@ -32,13 +39,54 @@ const plugins = PluginManager.getHooks(Hook.UnfurlProvider);
3239
router.post(
3340
"urls.unfurl",
3441
rateLimiter(RateLimiterStrategy.OneThousandPerHour),
35-
auth(),
42+
auth({ optional: true }),
3643
validate(T.UrlsUnfurlSchema),
3744
async (ctx: APIContext<T.UrlsUnfurlReq>) => {
3845
const { url, documentId } = ctx.input.body;
39-
const { user: actor } = ctx.state.auth;
4046
const urlObj = new URL(url);
4147

48+
// Public share URLs – does not require authentication
49+
if (isInternalUrl(url)) {
50+
const shareId = parseShareIdFromUrl(url);
51+
52+
if (shareId) {
53+
const actor = ctx.state.auth.user;
54+
// teamId is only needed when the share identifier is a slug, not a UUID
55+
let teamId: string | undefined = actor?.teamId;
56+
if (!teamId && !isUUID(shareId)) {
57+
const teamFromCtx = await getTeamFromContext(ctx, {
58+
includeStateCookie: false,
59+
});
60+
teamId = teamFromCtx?.id;
61+
}
62+
const previewDocumentId = parseDocumentSlug(url);
63+
const { share, document } = await loadPublicShare({
64+
id: shareId,
65+
documentId: previewDocumentId,
66+
teamId,
67+
});
68+
69+
if (!document) {
70+
ctx.response.status = 204;
71+
return;
72+
}
73+
74+
ctx.body = await presentUnfurl({
75+
type: UnfurlResourceType.Document,
76+
document,
77+
viewer: actor,
78+
url: `${share.canonicalUrl}/doc/${document.url.replace("/doc/", "")}`,
79+
});
80+
return;
81+
}
82+
}
83+
84+
// Everything below requires authentication
85+
const { user: actor } = ctx.state.auth;
86+
if (!actor) {
87+
throw AuthenticationError();
88+
}
89+
4290
// Mentions
4391
if (urlObj.protocol === "mention:") {
4492
if (!documentId) {
@@ -113,9 +161,9 @@ router.post(
113161
if (isInternalUrl(url) || parseDomain(url).host === actor.team.domain) {
114162
const previewDocumentId = parseDocumentSlug(url);
115163
if (previewDocumentId) {
116-
const document = previewDocumentId
117-
? await Document.findByPk(previewDocumentId, { userId: actor.id })
118-
: undefined;
164+
const document = await Document.findByPk(previewDocumentId, {
165+
userId: actor.id,
166+
});
119167
if (!document) {
120168
throw NotFoundError("Document does not exist");
121169
}
@@ -128,6 +176,7 @@ router.post(
128176
});
129177
return;
130178
}
179+
131180
ctx.response.status = 204;
132181
return;
133182
}

shared/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ export type UnfurlResponse = {
591591
/** Document summary */
592592
summary: string;
593593
/** Viewer's last activity on this document */
594-
lastActivityByViewer: string;
594+
lastActivityByViewer?: string;
595595
};
596596
[UnfurlResourceType.Issue]: {
597597
/** The resource type */

shared/utils/urls.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,40 @@ describe("sanitizeUrl", () => {
190190
});
191191
});
192192

193+
describe("parseShareIdFromUrl", () => {
194+
it("should return share id from url with doc path", () => {
195+
expect(
196+
urlsUtils.parseShareIdFromUrl(
197+
"https://app.example.com/s/my-share/doc/test-abc123"
198+
)
199+
).toBe("my-share");
200+
});
201+
202+
it("should return share uuid from url", () => {
203+
expect(
204+
urlsUtils.parseShareIdFromUrl(
205+
"https://app.example.com/s/2767ba0e-ac5c-4533-b9cf-4f5fc456600e/doc/test-abc123"
206+
)
207+
).toBe("2767ba0e-ac5c-4533-b9cf-4f5fc456600e");
208+
});
209+
210+
it("should return share id when no doc path is present", () => {
211+
expect(
212+
urlsUtils.parseShareIdFromUrl("https://app.example.com/s/my-share")
213+
).toBe("my-share");
214+
});
215+
216+
it("should return undefined for non-share urls", () => {
217+
expect(
218+
urlsUtils.parseShareIdFromUrl("https://app.example.com/doc/test-abc123")
219+
).toBeUndefined();
220+
});
221+
222+
it("should return undefined for invalid urls", () => {
223+
expect(urlsUtils.parseShareIdFromUrl("not a url")).toBeUndefined();
224+
});
225+
});
226+
193227
describe("#urlRegex", () => {
194228
it("should return undefined for invalid urls", () => {
195229
expect(urlRegex(undefined)).toBeUndefined();

shared/utils/urls.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,39 @@ export function urlRegex(url: string | null | undefined): RegExp | undefined {
221221
return new RegExp(escapeRegExp(`${urlObj.protocol}//${urlObj.host}`));
222222
}
223223

224+
/**
225+
* Parse the share identifier from a given url.
226+
*
227+
* @param url The url to parse.
228+
* @returns A share identifier or undefined if not found.
229+
*/
230+
export function parseShareIdFromUrl(url: string): string | undefined {
231+
if (url[0] === "/") {
232+
url = `${env.URL}${url}`;
233+
}
234+
235+
let pathname;
236+
try {
237+
pathname = new URL(url).pathname;
238+
} catch (_err) {
239+
return;
240+
}
241+
242+
const split = pathname.split("/");
243+
const indexOfS = split.indexOf("s");
244+
245+
if (indexOfS >= 0) {
246+
const shareId = split[indexOfS + 1];
247+
if (shareId) {
248+
// Remove trailing format like .md
249+
const dotIndex = shareId.indexOf(".");
250+
return dotIndex >= 0 ? shareId.substring(0, dotIndex) : shareId;
251+
}
252+
}
253+
254+
return undefined;
255+
}
256+
224257
/**
225258
* Extracts LIKELY urls from the given text, note this does not validate the urls.
226259
*

0 commit comments

Comments
 (0)