Skip to content

Commit 89e7a75

Browse files
florianbgtflorianbgt
authored
Merge pull request #2 from nanoapi-io/feature/prevent-user-from-spaming-login
prevent use from spamming login
2 parents 55433a7 + e9371fb commit 89e7a75

File tree

8 files changed

+256
-72
lines changed

8 files changed

+256
-72
lines changed

‎packages/app/src/pages/login.tsx‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,20 @@ export default function LoginPage() {
7979
body,
8080
);
8181

82+
if (!response.ok && response.status === 400) {
83+
const { error } = await response.json() as { error: string };
84+
if (error === "otp_already_requested") {
85+
toast.error(
86+
"One time password already requested, please wait and try again later",
87+
);
88+
setIsBusy(false);
89+
return;
90+
}
91+
throw new Error(error);
92+
}
93+
8294
if (!response.ok || response.status !== 200) {
83-
const data = await response.json();
84-
throw new Error(data.error);
95+
throw new Error("Failed to send one time password");
8596
}
8697

8798
setStep("otp");

‎packages/core/src/api/auth/router.test.ts‎

Lines changed: 191 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,17 @@ Deno.test("request otp for existing user", async () => {
115115
.where("id", "=", userId)
116116
.executeTakeFirstOrThrow();
117117

118+
// set otp_requested_at in the past to make sure the user can request an OTP again
119+
await db
120+
.updateTable("user")
121+
.set({
122+
otp_requested_at: new Date(
123+
Date.now() - settings.OTP.REQUEST_INTERVAL_SECONDS * 1000,
124+
),
125+
})
126+
.where("id", "=", userId)
127+
.execute();
128+
118129
assertEquals(user.email, email);
119130
assertEquals(user.otp, null);
120131
assertEquals(user.otp_expires_at, null);
@@ -159,7 +170,12 @@ Deno.test("verify otp for new user", async () => {
159170
const email = `test-${crypto.randomUUID()}@example.com`;
160171

161172
const authService = new AuthService();
162-
const otp = await authService.requestOtp(email);
173+
await authService.requestOtp(email);
174+
const { otp } = await db.selectFrom("user")
175+
.where("email", "=", email)
176+
.select("otp")
177+
.executeTakeFirstOrThrow();
178+
if (!otp) throw new Error("Failed to get OTP");
163179

164180
const requestInfo = prepareVerifyOtp({ email, otp });
165181

@@ -229,6 +245,74 @@ Deno.test("verify otp for new user", async () => {
229245
}
230246
});
231247

248+
Deno.test("request otp - should prevent spam requests within interval", async () => {
249+
initKyselyDb();
250+
await resetTables();
251+
252+
try {
253+
const email = `test-${crypto.randomUUID()}@example.com`;
254+
255+
// First OTP request should succeed
256+
const firstRequestInfo = prepareRequestOtp({ email });
257+
const firstResponse = await api.handle(
258+
new Request(`http://localhost:3000${firstRequestInfo.url}`, {
259+
method: firstRequestInfo.method,
260+
body: JSON.stringify(firstRequestInfo.body),
261+
headers: {
262+
"Content-Type": "application/json",
263+
},
264+
}),
265+
);
266+
267+
assertEquals(firstResponse!.status, 200);
268+
const firstResponseBody = await firstResponse?.json();
269+
assertEquals(firstResponseBody.message, "OTP sent successfully");
270+
271+
// Verify user was created with OTP
272+
const user = await db
273+
.selectFrom("user")
274+
.selectAll()
275+
.where("email", "=", email)
276+
.executeTakeFirst();
277+
278+
assertNotEquals(user, undefined);
279+
assertNotEquals(user?.otp, null);
280+
assertNotEquals(user?.otp_requested_at, null);
281+
282+
// Second OTP request immediately after should be rejected
283+
const secondRequestInfo = prepareRequestOtp({ email });
284+
const secondResponse = await api.handle(
285+
new Request(`http://localhost:3000${secondRequestInfo.url}`, {
286+
method: secondRequestInfo.method,
287+
body: JSON.stringify(secondRequestInfo.body),
288+
headers: {
289+
"Content-Type": "application/json",
290+
},
291+
}),
292+
);
293+
294+
assertEquals(secondResponse!.status, 400);
295+
const secondResponseBody = await secondResponse?.json();
296+
assertEquals(secondResponseBody.error, "otp_already_requested");
297+
298+
// Verify the OTP and timestamp weren't updated
299+
const userAfterSecondRequest = await db
300+
.selectFrom("user")
301+
.selectAll()
302+
.where("email", "=", email)
303+
.executeTakeFirst();
304+
305+
assertEquals(userAfterSecondRequest?.otp, user?.otp);
306+
assertEquals(
307+
userAfterSecondRequest?.otp_requested_at?.getTime(),
308+
user?.otp_requested_at?.getTime(),
309+
);
310+
} finally {
311+
await resetTables();
312+
await destroyKyselyDb();
313+
}
314+
});
315+
232316
Deno.test("verify otp for existing user", async () => {
233317
initKyselyDb();
234318
await resetTables();
@@ -270,8 +354,24 @@ Deno.test("verify otp for existing user", async () => {
270354

271355
assertEquals(personalWorkspaceMember.role, ADMIN_ROLE);
272356

357+
// set otp_requested_at in the past to make sure the user can request an OTP again
358+
await db
359+
.updateTable("user")
360+
.set({
361+
otp_requested_at: new Date(
362+
Date.now() - settings.OTP.REQUEST_INTERVAL_SECONDS * 1000,
363+
),
364+
})
365+
.where("id", "=", userId)
366+
.execute();
367+
273368
const authService = new AuthService();
274-
const otp = await authService.requestOtp(email);
369+
await authService.requestOtp(email);
370+
const { otp } = await db.selectFrom("user")
371+
.where("email", "=", email)
372+
.select("otp")
373+
.executeTakeFirstOrThrow();
374+
if (!otp) throw new Error("Failed to get OTP");
275375

276376
const requestInfo = prepareVerifyOtp({ email, otp });
277377

@@ -332,7 +432,13 @@ Deno.test("verify otp - should block after max failed attempts", async () => {
332432
const authService = new AuthService();
333433

334434
// Request OTP
335-
const validOtp = await authService.requestOtp(email);
435+
await authService.requestOtp(email);
436+
const { otp: validOtp } = await db.selectFrom("user")
437+
.where("email", "=", email)
438+
.select("otp")
439+
.executeTakeFirstOrThrow();
440+
if (!validOtp) throw new Error("Failed to get OTP");
441+
336442
const wrongOtp = "000000"; // Wrong OTP
337443

338444
// Make 3 failed attempts (assuming MAX_ATTEMPTS is 3)
@@ -391,79 +497,98 @@ Deno.test("verify otp - should block after max failed attempts", async () => {
391497
}
392498
});
393499

394-
Deno.test("verify otp - should reset attempts on new OTP request", async () => {
395-
initKyselyDb();
396-
await resetTables();
500+
Deno.test(
501+
"verify otp - should reset attempts on new OTP request",
502+
async () => {
503+
initKyselyDb();
504+
await resetTables();
397505

398-
try {
399-
const email = `test-${crypto.randomUUID()}@example.com`;
506+
try {
507+
const email = `test-${crypto.randomUUID()}@example.com`;
400508

401-
// Request initial OTP
402-
const requestOtpInfo = prepareRequestOtp({ email });
403-
await api.handle(
404-
new Request(`http://localhost:3000${requestOtpInfo.url}`, {
405-
method: requestOtpInfo.method,
406-
body: JSON.stringify(requestOtpInfo.body),
407-
headers: {
408-
"Content-Type": "application/json",
409-
},
410-
}),
411-
);
412-
413-
// Make some failed attempts
414-
const wrongOtp = "000000";
415-
for (let i = 0; i < 2; i++) {
416-
const verifyInfo = prepareVerifyOtp({ email, otp: wrongOtp });
509+
// Request initial OTP
510+
const requestOtpInfo = prepareRequestOtp({ email });
417511
await api.handle(
418-
new Request(`http://localhost:3000${verifyInfo.url}`, {
419-
method: verifyInfo.method,
420-
body: JSON.stringify(verifyInfo.body),
512+
new Request(`http://localhost:3000${requestOtpInfo.url}`, {
513+
method: requestOtpInfo.method,
514+
body: JSON.stringify(requestOtpInfo.body),
421515
headers: {
422516
"Content-Type": "application/json",
423517
},
424518
}),
425519
);
426-
}
427-
428-
// Verify attempts were recorded
429-
let user = await db
430-
.selectFrom("user")
431-
.selectAll()
432-
.where("email", "=", email)
433-
.executeTakeFirstOrThrow();
434-
assertEquals(user.otp_attempts, 2);
435-
436-
// Request new OTP - should reset attempts
437-
const authService = new AuthService();
438-
const newOtp = await authService.requestOtp(email);
439-
440-
user = await db
441-
.selectFrom("user")
442-
.selectAll()
443-
.where("email", "=", email)
444-
.executeTakeFirstOrThrow();
445-
assertEquals(user.otp_attempts, 0); // Should be reset
446520

447-
// Should be able to verify with new OTP
448-
const newVerifyInfo = prepareVerifyOtp({ email, otp: newOtp });
449-
const response = await api.handle(
450-
new Request(`http://localhost:3000${newVerifyInfo.url}`, {
451-
method: newVerifyInfo.method,
452-
body: JSON.stringify(newVerifyInfo.body),
453-
headers: {
454-
"Content-Type": "application/json",
455-
},
456-
}),
457-
);
521+
// Make some failed attempts
522+
const wrongOtp = "000000";
523+
for (let i = 0; i < 2; i++) {
524+
const verifyInfo = prepareVerifyOtp({ email, otp: wrongOtp });
525+
await api.handle(
526+
new Request(`http://localhost:3000${verifyInfo.url}`, {
527+
method: verifyInfo.method,
528+
body: JSON.stringify(verifyInfo.body),
529+
headers: {
530+
"Content-Type": "application/json",
531+
},
532+
}),
533+
);
534+
}
535+
536+
// Verify attempts were recorded
537+
let user = await db
538+
.selectFrom("user")
539+
.selectAll()
540+
.where("email", "=", email)
541+
.executeTakeFirstOrThrow();
542+
assertEquals(user.otp_attempts, 2);
543+
544+
// set otp_requested_at in the past to make sure the user can request an OTP again
545+
await db
546+
.updateTable("user")
547+
.set({
548+
otp_requested_at: new Date(
549+
Date.now() - settings.OTP.REQUEST_INTERVAL_SECONDS * 1000,
550+
),
551+
})
552+
.where("id", "=", user.id)
553+
.execute();
554+
555+
// Request new OTP - should reset attempts
556+
const authService = new AuthService();
557+
await authService.requestOtp(email);
558+
const { otp } = await db.selectFrom("user")
559+
.where("email", "=", email)
560+
.selectAll()
561+
.executeTakeFirstOrThrow();
562+
if (!otp) throw new Error("Failed to get OTP");
563+
564+
user = await db
565+
.selectFrom("user")
566+
.selectAll()
567+
.where("email", "=", email)
568+
.executeTakeFirstOrThrow();
569+
assertEquals(user.otp_attempts, 0); // Should be reset
570+
571+
// Should be able to verify with new OTP
572+
const newVerifyInfo = prepareVerifyOtp({ email, otp });
573+
const response = await api.handle(
574+
new Request(`http://localhost:3000${newVerifyInfo.url}`, {
575+
method: newVerifyInfo.method,
576+
body: JSON.stringify(newVerifyInfo.body),
577+
headers: {
578+
"Content-Type": "application/json",
579+
},
580+
}),
581+
);
458582

459-
assertEquals(response!.status, 200);
460-
const responseBody = await response!.json();
461-
assertGreater(responseBody.token.length, 0);
462-
} finally {
463-
await resetTables();
464-
await destroyKyselyDb();
465-
}
466-
});
583+
assertEquals(response!.status, 200);
584+
const responseBody = await response!.json();
585+
assertGreater(responseBody.token.length, 0);
586+
} finally {
587+
await resetTables();
588+
await destroyKyselyDb();
589+
}
590+
},
591+
);
467592

468593
Deno.test("verify otp - should track failed attempts correctly", async () => {
469594
initKyselyDb();

‎packages/core/src/api/auth/router.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,13 @@ router.post("/requestOtp", async (ctx) => {
1818
}
1919

2020
const authService = new AuthService();
21-
await authService.requestOtp(parsedBody.data.email);
21+
const response = await authService.requestOtp(parsedBody.data.email);
22+
23+
if (response.error) {
24+
ctx.response.status = Status.BadRequest;
25+
ctx.response.body = { error: response.error };
26+
return;
27+
}
2228

2329
ctx.response.status = Status.OK;
2430
ctx.response.body = { message: "OTP sent successfully" };

0 commit comments

Comments
 (0)