From f1fffdbac4de90c9500e8aaa1b56880d678162f4 Mon Sep 17 00:00:00 2001 From: Jacob Coffee Date: Wed, 7 Oct 2026 22:21:38 -0500 Subject: [PATCH 1/9] rm slugged urls --- apps/users/urls.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/users/urls.py b/apps/users/urls.py index 110090ff4..a452f17d6 100644 --- a/apps/users/urls.py +++ b/apps/users/urls.py @@ -7,6 +7,8 @@ app_name = "users" urlpatterns = [ path("edit/", views.UserUpdate.as_view(), name="user_profile_edit"), + path("profile/detail/", views.UserDetail.as_view(), name="user_detail"), + path("profile/delete/", views.UserDeleteView.as_view(), name="user_delete"), path("membership/", views.MembershipCreate.as_view(), name="user_membership_create"), path("membership/edit/", views.MembershipUpdate.as_view(), name="user_membership_edit"), re_path( @@ -44,6 +46,4 @@ views.SponsorshipDetailView.as_view(), name="sponsorship_application_detail", ), - re_path(r"^(?P[-a-zA-Z0-9_\@\.+]+)/delete/$", views.UserDeleteView.as_view(), name="user_delete"), - re_path(r"^(?P[-a-zA-Z0-9_\@\.+]+)/$", views.UserDetail.as_view(), name="user_detail"), ] From a990795f81ee98e7904b410d105ad115f7f1bc6b Mon Sep 17 00:00:00 2001 From: Jacob Coffee Date: Wed, 7 Oct 2026 22:22:20 -0500 Subject: [PATCH 2/9] show only users profile --- apps/users/views.py | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/apps/users/views.py b/apps/users/views.py index 9a3264d2e..f180eb707 100644 --- a/apps/users/views.py +++ b/apps/users/views.py @@ -127,8 +127,8 @@ class UserUpdate(LoginRequiredMixin, UpdateView): """Edit the current user's profile information.""" form_class = UserProfileForm - slug_field = "username" template_name = "users/user_form.html" + success_url = reverse_lazy("users:user_detail") @method_decorator(check_honeypot) def dispatch(self, *args, **kwargs): @@ -140,17 +140,14 @@ def get_object(self, queryset=None): return User.objects.get(username=self.request.user) -class UserDetail(DetailView): - """Display a user's public profile page.""" +class UserDetail(LoginRequiredMixin, DetailView): + """Display the logged-in user's own profile details.""" - slug_field = "username" + template_name = "users/user_detail.html" - def get_queryset(self): - """Return all users if viewing own profile, searchable users otherwise.""" - queryset = User.objects.select_related() - if self.request.user.username == self.kwargs["slug"]: - return queryset - return queryset.searchable() + def get_object(self, queryset=None): + """Return the current logged-in user.""" + return self.request.user class HoneypotSignupView(SignupView): @@ -178,18 +175,15 @@ def get_success_url(self): return reverse("users:user_profile_edit") -class UserDeleteView(LoginRequiredMixin, UserPassesTestMixin, DeleteView): +class UserDeleteView(LoginRequiredMixin, DeleteView): """Allow users to delete their own account.""" - model = User success_url = reverse_lazy("home") - slug_field = "username" - raise_exception = True http_method_names = ["post", "delete"] - def test_func(self): - """Only allow users to delete their own account.""" - return self.get_object() == self.request.user + def get_object(self, queryset=None): + """Return the current logged-in user.""" + return self.request.user class MembershipDeleteView(LoginRequiredMixin, UserPassesTestMixin, DeleteView): @@ -202,7 +196,7 @@ class MembershipDeleteView(LoginRequiredMixin, UserPassesTestMixin, DeleteView): def get_success_url(self): """Redirect to the user's profile page after deletion.""" - return reverse("users:user_detail", kwargs={"slug": self.request.user.username}) + return reverse("users:user_detail") def test_func(self): """Only allow the membership creator to delete it.""" From f82b910edfb759b8e704ed5c75f2135b47173585 Mon Sep 17 00:00:00 2001 From: Jacob Coffee Date: Wed, 7 Oct 2026 22:22:31 -0500 Subject: [PATCH 3/9] no username needed --- apps/users/templates/users/user_form.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/users/templates/users/user_form.html b/apps/users/templates/users/user_form.html index 0ccf7a900..3eab0c8d6 100644 --- a/apps/users/templates/users/user_form.html +++ b/apps/users/templates/users/user_form.html @@ -42,7 +42,7 @@

