diff --git a/plugins/email/server/auth/email.ts b/plugins/email/server/auth/email.ts index 2e7242962f..c418e0d64b 100644 --- a/plugins/email/server/auth/email.ts +++ b/plugins/email/server/auth/email.ts @@ -15,6 +15,7 @@ import { RateLimiterStrategy } from "@server/utils/RateLimiter"; import { VerificationCode } from "@server/utils/VerificationCode"; import { signIn } from "@server/utils/authentication"; import { getUserForEmailSigninToken } from "@server/utils/jwt"; +import { getTeamFromContext } from "@server/utils/passport"; import * as T from "./schema"; import { CSRF } from "@shared/constants"; @@ -128,28 +129,33 @@ const emailCallback = async (ctx: APIContext) => { return ctx.redirectOnClient(url.toString(), "POST"); } - let user!: User; + let user: User | null = null; try { if (token) { user = await getUserForEmailSigninToken(ctx, token as string); } else if (code && email) { + const team = await getTeamFromContext(ctx); + + if (!team) { + ctx.redirect("/?notice=auth-error&description=Unknown%20team"); + return; + } + user = await User.scope("withTeam").findOne({ - rejectOnEmpty: true, where: { + teamId: team.id, email: email.trim().toLowerCase(), }, }); - const isValid = await VerificationCode.verify(email, code); - - if (!isValid) { + if (!user || !(await VerificationCode.verify(team.id, email, code))) { ctx.redirect(`/?notice=invalid-code`); return; } // Delete the code after successful verification - await VerificationCode.delete(email); + await VerificationCode.delete(team.id, email); } else { ctx.redirect("/?notice=auth-error&description=Missing%20token"); return; @@ -159,6 +165,10 @@ const emailCallback = async (ctx: APIContext) => { return ctx.redirect(`/?notice=auth-error&description=${err.message}`); } + if (!user) { + return ctx.redirect(`/?notice=invalid-code`); + } + if (!user.team.emailSigninEnabled) { return ctx.redirect( "/?notice=auth-error&description=Disabled%20signin%20method" diff --git a/server/models/User.ts b/server/models/User.ts index d181d29fb5..34cd9ee4d7 100644 --- a/server/models/User.ts +++ b/server/models/User.ts @@ -683,7 +683,7 @@ class User extends ParanoidModel< } const code = VerificationCode.generate(); - await VerificationCode.store(this.email, code); + await VerificationCode.store(this.teamId, this.email, code); return code; }; diff --git a/server/utils/VerificationCode.test.ts b/server/utils/VerificationCode.test.ts index 5eb15397f9..a0bb22bdcf 100644 --- a/server/utils/VerificationCode.test.ts +++ b/server/utils/VerificationCode.test.ts @@ -1,10 +1,13 @@ import { VerificationCode } from "./VerificationCode"; describe("VerificationCode", () => { + const teamId = "team-1"; + const otherTeamId = "team-2"; const email = "user@example.com"; afterEach(async () => { - await VerificationCode.delete(email); + await VerificationCode.delete(teamId, email); + await VerificationCode.delete(otherTeamId, email); }); describe("generate", () => { @@ -17,88 +20,114 @@ describe("VerificationCode", () => { describe("store and retrieve", () => { it("should store and retrieve a code", async () => { const code = "123456"; - await VerificationCode.store(email, code); - const retrieved = await VerificationCode.retrieve(email); + await VerificationCode.store(teamId, email, code); + const retrieved = await VerificationCode.retrieve(teamId, email); expect(retrieved).toBe(code); }); it("should return undefined for an unknown email", async () => { - const retrieved = await VerificationCode.retrieve("unknown@example.com"); + const retrieved = await VerificationCode.retrieve( + teamId, + "unknown@example.com" + ); expect(retrieved).toBeUndefined(); }); it("should be case-insensitive for email", async () => { const code = "654321"; - await VerificationCode.store("User@Example.COM", code); - const retrieved = await VerificationCode.retrieve("user@example.com"); + await VerificationCode.store(teamId, "User@Example.COM", code); + const retrieved = await VerificationCode.retrieve( + teamId, + "user@example.com" + ); expect(retrieved).toBe(code); - await VerificationCode.delete("user@example.com"); + await VerificationCode.delete(teamId, "user@example.com"); + }); + + it("should isolate codes between teams for the same email", async () => { + await VerificationCode.store(teamId, email, "111111"); + await VerificationCode.store(otherTeamId, email, "222222"); + + expect(await VerificationCode.retrieve(teamId, email)).toBe("111111"); + expect(await VerificationCode.retrieve(otherTeamId, email)).toBe( + "222222" + ); }); }); describe("verify", () => { it("should return true for a matching code", async () => { const code = "123456"; - await VerificationCode.store(email, code); - const result = await VerificationCode.verify(email, code); + await VerificationCode.store(teamId, email, code); + const result = await VerificationCode.verify(teamId, email, code); expect(result).toBe(true); }); it("should return false for an incorrect code", async () => { - await VerificationCode.store(email, "123456"); - const result = await VerificationCode.verify(email, "000000"); + await VerificationCode.store(teamId, email, "123456"); + const result = await VerificationCode.verify(teamId, email, "000000"); expect(result).toBe(false); }); it("should return false when no code is stored", async () => { - const result = await VerificationCode.verify(email, "123456"); + const result = await VerificationCode.verify(teamId, email, "123456"); + expect(result).toBe(false); + }); + + it("should not verify a code issued for a different team", async () => { + await VerificationCode.store(teamId, email, "123456"); + const result = await VerificationCode.verify( + otherTeamId, + email, + "123456" + ); expect(result).toBe(false); }); it("should delete the code after exceeding max attempts", async () => { - await VerificationCode.store(email, "123456"); + await VerificationCode.store(teamId, email, "123456"); // Exhaust all 10 allowed attempts with wrong codes for (let i = 0; i < 10; i++) { - await VerificationCode.verify(email, "000000"); + await VerificationCode.verify(teamId, email, "000000"); } // 11th attempt should fail even with the correct code - const result = await VerificationCode.verify(email, "123456"); + const result = await VerificationCode.verify(teamId, email, "123456"); expect(result).toBe(false); // Code should be deleted from storage - const stored = await VerificationCode.retrieve(email); + const stored = await VerificationCode.retrieve(teamId, email); expect(stored).toBeUndefined(); }); }); describe("delete", () => { it("should remove the stored code", async () => { - await VerificationCode.store(email, "123456"); - await VerificationCode.delete(email); - const retrieved = await VerificationCode.retrieve(email); + await VerificationCode.store(teamId, email, "123456"); + await VerificationCode.delete(teamId, email); + const retrieved = await VerificationCode.retrieve(teamId, email); expect(retrieved).toBeUndefined(); }); it("should reset the attempt counter", async () => { - await VerificationCode.store(email, "123456"); + await VerificationCode.store(teamId, email, "123456"); // Use some attempts for (let i = 0; i < 5; i++) { - await VerificationCode.verify(email, "000000"); + await VerificationCode.verify(teamId, email, "000000"); } // Delete and re-store - await VerificationCode.delete(email); - await VerificationCode.store(email, "654321"); + await VerificationCode.delete(teamId, email); + await VerificationCode.store(teamId, email, "654321"); // Should have a fresh set of attempts for (let i = 0; i < 9; i++) { - await VerificationCode.verify(email, "000000"); + await VerificationCode.verify(teamId, email, "000000"); } - const result = await VerificationCode.verify(email, "654321"); + const result = await VerificationCode.verify(teamId, email, "654321"); expect(result).toBe(true); }); }); diff --git a/server/utils/VerificationCode.ts b/server/utils/VerificationCode.ts index 19589870fd..d870106275 100644 --- a/server/utils/VerificationCode.ts +++ b/server/utils/VerificationCode.ts @@ -48,12 +48,17 @@ export class VerificationCode { /** * Store a verification code in Redis with a 10-minute TTL * + * @param teamId The team the code is being issued for * @param email The email address associated with the code * @param code The 6-digit verification code * @returns Promise resolving to true if successful */ - public static async store(email: string, code: string): Promise { - const key = this.getKey(email); + public static async store( + teamId: string, + email: string, + code: string + ): Promise { + const key = this.getKey(teamId, email); await this.redis.set(key, code, "PX", this.TTL); return true; } @@ -61,29 +66,38 @@ export class VerificationCode { /** * Retrieve a verification code from Redis * + * @param teamId The team the code was issued for * @param email The email address associated with the code - * @returns Promise resolving to the code or null if not found + * @returns Promise resolving to the code or undefined if not found */ - public static async retrieve(email: string): Promise { - const key = this.getKey(email); + public static async retrieve( + teamId: string, + email: string + ): Promise { + const key = this.getKey(teamId, email); return (await this.redis.get(key)) ?? undefined; } /** - * Verify if a given code matches the stored code for an email + * Verify if a given code matches the stored code for an email within a team. * + * @param teamId The team the code was issued for * @param email The email address associated with the code * @param code The code to verify * @returns Promise resolving to true if the code matches, false otherwise */ - public static async verify(email: string, code: string): Promise { - const storedCode = await this.retrieve(email); + public static async verify( + teamId: string, + email: string, + code: string + ): Promise { + const storedCode = await this.retrieve(teamId, email); if (!storedCode) { return false; } - const attemptsKey = this.getAttemptsKey(email); + const attemptsKey = this.getAttemptsKey(teamId, email); const attempts = await this.redis.incr(attemptsKey); if (attempts === 1) { @@ -91,7 +105,7 @@ export class VerificationCode { } if (attempts > this.MAX_ATTEMPTS) { - await this.delete(email); + await this.delete(teamId, email); return false; } @@ -101,33 +115,36 @@ export class VerificationCode { /** * Delete a verification code from Redis * + * @param teamId The team the code was issued for * @param email The email address associated with the code * @returns Promise resolving to true if successful */ - public static async delete(email: string): Promise { - const key = this.getKey(email); - const attemptsKey = this.getAttemptsKey(email); + public static async delete(teamId: string, email: string): Promise { + const key = this.getKey(teamId, email); + const attemptsKey = this.getAttemptsKey(teamId, email); await this.redis.del(key, attemptsKey); return true; } /** - * Get the Redis key for an email address + * Get the Redis key for a code scoped to a team and email address. * + * @param teamId The team the code was issued for * @param email The email address * @returns The Redis key */ - private static getKey(email: string): string { - return `${this.KEY_PREFIX}${email.toLowerCase()}`; + private static getKey(teamId: string, email: string): string { + return `${this.KEY_PREFIX}${teamId}:${email.trim().toLowerCase()}`; } /** * Get the Redis key for tracking verification attempts. * + * @param teamId The team the code was issued for * @param email The email address. * @returns the Redis key for attempts. */ - private static getAttemptsKey(email: string): string { - return `${this.ATTEMPTS_PREFIX}${email.toLowerCase()}`; + private static getAttemptsKey(teamId: string, email: string): string { + return `${this.ATTEMPTS_PREFIX}${teamId}:${email.trim().toLowerCase()}`; } }