From 2c9b1bc4722aae9aaf705460de49e761f7483f00 Mon Sep 17 00:00:00 2001 From: Joey Chatelain Date: Mon, 18 Nov 2024 10:19:22 -0800 Subject: [PATCH 1/5] add profile page --- .../tom_common/partials/navbar_login.html | 4 +- .../tom_common/partials/user_data.html | 44 +++++++++++++++++++ .../templates/tom_common/user_profile.html | 26 +++++++++++ tom_common/templatetags/user_extras.py | 14 ++++++ tom_common/urls.py | 3 +- tom_common/views.py | 9 ++++ 6 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 tom_common/templates/tom_common/partials/user_data.html create mode 100644 tom_common/templates/tom_common/user_profile.html diff --git a/tom_common/templates/tom_common/partials/navbar_login.html b/tom_common/templates/tom_common/partials/navbar_login.html index 0f3057a80..5e1bb22ba 100644 --- a/tom_common/templates/tom_common/partials/navbar_login.html +++ b/tom_common/templates/tom_common/partials/navbar_login.html @@ -1,9 +1,9 @@ {% if user.is_authenticated %}
  • diff --git a/tom_common/templates/tom_common/partials/user_data.html b/tom_common/templates/tom_common/partials/user_data.html new file mode 100644 index 000000000..10b730928 --- /dev/null +++ b/tom_common/templates/tom_common/partials/user_data.html @@ -0,0 +1,44 @@ +{% load tom_common_extras %} + +
    +
    +
    +

    User Info

    +
    + + +
    +
    +
    +
    +
    + {% for key, value in profile_data.items %} + {% if value %} +
    {% verbose_name user key %}
    +
    {{ value }}
    + {% endif %} + {% endfor %} +
    +
    +
    + + + diff --git a/tom_common/templates/tom_common/user_profile.html b/tom_common/templates/tom_common/user_profile.html new file mode 100644 index 000000000..c7585e36b --- /dev/null +++ b/tom_common/templates/tom_common/user_profile.html @@ -0,0 +1,26 @@ +{% extends 'tom_common/base.html' %} +{% load tom_common_extras user_extras %} +{% load bootstrap4 %} +{% block title %}User Profile{% endblock %} +{% block content %} +

    + {% if user.first_name or user.last_name %} + {{ user.first_name }} {{ user.last_name }} ({{ user.username }}) + {% else %} + {{ user.username }} + {% endif %} +

    + +
    +
    +
    + {% user_data user %} + {% user_data user %} +
    +
    + {% user_data user %} +
    +
    +
    + +{% endblock %} diff --git a/tom_common/templatetags/user_extras.py b/tom_common/templatetags/user_extras.py index 7f8985476..f892fb9ce 100644 --- a/tom_common/templatetags/user_extras.py +++ b/tom_common/templatetags/user_extras.py @@ -1,5 +1,6 @@ from django import template from django.contrib.auth.models import Group, User +from django.forms.models import model_to_dict register = template.Library() @@ -24,3 +25,16 @@ def user_list(context): 'request': context['request'], 'users': User.objects.all() } + + +@register.inclusion_tag('tom_common/partials/user_data.html') +def user_data(user): + """ + Returns the user information as a dictionary. + """ + exlcude_fields = ['password', 'last_login', 'id', 'is_active'] + user_fields = [field.name for field in user._meta.fields if field.name not in exlcude_fields] + return { + 'user': user, + 'profile_data': model_to_dict(user, fields=user_fields), + } diff --git a/tom_common/urls.py b/tom_common/urls.py index efe1e81e5..89be74405 100644 --- a/tom_common/urls.py +++ b/tom_common/urls.py @@ -26,7 +26,7 @@ from tom_base import __version__ from tom_common.api_views import GroupViewSet from tom_common.views import UserListView, UserPasswordChangeView, UserCreateView, UserDeleteView, UserUpdateView -from tom_common.views import CommentDeleteView, GroupCreateView, GroupUpdateView, GroupDeleteView +from tom_common.views import CommentDeleteView, GroupCreateView, GroupUpdateView, GroupDeleteView, UserDetailView from tom_common.views import robots_txt from .api_router import collect_api_urls, SharedAPIRootRouter # DRF routers are setup in each INSTALL_APPS url.py @@ -49,6 +49,7 @@ path('users/create/', UserCreateView.as_view(), name='user-create'), path('users//delete/', UserDeleteView.as_view(), name='user-delete'), path('users//update/', UserUpdateView.as_view(), name='user-update'), + path('users//profile/', UserDetailView.as_view(), name='user-profile'), path('groups/create/', GroupCreateView.as_view(), name='group-create'), path('groups//update/', GroupUpdateView.as_view(), name='group-update'), path('groups//delete/', GroupDeleteView.as_view(), name='group-delete'), diff --git a/tom_common/views.py b/tom_common/views.py index d7e0fa847..edc246689 100644 --- a/tom_common/views.py +++ b/tom_common/views.py @@ -2,6 +2,7 @@ from django.views.generic import TemplateView from django.views.generic.edit import FormView, DeleteView from django.views.generic.edit import UpdateView, CreateView +from django.views.generic.detail import DetailView from django.conf import settings from django.contrib.auth.models import User, Group from django.contrib.auth.mixins import LoginRequiredMixin @@ -71,6 +72,14 @@ class UserDeleteView(SuperuserRequiredMixin, DeleteView): model = User +class UserDetailView(LoginRequiredMixin, DetailView): + """ + View to handle creating a user profile page. Requires a login. + """ + template_name = 'tom_common/user_profile.html' + model = User + + class UserPasswordChangeView(SuperuserRequiredMixin, FormView): """ View that handles modification of the password for a ``User``. Requires authorization. From c52b58f1b245dbc5b99f24410f141b84dade6c4e Mon Sep 17 00:00:00 2001 From: Joey Chatelain Date: Mon, 18 Nov 2024 15:17:51 -0800 Subject: [PATCH 2/5] Add self delete --- .../partials/confirm_user_delete.html | 24 +++++++++++++++++++ .../tom_common/partials/user_data.html | 20 +--------------- tom_common/views.py | 4 ++-- 3 files changed, 27 insertions(+), 21 deletions(-) create mode 100644 tom_common/templates/tom_common/partials/confirm_user_delete.html diff --git a/tom_common/templates/tom_common/partials/confirm_user_delete.html b/tom_common/templates/tom_common/partials/confirm_user_delete.html new file mode 100644 index 000000000..9dc349c1c --- /dev/null +++ b/tom_common/templates/tom_common/partials/confirm_user_delete.html @@ -0,0 +1,24 @@ +{% load tom_common_extras %} + + diff --git a/tom_common/templates/tom_common/partials/user_data.html b/tom_common/templates/tom_common/partials/user_data.html index 10b730928..234c6d29c 100644 --- a/tom_common/templates/tom_common/partials/user_data.html +++ b/tom_common/templates/tom_common/partials/user_data.html @@ -23,22 +23,4 @@

    User Info

    - +{% include 'tom_common/partials/confirm_user_delete.html' %} diff --git a/tom_common/views.py b/tom_common/views.py index edc246689..cbd176f9d 100644 --- a/tom_common/views.py +++ b/tom_common/views.py @@ -64,9 +64,9 @@ class UserListView(LoginRequiredMixin, TemplateView): template_name = 'auth/user_list.html' -class UserDeleteView(SuperuserRequiredMixin, DeleteView): +class UserDeleteView(LoginRequiredMixin, DeleteView): """ - View that handles deletion of a ``User``. Requires authorization. + View that handles deletion of a ``User``. Requires login. """ success_url = reverse_lazy('user-list') model = User From be24d99763c7132adfd1b6b3c96e05b30673dabe Mon Sep 17 00:00:00 2001 From: Joey Chatelain Date: Mon, 18 Nov 2024 15:34:50 -0800 Subject: [PATCH 3/5] remove test for superuser on delete. --- tom_common/tests.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tom_common/tests.py b/tom_common/tests.py index 59c8a5ac1..ae02b09c7 100644 --- a/tom_common/tests.py +++ b/tom_common/tests.py @@ -72,9 +72,6 @@ def test_must_be_superuser(self): response = self.client.get(reverse('admin-user-change-password', kwargs={'pk': user.id})) self.assertEqual(response.status_code, 302) - response = self.client.get(reverse('user-delete', kwargs={'pk': user.id})) - self.assertEqual(response.status_code, 302) - def test_user_can_update_self(self): user = User.objects.create(username='luke', password='forc3') self.client.force_login(user) From 6ca6e3cb2270fac1822dd2a00e7669bb4c46dda8 Mon Sep 17 00:00:00 2001 From: Joey Chatelain Date: Mon, 18 Nov 2024 16:12:53 -0800 Subject: [PATCH 4/5] create redirect so non-super user cannot delete other users --- tom_common/tests.py | 7 +++++++ tom_common/views.py | 10 ++++++++++ 2 files changed, 17 insertions(+) diff --git a/tom_common/tests.py b/tom_common/tests.py index ae02b09c7..105b952a8 100644 --- a/tom_common/tests.py +++ b/tom_common/tests.py @@ -66,6 +66,13 @@ def test_user_delete(self): self.assertEqual(response.status_code, 302) self.assertFalse(User.objects.filter(pk=user.id).exists()) + def test_non_superuser_cannot_delete_other_user(self): + user = User.objects.create(username='deleteme', email='deleteme@example.com', password='deleted') + other_user = User.objects.create_user(username='other', email='other@example.com', password='other') + self.client.force_login(user) + response = self.client.post(reverse('user-delete', kwargs={'pk': other_user.id})) + self.assertRedirects(response, reverse('user-delete', kwargs={'pk': user.id})) + def test_must_be_superuser(self): user = User.objects.create_user(username='notallowed', email='notallowed@example.com', password='notallowed') self.client.force_login(user) diff --git a/tom_common/views.py b/tom_common/views.py index cbd176f9d..1be1eaf38 100644 --- a/tom_common/views.py +++ b/tom_common/views.py @@ -71,6 +71,16 @@ class UserDeleteView(LoginRequiredMixin, DeleteView): success_url = reverse_lazy('user-list') model = User + def dispatch(self, *args, **kwargs): + """ + Directs the class-based view to the correct method for the HTTP request method. Ensures that non-superusers + are not incorrectly updating the profiles of other users. + """ + if not self.request.user.is_superuser and self.request.user.id != self.kwargs['pk']: + return redirect('user-delete', self.request.user.id) + else: + return super().dispatch(*args, **kwargs) + class UserDetailView(LoginRequiredMixin, DetailView): """ From 3d2253945aa57abfbe864b1145bf7276eb9e9738 Mon Sep 17 00:00:00 2001 From: Joey Chatelain Date: Mon, 18 Nov 2024 16:20:59 -0800 Subject: [PATCH 5/5] prevent non-superusers from going to other user profile page --- tom_common/templates/tom_common/user_profile.html | 2 -- tom_common/views.py | 10 ++++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tom_common/templates/tom_common/user_profile.html b/tom_common/templates/tom_common/user_profile.html index c7585e36b..8bb784031 100644 --- a/tom_common/templates/tom_common/user_profile.html +++ b/tom_common/templates/tom_common/user_profile.html @@ -15,10 +15,8 @@

    {% user_data user %} - {% user_data user %}
    - {% user_data user %}
    diff --git a/tom_common/views.py b/tom_common/views.py index 1be1eaf38..72b692189 100644 --- a/tom_common/views.py +++ b/tom_common/views.py @@ -89,6 +89,16 @@ class UserDetailView(LoginRequiredMixin, DetailView): template_name = 'tom_common/user_profile.html' model = User + def dispatch(self, *args, **kwargs): + """ + Directs the class-based view to the correct method for the HTTP request method. Ensures that non-superusers + are not incorrectly updating the profiles of other users. + """ + if not self.request.user.is_superuser and self.request.user.id != self.kwargs['pk']: + return redirect('user-profile', self.request.user.id) + else: + return super().dispatch(*args, **kwargs) + class UserPasswordChangeView(SuperuserRequiredMixin, FormView): """