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
5 changes: 5 additions & 0 deletions client/components/Honeypot/Honeypot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
/>
</label>
);
Expand Down
9 changes: 5 additions & 4 deletions client/containers/UserCreate/UserCreate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type Props = {
const UserCreate = (props: Props) => {
const { signupData } = props;
const altchaRef = useRef<import('components').AltchaRef>(null);
const formStartedAtMsRef = useRef(Date.now());
const [postUserIsLoading, setPostUserIsLoading] = useState(false);
const [postUserError, setPostUserError] = useState<string | undefined>(undefined);
const [subscribed, setSubscribed] = useState(false);
Expand Down Expand Up @@ -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 {
Expand All @@ -61,14 +62,14 @@ const UserCreate = (props: Props) => {
title,
bio,
location,
// useful to check
website: websiteHoneypot,
orcid,
github,
twitter,
facebook,
googleScholar,
_honeypot: confirmPwHoneypot || websiteHoneypot,
_honeypot: websiteHoneypot,
_passwordHoneypot: passwordHoneypot,
_formStartedAtMs: formStartedAtMsRef.current,
altcha: altchaPayload,
gdprConsent: gdprCookiePersistsSignup() ? getGdprConsentElection() : null,
};
Expand Down
134 changes: 133 additions & 1 deletion server/user/__tests__/api.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { vitest } from 'vitest';

import { User } from 'server/models';
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 {}
Expand All @@ -11,10 +17,58 @@ 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 } = vitest.hoisted(() => {
return {
deleteSessionsForUser: vitest.fn().mockResolvedValue(undefined),
};
});

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 () => {
await models.resolve();
});

teardown(afterAll);

describe('/api/users', () => {
Expand Down Expand Up @@ -44,6 +98,84 @@ 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 password 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 } });
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);
Expand Down
39 changes: 36 additions & 3 deletions server/user/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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',
{
Comment on lines +65 to 77
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new fast honeypot check only considers _passwordHoneypot, but the handler still treats any non-empty _honeypot as an immediate restriction. Since the client currently sends the confirmPassword honeypot value via _honeypot, legitimate autofill will still be restricted regardless of timing. Align the server logic so _honeypot represents the website honeypot only, and use _passwordHoneypot + _formStartedAtMs for the confirmPassword timing gate (and consider passing the actual fastHoneypotSignal value into handleHoneypotTriggered for logging).

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch robot

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);
Expand Down
Loading