feature/auth #33
feature/auth #33
Conversation
…nto feature/auth
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces several changes across multiple files, primarily focusing on API route handling and component structure. The Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@github-actions[bot] is attempting to deploy a commit to the BlueFinZ Team on Vercel. A member of the Team first needs to authorize it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (8)
apps/www/components/custom/signinComponent.tsx (4)
24-24: Consider a more descriptive component name.While renaming to
SignInComponentmaintains consistency withSignUpComponent, consider using a more specific name likeEmailSignInFormto better describe the component's purpose and authentication method.-export default function SignInComponent() { +export default function EmailSignInForm() {
Line range hint
18-21: Enhance password validation rules.The current password validation only checks for minimum length. Consider adding:
- Maximum length constraint
- Complexity requirements (uppercase, lowercase, numbers, special characters)
- Common password prevention
const formSchema = z.object({ email: z.string().email(), - password: z.string().min(8), + password: z.string() + .min(8, "Password must be at least 8 characters") + .max(100, "Password is too long") + .regex( + /^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[@$!%*?&])[A-Za-z\d@$!%*?&]+$/, + "Password must contain uppercase, lowercase, number and special character" + ), });
Line range hint
31-45: Improve error handling and submission flow.Several improvements are needed in the form submission logic:
- Replace alert with a more user-friendly error display
- Make callback URL configurable
- Add loading state during submission
- Implement proper success handling
+ const [isSubmitting, setIsSubmitting] = useState(false); const onSubmit = async (SignInData: z.infer<typeof formSchema>) => { + setIsSubmitting(true); const { data, error } = await authClient.signIn.email( { email: SignInData.email, password: SignInData.password, - callbackURL: "/dashboard", + callbackURL: process.env.NEXT_PUBLIC_AUTH_CALLBACK_URL || "/dashboard", }, { - onRequest: (ctx) => {}, - onSuccess: (ctx) => {}, + onRequest: (ctx) => { + setIsSubmitting(true); + }, + onSuccess: (ctx) => { + router.push("/dashboard"); + }, onError: (ctx) => { - alert(ctx.error.message); + toast.error(ctx.error.message); + setIsSubmitting(false); }, }, ); };Don't forget to:
- Import required dependencies (
useState,useRouter,toast)- Add loading state to the submit button
- Configure environment variables for the callback URL
Line range hint
52-78: Enhance form UX with essential features.The current form lacks several important UX features:
- Meaningful input placeholders
- Password visibility toggle
- "Remember me" option
- Social login alternatives
<FormField control={form.control} name="email" render={({ field }) => ( <FormItem> <FormLabel>email</FormLabel> <FormControl> - <Input placeholder="shadcn" {...field} /> + <Input placeholder="Enter your email" type="email" {...field} /> </FormControl> <FormMessage /> </FormItem> )} /> <FormField control={form.control} name="password" render={({ field }) => ( <FormItem> <FormLabel>password</FormLabel> <FormControl> - <Input placeholder="shadcn" type="password" {...field} /> + <div className="relative"> + <Input + placeholder="Enter your password" + type={showPassword ? "text" : "password"} + {...field} + /> + <Button + type="button" + variant="ghost" + className="absolute right-2 top-1/2 -translate-y-1/2" + onClick={() => setShowPassword(!showPassword)} + > + {showPassword ? <EyeOffIcon /> : <EyeIcon />} + </Button> + </div> </FormControl> <FormMessage /> </FormItem> )} /> +<FormField + control={form.control} + name="remember" + render={({ field }) => ( + <FormItem className="flex items-center space-x-2"> + <FormControl> + <Checkbox + checked={field.value} + onCheckedChange={field.onChange} + /> + </FormControl> + <FormLabel>Remember me</FormLabel> + </FormItem> + )} +/> <Button type="submit">Submit</Button> +<div className="mt-4"> + <p className="text-center">Or continue with</p> + <div className="mt-2 flex justify-center space-x-4"> + <Button variant="outline" onClick={() => authClient.signIn.google()}> + <GoogleIcon className="mr-2" /> Google + </Button> + <Button variant="outline" onClick={() => authClient.signIn.github()}> + <GitHubIcon className="mr-2" /> GitHub + </Button> + </div> +</div>apps/api/app/api/[[...route]]/route.ts (1)
Line range hint
8-12: Consider environment-specific CORS configuration.The allowed origins list includes both development (
localhost) and production URLs. Consider moving these to environment variables or configuration files to:
- Prevent accidental exposure of development origins in production
- Make the configuration more maintainable
- Allow for different CORS settings per environment
Example configuration approach:
const allowedOrigins = process.env.NODE_ENV === 'production' ? ['https://www.plura.pro', 'http://app.plura.pro'] : ['http://localhost:3003'];apps/www/components/custom/signupComponent.tsx (3)
Line range hint
19-23: Enhance form validation rulesThe current schema validation could be strengthened:
- Name field should have minimum/maximum length constraints and character validation
- Password should enforce stronger requirements (uppercase, lowercase, special characters)
const formSchema = z.object({ - name: z.string(), + name: z.string().min(2).max(50).regex(/^[a-zA-Z\s]*$/, 'Name can only contain letters and spaces'), email: z.string().email(), - password: z.string().min(8), + password: z + .string() + .min(8) + .regex(/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[@$!%*?&])[A-Za-z\d@$!%*?&]+$/, + 'Password must contain at least one uppercase letter, one lowercase letter, one number, and one special character'), });
Line range hint
32-47: Improve error handling and fix inconsistent namingSeveral issues need attention:
- Using
alert()for error handling provides poor user experience- Variable name
SignInDatais inconsistent in a signup component- Empty callback handlers could benefit from proper implementation
- const onSubmit = async (SignInData: z.infer<typeof formSchema>) => { + const onSubmit = async (signUpData: z.infer<typeof formSchema>) => { const { data, error } = await authClient.signUp.email( { - name: SignInData.name, - email: SignInData.email, - password: SignInData.password, + name: signUpData.name, + email: signUpData.email, + password: signUpData.password, }, { - onRequest: (ctx) => {}, - onSuccess: (ctx) => {}, + onRequest: (ctx) => { + // Show loading state + form.setError("root", { message: "Signing up..." }); + }, + onSuccess: (ctx) => { + // Handle successful signup + router.push("/dashboard"); + }, onError: (ctx) => { - alert(ctx.error.message); + form.setError("root", { message: ctx.error.message }); }, }, ); };
Line range hint
52-91: Improve form UX and accessibilityThe form UI needs several improvements:
- Form labels should be properly capitalized
- Placeholder text should be descriptive
- Add loading state during form submission
<FormField control={form.control} name="name" render={({ field }) => ( <FormItem> - <FormLabel>name</FormLabel> + <FormLabel>Name</FormLabel> <FormControl> - <Input placeholder="shadcn" {...field} /> + <Input placeholder="Enter your full name" {...field} /> </FormControl> <FormMessage /> </FormItem> )} />Similar changes should be applied to email and password fields.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
apps/api/app/api/[[...route]]/route.ts(1 hunks)apps/www/app/(auth)/sign-in/page.tsx(1 hunks)apps/www/app/(auth)/sign-up/page.tsx(1 hunks)apps/www/app/sign-in/[[...sign-in]]/layout.tsx(0 hunks)apps/www/app/sign-in/[[...sign-in]]/page.tsx(0 hunks)apps/www/components/custom/signinComponent.tsx(1 hunks)apps/www/components/custom/signupComponent.tsx(1 hunks)apps/www/package.json(0 hunks)
💤 Files with no reviewable changes (3)
- apps/www/app/sign-in/[[...sign-in]]/layout.tsx
- apps/www/app/sign-in/[[...sign-in]]/page.tsx
- apps/www/package.json
✅ Files skipped from review due to trivial changes (2)
- apps/www/app/(auth)/sign-in/page.tsx
- apps/www/app/(auth)/sign-up/page.tsx
🔇 Additional comments (4)
apps/api/app/api/[[...route]]/route.ts (2)
101-101: LGTM! Consistent HTTP method handling.
The OPTIONS handler is now defined consistently with other HTTP method handlers, maintaining a clean and symmetric API design.
103-103: Verify the impact of OPTIONS export change.
The consolidated export of HTTP methods is clean and consistent. However, let's verify that no other parts of the codebase were directly importing the OPTIONS handler separately.
✅ Verification successful
OPTIONS export change is safe to proceed
The consolidated export including OPTIONS is safe as there are no direct imports of the OPTIONS handler in the codebase. The only file importing from this route (apps/api/tests/hello.test.ts) only imports GET, POST, PATCH, and DELETE methods, and doesn't use OPTIONS.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any direct imports of OPTIONS from this route file
# This helps ensure the refactor hasn't broken any existing imports
echo "Checking for direct OPTIONS imports..."
rg -l "import.*OPTIONS.*route" --type ts
echo "Checking for destructured OPTIONS imports..."
rg -l "import.*\{.*OPTIONS.*\}.*route" --type ts
Length of output: 279
Script:
#!/bin/bash
# Let's try a broader search to catch any references to OPTIONS from this route file
# First, get the exact path pattern for the route file
fd "route.ts" apps/api
# Then search for any imports or references to OPTIONS in TypeScript files
echo "Checking for any OPTIONS references..."
rg -l "OPTIONS.*route" --type ts
# Also check for any direct usage of the route file
echo "Checking for imports from the route file..."
rg -l "from.*route'" --type ts
rg -l "from.*\[\[\.\.\.route\]\]" --type ts
Length of output: 391
Script:
#!/bin/bash
# Let's check the content of the test file to see how it imports from the route
echo "Checking test file imports..."
cat apps/api/tests/hello.test.ts
# Also let's verify if there are any other files importing from this route using a different pattern
echo "Checking for other import patterns..."
rg "from.*api.*route" --type ts
Length of output: 3152
apps/www/components/custom/signupComponent.tsx (2)
26-26: LGTM: Component naming follows conventions
The rename from SignUp to SignUpComponent aligns with component naming standards.
Line range hint 26-91: Verify security measures implementation
Please ensure the following security measures are in place:
- Rate limiting for signup attempts
- CSRF protection
- Password strength indicators for users
Consider adding a password strength indicator component to help users create strong passwords.
Summary by CodeRabbit
Release Notes
New Features
SignInComponentandSignUpComponentfor improved sign-in and sign-up functionality.Bug Fixes
Removals
Dependencies