From 12c71f267e7e598b8b5fe47e5d70fce0ce6c90ef Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Thu, 2 Apr 2026 08:08:05 -0400 Subject: [PATCH] Improve scoping of public share subscriptions (#11932) * Improve scoping of public share subscriptions * fix: Add missing transaction, includeChildDocuments check, and test documentId - Pass { transaction } to ShareSubscription.create in the subscribe handler so the insert runs atomically with the duplicate-check findOne/lock - Skip ancestor-scoped subscription notifications when the share has includeChildDocuments=false, preventing notifications for inaccessible docs - Add required documentId field to all share subscription tests Co-Authored-By: Claude Opus 4.6 * fix: Resolve type error for nullable share.documentId in tests Co-Authored-By: Claude Opus 4.6 * JSDoc * Hide subscription option for logged-in users --------- Co-authored-by: Claude Opus 4.6 --- app/components/Sharing/components/Actions.tsx | 10 +- .../Sharing/components/ShareSubscribeForm.tsx | 16 ++- app/hooks/useQueryNotices.ts | 35 +++-- app/scenes/Document/components/Header.tsx | 4 +- ...0-add-documentId-to-share-subscriptions.js | 62 +++++++++ server/models/ShareSubscription.test.ts | 82 ++++++++++-- server/models/ShareSubscription.ts | 24 ++++ ...ShareSubscriptionNotificationsTask.test.ts | 80 ++++++++++++ .../ShareSubscriptionNotificationsTask.ts | 120 ++++++++++-------- server/routes/api/shares/schema.ts | 1 + server/routes/api/shares/shares.test.ts | 15 +++ server/routes/api/shares/shares.ts | 44 ++++--- shared/i18n/locales/en_US/translation.json | 6 +- shared/types.ts | 1 + 14 files changed, 393 insertions(+), 107 deletions(-) create mode 100644 server/migrations/20260401000000-add-documentId-to-share-subscriptions.js diff --git a/app/components/Sharing/components/Actions.tsx b/app/components/Sharing/components/Actions.tsx index c63723fb4d..e5e0df51f5 100644 --- a/app/components/Sharing/components/Actions.tsx +++ b/app/components/Sharing/components/Actions.tsx @@ -50,7 +50,13 @@ export const AppearanceAction = observer(() => { ); }); -export function SubscribeAction({ shareId }: { shareId: string }) { +export function SubscribeAction({ + shareId, + documentId, +}: { + shareId: string; + documentId?: string; +}) { const { t } = useTranslation(); const [open, setOpen] = useState(false); @@ -68,7 +74,7 @@ export function SubscribeAction({ shareId }: { shareId: string }) { - + diff --git a/app/components/Sharing/components/ShareSubscribeForm.tsx b/app/components/Sharing/components/ShareSubscribeForm.tsx index 9f1968e8fa..529e1c968a 100644 --- a/app/components/Sharing/components/ShareSubscribeForm.tsx +++ b/app/components/Sharing/components/ShareSubscribeForm.tsx @@ -12,7 +12,13 @@ import { client } from "~/utils/ApiClient"; /** * Subscribe form content displayed inside the popover. */ -export function ShareSubscribeForm({ shareId }: { shareId: string }) { +export function ShareSubscribeForm({ + shareId, + documentId, +}: { + shareId: string; + documentId?: string; +}) { const { t } = useTranslation(); const [email, setEmail] = useState(""); const [status, setStatus] = useState< @@ -25,16 +31,16 @@ export function ShareSubscribeForm({ shareId }: { shareId: string }) { ev.preventDefault(); setStatus("loading"); try { - await client.post("/shares.subscribe", { shareId, email }); + await client.post("/shares.subscribe", { shareId, documentId, email }); setStatus("success"); } catch (err) { setErrorMessage( - err instanceof Error ? err.message : t("Something went wrong.") + err instanceof Error ? err.message : t("Something went wrong") ); setStatus("error"); } }, - [shareId, email] + [shareId, documentId, email] ); const handleChange = useCallback( @@ -52,7 +58,7 @@ export function ShareSubscribeForm({ shareId }: { shareId: string }) { return ( - {t("Check your email to confirm your subscription.")} + {t("Check your email to confirm your subscription")}. ); diff --git a/app/hooks/useQueryNotices.ts b/app/hooks/useQueryNotices.ts index 380cf6de03..c7c3426ce1 100644 --- a/app/hooks/useQueryNotices.ts +++ b/app/hooks/useQueryNotices.ts @@ -1,5 +1,6 @@ import { useEffect } from "react"; import { useTranslation } from "react-i18next"; +import { useHistory } from "react-router-dom"; import { toast } from "sonner"; import { QueryNotices } from "@shared/types"; import useQuery from "./useQuery"; @@ -11,28 +12,40 @@ import useQuery from "./useQuery"; */ export default function useQueryNotices() { const query = useQuery(); + const history = useHistory(); const { t } = useTranslation(); const notice = query.get("notice") as QueryNotices; useEffect(() => { + let message: string | undefined; + switch (notice) { case QueryNotices.UnsubscribeDocument: { - toast.success( - t("Unsubscribed from document", { - type: "success", - }) - ); + message = t("Unsubscribed from document"); break; } case QueryNotices.UnsubscribeCollection: { - toast.success( - t("Unsubscribed from collection", { - type: "success", - }) - ); + message = t("Unsubscribed from collection"); + break; + } + case QueryNotices.Subscribed: { + message = t("Subscription successful"); break; } default: } - }, [t, notice]); + + if (message) { + toast.success(message); + + // Remove the notice param from the URL to prevent duplicate toasts + const params = new URLSearchParams(window.location.search); + params.delete("notice"); + const search = params.toString(); + history.replace({ + pathname: window.location.pathname, + search: search ? `?${search}` : "", + }); + } + }, [t, notice, history]); } diff --git a/app/scenes/Document/components/Header.tsx b/app/scenes/Document/components/Header.tsx index 5b76b4fef9..d1aef18b02 100644 --- a/app/scenes/Document/components/Header.tsx +++ b/app/scenes/Document/components/Header.tsx @@ -213,8 +213,8 @@ function DocumentHeader({ } actions={ <> - {allowSubscriptions !== false && ( - + {allowSubscriptions !== false && !user && ( + )} {can.update && !isEditing ? editAction :
} diff --git a/server/migrations/20260401000000-add-documentId-to-share-subscriptions.js b/server/migrations/20260401000000-add-documentId-to-share-subscriptions.js new file mode 100644 index 0000000000..53a1639d96 --- /dev/null +++ b/server/migrations/20260401000000-add-documentId-to-share-subscriptions.js @@ -0,0 +1,62 @@ +"use strict"; + +module.exports = { + async up(queryInterface, Sequelize) { + return queryInterface.sequelize.transaction(async (transaction) => { + // Remove all existing records as they lack document scope + await queryInterface.bulkDelete( + "share_subscriptions", + {}, + { transaction } + ); + + await queryInterface.addColumn( + "share_subscriptions", + "documentId", + { + type: Sequelize.UUID, + allowNull: false, + references: { + model: "documents", + key: "id", + }, + onDelete: "CASCADE", + }, + { transaction } + ); + + // Replace old unique index with one that includes documentId + await queryInterface.removeIndex( + "share_subscriptions", + ["shareId", "emailFingerprint"], + { transaction } + ); + + await queryInterface.addIndex( + "share_subscriptions", + ["shareId", "documentId", "emailFingerprint"], + { unique: true, transaction } + ); + }); + }, + + async down(queryInterface) { + return queryInterface.sequelize.transaction(async (transaction) => { + await queryInterface.removeIndex( + "share_subscriptions", + ["shareId", "documentId", "emailFingerprint"], + { transaction } + ); + + await queryInterface.addIndex( + "share_subscriptions", + ["shareId", "emailFingerprint"], + { unique: true, transaction } + ); + + await queryInterface.removeColumn("share_subscriptions", "documentId", { + transaction, + }); + }); + }, +}; diff --git a/server/models/ShareSubscription.test.ts b/server/models/ShareSubscription.test.ts index 14d2278264..c1a5a919ac 100644 --- a/server/models/ShareSubscription.test.ts +++ b/server/models/ShareSubscription.test.ts @@ -1,13 +1,18 @@ import { randomString } from "@shared/random"; -import { buildShare } from "@server/test/factories"; +import { buildDocument, buildShare } from "@server/test/factories"; import ShareSubscription from "./ShareSubscription"; describe("ShareSubscription", () => { describe("generateConfirmToken / generateUnsubscribeToken", () => { it("should produce deterministic tokens", async () => { - const share = await buildShare(); + const document = await buildDocument(); + const share = await buildShare({ + documentId: document.id, + teamId: document.teamId, + }); const subscription = await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "test@example.com", emailFingerprint: "test@example.com", secret: randomString(32), @@ -19,9 +24,14 @@ describe("ShareSubscription", () => { }); it("should produce different tokens for confirm vs unsubscribe", async () => { - const share = await buildShare(); + const document = await buildDocument(); + const share = await buildShare({ + documentId: document.id, + teamId: document.teamId, + }); const subscription = await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "test@example.com", emailFingerprint: "test@example.com", secret: randomString(32), @@ -34,15 +44,22 @@ describe("ShareSubscription", () => { }); it("should produce different tokens for different secrets", async () => { - const share = await buildShare(); + const document = await buildDocument(); + const share = await buildShare({ + documentId: document.id, + teamId: document.teamId, + }); const sub1 = await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "test@example.com", emailFingerprint: "test@example.com", secret: randomString(32), }); + const document2 = await buildDocument({ teamId: document.teamId }); const sub2 = await ShareSubscription.create({ shareId: share.id, + documentId: document2.id, email: "test2@example.com", emailFingerprint: "test2@example.com", secret: randomString(32), @@ -149,12 +166,17 @@ describe("ShareSubscription", () => { }); it("should allow up to 3 unique emails from the same IP", async () => { - const share = await buildShare(); const ip = "192.168.1.1"; for (let i = 0; i < 3; i++) { + const document = await buildDocument(); + const share = await buildShare({ + documentId: document.id, + teamId: document.teamId, + }); await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: `user${i}@example.com`, emailFingerprint: `user${i}@example.com`, secret: randomString(32), @@ -162,9 +184,15 @@ describe("ShareSubscription", () => { }); } + const document = await buildDocument(); + const share = await buildShare({ + documentId: document.id, + teamId: document.teamId, + }); await expect( ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "user3@example.com", emailFingerprint: "user3@example.com", secret: randomString(32), @@ -174,11 +202,15 @@ describe("ShareSubscription", () => { }); it("should not count subscriptions from different IPs", async () => { - const share = await buildShare(); - for (let i = 0; i < 3; i++) { + const document = await buildDocument(); + const share = await buildShare({ + documentId: document.id, + teamId: document.teamId, + }); await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: `user${i}@example.com`, emailFingerprint: `user${i}@example.com`, secret: randomString(32), @@ -186,9 +218,15 @@ describe("ShareSubscription", () => { }); } + const document = await buildDocument(); + const share = await buildShare({ + documentId: document.id, + teamId: document.teamId, + }); await expect( ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "user3@example.com", emailFingerprint: "user3@example.com", secret: randomString(32), @@ -198,15 +236,28 @@ describe("ShareSubscription", () => { }); it("should not count duplicate fingerprints from the same IP", async () => { - const share1 = await buildShare(); - const share2 = await buildShare(); + const document1 = await buildDocument(); + const share1 = await buildShare({ + documentId: document1.id, + teamId: document1.teamId, + }); + const document2 = await buildDocument(); + const share2 = await buildShare({ + documentId: document2.id, + teamId: document2.teamId, + }); const ip = "192.168.2.1"; // Same fingerprint across different shares — should count as 1 for (let i = 0; i < 3; i++) { - const share = await buildShare(); + const doc = await buildDocument(); + const share = await buildShare({ + documentId: doc.id, + teamId: doc.teamId, + }); await ShareSubscription.create({ shareId: share.id, + documentId: doc.id, email: "same@example.com", emailFingerprint: "same@example.com", secret: randomString(32), @@ -216,8 +267,10 @@ describe("ShareSubscription", () => { // 2 more unique fingerprints — total distinct = 3 for (let i = 0; i < 2; i++) { + const doc = await buildDocument(); await ShareSubscription.create({ shareId: share1.id, + documentId: doc.id, email: `other${i}@example.com`, emailFingerprint: `other${i}@example.com`, secret: randomString(32), @@ -229,6 +282,7 @@ describe("ShareSubscription", () => { await expect( ShareSubscription.create({ shareId: share2.id, + documentId: document2.id, email: "blocked@example.com", emailFingerprint: "blocked@example.com", secret: randomString(32), @@ -238,11 +292,15 @@ describe("ShareSubscription", () => { }); it("should skip the check if ipAddress is null", async () => { - const share = await buildShare(); - for (let i = 0; i < 6; i++) { + const document = await buildDocument(); + const share = await buildShare({ + documentId: document.id, + teamId: document.teamId, + }); await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: `user${i}@example.com`, emailFingerprint: `user${i}@example.com`, secret: randomString(32), diff --git a/server/models/ShareSubscription.ts b/server/models/ShareSubscription.ts index 50b0ee05c5..f5c83a292c 100644 --- a/server/models/ShareSubscription.ts +++ b/server/models/ShareSubscription.ts @@ -12,10 +12,15 @@ import { } from "sequelize-typescript"; import { subHours } from "date-fns"; import { ValidationError } from "@server/errors"; +import Document from "./Document"; import Share from "./Share"; import IdModel from "./base/IdModel"; import Fix from "./decorators/Fix"; +/** + * A subscription to email notifications for updates to a publicly shared + * document and its descendants. + */ @Scopes(() => ({ active: { where: { @@ -37,15 +42,27 @@ class ShareSubscription extends IdModel< @Column(DataType.UUID) shareId: string; + /** The document to scope notifications to (the document and its descendants). */ + @BelongsTo(() => Document, "documentId") + document: Document; + + @ForeignKey(() => Document) + @Column(DataType.UUID) + documentId: string; + + /** The subscribed email */ @Column(DataType.STRING) email: string; + /** Normalized email fingerprint helps to improve spam detection through removal of common bypasses */ @Column(DataType.STRING) emailFingerprint: string; + /** Signing secret for subscribe/unsubscribe links */ @Column(DataType.STRING) secret: string; + /** IP address of the user that subscribed */ @Column(DataType.STRING(45)) ipAddress: string | null; @@ -61,6 +78,13 @@ class ShareSubscription extends IdModel< /** Maximum number of unique email subscriptions allowed per IP address. */ static maxSubscriptionsPerIP = 3; + /** + * Enforce a per-IP rate limit on subscription creation to prevent abuse. + * + * @param model The subscription being created. + * @param options The save options including the current transaction. + * @throws when the IP has reached the maximum number of subscriptions. + */ @BeforeCreate static async checkIPLimit( model: ShareSubscription, diff --git a/server/queues/tasks/ShareSubscriptionNotificationsTask.test.ts b/server/queues/tasks/ShareSubscriptionNotificationsTask.test.ts index 9b19c00793..30ed7253b5 100644 --- a/server/queues/tasks/ShareSubscriptionNotificationsTask.test.ts +++ b/server/queues/tasks/ShareSubscriptionNotificationsTask.test.ts @@ -22,6 +22,7 @@ describe("ShareSubscriptionNotificationsTask", () => { }); await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "subscriber@example.com", emailFingerprint: "subscriber@example.com", secret: randomString(32), @@ -51,6 +52,7 @@ describe("ShareSubscriptionNotificationsTask", () => { }); await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "subscriber@example.com", emailFingerprint: "subscriber@example.com", secret: randomString(32), @@ -79,6 +81,7 @@ describe("ShareSubscriptionNotificationsTask", () => { }); await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "subscriber@example.com", emailFingerprint: "subscriber@example.com", secret: randomString(32), @@ -109,6 +112,7 @@ describe("ShareSubscriptionNotificationsTask", () => { }); await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "subscriber@example.com", emailFingerprint: "subscriber@example.com", secret: randomString(32), @@ -139,6 +143,7 @@ describe("ShareSubscriptionNotificationsTask", () => { }); await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "subscriber@example.com", emailFingerprint: "subscriber@example.com", secret: randomString(32), @@ -170,6 +175,7 @@ describe("ShareSubscriptionNotificationsTask", () => { }); await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "subscriber@example.com", emailFingerprint: "subscriber@example.com", secret: randomString(32), @@ -199,6 +205,7 @@ describe("ShareSubscriptionNotificationsTask", () => { }); const subscription = await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "subscriber@example.com", emailFingerprint: "subscriber@example.com", secret: randomString(32), @@ -232,6 +239,7 @@ describe("ShareSubscriptionNotificationsTask", () => { await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "sub1@example.com", emailFingerprint: "sub1@example.com", secret: randomString(32), @@ -239,6 +247,7 @@ describe("ShareSubscriptionNotificationsTask", () => { }); await ShareSubscription.create({ shareId: share.id, + documentId: document.id, email: "sub2@example.com", emailFingerprint: "sub2@example.com", secret: randomString(32), @@ -274,4 +283,75 @@ describe("ShareSubscriptionNotificationsTask", () => { expect(spy).not.toHaveBeenCalled(); }); + + it("should send when child document is updated and subscription is scoped to parent", async () => { + const spy = jest.spyOn(ShareDocumentUpdatedEmail.prototype, "schedule"); + + const parent = await buildDocument(); + const child = await buildDocument({ + parentDocumentId: parent.id, + collectionId: parent.collectionId, + teamId: parent.teamId, + }); + const share = await buildShare({ + documentId: parent.id, + teamId: parent.teamId, + includeChildDocuments: true, + }); + await ShareSubscription.create({ + shareId: share.id, + documentId: parent.id, + email: "subscriber@example.com", + emailFingerprint: "subscriber@example.com", + secret: randomString(32), + confirmedAt: new Date(), + }); + + const task = new ShareSubscriptionNotificationsTask(); + await task.perform({ + name: "revisions.create", + documentId: child.id, + teamId: child.teamId, + actorId: child.createdById, + modelId: "revision-id", + ip, + }); + + expect(spy).toHaveBeenCalledTimes(1); + }); + + it("should not send when updated document is outside subscription scope", async () => { + const spy = jest.spyOn(ShareDocumentUpdatedEmail.prototype, "schedule"); + + const parent = await buildDocument(); + const sibling = await buildDocument({ + collectionId: parent.collectionId, + teamId: parent.teamId, + }); + const share = await buildShare({ + documentId: parent.id, + teamId: parent.teamId, + includeChildDocuments: true, + }); + await ShareSubscription.create({ + shareId: share.id, + documentId: parent.id, + email: "subscriber@example.com", + emailFingerprint: "subscriber@example.com", + secret: randomString(32), + confirmedAt: new Date(), + }); + + const task = new ShareSubscriptionNotificationsTask(); + await task.perform({ + name: "revisions.create", + documentId: sibling.id, + teamId: sibling.teamId, + actorId: sibling.createdById, + modelId: "revision-id", + ip, + }); + + expect(spy).not.toHaveBeenCalled(); + }); }); diff --git a/server/queues/tasks/ShareSubscriptionNotificationsTask.ts b/server/queues/tasks/ShareSubscriptionNotificationsTask.ts index 91bc06de4a..2cf3fc0ad8 100644 --- a/server/queues/tasks/ShareSubscriptionNotificationsTask.ts +++ b/server/queues/tasks/ShareSubscriptionNotificationsTask.ts @@ -1,5 +1,4 @@ import { subHours } from "date-fns"; -import { Op } from "sequelize"; import ShareDocumentUpdatedEmail from "@server/emails/templates/ShareDocumentUpdatedEmail"; import Logger from "@server/logging/Logger"; import { Document, Share, ShareSubscription } from "@server/models"; @@ -13,67 +12,76 @@ export default class ShareSubscriptionNotificationsTask extends BaseTask subHours(new Date(), 6) - ) { - Logger.info( - "processor", - `suppressing share subscription notification to ${subscription.id} as recently notified` - ); - continue; - } - - const baseShareUrl = share.canonicalUrl; - const shareUrl = - share.collectionId && document.path - ? `${baseShareUrl.replace(/\/$/, "")}${document.path}` - : baseShareUrl; - - new ShareDocumentUpdatedEmail({ - to: subscription.email, - shareSubscriptionId: subscription.id, - documentTitle: document.titleWithDefault, - shareUrl, - }).schedule(); - - subscription.lastNotifiedAt = new Date(); - await subscription.save(); + // Throttle: only one notification per 6 hours + if ( + subscription.lastNotifiedAt && + subscription.lastNotifiedAt > subHours(new Date(), 6) + ) { + Logger.info( + "processor", + `suppressing share subscription notification to ${subscription.id} as recently notified` + ); + continue; } + + const baseShareUrl = subscription.share.canonicalUrl; + const shareUrl = + document.id !== subscription.share.documentId && document.path + ? `${baseShareUrl.replace(/\/$/, "")}${document.path}` + : baseShareUrl; + + new ShareDocumentUpdatedEmail({ + to: subscription.email, + shareSubscriptionId: subscription.id, + documentTitle: document.titleWithDefault, + shareUrl, + }).schedule(); + + subscription.lastNotifiedAt = new Date(); + await subscription.save(); } } diff --git a/server/routes/api/shares/schema.ts b/server/routes/api/shares/schema.ts index f5b921b452..2046d93cd4 100644 --- a/server/routes/api/shares/schema.ts +++ b/server/routes/api/shares/schema.ts @@ -111,6 +111,7 @@ export type SharesSitemapReq = z.infer; export const SharesSubscribeSchema = BaseSchema.extend({ body: z.object({ shareId: z.string(), + documentId: z.uuid(), email: z.string().email(), }), }); diff --git a/server/routes/api/shares/shares.test.ts b/server/routes/api/shares/shares.test.ts index 45be330004..bc414aace4 100644 --- a/server/routes/api/shares/shares.test.ts +++ b/server/routes/api/shares/shares.test.ts @@ -1162,6 +1162,7 @@ describe("#shares.subscribe", () => { const res = await server.post("/api/shares.subscribe", { body: { shareId: share.id, + documentId: share.documentId!, email: "subscriber@example.com", }, }); @@ -1182,6 +1183,7 @@ describe("#shares.subscribe", () => { await server.post("/api/shares.subscribe", { body: { shareId: share.id, + documentId: share.documentId!, email: "First.Last+tag@Example.com", }, }); @@ -1199,12 +1201,14 @@ describe("#shares.subscribe", () => { await server.post("/api/shares.subscribe", { body: { shareId: share.id, + documentId: share.documentId!, email: "user@gmail.com", }, }); await server.post("/api/shares.subscribe", { body: { shareId: share.id, + documentId: share.documentId!, email: "u.s.e.r@gmail.com", }, }); @@ -1219,6 +1223,7 @@ describe("#shares.subscribe", () => { const share = await buildShare(); await ShareSubscription.create({ shareId: share.id, + documentId: share.documentId!, email: "user@example.com", emailFingerprint: ShareSubscription.normalizeEmailFingerprint("user@example.com"), @@ -1229,6 +1234,7 @@ describe("#shares.subscribe", () => { const res = await server.post("/api/shares.subscribe", { body: { shareId: share.id, + documentId: share.documentId!, email: "user@example.com", }, }); @@ -1241,6 +1247,7 @@ describe("#shares.subscribe", () => { const share = await buildShare(); const subscription = await ShareSubscription.create({ shareId: share.id, + documentId: share.documentId!, email: "user@example.com", emailFingerprint: ShareSubscription.normalizeEmailFingerprint("user@example.com"), @@ -1252,6 +1259,7 @@ describe("#shares.subscribe", () => { const res = await server.post("/api/shares.subscribe", { body: { shareId: share.id, + documentId: share.documentId!, email: "user@example.com", }, }); @@ -1267,6 +1275,7 @@ describe("#shares.subscribe", () => { const res = await server.post("/api/shares.subscribe", { body: { shareId: share.id, + documentId: share.documentId!, email: "user@example.com", }, }); @@ -1278,6 +1287,7 @@ describe("#shares.subscribe", () => { const res = await server.post("/api/shares.subscribe", { body: { shareId: share.id, + documentId: share.documentId!, email: "not-an-email", }, }); @@ -1290,6 +1300,7 @@ describe("#shares.confirmSubscription", () => { const share = await buildShare(); const subscription = await ShareSubscription.create({ shareId: share.id, + documentId: share.documentId!, email: "user@example.com", emailFingerprint: "user@example.com", secret: randomString(32), @@ -1315,6 +1326,7 @@ describe("#shares.confirmSubscription", () => { const share = await buildShare(); const subscription = await ShareSubscription.create({ shareId: share.id, + documentId: share.documentId!, email: "user@example.com", emailFingerprint: "user@example.com", secret: randomString(32), @@ -1339,6 +1351,7 @@ describe("#shares.confirmSubscription", () => { const share = await buildShare(); const subscription = await ShareSubscription.create({ shareId: share.id, + documentId: share.documentId!, email: "user@example.com", emailFingerprint: "user@example.com", secret: randomString(32), @@ -1370,6 +1383,7 @@ describe("#shares.unsubscribe", () => { const share = await buildShare(); const subscription = await ShareSubscription.create({ shareId: share.id, + documentId: share.documentId!, email: "user@example.com", emailFingerprint: "user@example.com", secret: randomString(32), @@ -1396,6 +1410,7 @@ describe("#shares.unsubscribe", () => { const share = await buildShare(); const subscription = await ShareSubscription.create({ shareId: share.id, + documentId: share.documentId!, email: "user@example.com", emailFingerprint: "user@example.com", secret: randomString(32), diff --git a/server/routes/api/shares/shares.ts b/server/routes/api/shares/shares.ts index b274e7332b..c9b5acba2b 100644 --- a/server/routes/api/shares/shares.ts +++ b/server/routes/api/shares/shares.ts @@ -441,13 +441,14 @@ router.post( validate(T.SharesSubscribeSchema), transaction(), async (ctx: APIContext) => { - const { shareId, email } = ctx.input.body; + const { shareId, documentId, email } = ctx.input.body; const { transaction } = ctx.state; const team = await getTeamFromContext(ctx, { includeStateCookie: false }); // Validate the share exists and is published const { share, document } = await loadPublicShare({ id: shareId, + documentId, teamId: team?.id, }); @@ -458,7 +459,7 @@ router.post( const emailFingerprint = ShareSubscription.normalizeEmailFingerprint(email); const existing = await ShareSubscription.findOne({ - where: { shareId: share.id, emailFingerprint }, + where: { shareId: share.id, documentId, emailFingerprint }, transaction, lock: transaction.LOCK.UPDATE, }); @@ -493,13 +494,17 @@ router.post( subscription = existing; } else { - subscription = await ShareSubscription.create({ - shareId: share.id, - email, - emailFingerprint, - secret: randomString(32), - ipAddress: ctx.request.ip, - }); + subscription = await ShareSubscription.create( + { + shareId: share.id, + documentId, + email, + emailFingerprint, + secret: randomString(32), + ipAddress: ctx.request.ip, + }, + { transaction } + ); } const confirmUrl = ShareSubscriptionHelper.confirmUrl(subscription); @@ -507,11 +512,7 @@ router.post( share.team?.getPreference(TeamPreference.PublicBranding) ?? false; new ShareSubscriptionConfirmEmail({ to: email, - documentTitle: - document?.titleWithDefault ?? - share.document?.title ?? - share.collection?.name ?? - "", + documentTitle: document?.titleWithDefault ?? "", confirmUrl, teamName: usePublicBranding ? share.team?.name : undefined, }).schedule(); @@ -566,8 +567,19 @@ router.get( subscription.confirmedAt = new Date(); await subscription.save({ transaction }); - const shareUrl = share?.canonicalUrl ?? env.URL; - ctx.redirect(`${shareUrl}?notice=subscribed`); + let redirectUrl = share?.canonicalUrl ?? env.URL; + + if ( + subscription.documentId && + subscription.documentId !== share.documentId + ) { + const doc = await Document.findByPk(subscription.documentId); + if (doc?.path) { + redirectUrl = `${redirectUrl.replace(/\/$/, "")}${doc.path}`; + } + } + + ctx.redirect(`${redirectUrl}?notice=subscribed`); } ); diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json index 26d5adcb21..3a5aa8a4b7 100644 --- a/shared/i18n/locales/en_US/translation.json +++ b/shared/i18n/locales/en_US/translation.json @@ -459,8 +459,8 @@ "Subscribe to updates": "Subscribe to updates", "Add": "Add", "Add or invite": "Add or invite", - "Something went wrong.": "Something went wrong.", - "Check your email to confirm your subscription.": "Check your email to confirm your subscription.", + "Something went wrong": "Something went wrong", + "Check your email to confirm your subscription": "Check your email to confirm your subscription", "Get notified when this document is updated": "Get notified when this document is updated", "Email address": "Email address", "Viewer": "Viewer", @@ -681,6 +681,7 @@ "Could not import file": "Could not import file", "Unsubscribed from document": "Unsubscribed from document", "Unsubscribed from collection": "Unsubscribed from collection", + "Subscription successful": "Subscription successful", "Account": "Account", "API & Access": "API & Access", "Details": "Details", @@ -886,7 +887,6 @@ "Your account has been suspended": "Your account has been suspended", "Warning Sign": "Warning Sign", "A workspace admin ({{ suspendedContactEmail }}) has suspended your account. To re-activate your account, please reach out to them directly.": "A workspace admin ({{ suspendedContactEmail }}) has suspended your account. To re-activate your account, please reach out to them directly.", - "Something went wrong": "Something went wrong", "Sorry, an unknown error occurred loading the page. Please try again or contact support if the issue persists.": "Sorry, an unknown error occurred loading the page. Please try again or contact support if the issue persists.", "Created by me": "Created by me", "Weird, this shouldn't ever be empty": "Weird, this shouldn't ever be empty", diff --git a/shared/types.ts b/shared/types.ts index f50d0620ec..3a4012a462 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -673,6 +673,7 @@ export type UnfurlResponse = { export enum QueryNotices { UnsubscribeDocument = "unsubscribe-document", UnsubscribeCollection = "unsubscribe-collection", + Subscribed = "subscribed", } export type JSONValue =