diff --git a/.github/ISSUE_TEMPLATE/update-table.md b/.github/ISSUE_TEMPLATE/update-table.md index ecb3f44a..0768bccc 100644 --- a/.github/ISSUE_TEMPLATE/update-table.md +++ b/.github/ISSUE_TEMPLATE/update-table.md @@ -1,6 +1,6 @@ --- name: Update Table -about: Describe this issue template's purpose here. +about: Describe the purpose of the issue template here title: 'Update Table: [TABLE NAME]' labels: 'feature: update table, good first issue, milestone: missing, role: back end, size: 0.25pt, stakeholder: missing' diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 58a8d53a..e5fc5d99 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -32,7 +32,7 @@ repos: - id: fix-byte-order-marker - id: name-tests-test args: [--pytest-test-first] - + exclude: ^app/core/tests/utils/ # general quality checks - id: mixed-line-ending - id: trailing-whitespace diff --git a/app/constants.py b/app/constants.py index c6a66a80..3186fc0d 100644 --- a/app/constants.py +++ b/app/constants.py @@ -1,5 +1,5 @@ -global_admin = "globalAdmin" +admin_global = "adminGlobal" admin_project = "adminProject" practice_lead_project = "practiceLeadProject" member_project = "memberProject" -self_value = "self" +field_permissions_csv_file = "core/api/field_permissions.csv" diff --git a/app/core/api/field_permissions.csv b/app/core/api/field_permissions.csv new file mode 100644 index 00000000..b6dd2078 --- /dev/null +++ b/app/core/api/field_permissions.csv @@ -0,0 +1,54 @@ +table_name,field_name,get,patch,post +User,username,memberProject,,adminGlobal +User,is_active,,, +User,is_staff,,, +User,first_name,memberProject,adminBrigade,adminGlobal +User,last_name,memberProject,adminBrigade,adminGlobal +User,gmail,practiceLeadProject,adminBrigade,adminGlobal +User,preferred_email,practiceLeadProject,adminBrigade,adminGlobal +User,created_at,adminProject,,, +User,user_status_id,adminBrigade,adminBrigade,adminGlobal +User,current_job_title,adminBrigade,adminBrigade,adminGlobal +User,target_job_title,adminBrigade,adminBrigade,adminGlobal +User,current_skills,adminBrigade,adminBrigade,adminGlobal +User,target_skills,adminBrigade,adminBrigade,adminGlobal +User,linkedin_account,memberProject,adminBrigade,adminGlobal +User,github_handle,memberProject,adminBrigade,adminGlobal +User,phone,practiceLeadProject,adminBrigade,adminGlobal +User,texting_ok,practiceLeadProject,adminBrigade,adminGlobal +User,slack_id,memberProject,adminBrigade,adminGlobal +User,time_zone,memberProject,adminBrigade,adminGlobal +User,last_updated,adminBrigade,,adminGlobal +User,password,,adminBrigade,adminGlobal,,,, +UserPermission,field1,memberProject,adminProject,adminProject + +John Smith adminGlobal + +Wanda adminProject website-project +Wally memberProject website-project +Paul memberProjbect peopledepot-project + +If Wanda tries to read Wally, the highest privilege is adminProject for Wanda, gmail should be includded +If Wanda tries to read Paul, there is no project in common, the highest privilege is None + +UserPermissions +user + +blackBox(logged in user, self.user, "UserPermission") + + +blackBox(logged in user, target user, table) + +UserOpportunities +user +blackBox(logged in user, self.user, "UserPermission") + + +User =< UserEvent =< UserAttendance + +UserEvent +user + +UserAttendance +user userEvent.user +userEvent diff --git a/app/core/api/permission_validation.py b/app/core/api/permission_validation.py new file mode 100644 index 00000000..96c3b003 --- /dev/null +++ b/app/core/api/permission_validation.py @@ -0,0 +1,145 @@ +import csv +from pathlib import Path +from typing import Any + +from rest_framework.exceptions import PermissionDenied + +from constants import admin_global # Assuming you have this constant +from constants import field_permissions_csv_file +from core.models import PermissionType +from core.models import UserPermission + + +class PermissionValidation: + @staticmethod + def is_admin(user) -> bool: + """Check if a user has admin permissions.""" + permission_type = PermissionType.objects.filter(name=admin_global).first() + # return True + return UserPermission.objects.filter( + permission_type=permission_type, user=user + ).exists() + + @staticmethod + def get_rank_dict() -> dict[str, int]: + """Return a dictionary mapping permission names to their ranks.""" + permissions = PermissionType.objects.values("name", "rank") + return {perm["name"]: perm["rank"] for perm in permissions} + + @staticmethod + def get_csv_field_permissions() -> dict[str, dict[str, list[dict[str, Any]]]]: + """Read the field permissions from a CSV file.""" + file_path = Path(field_permissions_csv_file) + with file_path.open() as file: + reader = csv.DictReader(file) + return list(reader) + + @classmethod + def get_fields( + cls, operation: str, permission_type: str, table_name: str + ) -> list[str]: + """Return the valid fields for the given permission type.""" + + valid_fields = [] + if permission_type == "": + return valid_fields + for field in cls.get_csv_field_permissions(): + if cls.is_field_valid( + operation=operation, + permission_type=permission_type, + table_name=table_name, + field=field, + ): + valid_fields += [field["field_name"]] + return valid_fields + + @classmethod + def get_fields_for_post_request(cls, request, table_name): + requesting_user = request.user + if not cls.is_admin(requesting_user): + raise PermissionDenied("You do not have privilges to create.") + fields = cls.get_fields( + operation="post", + table_name=table_name, + permission_type=admin_global, + ) + return fields + + @classmethod + def get_fields_for_patch_request(cls, request, table_name, response_related_user): + requesting_user = request.user + requesting_user = request.user + most_privileged_perm_type = cls.get_most_privileged_perm_type( + requesting_user, response_related_user + ) + fields = cls.get_fields( + operation="patch", + table_name=table_name, + permission_type=most_privileged_perm_type, + ) + return fields + + @classmethod + def get_fields_for_response(cls, request, table_name, response_related_user): + requesting_user = request.user + most_privileged_perm_type = cls.get_most_privileged_perm_type( + requesting_user, response_related_user + ) + fields = cls.get_fields( + operation="get", + table_name=table_name, + permission_type=most_privileged_perm_type, + ) + return fields + + @classmethod + def get_most_privileged_perm_type( + cls, requesting_user, response_related_user + ) -> str: + """Return the most privileged permission type between users.""" + if cls.is_admin(requesting_user): + return admin_global + + target_projects = UserPermission.objects.filter( + user=response_related_user + ).values_list("project__name", flat=True) + target_projects = UserPermission.objects.filter( + user=response_related_user + ).values_list("project__name", flat=True) + + permissions = UserPermission.objects.filter( + user=requesting_user, project__name__in=target_projects + ).values("permission_type__name", "permission_type__rank") + + if not permissions: + return "" + + min_permission = min(permissions, key=lambda p: p["permission_type__rank"]) + return min_permission["permission_type__name"] + + @classmethod + def get_response_fields(cls, request, table_name, response_related_user) -> None: + """Ensure the requesting user can patch the provided fields.""" + requesting_user = request.user + most_privileged_perm_type = cls.get_most_privileged_perm_type( + requesting_user, response_related_user + ) + fields = cls.get_fields( + operation="get", + table_name=table_name, + permission_type=most_privileged_perm_type, + ) + return fields + + @classmethod + def is_field_valid( + cls, operation: str, permission_type: str, table_name: str, field: dict + ): + operation_permission_type = field[operation] + if operation_permission_type == "" or field["table_name"] != table_name: + return False + rank_dict = cls.get_rank_dict() + source_rank = rank_dict[permission_type] + source_rank = rank_dict[permission_type] + rank_match = source_rank <= rank_dict[operation_permission_type] + return rank_match diff --git a/app/core/api/permissions.py b/app/core/api/permissions.py index d0036045..bcfd8f73 100644 --- a/app/core/api/permissions.py +++ b/app/core/api/permissions.py @@ -1,9 +1,25 @@ from rest_framework.permissions import BasePermission +from core.api.user_related_request import UserRelatedRequest + class DenyAny(BasePermission): - def has_permission(self, request, view): + def has_permission(self, __request__, __view__): return False - def has_object_permission(self, request, view, obj): + def has_object_permission(self, __request__, __view__, __obj__): return False + + +class GenericPermission(BasePermission): + def has_permission(self, request, view): + if request.method == "POST": + UserRelatedRequest.validate_post_fields(request=request, view=view) + return True # Default to allow the request + + def has_object_permission(self, request, view, obj): + if request.method == "PATCH": + UserRelatedRequest.validate_patch_fields( + view=view, obj=obj, request=request + ) + return True diff --git a/app/core/api/serializers.py b/app/core/api/serializers.py index cc9cc42d..f0e7e312 100644 --- a/app/core/api/serializers.py +++ b/app/core/api/serializers.py @@ -1,6 +1,7 @@ from rest_framework import serializers from timezone_field.rest_framework import TimeZoneSerializerField +from core.api.user_related_request import UserRelatedRequest from core.models import Affiliate from core.models import Affiliation from core.models import CheckType @@ -68,6 +69,50 @@ class UserSerializer(serializers.ModelSerializer): time_zone = TimeZoneSerializerField(use_pytz=False) + def to_representation(self, instance): + representation = super().to_representation(instance) + return UserRelatedRequest.get_serializer_representation( + self, instance, representation + ) + + class Meta: + model = User + fields = ( + "uuid", + "username", + "created_at", + "updated_at", + "is_superuser", + "is_active", + "is_staff", + "email", + "first_name", + "last_name", + "gmail", + "preferred_email", + "current_job_title", + "target_job_title", + "current_skills", + "target_skills", + "linkedin_account", + "github_handle", + "slack_id", + "phone", + "texting_ok", + "time_zone", + ) + read_only_fields = ( + "uuid", + "created_at", + "updated_at", + "username", + "email", + ) + + +class UserProfileSerializer(serializers.ModelSerializer): + time_zone = TimeZoneSerializerField(use_pytz=False) + class Meta: model = User fields = ( @@ -75,6 +120,9 @@ class Meta: "username", "created_at", "updated_at", + "is_superuser", + "is_active", + "is_staff", "email", "first_name", "last_name", diff --git a/app/core/api/user_related_request.py b/app/core/api/user_related_request.py new file mode 100644 index 00000000..69128255 --- /dev/null +++ b/app/core/api/user_related_request.py @@ -0,0 +1,105 @@ +from rest_framework.exceptions import PermissionDenied +from rest_framework.exceptions import ValidationError + +from core.api.permission_validation import PermissionValidation +from core.models import User +from core.models import UserPermission + + +class UserRelatedRequest: + @staticmethod + def get_allowed_users(request): + current_username = request.user.username + + current_user = User.objects.get(username=current_username) + user_permissions = UserPermission.objects.filter(user=current_user) + + if PermissionValidation.is_admin(current_user): + allowed_users = User.objects.all() + else: + # Get the users with user permissions for the same projects + # that the requesting_user has permission to view + projects = [p.project for p in user_permissions if p.project is not None] + allowed_users = User.objects.filter( + permissions__project__in=projects + ).distinct() + return allowed_users + + @classmethod + def get_queryset(cls, view): + """Get the queryset of users that the requesting user has permission to view. + + Called from get_queryset in UserViewSet in views.py. + + Args: + request: the request object + + Returns: + queryset: the queryset of users that the requesting user has permission to view + """ + allowed_users = cls.get_allowed_users(view.request) + current_model = view.serializer_class.Meta.model + if current_model == User: + queryset = allowed_users + else: + queryset = current_model.objects.filter(user__in=allowed_users) + + return queryset + + @staticmethod + def get_serializer_representation(self, instance, original_representation): + request = self.context.get("request") + model_class = self.Meta.model + if model_class == User: + response_related_user: User = instance + else: + response_related_user = instance.user + # Get dynamic fields from some logic + user_fields = PermissionValidation.get_response_fields( + request=request, + table_name=model_class.__name__, + response_related_user=response_related_user, + ) + # Only retain the fields you want to include in the output + return { + key: value + for key, value in original_representation.items() + if key in user_fields + } + + @classmethod + def validate_post_fields(cls, view, request): + # todo + serializer_class = view.serializer_class + table_name = serializer_class.Meta.model.__name__ + valid_fields = PermissionValidation.get_fields_for_post_request( + request=request, table_name=table_name + ) + cls.validate_request_fields(request, valid_fields) + + @classmethod + def validate_patch_fields(cls, view, request, obj): + serializer_class = view.serializer_class + model_class = serializer_class.Meta.model + table_name = model_class.__name__ + if model_class == User: + response_related_user = obj + else: + response_related_user = obj.user + valid_fields = PermissionValidation.get_fields_for_patch_request( + table_name=table_name, + request=request, + response_related_user=response_related_user, + ) + cls.validate_request_fields(request, valid_fields) + + @staticmethod + def validate_request_fields(request, valid_fields) -> None: + """Ensure the requesting user can patch the provided fields.""" + request_data_keys = set(request.data) + disallowed_fields = request_data_keys - set(valid_fields) + + if not valid_fields: + raise PermissionDenied("You do not have privileges ") + elif disallowed_fields: + raise ValidationError(f"Invalid fields: {', '.join(disallowed_fields)}") diff --git a/app/core/api/views.py b/app/core/api/views.py index f537e7b8..fd73fa61 100644 --- a/app/core/api/views.py +++ b/app/core/api/views.py @@ -1,4 +1,3 @@ -from django.contrib.auth import get_user_model from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import OpenApiExample from drf_spectacular.utils import OpenApiParameter @@ -8,8 +7,13 @@ from rest_framework import viewsets from rest_framework.generics import GenericAPIView from rest_framework.mixins import RetrieveModelMixin +from rest_framework.mixins import UpdateModelMixin from rest_framework.permissions import IsAuthenticated from rest_framework.permissions import IsAuthenticatedOrReadOnly +from rest_framework.response import Response + +from core.api.permissions import GenericPermission +from core.api.user_related_request import UserRelatedRequest from ..models import Affiliate from ..models import Affiliation @@ -48,25 +52,62 @@ from .serializers import StackElementTypeSerializer from .serializers import UrlTypeSerializer from .serializers import UserPermissionSerializer +from .serializers import UserProfileSerializer from .serializers import UserSerializer from .serializers import UserStatusTypeSerializer -class UserProfileAPIView(RetrieveModelMixin, GenericAPIView): - serializer_class = UserSerializer +@extend_schema_view( + list=extend_schema( + summary="Your Profile", + description="Return your profile information", + parameters=[], + ), + retrieve=extend_schema(description="Fetch your user profile"), + partial_update=extend_schema(description="Update your profile"), +) +class UserProfileAPIView(RetrieveModelMixin, UpdateModelMixin, GenericAPIView): + serializer_class = UserProfileSerializer permission_classes = [IsAuthenticated] + http_method_names = ["get", "patch"] def get_object(self): - return self.request.user + """Returns the user profile fetched by get + + Returns: + User: The user profile + """ + obj = self.request.user + self.check_object_permissions(self.request, obj) + return obj def get(self, request, *args, **kwargs): """ # User Profile - Get profile of current logged in user. + Get profile for current logged in user. + + Returns: + User: The user profile """ return self.retrieve(request, *args, **kwargs) + def patch(self, request, *args, **kwargs): + """ + # Update User Profile + + Partially update profile for the current logged-in user. + + Returns: + User: The updated user profile + """ + user = self.get_object() + serializer = self.get_serializer(user, data=request.data, partial=True) + if serializer.is_valid(): + serializer.save() + return Response(serializer.data) + return Response(serializer.errors, status=400) + @extend_schema_view( list=extend_schema( @@ -106,10 +147,10 @@ def get(self, request, *args, **kwargs): retrieve=extend_schema(description="Return the given user"), destroy=extend_schema(description="Delete the given user"), update=extend_schema(description="Update the given user"), - partial_update=extend_schema(description="Partially update the given user"), + partial_update=extend_schema(description="Update the given user"), ) class UserViewSet(viewsets.ModelViewSet): - permission_classes = [IsAuthenticated] + permission_classes = [IsAuthenticated, GenericPermission] serializer_class = UserSerializer lookup_field = "uuid" @@ -117,7 +158,8 @@ def get_queryset(self): """ Optionally filter users by an 'email' and/or 'username' query paramerter in the URL """ - queryset = get_user_model().objects.all() + queryset = UserRelatedRequest.get_queryset(view=self) + email = self.request.query_params.get("email") if email is not None: queryset = queryset.filter(email=email) @@ -210,7 +252,7 @@ class AffiliateViewSet(viewsets.ModelViewSet): retrieve=extend_schema(description="Return the given FAQ"), destroy=extend_schema(description="Delete the given FAQ"), update=extend_schema(description="Update the given FAQ"), - partial_update=extend_schema(description="Partially update the given FAQ"), + partial_update=extend_schema(description="Partially patch the given FAQ"), ) class FaqViewSet(viewsets.ModelViewSet): queryset = Faq.objects.all() diff --git a/app/core/models.py b/app/core/models.py index 1fffb341..81efe7c2 100644 --- a/app/core/models.py +++ b/app/core/models.py @@ -331,7 +331,7 @@ def __str__(self): class StackElementType(AbstractBaseModel): """ - Stack element type used to update a shared data store across projects + Stack element type used to patch a shared data store across projects """ name = models.CharField(max_length=255) diff --git a/app/core/tests/conftest.py b/app/core/tests/conftest.py index 7d2b2889..6874b3d3 100644 --- a/app/core/tests/conftest.py +++ b/app/core/tests/conftest.py @@ -24,6 +24,25 @@ from ..models import User from ..models import UserPermission from ..models import UserStatusType +from .utils.load_data import load_data + + +def pytest_configure(config): # noqa: PT004 + config.addinivalue_line( + "markers", "load_user_data_required: run load_data if any tests marked" + ) + + +@pytest.fixture(scope="session", autouse=True) +def _load_data_once_for_specific_tests(request, django_db_setup, django_db_blocker): + # Check if any tests marked with 'load_data_required' are going to be run + if request.node.items: + for item in request.node.items: + if "load_user_data_required" in item.keywords: + with django_db_blocker.unblock(): + print("Running load_data before any test classes in marked files") + load_data() + break # Run only once before all the test files @pytest.fixture @@ -61,7 +80,8 @@ def user_permissions(): @pytest.fixture def user_permission_admin_project(): user = User.objects.create( - username="TestUser Admin Project", email="TestUserAdminProject@example.com" + username="TestUser Admin Project", + email="TestUserAdminProject@example.com", ) project = Project.objects.create(name="Test Project Admin Project") permission_type = PermissionType.objects.filter(name=admin_project).first() @@ -95,6 +115,7 @@ def user_permission_practice_lead_project(): @pytest.fixture def user(django_user_model): + print("Creating") return django_user_model.objects.create_user( username="TestUser", email="testuser@email.com", @@ -132,11 +153,20 @@ def event_pm(project): name="PM", project=project, must_attend=[ - {"practice_area": "Development", "permission_type": "practiceLeadProject"}, - {"practice_area": "Design", "permission_type": "practiceLeadJrProject"}, + { + "practice_area": "Development", + "permission_type": "practiceLeadProject", + }, + { + "practice_area": "Design", + "permission_type": "practiceLeadJrProject", + }, ], should_attend=[ - {"practice_area": "Development", "permission_type": "memberProject"}, + { + "practice_area": "Development", + "permission_type": "memberProject", + }, {"practice_area": "Design", "permission_type": "adminProject"}, ], could_attend=[{"practice_area": "Design", "permission_type": "memberGeneral"}], @@ -228,13 +258,8 @@ def stack_element(stack_element_type): @pytest.fixture def permission_type1(): - return PermissionType.objects.create(name="Test Permission Type", description="") - - -@pytest.fixture -def permission_type2(): return PermissionType.objects.create( - name="Test Permission Type", description="A permission type description" + name="Test Permission Type", description="", rank=1000 ) @@ -277,7 +302,10 @@ def affiliation3(project, affiliate): @pytest.fixture def affiliation4(project, affiliate): return Affiliation.objects.create( - is_sponsor=False, is_partner=False, project=project, affiliate=affiliate + is_sponsor=False, + is_partner=False, + project=project, + affiliate=affiliate, ) diff --git a/app/core/tests/test_api.py b/app/core/tests/test_api.py index 47ea463b..46194a93 100644 --- a/app/core/tests/test_api.py +++ b/app/core/tests/test_api.py @@ -3,7 +3,6 @@ from rest_framework import status from core.api.serializers import ProgramAreaSerializer -from core.api.serializers import UserSerializer from core.models import ProgramArea from core.models import UserPermission @@ -25,145 +24,12 @@ STACK_ELEMENT_URL = reverse("stack-element-list") PERMISSION_TYPE = reverse("permission-type-list") STACK_ELEMENT_TYPE_URL = reverse("stack-element-type-list") -SDGS_URL = reverse("sdg-list") +SDG_URL = reverse("sdg-list") AFFILIATION_URL = reverse("affiliation-list") CHECK_TYPE_URL = reverse("check-type-list") SOC_MAJOR_URL = reverse("soc-major-list") URL_TYPE_URL = reverse("url-type-list") - -CREATE_USER_PAYLOAD = { - "username": "TestUserAPI", - "password": "testpass", - # time_zone is required because django_timezone_field doesn't yet support - # the blank string - "time_zone": "America/Los_Angeles", -} - - -@pytest.fixture -def users_url(): - return reverse("user-list") - - -@pytest.fixture -def user_url(user): - return reverse("user-detail", args=[user.uuid]) - - -def create_user(django_user_model, **params): - return django_user_model.objects.create_user(**params) - - -def test_list_users_fail(client): - res = client.get(USERS_URL) - - assert res.status_code == status.HTTP_401_UNAUTHORIZED - - -def test_get_profile(auth_client): - res = auth_client.get(ME_URL) - - assert res.status_code == status.HTTP_200_OK - assert res.data["username"] == "TestUser" - - -def test_get_users(auth_client, django_user_model): - create_user(django_user_model, username="TestUser2", password="testpass") - create_user(django_user_model, username="TestUser3", password="testpass") - - res = auth_client.get(USERS_URL) - - assert res.status_code == status.HTTP_200_OK - assert len(res.data) == 3 - - users = django_user_model.objects.all().order_by("created_at") - serializer = UserSerializer(users, many=True) - assert res.data == serializer.data - - -def test_get_single_user(auth_client, user): - res = auth_client.get(f"{USERS_URL}?email={user.email}") - assert res.status_code == status.HTTP_200_OK - - res = auth_client.get(f"{USERS_URL}?username={user.username}") - assert res.status_code == status.HTTP_200_OK - - -user_actions_test_data = [ - ( - "admin_client", - "post", - "users_url", - CREATE_USER_PAYLOAD, - status.HTTP_201_CREATED, - ), - ("admin_client", "get", "users_url", {}, status.HTTP_200_OK), - ( - "auth_client", - "post", - "users_url", - CREATE_USER_PAYLOAD, - status.HTTP_201_CREATED, - ), - ("auth_client", "get", "users_url", {}, status.HTTP_200_OK), - ( - "auth_client", - "patch", - "user_url", - {"first_name": "TestUser2"}, - status.HTTP_200_OK, - ), - ( - "auth_client", - "put", - "user_url", - CREATE_USER_PAYLOAD, - status.HTTP_200_OK, - ), - ("auth_client", "delete", "user_url", {}, status.HTTP_204_NO_CONTENT), - ( - "admin_client", - "patch", - "user_url", - {"first_name": "TestUser2"}, - status.HTTP_200_OK, - ), - ( - "admin_client", - "put", - "user_url", - CREATE_USER_PAYLOAD, - status.HTTP_200_OK, - ), - ("admin_client", "delete", "user_url", {}, status.HTTP_204_NO_CONTENT), - ( - "auth_client2", - "patch", - "user_url", - {"first_name": "TestUser2"}, - status.HTTP_200_OK, - ), - ( - "auth_client2", - "put", - "user_url", - CREATE_USER_PAYLOAD, - status.HTTP_200_OK, - ), - ("auth_client2", "delete", "user_url", {}, status.HTTP_204_NO_CONTENT), -] - - -@pytest.mark.parametrize( - ("client_name", "action", "endpoint", "payload", "expected_status"), - user_actions_test_data, -) -def test_user_actions(client_name, action, endpoint, payload, expected_status, request): - client = request.getfixturevalue(client_name) - action_fn = getattr(client, action) - url = request.getfixturevalue(endpoint) - res = action_fn(url, payload) - assert res.status_code == expected_status +USER_STATUS_TYPES_URL = reverse("user-status-type-list") def test_create_event(auth_client, project): @@ -329,7 +195,7 @@ def test_create_stack_element_type(auth_client): assert res.data["name"] == payload["name"] -def test_get_user_permissions(user_superuser_admin, user_permissions, auth_client): +def test_get_user_permissions(user_superuser_admin, auth_client): auth_client.force_authenticate(user=user_superuser_admin) permission_count = UserPermission.objects.count() res = auth_client.get(USER_PERMISSIONS_URL) @@ -343,7 +209,7 @@ def test_create_sdg(auth_client): "description": "Test SDG description", "image": "https://unsplash.com", } - res = auth_client.post(SDGS_URL, payload) + res = auth_client.post(SDG_URL, payload) assert res.status_code == status.HTTP_201_CREATED assert res.data["name"] == payload["name"] @@ -402,7 +268,7 @@ def get_object(objects, target_uuid): assert sdg.name in test_proj["sdgs"] assert sdg1.name in test_proj["sdgs"] - sdg_res = auth_client.get(SDGS_URL) + sdg_res = auth_client.get(SDG_URL) test_sdg = get_object(sdg_res.data, sdg.uuid) assert test_sdg is not None assert len(test_sdg["projects"]) == 1 diff --git a/app/core/tests/test_get_users.py b/app/core/tests/test_get_users.py new file mode 100644 index 00000000..f8b8fc5f --- /dev/null +++ b/app/core/tests/test_get_users.py @@ -0,0 +1,84 @@ +import pytest +from django.urls import reverse +from rest_framework.test import APIClient + +from constants import admin_project +from constants import member_project +from core.api.permission_validation import PermissionValidation +from core.tests.utils.seed_constants import valerie_name +from core.tests.utils.seed_constants import wally_name +from core.tests.utils.seed_constants import wanda_admin_project +from core.tests.utils.seed_constants import winona_name +from core.tests.utils.seed_user import SeedUser + +count_website_members = 5 +count_people_depot_members = 3 +count_members_either = 6 + +_user_get_url = reverse("user-list") + + +@pytest.mark.django_db +@pytest.mark.load_user_data_required # see load_user_data_required in conftest.py +class TestGetUser: + @staticmethod + def _get_response_fields(first_name, response_data): + response_related_user = None + + # look up target user in response_data by first name + for user in response_data: + if user["first_name"] == first_name: + response_related_user = user + break + + # Throw error if target user not found + if response_related_user is None: + raise ValueError( + "Test set up mistake. No user with first name of ${first_name}" + ) + + # Otherwise check if user fields in response data are the same as fields + return set(user) + + def test_get_url_results_for_admin_project(self): + """Test that the get user request returns (a) all users on the website project + and (b) the fields match fields configured for a project admin + **WHEN** the requesting_user is a project admin. + """ + client = APIClient() + client.force_authenticate(user=SeedUser.get_user(wanda_admin_project)) + response = client.get(_user_get_url) + assert response.status_code == 200 + assert len(response.json()) == count_website_members + response_fields = self._get_response_fields(winona_name, response.data) + valid_fields = PermissionValidation.get_fields( + operation="get", permission_type=admin_project, table_name="User" + ) + assert response_fields == set(valid_fields) + + def test_get_results_for_users_on_same_team(self): + """Test that get user request (a) returns users on the website project + and (b) the fields returned match the configured fields for + the team member permission type **WHEN** the requuster is a team member + of the web site project. + """ + client = APIClient() + client.force_authenticate(user=SeedUser.get_user(wally_name)) + response = client.get(_user_get_url) + + assert response.status_code == 200 + assert len(response.json()) == count_website_members + response_fields = self._get_response_fields(winona_name, response.data) + valid_fields = PermissionValidation.get_fields( + operation="get", permission_type=member_project, table_name="User" + ) + assert response_fields == set(valid_fields) + assert len(response.json()) == count_website_members + + def test_no_user_permission(self): + """Test that get user request returns no data when requesting_user has no permissions.""" + client = APIClient() + client.force_authenticate(user=SeedUser.get_user(valerie_name)) + response = client.get(_user_get_url) + assert response.status_code == 200 + assert len(response.json()) == 0 diff --git a/app/core/tests/test_models.py b/app/core/tests/test_models.py index 9de26469..e0ab534b 100644 --- a/app/core/tests/test_models.py +++ b/app/core/tests/test_models.py @@ -9,13 +9,6 @@ pytestmark = pytest.mark.django_db -def test_user(user, django_user_model): - assert django_user_model.objects.filter(is_staff=False).count() == 1 - assert str(user) == "testuser@email.com" - assert user.is_django_user is True - assert repr(user) == f"" - - def test_project(project): assert str(project) == "Test Project" @@ -72,14 +65,6 @@ def test_permission_type1(permission_type1): assert str(permission_type1) == "Test Permission Type" -def test_permission_type2(permission_type2): - assert str(permission_type2.name) == "Test Permission Type" - assert str(permission_type2.description) == "A permission type description" - assert ( - str(permission_type2) == "Test Permission Type: A permission type description" - ) - - def test_stack_element_type(stack_element_type): assert str(stack_element_type) == "Test Stack Element Type" @@ -130,7 +115,7 @@ def test_affiliation_partner_and_sponsor(affiliation3): assert ( str(xref_instance) == f"Sponsor {xref_instance.project} and Partner {xref_instance.affiliate}" - ) + ) # noqa def test_affiliation_is_neither_partner_and_sponsor(affiliation4): diff --git a/app/core/tests/test_patch_users.py b/app/core/tests/test_patch_users.py new file mode 100644 index 00000000..cee953b3 --- /dev/null +++ b/app/core/tests/test_patch_users.py @@ -0,0 +1,127 @@ +from unittest.mock import patch + +import pytest +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient + +from core.api.user_related_request import UserRelatedRequest +from core.tests.utils.seed_constants import garry_name +from core.tests.utils.seed_constants import valerie_name +from core.tests.utils.seed_constants import wanda_admin_project +from core.tests.utils.seed_user import SeedUser + + +@pytest.mark.django_db +@pytest.mark.load_user_data_required # see load_user_data_required in conftest.py +class TestPatchUser: + # @staticmethod + # def _patch_request_to_viewset(requesting_user, patch_data): + # new_data = patch_data.copy() + # factory = APIRequestFactory() + # request = factory.patch(reverse("user-detail"), data=new_data, format="json") + # force_authenticate(request, user=requesting_user) + # view = serViewSet.as_view({"patch": "partial_update"}) + # response = view(request) + # return response + @staticmethod + def _call_api(requesting_user_name, response_related_name, data): + requester = SeedUser.get_user(requesting_user_name) + client = APIClient() + client.force_authenticate(user=requester) + + response_related_user = SeedUser.get_user(response_related_name) + url = reverse("user-detail", args=[response_related_user.uuid]) + data = data + return client.patch(url, data, format="json") + + @patch.object(UserRelatedRequest, UserRelatedRequest.validate_patch_fields.__name__) + def test_patch_request_calls_validate_request(self, mock_validate_fields): + """Test that the patch requests succeeds when the requester is an admin.""" + requester = SeedUser.get_user(garry_name) + client = APIClient() + client.force_authenticate(user=requester) + + response_related_user = SeedUser.get_user(valerie_name) + url = reverse("user-detail", args=[response_related_user.uuid]) + data = { + "last_name": "Updated", + "gmail": "update@example.com", + } + client.patch(url, data, format="json") + __args__, kwargs = mock_validate_fields.call_args + request_received = kwargs.get("request") + response_related_user_received = kwargs.get("obj") + assert request_received.data == data + assert request_received.user == requester + assert response_related_user_received == response_related_user + + @classmethod + def test_valid_patch(cls): + patch_data = { + "last_name": "Foo", + # "gmail": "smith@example.com", + # "first_name": "John", + } + response = cls._call_api( + requesting_user_name=garry_name, + response_related_name=wanda_admin_project, + data=patch_data, + ) + response = cls._call_api( + requesting_user_name=garry_name, + response_related_name=wanda_admin_project, + data=patch_data, + ) + assert response.status_code == status.HTTP_200_OK + + @classmethod + def test_patch_with_not_allowed_fields(cls): + """Test patch request returns 400 response when request fields do not match configured fields. + + Fields are configured to not include last_name. The test will attempt to create a user + with last_name in the request data. The test should fail with a 400 status code. + + See documentation for test_allowable_patch_fields_configurable for more information. + """ + + patch_data = { + "gmail": "smith@example.com", + "created_at": "2022-01-01T00:00:00Z", + } + response = cls._call_api( + requesting_user_name=garry_name, + response_related_name=wanda_admin_project, + data=patch_data, + ) + response = cls._call_api( + requesting_user_name=garry_name, + response_related_name=wanda_admin_project, + data=patch_data, + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @classmethod + def test_patch_with_unprivileged_requesting_user(cls): + """Test patch request returns 400 response when request fields do not match configured fields. + + Fields are configured to not include last_name. The test will attempt to create a user + with last_name in the request data. The test should fail with a 400 status code. + + See documentation for test_allowable_patch_fields_configurable for more information. + """ + + patch_data = { + "gmail": "smith@example.com", + } + response = cls._call_api( + requesting_user_name=wanda_admin_project, + response_related_name=valerie_name, + data=patch_data, + ) + response = cls._call_api( + requesting_user_name=wanda_admin_project, + response_related_name=valerie_name, + data=patch_data, + ) + assert response.status_code == status.HTTP_404_NOT_FOUND diff --git a/app/core/tests/test_permission_check.py b/app/core/tests/test_permission_check.py new file mode 100644 index 00000000..4a9e200a --- /dev/null +++ b/app/core/tests/test_permission_check.py @@ -0,0 +1,220 @@ +from unittest.mock import mock_open +from unittest.mock import patch + +import pytest +from rest_framework.exceptions import PermissionDenied +from rest_framework.exceptions import ValidationError + +from constants import admin_global +from constants import admin_project +from constants import member_project +from constants import practice_lead_project +from core.api.permission_validation import PermissionValidation +from core.api.user_related_request import UserRelatedRequest +from core.api.views import UserViewSet +from core.tests.utils.seed_constants import garry_name +from core.tests.utils.seed_constants import patti_name +from core.tests.utils.seed_constants import wally_name +from core.tests.utils.seed_constants import wanda_admin_project +from core.tests.utils.seed_constants import zani_name +from core.tests.utils.seed_user import SeedUser + +keys = ["table_name", "field_name", "get", "patch", "post"] +rows = [ + ["User", "field1", member_project, practice_lead_project, admin_global], + ["User", "field2", admin_project, admin_project, admin_global], + ["User", "field3", admin_project, admin_global, admin_global], + ["User", "system_field", member_project, "", ""], + ["foo", "bar", member_project, member_project, member_project], +] +# Create an array of dictionaries with keys specified by keys[] andsss +# values for each row specified by rows +mock_data = [dict(zip(keys, row)) for row in rows] + + +class MockSimplifiedRequest: + def __init__(self, user, data, method): + self.user = user + self.data = data + self.method = method + + +@pytest.fixture +def mock_csv_data(): + """Fixture to provide mock CSV field permissions.""" + return [ + { + "operation": "update", + "table_name": "user", + "field_name": "email", + "view": "viewer", + "update": "moderator", + "create": admin_global, + }, + { + "operation": "create", + "table_name": "user", + "field_name": "name", + "view": "viewer", + "update": "moderator", + "create": admin_global, + }, + ] + + +# Beginner Tip: +# Mocking means creating a "fake" version of a function that behaves how you want for testing purposes. +# This allows us to test code without relying on external resources like databases. +@patch("builtins.open", new_callable=mock_open) +@patch("csv.DictReader") +def test_csv_field_permissions(mock_dict_reader, _, mock_csv_data): # noqa: PT019 + """Test that get_csv_field_permissions returns the correct parsed data.""" + mock_dict_reader.return_value = mock_csv_data + + result = PermissionValidation.get_csv_field_permissions() + assert result == mock_csv_data + + +@pytest.mark.django_db +@pytest.mark.load_user_data_required # see load_user_data_required in conftest.py +def test_is_admin(): + """Test that is_admin returns True for an admin user.""" + admin_user = SeedUser.get_user(garry_name) + + assert PermissionValidation.is_admin(admin_user) is True + + +@pytest.mark.django_db +@pytest.mark.load_user_data_required # see load_user_data_required in conftest.py +def test_is_not_admin(): + """Test that is_admin returns True for an admin user.""" + admin_user = SeedUser.get_user(wanda_admin_project) + assert PermissionValidation.is_admin(admin_user) is False + + +@pytest.mark.parametrize( # noqa: PT006 PT007 + "request_user_name, response_related_user_name, expected_permission_type", + ( + # Wanda is an admin project for website, Wally is on the same project => admin_project + (wanda_admin_project, wally_name, admin_project), + # Wally is a project member for website, Wanda is on the same project => member_project + (wally_name, wanda_admin_project, member_project), + # Garry is both a project admin for website and a global admin => admin_global + (garry_name, wally_name, admin_global), + # Wally is a project member of website and Garry is a project lead on the same team + # => member_project + (wally_name, garry_name, member_project), + # Garry is a global admin. Even though Patti is not assigned to same team => admin_global + (garry_name, patti_name, admin_global), + # Patti has no project in common with Garry => "" + (patti_name, wally_name, ""), + # Zani is part of two projects with different permission types + # Zani is a member_project for website, Wally is assigned same team => member_project + (zani_name, wally_name, member_project), + # Zani is a project admin for website, Wally is assigned same team => admin_project + (zani_name, patti_name, admin_project), + ), +) +@pytest.mark.django_db +@pytest.mark.load_user_data_required # see load_user_data_required in conftest.py +def test_get_most_privileged_perm_type( + request_user_name, response_related_user_name, expected_permission_type +): + """Test that the correct permission type is returned.""" + request_user = SeedUser.get_user(request_user_name) + response_related_user = SeedUser.get_user(response_related_user_name) + assert ( + PermissionValidation.get_most_privileged_perm_type( + request_user, response_related_user + ) + == expected_permission_type + ) + + +@pytest.mark.django_db +@pytest.mark.load_user_data_required +@patch.object(PermissionValidation, "get_csv_field_permissions", return_value=mock_data) +def test_patch_with_valid_fields(_): # noqa: PT019 + """Test that validate_user_fields_patchable does not raise an error for valid fields.""" + + # Create a PATCH request with a JSON payload + patch_data = {"field1": "foo", "field2": "bar"} + mock_simplified_request = MockSimplifiedRequest( + method="PATCH", user=SeedUser.get_user(wanda_admin_project), data=patch_data + ) + + UserRelatedRequest.validate_patch_fields( + view=UserViewSet, + obj=SeedUser.get_user(wally_name), + request=mock_simplified_request, + ) + assert True + + +@pytest.mark.django_db +@pytest.mark.load_user_data_required +@patch.object(PermissionValidation, "get_csv_field_permissions", return_value=mock_data) +def test_patch_with_invalid_fields(_): # noqa: PT019 + """Test that validate_user_fields_patchable raises a ValidationError for invalid fields.""" + patch_data = {"field1": "foo", "field2": "bar", "field3": "not valid for patch"} + mock_simplified_request = MockSimplifiedRequest( + method="PATCH", user=SeedUser.get_user(wanda_admin_project), data=patch_data + ) + + with pytest.raises(ValidationError): + UserRelatedRequest.validate_patch_fields( + obj=SeedUser.get_user(wanda_admin_project), + view=UserViewSet, + request=mock_simplified_request, + ) + + +@pytest.mark.django_db +@patch.object(PermissionValidation, "get_csv_field_permissions", return_value=mock_data) +def test_patch_fields_no_privileges(_): # noqa: PT019 + """Test that validate_user_fields_patchable raises a PermissionError when no privileges exist.""" + patch_data = {"field1": "foo"} + mock_simplified_request = MockSimplifiedRequest( + method="PATCH", user=SeedUser.get_user(wally_name), data=patch_data + ) + + with pytest.raises(PermissionDenied): + UserRelatedRequest.validate_patch_fields( + obj=SeedUser.get_user(wally_name), + view=UserViewSet, + request=mock_simplified_request, + ) + + +@pytest.mark.django_db +@pytest.mark.load_user_data_required +@patch.object(PermissionValidation, "get_csv_field_permissions", return_value=mock_data) +def test_post_with_valid_fields(_): # noqa: PT019 + """Test that validate_user_fields_patchable does not raise an error for valid fields.""" + + # Create a POST request with a JSON payload + post_data = {"field1": "foo", "field2": "bar"} + mock_simplified_request = MockSimplifiedRequest( + method="POST", user=SeedUser.get_user(garry_name), data=post_data + ) + + UserRelatedRequest.validate_post_fields( + request=mock_simplified_request, view=UserViewSet + ) + assert True + + +@pytest.mark.django_db +@pytest.mark.load_user_data_required +@patch.object(PermissionValidation, "get_csv_field_permissions", return_value=mock_data) +def test_post_with_invalid_fields(_): # noqa: PT019 + """Test that validate_user_fields_patchable raises a ValidationError for invalid fields.""" + post_data = {"field1": "foo", "field2": "bar", "system_field": "not valid for post"} + mock_simplified_request = MockSimplifiedRequest( + method="POST", user=SeedUser.get_user(garry_name), data=post_data + ) + + with pytest.raises(ValidationError): + UserRelatedRequest.validate_post_fields( + request=mock_simplified_request, view=UserViewSet + ) diff --git a/app/core/tests/test_post_users.py b/app/core/tests/test_post_users.py new file mode 100644 index 00000000..27367605 --- /dev/null +++ b/app/core/tests/test_post_users.py @@ -0,0 +1,97 @@ +import pytest +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIRequestFactory +from rest_framework.test import force_authenticate + +from core.api.views import UserViewSet +from core.tests.utils.seed_constants import garry_name +from core.tests.utils.seed_constants import wanda_admin_project +from core.tests.utils.seed_user import SeedUser + +count_website_members = 4 +count_people_depot_members = 3 +count_members_either = 6 + + +@pytest.mark.django_db +@pytest.mark.load_user_data_required # see load_user_data_required in conftest.py +class TestPostUser: + @staticmethod + def _post_request_to_viewset(requesting_user, create_data): + new_data = create_data.copy() + factory = APIRequestFactory() + request = factory.post(reverse("user-list"), data=new_data, format="json") + force_authenticate(request, user=requesting_user) + view = UserViewSet.as_view({"post": "create"}) + response = view(request) + return response + + @classmethod + def test_valid_post(cls): + """Test POST request returns success when the request fields match configured fields. + + This test mocks a PATCH request to skip submitting the request to the server and instead + calls the view directly with the request. This is done so that variables used by the + server can be set to test values. + """ + requesting_user = SeedUser.get_user(garry_name) # project lead for website + + create_data = { + "username": "foo", + "last_name": "Smith", + "first_name": "John", + "gmail": "smith@example.com", + "time_zone": "America/Los_Angeles", + "password": "password", + } + response = cls._post_request_to_viewset(requesting_user, create_data) + print(r"Debug", response.data) + + assert response.status_code == status.HTTP_201_CREATED + + def test_post_with_not_allowed_fields(self): + """Test post request returns 400 response when request fields do not match configured fields. + + Fields are configured to not include last_name. The test will attempt to create a user + with last_name in the request data. The test should fail with a 400 status code. + + See documentation for test_allowable_patch_fields_configurable for more information. + """ + + requesting_user = SeedUser.get_user(garry_name) # project lead for website + post_data = { + "username": "foo", + "first_name": "Mary", + "last_name": "Smith", + "gmail": "smith@example.com", + "time_zone": "America/Los_Angeles", + "password": "password", + "created_at": "2022-01-01T00:00:00Z", + } + response = TestPostUser._post_request_to_viewset(requesting_user, post_data) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_post_with_unprivileged_requesting_user(self): + """Test post request returns 400 response when request fields do not match configured fields. + + Fields are configured to not include last_name. The test will attempt to create a user + with last_name in the request data. The test should fail with a 400 status code. + + See documentation for test_allowable_patch_fields_configurable for more information. + """ + + requesting_user = SeedUser.get_user( + wanda_admin_project + ) # project lead for website + post_data = { + "username": "foo", + "first_name": "Mary", + "last_name": "Smith", + "gmail": "smith@example.com", + "time_zone": "America/Los_Angeles", + "password": "password", + "created_at": "2022-01-01T00:00:00Z", + } + response = TestPostUser._post_request_to_viewset(requesting_user, post_data) + assert response.status_code == status.HTTP_403_FORBIDDEN diff --git a/app/core/tests/utils/load_data.py b/app/core/tests/utils/load_data.py new file mode 100644 index 00000000..59ecf04b --- /dev/null +++ b/app/core/tests/utils/load_data.py @@ -0,0 +1,108 @@ +import copy + +from constants import admin_global +from constants import admin_project +from constants import member_project +from constants import practice_lead_project +from core.models import Project +from core.tests.utils.seed_constants import garry_name +from core.tests.utils.seed_constants import patrick_practice_lead +from core.tests.utils.seed_constants import patti_name +from core.tests.utils.seed_constants import people_depot_project +from core.tests.utils.seed_constants import valerie_name +from core.tests.utils.seed_constants import wally_name +from core.tests.utils.seed_constants import wanda_admin_project +from core.tests.utils.seed_constants import website_project_name +from core.tests.utils.seed_constants import winona_name +from core.tests.utils.seed_constants import zani_name +from core.tests.utils.seed_user import SeedUser + + +def load_data(): + """Populalates projects, users, and userpermissions with seed data + that is used by the tests in the core app. + + Called from django_db_setup which is automatcallly called by pytest-django + before any test is executed. + + Creates website_project and people_depot projects. Populates users + as follows: + - Wanda is the project lead for the website project + - Wally and Winona are members of the website project + - Patti is a member of the People Depot project + - Patrick is the project lead for the People Depot project + + - Garry is a global admin + - Zani is a member of the website project and the project lead for the People Depot project + - Valerie is a verified user with no UserPermission assignments. + """ + projects = [website_project_name, people_depot_project] + for project_name in projects: + project = Project.objects.create(name=project_name) + project.save() + SeedUser.create_user( + first_name=wanda_admin_project, description="Website project admin" + ) + SeedUser.create_user(first_name=wally_name, description="Website member") + SeedUser.create_user(first_name=winona_name, description="Website member") + SeedUser.create_user( + first_name=zani_name, + description="Website member and People Depot project admin", + ) + SeedUser.create_user(first_name=patti_name, description="People Depot member") + SeedUser.create_user( + first_name=patrick_practice_lead, description="People Depot project admin" + ) + SeedUser.create_user(first_name=garry_name, description="Global admin") + SeedUser.get_user(garry_name).save() + SeedUser.create_user(first_name=valerie_name, description="Verified user") + + related_data = [ + {"first_name": garry_name, "permission_type_name": admin_global}, + { + "first_name": garry_name, + "project_name": website_project_name, + "permission_type_name": admin_project, + }, + { + "first_name": wanda_admin_project, + "project_name": website_project_name, + "permission_type_name": admin_project, + }, + { + "first_name": wally_name, + "project_name": website_project_name, + "permission_type_name": member_project, + }, + { + "first_name": winona_name, + "project_name": website_project_name, + "permission_type_name": member_project, + }, + { + "first_name": patti_name, + "project_name": people_depot_project, + "permission_type_name": member_project, + }, + { + "first_name": patrick_practice_lead, + "project_name": people_depot_project, + "permission_type_name": practice_lead_project, + }, + { + "first_name": zani_name, + "project_name": people_depot_project, + "permission_type_name": admin_project, + }, + { + "first_name": zani_name, + "project_name": website_project_name, + "permission_type_name": member_project, + }, + ] + + for data in related_data: + user = SeedUser.get_user(data["first_name"]) + params = copy.deepcopy(data) + del params["first_name"] + SeedUser.create_related_data(user=user, **params) diff --git a/app/core/tests/utils/seed_constants.py b/app/core/tests/utils/seed_constants.py new file mode 100644 index 00000000..d46083cb --- /dev/null +++ b/app/core/tests/utils/seed_constants.py @@ -0,0 +1,23 @@ +wanda_admin_project = "Wanda" +wally_name = "Wally" +winona_name = "Winona" +zani_name = "Zani" +patti_name = "Patti" +patrick_practice_lead = "Patrick" +valerie_name = "Valerie" +garry_name = "Garry" + +descriptions = { + wally_name: "Website member", + wanda_admin_project: "Website project admin", + winona_name: "Website member", + zani_name: "Website member and People Depot project admin", + patti_name: "People Depot member", + patrick_practice_lead: "People Depot project lead", + valerie_name: "Verified user, no project", + garry_name: "Global admin", +} + +website_project_name = "Website" +people_depot_project = "People Depot" +password = "Hello2024" diff --git a/app/core/tests/utils/seed_user.py b/app/core/tests/utils/seed_user.py new file mode 100644 index 00000000..48d64401 --- /dev/null +++ b/app/core/tests/utils/seed_user.py @@ -0,0 +1,66 @@ +from core.models import PermissionType +from core.models import Project +from core.models import User +from core.models import UserPermission +from core.tests.utils.seed_constants import password + + +class SeedUser: + """Summary + Attributes: + seed_users_list (dict): Populated by the create_user method. + Used to store the users created by the SeedUser.create_user. + Users are retrieved by first name. The code uses constants + when creating and getting seed users. + """ + + seed_users_list = {} + + def __init__(self, first_name, last_name): + self.first_name = first_name + self.last_name = last_name + self.user_name = f"{first_name}{last_name}@example.com" + self.email = self.user_name + self.user = SeedUser.create_user(first_name=first_name, description=last_name) + self.seed_users_list[first_name] = self.user + + @classmethod + def create_user(cls, *, first_name, description=None): + """Creates a user with the given first_name and description and + stores the user in the seed_users_list dictionary. + """ + last_name = f"{description}" + email = f"{first_name}@example.com" + username = first_name + + user = User.objects.create( + username=username, + first_name=first_name, + last_name=last_name, + email=email, + is_active=True, + ) + user.set_password(password) + cls.seed_users_list[first_name] = user + user.save() + return user + + @classmethod + def create_related_data(cls, *, user, permission_type_name, project_name=None): + permission_type = PermissionType.objects.get(name=permission_type_name) + if project_name: + project_data = {"project": Project.objects.get(name=project_name)} + else: + project_data = {} + user_permission = UserPermission.objects.create( + user=user, permission_type=permission_type, **project_data + ) + user_permission.save() + return user_permission + + @classmethod + def get_user(cls, first_name): + """Looks up user info from seed_users_list dictionary. + For more info, see notes on seed_users_list in the class docstring. + """ + return cls.seed_users_list.get(first_name) diff --git a/app/peopledepot/settings.py b/app/peopledepot/settings.py index db019b76..4f2310c6 100644 --- a/app/peopledepot/settings.py +++ b/app/peopledepot/settings.py @@ -61,6 +61,7 @@ # Application definition INSTALLED_APPS = [ + "core.tests", "django.contrib.admin", "django.contrib.auth", "django.contrib.contenttypes", diff --git a/app/scripts/pydoc-generate.py b/app/scripts/pydoc-generate.py new file mode 100644 index 00000000..05c556d3 --- /dev/null +++ b/app/scripts/pydoc-generate.py @@ -0,0 +1,90 @@ +import os +import pydoc +from pathlib import Path + +import django + +os.environ.setdefault("DJANGO_SETTINGS_MODULE", "peopledepot.settings") +django.setup() +excluded_dirs = {"venv", "__pycache__", "migrations"} +excluded_files = {"settings.py", "wsgi.py", "asgi.py", "manage.py"} + + +def has_docstring(file_path): + with Path.open(file_path, encoding="utf-8") as file: + for line in file: + if '"""' in line or "'''" in line: + return True + return False + + +def is_dir_excluded(dirname): + for excluded_dir in excluded_dirs: + if excluded_dir in dirname: + return False + return True + + +def get_dirs(): + root_dir = Path.cwd() + dir_names = [] + for dirpath, __dirnames__, __filenames__ in os.walk(root_dir): + if not is_dir_excluded(dirpath): + continue + relative_dir = os.path.relpath(dirpath, root_dir) + dir_names.append(relative_dir) + return dir_names + + +def is_file_included(filename): + for exclude_file in excluded_files: + if exclude_file in filename: + return False + return True + + +def get_files_in_directory(directory): + files_in_dir = [] + for filename in os.listdir(directory): + if not filename.endswith(".py"): + continue + + if is_file_included(filename): + files_in_dir.append( + Path(directory, filename) + ) # Path.join(directory, filename) + return files_in_dir + + +def generate_pydoc(): # noqa: C901 + # Get directories to scan + dirs = get_dirs() + + # Get all files within each directory + files = [] + for dirname in dirs: + files.extend(get_files_in_directory(dirname)) + + # Print files being processed + + # Generate documentation for each file with a docstring + for file_spec in files: + if not has_docstring(file_spec): + print(f"Skipping {file_spec} as it does not have a docstring.") + continue + + # Convert file path to module name + file_spec_str = str(file_spec) + module_name = file_spec_str[:-3] + module_name = module_name.replace(os.sep, ".") + + try: + print(f"Generating documentation for {module_name}...") + pydoc.writedoc(module_name) + except Exception as e: + print(f"Failed to generate documentation for {module_name}: {e}") + + +if __name__ == "__main__": + generate_pydoc() + print("Pydoc generation complete.") diff --git a/app/setup.cfg b/app/setup.cfg index ef800bc1..1a11c666 100644 --- a/app/setup.cfg +++ b/app/setup.cfg @@ -1,8 +1,7 @@ [flake8] max-line-length = 119 -exclude = migrations +exclude = '^(app/core/migrations/.*)$' max-complexity = 4 - [isort] profile = black skip_glob = */migrations/*.py diff --git a/docs/architecture/technical-details-of-permission-for-user-fields.md b/docs/architecture/technical-details-of-permission-for-user-fields.md new file mode 100644 index 00000000..dbf91411 --- /dev/null +++ b/docs/architecture/technical-details-of-permission-for-user-fields.md @@ -0,0 +1,156 @@ +### Terminology: + +- user row: a user row refers to a row being updated. Row is redundant but included to + help distinguish between row and field level security. +- team mate: a user assigned through UserPermission to the same project as another user +- any team member: a user assigned to a project through UserPermission +- API end points / data operations + - get / read + - patch / update + - post / create + +### Source of Privileges + +Field level security specifics are derived from u[cru.py](../../app/core/cru_permissions.py). The file includes several lists that +you can use to derive different privileges. Search for these terms + +- `_cru_permissions[profile_value]` +- `_cru_permissions[member_project]` +- `_cru_permissions[practice_lead_project]` +- `_cru_permissions[admin_global]` + fields followed by CRU or a subset of CRU for Create/Read/Update. Example: + `first_name:["RU"]` for a list would indicate that first name is readable and updateable + for the list. + +### Functionality + +The following API endpoints retrieve users: + +#### /users endpoint functionality + +- Row level security + + - Functionality: + - Global admins, can create, read, and update any user row. + - Any team member can read any other project member. + - Project leads can update any team member. + - Practice leads can update any team member in the same practice area (not currently implemented) + +- Field level security: + + - /user end point: + - Global admins can read, update, and create fields specified in + [cru.py](../../app/core/cru.py). Search for + `_user_permissions[admin_global]`). + + - Project admins can read and update fields specified in + [cru.py](../../app/core/cru.py) for other project leads.\ + Search for for `_user_permissions[admin_project]` in [cru.py](../../app/core/cru.py) + + - Practice area leads can read and update fields specified in + [cru.py](../../app/core/cru.py) for fellow team members. If + the team member is in the same practice area,\ + Search for for `_user_permissions[practice_lead_project]` in [cru.py](../../app/core/cru.py) + + If user being queried is not from the same practice area then search for `_user_permissions[member_project]` + + Note: As of 24-Sep-2024, the implemented code treats practice area leads the same as project + admins. + + - Project members can read fields specified in + [cru.py](../../app/core/cru.py) for fellow team members. + Search for for `_user_permissions[member_project]` in [cru.py](../../app/core/cru.py) + + Note: for non global admins, the /me endpoint, which can be used when reading or + updating yourself, provides more field permissions. + +#### /me endpoint functionality + +Used for reading and updating information about the user that is logged in. User permission assignments do not apply. + +- Row Level Security: Logged in user can always read and update their own information. +- Field Level Security: For read and update permissions, see `_cru_permissions[profile_value]` in [cru.py](../../app/core/cru.py). + +#### /eligible-users/?scope=\ - List users. + +This is covered by issue #394. + +#### End Point Technical Implementation + +##### Field level specifics / cru.py + +The implemented field level security specifics can be derived from [cru.py](../../app/core/cru.py) and should match the requirements. If field privileges change or the requirements +don't match what is implemented this can be fixed by changing [cru.py](../../app/core/cru.py). + +##### /user endpoint technical implementation + +- response fields for get, patch, and post: + **serializers.py, permission_check.py** +- get (read) + - /user - see above bullet about response fields. + - /user/ fetches a specific user. See above bullet about response fields. If the requesting_user does not have permission + to view the user, PermisssionUtil.get_user_read_fields will find no fields to serialize and throw a ValidationError +- patch (update): `UserViewSet.partial_update` => `UserValidation.validate_patch_request(request)`.\ + validate_user_fields_patchable(requesting_user, response_related_user, request_fields)\` will compare request fields + against `cru.user_post_fields[admin_global]` which is derived from `_cru_permissions`. If the request fields + include a field outside the requesting_user's scope, the method returns a PermissionError, otherwise the + record is udated. **views.py, permission_check.py** +- post (create): UserViewSet.create: If the requesting_user is not a global admin, the create method + will throw an error. Calls UserValidation.validate_user_fields_postable which compares + pe **views.py** + +##### /me end point technical implementation + +- response fields for get and patch: `UserProfileAPISerializer.to_representation` => `UserValidation.get_user_read_fields` determines which fields are serialized. +- get: see response fields above. No request fields accepted. **views.py, serializer.py** +- patch (update): By default, calls super().update_partial of UserProfileAPIView for + the requesting user to update themselves. **views.py, serializer.py** +- post (create): not applicable. Prevented by setting http_method_names in + UserProfileAPIView to ["patch", "get"] + +#### Supporting Files + +Documentation is generated by pydoc package. pydoc reads comments between triple quotes. See Appendix A. + +- [permission_check.html](./docs/pydoc/permission_check.html) +- [permission_fields.py](./docs/pydoc/http_method_field_permissions.html) => called from permission_check to + determine permissiable fields. permission_fields.py derives permissable fields from + user_permission_fields. +- user_permission_fields_constants.py => see permission_fields.py +- constants.py => holds constants for permission types. +- urls.py + +### Test Technical Details + +Details of the purpose of each test and supporting code can be found in the the docs/pydoc documentation. Additional methods are automatically called based on the name +of the method. + +### Appendix A - Generate pydoc Documentation + +#### Adding New Documentation + +pydoc documentation are located between triple quotes. + +- See https://realpython.com/documenting-python-code/#docstring-types for format for creating class, method, + or module pydoc. For documenting specific variables, you can do this as part of the class, method, + or module documentation. +- Check the file is included in documentation.py +- After making the change, generate as explained below. + +#### Modifying pydoc Documentation + +Look for documentation between triple quotes. Modify the documentation, then generate as explained +below. + +#### Generating pydoc Documentation + +From Docker screen, locate web container. Select option to open terminal. To run locally, open local +terminal. From terminal: + +``` +cd app +../scripts/loadenv.sh +python scripts/ +documentation.py +mv *.html ../docs/pydoc +``` diff --git a/docs/contributing/howto/implement-user-based-security.md b/docs/contributing/howto/implement-user-based-security.md new file mode 100644 index 00000000..bd6aff05 --- /dev/null +++ b/docs/contributing/howto/implement-user-based-security.md @@ -0,0 +1,128 @@ +## Terminology + +- **one-to-many user-related data access policy:** policy for tables where each row in the table is related to one and only one user, directly or indirectly. +- **authorization data access policy:** policy that requires authorization for create, update, delete and optionally read access. +- **other data access policy:** any custom policy not covered by the previous two polices. For example, data access policy for create, update, and delete could be based on Djano roles. In that scenario, a specific table might only be updateable by a user with a specific Django role. + +## One-to-many user related data policy + +This is designed to work for these tables: `user`, `win`, `user_availability`, `user_employment_change`, `user_check`, `check_in`, `user_permission`. + +### user field + +A table that requires a user related data policy must have "user" as a field that references the one user for a particluar row. + +### Fetching Rows + +- determines which rows are returned for a get request + +- implementation: + + - modify views.py + - find `ViewSet` + + - add the following code: + + ``` + def get_queryset(self): + queryset = GenericRequest.get_queryset(view=self) + ``` + +### Record security + +- determines whether a specific record can be viewed, updated, or created. If the table requires field level security then implementing record level security is not required. +- implementation: + - modify views.py + - find `
ViewSet` + - find line `permission = [....]` + - add UserBasedRecordPermission to the list + +### Field security + +- determines which fields, if any, can be included in a request to update or create. +- implementation: + - modify field_permissions.csv to include field level configuration, if not already there + - modify views.py + - find `
ViewSet` + - find line `permission = [....]` + - add UserBasedFieldPermission to the list + +### Response data + +- determines the fields returned in a response for each row. +- implementation: + - modify serializer.py + - find `
Serializer` + + - add following code at end of serializer: + + ``` + def to_representation(self, instance): + representation = super().to_representation(instance) + return GenericRequest.get_serializer_representation(self, instance, representation) + ``` + +## Authorization data access policy + +For many tables, create, update, and delete for all rows in the table are allowed if the request is from an authenticated user. Ability to read all rows may or may not require authentication. To implement one of these +options modify view.py: + +- find `
ViewSet` +- find line `permission = [....]` +- if read access requires authentication, make sure the permission includes isAuthenticated +- if read access does not require authentication, add isAuthenticatedOrReadOnly and if applicable, remove isAuthenticated. + +## Appendix A - Notes on API endpoints + +### Functionality + +The following API endpoints retrieve users: + +#### /users endpoint functionality + +- Row level security + + - Functionality: + - Global admins, can create, read, and update any user row. + - Any team member can read any other project member. + - Project leads can update any team member. + - Practice leads can update any team member in the same practice area (not currently implemented) + +- Field level security: + + - /user end point: + - Global admins can read, update, and create fields specified in + [cru.py](../../app/core/cru.py). Search for + `_user_permissions[admin_global]`). + + - Project admins can read and update fields specified in + [cru.py](../../app/core/cru.py) for other project leads.\ + Search for for `_user_permissions[admin_project]` in [cru.py](../../app/core/cru.py) + + - Practice area leads can read and update fields specified in + [cru.py](../../app/core/cru.py) for fellow team members. If + the team member is in the same practice area,\ + Search for for `_user_permissions[practice_lead_project]` in [cru.py](../../app/core/cru.py) + + If user being queried is not from the same practice area then search for `_user_permissions[member_project]` + + Note: As of 24-Sep-2024, the implemented code treats practice area leads the same as project + admins. + + - Project members can read fields specified in + [cru.py](../../app/core/cru.py) for fellow team members. + Search for for `_user_permissions[member_project]` in [cru.py](../../app/core/cru.py) + + Note: for non global admins, the /me endpoint, which can be used when reading or + updating yourself, provides more field permissions. + +#### /me endpoint functionality + +Used for reading and updating information about the user that is logged in. User permission assignments do not apply. + +- Row Level Security: Logged in user can always read and update their own information. +- Field Level Security: For read and update permissions, see `_cru_permissions[profile_value]` in [cru.py](../../app/core/cru.py). + +#### /eligible-users/?scope=\ - List users. + +This is covered by issue #394. diff --git a/scripts/terminal.sh b/scripts/terminal.sh new file mode 100755 index 00000000..bfcbd0ce --- /dev/null +++ b/scripts/terminal.sh @@ -0,0 +1,8 @@ +#!/bin/bash +set -euo pipefail +IFS=$'\n\t' + +echo "\q to quit" + +set -x +docker-compose exec web /bin/sh -e .env.docker diff --git a/scripts/test.sh b/scripts/test.sh index b68816f2..4fe9f4dd 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -1,5 +1,6 @@ #!/bin/bash set -euo pipefail +trap 'echo "Error occurred in script at line $LINENO. Exiting..."; read -p "Press Enter to close... " -n1' ERR IFS=$'\n\t' # Default options COVERAGE="--no-cov"