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 00000000..9dc349c1 --- /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/navbar_login.html b/tom_common/templates/tom_common/partials/navbar_login.html index 0f3057a8..5e1bb22b 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 %}
  • {% if user.first_name or user.last_name %} - {{ user.first_name }} {{ user.last_name }} ({{ user.username }}) + {{ user.first_name }} {{ user.last_name }} ({{ user.username }}) {% else %} - {{ user.username }} + {{ user.username }} {% endif %}
  • 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 00000000..234c6d29 --- /dev/null +++ b/tom_common/templates/tom_common/partials/user_data.html @@ -0,0 +1,26 @@ +{% load tom_common_extras %} + +
    +
    +
    +

    User Info

    +
    + + +
    +
    +
    +
    +
    + {% for key, value in profile_data.items %} + {% if value %} +
    {% verbose_name user key %}
    +
    {{ value }}
    + {% endif %} + {% endfor %} +
    +
    +
    + + +{% include 'tom_common/partials/confirm_user_delete.html' %} 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 00000000..8bb78403 --- /dev/null +++ b/tom_common/templates/tom_common/user_profile.html @@ -0,0 +1,24 @@ +{% 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 %} +
    +
    +
    +
    +
    + +{% endblock %} diff --git a/tom_common/templatetags/user_extras.py b/tom_common/templatetags/user_extras.py index 7f898547..f892fb9c 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/tests.py b/tom_common/tests.py index 59c8a5ac..105b952a 100644 --- a/tom_common/tests.py +++ b/tom_common/tests.py @@ -66,15 +66,19 @@ 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) 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) diff --git a/tom_common/urls.py b/tom_common/urls.py index efe1e81e..89be7440 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 d7e0fa84..72b69218 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 @@ -63,13 +64,41 @@ 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 + 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): + """ + View to handle creating a user profile page. Requires a login. + """ + 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): """