Skip to content

Commit

Permalink
Remove data-leaking endpoints
Browse files Browse the repository at this point in the history
- /api/v1/groups/members is unused?
- (API-)User-search is only used in a location we can remove.
  • Loading branch information
henrikhorluck committed Apr 10, 2024
1 parent ee8b179 commit 59ba0c7
Show file tree
Hide file tree
Showing 12 changed files with 5 additions and 402 deletions.
167 changes: 0 additions & 167 deletions apps/authentication/api/tests/group_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,170 +181,3 @@ def test_deputy_group_leader_can_delete_group(self):
response = self.client.delete(self.id_url(online_group.id))

self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)


class GroupMemberTestCase(APITestCase):
@staticmethod
def create_group_roles():
for role_type in RoleType.values:
GroupRole.objects.create(role_type=role_type)

@staticmethod
def get_group_role(role_type: str) -> GroupRole:
return GroupRole.objects.get(role_type=role_type)

def get_role(self, role: RoleType):
return GroupRole.get_for_type(role)

def _create_group(self, **kwargs) -> OnlineGroup:
leader_role = self.get_group_role(RoleType.LEADER)
deputy_leader_role = self.get_group_role(RoleType.DEPUTY_LEADER)
group: OnlineGroup = G(OnlineGroup, **kwargs)
group.admin_roles.add(leader_role)
group.admin_roles.add(deputy_leader_role)
return group

def setUp(self):
self.user: User = generate_user(username="test_user")
self.user.is_superuser = True
self.user.save()
self.user.refresh_from_db()
self.client.force_authenticate(user=self.user)
self.other_user: User = generate_user(username="other_user")

self.url = reverse("group_members-list")
self.id_url = lambda _id: self.url + str(_id) + "/"
self.create_group_roles()

self.group_name = "Noenkom"
self.group_name_long = "Noenkomiteen"
self.group = G(Group, name=self.group_name)
self.group_data = {
"group": self.group.id,
"name_short": self.group_name,
"name_long": self.group_name_long,
}
self.create_group = lambda: self._create_group(
group=self.group, name_short=self.group_name, name_long=self.group_name_long
)
self.online_group = self.create_group()

self.membership_data = {"group": self.online_group.id, "user": self.user.id}
self.create_membership = lambda: GroupMember.objects.create(
user=self.user, group=self.online_group
)

def test_group_members_returns_200(self):
response = self.client.get(self.url)

self.assertEqual(response.status_code, status.HTTP_200_OK)

def test_un_authenticated_user_gets_200(self):
self.client.force_authenticate(user=None)
response = self.client.get(self.url)

self.assertEqual(response.status_code, status.HTTP_200_OK)

def test_superuser_can_create_memberships(self):
self.user.save()
response = self.client.post(self.url, self.membership_data)

self.assertEqual(response.status_code, status.HTTP_201_CREATED)

def test_user_cannot_have_multiple_memberships_in_one_group(self):
self.create_membership()
response = self.client.post(self.url, self.membership_data)

self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(
response.json().get("non_field_errors"),
["Feltene user, group må gjøre et unikt sett."],
)

def test_regular_user_cannot_create_groups(self):
self.user.is_superuser = False
self.user.save()

response = self.client.post(self.url, self.membership_data)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)

def test_users_with_permission_can_create_memberships(self):
self.user.is_superuser = False
self.user.save()
permission = Permission.objects.get(codename="add_groupmember")
self.user.user_permissions.add(permission)

response = self.client.post(self.url, self.membership_data)

self.assertEqual(response.status_code, status.HTTP_201_CREATED)

def test_superuser_can_delete_memberships(self):
membership = self.create_membership()

response = self.client.delete(self.id_url(membership.id))

self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)

def test_regular_user_cannot_delete_memberships(self):
self.user.is_superuser = False
self.user.save()
membership = self.create_membership()

response = self.client.delete(self.id_url(membership.id))

self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)

def test_group_leader_can_delete_memberships(self):
membership = self.create_membership()
membership.roles.add(self.get_group_role(RoleType.LEADER))
self.user.is_superuser = False
self.user.save()

response = self.client.delete(self.id_url(membership.id))

self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)

def test_deputy_group_leader_can_delete_memberships(self):
membership = self.create_membership()
membership.roles.add(self.get_group_role(RoleType.LEADER))
self.user.is_superuser = False
self.user.save()

