Skip to content

Commit

Permalink
auth: add check_user to replace user_passes_test
Browse files Browse the repository at this point in the history
To explicitly return a 403 instead of silently redirecting.
  • Loading branch information
xavfernandez committed Dec 4, 2024
1 parent b62830d commit a98d627
Show file tree
Hide file tree
Showing 12 changed files with 80 additions and 102 deletions.
17 changes: 17 additions & 0 deletions itou/utils/auth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
from functools import wraps

from django.core.exceptions import PermissionDenied


def check_user(test_func, err_msg=""):
def decorator(view_func):
def _check_user_view_wrapper(request, *args, **kwargs):
test_pass = test_func(request.user)

if test_pass:
return view_func(request, *args, **kwargs)
raise PermissionDenied(err_msg)

return wraps(view_func)(_check_user_view_wrapper)

return decorator
25 changes: 7 additions & 18 deletions itou/www/apply/views/list_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
from collections import defaultdict

from django.conf import settings
from django.contrib.auth.decorators import login_required, user_passes_test
from django.contrib.auth.decorators import login_required
from django.db import models
from django.db.models import Count, Exists, F, OuterRef, Q, Subquery
from django.shortcuts import render
from django.urls import reverse, reverse_lazy
from django.urls import reverse
from django.utils import timezone
from django.utils.text import slugify

Expand All @@ -15,6 +15,7 @@
from itou.job_applications.export import stream_xlsx_export
from itou.job_applications.models import JobApplication, JobApplicationWorkflow
from itou.rdv_insertion.models import InvitationRequest, Participation
from itou.utils.auth import check_user
from itou.utils.pagination import pager
from itou.utils.perms.company import get_current_company_or_404
from itou.utils.urls import get_safe_url
Expand Down Expand Up @@ -84,7 +85,7 @@ def _add_administrative_criteria(job_applications):


@login_required
@user_passes_test(lambda u: u.is_job_seeker, login_url=reverse_lazy("search:employers_home"), redirect_field_name=None)
@check_user(lambda u: u.is_job_seeker)
def list_for_job_seeker(request, template_name="apply/list_for_job_seeker.html"):
"""
List of applications for a job seeker.
Expand Down Expand Up @@ -155,11 +156,7 @@ def annotate_title(base_title, archived_choice):


@login_required
@user_passes_test(
lambda u: u.is_prescriber or u.is_employer,
login_url=reverse_lazy("search:employers_home"),
redirect_field_name=None,
)
@check_user(lambda u: u.is_prescriber or u.is_employer)
def list_prescriptions(request, template_name="apply/list_prescriptions.html"):
"""
List of applications for a prescriber.
Expand Down Expand Up @@ -201,11 +198,7 @@ def list_prescriptions(request, template_name="apply/list_prescriptions.html"):


