Skip to content

fix(email-validation): add email validation to prevent bouncing, fixed OTP validation #916

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.

Sign up for GitHub

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

Merged
waleedlatif1 merged 4 commits into from
Aug 9, 2025
Merged

fix(email-validation): add email validation to prevent bouncing, fixed OTP validation #916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
51d3ba6
feat(email-validation): add email validation to prevent bouncing
waleedlatif1 Aug 8, 2025
c345945
removed suspicious patterns
waleedlatif1 Aug 8, 2025
9afca3e
fix(verification): fixed OTP verification
waleedlatif1 Aug 8, 2025
75fd0b8
fix failing tests, cleanup
waleedlatif1 Aug 8, 2025
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
16 changes: 8 additions & 8 deletions apps/sim/app/(auth)/login/login-form.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ describe('LoginPage', () => {
const emailInput = screen.getByPlaceholderText(/enter your email/i)
const passwordInput = screen.getByPlaceholderText(/enter your password/i)

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

expect(emailInput).toHaveValue('test@example.com')
expect(emailInput).toHaveValue('user@company.com')
expect(passwordInput).toHaveValue('password123')
})

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

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

fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
fireEvent.change(passwordInput, { target: { value: 'password123' } })
fireEvent.click(submitButton)

await waitFor(() => {
expect(mockSignIn).toHaveBeenCalledWith(
{
email: 'test@example.com',
email: 'user@company.com',
password: 'password123',
callbackURL: '/workspace',
},
Expand Down Expand Up @@ -181,7 +181,7 @@ describe('LoginPage', () => {
const passwordInput = screen.getByPlaceholderText(/enter your password/i)
const submitButton = screen.getByRole('button', { name: /sign in/i })

fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
fireEvent.change(passwordInput, { target: { value: 'wrongpassword' } })
fireEvent.click(submitButton)

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

fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
fireEvent.change(passwordInput, { target: { value: 'password123' } })
fireEvent.click(submitButton)

await waitFor(() => {
expect(mockSendOtp).toHaveBeenCalledWith({
email: 'test@example.com',
email: 'user@company.com',
type: 'email-verification',
})
expect(mockRouter.push).toHaveBeenCalledWith('/verify')
Expand Down
53 changes: 17 additions & 36 deletions apps/sim/app/(auth)/login/login-form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,27 @@ import {
import { Input } from '@/components/ui/input'
import { Label } from '@/components/ui/label'
import { client } from '@/lib/auth-client'
import { quickValidateEmail } from '@/lib/email/validation'
import { createLogger } from '@/lib/logs/console/logger'
import { cn } from '@/lib/utils'
import { SocialLoginButtons } from '@/app/(auth)/components/social-login-buttons'

const logger = createLogger('LoginForm')

const EMAIL_VALIDATIONS = {
required: {
test: (value: string) => Boolean(value && typeof value === 'string'),
message: 'Email is required.',
},
notEmpty: {
test: (value: string) => value.trim().length > 0,
message: 'Email cannot be empty.',
},
basicFormat: {
regex: /^[^\s@]+@[^\s@]+\.[^\s@]+$/,
message: 'Please enter a valid email address.',
},
const validateEmailField = (emailValue: string): string[] => {
const errors: string[] = []

if (!emailValue || !emailValue.trim()) {
errors.push('Email is required.')
return errors
}

const validation = quickValidateEmail(emailValue.trim().toLowerCase())
if (!validation.isValid) {
errors.push(validation.reason || 'Please enter a valid email address.')
}

return errors
}
Comment on lines +25 to 39
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Aug 8, 2025

Choose a reason for hiding this comment

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

style: Consider extracting this validation logic into the centralized validation utility to maintain consistency with other forms

Context Used: Context - When defining properties for components, use a dedicated config file (.ts) for configuration and keep rendered components in their respective component files. (link)


const PASSWORD_VALIDATIONS = {
Expand Down Expand Up @@ -68,27 +70,6 @@ const validateCallbackUrl = (url: string): boolean => {
}
}

// Validate email and return array of error messages
const validateEmail = (emailValue: string): string[] => {
const errors: string[] = []

if (!EMAIL_VALIDATIONS.required.test(emailValue)) {
errors.push(EMAIL_VALIDATIONS.required.message)
return errors // Return early for required field
}

if (!EMAIL_VALIDATIONS.notEmpty.test(emailValue)) {
errors.push(EMAIL_VALIDATIONS.notEmpty.message)
return errors // Return early for empty field
}

if (!EMAIL_VALIDATIONS.basicFormat.regex.test(emailValue)) {
errors.push(EMAIL_VALIDATIONS.basicFormat.message)
}

return errors
}

// Validate password and return array of error messages
const validatePassword = (passwordValue: string): string[] => {
const errors: string[] = []
Expand Down Expand Up @@ -182,7 +163,7 @@ export default function LoginPage({
setEmail(newEmail)

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

// Validate email on submit
const emailValidationErrors = validateEmail(email)
const emailValidationErrors = validateEmailField(email)
setEmailErrors(emailValidationErrors)
setShowEmailValidationError(emailValidationErrors.length > 0)

Expand Down
32 changes: 15 additions & 17 deletions apps/sim/app/(auth)/signup/signup-form.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ describe('SignupPage', () => {
const passwordInput = screen.getByPlaceholderText(/enter your password/i)

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

expect(nameInput).toHaveValue('John Doe')
expect(emailInput).toHaveValue('test@example.com')
expect(emailInput).toHaveValue('user@company.com')
expect(passwordInput).toHaveValue('Password123!')
})

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

fireEvent.change(nameInput, { target: { value: 'John Doe' } })
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
fireEvent.change(passwordInput, { target: { value: 'Password123!' } })
fireEvent.click(submitButton)

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

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

await waitFor(() => {
expect(mockSignUp).toHaveBeenCalledWith(
{
email: 'test@example.com',
email: 'user@company.com',
password: 'Password123!',
name: 'John Doe',
},
Expand All @@ -174,7 +174,7 @@ describe('SignupPage', () => {

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

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

fireEvent.change(nameInput, { target: { value: 'John Doe' } })
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
fireEvent.change(passwordInput, { target: { value: 'Password123!' } })
fireEvent.click(submitButton)

await waitFor(() => {
expect(mockSendOtp).toHaveBeenCalledWith({
email: 'test@example.com',
type: 'email-verification',
})
// With sendVerificationOnSignUp: true, OTP is sent automatically by Better Auth
// No manual OTP sending in the component anymore
expect(mockRouter.push).toHaveBeenCalledWith('/verify?fromSignup=true')
})
})
Expand Down Expand Up @@ -267,7 +265,7 @@ describe('SignupPage', () => {
const submitButton = screen.getByRole('button', { name: /create account/i })

fireEvent.change(nameInput, { target: { value: longName } })
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
fireEvent.change(passwordInput, { target: { value: 'ValidPass123!' } })
fireEvent.click(submitButton)

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

fireEvent.change(nameInput, { target: { value: exactLengthName } })
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
fireEvent.change(passwordInput, { target: { value: 'ValidPass123!' } })
fireEvent.click(submitButton)

Expand All @@ -308,7 +306,7 @@ describe('SignupPage', () => {
await waitFor(() => {
expect(mockSignUp).toHaveBeenCalledWith(
{
email: 'test@example.com',
email: 'user@company.com',
password: 'ValidPass123!',
name: exactLengthName,
},
Expand Down Expand Up @@ -343,7 +341,7 @@ describe('SignupPage', () => {

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

fireEvent.change(nameInput, { target: { value: 'John Doe' } })
fireEvent.change(emailInput, { target: { value: 'test@example.com' } })
fireEvent.change(emailInput, { target: { value: 'user@company.com' } })
fireEvent.change(passwordInput, { target: { value: 'Password123!' } })
fireEvent.click(submitButton)

await waitFor(() => {
expect(mockRouter.push).toHaveBeenCalledWith('/invite/123')
expect(mockRouter.push).toHaveBeenCalledWith('/verify?fromSignup=true')
})
})

Expand Down
Loading