Merge pull request #34 from hackclub/security

Security

Author: MeMMa137

client/package.json

@@ -15,6 +15,6 @@
   },
   "devDependencies": {
     "@vitejs/plugin-react": "^4.3.4",
-    "vite": "^5.4.11"
+    "vite": "^6.2.0"
   }
 }

client/src/components/AdminReviewPage.css

@@ -582,3 +582,86 @@
   background: #1a2240;
   margin: 0.4rem 0;
 }
+
+.admin-review-hours-warning {
+  margin: 0.35rem 0 0;
+  color: #ffd4bf;
+  font-size: 0.85rem;
+  font-weight: 600;
+}
+
+.admin-review-modal-backdrop {
+  position: fixed;
+  inset: 0;
+  z-index: 100;
+  display: grid;
+  place-items: center;
+  padding: 1.5rem;
+  background: rgba(8, 16, 40, 0.72);
+}
+
+.admin-review-modal {
+  width: min(28rem, 100%);
+  padding: 1.25rem 1.35rem;
+  border: 3px solid #f7cf57;
+  border-radius: 14px;
+  background: rgba(27, 58, 122, 0.98);
+  color: #e8f4ff;
+  box-shadow: 0 12px 40px rgba(0, 0, 0, 0.35);
+}
+
+.admin-review-modal h2 {
+  margin: 0 0 0.65rem;
+  color: #f7cf57;
+  font-size: 1.2rem;
+}
+
+.admin-review-modal p {
+  margin: 0 0 1rem;
+  line-height: 1.45;
+  color: rgba(232, 244, 255, 0.9);
+}
+
+.admin-review-hours-approve-cell {
+  display: grid;
+  gap: 0.35rem;
+}
+
+.admin-review-exceed-ack {
+  display: flex;
+  align-items: flex-start;
+  gap: 0.55rem;
+  margin-bottom: 1.1rem;
+  cursor: pointer;
+  font-weight: 700;
+}
+
+.admin-review-exceed-ack--inline {
+  margin: 0.25rem 0 0;
+  padding: 0.55rem 0.65rem;
+  border-radius: 10px;
+  border: 2px solid rgba(247, 207, 87, 0.55);
+  background: rgba(247, 207, 87, 0.12);
+  color: #ffe9a8;
+}
+
+.admin-review-exceed-ack input {
+  width: 1.1rem;
+  height: 1.1rem;
+  margin-top: 0.15rem;
+  flex-shrink: 0;
+}
+
+.admin-review-modal-actions {
+  display: flex;
+  flex-wrap: wrap;
+  justify-content: flex-end;
+  gap: 0.65rem;
+}
+
+.admin-review-modal-actions .admin-review-submit:disabled {
+  opacity: 0.45;
+  cursor: not-allowed;
+  transform: none;
+  box-shadow: none;
+}

client/src/components/AdminReviewPage.jsx

@@ -49,15 +49,35 @@ function slackDisplay(user) {
   return user?.slug ? `@${user.slug}` : user?.email || "Unknown";
 }
 
