diff --git a/packages/backend/server/src/__tests__/__snapshots__/mails.spec.ts.md b/packages/backend/server/src/__tests__/__snapshots__/mails.spec.ts.md index dc531a626b..d67f5b0cd8 100644 --- a/packages/backend/server/src/__tests__/__snapshots__/mails.spec.ts.md +++ b/packages/backend/server/src/__tests__/__snapshots__/mails.spec.ts.md @@ -858,7 +858,7 @@ Generated by [AVA](https://avajs.dev). ␊ ` -> test@test.com invited you to join Test Workspace +> You were invited to join a workspace on AFFiNE `␊ ␊ ` -> test@test.com accepted your invitation +> Your workspace invitation was accepted `␊ ␊ ` -> test@test.com left Test Workspace +> A workspace member left `␊ ␊ ` -> New request to join Test Workspace +> New request to join a workspace `␊ ␊ ` -> Your request to join Test Workspace has been approved +> Your request to join a workspace has been approved `␊ ␊ ` -> Your request to join Test Workspace was declined +> Your request to join a workspace was declined `␊ ␊ ` -> You have been removed from Test Workspace +> You have been removed from a workspace `␊ ␊ ` -> Your ownership of Test Workspace has been transferred +> Your workspace ownership has been transferred `␊ ␊ ` -> You are now the owner of Test Workspace +> You are now the owner of a workspace `␊ ␊ ` -> test@test.com mentioned you in Test Doc +> You were mentioned in AFFiNE `␊ ␊ @@ -1601,7 +1601,7 @@ Generated by [AVA](https://avajs.dev). ␊ ` -> test@test.com commented on Test Doc +> New comment in AFFiNE `␊ ␊ @@ -1695,7 +1695,7 @@ Generated by [AVA](https://avajs.dev). ␊ ` -> test@test.com mentioned you in a comment on Test Doc +> You were mentioned in a comment `␊ ␊ @@ -1894,7 +1894,7 @@ Generated by [AVA](https://avajs.dev). ␊ ` -> You are now an admin of Test Workspace +> You are now a workspace admin `␊ ␊ ` -> Your role has been changed in Test Workspace +> Your workspace role has been changed `␊ ␊ ` -> [Action Required] Final warning: Your workspace Test Workspace will be deleted in 24 hours +> [Action Required] Final warning: Your workspace will be deleted in 24 hours `␊ ␊ ` -> [Action Required] Important: Your workspace Test Workspace will be deleted soon +> [Action Required] Important: Your workspace will be deleted soon `␊ ␊ ` -> Your workspace Test Workspace has been deleted +> Your workspace has been deleted `␊ ␊ ` -> [Action Required] Your Test Workspace team workspace will expire soon +> [Action Required] Your team workspace will expire soon `␊ ␊ ` -> Your Test Workspace team workspace has expired +> Your team workspace has expired `␊ test@test.com mentioned you in +> You were mentioned in AFFiNE `␊ ␊ diff --git a/packages/backend/server/src/__tests__/__snapshots__/mails.spec.ts.snap b/packages/backend/server/src/__tests__/__snapshots__/mails.spec.ts.snap index a8e784b77e..17eed3c5e8 100644 Binary files a/packages/backend/server/src/__tests__/__snapshots__/mails.spec.ts.snap and b/packages/backend/server/src/__tests__/__snapshots__/mails.spec.ts.snap differ diff --git a/packages/backend/server/src/core/mail/__tests__/job.spec.ts b/packages/backend/server/src/core/mail/__tests__/job.spec.ts index 2bdbfc0573..9e2283f1b7 100644 --- a/packages/backend/server/src/core/mail/__tests__/job.spec.ts +++ b/packages/backend/server/src/core/mail/__tests__/job.spec.ts @@ -106,6 +106,44 @@ test('should drop expired mail retry', async t => { t.false(send.called); }); +test('should drop time-sensitive mail after its business expiration', async t => { + const send = Sinon.stub(sender, 'send').resolves(true); + + await mailJob.sendMail({ + startTime: Date.now() - 31 * 60 * 1000, + name: 'SignIn', + to: 'expired-sign-in@example.com', + props: { + url: 'https://affine.pro/sign-in', + otp: '123456', + }, + }); + + t.false(send.called); +}); + +test('should use explicit mail expiration when provided', async t => { + const send = Sinon.stub(sender, 'send').resolves(true); + + await mailJob.sendMail({ + startTime: Date.now(), + expiresAt: Date.now() - 1, + name: 'MemberInvitation', + to: 'expired-invitation@example.com', + props: { + user: { + $$userId: 'owner-id', + }, + workspace: { + $$workspaceId: 'workspace-id', + }, + url: 'https://affine.pro/invite/test', + }, + }); + + t.false(send.called); +}); + test('should drop mail retry after max attempts', async t => { const send = Sinon.stub(sender, 'send').resolves(true); @@ -147,3 +185,59 @@ test('should requeue legacy stringified retry mail', async t => { t.true(module.queue.add.calledWith('notification.sendMail', job)); t.is(await cache.mapGet(retryMailKey, cacheKey), undefined); }); + +test('should skip member invitation mail when rendered workspace name contains domain', async t => { + const owner = await module.create(Mockers.User); + const member = await module.create(Mockers.User); + const workspace = await module.create(Mockers.Workspace, { + owner: { id: owner.id }, + name: 'BTC https://spam.example', + }); + const send = Sinon.stub(sender, 'send').resolves(true); + + await mailJob.sendMail({ + startTime: Date.now(), + name: 'MemberInvitation', + to: member.email, + props: { + user: { + $$userId: owner.id, + }, + workspace: { + $$workspaceId: workspace.id, + }, + url: 'https://affine.pro/invite/test', + }, + }); + + t.false(send.called); +}); + +test('should keep dynamic mail props untouched for retry', async t => { + const owner = await module.create(Mockers.User); + const member = await module.create(Mockers.User); + const workspace = await module.create(Mockers.Workspace, { + owner: { id: owner.id }, + name: 'Safe Workspace', + }); + Sinon.stub(sender, 'send').resolves(false); + const job: Jobs['notification.sendMail'] = { + startTime: Date.now(), + name: 'MemberInvitation', + to: member.email, + props: { + user: { + $$userId: owner.id, + }, + workspace: { + $$workspaceId: workspace.id, + }, + url: 'https://affine.pro/invite/test', + }, + }; + + await mailJob.sendMail(job); + + t.deepEqual(job.props.user, { $$userId: owner.id }); + t.deepEqual(job.props.workspace, { $$workspaceId: workspace.id }); +}); diff --git a/packages/backend/server/src/core/mail/job.ts b/packages/backend/server/src/core/mail/job.ts index bd2e08cd8e..434a63d767 100644 --- a/packages/backend/server/src/core/mail/job.ts +++ b/packages/backend/server/src/core/mail/job.ts @@ -8,6 +8,7 @@ import { UserProps, WorkspaceProps } from '../../mails/components'; import { Models } from '../../models'; import { DocReader } from '../doc/reader'; import { WorkspaceBlobStorage } from '../storage'; +import { containsUrlOrDomain } from '../workspaces/abuse'; import { MailSender, SendOptions } from './sender'; type DynamicallyFetchedProps = { @@ -35,7 +36,11 @@ type SendMailJob> = { declare global { interface Jobs { - 'notification.sendMail': { startTime: number; retryCount?: number } & { + 'notification.sendMail': { + startTime: number; + retryCount?: number; + expiresAt?: number; + } & { [K in MailName]: SendMailJob; }[MailName]; } @@ -49,6 +54,17 @@ const retryMaxPerTick = 20; const retryFirstTime = 3; const retryMaxAttempts = 12; const retryMaxAge = 24 * 60 * 60 * 1000; +const magicLinkExpiresIn = 30 * 60 * 1000; + +const mailExpiresIn: Partial> = { + SignIn: magicLinkExpiresIn, + SignUp: magicLinkExpiresIn, + SetPassword: magicLinkExpiresIn, + ChangePassword: magicLinkExpiresIn, + VerifyEmail: magicLinkExpiresIn, + ChangeEmail: magicLinkExpiresIn, + VerifyChangeEmail: magicLinkExpiresIn, +}; @Injectable() export class MailJob { @@ -71,8 +87,12 @@ export class MailJob { private getRetryExhaustedReason({ startTime, retryCount, + expiresAt, + name, }: Jobs['notification.sendMail']) { - if (Date.now() - startTime > retryMaxAge) { + const expiredAt = + expiresAt ?? startTime + (mailExpiresIn[name] ?? retryMaxAge); + if (Date.now() > expiredAt) { return 'expired'; } @@ -118,10 +138,11 @@ export class MailJob { } let options: Partial = {}; + const renderedProps = { ...props }; - for (const key in props) { + for (const key in renderedProps) { // @ts-expect-error allow - const val = props[key]; + const val = renderedProps[key]; if (val && typeof val === 'object') { if ('$$workspaceId' in val) { const workspaceProps = await this.fetchWorkspaceProps( @@ -132,6 +153,16 @@ export class MailJob { return; } + if ( + name === 'MemberInvitation' && + containsUrlOrDomain(workspaceProps.name) + ) { + this.logger.warn( + `Skip mail [${name}] to [${to}], reason=workspace name contains url or domain` + ); + return; + } + if (workspaceProps.avatar) { options.attachments = [ { @@ -144,7 +175,7 @@ export class MailJob { workspaceProps.avatar = 'cid:workspaceAvatar'; } // @ts-expect-error replacement - props[key] = workspaceProps; + renderedProps[key] = workspaceProps; } else if ('$$userId' in val) { const userProps = await this.fetchUserProps(val.$$userId); @@ -153,17 +184,30 @@ export class MailJob { } // @ts-expect-error replacement - props[key] = userProps; + renderedProps[key] = userProps; } } } + if ( + name === 'MemberInvitation' && + 'workspace' in renderedProps && + containsUrlOrDomain( + (renderedProps.workspace as WorkspaceProps | undefined)?.name + ) + ) { + this.logger.warn( + `Skip mail [${name}] to [${to}], reason=workspace name contains url or domain` + ); + return; + } + try { const result = await this.sender.send(name, { to, ...(await Renderers[name]( // @ts-expect-error the job trigger part has been typechecked - props + renderedProps )), ...options, }); diff --git a/packages/backend/server/src/core/notification/__tests__/service.spec.ts b/packages/backend/server/src/core/notification/__tests__/service.spec.ts index dd1f4fd540..ba82af220b 100644 --- a/packages/backend/server/src/core/notification/__tests__/service.spec.ts +++ b/packages/backend/server/src/core/notification/__tests__/service.spec.ts @@ -87,6 +87,29 @@ test('should create invitation notification and email', async t => { t.is(invitationMail.payload.name, 'MemberInvitation'); }); +test('should not send invitation email when workspace name contains domain', async t => { + const spamWorkspace = await module.create(Mockers.Workspace, { + owner: { + id: owner.id, + }, + name: 'BTC https://spam.example', + }); + const inviteId = randomUUID(); + const invitationMailCount = module.queue.count('notification.sendMail'); + + const notification = await notificationService.createInvitation({ + userId: member.id, + body: { + workspaceId: spamWorkspace.id, + createdByUserId: owner.id, + inviteId, + }, + }); + + t.truthy(notification); + t.is(module.queue.count('notification.sendMail'), invitationMailCount); +}); + test('should not send invitation email if user setting is not to receive invitation email', async t => { const inviteId = randomUUID(); await module.create(Mockers.UserSettings, { diff --git a/packages/backend/server/src/core/notification/service.ts b/packages/backend/server/src/core/notification/service.ts index 529d8397b1..7f1509a118 100644 --- a/packages/backend/server/src/core/notification/service.ts +++ b/packages/backend/server/src/core/notification/service.ts @@ -22,6 +22,7 @@ import { generateWorkspaceSettingsPath, WorkspaceSettingsTab, } from '../utils/workspace'; +import { containsUrlOrDomain } from '../workspaces/abuse'; @Injectable() export class NotificationService { @@ -151,6 +152,16 @@ export class NotificationService { } private async sendInvitationEmail(input: InvitationNotificationCreate) { + const workspace = await this.docReader.getWorkspaceContent( + input.body.workspaceId + ); + if (containsUrlOrDomain(workspace?.name)) { + this.logger.warn( + `Skip invitation email for workspace ${input.body.workspaceId}, reason=workspace name contains url or domain` + ); + return; + } + const inviteUrl = this.url.link(`/invite/${input.body.inviteId}`); if (env.dev) { // make it easier to test in dev mode diff --git a/packages/backend/server/src/core/workspaces/abuse.ts b/packages/backend/server/src/core/workspaces/abuse.ts new file mode 100644 index 0000000000..8dccb7a1f7 --- /dev/null +++ b/packages/backend/server/src/core/workspaces/abuse.ts @@ -0,0 +1,12 @@ +export const SHARE_ACTION_ACCOUNT_AGE_MS = 24 * 60 * 60 * 1000; + +const URL_OR_DOMAIN_PATTERN = + /(?:https?:\/\/|www\.|(?= SHARE_ACTION_ACCOUNT_AGE_MS; +} diff --git a/packages/backend/server/src/core/workspaces/resolvers/doc.ts b/packages/backend/server/src/core/workspaces/resolvers/doc.ts index 026e727652..b62081d3a8 100644 --- a/packages/backend/server/src/core/workspaces/resolvers/doc.ts +++ b/packages/backend/server/src/core/workspaces/resolvers/doc.ts @@ -15,6 +15,7 @@ import { PrismaClient } from '@prisma/client'; import { SafeIntResolver } from 'graphql-scalars'; import { + ActionForbidden, Cache, DocActionDenied, DocDefaultRoleCanNotBeOwner, @@ -40,6 +41,7 @@ import { DocRole, } from '../../permission'; import { PublicUserType, WorkspaceUserType } from '../../user'; +import { isUserOldEnoughForShareActions } from '../abuse'; import { WorkspaceType } from '../types'; import { TimeBucket, TimeWindow } from './analytics-types'; import { @@ -299,6 +301,15 @@ export class WorkspaceDocResolver { private readonly cache: Cache ) {} + private async assertCanShare(userId: string) { + const user = await this.models.user.get(userId); + if (!user || !isUserOldEnoughForShareActions(user)) { + throw new ActionForbidden( + 'Sharing links is unavailable during the first 24 hours after signup.' + ); + } + } + @ResolveField(() => WorkspaceDocMeta, { description: 'Cloud page metadata of workspace', complexity: 2, @@ -413,6 +424,7 @@ export class WorkspaceDocResolver { } await this.ac.user(user.id).doc(workspaceId, docId).assert('Doc.Publish'); + await this.assertCanShare(user.id); const doc = await this.models.doc.publish(workspaceId, docId, mode); diff --git a/packages/backend/server/src/core/workspaces/resolvers/member.ts b/packages/backend/server/src/core/workspaces/resolvers/member.ts index 2256089580..92970e7e1d 100644 --- a/packages/backend/server/src/core/workspaces/resolvers/member.ts +++ b/packages/backend/server/src/core/workspaces/resolvers/member.ts @@ -15,6 +15,7 @@ import { import { nanoid } from 'nanoid'; import { + ActionForbidden, ActionForbiddenOnNonTeamWorkspace, AlreadyInSpace, AuthenticationRequired, @@ -40,6 +41,7 @@ import { AccessController, WorkspaceRole } from '../../permission'; import { QuotaService } from '../../quota'; import { UserType } from '../../user'; import { validators } from '../../utils/validators'; +import { containsUrlOrDomain, isUserOldEnoughForShareActions } from '../abuse'; import { WorkspaceService } from '../service'; import { InvitationType, @@ -68,6 +70,24 @@ export class WorkspaceMemberResolver { private readonly quota: QuotaService ) {} + private async assertCanInviteOrShare(userId: string) { + const user = await this.models.user.get(userId); + if (!user || !isUserOldEnoughForShareActions(user)) { + throw new ActionForbidden( + 'Inviting members and creating share links are unavailable during the first 24 hours after signup.' + ); + } + } + + private async assertWorkspaceNameCanInvite(workspaceId: string) { + const workspace = await this.workspaceService.getWorkspaceInfo(workspaceId); + if (containsUrlOrDomain(workspace.name)) { + throw new ActionForbidden( + 'Workspace names containing links or domains cannot be used to invite members.' + ); + } + } + @ResolveField(() => UserType, { description: 'Owner of workspace', complexity: 2, @@ -141,6 +161,8 @@ export class WorkspaceMemberResolver { .user(me.id) .workspace(workspaceId) .assert('Workspace.Users.Manage'); + await this.assertCanInviteOrShare(me.id); + await this.assertWorkspaceNameCanInvite(workspaceId); if (emails.length > 512) { throw new TooManyRequest(); @@ -272,6 +294,8 @@ export class WorkspaceMemberResolver { .user(user.id) .workspace(workspaceId) .assert('Workspace.Users.Manage'); + await this.assertCanInviteOrShare(user.id); + await this.assertWorkspaceNameCanInvite(workspaceId); const cacheWorkspaceId = `workspace:inviteLink:${workspaceId}`; const invite = await this.cache.get<{ inviteId: string }>(cacheWorkspaceId); diff --git a/packages/backend/server/src/mails/index.tsx b/packages/backend/server/src/mails/index.tsx index be72c08aef..e0f597679d 100644 --- a/packages/backend/server/src/mails/index.tsx +++ b/packages/backend/server/src/mails/index.tsx @@ -83,95 +83,70 @@ export const Renderers = { //#region Workspace MemberInvitation: make( Invitation, - props => `${props.user.email} invited you to join ${props.workspace.name}` + 'You were invited to join a workspace on AFFiNE' ), MemberAccepted: make( InvitationAccepted, - props => `${props.user.email} accepted your invitation` - ), - MemberLeave: make( - MemberLeave, - props => `${props.user.email} left ${props.workspace.name}` + 'Your workspace invitation was accepted' ), + MemberLeave: make(MemberLeave, 'A workspace member left'), LinkInvitationReviewRequest: make( LinkInvitationReviewRequest, - props => `New request to join ${props.workspace.name}` + 'New request to join a workspace' ), LinkInvitationApprove: make( LinkInvitationApproved, - props => `Your request to join ${props.workspace.name} has been approved` + 'Your request to join a workspace has been approved' ), LinkInvitationDecline: make( LinkInvitationReviewDeclined, - props => `Your request to join ${props.workspace.name} was declined` - ), - MemberRemoved: make( - MemberRemoved, - props => `You have been removed from ${props.workspace.name}` + 'Your request to join a workspace was declined' ), + MemberRemoved: make(MemberRemoved, 'You have been removed from a workspace'), OwnershipTransferred: make( OwnershipTransferred, - props => `Your ownership of ${props.workspace.name} has been transferred` + 'Your workspace ownership has been transferred' ), OwnershipReceived: make( OwnershipReceived, - props => `You are now the owner of ${props.workspace.name}` + 'You are now the owner of a workspace' ), //#endregion //#region Doc - Mention: make( - Mention, - props => `${props.user.email} mentioned you in ${props.doc.title}` - ), - Comment: make( - Comment, - props => `${props.user.email} commented on ${props.doc.title}` - ), - CommentMention: make( - CommentMention, - props => - `${props.user.email} mentioned you in a comment on ${props.doc.title}` - ), + Mention: make(Mention, 'You were mentioned in AFFiNE'), + Comment: make(Comment, 'New comment in AFFiNE'), + CommentMention: make(CommentMention, 'You were mentioned in a comment'), //#endregion //#region Team TeamWorkspaceUpgraded: make(TeamWorkspaceUpgraded, props => props.isOwner ? 'Your workspace has been upgraded to team workspace! 🎉' - : `${props.workspace.name} has been upgraded to team workspace! 🎉` - ), - TeamBecomeAdmin: make( - TeamBecomeAdmin, - props => `You are now an admin of ${props.workspace.name}` + : 'A workspace has been upgraded to team workspace! 🎉' ), + TeamBecomeAdmin: make(TeamBecomeAdmin, 'You are now a workspace admin'), TeamBecomeCollaborator: make( TeamBecomeCollaborator, - props => `Your role has been changed in ${props.workspace.name}` + 'Your workspace role has been changed' ), TeamDeleteIn24Hours: make( TeamDeleteIn24Hours, - props => - `[Action Required] Final warning: Your workspace ${props.workspace.name} will be deleted in 24 hours` + '[Action Required] Final warning: Your workspace will be deleted in 24 hours' ), TeamDeleteInOneMonth: make( TeamDeleteInOneMonth, - props => - `[Action Required] Important: Your workspace ${props.workspace.name} will be deleted soon` + '[Action Required] Important: Your workspace will be deleted soon' ), TeamWorkspaceDeleted: make( TeamWorkspaceDeleted, - props => `Your workspace ${props.workspace.name} has been deleted` + 'Your workspace has been deleted' ), TeamWorkspaceExpireSoon: make( TeamExpireSoon, - props => - `[Action Required] Your ${props.workspace.name} team workspace will expire soon` - ), - TeamWorkspaceExpired: make( - TeamExpired, - props => `Your ${props.workspace.name} team workspace has expired` + '[Action Required] Your team workspace will expire soon' ), + TeamWorkspaceExpired: make(TeamExpired, 'Your team workspace has expired'), //#endregion //#region License