Merge pull request #387 from cs3216-a3-group-4/haoyang/merge-password…

…-reset-into-prod

Merge password reset fix into production

backend/src/auth/dependencies.py

@@ -1,3 +1,4 @@
+from http import HTTPStatus
 from typing import Annotated, Optional
 from fastapi.security import OAuth2PasswordBearer
 from passlib.context import CryptContext
@@ -56,9 +57,15 @@ def authenticate_user(email: str, password: str):
             )
         ).first()
         if not user:
-            return False
+            raise HTTPException(
+                status_code=HTTPStatus.UNAUTHORIZED,
+                detail={"error": "Invalid username.", "error_id": "1"},
+            )
         if not verify_password(password, user.hashed_password):
-            return False
+            raise HTTPException(
+                status_code=HTTPStatus.UNAUTHORIZED,
+                detail={"error": "Incorrect password.", "error_id": "2"},
+            )
         return user
 
 

backend/src/auth/router.py

@@ -7,7 +7,7 @@
 from fastapi import BackgroundTasks, Depends, APIRouter, HTTPException, Response
 from fastapi.security import OAuth2PasswordRequestForm
 import httpx
-from sqlalchemy import select
+from sqlalchemy import select, update
 from sqlalchemy.orm import selectinload
 from src.auth.utils import create_token, send_reset_password_email
 from src.common.constants import (
@@ -77,12 +77,10 @@ def sign_up(
 def log_in(
     form_data: Annotated[OAuth2PasswordRequestForm, Depends()], response: Response
 ) -> Token:
-    user = authenticate_user(form_data.username, form_data.password)
-    if not user:
-        raise HTTPException(
-            status_code=HTTPStatus.UNAUTHORIZED,
-            detail="Incorrect username or password.",
-        )
+    try:
+        user = authenticate_user(form_data.username, form_data.password)
+    except HTTPException as exc:
+        raise exc
 
     return create_token(user, response)
 
@@ -151,27 +149,10 @@ def auth_google(
 )
 
 
-@routerWithAuth.get("/session")
-def get_user(
-    current_user: Annotated[User, Depends(get_current_user)],
-    session=Depends(get_session),
-) -> UserPublic:
-    user = session.get(User, current_user.id)
-    if user:
-        user.last_accessed = datetime.now()
-        session.add(user)
-        session.commit()
-
-    return current_user
-
-
-@routerWithAuth.get("/logout")
-def logout(response: Response):
-    response.delete_cookie(key="session")
-    return ""
-
-
-@routerWithAuth.post("/password-reset")
+#######################
+# Reset password #
+#######################
+@router.post("/password-reset")
 def request_password_reset(
     data: PasswordResetRequestData,
     background_task: BackgroundTasks,
@@ -184,7 +165,16 @@ def request_password_reset(
         .where(User.account_type == AccountType.NORMAL)
     ).first()
     if not user:
-        return
+        print(
+            f"""ERROR: Attempt to reset password for email {email} that doesn't match any existing user"""
+        )
+        raise HTTPException(HTTPStatus.NOT_FOUND)
+
+    # Mark all existing password reset codes for the given user as used to prevent usage of old codes
+    session.execute(
+        update(PasswordReset).where(PasswordReset.user_id == user.id).values(used=True)
+    )
+    session.commit()
 
     code = str(uuid4())
     password_reset = PasswordReset(user_id=user.id, code=code, used=False)
@@ -193,7 +183,7 @@ def request_password_reset(
     background_task.add_task(send_reset_password_email, email, code)
 
 
-@routerWithAuth.put("/password-reset")
+@router.put("/password-reset")
 def complete_password_reset(
     code: str,
     data: PasswordResetCompleteData,
@@ -203,9 +193,16 @@ def complete_password_reset(
     password_reset = session.scalars(
         select(PasswordReset).where(PasswordReset.code == code)
     ).first()
-    if not password_reset or password_reset.used:
+    if not password_reset:
+        print(
+            "ERROR: Attempt to use password reset code that wasn't previously generated by Jippy"
+        )
         raise HTTPException(HTTPStatus.NOT_FOUND)
 
+    if password_reset.used:
+        print("ERROR: Attempt to use password reset code that has already been used")
+        raise (HTTPException(HTTPStatus.CONFLICT))
+
     user = session.get(User, password_reset.user_id)
     user.hashed_password = get_password_hash(data.password)
     password_reset.used = True
@@ -214,6 +211,26 @@ def complete_password_reset(
     session.commit()
 
 
+@routerWithAuth.get("/session")
+def get_user(
+    current_user: Annotated[User, Depends(get_current_user)],
+    session=Depends(get_session),
+) -> UserPublic:
+    user = session.get(User, current_user.id)
+    if user:
+        user.last_accessed = datetime.now()
+        session.add(user)
+        session.commit()
+
+    return current_user
+
+
+@routerWithAuth.get("/logout")
+def logout(response: Response):
+    response.delete_cookie(key="session")
+    return ""
+
+
 @routerWithAuth.put("/change-password")
 def change_password(
     user: Annotated[User, Depends(get_current_user)],

frontend/app/(unauthenticated)/login/page.tsx

@@ -95,7 +95,7 @@ function LoginPage() {
                 <CircleAlert className="h-5 w-5" />
                 <AlertDescription>
                   Your email or password is incorrect. Please try again, or{" "}
-                  <Link href="/password-reset" size="sm">
+                  <Link href="/reset-password" size="sm">
                     reset your password
                   </Link>
                   .

frontend/app/(unauthenticated)/register/page.tsx

@@ -86,8 +86,13 @@ function RegisterPage() {
         {/* Body */}
         <Box className="space-y-6 pt-0 flex-col w-full">
           {isError && (
-            <Alert variant="destructive">
-              <CircleAlert className="h-5 w-5" />
+            <Alert
+              className="flex flex-row items-center gap-x-2"
+              variant="destructive"
+            >
+              <div className="flex flex-row">
+                <CircleAlert className="h-5 w-5" />
+              </div>
               <AlertDescription>
                 This email is already registered.{" "}
                 <Link href="/login" size="sm">

frontend/app/(unauthenticated)/reset-password/_components/reset-password-complete-form.tsx

@@ -3,6 +3,7 @@
 import { useState } from "react";
 import { SubmitHandler, useForm } from "react-hook-form";
 import { zodResolver } from "@hookform/resolvers/zod";
+import { HttpStatusCode } from "axios";
 import { CircleAlert } from "lucide-react";
 import { z } from "zod";
 
@@ -54,6 +55,9 @@ export default function ResetPasswordCompleteForm({
   onComplete: () => void;
 }) {
   const [isError, setIsError] = useState<boolean>(false);
+  const [errorMsg, setErrorMsg] = useState<string>(
+    "Your password needs to match.",
+  );
 
   const form = useForm<ResetPasswordRequestForm>({
     resolver: zodResolver(resetPasswordCompleteFormSchema),
@@ -74,6 +78,17 @@ export default function ResetPasswordCompleteForm({
 
     if (response.error) {
       setIsError(true);
+      if (response.status === HttpStatusCode.Conflict) {
+        setErrorMsg(
+          "Password reset link has expired. Please check your email for the latest link",
+        );
+      } else if (response.status === HttpStatusCode.NotFound) {
+        setErrorMsg(
+          "Invalid password reset link. Please only click on links sent by us",
+        );
+      } else {
+        setErrorMsg("Error while resetting your password. Please try again");
+      }
     } else {
       setIsError(false);
       onComplete();
@@ -91,11 +106,14 @@ export default function ResetPasswordCompleteForm({
         <CardContent>
           <Box className="space-y-6">
             {isError && (
-              <Alert variant="destructive">
-                <CircleAlert className="h-5 w-5" />
-                <AlertDescription>
-                  Your password needs to match.
-                </AlertDescription>
+              <Alert
+                className="flex flex-row items-center gap-x-2"
+                variant="destructive"
+              >
+                <div className="flex flex-shrink-0">
+                  <CircleAlert className="h-5 w-5" />
+                </div>
+                <AlertDescription className="grow">{errorMsg}</AlertDescription>
               </Alert>
             )}
             <Form {...form}>

frontend/components/form/inputs/password-input.tsx

@@ -47,7 +47,7 @@ const PasswordInput = forwardRef<HTMLInputElement, PasswordInputProps>(
           </div>
 
           {showForgetPassword && (
-            <Link href="/user/password-reset" size="sm">
+            <Link href="/reset-password" size="sm">
               Forgot password?
             </Link>
           )}