diff --git a/backend/.idea/.gitignore b/backend/.idea/.gitignore new file mode 100644 index 0000000000..26d33521af --- /dev/null +++ b/backend/.idea/.gitignore @@ -0,0 +1,3 @@ +# Default ignored files +/shelf/ +/workspace.xml diff --git a/backend/.idea/backend.iml b/backend/.idea/backend.iml new file mode 100644 index 0000000000..5c1f14c14c --- /dev/null +++ b/backend/.idea/backend.iml @@ -0,0 +1,14 @@ + + + + + + + + + + + + diff --git a/backend/.idea/inspectionProfiles/profiles_settings.xml b/backend/.idea/inspectionProfiles/profiles_settings.xml new file mode 100644 index 0000000000..cc5462daf8 --- /dev/null +++ b/backend/.idea/inspectionProfiles/profiles_settings.xml @@ -0,0 +1,6 @@ + + + + diff --git a/backend/.idea/misc.xml b/backend/.idea/misc.xml new file mode 100644 index 0000000000..c4bf9ff881 --- /dev/null +++ b/backend/.idea/misc.xml @@ -0,0 +1,4 @@ + + + + diff --git a/backend/.idea/modules.xml b/backend/.idea/modules.xml new file mode 100644 index 0000000000..f11a6b0441 --- /dev/null +++ b/backend/.idea/modules.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/backend/.idea/vcs.xml b/backend/.idea/vcs.xml new file mode 100644 index 0000000000..54e4b961ee --- /dev/null +++ b/backend/.idea/vcs.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index c2b83c841d..d8d166aad0 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -3,7 +3,7 @@ import jwt from fastapi import Depends, HTTPException, status -from fastapi.security import OAuth2PasswordBearer +from fastapi.security import APIKeyCookie, OAuth2PasswordBearer from jwt.exceptions import InvalidTokenError from pydantic import ValidationError from sqlmodel import Session @@ -16,6 +16,7 @@ reusable_oauth2 = OAuth2PasswordBearer( tokenUrl=f"{settings.API_V1_STR}/login/access-token" ) +cookie_scheme = APIKeyCookie(name="http_only_auth_cookie") def get_db() -> Generator[Session, None, None]: @@ -27,10 +28,19 @@ def get_db() -> Generator[Session, None, None]: TokenDep = Annotated[str, Depends(reusable_oauth2)] -def get_current_user(session: SessionDep, token: TokenDep) -> User: +def get_current_user( + session: SessionDep, + http_only_auth_cookie: str = Depends(cookie_scheme), +) -> User: + if not http_only_auth_cookie: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Not authenticated", + ) + try: payload = jwt.decode( - token, settings.SECRET_KEY, algorithms=[security.ALGORITHM] + http_only_auth_cookie, settings.SECRET_KEY, algorithms=[security.ALGORITHM] ) token_data = TokenPayload(**payload) except (InvalidTokenError, ValidationError): @@ -38,11 +48,13 @@ def get_current_user(session: SessionDep, token: TokenDep) -> User: status_code=status.HTTP_403_FORBIDDEN, detail="Could not validate credentials", ) + user = session.get(User, token_data.sub) if not user: raise HTTPException(status_code=404, detail="User not found") if not user.is_active: raise HTTPException(status_code=400, detail="Inactive user") + return user diff --git a/backend/app/api/routes/login.py b/backend/app/api/routes/login.py index 980c66f86f..18906f0ea5 100644 --- a/backend/app/api/routes/login.py +++ b/backend/app/api/routes/login.py @@ -2,15 +2,20 @@ from typing import Annotated, Any from fastapi import APIRouter, Depends, HTTPException -from fastapi.responses import HTMLResponse +from fastapi.responses import HTMLResponse, JSONResponse from fastapi.security import OAuth2PasswordRequestForm from app import crud -from app.api.deps import CurrentUser, SessionDep, get_current_active_superuser +from app.api.deps import ( + CurrentUser, + SessionDep, + get_current_active_superuser, + get_current_user, +) from app.core import security from app.core.config import settings from app.core.security import get_password_hash -from app.models import Message, NewPassword, Token, UserPublic +from app.models import Message, NewPassword, UserPublic from app.utils import ( generate_password_reset_token, generate_reset_password_email, @@ -24,9 +29,9 @@ @router.post("/login/access-token") def login_access_token( session: SessionDep, form_data: Annotated[OAuth2PasswordRequestForm, Depends()] -) -> Token: +) -> JSONResponse: """ - OAuth2 compatible token login, get an access token for future requests + OAuth2-compatible token login: get an access token for future requests (sent in an HTTP-only cookie) """ user = crud.authenticate( session=session, email=form_data.username, password=form_data.password @@ -36,11 +41,7 @@ def login_access_token( elif not user.is_active: raise HTTPException(status_code=400, detail="Inactive user") access_token_expires = timedelta(minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES) - return Token( - access_token=security.create_access_token( - user.id, expires_delta=access_token_expires - ) - ) + return security.set_auth_cookie(user.id, access_token_expires) @router.post("/login/test-token", response_model=UserPublic) @@ -122,3 +123,11 @@ def recover_password_html_content(email: str, session: SessionDep) -> Any: return HTMLResponse( content=email_data.html_content, headers={"subject:": email_data.subject} ) + + +@router.post("/logout", dependencies=[Depends(get_current_user)]) +def logout() -> JSONResponse: + """ + Delete the HTTP-only cookie during logout + """ + return security.delete_auth_cookie() diff --git a/backend/app/api/routes/users.py b/backend/app/api/routes/users.py index 6429818458..3a5f3100ea 100644 --- a/backend/app/api/routes/users.py +++ b/backend/app/api/routes/users.py @@ -9,6 +9,7 @@ CurrentUser, SessionDep, get_current_active_superuser, + get_current_user, ) from app.core.config import settings from app.core.security import get_password_hash, verify_password @@ -117,15 +118,16 @@ def update_password_me( return Message(message="Password updated successfully") -@router.get("/me", response_model=UserPublic) +@router.get("/me", response_model=UserPublic, dependencies=[Depends(get_current_user)]) def read_user_me(current_user: CurrentUser) -> Any: """ Get current user. """ + print(current_user) return current_user -@router.delete("/me", response_model=Message) +@router.delete("/me", response_model=Message, dependencies=[Depends(get_current_user)]) def delete_user_me(session: SessionDep, current_user: CurrentUser) -> Any: """ Delete own user. diff --git a/backend/app/core/security.py b/backend/app/core/security.py index 7aff7cfb32..d532564d0c 100644 --- a/backend/app/core/security.py +++ b/backend/app/core/security.py @@ -2,13 +2,13 @@ from typing import Any import jwt +from fastapi.responses import JSONResponse from passlib.context import CryptContext from app.core.config import settings pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") - ALGORITHM = "HS256" @@ -19,6 +19,37 @@ def create_access_token(subject: str | Any, expires_delta: timedelta) -> str: return encoded_jwt +def set_auth_cookie(subject: str | Any, expires_delta: timedelta) -> JSONResponse: + access_token = create_access_token(subject, expires_delta) + response = JSONResponse(content={"message": "Login successful"}) + # Note: The secure flag on cookies ensures they're only sent over encrypted HTTPS connections. For local development (HTTP) set it to False + response.set_cookie( + key="http_only_auth_cookie", + value=access_token, + httponly=True, + max_age=3600, + expires=3600, + samesite="lax", + secure=True, + domain=None, + ) + return response + + +def delete_auth_cookie() -> JSONResponse: + response = JSONResponse(content={"message": "Logout successful"}) + + response.delete_cookie( + key="http_only_auth_cookie", + path="/", + domain=None, + httponly=True, + samesite="lax", + secure=False, # Should be True in production + ) + return response + + def verify_password(plain_password: str, hashed_password: str) -> bool: return pwd_context.verify(plain_password, hashed_password) diff --git a/backend/app/tests/api/routes/test_items.py b/backend/app/tests/api/routes/test_items.py index c215238a69..8e1a7e31ac 100644 --- a/backend/app/tests/api/routes/test_items.py +++ b/backend/app/tests/api/routes/test_items.py @@ -8,12 +8,12 @@ def test_create_item( - client: TestClient, superuser_token_headers: dict[str, str] + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: data = {"title": "Foo", "description": "Fighters"} response = client.post( f"{settings.API_V1_STR}/items/", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) assert response.status_code == 200 @@ -25,12 +25,12 @@ def test_create_item( def test_read_item( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: item = create_random_item(db) response = client.get( f"{settings.API_V1_STR}/items/{item.id}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, ) assert response.status_code == 200 content = response.json() @@ -41,11 +41,11 @@ def test_read_item( def test_read_item_not_found( - client: TestClient, superuser_token_headers: dict[str, str] + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: response = client.get( f"{settings.API_V1_STR}/items/{uuid.uuid4()}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, ) assert response.status_code == 404 content = response.json() @@ -53,12 +53,12 @@ def test_read_item_not_found( def test_read_item_not_enough_permissions( - client: TestClient, normal_user_token_headers: dict[str, str], db: Session + client: TestClient, normal_user_auth_cookies: dict[str, str], db: Session ) -> None: item = create_random_item(db) response = client.get( f"{settings.API_V1_STR}/items/{item.id}", - headers=normal_user_token_headers, + cookies=normal_user_auth_cookies, ) assert response.status_code == 400 content = response.json() @@ -66,13 +66,13 @@ def test_read_item_not_enough_permissions( def test_read_items( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: create_random_item(db) create_random_item(db) response = client.get( f"{settings.API_V1_STR}/items/", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, ) assert response.status_code == 200 content = response.json() @@ -80,13 +80,13 @@ def test_read_items( def test_update_item( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: item = create_random_item(db) data = {"title": "Updated title", "description": "Updated description"} response = client.put( f"{settings.API_V1_STR}/items/{item.id}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) assert response.status_code == 200 @@ -98,12 +98,12 @@ def test_update_item( def test_update_item_not_found( - client: TestClient, superuser_token_headers: dict[str, str] + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: data = {"title": "Updated title", "description": "Updated description"} response = client.put( f"{settings.API_V1_STR}/items/{uuid.uuid4()}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) assert response.status_code == 404 @@ -112,13 +112,13 @@ def test_update_item_not_found( def test_update_item_not_enough_permissions( - client: TestClient, normal_user_token_headers: dict[str, str], db: Session + client: TestClient, normal_user_auth_cookies: dict[str, str], db: Session ) -> None: item = create_random_item(db) data = {"title": "Updated title", "description": "Updated description"} response = client.put( f"{settings.API_V1_STR}/items/{item.id}", - headers=normal_user_token_headers, + cookies=normal_user_auth_cookies, json=data, ) assert response.status_code == 400 @@ -127,12 +127,12 @@ def test_update_item_not_enough_permissions( def test_delete_item( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: item = create_random_item(db) response = client.delete( f"{settings.API_V1_STR}/items/{item.id}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, ) assert response.status_code == 200 content = response.json() @@ -140,11 +140,11 @@ def test_delete_item( def test_delete_item_not_found( - client: TestClient, superuser_token_headers: dict[str, str] + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: response = client.delete( f"{settings.API_V1_STR}/items/{uuid.uuid4()}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, ) assert response.status_code == 404 content = response.json() @@ -152,12 +152,12 @@ def test_delete_item_not_found( def test_delete_item_not_enough_permissions( - client: TestClient, normal_user_token_headers: dict[str, str], db: Session + client: TestClient, normal_user_auth_cookies: dict[str, str], db: Session ) -> None: item = create_random_item(db) response = client.delete( f"{settings.API_V1_STR}/items/{item.id}", - headers=normal_user_token_headers, + cookies=normal_user_auth_cookies, ) assert response.status_code == 400 content = response.json() diff --git a/backend/app/tests/api/routes/test_login.py b/backend/app/tests/api/routes/test_login.py index 80fa787979..d1723ff0b1 100644 --- a/backend/app/tests/api/routes/test_login.py +++ b/backend/app/tests/api/routes/test_login.py @@ -12,19 +12,25 @@ from app.utils import generate_password_reset_token -def test_get_access_token(client: TestClient) -> None: +def test_get_auth_cookie(client: TestClient) -> None: login_data = { "username": settings.FIRST_SUPERUSER, "password": settings.FIRST_SUPERUSER_PASSWORD, } + r = client.post(f"{settings.API_V1_STR}/login/access-token", data=login_data) - tokens = r.json() + assert r.status_code == 200 - assert "access_token" in tokens - assert tokens["access_token"] + assert r.json()["message"] == "Login successful" + + cookie_header = r.headers.get("Set-Cookie") + assert "http_only_auth_cookie=" in cookie_header + + cookie_value = cookie_header.split("http_only_auth_cookie=")[1].split(";")[0] + assert cookie_value -def test_get_access_token_incorrect_password(client: TestClient) -> None: +def test_get_auth_cookie_incorrect_password(client: TestClient) -> None: login_data = { "username": settings.FIRST_SUPERUSER, "password": "incorrect", @@ -33,12 +39,12 @@ def test_get_access_token_incorrect_password(client: TestClient) -> None: assert r.status_code == 400 -def test_use_access_token( - client: TestClient, superuser_token_headers: dict[str, str] +def test_use_auth_cookie( + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: r = client.post( f"{settings.API_V1_STR}/login/test-token", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, ) result = r.json() assert r.status_code == 200 @@ -46,7 +52,7 @@ def test_use_access_token( def test_recovery_password( - client: TestClient, normal_user_token_headers: dict[str, str] + client: TestClient, normal_user_auth_cookies: dict[str, str] ) -> None: with ( patch("app.core.config.settings.SMTP_HOST", "smtp.example.com"), @@ -55,23 +61,24 @@ def test_recovery_password( email = "test@example.com" r = client.post( f"{settings.API_V1_STR}/password-recovery/{email}", - headers=normal_user_token_headers, + cookies=normal_user_auth_cookies, ) assert r.status_code == 200 assert r.json() == {"message": "Password recovery email sent"} def test_recovery_password_user_not_exits( - client: TestClient, normal_user_token_headers: dict[str, str] + client: TestClient, normal_user_auth_cookies: dict[str, str] ) -> None: email = "jVgQr@example.com" r = client.post( f"{settings.API_V1_STR}/password-recovery/{email}", - headers=normal_user_token_headers, + cookies=normal_user_auth_cookies, ) assert r.status_code == 404 +# TODO def test_reset_password(client: TestClient, db: Session) -> None: email = random_email() password = random_lower_string() @@ -91,7 +98,7 @@ def test_reset_password(client: TestClient, db: Session) -> None: r = client.post( f"{settings.API_V1_STR}/reset-password/", - headers=headers, + cookies=headers, json=data, ) @@ -103,12 +110,12 @@ def test_reset_password(client: TestClient, db: Session) -> None: def test_reset_password_invalid_token( - client: TestClient, superuser_token_headers: dict[str, str] + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: data = {"new_password": "changethis", "token": "invalid"} r = client.post( f"{settings.API_V1_STR}/reset-password/", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) response = r.json() diff --git a/backend/app/tests/api/routes/test_users.py b/backend/app/tests/api/routes/test_users.py index ba9be65426..691788520a 100644 --- a/backend/app/tests/api/routes/test_users.py +++ b/backend/app/tests/api/routes/test_users.py @@ -8,13 +8,13 @@ from app.core.config import settings from app.core.security import verify_password from app.models import User, UserCreate -from app.tests.utils.utils import random_email, random_lower_string +from app.tests.utils.utils import extract_cookies, random_email, random_lower_string def test_get_users_superuser_me( - client: TestClient, superuser_token_headers: dict[str, str] + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: - r = client.get(f"{settings.API_V1_STR}/users/me", headers=superuser_token_headers) + r = client.get(f"{settings.API_V1_STR}/users/me", cookies=superuser_auth_cookies) current_user = r.json() assert current_user assert current_user["is_active"] is True @@ -23,9 +23,9 @@ def test_get_users_superuser_me( def test_get_users_normal_user_me( - client: TestClient, normal_user_token_headers: dict[str, str] + client: TestClient, normal_user_auth_cookies: dict[str, str] ) -> None: - r = client.get(f"{settings.API_V1_STR}/users/me", headers=normal_user_token_headers) + r = client.get(f"{settings.API_V1_STR}/users/me", cookies=normal_user_auth_cookies) current_user = r.json() assert current_user assert current_user["is_active"] is True @@ -34,7 +34,7 @@ def test_get_users_normal_user_me( def test_create_user_new_email( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: with ( patch("app.utils.send_email", return_value=None), @@ -46,7 +46,7 @@ def test_create_user_new_email( data = {"email": username, "password": password} r = client.post( f"{settings.API_V1_STR}/users/", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) assert 200 <= r.status_code < 300 @@ -57,7 +57,7 @@ def test_create_user_new_email( def test_get_existing_user( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: username = random_email() password = random_lower_string() @@ -66,7 +66,7 @@ def test_get_existing_user( user_id = user.id r = client.get( f"{settings.API_V1_STR}/users/{user_id}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, ) assert 200 <= r.status_code < 300 api_user = r.json() @@ -87,13 +87,11 @@ def test_get_existing_user_current_user(client: TestClient, db: Session) -> None "password": password, } r = client.post(f"{settings.API_V1_STR}/login/access-token", data=login_data) - tokens = r.json() - a_token = tokens["access_token"] - headers = {"Authorization": f"Bearer {a_token}"} + cookies = extract_cookies(r) r = client.get( f"{settings.API_V1_STR}/users/{user_id}", - headers=headers, + cookies=cookies, ) assert 200 <= r.status_code < 300 api_user = r.json() @@ -103,18 +101,18 @@ def test_get_existing_user_current_user(client: TestClient, db: Session) -> None def test_get_existing_user_permissions_error( - client: TestClient, normal_user_token_headers: dict[str, str] + client: TestClient, normal_user_auth_cookies: dict[str, str] ) -> None: r = client.get( f"{settings.API_V1_STR}/users/{uuid.uuid4()}", - headers=normal_user_token_headers, + cookies=normal_user_auth_cookies, ) assert r.status_code == 403 assert r.json() == {"detail": "The user doesn't have enough privileges"} def test_create_user_existing_username( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: username = random_email() # username = email @@ -124,7 +122,7 @@ def test_create_user_existing_username( data = {"email": username, "password": password} r = client.post( f"{settings.API_V1_STR}/users/", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) created_user = r.json() @@ -133,21 +131,21 @@ def test_create_user_existing_username( def test_create_user_by_normal_user( - client: TestClient, normal_user_token_headers: dict[str, str] + client: TestClient, normal_user_auth_cookies: dict[str, str] ) -> None: username = random_email() password = random_lower_string() data = {"email": username, "password": password} r = client.post( f"{settings.API_V1_STR}/users/", - headers=normal_user_token_headers, + cookies=normal_user_auth_cookies, json=data, ) assert r.status_code == 403 def test_retrieve_users( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: username = random_email() password = random_lower_string() @@ -159,7 +157,7 @@ def test_retrieve_users( user_in2 = UserCreate(email=username2, password=password2) crud.create_user(session=db, user_create=user_in2) - r = client.get(f"{settings.API_V1_STR}/users/", headers=superuser_token_headers) + r = client.get(f"{settings.API_V1_STR}/users/", cookies=superuser_auth_cookies) all_users = r.json() assert len(all_users["data"]) > 1 @@ -169,14 +167,14 @@ def test_retrieve_users( def test_update_user_me( - client: TestClient, normal_user_token_headers: dict[str, str], db: Session + client: TestClient, normal_user_auth_cookies: dict[str, str], db: Session ) -> None: full_name = "Updated Name" email = random_email() data = {"full_name": full_name, "email": email} r = client.patch( f"{settings.API_V1_STR}/users/me", - headers=normal_user_token_headers, + cookies=normal_user_auth_cookies, json=data, ) assert r.status_code == 200 @@ -192,7 +190,7 @@ def test_update_user_me( def test_update_password_me( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: new_password = random_lower_string() data = { @@ -201,7 +199,7 @@ def test_update_password_me( } r = client.patch( f"{settings.API_V1_STR}/users/me/password", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) assert r.status_code == 200 @@ -221,7 +219,7 @@ def test_update_password_me( } r = client.patch( f"{settings.API_V1_STR}/users/me/password", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=old_data, ) db.refresh(user_db) @@ -231,13 +229,13 @@ def test_update_password_me( def test_update_password_me_incorrect_password( - client: TestClient, superuser_token_headers: dict[str, str] + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: new_password = random_lower_string() data = {"current_password": new_password, "new_password": new_password} r = client.patch( f"{settings.API_V1_STR}/users/me/password", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) assert r.status_code == 400 @@ -246,7 +244,7 @@ def test_update_password_me_incorrect_password( def test_update_user_me_email_exists( - client: TestClient, normal_user_token_headers: dict[str, str], db: Session + client: TestClient, normal_user_auth_cookies: dict[str, str], db: Session ) -> None: username = random_email() password = random_lower_string() @@ -256,7 +254,7 @@ def test_update_user_me_email_exists( data = {"email": user.email} r = client.patch( f"{settings.API_V1_STR}/users/me", - headers=normal_user_token_headers, + cookies=normal_user_auth_cookies, json=data, ) assert r.status_code == 409 @@ -264,7 +262,7 @@ def test_update_user_me_email_exists( def test_update_password_me_same_password_error( - client: TestClient, superuser_token_headers: dict[str, str] + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: data = { "current_password": settings.FIRST_SUPERUSER_PASSWORD, @@ -272,7 +270,7 @@ def test_update_password_me_same_password_error( } r = client.patch( f"{settings.API_V1_STR}/users/me/password", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) assert r.status_code == 400 @@ -321,7 +319,7 @@ def test_register_user_already_exists_error(client: TestClient) -> None: def test_update_user( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: username = random_email() password = random_lower_string() @@ -331,7 +329,7 @@ def test_update_user( data = {"full_name": "Updated_full_name"} r = client.patch( f"{settings.API_V1_STR}/users/{user.id}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) assert r.status_code == 200 @@ -347,12 +345,12 @@ def test_update_user( def test_update_user_not_exists( - client: TestClient, superuser_token_headers: dict[str, str] + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: data = {"full_name": "Updated_full_name"} r = client.patch( f"{settings.API_V1_STR}/users/{uuid.uuid4()}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) assert r.status_code == 404 @@ -360,7 +358,7 @@ def test_update_user_not_exists( def test_update_user_email_exists( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: username = random_email() password = random_lower_string() @@ -375,7 +373,7 @@ def test_update_user_email_exists( data = {"email": user2.email} r = client.patch( f"{settings.API_V1_STR}/users/{user.id}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, json=data, ) assert r.status_code == 409 @@ -394,13 +392,12 @@ def test_delete_user_me(client: TestClient, db: Session) -> None: "password": password, } r = client.post(f"{settings.API_V1_STR}/login/access-token", data=login_data) - tokens = r.json() - a_token = tokens["access_token"] - headers = {"Authorization": f"Bearer {a_token}"} + cookies = extract_cookies(r) + print(f"COOKIE_ {cookies}") r = client.delete( f"{settings.API_V1_STR}/users/me", - headers=headers, + cookies=cookies, ) assert r.status_code == 200 deleted_user = r.json() @@ -414,11 +411,11 @@ def test_delete_user_me(client: TestClient, db: Session) -> None: def test_delete_user_me_as_superuser( - client: TestClient, superuser_token_headers: dict[str, str] + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: r = client.delete( f"{settings.API_V1_STR}/users/me", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, ) assert r.status_code == 403 response = r.json() @@ -426,7 +423,7 @@ def test_delete_user_me_as_superuser( def test_delete_user_super_user( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: username = random_email() password = random_lower_string() @@ -435,7 +432,7 @@ def test_delete_user_super_user( user_id = user.id r = client.delete( f"{settings.API_V1_STR}/users/{user_id}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, ) assert r.status_code == 200 deleted_user = r.json() @@ -445,18 +442,18 @@ def test_delete_user_super_user( def test_delete_user_not_found( - client: TestClient, superuser_token_headers: dict[str, str] + client: TestClient, superuser_auth_cookies: dict[str, str] ) -> None: r = client.delete( f"{settings.API_V1_STR}/users/{uuid.uuid4()}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, ) assert r.status_code == 404 assert r.json()["detail"] == "User not found" def test_delete_user_current_super_user_error( - client: TestClient, superuser_token_headers: dict[str, str], db: Session + client: TestClient, superuser_auth_cookies: dict[str, str], db: Session ) -> None: super_user = crud.get_user_by_email(session=db, email=settings.FIRST_SUPERUSER) assert super_user @@ -464,14 +461,14 @@ def test_delete_user_current_super_user_error( r = client.delete( f"{settings.API_V1_STR}/users/{user_id}", - headers=superuser_token_headers, + cookies=superuser_auth_cookies, ) assert r.status_code == 403 assert r.json()["detail"] == "Super users are not allowed to delete themselves" def test_delete_user_without_privileges( - client: TestClient, normal_user_token_headers: dict[str, str], db: Session + client: TestClient, normal_user_auth_cookies: dict[str, str], db: Session ) -> None: username = random_email() password = random_lower_string() @@ -480,7 +477,7 @@ def test_delete_user_without_privileges( r = client.delete( f"{settings.API_V1_STR}/users/{user.id}", - headers=normal_user_token_headers, + cookies=normal_user_auth_cookies, ) assert r.status_code == 403 assert r.json()["detail"] == "The user doesn't have enough privileges" diff --git a/backend/app/tests/conftest.py b/backend/app/tests/conftest.py index 90ab39a357..056ca5a0a3 100644 --- a/backend/app/tests/conftest.py +++ b/backend/app/tests/conftest.py @@ -9,7 +9,7 @@ from app.main import app from app.models import Item, User from app.tests.utils.user import authentication_token_from_email -from app.tests.utils.utils import get_superuser_token_headers +from app.tests.utils.utils import get_superuser_auth_cookies @pytest.fixture(scope="session", autouse=True) @@ -31,12 +31,12 @@ def client() -> Generator[TestClient, None, None]: @pytest.fixture(scope="module") -def superuser_token_headers(client: TestClient) -> dict[str, str]: - return get_superuser_token_headers(client) +def superuser_auth_cookies(client: TestClient) -> dict[str, str]: + return get_superuser_auth_cookies(client) @pytest.fixture(scope="module") -def normal_user_token_headers(client: TestClient, db: Session) -> dict[str, str]: +def normal_user_auth_cookies(client: TestClient, db: Session) -> dict[str, str]: return authentication_token_from_email( client=client, email=settings.EMAIL_TEST_USER, db=db ) diff --git a/backend/app/tests/utils/user.py b/backend/app/tests/utils/user.py index 9c1b073109..39150e401d 100644 --- a/backend/app/tests/utils/user.py +++ b/backend/app/tests/utils/user.py @@ -4,7 +4,7 @@ from app import crud from app.core.config import settings from app.models import User, UserCreate, UserUpdate -from app.tests.utils.utils import random_email, random_lower_string +from app.tests.utils.utils import extract_cookies, random_email, random_lower_string def user_authentication_headers( @@ -13,10 +13,8 @@ def user_authentication_headers( data = {"username": email, "password": password} r = client.post(f"{settings.API_V1_STR}/login/access-token", data=data) - response = r.json() - auth_token = response["access_token"] - headers = {"Authorization": f"Bearer {auth_token}"} - return headers + print(f"user_authentication_headers: {r}") + return extract_cookies(r) def create_random_user(db: Session) -> User: diff --git a/backend/app/tests/utils/utils.py b/backend/app/tests/utils/utils.py index 184bac44d9..99a3c9d0e1 100644 --- a/backend/app/tests/utils/utils.py +++ b/backend/app/tests/utils/utils.py @@ -1,6 +1,9 @@ import random import string +import httpx +from fastapi import Response +from fastapi.responses import JSONResponse from fastapi.testclient import TestClient from app.core.config import settings @@ -14,13 +17,31 @@ def random_email() -> str: return f"{random_lower_string()}@{random_lower_string()}.com" -def get_superuser_token_headers(client: TestClient) -> dict[str, str]: +def get_superuser_auth_cookies(client: TestClient) -> dict[str, str]: login_data = { "username": settings.FIRST_SUPERUSER, "password": settings.FIRST_SUPERUSER_PASSWORD, } r = client.post(f"{settings.API_V1_STR}/login/access-token", data=login_data) - tokens = r.json() - a_token = tokens["access_token"] - headers = {"Authorization": f"Bearer {a_token}"} - return headers + return extract_cookies(r) + + +def extract_cookies( + response: JSONResponse | httpx._models.Response | Response, +) -> dict[str, str]: + if isinstance(response, httpx._models.Response): + # Handle httpx Response + cookie_value = response.cookies.get("http_only_auth_cookie") + if cookie_value: + return {"http_only_auth_cookie": cookie_value} + else: + # Handle Starlette Response + cookie_header = response.headers.get("Set-Cookie") + if cookie_header and "http_only_auth_cookie=" in cookie_header: + cookie_value = cookie_header.split("http_only_auth_cookie=")[1].split(";")[ + 0 + ] + if cookie_value: + return {"http_only_auth_cookie": cookie_value} + + raise AssertionError("Cookie value not found") diff --git a/frontend/src/client/core/ApiRequestOptions.ts b/frontend/src/client/core/ApiRequestOptions.ts index d1136f428b..98b86b0783 100644 --- a/frontend/src/client/core/ApiRequestOptions.ts +++ b/frontend/src/client/core/ApiRequestOptions.ts @@ -18,4 +18,5 @@ export type ApiRequestOptions = { readonly responseHeader?: string readonly responseTransformer?: (data: unknown) => Promise readonly url: string + readonly withCredentials?: boolean } diff --git a/frontend/src/client/core/request.ts b/frontend/src/client/core/request.ts index 8b42272b93..d79dfebe6b 100644 --- a/frontend/src/client/core/request.ts +++ b/frontend/src/client/core/request.ts @@ -178,6 +178,8 @@ export const getHeaders = async ( } else if (options.formData !== undefined) { if (options.mediaType) { headers["Content-Type"] = options.mediaType + } else { + headers["Content-Type"] = "application/x-www-form-urlencoded" } } @@ -203,15 +205,31 @@ export const sendRequest = async ( ): Promise> => { const controller = new AbortController() + let data = body; + + // If we have formData but it's not a FormData instance, + // and Content-Type is application/x-www-form-urlencoded + if (options.formData && !isFormData(options.formData) && + headers["Content-Type"] === "application/x-www-form-urlencoded") { + const params = new URLSearchParams(); + Object.entries(options.formData).forEach(([key, value]) => { + if (value !== undefined && value !== null) { + params.append(key, String(value)); + } + }); + data = params; + } else { + data = body ?? formData; + } + let requestConfig: AxiosRequestConfig = { - data: body ?? formData, + data: data, headers, method: options.method, signal: controller.signal, url, - withCredentials: config.WITH_CREDENTIALS, + withCredentials: options.withCredentials ?? config.WITH_CREDENTIALS, } - onCancel(() => controller.abort()) for (const fn of config.interceptors.request._fns) { @@ -367,7 +385,6 @@ export const request = ( if (options.responseTransformer && isSuccess(response.status)) { transformedBody = await options.responseTransformer(responseBody) } - const result: ApiResult = { url, ok: isSuccess(response.status), @@ -375,7 +392,6 @@ export const request = ( statusText: response.statusText, body: responseHeader ?? transformedBody, } - catchErrorCodes(options, result) resolve(result.body) diff --git a/frontend/src/client/sdk.gen.ts b/frontend/src/client/sdk.gen.ts index 92ded2bde8..5d952a0a72 100644 --- a/frontend/src/client/sdk.gen.ts +++ b/frontend/src/client/sdk.gen.ts @@ -15,6 +15,7 @@ import type { ItemsDeleteItemData, ItemsDeleteItemResponse, LoginLoginAccessTokenData, + LogoutResponse, LoginLoginAccessTokenResponse, LoginTestTokenResponse, LoginRecoverPasswordData, @@ -66,6 +67,7 @@ export class ItemsService { skip: data.skip, limit: data.limit, }, + withCredentials: true, errors: { 422: "Validation Error", }, @@ -88,6 +90,7 @@ export class ItemsService { url: "/api/v1/items/", body: data.requestBody, mediaType: "application/json", + withCredentials: true, errors: { 422: "Validation Error", }, @@ -111,6 +114,7 @@ export class ItemsService { path: { id: data.id, }, + withCredentials: true, errors: { 422: "Validation Error", }, @@ -135,6 +139,7 @@ export class ItemsService { path: { id: data.id, }, + withCredentials: true, body: data.requestBody, mediaType: "application/json", errors: { @@ -160,6 +165,7 @@ export class ItemsService { path: { id: data.id, }, + withCredentials: true, errors: { 422: "Validation Error", }, @@ -183,13 +189,42 @@ export class LoginService { method: "POST", url: "/api/v1/login/access-token", formData: data.formData, + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, mediaType: "application/x-www-form-urlencoded", + withCredentials: true, errors: { 422: "Validation Error", }, }) } + + /** + * Login Access Token + * OAuth2 compatible token login, get an access token for future requests + * @param data The data for the request. + * @param data.formData + * @returns Token Successful Response + * @throws ApiError + */ + public static logout(): CancelablePromise { + return __request(OpenAPI, { + method: "POST", + url: "/api/v1/logout", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + mediaType: "application/x-www-form-urlencoded", + withCredentials: true, + errors: { + 422: "Validation Error", + }, + }) + } + + /** * Test Token * Test access token @@ -200,6 +235,7 @@ export class LoginService { return __request(OpenAPI, { method: "POST", url: "/api/v1/login/test-token", + withCredentials: true, }) } @@ -330,6 +366,7 @@ export class UsersService { return __request(OpenAPI, { method: "GET", url: "/api/v1/users/me", + withCredentials: true, }) } @@ -343,6 +380,7 @@ export class UsersService { return __request(OpenAPI, { method: "DELETE", url: "/api/v1/users/me", + withCredentials: true, }) } @@ -362,6 +400,7 @@ export class UsersService { url: "/api/v1/users/me", body: data.requestBody, mediaType: "application/json", + withCredentials: true, errors: { 422: "Validation Error", }, @@ -384,6 +423,7 @@ export class UsersService { url: "/api/v1/users/me/password", body: data.requestBody, mediaType: "application/json", + withCredentials: true, errors: { 422: "Validation Error", }, @@ -455,6 +495,7 @@ export class UsersService { }, body: data.requestBody, mediaType: "application/json", + withCredentials: true, errors: { 422: "Validation Error", }, @@ -475,6 +516,7 @@ export class UsersService { return __request(OpenAPI, { method: "DELETE", url: "/api/v1/users/{user_id}", + withCredentials: true, path: { user_id: data.userId, }, diff --git a/frontend/src/client/types.gen.ts b/frontend/src/client/types.gen.ts index c2a58d06cb..271d8f0808 100644 --- a/frontend/src/client/types.gen.ts +++ b/frontend/src/client/types.gen.ts @@ -49,6 +49,9 @@ export type Token = { token_type?: string } +export type HTTPOnlyCookie = { + message: string +} export type UpdatePassword = { current_password: string new_password: string @@ -136,7 +139,9 @@ export type LoginLoginAccessTokenData = { formData: Body_login_login_access_token } -export type LoginLoginAccessTokenResponse = Token +export type LoginLoginAccessTokenResponse = HTTPOnlyCookie + +export type LogoutResponse = HTTPOnlyCookie export type LoginTestTokenResponse = UserPublic diff --git a/frontend/src/hooks/useAuth.ts b/frontend/src/hooks/useAuth.ts index 5344493d49..f22d78827e 100644 --- a/frontend/src/hooks/useAuth.ts +++ b/frontend/src/hooks/useAuth.ts @@ -13,7 +13,7 @@ import { import { handleError } from "@/utils" const isLoggedIn = () => { - return localStorage.getItem("access_token") !== null + return localStorage.getItem("is_authenticated") !== null } const useAuth = () => { @@ -42,10 +42,10 @@ const useAuth = () => { }) const login = async (data: AccessToken) => { - const response = await LoginService.loginAccessToken({ + await LoginService.loginAccessToken({ formData: data, }) - localStorage.setItem("access_token", response.access_token) + localStorage.setItem("is_authenticated", "true") } const loginMutation = useMutation({ @@ -58,9 +58,15 @@ const useAuth = () => { }, }) - const logout = () => { - localStorage.removeItem("access_token") - navigate({ to: "/login" }) + const logout = async () => { + try { + await LoginService.logout(); + localStorage.removeItem("is_authenticated"); + navigate({ to: "/login" }); + } catch (error) { + console.error("Logout failed:", error); + navigate({ to: "/login" }); + } } return { diff --git a/frontend/src/main.tsx b/frontend/src/main.tsx index 0d80ea6f8d..b20c68a4d3 100644 --- a/frontend/src/main.tsx +++ b/frontend/src/main.tsx @@ -13,13 +13,10 @@ import { ApiError, OpenAPI } from "./client" import { CustomProvider } from "./components/ui/provider" OpenAPI.BASE = import.meta.env.VITE_API_URL -OpenAPI.TOKEN = async () => { - return localStorage.getItem("access_token") || "" -} const handleApiError = (error: Error) => { if (error instanceof ApiError && [401, 403].includes(error.status)) { - localStorage.removeItem("access_token") + localStorage.removeItem("is_authenticated") window.location.href = "/login" } } diff --git a/frontend/tests/login.spec.ts b/frontend/tests/login.spec.ts index e482934916..b36d002f5e 100644 --- a/frontend/tests/login.spec.ts +++ b/frontend/tests/login.spec.ts @@ -116,11 +116,9 @@ test("Logged-out user cannot access protected routes", async ({ page }) => { await page.waitForURL("/login") }) -test("Redirects to /login when token is wrong", async ({ page }) => { - await page.goto("/settings") - await page.evaluate(() => { - localStorage.setItem("access_token", "invalid_token") - }) +test("Redirects to /login when authentication is invalid", async ({ page }) => { + await page.context().clearCookies() + await page.goto("/settings") await page.waitForURL("/login") await expect(page).toHaveURL("/login") diff --git a/frontend/tests/utils/privateApi.ts b/frontend/tests/utils/privateApi.ts index b6fa0afe67..82c479ef32 100644 --- a/frontend/tests/utils/privateApi.ts +++ b/frontend/tests/utils/privateApi.ts @@ -18,5 +18,6 @@ export const createUser = async ({ is_verified: true, full_name: "Test User", }, + credentials: "include", }) }