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

Restrict Event Booking to Verified Users #427

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
94 changes: 40 additions & 54 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
</a>
</td>
<td align="center">

<a href="https://github.com/haseebzaki-07">
<img src="https://avatars.githubusercontent.com/u/147314463?v=4" width="100;" alt="haseebzaki-07"/>
<br />
Expand All @@ -274,14 +275,13 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
</a>
</td>
<td align="center">

<a href="https://github.com/alo7lika">
<img src="https://avatars.githubusercontent.com/u/152315710?v=4" width="100;" alt="alo7lika"/>
<br />
<sub><b>alolika bhowmik</b></sub>
</a>
</td>
</tr>
<tr>
<td align="center">
<a href="https://github.com/Ashwinib26">
<img src="https://avatars.githubusercontent.com/u/149402720?v=4" width="100;" alt="Ashwinib26"/>
Expand All @@ -296,6 +296,8 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
<sub><b>Tejas Benibagde</b></sub>
</a>
</td>
</tr>
<tr>
haseebzaki-07 marked this conversation as resolved.
Show resolved Hide resolved
<td align="center">
<a href="https://github.com/itznayan">
<img src="https://avatars.githubusercontent.com/u/136584376?v=4" width="100;" alt="itznayan"/>
Expand All @@ -310,22 +312,6 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
<sub><b>Tyarla Shirisha</b></sub>
</a>
</td>
<td align="center">
<a href="https://github.com/meghanakn22">
<img src="https://avatars.githubusercontent.com/u/172406754?v=4" width="100;" alt="meghanakn22"/>
<br />
<sub><b>meghanakn22</b></sub>
</a>
</td>
<td align="center">
<a href="https://github.com/VinayLodhi1712">
<img src="https://avatars.githubusercontent.com/u/135756009?v=4" width="100;" alt="VinayLodhi1712"/>
<br />
<sub><b>Vinay Anand Lodhi</b></sub>
</a>
</td>
</tr>
<tr>
<td align="center">
<a href="https://github.com/Amnyadav">
<img src="https://avatars.githubusercontent.com/u/127370497?v=4" width="100;" alt="Amnyadav"/>
Expand All @@ -341,17 +327,10 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
</a>
</td>
<td align="center">
<a href="https://github.com/Suhas-Koheda">
<img src="https://avatars.githubusercontent.com/u/72063139?v=4" width="100;" alt="Suhas-Koheda"/>
<br />
<sub><b>Suhas Koheda</b></sub>
</a>
</td>
<td align="center">
<a href="https://github.com/Sumanbhadra">
<img src="https://avatars.githubusercontent.com/u/93245252?v=4" width="100;" alt="Sumanbhadra"/>
<a href="https://github.com/haseebzaki-07">
<img src="https://avatars.githubusercontent.com/u/147314463?v=4" width="100;" alt="haseebzaki-07"/>
<br />
<sub><b>Suman Bhadra</b></sub>
<sub><b>Haseeb Zaki</b></sub>
</a>
</td>
<td align="center">
Expand All @@ -361,23 +340,37 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
<sub><b>Sawan kushwah </b></sub>
</a>
</td>
</tr>
<tr>
<td align="center">

<a href="https://github.com/Suhas-Koheda">
<img src="https://avatars.githubusercontent.com/u/72063139?v=4" width="100;" alt="Suhas-Koheda"/>
<br />
<sub><b>Suhas Koheda</b></sub>

haseebzaki-07 marked this conversation as resolved.
Show resolved Hide resolved
<a href="https://github.com/Jay-1409">
<img src="https://avatars.githubusercontent.com/u/166749819?v=4" width="100;" alt="Jay-1409"/>
<br />
<sub><b>Jay shah</b></sub>

haseebzaki-07 marked this conversation as resolved.
Show resolved Hide resolved
</a>
</td>
</tr>
<tr>
<td align="center">

<a href="https://github.com/Jay-1409">
<img src="https://avatars.githubusercontent.com/u/166749819?v=4" width="100;" alt="Jay-1409"/>

<a href="https://github.com/vishnuprasad2004">
<img src="https://avatars.githubusercontent.com/u/116942066?v=4" width="100;" alt="vishnuprasad2004"/>
<br />
<sub><b>Vishnu Prasad Korada</b></sub>
</a>
</td>


<td align="center">

<a href="https://github.com/sajalbatra">
<img src="https://avatars.githubusercontent.com/u/125984550?v=4" width="100;" alt="sajalbatra"/>
<br />
Expand All @@ -389,13 +382,12 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
<img src="https://avatars.githubusercontent.com/u/98730339?v=4" width="100;" alt="PavanTeja2005"/>
<br />
<sub><b>PavanTeja2005</b></sub>
</a>
</td>
<td align="center">

