Skip to content

Commit 43a3416

Browse files
authored
fix(email-validation): add email validation to prevent bouncing, fixed OTP validation (#916)
* feat(email-validation): add email validation to prevent bouncing * removed suspicious patterns * fix(verification): fixed OTP verification * fix failing tests, cleanup
1 parent 7f39cd0 commit 43a3416

File tree

18 files changed

+469
-277
lines changed

18 files changed

+469
-277
lines changed

apps/sim/app/(auth)/login/login-form.test.tsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,10 @@ describe('LoginPage', () => {
9494
const emailInput = screen.getByPlaceholderText(/enter your email/i)
9595
const passwordInput = screen.getByPlaceholderText(/enter your password/i)
9696

97-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
97+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
9898
fireEvent.change(passwordInput, { target: { value: 'password123' } })
9999

100-
expect(emailInput).toHaveValue('test@example.com')
100+
expect(emailInput).toHaveValue('user@company.com')
101101
expect(passwordInput).toHaveValue('password123')
102102
})
103103

@@ -117,7 +117,7 @@ describe('LoginPage', () => {
117117
const submitButton = screen.getByRole('button', { name: /sign in/i })
118118

119119
await act(async () => {
120-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
120+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
121121
fireEvent.change(passwordInput, { target: { value: 'password123' } })
122122
fireEvent.click(submitButton)
123123
})
@@ -140,14 +140,14 @@ describe('LoginPage', () => {
140140
const passwordInput = screen.getByPlaceholderText(/enter your password/i)
141141
const submitButton = screen.getByRole('button', { name: /sign in/i })
142142

143-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
143+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
144144
fireEvent.change(passwordInput, { target: { value: 'password123' } })
145145
fireEvent.click(submitButton)
146146

147147
await waitFor(() => {
148148
expect(mockSignIn).toHaveBeenCalledWith(
149149
{
150-
email: 'test@example.com',
150+
email: 'user@company.com',
151151
password: 'password123',
152152
callbackURL: '/workspace',
153153
},
@@ -181,7 +181,7 @@ describe('LoginPage', () => {
181181
const passwordInput = screen.getByPlaceholderText(/enter your password/i)
182182
const submitButton = screen.getByRole('button', { name: /sign in/i })
183183

184-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
184+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
185185
fireEvent.change(passwordInput, { target: { value: 'wrongpassword' } })
186186
fireEvent.click(submitButton)
187187

@@ -242,13 +242,13 @@ describe('LoginPage', () => {
242242
const passwordInput = screen.getByPlaceholderText(/enter your password/i)
243243
const submitButton = screen.getByRole('button', { name: /sign in/i })
244244

245-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
245+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
246246
fireEvent.change(passwordInput, { target: { value: 'password123' } })
247247
fireEvent.click(submitButton)
248248

249249
await waitFor(() => {
250250
expect(mockSendOtp).toHaveBeenCalledWith({
251-
email: 'test@example.com',
251+
email: 'user@company.com',
252252
type: 'email-verification',
253253
})
254254
expect(mockRouter.push).toHaveBeenCalledWith('/verify')

apps/sim/app/(auth)/login/login-form.tsx

Lines changed: 17 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -15,25 +15,27 @@ import {
1515
import { Input } from '@/components/ui/input'
1616
import { Label } from '@/components/ui/label'
1717
import { client } from '@/lib/auth-client'
18+
import { quickValidateEmail } from '@/lib/email/validation'
1819
import { createLogger } from '@/lib/logs/console/logger'
1920
import { cn } from '@/lib/utils'
2021
import { SocialLoginButtons } from '@/app/(auth)/components/social-login-buttons'
2122

2223
const logger = createLogger('LoginForm')
2324

24-
const EMAIL_VALIDATIONS = {
25-
required: {
26-
test: (value: string) => Boolean(value && typeof value === 'string'),
27-
message: 'Email is required.',
28-
},
29-
notEmpty: {
30-
test: (value: string) => value.trim().length > 0,
31-
message: 'Email cannot be empty.',
32-
},
33-
basicFormat: {
34-
regex: /^[^\s@]+@[^\s@]+\.[^\s@]+$/,
35-
message: 'Please enter a valid email address.',
36-
},
25+
const validateEmailField = (emailValue: string): string[] => {
26+
const errors: string[] = []
27+
28+
if (!emailValue || !emailValue.trim()) {
29+
errors.push('Email is required.')
30+
return errors
31+
}
32+
33+
const validation = quickValidateEmail(emailValue.trim().toLowerCase())
34+
if (!validation.isValid) {
35+
errors.push(validation.reason || 'Please enter a valid email address.')
36+
}
37+
38+
return errors
3739
}
3840

3941
const PASSWORD_VALIDATIONS = {
@@ -68,27 +70,6 @@ const validateCallbackUrl = (url: string): boolean => {
6870
}
6971
}
7072

71-
// Validate email and return array of error messages
72-
const validateEmail = (emailValue: string): string[] => {
73-
const errors: string[] = []
74-
75-
if (!EMAIL_VALIDATIONS.required.test(emailValue)) {
76-
errors.push(EMAIL_VALIDATIONS.required.message)
77-
return errors // Return early for required field
78-
}
79-
80-
if (!EMAIL_VALIDATIONS.notEmpty.test(emailValue)) {
81-
errors.push(EMAIL_VALIDATIONS.notEmpty.message)
82-
return errors // Return early for empty field
83-
}
84-
85-
if (!EMAIL_VALIDATIONS.basicFormat.regex.test(emailValue)) {
86-
errors.push(EMAIL_VALIDATIONS.basicFormat.message)
87-
}
88-
89-
return errors
90-
}
91-
9273
// Validate password and return array of error messages
9374
const validatePassword = (passwordValue: string): string[] => {
9475
const errors: string[] = []
@@ -182,7 +163,7 @@ export default function LoginPage({
182163
setEmail(newEmail)
183164

184165
// Silently validate but don't show errors until submit
185-
const errors = validateEmail(newEmail)
166+
const errors = validateEmailField(newEmail)
186167
setEmailErrors(errors)
187168
setShowEmailValidationError(false)
188169
}
@@ -205,7 +186,7 @@ export default function LoginPage({
205186
const email = formData.get('email') as string
206187

207188
// Validate email on submit
208-
const emailValidationErrors = validateEmail(email)
189+
const emailValidationErrors = validateEmailField(email)
209190
setEmailErrors(emailValidationErrors)
210191
setShowEmailValidationError(emailValidationErrors.length > 0)
211192

apps/sim/app/(auth)/signup/signup-form.test.tsx

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ describe('SignupPage', () => {
9696
const passwordInput = screen.getByPlaceholderText(/enter your password/i)
9797

9898
fireEvent.change(nameInput, { target: { value: 'John Doe' } })
99-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
99+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
100100
fireEvent.change(passwordInput, { target: { value: 'Password123!' } })
101101

102102
expect(nameInput).toHaveValue('John Doe')
103-
expect(emailInput).toHaveValue('test@example.com')
103+
expect(emailInput).toHaveValue('user@company.com')
104104
expect(passwordInput).toHaveValue('Password123!')
105105
})
106106

@@ -118,7 +118,7 @@ describe('SignupPage', () => {
118118
const submitButton = screen.getByRole('button', { name: /create account/i })
119119

120120
fireEvent.change(nameInput, { target: { value: 'John Doe' } })
121-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
121+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
122122
fireEvent.change(passwordInput, { target: { value: 'Password123!' } })
123123
fireEvent.click(submitButton)
124124

@@ -144,14 +144,14 @@ describe('SignupPage', () => {
144144

145145
// Use valid input that passes all validation rules
146146
fireEvent.change(nameInput, { target: { value: 'John Doe' } })
147-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
147+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
148148
fireEvent.change(passwordInput, { target: { value: 'Password123!' } })
149149
fireEvent.click(submitButton)
150150

151151
await waitFor(() => {
152152
expect(mockSignUp).toHaveBeenCalledWith(
153153
{
154-
email: 'test@example.com',
154+
email: 'user@company.com',
155155
password: 'Password123!',
156156
name: 'John Doe',
157157
},
@@ -174,7 +174,7 @@ describe('SignupPage', () => {
174174

175175
// Use name with leading/trailing spaces which should fail validation
176176
fireEvent.change(nameInput, { target: { value: ' John Doe ' } })
177-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
177+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
178178
fireEvent.change(passwordInput, { target: { value: 'Password123!' } })
179179
fireEvent.click(submitButton)
180180

@@ -206,15 +206,13 @@ describe('SignupPage', () => {
206206
const submitButton = screen.getByRole('button', { name: /create account/i })
207207

208208
fireEvent.change(nameInput, { target: { value: 'John Doe' } })
209-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
209+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
210210
fireEvent.change(passwordInput, { target: { value: 'Password123!' } })
211211
fireEvent.click(submitButton)
212212

213213
await waitFor(() => {
214-
expect(mockSendOtp).toHaveBeenCalledWith({
215-
216-
type: 'email-verification',
217-
})
214+
// With sendVerificationOnSignUp: true, OTP is sent automatically by Better Auth
215+
// No manual OTP sending in the component anymore
218216
expect(mockRouter.push).toHaveBeenCalledWith('/verify?fromSignup=true')
219217
})
220218
})
@@ -267,7 +265,7 @@ describe('SignupPage', () => {
267265
const submitButton = screen.getByRole('button', { name: /create account/i })
268266

269267
fireEvent.change(nameInput, { target: { value: longName } })
270-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
268+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
271269
fireEvent.change(passwordInput, { target: { value: 'ValidPass123!' } })
272270
fireEvent.click(submitButton)
273271

@@ -295,7 +293,7 @@ describe('SignupPage', () => {
295293
const submitButton = screen.getByRole('button', { name: /create account/i })
296294

297295
fireEvent.change(nameInput, { target: { value: exactLengthName } })
298-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
296+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
299297
fireEvent.change(passwordInput, { target: { value: 'ValidPass123!' } })
300298
fireEvent.click(submitButton)
301299

@@ -308,7 +306,7 @@ describe('SignupPage', () => {
308306
await waitFor(() => {
309307
expect(mockSignUp).toHaveBeenCalledWith(
310308
{
311-
email: 'test@example.com',
309+
email: 'user@company.com',
312310
password: 'ValidPass123!',
313311
name: exactLengthName,
314312
},
@@ -343,7 +341,7 @@ describe('SignupPage', () => {
343341

344342
await act(async () => {
345343
fireEvent.change(nameInput, { target: { value: 'John Doe' } })
346-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
344+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
347345
fireEvent.change(passwordInput, { target: { value: 'Password123!' } })
348346
fireEvent.click(submitButton)
349347
})
@@ -385,12 +383,12 @@ describe('SignupPage', () => {
385383
const submitButton = screen.getByRole('button', { name: /create account/i })
386384

387385
fireEvent.change(nameInput, { target: { value: 'John Doe' } })
388-
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
386+
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
389387
fireEvent.change(passwordInput, { target: { value: 'Password123!' } })
390388
fireEvent.click(submitButton)
391389

392390
await waitFor(() => {
393-
expect(mockRouter.push).toHaveBeenCalledWith('/invite/123')
391+
expect(mockRouter.push).toHaveBeenCalledWith('/verify?fromSignup=true')
394392
})
395393
})
396394

0 commit comments

Comments
 (0)