@login_required
@user_passes_test(
lambda u: u.is_prescriber or u.is_employer,
login_url=reverse_lazy("search:employers_home"),
redirect_field_name=None,
)
@check_user(lambda u: u.is_prescriber or u.is_employer)
def list_prescriptions_exports(request, template_name="apply/list_of_available_exports.html"):
"""
List of applications for a prescriber, sorted by month, displaying the count of applications per month
Expand All @@ -226,11 +219,7 @@ def list_prescriptions_exports(request, template_name="apply/list_of_available_e


@login_required
@user_passes_test(
lambda u: u.is_prescriber or u.is_employer,
login_url=reverse_lazy("search:employers_home"),
redirect_field_name=None,
)
@check_user(lambda u: u.is_prescriber or u.is_employer)
def list_prescriptions_exports_download(request, month_identifier=None):
"""
List of applications for a prescriber for a given month identifier (YYYY-mm),
Expand Down
9 changes: 3 additions & 6 deletions itou/www/apply/views/process_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import sentry_sdk
from django.conf import settings
from django.contrib import messages
from django.contrib.auth.decorators import login_required, user_passes_test
from django.contrib.auth.decorators import login_required
from django.contrib.auth.mixins import LoginRequiredMixin
from django.core.exceptions import PermissionDenied
from django.db import transaction
Expand All @@ -30,6 +30,7 @@
from itou.rdv_insertion.api import get_api_credentials, get_invitation_status
from itou.rdv_insertion.models import Invitation, InvitationRequest, Participation
from itou.users.enums import Title
from itou.utils.auth import check_user
from itou.utils.urls import get_safe_url
from itou.www.apply.forms import (
AcceptForm,
Expand Down Expand Up @@ -273,11 +274,7 @@ def details_for_company(request, job_application_id, template_name="apply/proces


@login_required
@user_passes_test(
lambda u: u.is_prescriber or u.is_employer,
login_url=reverse_lazy("search:employers_home"),
redirect_field_name=None,
)
@check_user(lambda u: u.is_prescriber or u.is_employer)
def details_for_prescriber(request, job_application_id, template_name="apply/process_details.html"):
"""
Detail of an application for an SIAE with the ability:
Expand Down
10 changes: 3 additions & 7 deletions itou/www/autocomplete/views.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
from datetime import datetime

from django.contrib.auth.decorators import login_required, user_passes_test
from django.contrib.auth.decorators import login_required
from django.db.models import F, Q, Value
from django.db.models.functions import Least, Lower, NullIf, StrIndex
from django.http import JsonResponse
from django.urls import reverse_lazy
from unidecode import unidecode

from itou.asp.models import Commune
from itou.cities.models import City
from itou.jobs.models import Appellation
from itou.users.models import User
from itou.utils.auth import check_user
from itou.www.gps.views import is_allowed_to_use_gps_advanced_features


Expand Down Expand Up @@ -124,11 +124,7 @@ def communes_autocomplete(request):


@login_required
@user_passes_test(
is_allowed_to_use_gps_advanced_features,
login_url=reverse_lazy("dashboard:index"),
redirect_field_name=None,
)
@check_user(is_allowed_to_use_gps_advanced_features)
def gps_users_autocomplete(request):
"""
Returns JSON data compliant with Select2
Expand Down
25 changes: 9 additions & 16 deletions itou/www/geiq_views/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import logging

from django.contrib import messages
from django.contrib.auth.decorators import login_required, user_passes_test
from django.contrib.auth.decorators import login_required
from django.core.exceptions import ImproperlyConfigured
from django.core.files.storage import default_storage
from django.db.models import Count, F, OuterRef, Prefetch, Q, Subquery, Sum
Expand All @@ -27,6 +27,7 @@
from itou.institutions.enums import InstitutionKind
from itou.institutions.models import Institution
from itou.utils.apis import geiq_label
from itou.utils.auth import check_user
from itou.utils.emails import send_email_messages
from itou.utils.pagination import pager
from itou.utils.urls import get_safe_url
Expand Down Expand Up @@ -65,9 +66,7 @@ def _get_assessments_for_labor_inspector(request):


@login_required
@user_passes_test(
lambda user: user.is_active and (user.is_employer or user.is_labor_inspector), redirect_field_name=None
)
@check_user(lambda user: user.is_active and (user.is_employer or user.is_labor_inspector))
def assessment_info(request, assessment_pk):
if request.user.is_employer:
return _assessment_info_for_employer(request, assessment_pk)
Expand Down Expand Up @@ -133,7 +132,7 @@ def _assessment_info_for_labor_inspector(


@login_required
@user_passes_test(lambda user: user.is_active and user.is_labor_inspector, redirect_field_name=None)
@check_user(lambda user: user.is_active and user.is_labor_inspector)
def assessment_review(request, assessment_pk, template_name="geiq/assessment_review.html"):
assessment = get_object_or_404(_get_assessments_for_labor_inspector(request), pk=assessment_pk)
back_url = reverse("geiq:assessment_info", kwargs={"assessment_pk": assessment.pk})
Expand All @@ -156,9 +155,7 @@ def assessment_review(request, assessment_pk, template_name="geiq/assessment_rev


@login_required
@user_passes_test(
lambda user: user.is_active and (user.is_employer or user.is_labor_inspector), redirect_field_name=None
)
@check_user(lambda user: user.is_active and (user.is_employer or user.is_labor_inspector))
def employee_list(request, assessment_pk, info_type):
try:
info_type = InfoType(info_type)
Expand Down Expand Up @@ -253,7 +250,7 @@ def _lock_assessment_and_sync(assessment):

@login_required
@require_POST
@user_passes_test(lambda user: user.is_active and user.is_employer, redirect_field_name=None)
@check_user(lambda user: user.is_active and user.is_employer)
def label_sync(request, assessment_pk):
assessment = get_object_or_404(
ImplementationAssessment.objects.filter(
Expand All @@ -273,9 +270,7 @@ def label_sync(request, assessment_pk):

@login_required
@require_safe
@user_passes_test(
lambda user: user.is_active and (user.is_employer or user.is_labor_inspector), redirect_field_name=None
)
@check_user(lambda user: user.is_active and (user.is_employer or user.is_labor_inspector))
def employee_details(request, employee_pk):
if request.user.is_labor_inspector:
assessments = _get_assessments_for_labor_inspector(request)
Expand Down Expand Up @@ -303,7 +298,7 @@ def employee_details(request, employee_pk):


@login_required
@user_passes_test(lambda user: user.is_active and user.is_labor_inspector, redirect_field_name=None)
@check_user(lambda user: user.is_active and user.is_labor_inspector)
def geiq_list(request, institution_pk, year=None, template_name="geiq/geiq_list.html"):
institution = get_object_or_404(
Institution.objects.filter(
Expand Down Expand Up @@ -342,9 +337,7 @@ def geiq_list(request, institution_pk, year=None, template_name="geiq/geiq_list.

@require_safe
@login_required
@user_passes_test(
lambda user: user.is_active and (user.is_employer or user.is_labor_inspector), redirect_field_name=None
)
@check_user(lambda user: user.is_active and (user.is_employer or user.is_labor_inspector))
def assessment_report(request, assessment_pk):
if request.user.is_labor_inspector:
assessments = (
Expand Down
29 changes: 7 additions & 22 deletions itou/www/gps/views.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
from django.contrib.auth.decorators import login_required, user_passes_test
from django.contrib.auth.decorators import login_required
from django.db.models import Count
from django.http import HttpResponseRedirect
from django.shortcuts import render
from django.urls import reverse, reverse_lazy
from django.urls import reverse

from itou.gps.models import FollowUpGroup, FollowUpGroupMembership
from itou.utils.auth import check_user
from itou.utils.pagination import pager
from itou.utils.urls import get_safe_url
from itou.www.gps.forms import GpsUserSearchForm, MembershipsFiltersForm
Expand All @@ -19,11 +20,7 @@ def is_allowed_to_use_gps_advanced_features(user):


@login_required
@user_passes_test(
is_allowed_to_use_gps,
login_url=reverse_lazy("dashboard:index"),
redirect_field_name=None,
)
@check_user(is_allowed_to_use_gps)
def my_groups(request, template_name="gps/my_groups.html"):
memberships = (
FollowUpGroupMembership.objects.filter(member=request.user)
Expand Down Expand Up @@ -51,11 +48,7 @@ def my_groups(request, template_name="gps/my_groups.html"):


@login_required
@user_passes_test(
is_allowed_to_use_gps_advanced_features,
login_url=reverse_lazy("dashboard:index"),
redirect_field_name=None,
)
@check_user(is_allowed_to_use_gps_advanced_features)
def join_group(request, template_name="gps/join_group.html"):
form = GpsUserSearchForm(data=request.POST or None)

Expand All @@ -79,11 +72,7 @@ def join_group(request, template_name="gps/join_group.html"):


@login_required
@user_passes_test(
is_allowed_to_use_gps,
login_url=reverse_lazy("dashboard:index"),
redirect_field_name=None,
)
@check_user(is_allowed_to_use_gps)
def leave_group(request, group_id):
membership = (
FollowUpGroupMembership.objects.filter(member=request.user).filter(follow_up_group__id=group_id).first()
Expand All @@ -97,11 +86,7 @@ def leave_group(request, group_id):


@login_required
@user_passes_test(
is_allowed_to_use_gps,
login_url=reverse_lazy("dashboard:index"),
redirect_field_name=None,
)
@check_user(is_allowed_to_use_gps)
def toggle_referent(request, group_id):
membership = (
FollowUpGroupMembership.objects.filter(member=request.user).filter(follow_up_group__id=group_id).first()
Expand Down
17 changes: 5 additions & 12 deletions itou/www/itou_staff_views/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@

from dateutil.relativedelta import relativedelta
from django.contrib import messages
from django.contrib.auth.decorators import login_required, user_passes_test
from django.contrib.auth.decorators import login_required
from django.db.models import Q
from django.http import FileResponse, Http404, HttpResponseRedirect, StreamingHttpResponse
from django.shortcuts import get_object_or_404, render
from django.urls import reverse, reverse_lazy
from django.urls import reverse
from django.utils import timezone
from django.utils.html import format_html
from django.utils.http import content_disposition_header
Expand All @@ -19,6 +19,7 @@
from itou.prescribers.models import PrescriberMembership
from itou.users.enums import UserKind
from itou.users.models import User
from itou.utils.auth import check_user
from itou.utils.db import or_queries
from itou.utils.export import generate_excel_sheet
from itou.www.itou_staff_views import merge_utils
Expand Down Expand Up @@ -206,11 +207,7 @@ def content():


@login_required
@user_passes_test(
lambda u: u.is_superuser,
login_url=reverse_lazy("dashboard:index"),
redirect_field_name=None,
)
@check_user(lambda u: u.is_superuser)
def merge_users(request, template_name="itou_staff_views/merge_users.html"):
form = MergeUserForm(data=request.POST or None)

Expand All @@ -226,11 +223,7 @@ def merge_users(request, template_name="itou_staff_views/merge_users.html"):


@login_required
@user_passes_test(
lambda u: u.is_superuser,
login_url=reverse_lazy("dashboard:index"),
redirect_field_name=None,
)
@check_user(lambda u: u.is_superuser)
def merge_users_confirm(request, user_1_pk, user_2_pk, template_name="itou_staff_views/merge_users_confirm.html"):
ALLOWED_USER_KINDS = [UserKind.PRESCRIBER, UserKind.EMPLOYER]

Expand Down
2 changes: 1 addition & 1 deletion tests/gps/test_create_beneficiary.py
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ def test_creation_by_user_kind(client, UserFactory, factory_args, expected_acces
if expected_access:
assertContains(response, create_beneficiary_url)
else:
assertRedirects(response, reverse("dashboard:index"))
assert response.status_code == 403

response = client.get(create_beneficiary_url)
[job_seeker_session_name] = [k for k in client.session.keys() if k not in KNOWN_SESSION_KEYS]
Expand Down
8 changes: 4 additions & 4 deletions tests/gps/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import freezegun
import pytest
from django.urls import reverse
from pytest_django.asserts import assertContains, assertNotContains, assertQuerySetEqual, assertRedirects
from pytest_django.asserts import assertContains, assertNotContains, assertQuerySetEqual

from itou.gps.models import FollowUpGroup, FollowUpGroupMembership, FranceTravailContact
from itou.users.models import User
Expand Down Expand Up @@ -31,7 +31,7 @@ def test_job_seeker_cannot_use_gps(client):
("gps:toggle_referent", {"group_id": group.pk}),
]:
response = client.get(reverse(route, kwargs=kwargs))
assertRedirects(response, reverse("dashboard:index"), fetch_redirect_response=False)
assert response.status_code == 403
response = client.get(reverse("users:details", kwargs={"public_id": job_seeker.public_id}))
assert response.status_code == 403

Expand Down Expand Up @@ -186,7 +186,7 @@ def test_gps_access(client, factory, access):
FEATURE_INVITE = "<span>Inviter un partenaire</span>"
FEATURE_ADD = "<span>Ajouter un bénéficiaire</span>"
if access is None:
assertRedirects(response, reverse("dashboard:index"))
assert response.status_code == 403
elif access == "partial":
assertContains(response, FEATURE_INVITE)
assertNotContains(response, FEATURE_ADD)
Expand All @@ -196,7 +196,7 @@ def test_gps_access(client, factory, access):

response = client.get(reverse("gps:join_group"))
if access is None or access == "partial":
assertRedirects(response, reverse("dashboard:index"))
assert response.status_code == 403
else:
assert response.status_code == 200

Expand Down
Loading

0 comments on commit a98d627

Please sign in to comment.