From e8e96a7e79f55d22307c856bc6bcb1a62c049b4b Mon Sep 17 00:00:00 2001 From: "Thomas F. K. Jorna" Date: Mon, 9 Mar 2026 21:23:37 +0100 Subject: [PATCH 1/5] fix: handle confirmpassword honeypot better --- client/components/Honeypot/Honeypot.tsx | 5 + client/containers/UserCreate/UserCreate.tsx | 6 +- server/user/__tests__/api.test.ts | 140 +++++++++++++++++++- server/user/api.ts | 39 +++++- 4 files changed, 184 insertions(+), 6 deletions(-) diff --git a/client/components/Honeypot/Honeypot.tsx b/client/components/Honeypot/Honeypot.tsx index 88be37c7ab..95d38387e7 100644 --- a/client/components/Honeypot/Honeypot.tsx +++ b/client/components/Honeypot/Honeypot.tsx @@ -34,6 +34,11 @@ const Honeypot = (props: HoneypotProps) => { name={name} autoComplete="off" style={{ width: 80, fontSize: 11, padding: '1px 4px' }} + data-1p-ignore + data-lpignore="true" + data-bwignore + data-form-type="other" + data-protonpass-ignore /> ); diff --git a/client/containers/UserCreate/UserCreate.tsx b/client/containers/UserCreate/UserCreate.tsx index 077ca21cae..cb4f3a2a62 100644 --- a/client/containers/UserCreate/UserCreate.tsx +++ b/client/containers/UserCreate/UserCreate.tsx @@ -17,6 +17,7 @@ type Props = { const UserCreate = (props: Props) => { const { signupData } = props; const altchaRef = useRef(null); + const formStartedAtMsRef = useRef(Date.now()); const [postUserIsLoading, setPostUserIsLoading] = useState(false); const [postUserError, setPostUserError] = useState(undefined); const [subscribed, setSubscribed] = useState(false); @@ -44,8 +45,8 @@ const UserCreate = (props: Props) => { evt.preventDefault(); if (!acceptTerms) return; const formData = new FormData(evt.currentTarget); - const confirmPwHoneypot = formData.get('confirmPassword') as string; const websiteHoneypot = formData.get('website') as string; + const passwordHoneypot = formData.get('confirmPassword') as string; setPostUserIsLoading(true); setPostUserError(undefined); try { @@ -68,7 +69,8 @@ const UserCreate = (props: Props) => { twitter, facebook, googleScholar, - _honeypot: confirmPwHoneypot || websiteHoneypot, + _honeypot: websiteHoneypot || passwordHoneypot, + _formStartedAtMs: formStartedAtMsRef.current, altcha: altchaPayload, gdprConsent: gdprCookiePersistsSignup() ? getGdprConsentElection() : null, }; diff --git a/server/user/__tests__/api.test.ts b/server/user/__tests__/api.test.ts index 8cb8ae0deb..d948be1921 100644 --- a/server/user/__tests__/api.test.ts +++ b/server/user/__tests__/api.test.ts @@ -1,7 +1,13 @@ +/** biome-ignore-all lint/correctness/noUndeclaredVariables: */ import { User } from 'server/models'; +// import { getSpamTagForUser } from 'server/spamTag/userQueries'; import { login, modelize, setup, teardown } from 'stubstub'; const tonyEmail = `${crypto.randomUUID()}@gmail.com`; +const autofillEmail = `${crypto.randomUUID()}@gmail.com`; +const restrictedEmail = `${crypto.randomUUID()}@gmail.com`; +const delayedHoneypotEmail = `${crypto.randomUUID()}@gmail.com`; +const spamEmail = `${crypto.randomUUID()}@gmail.com`; const models = modelize` User user {} @@ -11,10 +17,61 @@ const models = modelize` completed: false count: 1 } + Signup autofillSignup { + email: ${autofillEmail} + hash: "hash-autofill" + completed: false + count: 1 + } + Signup restrictedSignup { + email: ${restrictedEmail} + hash: "hash-restricted" + completed: false + count: 1 + } + Signup delayedHoneypotSignup { + email: ${delayedHoneypotEmail} + hash: "hash-delayed-honeypot" + completed: false + count: 1 + } + Signup spamSignup { + email: ${spamEmail} + hash: "hash-spam" + completed: false + count: 1 + } User suggestionUser {} `; -setup(beforeAll, models.resolve); +const { deleteSessionsForUser } = vi.hoisted(() => { + return { + deleteSessionsForUser: vi.fn().mockResolvedValue(undefined), + }; +}); + +setup(beforeAll, async () => { + vi.mock('server/utils/session', (importOriginal) => { + const og = importOriginal(); + return { + ...og, + deleteSessionsForUser: deleteSessionsForUser, + }; + }); + vi.mock('server/spamTag/notifications', (importOriginal) => { + const og = importOriginal(); + return { + ...og, + contextFromUser: vi.fn().mockReturnValue({ + userId: '123', + userEmail: 'test@test.com', + }), + notify: vi.fn().mockResolvedValue(undefined), + }; + }); + await models.resolve(); +}); + teardown(afterAll); describe('/api/users', () => { @@ -44,6 +101,87 @@ describe('/api/users', () => { expect(userNow?.isSuperAdmin).toEqual(false); }); + it('immediately restricts users when website honeypot is filled', async () => { + const { spamSignup } = models; + const agent = await login(); + await agent + .post('/api/users') + .send({ + email: spamSignup.email, + hash: spamSignup.hash, + firstName: 'Slow', + lastName: 'Fill', + password: 'oh!', + _honeypot: 'oh!', + _formStartedAtMs: Date.now() - 6000, + }) + .expect(403); + const createdUser = await User.findOne({ where: { email: spamSignup.email } }); + expect(createdUser).toBeDefined(); + const { getSpamTagForUser } = await import('server/spamTag/userQueries'); + const spamTag = await getSpamTagForUser(createdUser!.id); + expect(spamTag?.status).toBe('confirmed-spam'); + await agent + .post('/api/login') + .send({ email: createdUser!.email, password: 'oh!' }) + .expect(403); + }); + + it('does not restrict users when honeypot is filled after 5 seconds', async () => { + const { delayedHoneypotSignup } = models; + const agent = await login(); + await agent + .post('/api/users') + .send({ + email: delayedHoneypotSignup.email, + hash: delayedHoneypotSignup.hash, + firstName: 'Slow', + lastName: 'Fill', + password: 'oh!', + _passwordHoneypot: 'oh!', + _formStartedAtMs: Date.now() - 6000, + }) + .expect(201); + const createdUser = await User.findOne({ where: { email: delayedHoneypotSignup.email } }); + if (!createdUser) { + throw new Error('Expected user to be created'); + } + const { getSpamTagForUser } = await import('server/spamTag/userQueries'); + const spamTag = await getSpamTagForUser(createdUser.id); + expect(spamTag).toBeNull(); + await agent + .put('/api/users') + .send({ userId: createdUser.id, firstName: 'Still', lastName: 'Allowed' }) + .expect(201); + }); + + it('restricts and does not authenticate users when website honeypot is filled within 5 seconds', async () => { + const { restrictedSignup } = models; + const agent = await login(); + await agent + .post('/api/users') + .send({ + email: restrictedSignup.email, + hash: restrictedSignup.hash, + firstName: 'Bot', + lastName: 'Account', + password: 'oh!', + _passwordHoneypot: 'oh!', + _formStartedAtMs: Date.now(), + }) + .expect(403); + const createdUser = await User.findOne({ where: { email: restrictedSignup.email } }); + // if (!createdUser) { + // throw new Error('Expected user to be created'); + // } + expect(createdUser).toBeDefined(); + const { getSpamTagForUser } = await import('server/spamTag/userQueries'); + const spamTag = await getSpamTagForUser(createdUser!.id); + expect(spamTag?.status).toEqual('confirmed-spam'); + + expect(deleteSessionsForUser).toHaveBeenCalledWith(createdUser!.email); + }); + it('allows an exisiting user to read another users profile info', async () => { const { user, suggestionUser } = models; const agent = await login(user); diff --git a/server/user/api.ts b/server/user/api.ts index dd44da4141..dd705b7c01 100644 --- a/server/user/api.ts +++ b/server/user/api.ts @@ -14,6 +14,33 @@ import { createUser, getSuggestedEditsUserInfo, updateUser } from './queries'; export const router = Router(); +const ACCOUNT_RESTRICTED_MESSAGE = + 'Your account has been restricted. If you believe this is an error, please contact hello@pubpub.org.'; +const FAST_HONEYPOT_WINDOW_MS = 5_000; + +const getFastHoneypotSignal = ({ + honeypot, + formStartedAtMs, + nowMs = Date.now(), +}: { + honeypot: unknown; + formStartedAtMs: unknown; + nowMs?: number; +}): string | null => { + const honeypotValue = typeof honeypot === 'string' ? honeypot.trim() : ''; + + if (honeypotValue.length === 0) return null; + + const parsedFormStartedAtMs = Number(formStartedAtMs); + + if (!Number.isFinite(parsedFormStartedAtMs)) return null; + + const elapsedMs = nowMs - parsedFormStartedAtMs; + if (elapsedMs < 0 || elapsedMs > FAST_HONEYPOT_WINDOW_MS) return null; + + return honeypotValue; +}; + const getRequestIds = (req) => { const user = req.user || {}; return { @@ -35,18 +62,24 @@ router.post('/api/users', async (req, res) => { if (!ok) { return res.status(400).json('Please complete the verification and try again.'); } - const { altcha, _honeypot, website, ...body } = { ...req.body }; + const { altcha, _honeypot, _passwordHoneypot, _formStartedAtMs, ...body } = { ...req.body }; + const fastHoneypotSignal = getFastHoneypotSignal({ + honeypot: _passwordHoneypot, + formStartedAtMs: _formStartedAtMs, + }); + const newUser = await createUser(body); - if (isHoneypotFilled(req.body)) { + if (fastHoneypotSignal || _honeypot) { await handleHoneypotTriggered( newUser.id, 'create-user', - req.body.website ?? 'confirmPassword honeypot', + _honeypot || 'confirmPassword + very fast', { communitySubdomain: req.headers.host?.split('.')[0], content: req.body.fullName ? `name: ${req.body.fullName}` : undefined, }, ); + return res.status(403).json(ACCOUNT_RESTRICTED_MESSAGE); } passport.authenticate('local')(req, res, () => { const hashedUserId = getHashedUserId(newUser); From cae20927931ea50e86cae10189a83ae7b1fbd7ac Mon Sep 17 00:00:00 2001 From: "Thomas F. K. Jorna" Date: Thu, 12 Mar 2026 13:29:03 +0100 Subject: [PATCH 2/5] fix: use importOriginal together --- server/user/__tests__/api.test.ts | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/server/user/__tests__/api.test.ts b/server/user/__tests__/api.test.ts index d948be1921..8e8a4f026e 100644 --- a/server/user/__tests__/api.test.ts +++ b/server/user/__tests__/api.test.ts @@ -1,6 +1,6 @@ -/** biome-ignore-all lint/correctness/noUndeclaredVariables: */ +import { vitest } from 'vitest'; + import { User } from 'server/models'; -// import { getSpamTagForUser } from 'server/spamTag/userQueries'; import { login, modelize, setup, teardown } from 'stubstub'; const tonyEmail = `${crypto.randomUUID()}@gmail.com`; @@ -44,29 +44,25 @@ const models = modelize` User suggestionUser {} `; -const { deleteSessionsForUser } = vi.hoisted(() => { +const { deleteSessionsForUser } = vitest.hoisted(() => { return { - deleteSessionsForUser: vi.fn().mockResolvedValue(undefined), + deleteSessionsForUser: vitest.fn().mockResolvedValue(undefined), }; }); setup(beforeAll, async () => { - vi.mock('server/utils/session', (importOriginal) => { - const og = importOriginal(); + vitest.mock(import('server/utils/session'), async (importOriginal) => { + const og = await importOriginal(); return { ...og, deleteSessionsForUser: deleteSessionsForUser, }; }); - vi.mock('server/spamTag/notifications', (importOriginal) => { - const og = importOriginal(); + vitest.mock(import('server/spamTag/notifications'), async (importOriginal) => { + const og = await importOriginal(); return { ...og, - contextFromUser: vi.fn().mockReturnValue({ - userId: '123', - userEmail: 'test@test.com', - }), - notify: vi.fn().mockResolvedValue(undefined), + notify: vitest.fn().mockResolvedValue(undefined), }; }); await models.resolve(); From f03fefc1558ef77ee94871aec346cf9d78f8d1df Mon Sep 17 00:00:00 2001 From: "Thomas F. K. Jorna" Date: Thu, 12 Mar 2026 13:32:39 +0100 Subject: [PATCH 3/5] fix: make the honeypot work correctly --- client/containers/UserCreate/UserCreate.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/client/containers/UserCreate/UserCreate.tsx b/client/containers/UserCreate/UserCreate.tsx index cb4f3a2a62..9960fdba17 100644 --- a/client/containers/UserCreate/UserCreate.tsx +++ b/client/containers/UserCreate/UserCreate.tsx @@ -62,14 +62,13 @@ const UserCreate = (props: Props) => { title, bio, location, - // useful to check - website: websiteHoneypot, orcid, github, twitter, facebook, googleScholar, - _honeypot: websiteHoneypot || passwordHoneypot, + _honeypot: websiteHoneypot, + _passwordHoneypot: passwordHoneypot, _formStartedAtMs: formStartedAtMsRef.current, altcha: altchaPayload, gdprConsent: gdprCookiePersistsSignup() ? getGdprConsentElection() : null, From 8bb848537773768eed08fbfbc27d5dbe1d63923f Mon Sep 17 00:00:00 2001 From: "Thomas F. K. Jorna" Date: Thu, 12 Mar 2026 13:33:46 +0100 Subject: [PATCH 4/5] fix: move mocks outside --- server/user/__tests__/api.test.ts | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/server/user/__tests__/api.test.ts b/server/user/__tests__/api.test.ts index 8e8a4f026e..980cd41d51 100644 --- a/server/user/__tests__/api.test.ts +++ b/server/user/__tests__/api.test.ts @@ -50,21 +50,22 @@ const { deleteSessionsForUser } = vitest.hoisted(() => { }; }); +vitest.mock(import('server/utils/session'), async (importOriginal) => { + const og = await importOriginal(); + return { + ...og, + deleteSessionsForUser: deleteSessionsForUser, + }; +}); +vitest.mock(import('server/spamTag/notifications'), async (importOriginal) => { + const og = await importOriginal(); + return { + ...og, + notify: vitest.fn().mockResolvedValue(undefined), + }; +}); + setup(beforeAll, async () => { - vitest.mock(import('server/utils/session'), async (importOriginal) => { - const og = await importOriginal(); - return { - ...og, - deleteSessionsForUser: deleteSessionsForUser, - }; - }); - vitest.mock(import('server/spamTag/notifications'), async (importOriginal) => { - const og = await importOriginal(); - return { - ...og, - notify: vitest.fn().mockResolvedValue(undefined), - }; - }); await models.resolve(); }); From ccfcfda8a492742f337c254490d5c20a4239dced Mon Sep 17 00:00:00 2001 From: "Thomas F. K. Jorna" Date: Thu, 12 Mar 2026 13:34:47 +0100 Subject: [PATCH 5/5] fix: make tests more clear --- server/user/__tests__/api.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/server/user/__tests__/api.test.ts b/server/user/__tests__/api.test.ts index 980cd41d51..be521af579 100644 --- a/server/user/__tests__/api.test.ts +++ b/server/user/__tests__/api.test.ts @@ -152,7 +152,7 @@ describe('/api/users', () => { .expect(201); }); - it('restricts and does not authenticate users when website honeypot is filled within 5 seconds', async () => { + it('restricts and does not authenticate users when password honeypot is filled within 5 seconds', async () => { const { restrictedSignup } = models; const agent = await login(); await agent @@ -168,9 +168,6 @@ describe('/api/users', () => { }) .expect(403); const createdUser = await User.findOne({ where: { email: restrictedSignup.email } }); - // if (!createdUser) { - // throw new Error('Expected user to be created'); - // } expect(createdUser).toBeDefined(); const { getSpamTagForUser } = await import('server/spamTag/userQueries'); const spamTag = await getSpamTagForUser(createdUser!.id);