From 50cbc890c2b66e89aa5d392046e17947b759bda0 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 29 May 2025 23:45:33 -0700 Subject: [PATCH] fix(validation): added validation for inputs for signup & email, added tests (#438) * add validation for signup/login fields * added validation for inputs for signup & email, added tests * acknowledged PR comments --- apps/sim/app/(auth)/login/login-form.test.tsx | 251 ++++++++++ apps/sim/app/(auth)/login/login-form.tsx | 138 +++++- .../app/(auth)/signup/signup-form.test.tsx | 442 ++++++++++++++++++ apps/sim/app/(auth)/signup/signup-form.tsx | 219 ++++++++- apps/sim/executor/index.test.ts | 19 +- apps/sim/package.json | 3 +- apps/sim/vitest.config.ts | 1 + apps/sim/vitest.setup.ts | 4 +- bun.lock | 15 +- 9 files changed, 1056 insertions(+), 36 deletions(-) create mode 100644 apps/sim/app/(auth)/login/login-form.test.tsx create mode 100644 apps/sim/app/(auth)/signup/signup-form.test.tsx diff --git a/apps/sim/app/(auth)/login/login-form.test.tsx b/apps/sim/app/(auth)/login/login-form.test.tsx new file mode 100644 index 000000000..481d54633 --- /dev/null +++ b/apps/sim/app/(auth)/login/login-form.test.tsx @@ -0,0 +1,251 @@ +/** + * @vitest-environment jsdom + */ + +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { useRouter, useSearchParams } from 'next/navigation' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { client } from '@/lib/auth-client' +import LoginPage from './login-form' + +vi.mock('next/navigation', () => ({ + useRouter: vi.fn(), + useSearchParams: vi.fn(), +})) + +vi.mock('@/lib/auth-client', () => ({ + client: { + signIn: { + email: vi.fn(), + }, + emailOtp: { + sendVerificationOtp: vi.fn(), + }, + }, +})) + +vi.mock('@/app/(auth)/components/social-login-buttons', () => ({ + SocialLoginButtons: () =>
Social Login Buttons
, +})) + +const mockRouter = { + push: vi.fn(), + replace: vi.fn(), + back: vi.fn(), +} + +const mockSearchParams = { + get: vi.fn(), +} + +describe('LoginPage', () => { + beforeEach(() => { + vi.clearAllMocks() + ;(useRouter as any).mockReturnValue(mockRouter) + ;(useSearchParams as any).mockReturnValue(mockSearchParams) + mockSearchParams.get.mockReturnValue(null) + }) + + const defaultProps = { + githubAvailable: true, + googleAvailable: true, + isProduction: false, + } + + describe('Basic Rendering', () => { + it('should render login form with all required elements', () => { + render() + + expect(screen.getByPlaceholderText(/enter your email/i)).toBeInTheDocument() + expect(screen.getByPlaceholderText(/enter your password/i)).toBeInTheDocument() + expect(screen.getByRole('button', { name: /sign in/i })).toBeInTheDocument() + expect(screen.getByText(/forgot password/i)).toBeInTheDocument() + expect(screen.getByText(/sign up/i)).toBeInTheDocument() + }) + + it('should render social login buttons', () => { + render() + + expect(screen.getByTestId('social-login-buttons')).toBeInTheDocument() + }) + }) + + describe('Password Visibility Toggle', () => { + it('should toggle password visibility when button is clicked', () => { + render() + + const passwordInput = screen.getByPlaceholderText(/enter your password/i) + const toggleButton = screen.getByLabelText(/show password/i) + + expect(passwordInput).toHaveAttribute('type', 'password') + + fireEvent.click(toggleButton) + expect(passwordInput).toHaveAttribute('type', 'text') + + fireEvent.click(toggleButton) + expect(passwordInput).toHaveAttribute('type', 'password') + }) + }) + + describe('Form Interaction', () => { + it('should allow users to type in form fields', () => { + render() + + 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(passwordInput, { target: { value: 'password123' } }) + + expect(emailInput).toHaveValue('test@example.com') + expect(passwordInput).toHaveValue('password123') + }) + + it('should show loading state during form submission', async () => { + const mockSignIn = vi.mocked(client.signIn.email) + mockSignIn.mockImplementation( + () => new Promise((resolve) => resolve({ data: { user: { id: '1' } }, error: null })) + ) + + render() + + const emailInput = screen.getByPlaceholderText(/enter your email/i) + 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(passwordInput, { target: { value: 'password123' } }) + fireEvent.click(submitButton) + + expect(screen.getByText('Signing in...')).toBeInTheDocument() + expect(submitButton).toBeDisabled() + }) + }) + + describe('Form Submission', () => { + it('should call signIn with correct credentials', async () => { + const mockSignIn = vi.mocked(client.signIn.email) + mockSignIn.mockResolvedValue({ data: { user: { id: '1' } }, error: null }) + + render() + + const emailInput = screen.getByPlaceholderText(/enter your email/i) + 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(passwordInput, { target: { value: 'password123' } }) + fireEvent.click(submitButton) + + await waitFor(() => { + expect(mockSignIn).toHaveBeenCalledWith( + { + email: 'test@example.com', + password: 'password123', + callbackURL: '/w', + }, + expect.objectContaining({ + onError: expect.any(Function), + }) + ) + }) + }) + + it('should handle authentication errors', async () => { + const mockSignIn = vi.mocked(client.signIn.email) + + mockSignIn.mockImplementation((credentials, options) => { + if (options?.onError) { + options.onError({ + error: { + code: 'INVALID_CREDENTIALS', + message: 'Invalid credentials', + } as any, + response: {} as any, + request: {} as any, + } as any) + } + return Promise.resolve({ data: null, error: 'Invalid credentials' }) + }) + + render() + + const emailInput = screen.getByPlaceholderText(/enter your email/i) + 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(passwordInput, { target: { value: 'wrongpassword' } }) + fireEvent.click(submitButton) + + await waitFor(() => { + expect(screen.getByText('Invalid email or password')).toBeInTheDocument() + }) + }) + }) + + describe('Forgot Password', () => { + it('should open forgot password dialog', () => { + render() + + const forgotPasswordButton = screen.getByText(/forgot password/i) + fireEvent.click(forgotPasswordButton) + + expect(screen.getByText('Reset Password')).toBeInTheDocument() + }) + }) + + describe('URL Parameters', () => { + it('should handle invite flow parameter in signup link', () => { + mockSearchParams.get.mockImplementation((param) => { + if (param === 'invite_flow') return 'true' + if (param === 'callbackUrl') return '/invite/123' + return null + }) + + render() + + const signupLink = screen.getByText(/sign up/i) + expect(signupLink).toHaveAttribute('href', '/signup?invite_flow=true&callbackUrl=/invite/123') + }) + + it('should default to regular signup link when no invite flow', () => { + render() + + const signupLink = screen.getByText(/sign up/i) + expect(signupLink).toHaveAttribute('href', '/signup') + }) + }) + + describe('Email Verification Flow', () => { + it('should redirect to verification page when email not verified', async () => { + const mockSignIn = vi.mocked(client.signIn.email) + const mockSendOtp = vi.mocked(client.emailOtp.sendVerificationOtp) + + mockSignIn.mockRejectedValue({ + message: 'Email not verified', + code: 'EMAIL_NOT_VERIFIED', + }) + + mockSendOtp.mockResolvedValue({ data: null, error: null }) + + render() + + const emailInput = screen.getByPlaceholderText(/enter your email/i) + 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(passwordInput, { target: { value: 'password123' } }) + fireEvent.click(submitButton) + + await waitFor(() => { + expect(mockSendOtp).toHaveBeenCalledWith({ + email: 'test@example.com', + type: 'email-verification', + }) + expect(mockRouter.push).toHaveBeenCalledWith('/verify') + }) + }) + }) +}) diff --git a/apps/sim/app/(auth)/login/login-form.tsx b/apps/sim/app/(auth)/login/login-form.tsx index 5eb46d8a9..548d20b05 100644 --- a/apps/sim/app/(auth)/login/login-form.tsx +++ b/apps/sim/app/(auth)/login/login-form.tsx @@ -10,10 +10,37 @@ import { Input } from '@/components/ui/input' import { Label } from '@/components/ui/label' import { client } from '@/lib/auth-client' 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 PASSWORD_VALIDATIONS = { + required: { + test: (value: string) => Boolean(value && typeof value === 'string'), + message: 'Password is required.', + }, + notEmpty: { + test: (value: string) => value.trim().length > 0, + message: 'Password cannot be empty.', + }, +} + // Validate callback URL to prevent open redirect vulnerabilities const validateCallbackUrl = (url: string): boolean => { try { @@ -35,6 +62,44 @@ 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[] = [] + + if (!PASSWORD_VALIDATIONS.required.test(passwordValue)) { + errors.push(PASSWORD_VALIDATIONS.required.message) + return errors // Return early for required field + } + + if (!PASSWORD_VALIDATIONS.notEmpty.test(passwordValue)) { + errors.push(PASSWORD_VALIDATIONS.notEmpty.message) + return errors // Return early for empty field + } + + return errors +} + export default function LoginPage({ githubAvailable, googleAvailable, @@ -66,6 +131,11 @@ export default function LoginPage({ message: string }>({ type: null, message: '' }) + // Email validation state + const [email, setEmail] = useState('') + const [emailErrors, setEmailErrors] = useState([]) + const [showEmailValidationError, setShowEmailValidationError] = useState(false) + // Extract URL parameters after component mounts to avoid SSR issues useEffect(() => { setMounted(true) @@ -101,6 +171,26 @@ export default function LoginPage({ } }, [forgotPasswordEmail, forgotPasswordOpen]) + const handleEmailChange = (e: React.ChangeEvent) => { + const newEmail = e.target.value + setEmail(newEmail) + + // Silently validate but don't show errors until submit + const errors = validateEmail(newEmail) + setEmailErrors(errors) + setShowEmailValidationError(false) + } + + const handlePasswordChange = (e: React.ChangeEvent) => { + const newPassword = e.target.value + setPassword(newPassword) + + // Silently validate but don't show errors until submit + const errors = validatePassword(newPassword) + setPasswordErrors(errors) + setShowValidationError(false) + } + async function onSubmit(e: React.FormEvent) { e.preventDefault() setIsLoading(true) @@ -108,6 +198,22 @@ export default function LoginPage({ const formData = new FormData(e.currentTarget) const email = formData.get('email') as string + // Validate email on submit + const emailValidationErrors = validateEmail(email) + setEmailErrors(emailValidationErrors) + setShowEmailValidationError(emailValidationErrors.length > 0) + + // Validate password on submit + const passwordValidationErrors = validatePassword(password) + setPasswordErrors(passwordValidationErrors) + setShowValidationError(passwordValidationErrors.length > 0) + + // If there are validation errors, stop submission + if (emailValidationErrors.length > 0 || passwordValidationErrors.length > 0) { + setIsLoading(false) + return + } + try { // Final validation before submission const safeCallbackUrl = validateCallbackUrl(callbackUrl) ? callbackUrl : '/w' @@ -290,12 +396,25 @@ export default function LoginPage({ name='email' placeholder='Enter your email' required - type='email' autoCapitalize='none' autoComplete='email' autoCorrect='off' - className='border-neutral-700 bg-neutral-900 text-white placeholder:text-white/60' + value={email} + onChange={handleEmailChange} + className={cn( + 'border-neutral-700 bg-neutral-900 text-white placeholder:text-white/60', + showEmailValidationError && + emailErrors.length > 0 && + 'border-red-500 focus-visible:ring-red-500' + )} /> + {showEmailValidationError && emailErrors.length > 0 && ( +
+ {emailErrors.map((error, index) => ( +

{error}

+ ))} +
+ )}
@@ -321,14 +440,13 @@ export default function LoginPage({ autoCorrect='off' placeholder='Enter your password' value={password} - onChange={(e) => { - setPassword(e.target.value) - if (showValidationError) { - setShowValidationError(false) - setPasswordErrors([]) - } - }} - className='border-neutral-700 bg-neutral-900 pr-10 text-white placeholder:text-white/60' + onChange={handlePasswordChange} + className={cn( + 'border-neutral-700 bg-neutral-900 pr-10 text-white placeholder:text-white/60', + showValidationError && + passwordErrors.length > 0 && + 'border-red-500 focus-visible:ring-red-500' + )} />