Skip to content
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

implemented admin login/signup #442

Merged
merged 3 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
7 changes: 6 additions & 1 deletion backend/controller/admin.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,16 @@ async function loginAdmin(req, res) {
const token = jwt.sign(payload, process.env.JWT_SECRET, {
expiresIn: "1h",
});
res.cookie("authToken", token, {
maxAge: 1000 * 60 * 60,
httpOnly: true,
secure: true,
});
Comment on lines +76 to +80
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance cookie security configuration

While the basic security settings are good, consider strengthening the cookie configuration:

Add sameSite and domain attributes to prevent CSRF attacks and restrict cookie scope:

 res.cookie("authToken", token, {
   maxAge: 1000 * 60 * 60,
   httpOnly: true,               
   secure: true,                
+  sameSite: 'strict',          // Prevent CSRF attacks
+  domain: process.env.COOKIE_DOMAIN, // Restrict to specific domain
 });

Committable suggestion was skipped due to low confidence.

res.json({
message: "Login successful",
token,
role: "admin",
admin: { id: admin._id, name: admin.name, email: admin.email },
admin: { id: admin._id, name: admin.name, email: admin.email, role: "admin" },
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor role handling and minimize sensitive data exposure

The response includes more admin details than might be necessary, and the role is hardcoded in multiple places.

Consider these improvements:

- admin: { id: admin._id, name: admin.name, email: admin.email, role: "admin" },
+ admin: { 
+   id: admin._id, 
+   name: admin.name,
+   role: admin.role || 'admin' // Get role from DB model if available
+ },

Also, consider:

  1. Adding a role field to the Admin model schema
  2. Creating a constant for role types
  3. Implementing a method to filter sensitive admin data before sending in response

Committable suggestion was skipped due to low confidence.

});
} catch (error) {
logger.error("Error logging in admin:", {
Expand Down
1 change: 1 addition & 0 deletions backend/controller/customer.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ async function loginCustomer(req, res) {
id: customer._id,
name: customer.name,
email: customer.email,
role: "customer"
},
});
} catch (error) {
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/Pages/Admin.jsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { message } from 'antd';
import React, { useEffect, useState } from 'react';
import { useNavigate } from 'react-router-dom';
import { useUser } from '../../context/userContext';

const Admin = () => {
const [events, setEvents] = useState([]);
const [error, setError] = useState(null);
const navigate = useNavigate();
const {user} = useUser();

// Fetch all events
const fetchData = async () => {
Expand Down Expand Up @@ -122,7 +124,7 @@ const Admin = () => {
<div className="h-fit min-h-screen w-screen flex flex-col items-center justify-start p-12 pt-[10vh]">
<div className="Header w-full flex flex-col items-center">
<h1 className="title text-[#323232] font-black text-7xl mb-6">
Hi {Admin.name}!
Hi {user.name}!
</h1>
<h1 className="mt-[-2vh] text-[#666] font-semibold text-2xl">
Welcome to Admin Panel
Expand Down
150 changes: 150 additions & 0 deletions frontend/src/components/Pages/Admin/AdminLogin.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
import React, { useState, useEffect } from 'react';
import photo from '../../../assets/login.png';
import { Link, useNavigate } from 'react-router-dom';
import { message } from 'antd';
import Cookies from 'js-cookie';
import { FaEye } from 'react-icons/fa';
import { FaEyeSlash } from 'react-icons/fa6';
import { useUser } from '../../../context/userContext';

const AdminLogin = () => {
const API_URL = import.meta.env.VITE_BACKEND_URL || 'http://localhost:3000';
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using a more secure API URL configuration.

The current fallback to localhost could potentially cause issues in different environments.

Consider using a configuration object:

-const API_URL = import.meta.env.VITE_BACKEND_URL || 'http://localhost:3000';
+const API_URL = import.meta.env.VITE_BACKEND_URL;
+if (!API_URL) {
+  throw new Error('Backend URL configuration is missing');
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const API_URL = import.meta.env.VITE_BACKEND_URL || 'http://localhost:3000';
const API_URL = import.meta.env.VITE_BACKEND_URL;
if (!API_URL) {
throw new Error('Backend URL configuration is missing');
}

const [data, setData] = useState({
email: '',
password: '',
});
const [hidden, setHidden] = useState(true);
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState(null);
const {user, setUser} = useUser();

const navigate = useNavigate();

const handleChange = (e) => {
setData({ ...data, [e.target.name]: e.target.value });
};

const handleSubmit = async (e) => {
e.preventDefault();
setIsLoading(true);
setError(null);
try {
const response = await fetch(`${API_URL}/api/admin/login`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify(data),
});
Comment on lines +41 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add CSRF protection to API requests.

The current implementation lacks CSRF protection for the login endpoint.

Consider adding CSRF token to the request headers:

 const response = await fetch(`${API_URL}/api/admin/login`, {
   method: 'POST',
   headers: {
     'Content-Type': 'application/json',
+    'X-CSRF-Token': document.querySelector('meta[name="csrf-token"]').content,
   },
   body: JSON.stringify(data),
 });

Committable suggestion was skipped due to low confidence.

const result = await response.json();
if (!response.ok) {
throw new Error(result.message || 'Login failed');
}
Comment on lines +49 to +51
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance error handling with more specific error messages.

The current error handling could be more informative by providing specific error messages based on different error scenarios.

 if (!response.ok) {
-  throw new Error(result.message || 'Login failed');
+  const errorMessage = {
+    401: 'Invalid email or password',
+    403: 'Account not verified',
+    429: 'Too many login attempts. Please try again later',
+  }[response.status] || result.message || 'Login failed';
+  throw new Error(errorMessage);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!response.ok) {
throw new Error(result.message || 'Login failed');
}
if (!response.ok) {
const errorMessage = {
401: 'Invalid email or password',
403: 'Account not verified',
429: 'Too many login attempts. Please try again later',
}[response.status] || result.message || 'Login failed';
throw new Error(errorMessage);
}

const res = JSON.stringify(result.admin)
Cookies.set('authToken', result.token, { expires: 1, secure: true });
Cookies.set("authenticatedUser", res, {expires: 1, secure: true})
Comment on lines +53 to +54
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance cookie security settings.

The current cookie configuration could be strengthened to better protect against XSS and CSRF attacks.

Apply these security enhancements:

-Cookies.set('authToken', result.token, { expires: 1, secure: true });
-Cookies.set("authenticatedUser", res, {expires: 1, secure: true})
+Cookies.set('authToken', result.token, {
+  expires: 1,
+  secure: true,
+  sameSite: 'strict',
+  path: '/'
+});
+Cookies.set("authenticatedUser", res, {
+  expires: 1,
+  secure: true,
+  sameSite: 'strict',
+  path: '/'
+});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Cookies.set('authToken', result.token, { expires: 1, secure: true });
Cookies.set("authenticatedUser", res, {expires: 1, secure: true})
Cookies.set('authToken', result.token, {
expires: 1,
secure: true,
sameSite: 'strict',
path: '/'
});
Cookies.set("authenticatedUser", res, {
expires: 1,
secure: true,
sameSite: 'strict',
path: '/'
});

setUser(result.admin)
message.success('Login successful');
navigate('/admin');
} catch (err) {
setError(err.message || 'An error occurred. Please try again.');
} finally {
setIsLoading(false);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add input validation before form submission.

The form submission lacks client-side validation for email and password fields.

Add validation before submission:

 const handleSubmit = async (e) => {
   e.preventDefault();
+  // Validate email format
+  const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
+  if (!emailRegex.test(data.email)) {
+    setError('Please enter a valid email address');
+    return;
+  }
+  // Validate password
+  if (data.password.length < 8) {
+    setError('Password must be at least 8 characters long');
+    return;
+  }
   setIsLoading(true);
   setError(null);
   try {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleSubmit = async (e) => {
e.preventDefault();
setIsLoading(true);
setError(null);
try {
const response = await fetch(`${API_URL}/api/admin/login`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify(data),
});
const result = await response.json();
if (!response.ok) {
throw new Error(result.message || 'Login failed');
}
const res = JSON.stringify(result.admin)
Cookies.set('authToken', result.token, { expires: 1, secure: true });
Cookies.set("authenticatedUser", res, {expires: 1, secure: true})
setUser(result.admin)
message.success('Login successful');
navigate('/admin');
} catch (err) {
setError(err.message || 'An error occurred. Please try again.');
} finally {
setIsLoading(false);
}
};
const handleSubmit = async (e) => {
e.preventDefault();
// Validate email format
const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
if (!emailRegex.test(data.email)) {
setError('Please enter a valid email address');
return;
}
// Validate password
if (data.password.length < 8) {
setError('Password must be at least 8 characters long');
return;
}
setIsLoading(true);
setError(null);
try {
const response = await fetch(`${API_URL}/api/admin/login`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify(data),
});
const result = await response.json();
if (!response.ok) {
throw new Error(result.message || 'Login failed');
}
const res = JSON.stringify(result.admin)
Cookies.set('authToken', result.token, { expires: 1, secure: true });
Cookies.set("authenticatedUser", res, {expires: 1, secure: true})
setUser(result.admin)
message.success('Login successful');
navigate('/admin');
} catch (err) {
setError(err.message || 'An error occurred. Please try again.');
} finally {
setIsLoading(false);
}
};


useEffect(() => {
window.scrollTo(0, 0);
}, []);

return (
<div className="w-screen h-screen dark:bg-black flex items-center justify-center lg:pt-10 px-4">
{/* Background Image */}
<img
src={photo}
alt="login"
loading="lazy"
className="absolute w-3/4 lg:w-auto lg:opacity-100 opacity-10 object-cover"
/>
{/* Login Form */}
<form
onSubmit={handleSubmit}
className="z-10 p-8 lg:p-14 bg-[#f1e9dc] dark:bg-amber-800 dark:text-white flex flex-col gap-6 rounded-lg border-2 border-black shadow-[4px_4px_0px_0px_black] w-full max-w-md lg:max-w-xl"
>
<div className="text-[#323232] dark:text-white font-black text-4xl lg:text-6xl mb-2">
Admin Login,
<span className="block text-[#666] dark:text-gray-400 font-semibold text-lg lg:text-2xl mt-1">
Log in to continue
</span>
</div>

<input
className="w-full h-12 rounded-md border-2 border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[15px] font-semibold text-[#323232] p-2.5 focus:outline-none focus:border-[#2d8cf0] placeholder-[#666]"
name="email"
placeholder="Email"
type="email"
onChange={handleChange}
/>
Comment on lines +90 to +96
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance form accessibility.

The form inputs lack proper accessibility attributes.

Add necessary accessibility attributes:

+<label htmlFor="email" className="sr-only">Email</label>
 <input
+  id="email"
   className="w-full h-12 rounded-md border-2 border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[15px] font-semibold text-[#323232] p-2.5 focus:outline-none focus:border-[#2d8cf0] placeholder-[#666]"
   name="email"
   placeholder="Email"
   type="email"
+  aria-required="true"
+  aria-invalid={error ? "true" : "false"}
   onChange={handleChange}
 />

 <div className="relative w-full">
+  <label htmlFor="password" className="sr-only">Password</label>
   <input
+    id="password"
     className="w-full h-12 rounded-md border-2 border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[15px] font-semibold text-[#323232] p-2.5 focus:outline-none focus:border-[#2d8cf0] placeholder-[#666]"
     name="password"
     placeholder="Password"
     type={hidden ? 'password' : 'text'}
+    aria-required="true"
+    aria-invalid={error ? "true" : "false"}
     onChange={handleChange}
   />
   <button
     className="absolute top-1/2 transform -translate-y-1/2 right-4"
+    aria-label={hidden ? "Show password" : "Hide password"}
     onClick={(e) => {

Also applies to: 89-106


<div className="relative w-full">
<input
className="w-full h-12 rounded-md border-2 border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[15px] font-semibold text-[#323232] p-2.5 focus:outline-none focus:border-[#2d8cf0] placeholder-[#666]"
name="password"
placeholder="Password"
type={hidden ? 'password' : 'text'}
onChange={handleChange}
/>
<button
className="absolute top-1/2 transform -translate-y-1/2 right-4"
onClick={(e) => {
e.preventDefault();
setHidden(!hidden);
}}
>
{hidden ? <FaEyeSlash /> : <FaEye />}
</button>
</div>

<Link
to="/email-verify"
className="text-sm lg:text-base text-gray-500 dark:text-gray-200 hover:text-red-500 transition"
>
Forgot Password?
</Link>

<h3 className="flex justify-between items-center w-full text-sm lg:text-base">
Don’t have an account?
<Link
to="/admin-signup"
className="text-green-500 font-semibold hover:scale-110 transition"
>
Register Here
</Link>
</h3>

<a
href={`${API_URL}/api/user/auth/google`}
className="w-full"
>
<button
type="button"
className="w-full h-12 rounded-md border-2 dark:text-white border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[17px] font-semibold text-[#323232] transition active:translate-x-[3px] active:translate-y-[3px]"
>
Sign in with Google
</button>
</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Reconsider direct Google auth href implementation.

The current implementation of Google authentication might be vulnerable to open redirects.

Consider using a more secure approach:

-<a
-  href={`${API_URL}/api/user/auth/google`}
-  className="w-full"
->
+<button
+  onClick={() => {
+    window.location.href = `${API_URL}/api/user/auth/google/admin`;
+  }}
+  className="w-full"
+>
   <button
     type="button"
     className="w-full h-12 rounded-md border-2 dark:text-white border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[17px] font-semibold text-[#323232] transition active:translate-x-[3px] active:translate-y-[3px]"
   >
     Sign in with Google
   </button>
-</a>
+</button>

Committable suggestion was skipped due to low confidence.


{error && <p className="text-red-500 mt-2">{error}</p>}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error message accessibility and visibility.

The error message display could be enhanced for better accessibility and user experience.

-{error && <p className="text-red-500 mt-2">{error}</p>}
+{error && (
+  <div
+    role="alert"
+    aria-live="polite"
+    className="p-3 bg-red-100 border border-red-400 text-red-700 rounded relative"
+  >
+    <span className="block sm:inline">{error}</span>
+  </div>
+)}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{error && <p className="text-red-500 mt-2">{error}</p>}
{error && (
<div
role="alert"
aria-live="polite"
className="p-3 bg-red-100 border border-red-400 text-red-700 rounded relative"
>
<span className="block sm:inline">{error}</span>
</div>
)}


<button
type="submit"
className="w-full h-12 rounded-md dark:text-white border-2 border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[17px] font-semibold text-[#323232] transition active:translate-x-[3px] active:translate-y-[3px]"
>
{isLoading ? 'Loading...' : 'Let’s Log you in →'}
</button>
Comment on lines +148 to +153
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance loading state handling.

The current loading state implementation could be improved to prevent multiple form submissions and provide better visual feedback.

 <button
   type="submit"
+  disabled={isLoading}
   className="w-full h-12 rounded-md dark:text-white border-2 border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[17px] font-semibold text-[#323232] transition active:translate-x-[3px] active:translate-y-[3px]"
 >
-  {isLoading ? 'Loading...' : 'Let's Log you in →'}
+  {isLoading ? (
+    <span className="flex items-center justify-center">
+      <svg className="animate-spin -ml-1 mr-3 h-5 w-5 text-white" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24">
+        <circle className="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" strokeWidth="4"></circle>
+        <path className="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z"></path>
+      </svg>
+      Signing in...
+    </span>
+  ) : (
+    'Sign in →'
+  )}
 </button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button
type="submit"
className="w-full h-12 rounded-md dark:text-white border-2 border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[17px] font-semibold text-[#323232] transition active:translate-x-[3px] active:translate-y-[3px]"
>
{isLoading ? 'Loading...' : 'Let’s Log you in →'}
</button>
<button
type="submit"
disabled={isLoading}
className="w-full h-12 rounded-md dark:text-white border-2 border-black bg-beige shadow-[4px_4px_0px_0px_black] text-[17px] font-semibold text-[#323232] transition active:translate-x-[3px] active:translate-y-[3px]"
>
{isLoading ? (
<span className="flex items-center justify-center">
<svg className="animate-spin -ml-1 mr-3 h-5 w-5 text-white" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24">
<circle className="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" strokeWidth="4"></circle>
<path className="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z"></path>
</svg>
Signing in...
</span>
) : (
'Sign in →'
)}
</button>

</form>
</div>
);
};

export default AdminLogin;
182 changes: 182 additions & 0 deletions frontend/src/components/Pages/Admin/AdminSignup.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
import { useState, useEffect } from 'react';
import photo from '../../../assets/login.png';
import { useNavigate, Link } from 'react-router-dom';
import { FaEye } from 'react-icons/fa';
import { FaEyeSlash } from 'react-icons/fa6';
import zxcvbn from 'zxcvbn'; // Password strength checker

const AdminSignup = () => {
const API_URL = import.meta.env.VITE_BACKEND_URL || 'http://localhost:3000';
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance API URL configuration security.

The fallback URL should be configured based on the environment to prevent potential localhost exposure in production.

Consider this approach:

-const API_URL = import.meta.env.VITE_BACKEND_URL || 'http://localhost:3000';
+const API_URL = import.meta.env.VITE_BACKEND_URL;
+if (!API_URL) {
+  throw new Error('VITE_BACKEND_URL environment variable is not set');
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const API_URL = import.meta.env.VITE_BACKEND_URL || 'http://localhost:3000';
const API_URL = import.meta.env.VITE_BACKEND_URL;
if (!API_URL) {
throw new Error('VITE_BACKEND_URL environment variable is not set');
}

const navigate = useNavigate();
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState(null);
const [passwordStrength, setPasswordStrength] = useState(0);
const [data, setData] = useState({ name: '', email: '', password: '' });
const [hidden, setHidden] = useState(true);
Comment on lines +11 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding PropTypes or TypeScript for better type safety.

The state variables would benefit from type validation to prevent potential runtime errors and improve code maintainability.

Consider migrating to TypeScript or adding PropTypes:

AdminSignup.propTypes = {
  initialData: PropTypes.shape({
    name: PropTypes.string,
    email: PropTypes.string,
    password: PropTypes.string
  })
};


const handleChange = (e) => {
setData({ ...data, [e.target.name]: e.target.value });
if (e.target.name === 'password') {
const result = zxcvbn(e.target.value);
setPasswordStrength(result.score);
}
};

const handleSubmit = async (e) => {
e.preventDefault();
setIsLoading(true);

// Input validation
if (!data.email || !data.password || !data.name) {
setError('Please fill in all fields');
setIsLoading(false);
return;
}
if (data.password.length < 8) {
setError('Password must be at least 8 characters long');
setIsLoading(false);
return;
}
if (data.name.length < 3) {
setError('Name must be at least 3 characters long');
setIsLoading(false);
return;
}
if (!data.email.includes('@')) {
setError('Please enter a valid email address');
setIsLoading(false);
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance email validation security.

The current email validation is too basic and could allow invalid email formats.

Consider using a proper email validation regex or a validation library:

-if (!data.email.includes('@')) {
+const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
+if (!emailRegex.test(data.email)) {
  setError('Please enter a valid email address');
  setIsLoading(false);
  return;
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!data.email.includes('@')) {
setError('Please enter a valid email address');
setIsLoading(false);
return;
}
const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
if (!emailRegex.test(data.email)) {
setError('Please enter a valid email address');
setIsLoading(false);
return;
}


try {
const response = await fetch(`${API_URL}/api/admin/register`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(data),
});
const result = await response.json();

if (!response.ok) {
setIsLoading(false);
setError(result.error);
return;
}

Comment on lines +52 to +65
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance API request security.

The API request lacks CSRF protection and rate limiting considerations.

Consider these security enhancements:

 try {
   const response = await fetch(`${API_URL}/api/admin/register`, {
     method: 'POST',
     headers: { 
       'Content-Type': 'application/json',
+      'X-CSRF-Token': document.querySelector('meta[name="csrf-token"]').content
     },
     credentials: 'include',
     body: JSON.stringify(data),
   });

Committable suggestion was skipped due to low confidence.


// alert('OTP sent to your email. Verify to complete registration.');
navigate('/admin-login');

} catch (error) {
setError(error.message);
console.error('Error:', error);
} finally {
setIsLoading(false); // Ensure loading state is reset after request
}
};
Comment on lines +52 to +76
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Implement rate limiting for failed signup attempts.

As this is an admin signup endpoint, it should be protected against brute force attempts.

Consider implementing a client-side attempt counter:

const MAX_ATTEMPTS = 3;
const LOCKOUT_TIME = 15 * 60 * 1000; // 15 minutes

const [attempts, setAttempts] = useState(0);
const [lockoutUntil, setLockoutUntil] = useState(null);

// In handleSubmit, before API call:
if (lockoutUntil && Date.now() < lockoutUntil) {
  setError(`Too many attempts. Please try again in ${Math.ceil((lockoutUntil - Date.now()) / 60000)} minutes`);
  return;
}

// After failed attempt:
if (!response.ok) {
  setAttempts(prev => {
    if (prev + 1 >= MAX_ATTEMPTS) {
      setLockoutUntil(Date.now() + LOCKOUT_TIME);
      return 0;
    }
    return prev + 1;
  });
}



useEffect(() => {
window.scrollTo(0, 0);
}, []);
Comment on lines +79 to +81
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance scroll behavior accessibility.

The current scroll implementation might interfere with assistive technologies.

Consider using a more accessible approach:

useEffect(() => {
  // Announce page load to screen readers
  const pageTitle = document.title;
  document.title = "Admin Signup - " + pageTitle;
  
  // Smooth scroll to top
  window.scrollTo({
    top: 0,
    behavior: 'smooth'
  });
  
  return () => {
    document.title = pageTitle;
  };
}, []);


const getPasswordStrengthColor = (score) => {
const colors = ['#ff4d4d', '#ff944d', '#ffd24d', '#d2ff4d', '#4dff88'];
return colors[score];
};

const getPasswordStrengthText = (score) => {
const strengthLevels = ['Very Weak', 'Weak', 'Okay', 'Good', 'Strong'];
return strengthLevels[score];
};
Comment on lines +88 to +91
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance password strength feedback.

The current strength indicators could be more informative to help users create better passwords.

Consider enhancing the feedback:

 const getPasswordStrengthText = (score) => {
-  const strengthLevels = ['Very Weak', 'Weak', 'Okay', 'Good', 'Strong'];
+  const strengthLevels = [
+    'Very Weak - Add more characters',
+    'Weak - Try adding numbers',
+    'Okay - Consider special characters',
+    'Good - Length is important too',
+    'Strong - Excellent password!'
+  ];
   return strengthLevels[score];
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const getPasswordStrengthText = (score) => {
const strengthLevels = ['Very Weak', 'Weak', 'Okay', 'Good', 'Strong'];
return strengthLevels[score];
};
const getPasswordStrengthText = (score) => {
const strengthLevels = [
'Very Weak - Add more characters',
'Weak - Try adding numbers',
'Okay - Consider special characters',
'Good - Length is important too',
'Strong - Excellent password!'
];
return strengthLevels[score];
};


return (
<div className="w-screen h-screen flex items-center dark:text-white dark:bg-black justify-center pt-10 relative overflow-hidden">
<img
src={photo}
alt="login"
loading="lazy"
className="absolute w-3/4 lg:w-auto lg:opacity-100 opacity-10 object-cover"
/>
<form className="z-10 p-8 sm:p-16 bg-[#f1e9dc] dark:bg-amber-800 dark:text-white rounded-lg border-2 border-black shadow-[4px_4px_0px_0px_black] flex flex-col gap-5 w-11/12 sm:w-auto px-[3vw] pt-[5vh]">
<div className="text-[#323232] dark:text-white font-black text-4xl sm:text-7xl mb-4 sm:mb-6 mt-4">
Admin SignUp
<br />
<span className="block text-[#666] dark:text-gray-400 font-semibold text-xl sm:text-2xl">
Register to continue
</span>
</div>
<input
className="input w-full h-10 rounded-md border-2 border-black bg-beige p-2.5 shadow-[4px_4px_0px_0px_black] focus:outline-none"
name="name"
placeholder="Name"
type="text"
onChange={handleChange}
/>
Comment on lines +109 to +115
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add accessibility attributes to form inputs.

The form inputs lack proper accessibility attributes.

Add necessary ARIA attributes and labels:

+<label htmlFor="name" className="sr-only">Name</label>
 <input
   className="input w-full h-10 rounded-md border-2 border-black bg-beige p-2.5 shadow-[4px_4px_0px_0px_black] focus:outline-none"
   name="name"
+  id="name"
   placeholder="Name"
   type="text"
+  aria-required="true"
   onChange={handleChange}
 />

Also applies to: 115-121, 123-129

<input
className="input w-full h-10 rounded-md border-2 border-black bg-beige p-2.5 shadow-[4px_4px_0px_0px_black] focus:outline-none"
name="email"
placeholder="Email"
type="email"
onChange={handleChange}
/>
<div className="relative w-full">
<input
className="input w-full h-10 rounded-md border-2 border-black bg-beige p-2.5 shadow-[4px_4px_0px_0px_black] focus:outline-none"
name="password"
placeholder="Password"
type={hidden ? 'password' : 'text'}
onChange={handleChange}
/>
<button
className="absolute top-1/2 -translate-y-1/2 right-4"
onClick={(e) => {
e.preventDefault();
setHidden(!hidden);
}}
>
{hidden ? <FaEyeSlash /> : <FaEye />}
</button>
</div>
<div className="w-full mt-2">
<div
className="h-2 rounded-full"
style={{
backgroundColor: getPasswordStrengthColor(passwordStrength),
width: `${(passwordStrength + 1) * 20}%`,
}}
></div>
<p className="text-sm text-[#666] dark:text-gray-200 mt-1">
Strength: {getPasswordStrengthText(passwordStrength)}
</p>
</div>
{error && (
<div className="w-full p-2 bg-red-100 text-red-700 border border-red-400 rounded-md">
{error}
</div>
)}
<h3 className="flex justify-between w-full">
Already have an account?
<Link
to="/admin-login"
className="text-[#666] dark:text-white font-semibold hover:text-green-500"
>
Login
</Link>
</h3>
<a href={`${API_URL}/api/user/auth/google`} className="w-full">
<button className="button-confirm w-full h-10 rounded-md border-2 border-black bg-beige text-[17px] font-semibold shadow-[4px_4px_0px_0px_black] hover:text-green-300">
Sign up with Google
</button>
</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance Google authentication security and UX.

The Google authentication link lacks CSRF protection and loading state handling.

Consider these improvements:

-<a href={`${API_URL}/api/user/auth/google`} className="w-full">
+<a 
+  href={`${API_URL}/api/user/auth/google?csrf=${document.querySelector('meta[name="csrf-token"]').content}`}
+  className="w-full"
+  onClick={(e) => {
+    e.preventDefault();
+    setIsLoading(true);
+    window.location.href = e.target.href;
+  }}
+>
   <button className="button-confirm w-full h-10 rounded-md border-2 border-black bg-beige text-[17px] font-semibold shadow-[4px_4px_0px_0px_black] hover:text-green-300">
-    Sign up with Google
+    {isLoading ? 'Redirecting...' : 'Sign up with Google'}
   </button>
 </a>

Committable suggestion was skipped due to low confidence.

<button
className="button-confirm w-full h-10 rounded-md border-2 border-black bg-beige text-[17px] font-semibold shadow-[4px_4px_0px_0px_black] mb-2 hover:text-green-300"
onClick={handleSubmit}
>
{isLoading ? 'Loading...' : "Let's go →"}
</button>
Comment on lines +172 to +177
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance loading state and form submission feedback.

The current loading state only updates the button text. Consider providing more comprehensive feedback.

 <button
   className="button-confirm w-full h-10 rounded-md border-2 border-black bg-beige text-[17px] font-semibold shadow-[4px_4px_0px_0px_black] mb-2 hover:text-green-300"
   onClick={handleSubmit}
+  disabled={isLoading}
 >
-  {isLoading ? 'Loading...' : "Let's go →"}
+  {isLoading ? (
+    <span className="flex items-center justify-center">
+      <svg className="animate-spin h-5 w-5 mr-3" viewBox="0 0 24 24">
+        <circle className="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" strokeWidth="4" fill="none" />
+        <path className="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z" />
+      </svg>
+      Creating account...
+    </span>
+  ) : (
+    "Let's go →"
+  )}
 </button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button
className="button-confirm w-full h-10 rounded-md border-2 border-black bg-beige text-[17px] font-semibold shadow-[4px_4px_0px_0px_black] mb-2 hover:text-green-300"
onClick={handleSubmit}
>
{isLoading ? 'Loading...' : "Let's go →"}
</button>
<button
className="button-confirm w-full h-10 rounded-md border-2 border-black bg-beige text-[17px] font-semibold shadow-[4px_4px_0px_0px_black] mb-2 hover:text-green-300"
onClick={handleSubmit}
disabled={isLoading}
>
{isLoading ? (
<span className="flex items-center justify-center">
<svg className="animate-spin h-5 w-5 mr-3" viewBox="0 0 24 24">
<circle className="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" strokeWidth="4" fill="none" />
<path className="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z" />
</svg>
Creating account...
</span>
) : (
"Let's go →"
)}
</button>

</form>
</div>
);
};

export default AdminSignup;
Loading
Loading