Address comments

This commit is contained in:
Carlos Monastyrski
2025-09-11 10:50:48 -03:00
parent 0293efbed9
commit b065e7ceba
10 changed files with 53 additions and 30 deletions

View File

@@ -0,0 +1,23 @@
import { Knex } from "knex";
import { TableName } from "../schemas";
export async function up(knex: Knex): Promise<void> {
const hasPayloadCol = await knex.schema.hasColumn(TableName.AuthTokens, "payload");
if (!hasPayloadCol) {
await knex.schema.alterTable(TableName.AuthTokens, (t) => {
t.text("payload").nullable();
});
}
}
export async function down(knex: Knex): Promise<void> {
const hasPayloadCol = await knex.schema.hasColumn(TableName.AuthTokens, "payload");
if (hasPayloadCol) {
await knex.schema.alterTable(TableName.AuthTokens, (t) => {
t.dropColumn("payload");
});
}
}

View File

@@ -18,7 +18,8 @@ export const AuthTokensSchema = z.object({
updatedAt: z.date(), updatedAt: z.date(),
userId: z.string().uuid().nullable().optional(), userId: z.string().uuid().nullable().optional(),
orgId: z.string().uuid().nullable().optional(), orgId: z.string().uuid().nullable().optional(),
aliasId: z.string().nullable().optional() aliasId: z.string().nullable().optional(),
payload: z.string().nullable().optional()
}); });
export type TAuthTokens = z.infer<typeof AuthTokensSchema>; export type TAuthTokens = z.infer<typeof AuthTokensSchema>;

View File

