Skip to content
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
20 changes: 16 additions & 4 deletions firestore.rules
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,17 @@ service cloud.firestore {
allow update, delete: if isOwner(userId);
}

// Study spots: any authenticated user can read; writes restricted
// Study spots: any authenticated user can read; creator manages metadata,
// any signed-in user may update only the rating aggregates (ratingSum/ratingCount)
match /spots/{spotId} {
allow read: if isSignedIn();
allow create: if isSignedIn();
allow update, delete: if isSignedIn()
allow update: if isSignedIn() && (
(resource.data.createdBy is string && resource.data.createdBy == request.auth.uid)
|| request.resource.data.diff(resource.data).affectedKeys().hasOnly(['ratingSum', 'ratingCount'])
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rules allow arbitrary rating aggregate values from any user

Medium Severity

The spots update rule uses affectedKeys().hasOnly(['ratingSum', 'ratingCount']) to gate non-owner writes. This validates which fields change but not what values they're set to. Any signed-in user can set ratingSum and ratingCount to arbitrary numbers (e.g., setting ratingSum to 500 and ratingCount to 1), allowing them to manipulate any spot's displayed average rating.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8904d30. Configure here.

allow delete: if isSignedIn()
&& resource.data.createdBy is string
&& resource.data.createdBy == request.auth.uid;
}

Expand All @@ -33,11 +39,17 @@ service cloud.firestore {
&& resource.data.userId == request.auth.uid;
}

// Study groups: authenticated users can read; owner manages
// Study groups: authenticated users can read; owner manages metadata,
// any signed-in user may update only the membership lists (memberIds/members) to join/leave
match /groups/{groupId} {
allow read: if isSignedIn();
allow create: if isSignedIn();
allow update, delete: if isSignedIn()
allow update: if isSignedIn() && (
(resource.data.ownerId is string && resource.data.ownerId == request.auth.uid)
|| request.resource.data.diff(resource.data).affectedKeys().hasOnly(['memberIds', 'members'])
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rules allow any user to alter any group's membership

Medium Severity

The groups update rule allows any signed-in user to write arbitrary values to memberIds and members on any group. There's no validation that the user is only adding or removing themselves. A malicious user could remove all members from any group or inject arbitrary user IDs into the member list.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8904d30. Configure here.

allow delete: if isSignedIn()
&& resource.data.ownerId is string
&& resource.data.ownerId == request.auth.uid;
}

Expand Down
169 changes: 169 additions & 0 deletions scripts/seed-firestore.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
/* eslint-disable */
// Idempotent Firestore seed script.
// Run from repo root: node scripts/seed-firestore.js
// Requires Backend/serviceAccountKey.json (already gitignored).

const path = require('path');
const fs = require('fs');
const Module = require('module');

// firebase-admin is installed in Backend/node_modules, not root
Module.globalPaths.push(path.join(__dirname, '..', 'Backend', 'node_modules'));
const admin = require(path.join(__dirname, '..', 'Backend', 'node_modules', 'firebase-admin'));

const serviceAccountPath = path.join(__dirname, '..', 'Backend', 'serviceAccountKey.json');
if (!fs.existsSync(serviceAccountPath)) {
console.error(
`Missing ${serviceAccountPath}.\n` +
'Download a service-account key from Firebase Console > Project settings > Service accounts.\n' +
'See Backend/SETUP_FIREBASE.md.',
);
process.exit(1);
}

const serviceAccount = require(serviceAccountPath);
admin.initializeApp({ credential: admin.credential.cert(serviceAccount) });
const db = admin.firestore();

// Mock data sources (kept as the seed of truth)
const studySpots = [
{
id: '1',
name: 'Doe Library',
location: 'On campus',
description: 'Convenient, beautiful library',
distance: 0.3,
hours: '9AM - 9PM',
noiseLevel: 'Silent',
outlets: true,
lighting: 'Bright',
crowded: 'Low',
roomType: 'Library',
open: true,
rating: 4.7,
image: '/cat.webp',
},
{
id: '2',
name: 'MLK Student Union',
location: 'On campus',
description: 'Collaborative study environment',
distance: 0.5,
hours: '10AM - 11PM',
noiseLevel: 'Medium',
outlets: true,
lighting: 'Medium',
crowded: 'High',
roomType: 'Student Center',
open: true,
rating: 4.1,
image: '/anothercat.jpg',
},
{
id: '3',
name: 'Cafe Strada',
location: 'Off campus',
description: 'Great outdoor seating',
distance: 0.4,
hours: '8AM - 6PM',
noiseLevel: 'Loud',
outlets: false,
lighting: 'Dim',
crowded: 'Medium',
roomType: 'Cafe',
open: true,
rating: 3.2,
image: '/yetanothercat.jpg',
},
];

const studyGroups = [
{
id: '1',
course: 'CS 61A',
name: 'cs warriors',
pace: 'Fast',
noiseLevel: 'Medium',
groupSize: 4,
availability: 'Evenings',
vibe: 'Focused',
method: 'Practice problems',
description: 'Looking for 2 more members!',
creator: 'Alex',
meetingTime: 'Wed, April 10, 6:00PM',
meetingPlace: 'Doe Library Room 123',
members: ['Alex', 'Taylor', 'Jordan'],
image: '/cat.webp',
},
{
id: '2',
course: 'MATH 54',
name: 'we love arun sharma',
pace: 'Medium',
noiseLevel: 'Quiet',
groupSize: 3,
availability: 'Afternoons',
vibe: 'Chill',
method: 'Concept discussion',
description: 'Hoping to study collaboratively and meet new people!',
creator: 'Jamie',
meetingTime: 'Thu, April 11, 3:00PM',
meetingPlace: 'Evans Hall Room 210',
members: ['Jamie', 'Sam'],
image: '/anothercat.jpg',
},
];

async function seedSpots() {
let created = 0;
let skipped = 0;
for (const { id, ...data } of studySpots) {
const ref = db.collection('spots').doc(id);
const snap = await ref.get();
if (snap.exists) {
skipped++;
continue;
}
await ref.set({
...data,
ratingSum: 0,
ratingCount: 0,
createdAt: admin.firestore.FieldValue.serverTimestamp(),
});
created++;
}
console.log(`spots: ${created} created, ${skipped} already existed`);
}

async function seedGroups() {
let created = 0;
let skipped = 0;
for (const { id, ...data } of studyGroups) {
const ref = db.collection('groups').doc(id);
const snap = await ref.get();
if (snap.exists) {
skipped++;
continue;
}
await ref.set({
...data,
ownerId: '',
memberIds: [],
createdAt: admin.firestore.FieldValue.serverTimestamp(),
});
created++;
}
console.log(`groups: ${created} created, ${skipped} already existed`);
}

(async () => {
try {
await seedSpots();
await seedGroups();
console.log('Seed complete.');
process.exit(0);
} catch (e) {
console.error('Seed failed:', e);
process.exit(1);
}
})();
39 changes: 25 additions & 14 deletions src/App.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { Routes, Route } from "react-router-dom";
import { Navigate, Routes, Route } from "react-router-dom";

import AppLayout from "./layouts/AppLayout";
import AuthLayout from "./layouts/AuthLayout";
import RequireAuth from "./components/RequireAuth";
import { useAuth } from "./context/AuthContext";

import Login from "./pages/auth/Login";
import Signup from "./pages/auth/Signup";
Expand All @@ -13,29 +15,38 @@ import StudyGroupInfo from "./pages/study-groups/StudyGroupInfo";
import StudyGroupCreate from "./pages/study-groups/StudyGroupCreate";
import Insights from "./pages/Insights";

function PublicOnly({ children }) {
const { user, loading } = useAuth();
if (loading) return null;
if (user) return <Navigate to="/study-spots" replace />;
return children;
}

function App() {
return (
<Routes>

{/* Auth pages (NO nav) */}
{/* Auth pages (NO nav) — redirect signed-in users to app */}
<Route element={<AuthLayout />}>
<Route path="/" element={<Login />} />
<Route path="/signup" element={<Signup />} />
<Route path="/" element={<PublicOnly><Login /></PublicOnly>} />
<Route path="/signup" element={<PublicOnly><Signup /></PublicOnly>} />
</Route>

{/* App pages (WITH nav) */}
<Route element={<AppLayout />}>
<Route path="/study-spots/" element={<StudySpotDiscovery />} />
<Route path="/study-spots/:id" element={<StudySpotInfo />} />
<Route path="/study-spots/log/:id" element={<StudySessionLog />} />
<Route path="/study-groups" element={<StudyGroupDiscovery />} />
<Route path="/study-groups/:id" element={<StudyGroupInfo />} />
<Route path="/study-groups/create" element={<StudyGroupCreate />} />
<Route path="/insights" element={<Insights />} />
{/* App pages (WITH nav) — protected */}
<Route element={<RequireAuth />}>
<Route element={<AppLayout />}>
<Route path="/study-spots/" element={<StudySpotDiscovery />} />
<Route path="/study-spots/:id" element={<StudySpotInfo />} />
<Route path="/study-spots/log/:id" element={<StudySessionLog />} />
<Route path="/study-groups" element={<StudyGroupDiscovery />} />
<Route path="/study-groups/:id" element={<StudyGroupInfo />} />
<Route path="/study-groups/create" element={<StudyGroupCreate />} />
<Route path="/insights" element={<Insights />} />
</Route>
</Route>

</Routes>
);
}

export default App;
export default App;
21 changes: 21 additions & 0 deletions src/components/RequireAuth.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Navigate, Outlet, useLocation } from "react-router-dom";
import { useAuth } from "../context/AuthContext";

export default function RequireAuth() {
const { user, loading } = useAuth();
const location = useLocation();

if (loading) {
return (
<div className="min-h-screen flex items-center justify-center bg-white">
<p className="text-black" style={{ fontFamily: "'Jost', sans-serif" }}>Loading...</p>
</div>
);
}

if (!user) {
return <Navigate to="/" state={{ from: location.pathname }} replace />;
}

return <Outlet />;
}
13 changes: 11 additions & 2 deletions src/components/cards/StudySpotCard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,17 @@ const crowdBadgeColor = (level) =>
level === "Low" || level === "Small" ? "green" : level === "Medium" ? "orange" : "red";
const ratingBadgeColor = (rating) => (rating >= 4 ? "green" : rating >= 3.0 ? "orange" : "red");

function displayRating(spot) {
if (spot.ratingCount && spot.ratingCount > 0) {
return (spot.ratingSum / spot.ratingCount).toFixed(1);
}
if (spot.rating !== undefined) return Number(spot.rating).toFixed(1);
return null;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Identical displayRating duplicated across two card components

Low Severity

The displayRating function is identically defined in both StudySpotCard.jsx and StudySpotCardL.jsx. This duplicated logic means any future change to rating display (e.g., handling edge cases or format changes) needs to be applied in two places, risking divergence.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8904d30. Configure here.


export default function StudySpotCard({ spot }) {
const navigate = useNavigate();
const rating = displayRating(spot);

return (
<div className="flex justify-center w-full">
Expand Down Expand Up @@ -73,10 +82,10 @@ export default function StudySpotCard({ spot }) {
<Badge label={spot.open ? "Open" : "Closed"} color={spot.open ? "green" : "red"} />
</div>
)}
{spot.rating !== undefined && (
{rating !== null && (
<div className="flex flex-col items-center">
<span className="text-gray-500 text-[0.65rem]" style={{ fontFamily: "'Jost', sans-serif" }}>Rating</span>
<Badge label={`${spot.rating}`} color={ratingBadgeColor(spot.rating)} />
<Badge label={rating} color={ratingBadgeColor(Number(rating))} />
</div>
)}
</div>
Expand Down
13 changes: 11 additions & 2 deletions src/components/cards/StudySpotCardL.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,18 @@ const crowdBadgeColor = (level) => (level === "Low" ? "green" : level === "Mediu
const openBadgeColor = (isOpen) => (isOpen ? "green" : "red");
const ratingBadgeColor = (rating) => (rating >= 4.5 ? "green" : rating >= 4.0 ? "orange" : "red");

function displayRating(data) {
if (data.ratingCount && data.ratingCount > 0) {
return (data.ratingSum / data.ratingCount).toFixed(1);
}
if (data.rating !== undefined) return Number(data.rating).toFixed(1);
return null;
}

export default function StudySpotCardL({ data, buttonText = "Join", onJoin, onConfirmJoin }) {
const navigate = useNavigate();
const [modalOpen, setModalOpen] = useState(false);
const rating = displayRating(data);

return (
<>
Expand Down Expand Up @@ -75,10 +84,10 @@ export default function StudySpotCardL({ data, buttonText = "Join", onJoin, onCo
<Badge label={data.open ? "Open" : "Closed"} color={openBadgeColor(data.open)} />
</div>
)}
{data.rating !== undefined && (
{rating !== null && (
<div className="flex flex-col items-center">
<span className="text-gray-500 text-[0.65rem]" style={{ fontFamily: "'Jost', sans-serif" }}>Rating</span>
<Badge label={`${data.rating}`} color={ratingBadgeColor(data.rating)} />
<Badge label={rating} color={ratingBadgeColor(Number(rating))} />
</div>
)}
</div>
Expand Down
Loading
Loading