<a href="https://github.com/AE-Hertz">
<img src="https://avatars.githubusercontent.com/u/93651229?v=4" width="100;" alt="AE-Hertz"/>
<br />
<sub><b>Abhinandan</b></sub>

</a>
</td>
<td align="center">
Expand All @@ -405,6 +397,8 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
<sub><b>Abhijit Motekar</b></sub>
</a>
</td>
</tr>
<tr>
<td align="center">
<a href="https://github.com/Navneetdadhich">
<img src="https://avatars.githubusercontent.com/u/156535853?v=4" width="100;" alt="Navneetdadhich"/>
Expand All @@ -414,6 +408,13 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
</td>
</tr>
<tr>
<td align="center">
<a href="https://github.com/VinayLodhi1712">
<img src="https://avatars.githubusercontent.com/u/135756009?v=4" width="100;" alt="VinayLodhi1712"/>
<br />
<sub><b>Vinay Anand Lodhi</b></sub>
</a>
</td>
<td align="center">
<a href="https://github.com/lade6501">
<img src="https://avatars.githubusercontent.com/u/83055827?v=4" width="100;" alt="lade6501"/>
Expand Down Expand Up @@ -442,13 +443,8 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
<sub><b>Aditya Bakshi</b></sub>
</a>
</td>
<td align="center">
<a href="https://github.com/vaishnavipal1869">
<img src="https://avatars.githubusercontent.com/u/180996531?v=4" width="100;" alt="vaishnavipal1869"/>
<br />
<sub><b>vaishnavipal1869</b></sub>
</a>
</td>
</tr>
<tr>
<td align="center">
<a href="https://github.com/tanishirai">
<img src="https://avatars.githubusercontent.com/u/178164785?v=4" width="100;" alt="tanishirai"/>
Expand Down Expand Up @@ -479,13 +475,6 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
<sub><b>Shiva Bajpai</b></sub>
</a>
</td>
<td align="center">
<a href="https://github.com/Pushpa472">
<img src="https://avatars.githubusercontent.com/u/116655535?v=4" width="100;" alt="Pushpa472"/>
<br />
<sub><b>Pushpa Vishwakarma </b></sub>
</a>
</td>
<td align="center">
<a href="https://github.com/devxMani">
<img src="https://avatars.githubusercontent.com/u/122438942?v=4" width="100;" alt="devxMani"/>
Expand All @@ -494,6 +483,7 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
</a>
</td>
<td align="center">

<a href="https://github.com/meghanakn473">
<img src="https://avatars.githubusercontent.com/u/165137755?v=4" width="100;" alt="meghanakn473"/>
<br />
Expand All @@ -503,6 +493,7 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
</tr>
<tr>
<td align="center">

<a href="https://github.com/Ayush215mb">
<img src="https://avatars.githubusercontent.com/u/154300084?v=4" width="100;" alt="Ayush215mb"/>
<br />
Expand Down Expand Up @@ -553,13 +544,6 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
<sub><b>Sapna Kul</b></sub>
</a>
</td>
<td align="center">
<a href="https://github.com/Nikhil0-3">
<img src="https://avatars.githubusercontent.com/u/149102391?v=4" width="100;" alt="Nikhil0-3"/>
<br />
<sub><b>Nikhil More</b></sub>
</a>
</td>
<td align="center">
<a href="https://github.com/MutiatBash">
<img src="https://avatars.githubusercontent.com/u/108807732?v=4" width="100;" alt="MutiatBash"/>
Expand All @@ -582,6 +566,7 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
</a>
</td>
<td align="center">

<a href="https://github.com/harjasae2001">
<img src="https://avatars.githubusercontent.com/u/83627055?v=4" width="100;" alt="harjasae2001"/>
<br />
Expand All @@ -591,6 +576,7 @@ We extend our heartfelt gratitude to all the amazing contributors who have made
</tr>
<tr>
<td align="center">

<a href="https://github.com/mishradev1">
<img src="https://avatars.githubusercontent.com/u/118660840?v=4" width="100;" alt="mishradev1"/>
<br />
Expand Down
2 changes: 2 additions & 0 deletions backend/controller/customer.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ async function loginCustomer(req, res) {
process.env.JWT_SECRET,
{ expiresIn: "1h" } // Expires in 1 hour
);


req.session.user = {
id: customer._id,
Expand All @@ -136,6 +137,7 @@ async function loginCustomer(req, res) {
});

