Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions src/backend/clients/email/EmailClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ describe('EmailClient — share notification templates', () => {
items: [{ name: 'a.txt' }, { name: 'b.txt' }],
},
]),
link: 'https://puter.test',
link: 'https://puter.test/?shared=%2Fbob%2Fu1%2Fnotes.md',
origin: 'https://puter.test',
unsubscribe_uuid: null,
};

Expand Down Expand Up @@ -364,15 +365,21 @@ describe('EmailClient — share notification templates', () => {
expect(preheader).toBeLessThan(html.indexOf('Hi alice,'));
});

it('points the call to action at the app and nowhere else', async () => {
const { html, text } = await renderShare(
'file_shared_with_you',
HOLDER,
);
// The button carries every item in the mail, so it is a query string with
// `&` and `=` in it — which must reach the client as written, in both parts.
it('points the call to action at Shared with every item picked out', async () => {
const link =
'https://puter.test/?shared=%2Fbob%2Fu1%2Fa.txt&shared=%2Fbob%2Fu2%2Fb.txt';
const { html, text } = await renderShare('file_shared_with_you', {
...HOLDER,
link,
});

expect(html).toContain('href="https://puter.test"');
expect(html).toContain(`href="${link}"`);
expect(html).toContain('>Open Puter</a>');
expect(text).toContain('Open Puter: https://puter.test');
expect(text).toContain(`Open Puter: ${link}`);
expect(html).not.toContain('&#x3D;');
expect(html).not.toContain('&amp;shared');
});

it('separates senders without a rule above the first', async () => {
Expand Down
13 changes: 9 additions & 4 deletions src/backend/clients/email/templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,17 @@ const textRow = (text: string, padding = '18px 0 0'): string => `
* The call to action. Padding sits on the cell and the color on `bgcolor` so
* Outlook still draws a real button (square-cornered, which is fine); the table
* goes full width under 600px so the tap target spans the card.
*
* `link` is triple-braced for the same reason as an item's: we build it, and a
* deep link's `=` and `&` should read the same in the html as in the text.
*/
const buttonRow = (label: string): string => `
<tr>
<td style="padding: 26px 0 0;">
<table role="presentation" class="btn" cellpadding="0" cellspacing="0" border="0" style="border-collapse: separate;">
<tr>
<td align="center" bgcolor="${ACCENT}" style="background-color: ${ACCENT}; border-radius: 10px; padding: 14px 26px;">
<a href="{{link}}" style="display: inline-block; font-family: ${FONT}; font-size: 16px; line-height: 20px; font-weight: 600; color: #ffffff; text-decoration: none;">${label}</a>
<a href="{{{link}}}" style="display: inline-block; font-family: ${FONT}; font-size: 16px; line-height: 20px; font-weight: 600; color: #ffffff; text-decoration: none;">${label}</a>
</td>
</tr>
</table>
Expand Down Expand Up @@ -370,7 +373,9 @@ immediately</p>
/**
* A digest: shares to one recipient are held briefly and merged, so
* `shares` may carry several senders. The subject is composed by the
* service (see `digestSubject`), which owns the grouped wording.
* service (see `digestSubject`), which owns the grouped wording. `link`
* opens Shared with every item highlighted; `origin` is the bare site, for
* the unsubscribe link.
*/
file_shared_with_you: {
subject: '{{subject_line}}',
Expand All @@ -386,7 +391,7 @@ immediately</p>
'24px 0 0',
),
footer: `You're receiving this because someone shared with your Puter account.{{#if unsubscribe_uuid}}
<br /><a href="{{link}}/unsubscribe?user_uuid={{unsubscribe_uuid}}" style="color: inherit; text-decoration: underline;">Unsubscribe from notification emails</a>{{/if}}`,
<br /><a href="{{origin}}/unsubscribe?user_uuid={{unsubscribe_uuid}}" style="color: inherit; text-decoration: underline;">Unsubscribe from notification emails</a>{{/if}}`,
}),
text: `
Hi{{#if recipient}} {{recipient}}{{/if}},
Expand All @@ -404,7 +409,7 @@ immediately</p>
--
You're receiving this because someone shared with your Puter account.
{{#if unsubscribe_uuid}}Unsubscribe from notification emails:
{{link}}/unsubscribe?user_uuid={{unsubscribe_uuid}}{{/if}}
{{origin}}/unsubscribe?user_uuid={{unsubscribe_uuid}}{{/if}}
`,
},
// The only way to reach someone with no account. Same digest shape.
Expand Down
21 changes: 17 additions & 4 deletions src/backend/services/share/ShareNotificationService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type { Actor } from '../../core/actor';
import { checkRateLimit } from '../../core/http/middleware/rateLimit.js';
import { PuterService } from '../types';
import {
digestItemPaths,
digestLines,
digestSubject,
mergeDigestEntry,
Expand All @@ -37,6 +38,7 @@ import {
maskedSharePath,
ownerFromSharePath,
shareDeepLink,
sharedViewLink,
} from './shareDeepLink';
import type { ResolvedShare } from './ShareService';

Expand Down Expand Up @@ -591,7 +593,11 @@ export class ShareNotificationService extends PuterService {
if (!share.name) return null;
const path = this.#targetPath(share);
if (!path) return { name: share.name };
return { name: share.name, link: shareDeepLink(this.#appLink(), path) };
return {
name: share.name,
link: shareDeepLink(this.#appLink(), path),
path,
};
}

/** The masked path for a share, or `null` when it isn't addressable. */
Expand Down Expand Up @@ -798,9 +804,16 @@ export class ShareNotificationService extends PuterService {
recipient: first.recipient,
subject_line: digestSubject(entries),
shares: digestLines(entries),
link: this.#appLink(),
// The template composes the URL, so `?` and `=`
// stay literal instead of escaping to `&#x3D;`.
// "Open Puter" lands on Shared with everything
// in this mail picked out, not just one item.
link: sharedViewLink(
this.#appLink(),
digestItemPaths(entries),
),
// The template composes the unsubscribe URL from
// the origin, so `?` and `=` stay literal instead
// of escaping to `&#x3D;`.
origin: this.#appLink(),
unsubscribe_uuid: first.recipientUuid ?? null,
},
);
Expand Down
88 changes: 88 additions & 0 deletions src/backend/services/share/shareDeepLink.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ import { describe, expect, it } from 'vitest';
import {
maskedSharePath,
ownerFromSharePath,
SHARE_DEEP_LINK_ITEMS_LIMIT,
SHARE_DEEP_LINK_MAX_LENGTH,
shareDeepLink,
sharedViewLink,
shareTargetLink,
} from './shareDeepLink.js';

Expand Down Expand Up @@ -94,6 +97,91 @@ describe('shareDeepLink', () => {
});
});

describe('sharedViewLink', () => {
it('repeats the parameter once per item, in order', () => {
const link = sharedViewLink('https://puter.com', [
`/alice/${UID}/a.txt`,
`/bob/${UID}/b.txt`,
]);
expect(link).toBe(
`https://puter.com/?shared=%2Falice%2F${UID}%2Fa.txt&shared=%2Fbob%2F${UID}%2Fb.txt`,
);
// Round-trips: the GUI reads back every path, as it was.
expect(new URL(link).searchParams.getAll('shared')).toEqual([
`/alice/${UID}/a.txt`,
`/bob/${UID}/b.txt`,
]);
});

// Nothing addressable still deserves a way in: the parameter alone opens
// Shared, the same place a link with items lands.
it('still opens Shared when there is nothing to pick out', () => {
expect(sharedViewLink('https://puter.com', [])).toBe(
'https://puter.com/?shared=',
);
});

// A name can run to hundreds of characters, and encoding multiplies
// non-ASCII ones; the link exists to name the item, so it always does,
// however long — the length cap only limits how many more join it.
it('keeps the first item even when it alone outgrows the length cap', () => {
const long = `/alice/${UID}/${'\u5831\u544a'.repeat(120)}.pdf`;
const short = `/alice/${UID}/a.txt`;
expect(shareDeepLink('https://puter.com', long).length).toBeGreaterThan(
SHARE_DEEP_LINK_MAX_LENGTH,
);
expect(
new URL(
shareDeepLink('https://puter.com', long),
).searchParams.getAll('shared'),
).toEqual([long]);
// Nothing fits after it, and nothing later is taken instead.
expect(
new URL(
sharedViewLink('https://puter.com', [long, short]),
).searchParams.getAll('shared'),
).toEqual([long]);
});

it('names an item once however often it was queued', () => {
const path = `/alice/${UID}/a.txt`;
expect(sharedViewLink('https://puter.com', [path, path])).toBe(
shareDeepLink('https://puter.com', path),
);
});

it('stops adding items where mail clients stop tolerating the length', () => {
const paths = Array.from(
{ length: SHARE_DEEP_LINK_ITEMS_LIMIT + 5 },
(_, i) => `/alice/${UID}/file-${i}.txt`,
);
const shared = new URL(
sharedViewLink('https://puter.com', paths),
).searchParams.getAll('shared');
expect(shared).toEqual(paths.slice(0, SHARE_DEEP_LINK_ITEMS_LIMIT));
});

// Twenty ordinary names already run to several kilobytes once encoded, so
// the count alone is no guard; the link itself has to stay short enough.
it('stops adding items before the link outgrows what mail clients tolerate', () => {
const paths = Array.from(
{ length: SHARE_DEEP_LINK_ITEMS_LIMIT },
(_, i) => `/alice/${UID}/${'quarterly report '.repeat(8)}${i}.pdf`,
);
const link = sharedViewLink('https://puter.com', paths);
expect(link.length).toBeLessThanOrEqual(SHARE_DEEP_LINK_MAX_LENGTH);
const shared = new URL(link).searchParams.getAll('shared');
// Only a leading run made it — the first items, none skipped.
expect(shared.length).toBeGreaterThan(1);
expect(shared.length).toBeLessThan(paths.length);
expect(shared).toEqual(paths.slice(0, shared.length));
// The next item would not have fit.
expect(
link.length + `&shared=${encodeURIComponent(paths[shared.length])}`.length,
).toBeGreaterThan(SHARE_DEEP_LINK_MAX_LENGTH);
});
});

describe('shareTargetLink', () => {
it('links an addressable target and nothing else', () => {
expect(
Expand Down
56 changes: 47 additions & 9 deletions src/backend/services/share/shareDeepLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
*/

/**
* Links that open a shared item. Not derived from `ResolvedShare.path`: that is
* masked for the requester, and the issuer owns the entry, so it comes back as
* the owner's real path — which mailing would leak.
* Links that open a shared item: the dashboard's Files tab, on Shared, with the
* item highlighted. Not derived from `ResolvedShare.path`: that is masked for
* the requester, and the issuer owns the entry, so it comes back as the owner's
* real path — which mailing would leak.
*/

/** The query parameter the GUI routes on. */
Expand Down Expand Up @@ -53,15 +54,52 @@ export const maskedSharePath = (target: ShareTarget): string | null => {
};

/**
* A link that opens `path` once the recipient is signed in. Only the masked
* path travels — its second segment is the uuid, so a rename is recoverable and
* there is no second copy to disagree with the first.
* Items one link will highlight, at most. Past this the link still opens
* Shared, just without picking the rest out.
*/
export const shareDeepLink = (origin: string, path: string): string => {
const base = origin.replace(/\/+$/, '');
return `${base}/?${SHARE_DEEP_LINK_PARAM}=${encodeURIComponent(path)}`;
export const SHARE_DEEP_LINK_ITEMS_LIMIT = 20;

/**
* How long a link may run, in characters. Somewhere past two thousand, older
* mail clients cut a URL off or stop making it clickable — and this is the
* button — so items are added only while the whole link stays within this. The
* first item goes in regardless: a link that names nothing is no better than
* the origin, and one long name (hundreds of characters, tripled by encoding
* when non-ASCII) is still the item the mail is about.
*/
export const SHARE_DEEP_LINK_MAX_LENGTH = 2000;

/**
* A link that opens the recipient's Shared view with `paths` highlighted, once
* they are signed in. Only masked paths travel — each one's second segment is
* the uuid, so a rename is recoverable and there is no second copy to disagree
* with the first. With no paths the link still lands on Shared.
*/
export const sharedViewLink = (origin: string, paths: string[]): string => {
const base = `${origin.replace(/\/+$/, '')}/?`;
// The first items that fit, in order — never a later one over an
// earlier, so what is highlighted reads as the top of the list.
const params: string[] = [];
let length = base.length;
for (const path of new Set(paths)) {
if (params.length === SHARE_DEEP_LINK_ITEMS_LIMIT) break;
const param = `${SHARE_DEEP_LINK_PARAM}=${encodeURIComponent(path)}`;
const added = param.length + (params.length === 0 ? 0 : '&'.length);
const overLength = length + added > SHARE_DEEP_LINK_MAX_LENGTH;
if (params.length > 0 && overLength) break;
params.push(param);
length += added;
}
return (
base +
(params.length === 0 ? `${SHARE_DEEP_LINK_PARAM}=` : params.join('&'))
);
};

/** A link that opens `path`: the Shared view with that one item highlighted. */
export const shareDeepLink = (origin: string, path: string): string =>
sharedViewLink(origin, [path]);

/** The link for a target, or `null` when it isn't addressable. */
export const shareTargetLink = (
origin: string,
Expand Down
31 changes: 24 additions & 7 deletions src/backend/services/share/shareEmail.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ describe('share email', () => {
vi.restoreAllMocks();
});

/** Where the "Open Puter" button points, as distinct from the item links. */
const openPuterHref = (html: string): string | undefined =>
html.match(/href="([^"]+)"[^>]*>Open Puter<\/a>/)?.[1];

const post = (path: string, token: string, body: unknown) =>
fetch(new URL(path, env.apiOrigin), {
method: 'POST',
Expand Down Expand Up @@ -351,8 +355,13 @@ describe('share email', () => {
`${owner.username} shared ${first.name} with you`,
);
expect(mail.html).toContain(first.name);
expect(mail.html).toContain('Open Puter');
expect(mail.html).toContain(`href="${env.origin}"`);
// The button opens Shared with the item picked out — on the full
// origin, port included, like every other link in the mail.
expect(openPuterHref(mail.html)).toBe(
`${env.origin}/?shared=${encodeURIComponent(
`/${owner.username}/${first.uid}/${first.name}`,
)}`,
);
expect(mail.html).toContain(recipient.username);

// A second share to the same pair inside the window is one more thing to
Expand Down Expand Up @@ -509,12 +518,20 @@ describe('share email', () => {
);

const mail = await waitForMail({ to: recipient.email });
for (const file of files) {
const masked = `/${sender.username}/${file.uid}/${file.name}`;
expect(mail.html).toContain(
`?shared=${encodeURIComponent(masked)}`,
);
const masked = files.map(
(file) => `/${sender.username}/${file.uid}/${file.name}`,
);
for (const path of masked) {
expect(mail.html).toContain(`?shared=${encodeURIComponent(path)}`);
}
// "Open Puter" is one link for the whole mail: Shared, with every item
// in it picked out — not the origin, which would land them on Home.
const href = openPuterHref(mail.html);
expect(href).toBeDefined();
expect(new URL(href!).searchParams.getAll('shared').sort()).toEqual(
[...masked].sort(),
);
expect(mail.text).toContain(`Open Puter: ${href}`);
});

// Nothing to route to yet, so the names stay plain and the call to action
Expand Down
21 changes: 21 additions & 0 deletions src/backend/services/share/shareNotifyTitle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import { describe, expect, it } from 'vitest';
import {
digestItemPaths,
digestLines,
digestSubject,
mergeDigestEntry,
Expand Down Expand Up @@ -210,6 +211,26 @@ describe('email digests', () => {
}
});

it('lists every addressable item once, across senders, for one link', () => {
const paths = digestItemPaths([
{
username: 'alice',
count: 3,
items: [
{ name: 'a.txt', link: 'l1', path: '/alice/u1/a.txt' },
{ name: 'plain.txt' },
{ name: 'a.txt', link: 'l1', path: '/alice/u1/a.txt' },
],
},
{
username: 'bob',
count: 1,
items: [{ name: 'b.txt', link: 'l2', path: '/bob/u2/b.txt' }],
},
]);
expect(paths).toEqual(['/alice/u1/a.txt', '/bob/u2/b.txt']);
});

it('merges a sender back into their own digest entry', () => {
const merged = mergeDigestEntry(
[{ username: 'alice', count: 1, items: [item('a.txt', 'l1')] }],
Expand Down
Loading
Loading