-function clampApprovalHours(value, maxHours) {
+function clampApprovalHours(value, maxHours, allowAboveMax = false) {
   if (value === "") return "";
   const numeric = Number.parseFloat(value);
   if (!Number.isFinite(numeric)) return value;
   if (numeric < 0) return "0";
-  if (Number.isFinite(maxHours) && numeric > maxHours) return maxHours.toFixed(2);
+  if (!allowAboveMax && Number.isFinite(maxHours) && numeric > maxHours) return maxHours.toFixed(2);
   return value;
 }
 
+function combinedLoggedHours(project) {
+  const journal = Number(project.journalHours ?? project.totalHours ?? 0);
+  const hackatime = Number(project.hackatimeHours ?? 0);
+  return Number(project.combinedHours ?? journal + hackatime);
+}
+
+/** True when "new hours to approve" is above the logged/pending caps shown on the review page. */
+function approvalNeedsExceedAck(project, requestedNewHours) {
+  if (!project || !Number.isFinite(requestedNewHours)) return false;
+  const pendingCap = Number(project.pendingReviewHours ?? 0);
+  const logged = combinedLoggedHours(project);
+  const banked = Math.max(Number(project.pastApprovedHours ?? 0), Number(project.approvedHours ?? 0));
+  const newTotal = banked + requestedNewHours;
+  return (
+    requestedNewHours > pendingCap + 1e-9 ||
+    requestedNewHours > logged + 1e-9 ||
+    newTotal > logged + 1e-9
+  );
+}
+
 export function AdminReviewPage({ projectId }) {
   if (projectId) return <AdminReviewDetail projectId={projectId} />;
   return <AdminReviewIndex />;
@@ -215,6 +235,8 @@ function AdminReviewDetail({ projectId }) {
   const [message, setMessage] = useState("");
   const [fraudSaving, setFraudSaving] = useState(false);
   const [deleteBusy, setDeleteBusy] = useState(false);
+  const [exceedModalOpen, setExceedModalOpen] = useState(false);
+  const [exceedAcknowledged, setExceedAcknowledged] = useState(false);
 
   useEffect(() => {
     loadProject();
@@ -264,7 +286,7 @@ function AdminReviewDetail({ projectId }) {
     }
   }
 
-  async function submitReview() {
+  async function submitReview({ acknowledgeExceedLoggedHours = false } = {}) {
     if (!project) return;
     setMessage("");
 
@@ -273,10 +295,14 @@ function AdminReviewDetail({ projectId }) {
       return;
     }
 
-    const newHoursMax = Number(project.pendingReviewHours ?? 0);
     const requestedApprovedHours = Number.parseFloat(approvedHours) || 0;
-    if (selectedAction === "approve" && requestedApprovedHours > newHoursMax) {
-      setMessage(`Approval cannot exceed ${formatHours(newHoursMax)} new hours.`);
+    const needsExceedAck =
+      selectedAction === "approve" && approvalNeedsExceedAck(project, requestedApprovedHours);
+    const hasExceedAck = acknowledgeExceedLoggedHours || exceedAcknowledged;
+
+    if (needsExceedAck && !hasExceedAck) {
+      setExceedModalOpen(true);
+      setMessage('Check the acknowledgment box (below the hours field or in this dialog), then submit again.');
       return;
     }
 
@@ -287,7 +313,11 @@ function AdminReviewDetail({ projectId }) {
     const body =
       selectedAction === "reject"
         ? { feedback }
-        : { approvedHours: Number.parseFloat(approvedHours) || 0, feedback };
+        : {
+            approvedHours: requestedApprovedHours,
+            feedback,
+            ...(hasExceedAck && needsExceedAck ? { acknowledgeExceedLoggedHours: true } : {}),
+          };
 
     try {
       const response = await fetch(endpoint, {
@@ -298,6 +328,8 @@ function AdminReviewDetail({ projectId }) {
       });
       const data = await response.json();
       if (!response.ok) throw new Error(data.error || "Failed to submit review.");
+      setExceedModalOpen(false);
+      setExceedAcknowledged(false);
       setProject((current) => ({
         ...current,
         ...data.project,
@@ -315,6 +347,26 @@ function AdminReviewDetail({ projectId }) {
     }
   }
 
+  function confirmExceedAndSubmit() {
+    if (!exceedAcknowledged) {
+      setMessage("Check the acknowledgment box to approve above logged hours.");
+      return;
+    }
+    setExceedModalOpen(false);
+    submitReview({ acknowledgeExceedLoggedHours: true });
+  }
+
+  function handleApprovedHoursChange(rawValue) {
+    if (!project) return;
+    const pendingCap = Number(project.pendingReviewHours ?? 0);
+    const next = clampApprovalHours(rawValue, pendingCap, true);
+    setApprovedHours(next);
+    const numeric = Number.parseFloat(next);
+    if (Number.isFinite(numeric) && !approvalNeedsExceedAck(project, numeric)) {
+      setExceedAcknowledged(false);
+    }
+  }
+
   async function deleteProjectPermanently() {
     if (!project) return;
     const ok = window.confirm(
@@ -355,6 +407,12 @@ function AdminReviewDetail({ projectId }) {
   const newHoursPlaceholder = formatHours(newHoursMax);
   const approvedHoursNumber = Number.parseFloat(approvedHours);
   const awardPreview = Number.isFinite(approvedHoursNumber) ? approvedHoursNumber * BRICKS_PER_APPROVED_HOUR : 0;
+  const needsExceedAckPreview =
+    !isAlreadyReviewed &&
+    selectedAction === "approve" &&
+    Number.isFinite(approvedHoursNumber) &&
+    approvalNeedsExceedAck(project, approvedHoursNumber);
+  const loggedHoursPreview = combinedLoggedHours(project);
 
   return (
     <main className="admin-review-page">
@@ -440,20 +498,36 @@ function AdminReviewDetail({ projectId }) {
               <span>New hours</span>
               <strong>{newHoursPlaceholder} h</strong>
             </div>
-            <div>
+            <div className="admin-review-hours-approve-cell">
               <span>New hours to approve</span>
               <input
                 className="admin-review-hours-input"
                 type="number"
                 min="0"
-                max={newHoursPlaceholder}
                 step="0.25"
                 placeholder={newHoursPlaceholder}
                 value={approvedHours}
                 disabled={isAlreadyReviewed}
-                onChange={(event) => setApprovedHours(clampApprovalHours(event.target.value, newHoursMax))}
+                onChange={(event) => handleApprovedHoursChange(event.target.value)}
               />
               <small>Only this approved amount awards bricks: {formatHours(awardPreview)} bricks</small>
+              {needsExceedAckPreview ? (
+                <>
+                  <p className="admin-review-hours-warning">
+                    Above logged hours ({formatHours(loggedHoursPreview)} h combined
+                    {newHoursMax < loggedHoursPreview ? `, pending cap ${formatHours(newHoursMax)} h` : ""}).
+                  </p>
+                  <label className="admin-review-exceed-ack admin-review-exceed-ack--inline">
+                    <input
+                      type="checkbox"
+                      checked={exceedAcknowledged}
+                      disabled={isAlreadyReviewed}
+                      onChange={(event) => setExceedAcknowledged(event.target.checked)}
+                    />
+                    <span>I&apos;m aware that I&apos;m giving more hours than the logged ones</span>
+                  </label>
+                </>
+              ) : null}
             </div>
             <div>
               <span>Journal hours</span>
@@ -510,7 +584,7 @@ function AdminReviewDetail({ projectId }) {
                 Comment (optional for approve; required for reject)
                 <textarea value={feedback} onChange={(event) => setFeedback(event.target.value)} />
               </label>
-              <button className="admin-review-submit" type="button" onClick={submitReview}>
+              <button className="admin-review-submit" type="button" onClick={() => submitReview()}>
                 Submit Review
               </button>
             </>
@@ -533,6 +607,50 @@ function AdminReviewDetail({ projectId }) {
           ) : null}
         </section>
       </section>
+
+      {exceedModalOpen ? (
+        <div
+          className="admin-review-modal-backdrop"
+          role="presentation"
+          onClick={() => setExceedModalOpen(false)}
+        >
+          <div
+            className="admin-review-modal"
+            role="dialog"
+            aria-labelledby="admin-review-exceed-title"
+            aria-modal="true"
+            onClick={(event) => event.stopPropagation()}
+          >
+            <h2 id="admin-review-exceed-title">Approve above logged hours?</h2>
+            <p>
+              You are approving <strong>{formatHours(approvedHoursNumber)} h</strong> new, but only{" "}
+              <strong>{formatHours(project.combinedHours ?? 0)} h</strong> are logged (pending cap:{" "}
+              {formatHours(newHoursMax)} h).
+            </p>
+            <label className="admin-review-exceed-ack">
+              <input
+                type="checkbox"
+                checked={exceedAcknowledged}
+                onChange={(event) => setExceedAcknowledged(event.target.checked)}
+              />
+              <span>I&apos;m aware that I&apos;m giving more hours than the logged ones</span>
+            </label>
+            <div className="admin-review-modal-actions">
+              <button type="button" className="admin-review-tool-btn" onClick={() => setExceedModalOpen(false)}>
+                Cancel
+              </button>
+              <button
+                type="button"
+                className="admin-review-submit"
+                disabled={!exceedAcknowledged}
+                onClick={confirmExceedAndSubmit}
+              >
+                Approve anyway
+              </button>
+            </div>
+          </div>
+        </div>
+      ) : null}
     </main>
   );
 }