response = self.client.delete(self.id_url(membership.id))

self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)

def test_super_user_can_assign_roles_to_member(self):
self.user.is_superuser = True
self.user.save()

role_ids = [self.get_group_role(RoleType.TREASURER).id]
membership = self.create_membership()

response = self.client.patch(self.id_url(membership.id), {"roles": role_ids})

self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.json().get("roles"), role_ids)

def test_group_leader_can_assign_roles_to_member(self):
self.user.is_superuser = False
self.user.save()

membership = self.create_membership()
membership.roles.add(self.get_group_role(RoleType.LEADER).id)
role_id = self.get_group_role(RoleType.TREASURER).id

response = self.client.patch(self.id_url(membership.id), {"roles": [role_id]})

self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertIn(role_id, response.json().get("roles"))

def test_regular_user_cannot_assign_roles_to_member(self):
self.user.is_superuser = False
self.user.save()

membership = self.create_membership()

response = self.client.patch(self.id_url(membership.id))

self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
14 changes: 1 addition & 13 deletions apps/authentication/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@
from rest_framework.permissions import AllowAny, IsAuthenticated
from rest_framework.response import Response

from apps.authentication.models import GroupMember, GroupRole, OnlineGroup
from apps.authentication.models import GroupRole, OnlineGroup
from apps.authentication.models import OnlineUser as User
from apps.authentication.models import Position, SpecialPosition
from apps.authentication.serializers import (
AnonymizeUserSerializer,
GroupMemberCreateSerializer,
GroupMemberReadOnlySerializer,
GroupMemberUpdateSerializer,
GroupReadOnlySerializer,
GroupRoleReadOnlySerializer,
OnlineGroupCreateOrUpdateSerializer,
Expand Down Expand Up @@ -172,16 +170,6 @@ def group_users(self, request, pk: int = None):
return Response(data=serializer.data, status=status.HTTP_200_OK)


class GroupMemberViewSet(MultiSerializerMixin, viewsets.ModelViewSet):
permission_classes = (DjangoObjectPermissionOrAnonReadOnly,)
queryset = GroupMember.objects.all()
serializer_classes = {
"create": GroupMemberCreateSerializer,
"update": GroupMemberUpdateSerializer,
"read": GroupMemberReadOnlySerializer,
}


class GroupRoleViewSet(viewsets.ReadOnlyModelViewSet):
permission_classes = (AllowAny,)
serializer_class = GroupRoleReadOnlySerializer
Expand Down
17 changes: 0 additions & 17 deletions apps/authentication/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,23 +225,6 @@ class Meta:
read_only = True


class GroupMemberCreateSerializer(serializers.ModelSerializer):
user = serializers.PrimaryKeyRelatedField(
required=True, queryset=User.objects.all()
)
group = serializers.PrimaryKeyRelatedField(
required=True, queryset=OnlineGroup.objects.all()
)
roles = serializers.PrimaryKeyRelatedField(
required=False, queryset=GroupRole.objects.all(), many=True
)

class Meta:
model = GroupMember
fields = ("id", "user", "group", "added", "roles", "is_on_leave", "is_retired")
read_only_fields = ("added",)


class GroupMemberUpdateSerializer(serializers.ModelSerializer):
roles = serializers.PrimaryKeyRelatedField(
required=False, queryset=GroupRole.objects.all(), many=True
Expand Down
1 change: 0 additions & 1 deletion apps/authentication/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,4 @@
router.register(
"group/online-groups", api_views.OnlineGroupViewSet, basename="online_groups"
)
router.register("group/members", api_views.GroupMemberViewSet, basename="group_members")
router.register("group/roles", api_views.GroupRoleViewSet, basename="group_roles")
13 changes: 0 additions & 13 deletions apps/profiles/tests.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,12 @@
from django.test import TestCase
from django.urls import reverse
from django_dynamic_fixture import G
from rest_framework import status

from apps.authentication.models import OnlineUser as User
from apps.profiles.forms import ZIP_CODE_VALIDATION_ERROR, ProfileForm
from apps.profiles.models import Privacy


class ProfilesURLTestCase(TestCase):
def test_user_search(self):
user = G(User)
url = reverse("profiles_user_search")

self.client.force_login(user)

response = self.client.get(url)

self.assertEqual(response.status_code, status.HTTP_200_OK)


class ProfileInitTestCase(TestCase):
def test_privacy_profile_save(self):
user = G(User)
Expand Down
7 changes: 1 addition & 6 deletions apps/profiles/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,12 @@
views.update_mark_rules,
name="profile_update_mark_rules",
),
# Endpoint that exposes a json lump of all users but only id and name.
# Endpoint that exposes a json lump of all users but only id and name. Only used for autocomplete in the dashboard
re_path(
r"^api_plain_user_search/$",
views.api_plain_user_search,
name="profiles_api_plain_user_search",
),
# Endpoint that exposes a json lump of all users which have set their profile to public.
re_path(
"^api_user_search/$", views.api_user_search, name="profiles_api_user_search"
),
re_path(r"^user_search/$", views.user_search, name="profiles_user_search"),
# Profile index with active tab.
re_path(r"^(?P<active_tab>\w+)/$", views.index, name="profiles_active"),
]
Expand Down
41 changes: 3 additions & 38 deletions apps/profiles/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from django.contrib import messages
from django.contrib.auth.decorators import login_required
from django.contrib.auth.models import Group
from django.db.models import Q
from django.http import Http404, HttpResponse, JsonResponse
from django.shortcuts import get_object_or_404, redirect, render
from django.utils.translation import gettext as _
Expand All @@ -17,8 +16,6 @@

