-
Notifications
You must be signed in to change notification settings - Fork 43
feat: Added a Basic Authentication System #123
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.
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduce a real authentication system using Supabase for the frontend. New pages for sign-up, password reset, and password recovery are added, and the login flow is refactored to use Supabase authentication. Environment configuration and Supabase client initialization are included. Error handling and session management are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Supabase
User->>App: Visit /login
App->>Supabase: getSession()
Supabase-->>App: Session info
App->>User: Show LoginPage
User->>App: Submit login form (email, password)
App->>Supabase: signInWithPassword(email, password)
Supabase-->>App: Auth result (success/failure)
App->>User: Show success/error toast, redirect if authenticated
User->>App: Visit /signup
App->>User: Show SignUpPage
User->>App: Submit sign-up form
App->>Supabase: auth.signUp(email, password, ...)
Supabase-->>App: Sign-up result
App->>User: Show confirmation or error
User->>App: Visit /forgot-password
App->>User: Show ForgotPasswrdPage
User->>App: Submit email for reset
App->>Supabase: resetPasswordForEmail(email)
Supabase-->>App: Reset email result
App->>User: Show confirmation or error
User->>App: Visit /reset-password with tokens
App->>Supabase: setSession(access_token, refresh_token)
Supabase-->>App: Session result
App->>User: Show ResetPasswordPage
User->>App: Submit new password
App->>Supabase: updateUser({ password })
Supabase-->>App: Update result
App->>User: Show success/error, redirect
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
.vimrc (1)
1-3
:.vimrc
shouldn’t live in the repo ‒ and the directive is invalid anyway
- Committing personal editor configuration clutters the project root and forces unrelated settings on every contributor. If shared editor settings are truly needed, place them under
./.editorconfig
,./.vscode/
, or a dedicatedtools/ide/
folder—otherwise drop the file from the PR.syntax-highlighting=True
is not a valid Vim command; the canonical directive is simplysyntax on
(orsyntax enable
).Recommended fix:
- syntax-highlighting=True + " Remove this file from the repository, or, if deliberate: + syntax on
🧹 Nitpick comments (7)
frontend/.env example (1)
1-3
: Improve placeholder format for environment variables.The environment variable placeholders contain spaces which could cause configuration issues if copied directly. Consider using underscores or hyphens instead.
-VITE_SUPABASE_URL=YOUR SUPABASE URL +VITE_SUPABASE_URL=YOUR_SUPABASE_URL -VITE_SUPABASE_KEY=YOUR SUPABASE ANON KEY +VITE_SUPABASE_KEY=YOUR_SUPABASE_ANON_KEYfrontend/src/lib/supabaseClient.tsx (1)
1-9
: Consider changing file extension from .tsx to .ts.This file contains no JSX elements, so the
.ts
extension would be more appropriate than.tsx
.frontend/src/App.tsx (1)
84-91
: Fix formatting and spacing consistency.The signup route formatting is inconsistent with the login route above it.
<Route path="/signup" - element= {isAuthenticated ? ( - <Navigate to="/" replace /> - ) : ( - <SignUpPage/> - )} + element={ + isAuthenticated ? ( + <Navigate to="/" replace /> + ) : ( + <SignUpPage /> + ) + } />frontend/src/components/pages/SignUpPage.tsx (4)
23-25
: Remove unused interface.The
LoginPageProps
interface is not used in this SignUpPage component and appears to be leftover from copying the LoginPage component.-interface LoginPageProps { - onLogin: () => void; -}
107-112
: Use appropriate icon for username field.The username field is using the Mail icon, which is semantically incorrect and confusing for users.
+import { + Settings, + Mail, + Lock, + LogIn, + User +} from 'lucide-react'; <InputField - icon={Mail} + icon={User} name="name" placeholder="Username" required />
191-197
: Remove unnecessary state update before navigation.Setting
mailPage
to false before navigating away is unnecessary since the component will unmount.<button type="button" - onClick={() => { setMailPage(false); navigate('/login') }} + onClick={() => navigate('/login')} className="mt-4 px-6 py-2 bg-green-500 hover:bg-green-600 text-white rounded-lg font-medium transition-colors" > Back to Sign In </button>
105-134
: Consider adding client-side form validation for better UX.Adding real-time validation feedback would improve user experience by catching errors before form submission.
Consider implementing client-side validation with visual feedback:
const [errors, setErrors] = useState<{[key: string]: string}>({}); const validateField = (name: string, value: string) => { const newErrors = { ...errors }; switch (name) { case 'email': const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; if (!emailRegex.test(value)) { newErrors.email = 'Please enter a valid email address'; } else { delete newErrors.email; } break; case 'password': if (value.length < 6) { newErrors.password = 'Password must be at least 6 characters'; } else { delete newErrors.password; } break; } setErrors(newErrors); };Then add
onChange
handlers to InputField components and display error messages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (7)
.vimrc
(1 hunks)frontend/.env example
(1 hunks)frontend/package.json
(1 hunks)frontend/src/App.tsx
(2 hunks)frontend/src/components/pages/LoginPage.tsx
(3 hunks)frontend/src/components/pages/SignUpPage.tsx
(1 hunks)frontend/src/lib/supabaseClient.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/pages/LoginPage.tsx (1)
frontend/src/lib/supabaseClient.tsx (1)
supabase
(8-8)
🔇 Additional comments (7)
frontend/package.json (1)
13-13
: Supabase dependency version is current and secureAs of August 6, 2025,
@supabase/supabase-js@^2.53.0
is the latest release and has no publicly reported security vulnerabilities. Continue to monitor the Supabase GitHub repo and major CVE databases for future updates.• File
frontend/package.json
, line 13:"@supabase/supabase-js": "^2.53.0",
frontend/src/components/pages/LoginPage.tsx (3)
56-63
: LGTM on form data extraction and authentication call.The FormData approach for extracting form values and the Supabase authentication call are implemented correctly.
93-93
: LGTM on form field naming.Adding
name
attributes to form fields enables proper FormData extraction.
100-100
: LGTM on form field naming.Adding
name
attributes to form fields enables proper FormData extraction.frontend/src/components/pages/SignUpPage.tsx (3)
29-40
: LGTM!The AuthLayout component provides a clean, reusable wrapper with proper responsive design and smooth animations.
42-52
: LGTM!The InputField component is well-designed with proper TypeScript typing, icon integration, and consistent styling. Good use of prop spreading for flexibility.
55-58
: LGTM!Clean component setup with appropriate state management and hook usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🧹 Nitpick comments (6)
frontend/src/components/pages/ResetPasswrdPage.tsx (4)
83-85
: Use strict equality and fix grammarMinor correctness and UX polish.
- if (password != repassword) { - toast.error("Passwords doesn't match. Try Again"); + if (password !== repassword) { + toast.error("Passwords do not match. Try again."); return; }
88-95
: Optional: confirm success to the user before redirectA quick feedback toast improves UX.
- if (error) { + if (error) { toast.error(error.message || "An unknown error occurred!"); return; } - navigate("/"); + toast.success("Password updated successfully."); + navigate("/", { replace: true });
111-118
: Client-side validation hints for password fieldsAdd
autoComplete="new-password"
and a minimal length consistent with Supabase (default 6). This improves UX and browser behavior.<InputField icon={Key} type="password" name="password-1" className="mb-7" placeholder="New Password" + autoComplete="new-password" + minLength={6} required /> <InputField icon={Key} type="password" name="password-2" className="mb-7" placeholder="Reenter Password" + autoComplete="new-password" + minLength={6} required />Also applies to: 119-126
21-33
: Deduplicate local Auth UI (DRY)
AuthLayout
andInputField
are likely repeated across auth pages. Centralize them under a sharedcomponents/auth/
module to reduce duplication and ensure consistent fixes/styles in one place.I can draft a shared
AuthLayout
andInputField
with full type safety and className merging. Want me to open a follow-up PR?Also applies to: 34-44
frontend/src/components/pages/ForgotPasswrdPage.tsx (2)
36-46
: Caller-suppliedclassName
is lost
<InputField>
spreads...props
before hard-codingclassName
, so anyclassName
passed by the caller (e.g.mb-7
) is overridden. Merge the classes instead:- <input - {...props} - className="block w-full pl-10 pr-3 py-2 border ... " - /> + const { className, ...rest } = props; + <input + {...rest} + className={`block w-full pl-10 pr-3 py-2 border ... ${className ?? ''}`} + />
49-49
: Typo in component/file name
ForgotPasswrdPage
is missing an “o”. Standardising toForgotPasswordPage
improves discoverability and avoids future import mistakes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/src/App.tsx
(2 hunks)frontend/src/components/pages/ForgotPasswrdPage.tsx
(1 hunks)frontend/src/components/pages/LoginPage.tsx
(4 hunks)frontend/src/components/pages/ResetPasswrdPage.tsx
(1 hunks)frontend/src/components/pages/SignUpPage.tsx
(1 hunks)frontend/src/lib/supabaseClient.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/lib/supabaseClient.tsx
- frontend/src/components/pages/SignUpPage.tsx
- frontend/src/App.tsx
- frontend/src/components/pages/LoginPage.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/pages/ForgotPasswrdPage.tsx (2)
frontend/src/lib/supabaseClient.tsx (1)
supabase
(12-12)backend/app/core/config/settings.py (1)
Settings
(8-51)
Hey @PinJinx , |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
frontend/src/components/pages/ResetPasswordPage.tsx (4)
67-90
: Validatetype=recovery
before setting sessionSupabase recovery links include
type=recovery
. Guarding on this reduces accidental entry to this page with non-recovery links.- if (accessToken && refreshToken) { + const type = params.get('type'); + if (type === 'recovery' && accessToken && refreshToken) { (async () => { try { const { error } = await supabase.auth.setSession({ access_token: accessToken, refresh_token: refreshToken, }); if (error) { toast.error("Error setting session: " + error.message); navigate('/login', { replace: true }); } } catch { toast.error("Error setting session"); navigate('/login', { replace: true }); } finally { clearUrlHash(); } })(); } else { - toast.error("Access denied"); + toast.error("Invalid or expired recovery link"); navigate('/login', { replace: true }); clearUrlHash(); }
98-101
: Fix grammar in mismatch message- toast.error("Passwords doesn't match. Try Again"); + toast.error("Passwords don't match. Try again.");
146-161
: Improve UX and browser behavior: add autocomplete, minlength, and aria-labelsThese help password managers and accessibility, and align with your validation rules.
<InputField icon={Key} type="password" name="password-1" - className="mb-7" + className="mb-7" placeholder="New Password" + aria-label="New password" + autoComplete="new-password" + minLength={8} required /> <InputField icon={Key} type="password" name="password-2" - className="mb-7" + className="mb-7" placeholder="Reenter Password" + aria-label="Confirm new password" + autoComplete="new-password" + minLength={8} required />
135-135
: Mount a single Toaster at the app rootConsider moving
<Toaster />
to a top-level (e.g., App.tsx) to avoid multiple instances across pages and reduce DOM duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/App.tsx
(3 hunks)frontend/src/components/pages/ForgotPasswrdPage.tsx
(1 hunks)frontend/src/components/pages/ResetPasswordPage.tsx
(1 hunks)frontend/src/components/pages/SignUpPage.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/pages/ForgotPasswrdPage.tsx
- frontend/src/App.tsx
- frontend/src/components/pages/SignUpPage.tsx
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
frontend/src/components/pages/ResetPasswordPage.tsx (2)
2-2
: Import React types explicitly and avoidReact.
namespace referencesMatches prior feedback: import the types and update the interface to prevent namespace issues in strict TS configs.
-import type { ReactNode, FormEvent } from "react"; +import type { ReactNode, FormEvent, InputHTMLAttributes, ElementType } from "react"; @@ -interface InputFieldProps extends React.InputHTMLAttributes<HTMLInputElement> { - icon: React.ElementType; +interface InputFieldProps extends InputHTMLAttributes<HTMLInputElement> { + icon: ElementType; }Also applies to: 17-19
35-45
: Preserve caller-provided className in InputFieldCurrent
{...props}
+ fixedclassName
drops caller classes (e.g., Line 155, 163). Merge them.-const InputField = ({ icon: Icon, ...props }: InputFieldProps) => ( +const InputField = ({ icon: Icon, className, ...props }: InputFieldProps) => ( <div className="relative"> <div className="absolute inset-y-0 left-0 pl-3 flex items-center pointer-events-none"> <Icon className="h-5 w-5 text-gray-400" /> </div> <input - {...props} - className="block w-full pl-10 pr-3 py-2 border border-gray-800 rounded-lg bg-gray-900 text-white placeholder-gray-400 focus:outline-none focus:ring-2 focus:ring-green-500 focus:border-transparent" + {...props} + className={`block w-full pl-10 pr-3 py-2 border border-gray-800 rounded-lg bg-gray-900 text-white placeholder-gray-400 focus:outline-none focus:ring-2 focus:ring-green-500 focus:border-transparent ${className ?? ""}`} /> </div> );
🧹 Nitpick comments (3)
frontend/src/components/pages/ResetPasswordPage.tsx (3)
98-101
: Use strict comparison and fix grammarAvoid type-coercing
!=
and tweak the message.- if (password != repassword) { - toast.error("Passwords doesn't match. Try Again"); + if (password !== repassword) { + toast.error("Passwords don't match. Try again."); return; }
118-121
: Refine “special character” check (spaces currently count as special)
/[^A-Za-z0-9]/
matches spaces and other whitespace. If spaces shouldn’t satisfy the rule, use a stricter check.- if (!/[^A-Za-z0-9]/.test(password)) { - toast.error("Password must contain at least one special character."); + if (!/[^\w\s]/.test(password)) { + toast.error("Password must contain at least one non-alphanumeric symbol (excluding spaces)."); return; }If you prefer an allowlist, we can switch to an explicit symbol set (e.g.,
!@#$...
). Confirm desired policy and I’ll adjust.
151-166
: Improve UX and browser integration for password fieldsAdd
autoComplete="new-password"
andminLength
hints; consider ARIA labels.<InputField icon={Key} type="password" name="password-1" className="mb-7" placeholder="New Password" + autoComplete="new-password" + minLength={8} + aria-label="New password" required /> <InputField icon={Key} type="password" name="password-2" className="mb-7" placeholder="Reenter Password" + autoComplete="new-password" + minLength={8} + aria-label="Confirm new password" required />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/pages/ResetPasswordPage.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/pages/ResetPasswordPage.tsx (2)
frontend/src/lib/supabaseClient.tsx (1)
supabase
(12-12)backend/app/core/config/settings.py (1)
Settings
(8-51)
🪛 Biome (2.1.2)
frontend/src/components/pages/ResetPasswordPage.tsx
[error] 202-202: expected }
but instead the file ends
the file ends here
(parse)
🔇 Additional comments (1)
frontend/src/components/pages/ResetPasswordPage.tsx (1)
122-135
: Good: robust loading state and success feedback aroundupdateUser
This addresses prior feedback: loading is reset via
finally
, errors are handled, and success shows a toast before navigating.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few things to be taken care of -
- Even if the user has never registered, the forgot password option still sends a mail to them. However it does nothing and just redirects to login.
- Could you please add on a logout button too in the profile section. The only way to logout currently is by clearing browser cache.
- Auth state not subscribed globally (can break post-reset flow). Only an initial
getSession()
runs; the app doesn't react to token changes (confirm email, password reset, sign-out in other tabs). Something likeonAuthStateChange
should be implemented. - Multiple toasters being rendered throughout. There's a global toaster in App. Please keep a single global toaster.
- supabaseClient.tsx contains no JSX; rename to supabaseClient.ts.
- .env example is missing VITE_BASE_URL.
- reset-password seems to be not implement. It can probably be added in the profile section.
Could you please resolve 1, 3, 4, 5, 6.
Rest for 2 and 7 separate issue can be raised.
Thanks a lot for you contribution. Rest PR seems fine to me.
<p className="text-center text-gray-400 text-sm"> | ||
<button | ||
type="button" | ||
onClick={() => navigate('/signup')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should re-direct to sign in. It says "Back to SignIn" but redirect to SignUp
Closes #115
📝 Description
In this PR i have added a basic Authentication system for the frontend with supabases built in email authentication system.
🔧 Changes Made
📷 Screenshots or Visual Changes (if applicable)
Login Screen
SignUp Screen
After SignUp Screen
🤝 Collaboration
nil
✅ Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Chores