Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(auth): prevent email enumeration during auth flows #1540

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
9 changes: 5 additions & 4 deletions apps/web/public/locales/en/app.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
"preferences": "Preferences",
"previousMonth": "Previous month",
"register": "Register",
"requiredString": "{name} is required",
"requiredString": "{name} is required",
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this change is necessary or relevant to this PR. Can you please revert this?

"response": "Response",
"save": "Save",
"saveInstruction": "Select your availability and click <b>{action}</b>",
Expand All @@ -72,7 +72,6 @@
"title": "Title",
"titlePlaceholder": "Monthly Meetup",
"today": "Today",
"userAlreadyExists": "A user with that email already exists",
"validEmail": "Please enter a valid email",
"verificationCodeHelp": "Didn't get the email? Check your spam/junk.",
"verificationCodePlaceholder": "Enter your 6-digit code",
Expand Down Expand Up @@ -304,6 +303,8 @@
"registerVerifyTitle": "Finish Registering",
"registerVerifyDescription": "Check your email for the verification code",
"loginVerifyTitle": "Finish Logging In",
"loginVerifyDescription": "Check your email for the verification code",
"createAccount": "Create Account"
"loginVerifyDescription": "If an account exists, a verification code will be sent to your email",
"createAccount": "Create Account",
"verifyEmailTitle": "Verify Your Email",
"verifyEmailDescription": "Please check your email for a verification code"
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,26 +49,15 @@ export function LoginWithEmailForm() {
<form
className="space-y-4"
onSubmit={handleSubmit(async ({ identifier }) => {
const doesExist = await setVerificationEmail(identifier);
if (doesExist) {
await signIn("email", {
email: identifier,
callbackUrl: searchParams?.get("callbackUrl") ?? undefined,
redirect: false,
});
// redirect to verify page with callbackUrl
router.push(
`/login/verify?callbackUrl=${encodeURIComponent(
searchParams?.get("callbackUrl") ?? "",
)}`,
);
} else {
form.setError("identifier", {
message: t("userNotFound", {
defaultValue: "A user with that email doesn't exist",
}),
});
}
// Store the email in a cookie regardless of whether user exists
await setVerificationEmail(identifier);

// Always send them to verify page
router.push(
`/login/verify?callbackUrl=${encodeURIComponent(
searchParams?.get("callbackUrl") ?? "",
)}`,
);
})}
>
<FormField
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/app/[locale]/(auth)/login/verify/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export default async function VerifyPage() {
t={t}
ns="app"
i18nKey="loginVerifyDescription"
defaults="Check your email for the verification code"
defaults="If an account exists, a verification code will be sent to your email"
/>
</AuthPageDescription>
</AuthPageHeader>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { trpc } from "@/trpc/client";

import { setToken } from "../actions";
import { registerNameFormSchema } from "./schema";
import { signIn } from "next-auth/react";

type RegisterNameFormValues = z.infer<typeof registerNameFormSchema>;

Expand All @@ -44,19 +45,16 @@ export function RegisterNameForm() {
if (res.ok) {
await setToken(res.token);
router.push("/register/verify");
} else {
switch (res.reason) {
case "emailNotAllowed":
form.setError("email", {
message: t("emailNotAllowed"),
});
break;
case "userAlreadyExists":
form.setError("email", {
message: t("userAlreadyExists"),
});
break;
}
} else if (res.reason === "emailNotAllowed") {
form.setError("email", {
message: t("emailNotAllowed"),
});
} else if (res.reason === "userAlreadyExists") {
await signIn("email", {
email: data.email,
redirect: false,
});
router.push("/login/verify");
}
})}
>
Expand Down
16 changes: 6 additions & 10 deletions apps/web/src/app/[locale]/(auth)/register/verify/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,8 @@ import {
} from "../../components/auth-page";
import { OTPForm } from "./components/otp-form";

export default async function VerifyPage({
params: { locale },
}: {
params: { locale: string };
}) {
const { t } = await getTranslation(locale);
export default async function VerifyPage() {
const { t } = await getTranslation();
const token = cookies().get("registration-token")?.value;

if (!token) {
Expand All @@ -34,16 +30,16 @@ export default async function VerifyPage({
<Trans
t={t}
ns="app"
i18nKey="registerVerifyTitle"
defaults="Finish Registering"
i18nKey="verifyEmailTitle"
defaults="Verify Your Email"
/>
</AuthPageTitle>
<AuthPageDescription>
<Trans
t={t}
ns="app"
i18nKey="registerVerifyDescription"
defaults="Check your email for the verification code"
i18nKey="verifyEmailDescription"
defaults="Please check your email for a verification code"
/>
</AuthPageDescription>
</AuthPageHeader>
Expand Down
29 changes: 18 additions & 11 deletions apps/web/src/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,19 +96,26 @@ const providers: Provider[] = [
},
});

// Send appropriate email based on whether user exists
if (user) {
await getEmailClient(user.locale ?? undefined).sendTemplate(
"LoginEmail",
{
to: email,
props: {
magicLink: absoluteUrl("/auth/login", {
magicLink: url,
}),
code: token,
},
// Send login email
await getEmailClient(user.locale ?? undefined).sendTemplate("LoginEmail", {
to: email,
props: {
magicLink: absoluteUrl("/auth/login", {
magicLink: url,
}),
code: token,
},
);
});
} else {
// Send registration email
Copy link
Owner

@lukevella lukevella Feb 1, 2025

Choose a reason for hiding this comment

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

If the user doesn't exist we shouldn't send any email. It is not desirable or possible to complete a registration from the login form.

await getEmailClient().sendTemplate("RegisterEmail", {
to: email,
props: {
code: token,
},
});
}
},
}),
Expand Down