from apps.approval.forms import FieldOfStudyApplicationForm
from apps.approval.models import MembershipApproval
from apps.authentication.constants import GroupType
from apps.authentication.models import OnlineGroup
from apps.authentication.models import OnlineUser as User
from apps.authentication.models import Position
from apps.dashboard.tools import has_access
Expand Down Expand Up @@ -265,44 +262,12 @@ def toggle_jobmail(request):
raise Http404


@login_required
def user_search(request):
committee_groups = OnlineGroup.objects.filter(
Q(group_type=GroupType.COMMITTEE) | Q(group_type=GroupType.NODE_COMMITTEE)
)
groups_to_include = [online_group.group.pk for online_group in committee_groups]
groups = Group.objects.filter(pk__in=groups_to_include).order_by("name")
users_to_display = User.objects.filter(privacy__visible_for_other_users=True)

context = {"users": users_to_display, "groups": groups}
return render(request, "profiles/users.html", context)


@login_required
def api_user_search(request):
if request.GET.get("query"):
users = search_for_users(request.GET.get("query"))
return render_json(users)
return render_json(error="Mangler søkestreng")


def search_for_users(query, limit=10):
if not query:
return []

results = []

for result in watson.search(
query, models=(User.objects.filter(privacy__visible_for_other_users=True),)
):
results.append(result.object)

return results[:limit]


@login_required
def api_plain_user_search(request):
"""The difference between plain_user_search and the other is exposing only id and name."""
if not request.user.is_staff:
raise PermissionError()

if request.GET.get("query"):
users = search_for_plain_users(request.GET.get("query"))
return JsonResponse(users, safe=False)
Expand Down
16 changes: 0 additions & 16 deletions assets/common/typeahead/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,4 @@ export const plainUserTypeahead = (element, select) => {
});
};

export const userTypeahead = (element, select) => {
const userSearchTemplate = template(`
<div>
<img width="100%" src="<%= image %>" alt="" />
<span data-id="<%= id %>" class="user-meta"><h4><%= name %></h4>
</div>
`);

typeahead(element, {
template: userSearchTemplate,
url: `${Urls.profiles_api_user_search()}?query=%QUERY`,
select,
name: 'users',
});
};

export { typeahead as default };
5 changes: 0 additions & 5 deletions assets/profiles/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { loadCityFromZipCode } from 'common/utils/';
import DependentDocumentation from './dependent_fields';
import enableUserSearch from './userSearch';
import './profiles';
import './less/profiles.less';

Expand All @@ -10,8 +9,4 @@ if (zipCodeElement) {
loadCityFromZipCode(zipCodeElement, cityElement);
}

const userSearchElement = document.getElementById('user-search');
if (userSearchElement) {
enableUserSearch();
}
DependentDocumentation();
Loading

0 comments on commit 59ba0c7

Please sign in to comment.