auth #34
auth #34
Conversation
…nto feature/auth
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@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. |
WalkthroughThe pull request introduces several modifications across multiple files in the API and web application components. Key changes include the adjustment of 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (10)
apps/www/app/(auth)/sign-in/page.tsx (1)
3-9: Add metadata for SEO and security.Consider adding page metadata using Next.js metadata API to improve SEO and security for the authentication page.
Add the following metadata configuration:
export const metadata = { title: 'Sign In', description: 'Sign in to your account', robots: { index: false, follow: false, }, };apps/www/app/(auth)/sign-up/page.tsx (1)
3-9: Consider using PascalCase for the page component name.While the implementation is correct, consider renaming the default export function from
pagetoSignUpPageto follow React component naming conventions.-export default function page() { +export default function SignUpPage() { return ( <div className="w-full h-screen flex items-center justify-center"> <SignUpComponent /> </div> ); }apps/www/components/custom/signinComponent.tsx (3)
Line range hint
49-67: Improve form field labels and placeholders for better UX.The current implementation has several UX issues:
- Placeholders show "shadcn" which is not descriptive
- Form labels are lowercase which doesn't follow conventional capitalization
Consider applying these improvements:
<FormField control={form.control} name="email" render={({ field }) => ( <FormItem> - <FormLabel>email</FormLabel> + <FormLabel>Email</FormLabel> <FormControl> - <Input placeholder="shadcn" {...field} /> + <Input placeholder="Enter your email" {...field} /> </FormControl> <FormMessage /> </FormItem> )} /> <FormField control={form.control} name="password" render={({ field }) => ( <FormItem> - <FormLabel>password</FormLabel> + <FormLabel>Password</FormLabel> <FormControl> - <Input placeholder="shadcn" type="password" {...field} /> + <Input placeholder="Enter your password" type="password" {...field} /> </FormControl> <FormMessage /> </FormItem> )} />
Line range hint
31-45: Enhance error handling and user feedback.Current implementation has several areas for improvement:
- Uses basic
alert()for error messages which isn't user-friendly- No loading state during form submission
- Empty success and request handlers
Consider implementing:
- Toast notifications for errors instead of alerts
- Loading state during submission
- Proper success handling for better UX
const onSubmit = async (SignInData: z.infer<typeof formSchema>) => { + form.setError("root", { message: "" }); // Clear previous errors + const [isLoading, setIsLoading] = useState(false); + setIsLoading(true); const { data, error } = await authClient.signIn.email( { email: SignInData.email, password: SignInData.password, callbackURL: "/dashboard", }, { - onRequest: (ctx) => {}, + onRequest: (ctx) => { + setIsLoading(true); + }, - onSuccess: (ctx) => {}, + onSuccess: (ctx) => { + toast.success("Successfully signed in!"); + }, onError: (ctx) => { - alert(ctx.error.message); + toast.error(ctx.error.message); + form.setError("root", { message: ctx.error.message }); }, }, ); + setIsLoading(false); };Also update the submit button to show loading state:
- <Button type="submit">Submit</Button> + <Button type="submit" disabled={isLoading}> + {isLoading ? "Signing in..." : "Sign in"} + </Button>
Line range hint
18-21: Consider strengthening password validation requirements.The current password validation only checks for minimum length. Consider adding more robust requirements for better security.
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") + .regex(/[A-Z]/, "Password must contain at least one uppercase letter") + .regex(/[a-z]/, "Password must contain at least one lowercase letter") + .regex(/[0-9]/, "Password must contain at least one number") + .regex(/[^A-Za-z0-9]/, "Password must contain at least one special character"), });apps/api/app/api/[[...route]]/route.ts (1)
Line range hint
8-12: Consider environment-based configuration for allowed origins.While the CORS configuration is secure, consider moving the
allowedOriginsarray to environment variables for better maintainability and environment-specific configuration.Example implementation:
const allowedOrigins = process.env.ALLOWED_ORIGINS?.split(',') ?? [ "http://localhost:3003", "https://www.plura.pro", "http://app.plura.pro", ];apps/www/components/custom/signupComponent.tsx (4)
Line range hint
19-23: Strengthen form validation rules for better securityThe current validation schema could be enhanced with stronger rules:
- Add minimum length for name field
- Implement password complexity requirements
- Add maximum length constraints to prevent abuse
const formSchema = z.object({ - name: z.string(), + name: z.string().min(2).max(50), email: z.string().email(), - password: z.string().min(8), + password: z.string() + .min(8) + .max(100) + .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
29-45: Improve error handling and user feedbackThe current implementation uses alert() for errors and lacks proper loading/success states:
- Replace alert() with a proper toast/notification system
- Add loading state during form submission
- Provide success feedback
+ const [isLoading, setIsLoading] = useState(false); const onSubmit = async (SignInData: z.infer<typeof formSchema>) => { + setIsLoading(true); const { data, error } = await authClient.signUp.email( { name: SignInData.name, email: SignInData.email, password: SignInData.password, }, { onRequest: (ctx) => {}, - onSuccess: (ctx) => {}, + onSuccess: (ctx) => { + toast.success("Account created successfully!"); + }, onError: (ctx) => { - alert(ctx.error.message); + toast.error(ctx.error.message); }, }, ); + setIsLoading(false); };
Line range hint
52-91: Enhance form fields UX and accessibilityThe form fields need improvements:
- Generic "shadcn" placeholder text should be replaced
- Add proper aria-labels and accessibility attributes
- Add helper text for password requirements
<FormField control={form.control} name="name" render={({ field }) => ( <FormItem> <FormLabel>name</FormLabel> <FormControl> - <Input placeholder="shadcn" {...field} /> + <Input + placeholder="Enter your full name" + aria-label="Full name" + {...field} + /> </FormControl> <FormMessage /> </FormItem> )} /> // Similar changes for email field... <FormField control={form.control} name="password" render={({ field }) => ( <FormItem> <FormLabel>password</FormLabel> <FormControl> - <Input placeholder="shadcn" type="password" {...field} /> + <Input + placeholder="Enter your password" + type="password" + aria-label="Password" + {...field} + /> </FormControl> + <FormDescription> + Password must be at least 8 characters long and contain uppercase, lowercase, + number, and special character + </FormDescription> <FormMessage /> </FormItem> )} />
Line range hint
29-30: Improve form submission handling and fix naming inconsistency
- The parameter name
SignInDatais inconsistent in a signup component- Add protection against multiple submissions
- const onSubmit = async (SignInData: z.infer<typeof formSchema>) => { + const onSubmit = async (signUpData: z.infer<typeof formSchema>) => { + if (isLoading) return;
📜 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
🔇 Additional comments (4)
apps/www/app/(auth)/sign-up/page.tsx (1)
1-1: LGTM! Good improvements in import structure and naming.
The switch to absolute imports using the @ alias and the consistent camelCase naming for the component improves code maintainability and follows better naming conventions.
apps/www/components/custom/signinComponent.tsx (1)
24-24: LGTM: Component renaming is consistent with naming conventions.
The renaming from SignIn to SignInComponent follows better naming practices by being more explicit about the component's nature.
apps/api/app/api/[[...route]]/route.ts (2)
101-103: LGTM! Consistent export pattern for all handlers.
The change improves code organization by making the OPTIONS handler export consistent with other HTTP method handlers.
101-103: Verify impact of OPTIONS handler export change.
While the change improves consistency, let's verify that no other parts of the codebase directly import the OPTIONS handler.
✅ Verification successful
No impact found from OPTIONS handler export change
The verification shows that the OPTIONS handler is only exported in the route file itself (apps/api/app/api/[[...route]]/route.ts) and there are no direct imports or references to it elsewhere in the codebase. This change is safe to proceed with.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for direct imports of OPTIONS from this route file
# Expected: No direct imports of OPTIONS should exist
# Search for direct imports of OPTIONS from this route file
rg -l "import.*\{.*OPTIONS.*\}.*from.*\[\[\.\.\.route\]\]/route"
# Search for potential dynamic imports
rg -l "import.*from.*\[\[\.\.\.route\]\]/route.*OPTIONS"
Length of output: 125
Script:
#!/bin/bash
# Let's try a broader search approach to find any references to OPTIONS from route files
# First, let's find the full path of the route file
fd "route.ts" apps/api
# Then search for any imports or references to OPTIONS in the codebase
rg -i "OPTIONS.*route"
# Also search for any direct references to OPTIONS handler
rg -i "export.*OPTIONS"
# And check for any dynamic route handlers that might use OPTIONS
rg -i "handler.*OPTIONS"
Length of output: 224
| @@ -1,10 +1,9 @@ | |||
| import SignIn from "@/components/custom/Sign-In"; | |||
| import SignInComponent from "@/components/custom/signinComponent"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update file naming convention to follow React best practices.
The import path signinComponent doesn't follow standard React component file naming conventions. React components and their files typically use PascalCase.
Consider renaming:
-import SignInComponent from "@/components/custom/signinComponent";
+import SignInComponent from "@/components/custom/SignInComponent";And rename the actual file from signinComponent.tsx to SignInComponent.tsx.
Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
SignIntoSignInComponentandSignUptoSignUpComponent.Dependencies