return res.json({

message: "Login successful",
token,
role: "customer",
Expand Down
48 changes: 33 additions & 15 deletions backend/middlewares/authCustomer.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,42 @@
const jwt = require("jsonwebtoken");
const logger = require("../config/logger");
const config = require("../config/secret");
const Customer = require("../models/customer.model");
// Assuming the Customer model is located here

const authenticateCustomer = (req, res, next) => {
const authenticateCustomer = async (req, res, next) => {
const token = req.header("Authorization")?.split(" ")[1]; // Expecting "Bearer <token>"

if (token) {
jwt.verify(token, config.JWT_SECRET, (err, user) => {
if (err) {
if (err.name === "TokenExpiredError") {
return res.status(401).json({ message: "Token expired" });
}
return res.status(403).json({ message: "Invalid token" });
}
req.user = user;
logger.info(`Customer authenticated: ${JSON.stringify(user.username)}`);
next();
});
} else {
res.sendStatus(401); // Unauthorized
if (!token) {
return res.status(401).json({ message: "Authorization token is missing" });
}

try {
// Verify token
const decoded = jwt.verify(token, config.JWT_SECRET);

// Retrieve user from database to check verification status
const user = await Customer.findById(decoded.sub);

if (!user) {
return res.status(404).json({ message: "User not found" });
}

if (!user.isVerified) {
return res.status(403).json({ message: "Account not verified" });
}

// If verified, attach user to request and proceed
req.user = user;
logger.info(`Customer authenticated: ${user.name}`);
next();

} catch (err) {
if (err.name === "TokenExpiredError") {
return res.status(401).json({ message: "Token expired" });
}
logger.error("Token verification failed:", err);
return res.status(403).json({ message: "Invalid token" });
Comment on lines +34 to +39
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

Change status code to 401 Unauthorized for invalid token.

When token verification fails due to an invalid token, it's more appropriate to return a 401 Unauthorized response instead of 403 Forbidden. The 401 status code indicates that authentication has failed, which aligns with the nature of an invalid token.

Apply this diff to change the status code:

     logger.error("Token verification failed:", err);
-    return res.status(403).json({ message: "Invalid token" });
+    return res.status(401).json({ message: "Invalid token" });
📝 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
} catch (err) {
if (err.name === "TokenExpiredError") {
return res.status(401).json({ message: "Token expired" });
}
logger.error("Token verification failed:", err);
return res.status(403).json({ message: "Invalid token" });
} catch (err) {
if (err.name === "TokenExpiredError") {
return res.status(401).json({ message: "Token expired" });
}
logger.error("Token verification failed:", err);
return res.status(401).json({ message: "Invalid token" });

}
};

Expand Down
7 changes: 4 additions & 3 deletions backend/routes/eventRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const {
getEvents,
deleteEvent,
} = require("../controller/event.controller");
const authenticateCustomer = require("../middlewares/authCustomer");

const router = express.Router();

Expand All @@ -24,8 +25,8 @@ router.get("/", async (req, res) => {
res.status(500).json({ error: "Internal server error" });
}
});
router.post("/create", createEvent);
router.get("/all", getEvents);
router.get("/delete", deleteEvent);
router.post("/create",authenticateCustomer, createEvent);
router.get("/all",authenticateCustomer, getEvents);
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the gathered information, I can now provide a final response:

Authentication requirement for GET /all should be removed

The frontend Event.jsx component makes an unauthenticated request to fetch events (/api/event/all), while the backend requires authentication. This mismatch causes a functionality gap where:

  • The events page is publicly accessible and attempts to display events
  • The backend blocks the event data due to authentication
  • Only the "Register" action appropriately checks for authentication

Recommended changes:

  • Remove authenticateCustomer middleware from the GET /all route
  • Keep authentication for POST /create and GET /delete routes
  • Keep the frontend's authentication check for event registration
🔗 Analysis chain

Reconsider authentication requirement for GET /all

The current implementation requires authentication for viewing all events, which might be too restrictive. Consider whether events should be publicly viewable while keeping the booking functionality restricted to verified users (as per PR objectives).

Let's verify the requirements:


Let me gather more specific information about the authentication and event access patterns.


Let me try one more search to understand the event-related components and authentication flow better.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any public-facing components that might need event data
echo "Checking for frontend components that might display events..."
rg -l "events" --type js --type jsx --type ts --type tsx

# Check if there are any documentation or requirements specifying public access
echo "Checking for documentation about event visibility..."
rg -l "event.*public|public.*event" --type md

Length of output: 399


Script:

#!/bin/bash
# Check the event-related routes and their authentication requirements
echo "Checking event routes and authentication patterns..."
rg -A 5 -B 5 "router\.(get|post|put|delete).*event" --type js

# Check for any frontend API calls or services related to events
echo "Checking frontend API calls..."
rg -l "api.*event|event.*api" --type js

# Check for any documentation about authentication requirements
echo "Checking authentication requirements..."
rg "auth.*require|require.*auth" --type md README.md

Length of output: 515


Script:

#!/bin/bash
# Check the content of event-related frontend components
echo "Checking Event.jsx content..."
cat frontend/src/components/Pages/Event.jsx

# Check the event router implementation
echo "Checking eventRouter.js content..."
cat backend/routes/eventRouter.js

# Look for any authentication middleware implementation
echo "Checking authentication middleware..."
fd authenticateCustomer --type f --exec cat {}

Length of output: 12050

router.get("/delete",authenticateCustomer, deleteEvent);
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

Replace GET /delete with DELETE method

Using GET for delete operations violates RESTful principles and can lead to unintended consequences (e.g., search engine crawlers accidentally deleting resources).

Apply this change:

-router.get("/delete", authenticateCustomer, deleteEvent);
+router.delete("/:id", authenticateCustomer, deleteEvent);
📝 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
router.get("/delete",authenticateCustomer, deleteEvent);
router.delete("/:id", authenticateCustomer, deleteEvent);


module.exports = router;
16 changes: 15 additions & 1 deletion frontend/src/components/Pages/Event.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import game from '../../assets/Boardgames/carrom.gif';
import spin from '../../assets/Boardgames/spin.gif';
import MainHOC from '../MainHOC';
const daysOfWeek = ['Sun', 'Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat'];
import { useNavigate } from 'react-router-dom';
import Cookies from 'js-cookie';
const months = [
'January',
'February',
Expand All @@ -31,6 +33,18 @@ const months = [
function Event() {
const [events, setEvents] = useState([]);
const [error, setError] = useState(null);
const navigate = useNavigate();



const handleRegisterClick = () => {
const isAuthenticated = Boolean(Cookies.get('authToken'));

if (!isAuthenticated) {
alert("Please sign in to register for the event.");
navigate('/login');
}
};
Comment on lines +40 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

Authentication check needs enhancement

The current implementation has several areas for improvement:

  1. It only checks for token presence but not user verification status (required by PR objectives)
  2. The alert is not user-friendly and may be blocked by browsers
  3. After login, users lose context of which event they were trying to register for

Consider implementing these improvements:

 const handleRegisterClick = () => {
-  const isAuthenticated = Boolean(Cookies.get('authToken'));
+  try {
+    const authToken = Cookies.get('authToken');
+    const isVerified = Cookies.get('isVerified'); // Add verification check
 
-    if (!isAuthenticated) {
-      alert("Please sign in to register for the event.");
-      navigate('/login'); 
+    if (!authToken || !isVerified) {
+      // Store current path for redirect after login
+      sessionStorage.setItem('redirectAfterLogin', window.location.pathname);
+      // Use a more user-friendly notification system
+      setError(isVerified ? 
+        "Please sign in to register for the event" : 
+        "Please verify your email before registering");
+      navigate('/login');
+      return;
     }
+  } catch (error) {
+    console.error('Error checking authentication:', error);
+    setError('Something went wrong. Please try again.');
+  }
 };

Also, consider adding a state management solution (like Redux or Context) to handle authentication state more robustly.

Committable suggestion was skipped due to low confidence.

useEffect(() => {
const fetchData = async () => {
try {
Expand Down Expand Up @@ -183,7 +197,7 @@ function Event() {
))}
</div>
<div className="text-center mt-4">
<button className="bg-[#FEF3C7] dark:bg-black text-gray-700 dark:text-white px-4 py-2 rounded-md mt-4">
<button onClick={handleRegisterClick} className="bg-[#FEF3C7] dark:bg-black text-gray-700 dark:text-white px-4 py-2 rounded-md mt-4">
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

Inconsistent registration button behavior

There are two different registration buttons in the component:

  1. "Register for Event" button with authentication check
  2. "Book your slot" button without authentication check

This creates inconsistent user experience and potential security gaps.

Consolidate the registration functionality:

- <button onClick={handleRegisterClick} className="bg-[#FEF3C7] dark:bg-black text-gray-700 dark:text-white px-4 py-2 rounded-md mt-4">
-   Register for Event
- </button>

- <button className="btn btn-primary bg-[#C3E0DC] p-4 rounded-xl text-md font-medium mt-4 hover:bg-[#FFF9B1]">
-   Book your slot
- </button>

+ <button 
+   onClick={() => handleRegisterClick(event._id)} 
+   className="btn btn-primary bg-[#C3E0DC] p-4 rounded-xl text-md font-medium mt-4 hover:bg-[#FFF9B1]"
+ >
+   Register for Event
+ </button>

Also applies to: 308-308

Register for Event
</button>
</div>
Expand Down
Loading