Edit information for: {% firstof user.get_full

-
From eefea30acb731eccd8468405304bd9de05f83007 Mon Sep 17 00:00:00 2001 From: Jacob Coffee Date: Wed, 7 Oct 2026 22:22:44 -0500 Subject: [PATCH 4/9] rm url for lookup --- apps/users/models.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/apps/users/models.py b/apps/users/models.py index b2eda9c6e..413c27e5e 100644 --- a/apps/users/models.py +++ b/apps/users/models.py @@ -5,7 +5,6 @@ from django.conf import settings from django.contrib.auth.models import AbstractUser from django.db import models -from django.urls import reverse from django.utils import timezone from markupfield.fields import MarkupField from tastypie.models import create_api_key @@ -51,10 +50,6 @@ class User(AbstractUser): objects = CustomUserManager() - def get_absolute_url(self): - """Return the URL for the user's profile page.""" - return reverse("users:user_detail", kwargs={"slug": self.username}) - @property def has_membership(self): """Return True if the user has an associated PSF membership.""" From 9752c12265efb8be7f5d169e28ba5198d0e213c1 Mon Sep 17 00:00:00 2001 From: Jacob Coffee Date: Wed, 7 Oct 2026 22:23:54 -0500 Subject: [PATCH 5/9] no slug --- apps/users/tests/test_views.py | 93 +++++------------------ pydotorg/context_processors.py | 2 +- pydotorg/tests/test_context_processors.py | 4 +- templates/includes/authenticated.html | 2 +- 4 files changed, 23 insertions(+), 78 deletions(-) diff --git a/apps/users/tests/test_views.py b/apps/users/tests/test_views.py index 1c8109053..b8251698b 100644 --- a/apps/users/tests/test_views.py +++ b/apps/users/tests/test_views.py @@ -146,80 +146,30 @@ def test_user_update_redirect(self): settings.HONEYPOT_FIELD_NAME: settings.HONEYPOT_VALUE, } response = self.client.post(url, post_data) - profile_url = reverse("users:user_detail", kwargs={"slug": "username"}) + profile_url = reverse("users:user_detail") self.assertRedirects(response, profile_url) - # should return 404 for another user - another_user_url = reverse("users:user_detail", kwargs={"slug": "spameggs"}) - response = self.client.get(another_user_url) - self.assertEqual(response.status_code, 404) - - # should return 404 if the user is not logged-in + # should redirect to login if the user is not logged-in self.client.logout() response = self.client.get(profile_url) - self.assertEqual(response.status_code, 404) + self.assertRedirects(response, "{}?next={}".format(reverse("account_login"), profile_url)) def test_user_detail(self): - # Ensure detail page is viewable without login, but that edit URLs - # do not appear - detail_url = reverse("users:user_detail", kwargs={"slug": self.user.username}) + # Ensure the detail page shows the logged-in user's own profile + detail_url = reverse("users:user_detail") edit_url = reverse("users:user_profile_edit") + self.client.login(username=self.user2.username, password="password") response = self.client.get(detail_url) - self.assertTrue(self.user.is_active) - self.assertNotContains(response, edit_url) - - # Ensure edit url is available to logged in users - self.client.login(username="username", password="password") - response = self.client.get(detail_url) + self.assertEqual(response.context["object"], self.user2) self.assertContains(response, edit_url) - # Ensure inactive accounts shouldn't be shown to users. - user = User.objects.create_user( - username="foobar", - password="baz", - email="paradiselost@example.com", - ) - user.is_active = False - user.save() - self.assertFalse(user.is_active) - detail_url = reverse("users:user_detail", kwargs={"slug": user.username}) - response = self.client.get(detail_url) - self.assertEqual(response.status_code, 404) - - def test_special_usernames(self): - # Ensure usernames in the forms of: - # first.last - # user@host.com - # are allowed to view their profile pages since we allow them in - # the username field - u1 = User.objects.create_user( - username="user.name", - password="password", - ) - detail_url = reverse("users:user_detail", kwargs={"slug": u1.username}) - edit_url = reverse("users:user_profile_edit") - - self.client.login(username=u1.username, password="password") - response = self.client.get(detail_url) - self.assertEqual(response.status_code, 200) - - response = self.client.get(edit_url) - self.assertEqual(response.status_code, 200) - - u2 = User.objects.create_user( - username="user@example.com", - password="password", - ) - - detail_url = reverse("users:user_detail", kwargs={"slug": u2.username}) - edit_url = reverse("users:user_profile_edit") - - self.client.login(username=u2.username, password="password") - response = self.client.get(detail_url) - self.assertEqual(response.status_code, 200) - - response = self.client.get(edit_url) - self.assertEqual(response.status_code, 200) + def test_user_detail_not_addressable_by_username(self): + # Profiles used to live at /users//; existing and unknown + # usernames must be indistinguishable to prevent user enumeration. + existing = self.client.get(f"/users/{self.user.username}/") + unknown = self.client.get("/users/thisusernamedoesntexist/") + self.assertEqual(existing.status_code, 404) + self.assertEqual(unknown.status_code, 404) def test_user_new_account(self): self.assertUserCreated( @@ -298,29 +248,24 @@ def test_is_active_login(self): self.assertRedirects(response, "{}?next={}".format(reverse("account_login"), url)) def test_user_delete_needs_to_be_logged_in(self): - url = reverse("users:user_delete", kwargs={"slug": self.user.username}) + url = reverse("users:user_delete") response = self.client.delete(url) self.assertRedirects(response, "{}?next={}".format(reverse("account_login"), url)) def test_user_delete_invalid_request_method(self): - url = reverse("users:user_delete", kwargs={"slug": self.user.username}) + url = reverse("users:user_delete") self.client.login(username=self.user.username, password="password") response = self.client.get(url) self.assertEqual(response.status_code, 405) - def test_user_delete_different_user(self): - url = reverse("users:user_delete", kwargs={"slug": self.user.username}) - self.client.login(username=self.user2.username, password="password") - response = self.client.delete(url) - self.assertEqual(response.status_code, 403) - def test_user_delete(self): - url = reverse("users:user_delete", kwargs={"slug": self.user.username}) + url = reverse("users:user_delete") self.client.login(username=self.user.username, password="password") response = self.client.delete(url) self.assertRedirects(response, reverse("home")) self.assertRaises(User.DoesNotExist, User.objects.get, username=self.user.username) self.assertRaises(Membership.DoesNotExist, Membership.objects.get, creator=self.user) + self.assertTrue(User.objects.filter(username=self.user2.username).exists()) def test_membership_delete_needs_to_be_logged_in(self): url = reverse("users:user_membership_delete", kwargs={"slug": self.user2.username}) @@ -353,7 +298,7 @@ def test_membership_delete(self): url = reverse("users:user_membership_delete", kwargs={"slug": self.user2.username}) self.client.login(username=self.user2.username, password="password") response = self.client.delete(url) - self.assertRedirects(response, reverse("users:user_detail", kwargs={"slug": self.user2.username})) + self.assertRedirects(response, reverse("users:user_detail")) # TODO: We can't use 'self.user2.refresh_from_db()' because # of https://code.djangoproject.com/ticket/27846. with self.assertRaises(Membership.DoesNotExist): diff --git a/pydotorg/context_processors.py b/pydotorg/context_processors.py index d9a74637d..658e77cb2 100644 --- a/pydotorg/context_processors.py +++ b/pydotorg/context_processors.py @@ -51,7 +51,7 @@ def user_nav_bar_links(request): "account": { "label": "Your Account", "urls": [ - {"url": reverse("users:user_detail", args=[user.username]), "label": "View profile"}, + {"url": reverse("users:user_detail"), "label": "View profile"}, {"url": reverse("users:user_profile_edit"), "label": "Edit profile"}, {"url": reverse("account_change_password"), "label": "Change password"}, ], diff --git a/pydotorg/tests/test_context_processors.py b/pydotorg/tests/test_context_processors.py index 0b941e12c..4d7ac9b84 100644 --- a/pydotorg/tests/test_context_processors.py +++ b/pydotorg/tests/test_context_processors.py @@ -41,7 +41,7 @@ def test_user_nav_bar_links_for_non_psf_members(self): "account": { "label": "Your Account", "urls": [ - {"url": reverse("users:user_detail", args=["foo"]), "label": "View profile"}, + {"url": reverse("users:user_detail"), "label": "View profile"}, {"url": reverse("users:user_profile_edit"), "label": "Edit profile"}, {"url": reverse("account_change_password"), "label": "Change password"}, ], @@ -70,7 +70,7 @@ def test_user_nav_bar_links_for_psf_members(self): "account": { "label": "Your Account", "urls": [ - {"url": reverse("users:user_detail", args=["foo"]), "label": "View profile"}, + {"url": reverse("users:user_detail"), "label": "View profile"}, {"url": reverse("users:user_profile_edit"), "label": "Edit profile"}, {"url": reverse("account_change_password"), "label": "Change password"}, ], diff --git a/templates/includes/authenticated.html b/templates/includes/authenticated.html index 457dc13a7..d453c5900 100644 --- a/templates/includes/authenticated.html +++ b/templates/includes/authenticated.html @@ -2,7 +2,7 @@