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
13 changes: 10 additions & 3 deletions client/components/Editor/schemas/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,15 +268,22 @@ export const baseMarks = {
},
],
toDOM: (node) => {
/* Links seem to be recieving a target attr that is a dom element */
/* Links seem to be receiving a target attr that is a dom element */
/* coming from the wrong source in some interfaces. This ensures */
/* only strings can be a target attr. */
const attrs = node.attrs;
const attrs = { ...node.attrs };
if (attrs.target && typeof attrs.target !== 'string') {
attrs.target = null;
}

const { pubEdgeId, ...restAttrs } = attrs;
return ['a', { 'data-pub-edge-id': pubEdgeId, ...restAttrs }] as DOMOutputSpec;
return [
'a',
{
'data-pub-edge-id': pubEdgeId,
...restAttrs,
},
] as DOMOutputSpec;
},
},
sub: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { buttons, FormattingBar } from 'components/FormattingBar';
import { usePubContext } from 'containers/Pub/pubHooks';
import { usePageContext } from 'utils/hooks';

import { commentEditorCustomMarks } from './commentEditorMarks';

type OwnProps = {
discussionData: any;
isPubBottomInput?: boolean;
Expand Down Expand Up @@ -237,6 +239,7 @@ const DiscussionInput = (props: Props) => {
<Editor
key={editorKey}
placeholder={getPlaceholderText(isNewThread, isPubBottomInput)}
customMarks={commentEditorCustomMarks}
onChange={(editorChangeObject) => {
setChangeObject(editorChangeObject);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { buttons, FormattingBar } from 'components/FormattingBar';
import { usePageContext } from 'utils/hooks';
import { getPartsOfFullName } from 'utils/names';

import { commentEditorCustomMarks } from './commentEditorMarks';

import './threadComment.scss';

import type { CommunityBan } from 'server/models';
Expand Down Expand Up @@ -185,6 +187,7 @@ const ThreadComment = (props: Props) => {
key={key}
isReadOnly={isReadOnly}
initialContent={threadCommentData.content}
customMarks={commentEditorCustomMarks}
onChange={onChange}
/>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import type { DOMOutputSpec, MarkSpec } from 'prosemirror-model';

import { baseMarks } from 'components/Editor/schemas/base';

// this just adds a rel=nofollow to the link
const commentLinkMark: MarkSpec = {
...baseMarks.link,
toDOM: (node) => {
const attrs = { ...node.attrs };
const hasInvalidTarget = attrs.target && typeof attrs.target !== 'string';

if (hasInvalidTarget) {
attrs.target = null;
}

const { pubEdgeId, ...restAttrs } = attrs;

return [
'a',
{
'data-pub-edge-id': pubEdgeId,
...restAttrs,
rel: 'nofollow',
},
] as DOMOutputSpec;
},
};

export const commentEditorCustomMarks = {
link: commentLinkMark,
};
91 changes: 46 additions & 45 deletions infra/.env.dev.enc

Large diffs are not rendered by default.

93 changes: 47 additions & 46 deletions infra/.env.enc

Large diffs are not rendered by default.

29 changes: 28 additions & 1 deletion server/discussion/__tests__/api.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import uuid from 'uuid';

import { Discussion, Thread, ThreadComment } from 'server/models';
import { Discussion, SpamTag, Thread, ThreadComment, User } from 'server/models';
import { expectCreatedActivityItem, login, modelize, setup, teardown } from 'stubstub';

const alreadyAppliedManagedLabel = {
Expand Down Expand Up @@ -474,3 +474,30 @@ it('lets admins remove managed labels from discussions', async () => {

expect(discussion.labels).toEqual(targetLabels);
});

it('bans new accounts that post discussion starter comments with links', async () => {
const { guest, releasePub } = models;
const agent = await login(guest);
await agent
.post('/api/discussions')
.send(
makeDiscussion({
pub: releasePub,
text: 'Visit https://spam.example now',
visibilityAccess: 'public',
}),
)
.expect(403);
const user = await User.findOne({
where: { id: guest.id },
include: [{ model: SpamTag, as: 'spamTag' }],
});
expect(user?.spamTag?.status).toEqual('confirmed-spam');
const newAccountLinkCommentTriggers = (user?.spamTag?.fields as any)
?.newAccountLinkCommentTriggers;
expect(newAccountLinkCommentTriggers?.length).toEqual(1);
expect(newAccountLinkCommentTriggers?.[0]).toMatchObject({
source: 'discussion',
value: 'https://spam.example',
});
});
40 changes: 38 additions & 2 deletions server/discussion/api.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Router } from 'express';

import { autoBanForNewAccountLinkComment } from 'server/spamTag/commentSpam';
import { verifyCaptchaPayload } from 'server/utils/captcha';
import { BadRequestError, ForbiddenError } from 'server/utils/errors';
import { handleHoneypotTriggered, isHoneypotFilled } from 'server/utils/honeypot';
Expand All @@ -13,6 +14,7 @@ export const router = Router();

const getRequestIds = (req) => {
const user = req.user || {};

return {
userId: user.id,
discussionId: req.body.discussionId || null,
Expand All @@ -36,9 +38,26 @@ router.post(
if (!canCreate) {
throw new ForbiddenError();
}
const userId = (req.user?.id as string) || null;

const userId: string | undefined | null = req.user?.id ?? null;
const options = { ...req.body, userId };

let isAutoBanned = false;
if (userId) {
isAutoBanned = await autoBanForNewAccountLinkComment({
userId,
text: options.text,
content: options.content,
source: 'discussion',
});
}

if (isAutoBanned) {
throw new ForbiddenError();
}

const newDiscussion = await createDiscussion(options);

return res.status(201).json(newDiscussion);
}),
);
Expand Down Expand Up @@ -72,10 +91,27 @@ router.post(
if (!ok) {
throw new BadRequestError(new Error('Please complete the verification and try again.'));
}
const userId = (req.user?.id as string) || null;

const userId: string | undefined | null = req.user?.id ?? null;
const { altcha: _altcha, _honeypot, ...rest } = req.body;
const options = { ...rest, userId };

let isAutoBanned = false;
if (userId) {
isAutoBanned = await autoBanForNewAccountLinkComment({
userId,
text: options.text,
content: options.content,
source: 'discussion',
});
}

if (isAutoBanned) {
throw new ForbiddenError();
}

const newDiscussion = await createDiscussion(options);

return res.status(201).json(newDiscussion);
}),
);
Expand Down
159 changes: 159 additions & 0 deletions server/spamTag/commentSpam.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
import type { DocJson, NewAccountLinkCommentTriggerSource, UserSpamTagFields } from 'types';

import { type Mark, Node } from 'prosemirror-model';

import { editorSchema } from 'client/components/Editor';
import { SpamTag, User } from 'server/models';
import { contextFromUser, notify } from 'server/spamTag/notifications';
import { upsertSpamTag } from 'server/spamTag/userQueries';

const DEFAULT_NEW_ACCOUNT_LINK_COMMENT_WINDOW_MINUTES = 10;

const parsedWindowMinutes = parseInt(
process.env.NEW_ACCOUNT_LINK_COMMENT_WINDOW_MINUTES ||
DEFAULT_NEW_ACCOUNT_LINK_COMMENT_WINDOW_MINUTES.toString(),
10,
);

const IS_WINDOW_MINUTES_VALID = Number.isFinite(parsedWindowMinutes) && parsedWindowMinutes > 0;

const NEW_ACCOUNT_LINK_COMMENT_WINDOW_MINUTES = IS_WINDOW_MINUTES_VALID
? parsedWindowMinutes
: DEFAULT_NEW_ACCOUNT_LINK_COMMENT_WINDOW_MINUTES;

const NEW_ACCOUNT_LINK_COMMENT_WINDOW_MS = NEW_ACCOUNT_LINK_COMMENT_WINDOW_MINUTES * 60 * 1000;
const URL_REGEX = /\b(?:https?:\/\/|www\.)[^\s<]+/i;
const MAX_TRIGGER_VALUE_LENGTH = 500;

type AutoBanNewAccountLinkCommentOptions = {
userId: string;
text: string;
content: DocJson;
source: NewAccountLinkCommentTriggerSource;
};

const extractUrlFromString = (value: string): string | null => {
if (!value) {
return null;
}

const matchedUrl = value.match(URL_REGEX)?.[0];
if (!matchedUrl) {
return null;
}

return matchedUrl.slice(0, MAX_TRIGGER_VALUE_LENGTH);
};

const hasValidContentShape = (value: unknown): value is DocJson => {
if (!value || typeof value !== 'object') {
return false;
}

const content = value as { type?: unknown };
return typeof content.type === 'string';
};

const extractFirstLinkFromContent = (content: DocJson): string | null => {
if (!hasValidContentShape(content)) {
return null;
}

try {
const contentTree = Node.fromJSON(editorSchema, content);
const links: Mark[] = [];

contentTree.descendants((node) => {
node.marks.forEach((mark) => {
if (mark.type.name === 'link') {
links.push(mark);
}
});
});

const linkFromTree = links[0]?.attrs.href;
if (typeof linkFromTree !== 'string' || !linkFromTree.length) {
return null;
}

return linkFromTree.slice(0, MAX_TRIGGER_VALUE_LENGTH);
} catch {
return null;
}
};

const getAccountAgeMs = (createdAt: Date | null | undefined): number => {
if (!(createdAt instanceof Date)) {
return Number.POSITIVE_INFINITY;
}

return Date.now() - createdAt.getTime();
};

const buildTriggerFields = (
linkValue: string,
source: NewAccountLinkCommentTriggerSource,
accountAgeMs: number,
): UserSpamTagFields => {
return {
newAccountLinkCommentTriggers: [
{
source,
value: linkValue,
accountAgeMinutes: Math.max(0, Math.floor(accountAgeMs / (60 * 1000))),
triggeredAt: new Date().toISOString(),
},
],
};
};

export const autoBanForNewAccountLinkComment = async (
options: AutoBanNewAccountLinkCommentOptions,
): Promise<boolean> => {
const { userId, text, content, source } = options;

const user = await User.findOne({
where: { id: userId },
include: [{ model: SpamTag, as: 'spamTag' }],
});

const accountAgeMs = getAccountAgeMs(user?.createdAt);
const shouldSkipAutoBan = !user || accountAgeMs > NEW_ACCOUNT_LINK_COMMENT_WINDOW_MS;
Comment on lines +115 to +121

if (shouldSkipAutoBan) {
return false;
}

const linkFromTree = extractFirstLinkFromContent(content);
const linkFromText = extractUrlFromString(text);

const firstLink = linkFromTree || linkFromText;

if (!firstLink) {
return false;
}

const previousStatus = user.spamTag?.status ?? null;

const fields = buildTriggerFields(firstLink, source, accountAgeMs);

const { spamTag, user: taggedUser } = await upsertSpamTag({
userId,
status: 'confirmed-spam',
fields,
});

const shouldNotify = previousStatus !== 'confirmed-spam' && process.env.NODE_ENV !== 'test';

if (shouldNotify) {
await notify(
'new-account-link-comment-ban',
contextFromUser(taggedUser, {
previousStatus,
spamFields: spamTag.fields as UserSpamTagFields,
}),
);
}

return true;
};
14 changes: 14 additions & 0 deletions server/spamTag/notifications/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export type SpamEvent =
| 'new-spam-tag'
| 'suspicious-upload'
| 'honeypot-ban'
| 'new-account-link-comment-ban'
| 'manual-ban'
| 'spam-lifted'
| 'community-flag'
Expand Down Expand Up @@ -96,6 +97,7 @@ type Contexts = {
'community-flag': CommunityFlagContext;
'community-flag-retracted': CommunityFlagRetractedContext;
'community-flag-resolved': CommunityFlagResolvedContext;
'new-account-link-comment-ban': NewSpamTagContext;
};

const handlers = {
Expand All @@ -122,6 +124,18 @@ const handlers = {
}),
],
'honeypot-ban': [(ctx) => sendSpamBanEmail({ toEmail: ctx.userEmail, userName: ctx.userName })],
'new-account-link-comment-ban': [
(ctx) => sendSpamBanEmail({ toEmail: ctx.userEmail, userName: ctx.userName }),
(ctx) => sendBanDevEmail({ userEmail: ctx.userEmail, userName: ctx.userName }),
(ctx) =>
postToSlackAboutUserBan({
userId: ctx.userId,
userName: ctx.userName,
userSlug: ctx.userSlug,
userAvatar: ctx.userAvatar,
reason: ctx.spamFields,
}),
],
'manual-ban': [
(ctx) =>
postToSlackAboutUserBan({
Expand Down
Loading
Loading