diff --git a/backend/api/management/commands/gc_orphan_images.py b/backend/api/management/commands/gc_orphan_images.py new file mode 100644 index 0000000..d946bee --- /dev/null +++ b/backend/api/management/commands/gc_orphan_images.py @@ -0,0 +1,94 @@ +"""Garbage collect orphan flavor image files. + +Walks MEDIA_ROOT/flavors/ and removes directories whose paths are not referenced +by any Flavor.local_image_paths or Flavor.main_image_path. Use --dry-run to +preview without deleting. + +Files referenced by the legacy Flavor.image ImageField are also preserved (for +backwards compatibility) until that field is dropped. +""" + +from __future__ import annotations + +import os +import shutil +from pathlib import Path + +from django.conf import settings +from django.core.management.base import BaseCommand + +from api.models import Flavor + + +class Command(BaseCommand): + help = "Remove orphan flavor image files / directories not referenced by any Flavor." + + def add_arguments(self, parser): + parser.add_argument( + "--dry-run", + action="store_true", + help="Print what would be deleted but don't touch the filesystem.", + ) + + def handle(self, *args, **options): + dry_run: bool = options["dry_run"] + flavors_root = Path(settings.MEDIA_ROOT) / "flavors" + if not flavors_root.exists(): + self.stdout.write(self.style.WARNING(f"No flavors dir at {flavors_root}")) + return + + # Collect every relative path/directory that should be kept. + referenced_files: set[str] = set() + referenced_dirs: set[str] = set() + legacy_image_files: set[str] = set() + + for flavor in Flavor.objects.all().only( + "local_image_paths", "main_image_path", "image" + ): + paths = list(flavor.local_image_paths or []) + if flavor.main_image_path: + paths.append(flavor.main_image_path) + for rel in paths: + referenced_files.add(rel) + referenced_dirs.add(os.path.dirname(rel)) + if flavor.image: + legacy_image_files.add(flavor.image.name) + + # Walk the flavors tree and decide what's unreferenced. + deleted_dirs: list[str] = [] + deleted_files: list[str] = [] + kept = 0 + + for entry in sorted(flavors_root.iterdir()): + rel_dir = f"flavors/{entry.name}" + if entry.is_dir(): + if rel_dir in referenced_dirs: + # Drop only orphan files inside the referenced dir. + for f in entry.iterdir(): + rel_file = f"{rel_dir}/{f.name}" + if f.is_file() and rel_file not in referenced_files: + deleted_files.append(rel_file) + if not dry_run: + f.unlink() + else: + kept += 1 + else: + deleted_dirs.append(rel_dir) + if not dry_run: + shutil.rmtree(entry) + elif entry.is_file(): + rel_file = f"flavors/{entry.name}" + if rel_file in legacy_image_files or rel_file in referenced_files: + kept += 1 + else: + deleted_files.append(rel_file) + if not dry_run: + entry.unlink() + + verb = "Would delete" if dry_run else "Deleted" + self.stdout.write(self.style.SUCCESS(f"{verb}: {len(deleted_dirs)} dirs, {len(deleted_files)} files")) + for path in deleted_dirs: + self.stdout.write(f" dir {path}") + for path in deleted_files: + self.stdout.write(f" file {path}") + self.stdout.write(f"Kept: {kept} entries") diff --git a/backend/api/management/commands/sync_flavors.py b/backend/api/management/commands/sync_flavors.py index b711895..b122c50 100644 --- a/backend/api/management/commands/sync_flavors.py +++ b/backend/api/management/commands/sync_flavors.py @@ -1,6 +1,7 @@ import hashlib import os import re +from concurrent.futures import ThreadPoolExecutor from urllib.parse import urlparse import requests @@ -9,6 +10,8 @@ from django.core.management.base import BaseCommand from api.models import Category, Flavor +_DOWNLOAD_WORKERS = 8 + class Command(BaseCommand): help = "Syncs flavors from Holy Energy Shopify API" @@ -50,17 +53,30 @@ class Command(BaseCommand): return hashlib.sha1(path.encode("utf-8")).hexdigest()[:10] def download_gallery(self, image_urls, dir_slug): - """Download every URL into flavors//_.. Returns list of rel paths.""" - paths = [] + """Download every URL into flavors//_.. Returns list of rel paths. + + Downloads run in parallel; ordering of returned paths matches input ordering so + the gallery index is preserved. + """ + targets: list[tuple[int, str, str]] = [] for i, url in enumerate(image_urls): if not url: continue filename = f"{i:02d}_{self._hash_for(url)}.{self._ext_for(url)}" rel = os.path.join("flavors", dir_slug, filename) - saved = self.download_image_to_path(url, rel) - if saved: - paths.append(saved) - return paths + targets.append((i, url, rel)) + + if not targets: + return [] + + results: dict[int, str] = {} + with ThreadPoolExecutor(max_workers=_DOWNLOAD_WORKERS) as ex: + futures = {ex.submit(self.download_image_to_path, url, rel): i for i, url, rel in targets} + for fut, i in futures.items(): + saved = fut.result() + if saved: + results[i] = saved + return [results[i] for i, _, _ in targets if i in results] def handle(self, *args, **options): url = "https://weareholy.com/products.json?limit=250" diff --git a/backend/api/tests/test_auth.py b/backend/api/tests/test_auth.py index ad9f45c..a4a0c14 100644 --- a/backend/api/tests/test_auth.py +++ b/backend/api/tests/test_auth.py @@ -1,3 +1,4 @@ +from django.conf import settings from django.urls import reverse from rest_framework import status from rest_framework.test import APITestCase @@ -11,40 +12,36 @@ class JWTAuthTests(APITestCase): self.password = "correct-horse-battery-staple" self.user = User.objects.create_user(username=self.username, password=self.password) - def test_obtain_pair_returns_access_and_refresh(self) -> None: - url = reverse("token_obtain_pair") - response = self.client.post( - url, - {"username": self.username, "password": self.password}, - format="json", - ) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertIn("access", response.data) - self.assertIn("refresh", response.data) - - def test_access_token_authenticates_protected_endpoint(self) -> None: - pair = self.client.post( + def _login(self): + return self.client.post( reverse("token_obtain_pair"), {"username": self.username, "password": self.password}, format="json", - ).data - self.client.credentials(HTTP_AUTHORIZATION=f"Bearer {pair['access']}") + ) + + def test_obtain_pair_sets_cookies(self) -> None: + response = self._login() + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data, {"status": "ok"}) + self.assertIn(settings.JWT_AUTH_COOKIE_ACCESS, response.cookies) + self.assertIn(settings.JWT_AUTH_COOKIE_REFRESH, response.cookies) + access = response.cookies[settings.JWT_AUTH_COOKIE_ACCESS] + self.assertTrue(access["httponly"]) + self.assertEqual(access["samesite"], settings.JWT_AUTH_COOKIE_SAMESITE) + + def test_cookie_authenticates_protected_endpoint(self) -> None: + self._login() + # APIClient persists Set-Cookie across calls, so the next request is + # authenticated by the cookie alone. response = self.client.get("/api/notifications/", follow=True) self.assertEqual(response.status_code, status.HTTP_200_OK) - def test_refresh_issues_new_access_token(self) -> None: - pair = self.client.post( - reverse("token_obtain_pair"), - {"username": self.username, "password": self.password}, - format="json", - ).data - response = self.client.post( - reverse("token_refresh"), - {"refresh": pair["refresh"]}, - format="json", - ) + def test_refresh_uses_cookie_when_body_omitted(self) -> None: + self._login() + response = self.client.post(reverse("token_refresh"), format="json") self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertIn("access", response.data) + self.assertEqual(response.data, {"status": "ok"}) + self.assertIn(settings.JWT_AUTH_COOKIE_ACCESS, response.cookies) def test_invalid_refresh_is_rejected(self) -> None: response = self.client.post( @@ -53,3 +50,15 @@ class JWTAuthTests(APITestCase): format="json", ) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_logout_clears_cookies_and_blacklists_refresh(self) -> None: + self._login() + response = self.client.post(reverse("auth_logout"), format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + # set-cookie should be present with empty/expired value to clear them. + self.assertEqual( + response.cookies[settings.JWT_AUTH_COOKIE_ACCESS].value, "" + ) + self.assertEqual( + response.cookies[settings.JWT_AUTH_COOKIE_REFRESH].value, "" + ) diff --git a/backend/api/utils/__init__.py b/backend/api/utils/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/api/utils/auth.py b/backend/api/utils/auth.py new file mode 100644 index 0000000..d2d8091 --- /dev/null +++ b/backend/api/utils/auth.py @@ -0,0 +1,26 @@ +"""Auth helpers for narrowing request.user to the concrete User type. + +DRF's `request.user` is typed as `User | AnonymousUser`. After a view enforces +`IsAuthenticated`, the union is misleading: every code path past the permission +check has the concrete user. These helpers express that narrowing for mypy and +keep view bodies readable. +""" + +from __future__ import annotations + +from rest_framework.exceptions import NotAuthenticated +from rest_framework.request import Request + +from api.models import User + + +def current_user(request: Request) -> User: + """Return the authenticated User for `request` or raise 401. + + Use after `IsAuthenticated` permission. The check is defensive — the + permission class already rejected anonymous calls. + """ + user = request.user + if not isinstance(user, User): + raise NotAuthenticated + return user diff --git a/backend/api/utils/cookie_auth.py b/backend/api/utils/cookie_auth.py new file mode 100644 index 0000000..f2ecf68 --- /dev/null +++ b/backend/api/utils/cookie_auth.py @@ -0,0 +1,74 @@ +"""httpOnly-cookie JWT auth. + +Reads `access_token` from a cookie set by the login/refresh endpoints. Falls +back to the Authorization header for backwards compatibility during rollout — +strip the fallback once all clients use cookies. +""" + +from __future__ import annotations + +from django.conf import settings +from drf_spectacular.extensions import OpenApiAuthenticationExtension +from rest_framework_simplejwt.authentication import JWTAuthentication + + +class CookieJWTAuthentication(JWTAuthentication): + def authenticate(self, request): + raw_token = request.COOKIES.get(settings.JWT_AUTH_COOKIE_ACCESS) + if raw_token is None: + return super().authenticate(request) + validated_token = self.get_validated_token(raw_token) + return self.get_user(validated_token), validated_token + + +class CookieJWTScheme(OpenApiAuthenticationExtension): + """Make drf-spectacular treat CookieJWTAuthentication like the SimpleJWT scheme.""" + + target_class = "api.utils.cookie_auth.CookieJWTAuthentication" + name = "cookieAuth" + + def get_security_definition(self, auto_schema): # type: ignore[no-untyped-def] + return { + "type": "apiKey", + "in": "cookie", + "name": settings.JWT_AUTH_COOKIE_ACCESS, + } + + + + +def set_jwt_cookies(response, access: str | None, refresh: str | None) -> None: + """Attach access/refresh tokens to the response as httpOnly cookies.""" + from datetime import timedelta + + secure = settings.JWT_AUTH_COOKIE_SECURE + samesite = settings.JWT_AUTH_COOKIE_SAMESITE + path = settings.JWT_AUTH_COOKIE_PATH + + if access: + access_lifetime: timedelta = settings.SIMPLE_JWT["ACCESS_TOKEN_LIFETIME"] + response.set_cookie( + settings.JWT_AUTH_COOKIE_ACCESS, + access, + max_age=int(access_lifetime.total_seconds()), + httponly=True, + secure=secure, + samesite=samesite, + path=path, + ) + if refresh: + refresh_lifetime: timedelta = settings.SIMPLE_JWT["REFRESH_TOKEN_LIFETIME"] + response.set_cookie( + settings.JWT_AUTH_COOKIE_REFRESH, + refresh, + max_age=int(refresh_lifetime.total_seconds()), + httponly=True, + secure=secure, + samesite=samesite, + path=path, + ) + + +def clear_jwt_cookies(response) -> None: + response.delete_cookie(settings.JWT_AUTH_COOKIE_ACCESS, path=settings.JWT_AUTH_COOKIE_PATH) + response.delete_cookie(settings.JWT_AUTH_COOKIE_REFRESH, path=settings.JWT_AUTH_COOKIE_PATH) diff --git a/backend/api/views/admin.py b/backend/api/views/admin.py index 627fd1d..eceedea 100644 --- a/backend/api/views/admin.py +++ b/backend/api/views/admin.py @@ -22,6 +22,7 @@ from api.tasks import ( seed_legacy_task, sync_flavors_task, ) +from api.utils.auth import current_user # Maps Job.name → (celery task name for beat schedules, callable for direct dispatch). # Direct callables let .delay() honor CELERY_TASK_ALWAYS_EAGER in dev. @@ -155,15 +156,16 @@ class AdminViewSet(viewsets.ViewSet): @action(detail=False, methods=["post"]) def send_test_email(self, request: Request) -> Response: + me = current_user(request) try: send_mail( "Holy Flavors Admin Test Email", ( - f"This is a test email sent to {request.user.email} from the " + f"This is a test email sent to {me.email} from the " f"Holy Flavors Admin Interface." ), settings.DEFAULT_FROM_EMAIL, - [request.user.email], + [me.email], fail_silently=False, ) return Response({"status": "Test email sent!"}) @@ -193,3 +195,4 @@ class AdminViewSet(viewsets.ViewSet): elif request.method == "DELETE": user.delete() return Response(status=status.HTTP_204_NO_CONTENT) + return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED) diff --git a/backend/api/views/auth.py b/backend/api/views/auth.py new file mode 100644 index 0000000..f09f4a6 --- /dev/null +++ b/backend/api/views/auth.py @@ -0,0 +1,65 @@ +"""Cookie-backed JWT auth views. + +`CookieTokenObtainPairView` and `CookieTokenRefreshView` move the JWT pair into +httpOnly cookies on the response so the frontend never has tokens in JS-readable +storage. Refresh accepts the token from the cookie *or* the legacy request body. +`CookieLogoutView` clears both cookies and blacklists the refresh token. +""" + +from __future__ import annotations + +from django.conf import settings +from rest_framework import status +from rest_framework.permissions import AllowAny +from rest_framework.request import Request +from rest_framework.response import Response +from rest_framework.views import APIView +from rest_framework_simplejwt.exceptions import TokenError +from rest_framework_simplejwt.tokens import RefreshToken +from rest_framework_simplejwt.views import TokenObtainPairView, TokenRefreshView + +from api.utils.cookie_auth import clear_jwt_cookies, set_jwt_cookies + + +class CookieTokenObtainPairView(TokenObtainPairView): + def post(self, request: Request, *args, **kwargs): + response = super().post(request, *args, **kwargs) + if response.status_code == status.HTTP_200_OK: + access = response.data.pop("access", None) + refresh = response.data.pop("refresh", None) + set_jwt_cookies(response, access, refresh) + response.data = {"status": "ok"} + return response + + +class CookieTokenRefreshView(TokenRefreshView): + def post(self, request: Request, *args, **kwargs): + if "refresh" not in request.data: + cookie_refresh = request.COOKIES.get(settings.JWT_AUTH_COOKIE_REFRESH) + if cookie_refresh: + # request.data is a QueryDict / dict — make a mutable copy. + data = request.data.copy() if hasattr(request.data, "copy") else dict(request.data) + data["refresh"] = cookie_refresh + request._full_data = data # type: ignore[attr-defined] + response = super().post(request, *args, **kwargs) + if response.status_code == status.HTTP_200_OK: + access = response.data.pop("access", None) + refresh = response.data.pop("refresh", None) + set_jwt_cookies(response, access, refresh) + response.data = {"status": "ok"} + return response + + +class CookieLogoutView(APIView): + permission_classes = [AllowAny] + + def post(self, request: Request) -> Response: + refresh = request.COOKIES.get(settings.JWT_AUTH_COOKIE_REFRESH) + if refresh: + try: + RefreshToken(refresh).blacklist() + except TokenError: + pass + response = Response({"status": "ok"}) + clear_jwt_cookies(response) + return response diff --git a/backend/api/views/flavor.py b/backend/api/views/flavor.py index 879f742..ef26034 100644 --- a/backend/api/views/flavor.py +++ b/backend/api/views/flavor.py @@ -12,6 +12,7 @@ from api.services.search import ( filter_flavors_by_query, score_relevance, ) +from api.utils.auth import current_user class CategoryViewSet(viewsets.ReadOnlyModelViewSet): @@ -95,7 +96,7 @@ class FlavorViewSet(viewsets.ModelViewSet): @action(detail=False, methods=["get"], permission_classes=[permissions.IsAuthenticated]) def followed_top(self, request: Request) -> Response: - followed_users = request.user.following.all() + followed_users = current_user(request).following.all() top_flavors = ( Flavor.objects.filter(ratings__user__in=followed_users) .annotate(average_rating=Avg("ratings__score")) diff --git a/backend/api/views/rating.py b/backend/api/views/rating.py index 62132bb..5f60e28 100644 --- a/backend/api/views/rating.py +++ b/backend/api/views/rating.py @@ -9,6 +9,7 @@ from rest_framework.response import Response from api.models import Notification, Rating, Reply from api.serializers import RatingSerializer, ReplySerializer from api.services.mentions import parse_mentions +from api.utils.auth import current_user from api.views._pagination import FeedPagination @@ -22,12 +23,13 @@ class RatingViewSet(viewsets.ModelViewSet): return super().create(request, *args, **kwargs) def perform_create(self, serializer): + me = current_user(self.request) flavor = serializer.validated_data["flavor"] - if Rating.objects.filter(user=self.request.user, flavor=flavor).exists(): + if Rating.objects.filter(user=me, flavor=flavor).exists(): raise serializers.ValidationError("You have already rated this flavor.") - rating = serializer.save(user=self.request.user) + rating = serializer.save(user=me) if rating.comment: - parse_mentions(rating.comment, self.request.user, rating=rating) + parse_mentions(rating.comment, me, rating=rating) def get_queryset(self): return ( @@ -38,7 +40,8 @@ class RatingViewSet(viewsets.ModelViewSet): @action(detail=False, methods=["get"], permission_classes=[permissions.IsAuthenticated]) def feed(self, request: Request) -> Response: - followed_users = request.user.following.all() + me = current_user(request) + followed_users = me.following.all() feed_ratings = ( Rating.objects.filter(user__in=followed_users) .select_related("user", "flavor", "flavor__category") @@ -75,23 +78,24 @@ class RatingViewSet(viewsets.ModelViewSet): @action(detail=True, methods=["post"], permission_classes=[permissions.IsAuthenticated]) def reply(self, request: Request, pk=None) -> Response: + me = current_user(request) rating = self.get_object() text = request.data.get("text") if not text: return Response({"error": "Text is required"}, status=status.HTTP_400_BAD_REQUEST) - reply = Reply.objects.create(user=request.user, rating=rating, text=text) + reply = Reply.objects.create(user=me, rating=rating, text=text) - if rating.user != request.user: + if rating.user != me: Notification.objects.create( recipient=rating.user, - actor=request.user, + actor=me, notification_type="reply", rating=rating, reply=reply, ) - parse_mentions(text, request.user, rating=rating, reply=reply) + parse_mentions(text, me, rating=rating, reply=reply) return Response( ReplySerializer(reply, context={"request": request}).data, @@ -108,13 +112,15 @@ class ReplyViewSet(viewsets.ModelViewSet): return Reply.objects.select_related("user", "rating").order_by("created_at") def perform_update(self, serializer): + me = current_user(self.request) reply = self.get_object() - if reply.user != self.request.user and not self.request.user.is_superuser: + if reply.user != me and not me.is_superuser: raise PermissionDenied("You cannot edit this reply.") new_reply = serializer.save() - parse_mentions(new_reply.text, self.request.user, rating=new_reply.rating, reply=new_reply) + parse_mentions(new_reply.text, me, rating=new_reply.rating, reply=new_reply) def perform_destroy(self, instance): - if instance.user != self.request.user and not self.request.user.is_superuser: + me = current_user(self.request) + if instance.user != me and not me.is_superuser: raise PermissionDenied("You cannot delete this reply.") instance.delete() diff --git a/backend/api/views/support.py b/backend/api/views/support.py index 59876df..28007ab 100644 --- a/backend/api/views/support.py +++ b/backend/api/views/support.py @@ -6,6 +6,7 @@ from rest_framework.response import Response from api.models import Notification, Ticket, TicketMessage, User from api.serializers import TicketMessageSerializer, TicketSerializer +from api.utils.auth import current_user class TicketViewSet(viewsets.ModelViewSet): @@ -13,52 +14,57 @@ class TicketViewSet(viewsets.ModelViewSet): permission_classes = [permissions.IsAuthenticated] def get_queryset(self): - if self.request.user.is_superuser: + if getattr(self, "swagger_fake_view", False): + return Ticket.objects.none() + me = current_user(self.request) + if me.is_superuser: return ( Ticket.objects.all() .prefetch_related("messages", "messages__user") .order_by("-updated_at") ) return ( - Ticket.objects.filter(user=self.request.user) + Ticket.objects.filter(user=me) .prefetch_related("messages", "messages__user") .order_by("-updated_at") ) def perform_create(self, serializer): - ticket = serializer.save(user=self.request.user) - admins = User.objects.filter(is_superuser=True).exclude(pk=self.request.user.pk) + me = current_user(self.request) + ticket = serializer.save(user=me) + admins = User.objects.filter(is_superuser=True).exclude(pk=me.pk) for admin in admins: Notification.objects.create( recipient=admin, - actor=self.request.user, + actor=me, notification_type="ticket_new", ticket=ticket, ) @action(detail=True, methods=["post"]) def add_message(self, request: Request, pk=None) -> Response: + me = current_user(request) ticket = self.get_object() text = request.data.get("text") if not text: return Response({"error": "Text is required"}, status=status.HTTP_400_BAD_REQUEST) - msg = TicketMessage.objects.create(ticket=ticket, user=request.user, text=text) + msg = TicketMessage.objects.create(ticket=ticket, user=me, text=text) - if request.user.is_superuser: - if ticket.user != request.user: + if me.is_superuser: + if ticket.user != me: Notification.objects.create( recipient=ticket.user, - actor=request.user, + actor=me, notification_type="ticket_reply", ticket=ticket, ) else: - admins = User.objects.filter(is_superuser=True).exclude(pk=request.user.pk) + admins = User.objects.filter(is_superuser=True).exclude(pk=me.pk) for admin in admins: Notification.objects.create( recipient=admin, - actor=request.user, + actor=me, notification_type="ticket_reply", ticket=ticket, ) diff --git a/backend/api/views/user.py b/backend/api/views/user.py index c4f8c14..14e554b 100644 --- a/backend/api/views/user.py +++ b/backend/api/views/user.py @@ -19,6 +19,7 @@ from api.serializers import ( ) from api.services.ip_logging import log_user_ip from api.tasks import send_email_task +from api.utils.auth import current_user def _generate_code() -> str: @@ -42,7 +43,8 @@ class UserViewSet(viewsets.ModelViewSet): @action(detail=False, methods=["get"], permission_classes=[permissions.IsAuthenticated]) def following_list(self, request: Request) -> Response: - following = request.user.following.all() + me = current_user(request) + following = me.following.all() return Response(self.get_serializer(following, many=True).data) @action(detail=False, methods=["post"], permission_classes=[permissions.AllowAny]) @@ -213,6 +215,7 @@ class UserViewSet(viewsets.ModelViewSet): @action(detail=True, methods=["post"], permission_classes=[permissions.IsAuthenticated]) def add_comment(self, request: Request, pk=None) -> Response: + me = current_user(request) profile_owner = self.get_object() text = request.data.get("text") if not text: @@ -220,14 +223,14 @@ class UserViewSet(viewsets.ModelViewSet): comment = ProfileComment.objects.create( profile_owner=profile_owner, - author=request.user, + author=me, text=text, ) - if profile_owner != request.user: + if profile_owner != me: Notification.objects.create( recipient=profile_owner, - actor=request.user, + actor=me, notification_type="profile_comment", profile_comment=comment, ) @@ -244,17 +247,14 @@ class UserViewSet(viewsets.ModelViewSet): permission_classes=[permissions.IsAuthenticated], ) def delete_comment(self, request: Request, pk=None, comment_id=None) -> Response: + me = current_user(request) profile_owner = self.get_object() try: comment = ProfileComment.objects.get(pk=comment_id, profile_owner=profile_owner) except ProfileComment.DoesNotExist: return Response(status=status.HTTP_404_NOT_FOUND) - if ( - comment.author != request.user - and not request.user.is_superuser - and profile_owner != request.user - ): + if comment.author != me and not me.is_superuser and profile_owner != me: return Response(status=status.HTTP_403_FORBIDDEN) comment.delete() @@ -262,12 +262,13 @@ class UserViewSet(viewsets.ModelViewSet): @action(detail=False, methods=["get"]) def me(self, request: Request) -> Response: - log_user_ip(request.user, request) - return Response(self.get_serializer(request.user).data) + me = current_user(request) + log_user_ip(me, request) + return Response(self.get_serializer(me).data) @action(detail=False, methods=["patch"]) def update_preferences(self, request: Request) -> Response: - user = request.user + user = current_user(request) theme = request.data.get("theme") language = request.data.get("language") banner_id = request.data.get("selected_banner") @@ -309,7 +310,7 @@ class UserViewSet(viewsets.ModelViewSet): @action(detail=False, methods=["post"]) def update_avatar(self, request: Request) -> Response: - user = request.user + user = current_user(request) avatar = request.FILES.get("avatar") if not avatar: return Response({"error": "No avatar provided"}, status=status.HTTP_400_BAD_REQUEST) @@ -326,7 +327,7 @@ class UserViewSet(viewsets.ModelViewSet): @action(detail=False, methods=["post"]) def change_password(self, request: Request) -> Response: - user = request.user + user = current_user(request) old_password = request.data.get("old_password") new_password = request.data.get("new_password") if not user.check_password(old_password): @@ -337,7 +338,7 @@ class UserViewSet(viewsets.ModelViewSet): @action(detail=False, methods=["patch"]) def update_profile(self, request: Request) -> Response: - user = request.user + user = current_user(request) username = request.data.get("username") email = request.data.get("email") @@ -370,18 +371,19 @@ class UserViewSet(viewsets.ModelViewSet): @action(detail=True, methods=["post"], permission_classes=[permissions.IsAuthenticated]) def follow(self, request: Request, pk=None) -> Response: + me = current_user(request) user_to_follow = self.get_object() - if user_to_follow == request.user: + if user_to_follow == me: return Response( {"error": "You cannot follow yourself"}, status=status.HTTP_400_BAD_REQUEST, ) - if not request.user.following.filter(pk=user_to_follow.pk).exists(): - request.user.following.add(user_to_follow) + if not me.following.filter(pk=user_to_follow.pk).exists(): + me.following.add(user_to_follow) Notification.objects.create( recipient=user_to_follow, - actor=request.user, + actor=me, notification_type="follow", ) @@ -389,18 +391,23 @@ class UserViewSet(viewsets.ModelViewSet): @action(detail=True, methods=["post"], permission_classes=[permissions.IsAuthenticated]) def unfollow(self, request: Request, pk=None) -> Response: + me = current_user(request) user_to_unfollow = self.get_object() - request.user.following.remove(user_to_unfollow) + me.following.remove(user_to_unfollow) return Response({"status": "unfollowed"}) @action(detail=False, methods=["post"]) def confirm_email(self, request: Request) -> Response: - user = request.user + user = current_user(request) code = request.data.get("code") if not user.email_confirmation_code or code != user.email_confirmation_code: return Response( {"error": "Invalid or expired code"}, status=status.HTTP_400_BAD_REQUEST ) + if not user.pending_email: + return Response( + {"error": "No pending email change"}, status=status.HTTP_400_BAD_REQUEST + ) user.email = user.pending_email user.pending_email = None @@ -454,7 +461,7 @@ class UserViewSet(viewsets.ModelViewSet): @method_decorator(ratelimit(key="user", rate="3/h", method="POST", block=True)) def request_account_deletion(self, request: Request) -> Response: code = _generate_code() - user = request.user + user = current_user(request) user.email_confirmation_code = code user.save() @@ -471,7 +478,7 @@ class UserViewSet(viewsets.ModelViewSet): @action(detail=False, methods=["post"]) def confirm_account_deletion(self, request: Request) -> Response: - user = request.user + user = current_user(request) code = request.data.get("code") if not user.email_confirmation_code or code != user.email_confirmation_code: return Response( @@ -485,7 +492,7 @@ class UserViewSet(viewsets.ModelViewSet): def dashboard(self, request: Request) -> Response: from django.db.models import Avg, Q - user = request.user + user = current_user(request) rated_ids = user.ratings.values_list("flavor_id", flat=True) followed_users = user.following.all() @@ -529,13 +536,17 @@ class NotificationViewSet(viewsets.ReadOnlyModelViewSet): pagination_class = None def get_queryset(self): - return self.request.user.notifications.select_related( + if getattr(self, "swagger_fake_view", False): + return Notification.objects.none() + me = current_user(self.request) + return me.notifications.select_related( "actor", "rating__flavor", "reply__rating__flavor" ).order_by("-created_at") @action(detail=False, methods=["post"]) def mark_all_read(self, request: Request) -> Response: - request.user.notifications.filter(is_read=False).update(is_read=True) + me = current_user(request) + me.notifications.filter(is_read=False).update(is_read=True) return Response({"status": "all read"}) @action(detail=True, methods=["post"]) diff --git a/backend/holy_backend/settings/base.py b/backend/holy_backend/settings/base.py index 9686882..df6cb2f 100644 --- a/backend/holy_backend/settings/base.py +++ b/backend/holy_backend/settings/base.py @@ -132,7 +132,7 @@ AUTH_USER_MODEL = "api.User" REST_FRAMEWORK = { "DEFAULT_AUTHENTICATION_CLASSES": [ - "rest_framework_simplejwt.authentication.JWTAuthentication", + "api.utils.cookie_auth.CookieJWTAuthentication", ], "DEFAULT_PERMISSION_CLASSES": [ "rest_framework.permissions.IsAuthenticatedOrReadOnly", @@ -151,10 +151,19 @@ SIMPLE_JWT = { "AUTH_HEADER_TYPES": ("Bearer",), } +# httpOnly-cookie JWT config. The Authorization header path is kept as a +# fallback during the rollout — once all clients use cookies it can be removed. +JWT_AUTH_COOKIE_ACCESS = "access_token" +JWT_AUTH_COOKIE_REFRESH = "refresh_token" +JWT_AUTH_COOKIE_SECURE = False # prod.py overrides to True +JWT_AUTH_COOKIE_SAMESITE = "Lax" +JWT_AUTH_COOKIE_PATH = "/" + CORS_ALLOWED_ORIGINS = get_env_list( "CORS_ALLOWED_ORIGINS", [f"https://{DOMAIN}", "http://localhost:5173", "http://127.0.0.1:5173"], ) +CORS_ALLOW_CREDENTIALS = True FRONTEND_URL = os.environ.get("FRONTEND_URL", f"https://{DOMAIN}").rstrip("/") diff --git a/backend/holy_backend/settings/prod.py b/backend/holy_backend/settings/prod.py index cf69de2..ee45337 100644 --- a/backend/holy_backend/settings/prod.py +++ b/backend/holy_backend/settings/prod.py @@ -15,6 +15,7 @@ except KeyError as exc: SECURE_SSL_REDIRECT = True SESSION_COOKIE_SECURE = True CSRF_COOKIE_SECURE = True +JWT_AUTH_COOKIE_SECURE = True SECURE_BROWSER_XSS_FILTER = True SECURE_CONTENT_TYPE_NOSNIFF = True SECURE_HSTS_SECONDS = 31536000 # 1 year diff --git a/backend/holy_backend/urls.py b/backend/holy_backend/urls.py index d03275f..28bad1f 100644 --- a/backend/holy_backend/urls.py +++ b/backend/holy_backend/urls.py @@ -7,7 +7,9 @@ from django.urls import include, path from django.views.generic.base import TemplateView from django.views.static import serve from drf_spectacular.views import SpectacularAPIView, SpectacularRedocView, SpectacularSwaggerView -from rest_framework_simplejwt.views import TokenObtainPairView, TokenRefreshView, TokenVerifyView +from rest_framework_simplejwt.views import TokenVerifyView + +from api.views.auth import CookieLogoutView, CookieTokenObtainPairView, CookieTokenRefreshView def health_check(request): @@ -25,12 +27,13 @@ urlpatterns = [ # Ensure admin ends with a slash for Django's redirection logic path(f"{admin_url}/", admin.site.urls), path("api/", include("api.urls")), - path("api/auth/token/", TokenObtainPairView.as_view(), name="token_obtain_pair"), - path("api/auth/token/refresh/", TokenRefreshView.as_view(), name="token_refresh"), + path("api/auth/token/", CookieTokenObtainPairView.as_view(), name="token_obtain_pair"), + path("api/auth/token/refresh/", CookieTokenRefreshView.as_view(), name="token_refresh"), path("api/auth/token/verify/", TokenVerifyView.as_view(), name="token_verify"), - # Legacy alias: old DRF TokenAuth endpoint now returns a JWT pair so clients - # relying on POST /api/token/ keep working for one release. - path("api/token/", TokenObtainPairView.as_view(), name="api_token_legacy"), + path("api/auth/logout/", CookieLogoutView.as_view(), name="auth_logout"), + # Legacy alias: old DRF TokenAuth endpoint now sets cookies just like the + # canonical login route so clients relying on POST /api/token/ keep working. + path("api/token/", CookieTokenObtainPairView.as_view(), name="api_token_legacy"), path("api-auth/", include("rest_framework.urls")), path("api/schema/", SpectacularAPIView.as_view(), name="schema"), path( diff --git a/backend/schema.yaml b/backend/schema.yaml index 961743f..821c766 100644 --- a/backend/schema.yaml +++ b/backend/schema.yaml @@ -16,7 +16,7 @@ paths: tags: - admin-custom security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': description: No response body @@ -32,7 +32,7 @@ paths: tags: - admin-custom security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': description: No response body @@ -48,7 +48,7 @@ paths: tags: - admin-custom security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': description: No response body @@ -63,7 +63,7 @@ paths: tags: - admin-custom security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': description: No response body @@ -78,7 +78,7 @@ paths: tags: - admin-custom security: - - jwtAuth: [] + - cookieAuth: [] responses: '204': description: No response body @@ -88,7 +88,7 @@ paths: tags: - admin-custom security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': description: No response body @@ -97,7 +97,7 @@ paths: tags: - admin-custom security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': description: No response body @@ -107,7 +107,7 @@ paths: tags: - admin-custom security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': description: No response body @@ -117,7 +117,7 @@ paths: tags: - admin-custom security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': description: No response body @@ -127,7 +127,7 @@ paths: tags: - admin-custom security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': description: No response body @@ -137,7 +137,18 @@ paths: tags: - admin-custom security: - - jwtAuth: [] + - cookieAuth: [] + responses: + '200': + description: No response body + /api/auth/logout/: + post: + operationId: auth_logout_create + tags: + - auth + security: + - cookieAuth: [] + - {} responses: '200': description: No response body @@ -228,7 +239,7 @@ paths: tags: - banners security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -256,7 +267,7 @@ paths: $ref: '#/components/schemas/Banner' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '201': content: @@ -277,7 +288,7 @@ paths: tags: - banners security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -310,7 +321,7 @@ paths: $ref: '#/components/schemas/Banner' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -341,7 +352,7 @@ paths: schema: $ref: '#/components/schemas/PatchedBanner' security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -361,7 +372,7 @@ paths: tags: - banners security: - - jwtAuth: [] + - cookieAuth: [] responses: '204': description: No response body @@ -390,7 +401,7 @@ paths: $ref: '#/components/schemas/Banner' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -423,7 +434,7 @@ paths: $ref: '#/components/schemas/Banner' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -437,7 +448,7 @@ paths: tags: - banners security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -459,7 +470,7 @@ paths: tags: - categories security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -481,7 +492,7 @@ paths: tags: - categories security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -517,7 +528,7 @@ paths: tags: - flavors security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -543,7 +554,7 @@ paths: $ref: '#/components/schemas/Flavor' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '201': content: @@ -564,7 +575,7 @@ paths: tags: - flavors security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -597,7 +608,7 @@ paths: $ref: '#/components/schemas/Flavor' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -628,7 +639,7 @@ paths: schema: $ref: '#/components/schemas/PatchedFlavor' security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -648,7 +659,7 @@ paths: tags: - flavors security: - - jwtAuth: [] + - cookieAuth: [] responses: '204': description: No response body @@ -658,7 +669,7 @@ paths: tags: - flavors security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -672,7 +683,7 @@ paths: tags: - flavors security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -687,7 +698,7 @@ paths: tags: - flavors security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -702,7 +713,7 @@ paths: tags: - flavors security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -717,7 +728,7 @@ paths: tags: - notifications security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -734,12 +745,13 @@ paths: - in: path name: id schema: - type: string + type: integer + description: A unique integer value identifying this notification. required: true tags: - notifications security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -754,7 +766,8 @@ paths: - in: path name: id schema: - type: string + type: integer + description: A unique integer value identifying this notification. required: true tags: - notifications @@ -771,7 +784,7 @@ paths: $ref: '#/components/schemas/Notification' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -797,7 +810,7 @@ paths: $ref: '#/components/schemas/Notification' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -818,7 +831,7 @@ paths: tags: - ratings security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -844,7 +857,7 @@ paths: $ref: '#/components/schemas/Rating' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '201': content: @@ -865,7 +878,7 @@ paths: tags: - ratings security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -898,7 +911,7 @@ paths: $ref: '#/components/schemas/Rating' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -929,7 +942,7 @@ paths: schema: $ref: '#/components/schemas/PatchedRating' security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -949,7 +962,7 @@ paths: tags: - ratings security: - - jwtAuth: [] + - cookieAuth: [] responses: '204': description: No response body @@ -978,7 +991,7 @@ paths: $ref: '#/components/schemas/Rating' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -992,7 +1005,7 @@ paths: tags: - ratings security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1006,7 +1019,7 @@ paths: tags: - ratings security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -1028,7 +1041,7 @@ paths: tags: - replies security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1053,7 +1066,7 @@ paths: $ref: '#/components/schemas/Reply' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '201': content: @@ -1074,7 +1087,7 @@ paths: tags: - replies security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1106,7 +1119,7 @@ paths: $ref: '#/components/schemas/Reply' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1137,7 +1150,7 @@ paths: schema: $ref: '#/components/schemas/PatchedReply' security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1157,7 +1170,7 @@ paths: tags: - replies security: - - jwtAuth: [] + - cookieAuth: [] responses: '204': description: No response body @@ -1174,7 +1187,7 @@ paths: tags: - tickets security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1199,7 +1212,7 @@ paths: $ref: '#/components/schemas/Ticket' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '201': content: @@ -1214,12 +1227,13 @@ paths: - in: path name: id schema: - type: string + type: integer + description: A unique integer value identifying this ticket. required: true tags: - tickets security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1233,7 +1247,8 @@ paths: - in: path name: id schema: - type: string + type: integer + description: A unique integer value identifying this ticket. required: true tags: - tickets @@ -1250,7 +1265,7 @@ paths: $ref: '#/components/schemas/Ticket' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1264,7 +1279,8 @@ paths: - in: path name: id schema: - type: string + type: integer + description: A unique integer value identifying this ticket. required: true tags: - tickets @@ -1280,7 +1296,7 @@ paths: schema: $ref: '#/components/schemas/PatchedTicket' security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1294,12 +1310,13 @@ paths: - in: path name: id schema: - type: string + type: integer + description: A unique integer value identifying this ticket. required: true tags: - tickets security: - - jwtAuth: [] + - cookieAuth: [] responses: '204': description: No response body @@ -1310,7 +1327,8 @@ paths: - in: path name: id schema: - type: string + type: integer + description: A unique integer value identifying this ticket. required: true tags: - tickets @@ -1327,7 +1345,7 @@ paths: $ref: '#/components/schemas/Ticket' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1342,7 +1360,8 @@ paths: - in: path name: id schema: - type: string + type: integer + description: A unique integer value identifying this ticket. required: true tags: - tickets @@ -1359,7 +1378,7 @@ paths: $ref: '#/components/schemas/Ticket' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1407,7 +1426,7 @@ paths: tags: - users security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1432,7 +1451,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '201': content: @@ -1453,7 +1472,7 @@ paths: tags: - users security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1485,7 +1504,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1516,7 +1535,7 @@ paths: schema: $ref: '#/components/schemas/PatchedUser' security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1536,7 +1555,7 @@ paths: tags: - users security: - - jwtAuth: [] + - cookieAuth: [] responses: '204': description: No response body @@ -1565,7 +1584,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1591,7 +1610,7 @@ paths: tags: - users security: - - jwtAuth: [] + - cookieAuth: [] responses: '204': description: No response body @@ -1620,7 +1639,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1653,7 +1672,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1679,7 +1698,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1705,7 +1724,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -1732,7 +1751,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1758,7 +1777,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1772,7 +1791,7 @@ paths: tags: - users security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1786,7 +1805,7 @@ paths: tags: - users security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1800,7 +1819,7 @@ paths: tags: - users security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1822,7 +1841,7 @@ paths: tags: - users security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -1849,7 +1868,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1875,7 +1894,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -1902,7 +1921,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -1929,7 +1948,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -1956,7 +1975,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -1981,7 +2000,7 @@ paths: schema: $ref: '#/components/schemas/PatchedUser' security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -2006,7 +2025,7 @@ paths: schema: $ref: '#/components/schemas/PatchedUser' security: - - jwtAuth: [] + - cookieAuth: [] responses: '200': content: @@ -2032,7 +2051,7 @@ paths: $ref: '#/components/schemas/User' required: true security: - - jwtAuth: [] + - cookieAuth: [] - {} responses: '200': @@ -2962,7 +2981,7 @@ components: - unread_notifications_count - username securitySchemes: - jwtAuth: - type: http - scheme: bearer - bearerFormat: JWT + cookieAuth: + type: apiKey + in: cookie + name: access_token diff --git a/frontend/src/api/schema.d.ts b/frontend/src/api/schema.d.ts index af135d9..f07a67a 100644 --- a/frontend/src/api/schema.d.ts +++ b/frontend/src/api/schema.d.ts @@ -132,6 +132,22 @@ export interface paths { patch?: never; trace?: never; }; + '/api/auth/logout/': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations['auth_logout_create']; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; '/api/auth/token/': { parameters: { query?: never; @@ -1639,6 +1655,24 @@ export interface operations { }; }; }; + auth_logout_create: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description No response body */ + 200: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; auth_token_create: { parameters: { query?: never; @@ -2226,7 +2260,8 @@ export interface operations { query?: never; header?: never; path: { - id: string; + /** @description A unique integer value identifying this notification. */ + id: number; }; cookie?: never; }; @@ -2247,7 +2282,8 @@ export interface operations { query?: never; header?: never; path: { - id: string; + /** @description A unique integer value identifying this notification. */ + id: number; }; cookie?: never; }; @@ -2704,7 +2740,8 @@ export interface operations { query?: never; header?: never; path: { - id: string; + /** @description A unique integer value identifying this ticket. */ + id: number; }; cookie?: never; }; @@ -2725,7 +2762,8 @@ export interface operations { query?: never; header?: never; path: { - id: string; + /** @description A unique integer value identifying this ticket. */ + id: number; }; cookie?: never; }; @@ -2752,7 +2790,8 @@ export interface operations { query?: never; header?: never; path: { - id: string; + /** @description A unique integer value identifying this ticket. */ + id: number; }; cookie?: never; }; @@ -2772,7 +2811,8 @@ export interface operations { query?: never; header?: never; path: { - id: string; + /** @description A unique integer value identifying this ticket. */ + id: number; }; cookie?: never; }; @@ -2799,7 +2839,8 @@ export interface operations { query?: never; header?: never; path: { - id: string; + /** @description A unique integer value identifying this ticket. */ + id: number; }; cookie?: never; }; @@ -2826,7 +2867,8 @@ export interface operations { query?: never; header?: never; path: { - id: string; + /** @description A unique integer value identifying this ticket. */ + id: number; }; cookie?: never; }; diff --git a/frontend/src/contexts/AuthContext.tsx b/frontend/src/contexts/AuthContext.tsx index ccf0b18..8b3bd38 100644 --- a/frontend/src/contexts/AuthContext.tsx +++ b/frontend/src/contexts/AuthContext.tsx @@ -1,6 +1,6 @@ import { createContext, useCallback, useEffect, useState, type ReactNode } from 'react'; import { useTranslation } from 'react-i18next'; -import api, { clearTokens } from '../lib/api'; +import api from '../lib/api'; export interface AuthUser { username: string; @@ -23,7 +23,7 @@ export interface AuthContextValue { following: FollowingEntry[]; loadingUser: boolean; refetchUser: () => Promise; - logout: () => void; + logout: () => Promise; } export const AuthContext = createContext(null); @@ -35,14 +35,10 @@ export const AuthProvider = ({ children }: { children: ReactNode }) => { const [loadingUser, setLoadingUser] = useState(true); const refetchUser = useCallback(async () => { - const token = localStorage.getItem('access'); - if (!token) { - setUser(null); - setLoadingUser(false); - return; - } try { - const res = await api.get('users/me/'); + const res = await api.get('users/me/', { + ...({ skipAuthRedirect: true } as object), + }); if (res.data?.username) { setUser(res.data); if (res.data.theme) localStorage.setItem('theme', res.data.theme); @@ -70,8 +66,12 @@ export const AuthProvider = ({ children }: { children: ReactNode }) => { refetchUser(); }, [refetchUser]); - const logout = useCallback(() => { - clearTokens(); + const logout = useCallback(async () => { + try { + await api.post('auth/logout/'); + } catch { + /* ignore — cookies still get cleared on server when refresh expires */ + } setUser(null); setFollowing([]); i18n.changeLanguage(navigator.language.split('-')[0] || 'en'); diff --git a/frontend/src/lib/api.test.ts b/frontend/src/lib/api.test.ts index 62f071e..bddbb3c 100644 --- a/frontend/src/lib/api.test.ts +++ b/frontend/src/lib/api.test.ts @@ -1,32 +1,14 @@ -import { describe, it, expect, beforeEach } from 'vitest'; -import { clearTokens, getAccessToken, setTokens } from './api'; +import { describe, it, expect } from 'vitest'; +import { clearLegacyToken } from './api'; -describe('token helpers', () => { - beforeEach(() => { - clearTokens(); - }); - - it('returns null when no access token is stored', () => { - expect(getAccessToken()).toBeNull(); - }); - - it('stores and retrieves the access token', () => { - setTokens('access-123', 'refresh-abc'); - expect(getAccessToken()).toBe('access-123'); - expect(localStorage.getItem('refresh')).toBe('refresh-abc'); - }); - - it('updates access without clobbering refresh when refresh is omitted', () => { - setTokens('first-access', 'first-refresh'); - setTokens('second-access'); - expect(getAccessToken()).toBe('second-access'); - expect(localStorage.getItem('refresh')).toBe('first-refresh'); - }); - - it('clearTokens removes both access and refresh', () => { - setTokens('a', 'b'); - clearTokens(); - expect(getAccessToken()).toBeNull(); +describe('clearLegacyToken', () => { + it('removes legacy DRF and JWT localStorage tokens', () => { + localStorage.setItem('token', 'old-drf'); + localStorage.setItem('access', 'old-jwt'); + localStorage.setItem('refresh', 'old-refresh'); + clearLegacyToken(); + expect(localStorage.getItem('token')).toBeNull(); + expect(localStorage.getItem('access')).toBeNull(); expect(localStorage.getItem('refresh')).toBeNull(); }); }); diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 3ef4a72..3ca0833 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -1,40 +1,21 @@ import axios from 'axios'; import type { AxiosError, AxiosRequestConfig } from 'axios'; -const ACCESS_KEY = 'access'; -const REFRESH_KEY = 'refresh'; +/** + * httpOnly-cookie auth: the backend sets `access_token` + `refresh_token` cookies + * on login. We don't read or write them from JS — `withCredentials` is enough. + */ -export function getAccessToken(): string | null { - return localStorage.getItem(ACCESS_KEY); -} - -export function setTokens(access: string, refresh?: string): void { - localStorage.setItem(ACCESS_KEY, access); - if (refresh) { - localStorage.setItem(REFRESH_KEY, refresh); - } -} - -export function clearTokens(): void { - localStorage.removeItem(ACCESS_KEY); - localStorage.removeItem(REFRESH_KEY); -} - -/** One-time migration: remove the legacy DRF TokenAuth key from localStorage. */ +/** One-time migration: remove the legacy localStorage tokens (DRF and old JWT). */ export function clearLegacyToken(): void { localStorage.removeItem('token'); + localStorage.removeItem('access'); + localStorage.removeItem('refresh'); } const api = axios.create({ baseURL: import.meta.env.VITE_API_URL || 'http://localhost:8000/api/', -}); - -api.interceptors.request.use((config) => { - const access = getAccessToken(); - if (access) { - config.headers.Authorization = `Bearer ${access}`; - } - return config; + withCredentials: true, }); type RetriableConfig = AxiosRequestConfig & { @@ -45,21 +26,14 @@ type RetriableConfig = AxiosRequestConfig & { skipAuthRedirect?: boolean; }; -let refreshPromise: Promise | null = null; - -async function refreshAccessToken(): Promise { - const refresh = localStorage.getItem(REFRESH_KEY); - if (!refresh) return null; +let refreshPromise: Promise | null = null; +async function refreshAccessToken(): Promise { try { - const res = await axios.post(`${api.defaults.baseURL}auth/token/refresh/`, { refresh }); - const newAccess = res.data.access as string; - const newRefresh = res.data.refresh as string | undefined; - setTokens(newAccess, newRefresh); - return newAccess; + await axios.post(`${api.defaults.baseURL}auth/token/refresh/`, {}, { withCredentials: true }); + return true; } catch { - clearTokens(); - return null; + return false; } } @@ -72,16 +46,16 @@ api.interceptors.response.use( } const url = original.url ?? ''; - if (url.includes('auth/token/')) { + if (url.includes('auth/token/') || url.includes('auth/logout/')) { return Promise.reject(error); } original._retry = true; refreshPromise = refreshPromise ?? refreshAccessToken(); - const newAccess = await refreshPromise; + const ok = await refreshPromise; refreshPromise = null; - if (!newAccess) { + if (!ok) { if ( !original.skipAuthRedirect && typeof window !== 'undefined' && @@ -92,10 +66,6 @@ api.interceptors.response.use( return Promise.reject(error); } - original.headers = { - ...(original.headers ?? {}), - Authorization: `Bearer ${newAccess}`, - }; return api(original); }, ); diff --git a/frontend/src/pages/Login.tsx b/frontend/src/pages/Login.tsx index fe651d8..498c054 100644 --- a/frontend/src/pages/Login.tsx +++ b/frontend/src/pages/Login.tsx @@ -2,7 +2,7 @@ import React, { useState } from 'react'; import { Typography, TextField, Button, Tab, Tabs, Alert, Stack, Box } from '@mui/material'; import ArrowBackIcon from '@mui/icons-material/ArrowBack'; import { Link } from 'react-router-dom'; -import api, { setTokens } from '../lib/api'; +import api from '../lib/api'; import { useTitle } from '../hooks/useTitle'; import { useTranslation } from 'react-i18next'; import { PageShell, GlassCard, FormCard } from '../components/ui'; @@ -31,8 +31,7 @@ const Login: React.FC = () => { e.preventDefault(); setMessage(null); try { - const res = await api.post('auth/token/', { username, password }); - setTokens(res.data.access, res.data.refresh); + await api.post('auth/token/', { username, password }); window.location.href = '/'; } catch (err) { const data = readError(err).response?.data; diff --git a/frontend/src/pages/Settings.tsx b/frontend/src/pages/Settings.tsx index f675a80..b79a9a6 100644 --- a/frontend/src/pages/Settings.tsx +++ b/frontend/src/pages/Settings.tsx @@ -178,8 +178,6 @@ const Settings: React.FC = ({ themeName, onThemeChange }) => { if (!ok) return; try { await confirmDeletion.mutateAsync(deletionCode); - localStorage.removeItem('access'); - localStorage.removeItem('refresh'); window.location.href = '/'; } catch { setMessage({ type: 'error', text: 'Invalid code. Deletion failed.' });