Skip to content

Commit

Permalink
reverting unneeded changes
Browse files Browse the repository at this point in the history
  • Loading branch information
Jacobjeevan committed Dec 9, 2024
1 parent c38cfc3 commit 5b6397c
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 103 deletions.
10 changes: 1 addition & 9 deletions care/facility/api/viewsets/facility_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class UserFilter(filters.FilterSet):
choices=[(key, key) for key in User.TYPE_VALUE_MAP],
coerce=lambda role: User.TYPE_VALUE_MAP[role],
)
username = filters.CharFilter(field_name="username", lookup_expr="icontains")

class Meta:
model = User
Expand All @@ -35,21 +36,12 @@ class FacilityUserViewSet(GenericViewSet, mixins.ListModelMixin):

def get_queryset(self):
try:
search_fields = {
key: self.request.query_params.get(key)
for key in self.search_fields
if self.request.query_params.get(key)
}
facility = Facility.objects.get(
external_id=self.kwargs.get("facility_external_id"),
)
queryset = facility.users.filter(
deleted=False,
).order_by("-last_login")
if search_fields:
queryset = queryset.filter(
**{key: value for key, value in search_fields.items() if value}
)
return queryset.prefetch_related(
Prefetch(
"skills",
Expand Down
22 changes: 1 addition & 21 deletions care/users/api/viewsets/change_password.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from django.contrib.auth import get_user_model
from django.shortcuts import get_object_or_404
from drf_spectacular.utils import extend_schema, extend_schema_view
from rest_framework import serializers, status
from rest_framework.generics import UpdateAPIView
Expand Down Expand Up @@ -30,22 +29,7 @@ class ChangePasswordView(UpdateAPIView):
model = User

def update(self, request, *args, **kwargs):
username = request.data.get("username")
if not username:
return Response(
{"message": ["Username is required"]},
status=status.HTTP_400_BAD_REQUEST,
)
self.object = get_object_or_404(User, username=username)
if not self.has_permission(request, self.object):
return Response(
{
"message": [
"User does not have elevated permissions to change password"
]
},
status=status.HTTP_403_FORBIDDEN,
)
self.object = self.request.user
serializer = self.get_serializer(data=request.data)

if serializer.is_valid():
Expand All @@ -64,7 +48,3 @@ def update(self, request, *args, **kwargs):
return Response({"message": "Password updated successfully"})

return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)

def has_permission(self, request, user):
authuser = request.user
return authuser == user or authuser.is_superuser
17 changes: 1 addition & 16 deletions care/users/api/viewsets/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,18 +91,6 @@ def last_active_after(self, queryset, name, value):
return queryset.filter(last_login__gte=date)


class UserViewSetPermission(DRYPermissions):
def has_permission(self, request, view):
if request.method == "GET" and view.action == "retrieve":
return True
return super().has_permission(request, view)

def has_object_permission(self, request, view, obj):
if request.method == "GET" and view.action == "retrieve":
return True
return super().has_object_permission(request, view, obj)


class UserViewSet(
mixins.RetrieveModelMixin,
mixins.UpdateModelMixin,
Expand All @@ -125,7 +113,7 @@ class UserViewSet(
queryset = queryset.filter(Q(asset__isnull=True))
lookup_field = "username"
lookup_value_regex = "[^/]+"
permission_classes = (IsAuthenticated, UserViewSetPermission)
permission_classes = (IsAuthenticated, DRYPermissions)
filter_backends = (
filters.DjangoFilterBackend,
rest_framework_filters.OrderingFilter,
Expand Down Expand Up @@ -167,9 +155,6 @@ def get_queryset(self):

def get_object(self) -> User:
try:
if self.request.method == "GET" and self.action == "retrieve":
username = self.kwargs.get("username")
return get_object_or_404(User, username=username)
return super().get_object()
except Http404 as e:
error = "User not found"
Expand Down
2 changes: 1 addition & 1 deletion care/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ def has_read_permission(request):
return True

def has_object_read_permission(self, request):
return request.user.is_superuser or self == request.user
return True

@staticmethod
def has_write_permission(request):
Expand Down
57 changes: 1 addition & 56 deletions care/users/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,61 +113,6 @@ def test_superuser_can_delete(self):
deleted=False,
)

def test_superuser_can_change_password_of_others(self):
"""Test a user with superuser access can change the password of other users underneath the hierarchy"""
username = self.data_2["username"]
password = self.data_2["password"]
response = self.client.put(
"/api/v1/password_change/",
{
"username": username,
"old_password": password,
"new_password": "password2",
},
)
self.assertEqual(response.status_code, status.HTTP_200_OK)

def test_superuser_cannot_change_password_of_others_without_username(
self,
):
"""Test a user with superuser access cannot change the password of other users without username"""
response = self.client.put(
"/api/v1/password_change/",
{"old_password": "password", "new_password": "password2"},
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.data["message"][0], "Username is required")

def test_superuser_cannot_change_password_of_non_existing_user(self):
"""Test a user with superuser access cannot change the password of a non existing user"""
response = self.client.put(
"/api/v1/password_change/",
{
"username": "foobar",
"old_password": "password",
"new_password": "password2",
},
)
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)

def test_superuser_cannot_change_password_of_others_with_invalid_old_password(
self,
):
"""Test a user with superuser access cannot change the password of other users with invalid old password"""
response = self.client.put(
"/api/v1/password_change/",
{
"username": self.data_2["username"],
"old_password": "wrong_password",
"new_password": "password2",
},
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(
response.data["old_password"][0],
"Wrong password entered. Please check your password.",
)


class TestUser(TestUtils, APITestCase):
def get_detail_representation(self, obj=None) -> dict:
Expand Down Expand Up @@ -309,7 +254,7 @@ def test_user_cannot_change_password_of_others(self):
"new_password": "password2",
},
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)

def test_user_with_districtadmin_access_can_modify_others(self):
"""Test a user with district admin access can modify others underneath the hierarchy"""
Expand Down

0 comments on commit 5b6397c

Please sign in to comment.