@@ -131,7 +131,7 @@ export const registerUserRouter = async (server: FastifyZodProvider) => {
server.route({ server.route({
method: "POST", method: "POST",
url: "/me/email/otp", url: "/me/email-change/otp",
config: { config: {
rateLimit: smtpRateLimit({ rateLimit: smtpRateLimit({
keyGenerator: (req) => req.permission.id keyGenerator: (req) => req.permission.id
@@ -167,7 +167,7 @@ export const registerUserRouter = async (server: FastifyZodProvider) => {
schema: { schema: {
body: z.object({ body: z.object({
newEmail: z.string().email().trim(), newEmail: z.string().email().trim(),
otpCode: z.string().trim().length(8) otpCode: z.string().trim().length(6)
}), }),
response: { response: {
200: z.object({ 200: z.object({

View File

@@ -37,9 +37,9 @@ export const getTokenConfig = (tokenType: TokenType) => {
return { token, triesLeft, expiresAt }; return { token, triesLeft, expiresAt };
} }
case TokenType.TOKEN_EMAIL_CHANGE_OTP: { case TokenType.TOKEN_EMAIL_CHANGE_OTP: {
const token = String(crypto.randomInt(10 ** 7, 10 ** 8 - 1)); const token = String(crypto.randomInt(10 ** 5, 10 ** 6 - 1));
const triesLeft = 3; const triesLeft = 1;
const expiresAt = new Date(new Date().getTime() + 600000); // 10 minutes expiry const expiresAt = new Date(new Date().getTime() + 600000);
return { token, triesLeft, expiresAt }; return { token, triesLeft, expiresAt };
} }
case TokenType.TOKEN_EMAIL_MFA: { case TokenType.TOKEN_EMAIL_MFA: {
@@ -81,7 +81,7 @@ export const getTokenConfig = (tokenType: TokenType) => {
}; };
export const tokenServiceFactory = ({ tokenDAL, userDAL, orgMembershipDAL }: TAuthTokenServiceFactoryDep) => { export const tokenServiceFactory = ({ tokenDAL, userDAL, orgMembershipDAL }: TAuthTokenServiceFactoryDep) => {
const createTokenForUser = async ({ type, userId, orgId, aliasId }: TCreateTokenForUserDTO) => { const createTokenForUser = async ({ type, userId, orgId, aliasId, payload }: TCreateTokenForUserDTO) => {
const { token, ...tkCfg } = getTokenConfig(type); const { token, ...tkCfg } = getTokenConfig(type);
const appCfg = getConfig(); const appCfg = getConfig();
const tokenHash = await crypto.hashing().createHash(token, appCfg.SALT_ROUNDS); const tokenHash = await crypto.hashing().createHash(token, appCfg.SALT_ROUNDS);
@@ -95,7 +95,8 @@ export const tokenServiceFactory = ({ tokenDAL, userDAL, orgMembershipDAL }: TAu
userId, userId,
orgId, orgId,
triesLeft: tkCfg?.triesLeft, triesLeft: tkCfg?.triesLeft,
aliasId aliasId,
payload
}, },
tx tx
); );

View File

@@ -16,6 +16,7 @@ export type TCreateTokenForUserDTO = {
userId: string; userId: string;
orgId?: string; orgId?: string;
aliasId?: string; aliasId?: string;
payload?: string;
}; };
export type TCreateOrgInviteTokenDTO = { export type TCreateOrgInviteTokenDTO = {

View File

@@ -217,17 +217,18 @@ export const userServiceFactory = ({
const existingUsers = await userDAL.findUserByUsername(newEmail.toLowerCase(), tx); const existingUsers = await userDAL.findUserByUsername(newEmail.toLowerCase(), tx);
const existingUser = existingUsers?.find((u) => u.id !== userId); const existingUser = existingUsers?.find((u) => u.id !== userId);
if (existingUser) { if (existingUser) {
// Don't reveal that email is taken - just don't send OTP // Don't reveal that email is taken - just don't send OTP.
// Frontend will show generic "check your email" message await new Promise((resolve) => {
setTimeout(resolve, 2000);
});
return { success: true, message: "Verification code sent to new email address" }; return { success: true, message: "Verification code sent to new email address" };
} }
// Generate 8-digit OTP and store newEmail in aliasId field temporarily // Generate 6-digit OTP
const otpCode = await tokenService.createTokenForUser({ const otpCode = await tokenService.createTokenForUser({
type: TokenType.TOKEN_EMAIL_CHANGE_OTP, type: TokenType.TOKEN_EMAIL_CHANGE_OTP,
userId, userId,
// Use aliasId to store the new email (we'll parse this back later) payload: newEmail.toLowerCase()
aliasId: newEmail.toLowerCase()
}); });
// Send OTP to NEW email address // Send OTP to NEW email address
@@ -271,18 +272,16 @@ export const userServiceFactory = ({
code: otpCode code: otpCode
}); });
} catch (error) { } catch (error) {
// For security reasons, always return "Invalid verification code" regardless of the actual error
// This prevents information disclosure about existing emails
throw new BadRequestError({ message: "Invalid verification code", name: "UpdateUserEmail" }); throw new BadRequestError({ message: "Invalid verification code", name: "UpdateUserEmail" });
} }
// Verify the new email matches what was stored in aliasId // Verify the new email matches what was stored in payload
const tokenNewEmail = tokenData?.aliasId; const tokenNewEmail = tokenData?.payload;
if (!tokenNewEmail || tokenNewEmail !== newEmail.toLowerCase()) { if (!tokenNewEmail || tokenNewEmail !== newEmail.toLowerCase()) {
throw new BadRequestError({ message: "Invalid verification code", name: "UpdateUserEmail" }); throw new BadRequestError({ message: "Invalid verification code", name: "UpdateUserEmail" });
} }
// Final check if another user has this email (in case it was taken between OTP request and verification) // Final check if another user has this email
const existingUsers = await userDAL.findUserByUsername(newEmail.toLowerCase(), tx); const existingUsers = await userDAL.findUserByUsername(newEmail.toLowerCase(), tx);
const existingUser = existingUsers?.find((u) => u.id !== userId); const existingUser = existingUsers?.find((u) => u.id !== userId);
if (existingUser) { if (existingUser) {
@@ -292,13 +291,11 @@ export const userServiceFactory = ({
// Delete all user aliases since the email is changing // Delete all user aliases since the email is changing
await userAliasDAL.delete({ userId }, tx); await userAliasDAL.delete({ userId }, tx);
// Update the user's email and KEEP email as verified (as requested)
const updatedUser = await userDAL.updateById( const updatedUser = await userDAL.updateById(
userId, userId,
{ {
email: newEmail.toLowerCase(), email: newEmail.toLowerCase(),
username: newEmail.toLowerCase(), username: newEmail.toLowerCase()
isEmailVerified: true // Keep verified as per requirement
}, },
tx tx
); );

View File

@@ -30,8 +30,8 @@ You can update your account email address:
2. Navigate to the `Authentication` tab. 2. Navigate to the `Authentication` tab.
3. In the `Change Email` section, enter your new email address. 3. In the `Change Email` section, enter your new email address.
![change email section](../../../images/auth-methods/personal-settings-authentication-change-email-password.png) ![change email section](../../../images/auth-methods/personal-settings-authentication-change-email-password.png)
4. Click `Send Verification Code` to receive an 8-digit verification code at your new email address. 4. Click `Send Verification Code` to receive an 6-digit verification code at your new email address.
5. Check your new email inbox and enter the verification code in the form. 5. Check your new email inbox and enter the verification code.
![change email section](../../../images/auth-methods/personal-settings-authentication-change-email-confirmation.png) ![change email section](../../../images/auth-methods/personal-settings-authentication-change-email-confirmation.png)
6. Click `Confirm Email Change` to complete the process. 6. Click `Confirm Email Change` to complete the process.
7. You will be logged out and need to sign in again with your new email address. 7. You will be logged out and need to sign in again with your new email address.

Binary file not shown.

Before

Width:  |  Height:  |  Size: 766 KiB

After

Width:  |  Height:  |  Size: 437 KiB

View File

@@ -156,7 +156,7 @@ export const useRemoveMyDuplicateAccounts = () => {
export const useRequestEmailChangeOTP = () => { export const useRequestEmailChangeOTP = () => {
return useMutation({ return useMutation({
mutationFn: async ({ newEmail }: { newEmail: string }) => { mutationFn: async ({ newEmail }: { newEmail: string }) => {
const { data } = await apiRequest.post("/api/v2/users/me/email/otp", { const { data } = await apiRequest.post("/api/v2/users/me/email-change/otp", {
newEmail newEmail
}); });
return data; return data;

View File

@@ -103,9 +103,9 @@ export const ChangeEmailSection = () => {
const [typedOTP, setTypedOTP] = useState(""); const [typedOTP, setTypedOTP] = useState("");
const handleOTPSubmit = async () => { const handleOTPSubmit = async () => {
if (typedOTP.length !== 8) { if (typedOTP.length !== 6) {
createNotification({ createNotification({
text: "Please enter the complete 8-digit verification code", text: "Please enter the complete 6-digit verification code",
type: "error" type: "error"
}); });
return; return;
@@ -197,7 +197,7 @@ export const ChangeEmailSection = () => {
Send Verification Code Send Verification Code
</Button> </Button>
<p className="mt-2 font-inter text-sm text-mineshaft-400"> <p className="mt-2 font-inter text-sm text-mineshaft-400">
We&apos;ll send an 8-digit verification code to your new email address. We&apos;ll send an 6-digit verification code to your new email address.
</p> </p>
</form> </form>
</div> </div>
@@ -210,7 +210,7 @@ export const ChangeEmailSection = () => {
> >
<ModalContent <ModalContent
title="Email Verification" title="Email Verification"
subTitle={`Enter the 8-digit verification code sent to: ${pendingEmail}`} subTitle={`Enter the 6-digit verification code sent to: ${pendingEmail}`}
> >
<div className="flex flex-col items-center space-y-4"> <div className="flex flex-col items-center space-y-4">
<div className="flex justify-center"> <div className="flex justify-center">
@@ -218,7 +218,7 @@ export const ChangeEmailSection = () => {
name="otp-input" name="otp-input"
inputMode="tel" inputMode="tel"
type="text" type="text"
fields={8} fields={6}
onChange={setTypedOTP} onChange={setTypedOTP}
value={typedOTP} value={typedOTP}
{...otpInputProps} {...otpInputProps}
@@ -232,7 +232,7 @@ export const ChangeEmailSection = () => {
<Button <Button
onClick={handleOTPSubmit} onClick={handleOTPSubmit}
isLoading={isUpdatingEmail} isLoading={isUpdatingEmail}
isDisabled={typedOTP.length !== 8} isDisabled={typedOTP.length !== 6}
> >
Confirm Email Change Confirm Email Change
</Button> </Button>