Skip to content

Commit

Permalink
suspensions: fix back_url handling in deletion processes
Browse files Browse the repository at this point in the history
  • Loading branch information
xavfernandez committed Sep 27, 2024
1 parent 6046bf2 commit 83faa1f
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 27 deletions.
2 changes: 1 addition & 1 deletion itou/templates/approvals/suspension_delete.html
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ <h1>
<form method="post" class="js-prevent-multiple-submit">
{% csrf_token %}
<input type="hidden" name="confirm" value="true">
{% itou_buttons_form primary_label="Confirmer la suppression" secondary_url=back_url reset_url=reset_url %}
{% itou_buttons_form primary_label="Confirmer la suppression" secondary_url=secondary_url reset_url=reset_url %}
</form>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion itou/templates/approvals/suspension_update_enddate.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ <h1>

{% bootstrap_form form %}

{% itou_buttons_form primary_label="Suivant" secondary_url=back_url reset_url=reset_url %}
{% itou_buttons_form primary_label="Suivant" secondary_url=secondary_url reset_url=reset_url %}
</form>
</div>
</div>
Expand Down
43 changes: 27 additions & 16 deletions itou/www/approvals_views/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,24 +480,31 @@ def suspension_action_choice(request, suspension_id, template_name="approvals/su
if not suspension.can_be_handled_by_siae(siae):
raise PermissionDenied()

back_url = get_safe_url(
request,
"back_url",
fallback_url=reverse("employees:detail", kwargs={"public_id": suspension.approval.user.public_id}),
)

if request.method == "POST":
if request.POST.get("action") == "delete":
return HttpResponseRedirect(
reverse("approvals:suspension_delete", kwargs={"suspension_id": suspension.id})
add_url_params(
reverse("approvals:suspension_delete", kwargs={"suspension_id": suspension.id}),
{"back_url": back_url},
)
)

if request.POST.get("action") == "update_enddate":
return HttpResponseRedirect(
reverse("approvals:suspension_update_enddate", kwargs={"suspension_id": suspension.id})
add_url_params(
reverse("approvals:suspension_update_enddate", kwargs={"suspension_id": suspension.id}),
{"back_url": back_url},
)
)

return HttpResponseBadRequest('invalid "action" parameter')

back_url = get_safe_url(
request,
"back_url",
fallback_url=reverse("employees:detail", kwargs={"public_id": suspension.approval.user.public_id}),
)
context = {
"suspension": suspension,
"back_url": back_url,
Expand Down Expand Up @@ -559,22 +566,23 @@ def suspension_update_enddate(request, suspension_id, template_name="approvals/s
back_url = get_safe_url(
request,
"back_url",
fallback_url=reverse("approvals:suspension_action_choice", kwargs={"suspension_id": suspension_id}),
fallback_url=reverse("employees:detail", kwargs={"public_id": suspension.approval.user.public_id}),
)

if request.method == "POST" and form.is_valid():
suspension.end_at = form.cleaned_data["first_day_back_to_work"] - timedelta(days=1)
suspension.updated_by = request.user
suspension.save()
messages.success(request, "Modification de suspension effectuée.", extra_tags="toast")
return HttpResponseRedirect(
reverse("employees:detail", kwargs={"public_id": suspension.approval.user.public_id})
)
return HttpResponseRedirect(back_url)

Check warning

Code scanning / CodeQL

URL redirection from remote source Medium

Untrusted URL redirection depends on a
user-provided value
.

context = {
"suspension": suspension,
"back_url": back_url,
"reset_url": reverse("employees:detail", kwargs={"public_id": suspension.approval.user.public_id}),
"secondary_url": add_url_params(
reverse("approvals:suspension_action_choice", kwargs={"suspension_id": suspension_id}),
{"back_url": back_url},
),
"reset_url": back_url,
"form": form,
}
return render(request, template_name, context)
Expand All @@ -595,7 +603,7 @@ def suspension_delete(request, suspension_id, template_name="approvals/suspensio
back_url = get_safe_url(
request,
"back_url",
fallback_url=reverse("approvals:suspension_action_choice", kwargs={"suspension_id": suspension_id}),
fallback_url=reverse("employees:detail", kwargs={"public_id": suspension.approval.user.public_id}),
)

if request.method == "POST" and request.POST.get("confirm") == "true":
Expand All @@ -611,8 +619,11 @@ def suspension_delete(request, suspension_id, template_name="approvals/suspensio

context = {
"suspension": suspension,
"back_url": back_url,
"reset_url": reverse("employees:detail", kwargs={"public_id": suspension.approval.user.public_id}),
"secondary_url": add_url_params(
reverse("approvals:suspension_action_choice", kwargs={"suspension_id": suspension_id}),
{"back_url": back_url},
),
"reset_url": back_url,
"lost_days": (timezone.localdate() - suspension.start_at).days + 1,
}
return render(request, template_name, context)
4 changes: 2 additions & 2 deletions tests/www/approvals_views/__snapshots__/test_suspend.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@

<div class="form-group col col-lg-auto order-1 order-lg-2">

<a aria-label="Retourner à l’étape précédente" class="btn btn-block btn-outline-primary" href="/approvals/suspension/[PK of Suspension]/action/">
<a aria-label="Retourner à l’étape précédente" class="btn btn-block btn-outline-primary" href="/approvals/suspension/[PK of Suspension]/action/?back_url=%2Fsearch%2Femployers">
<span>Retour</span>
</a>

Expand Down Expand Up @@ -854,7 +854,7 @@
</div>
<div class="modal-footer">
<button class="btn btn-sm btn-outline-primary" data-bs-dismiss="modal" type="button">Retour</button>
<a class="btn btn-sm btn-danger" href="/approvals/detail/[public_id of User]">Confirmer l'annulation</a>
<a class="btn btn-sm btn-danger" href="/search/employers">Confirmer l'annulation</a>
</div>
</div>
</div>
Expand Down
35 changes: 28 additions & 7 deletions tests/www/approvals_views/test_suspend.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from itou.approvals.models import Suspension
from itou.employee_record.enums import Status
from itou.utils.urls import add_url_params
from itou.utils.widgets import DuetDatePickerWidget
from itou.www.approvals_views.forms import SuspensionForm
from tests.approvals.factories import SuspensionFactory
Expand Down Expand Up @@ -218,7 +219,7 @@ def test_delete_suspension(self):

self.client.force_login(employer)

back_url = reverse("approvals:suspension_action_choice", kwargs={"suspension_id": suspension.pk})
back_url = reverse("search:employers_home")
redirect_url = reverse("employees:detail", kwargs={"public_id": suspension.approval.user.public_id})
params = urlencode({"back_url": back_url})
url = reverse("approvals:suspension_delete", kwargs={"suspension_id": suspension.pk})
Expand All @@ -240,7 +241,7 @@ def test_delete_suspension(self):
],
)
assert str(form) == self.snapshot(name="delete_suspension_form")
assert response.context["reset_url"] == redirect_url
assert response.context["reset_url"] == back_url

lost_days = (timezone.localdate() - start_at).days + 1 # including start and end dates
assertContains(response, f"Réduire la durée restante de ce PASS IAE de {lost_days} jour")
Expand Down Expand Up @@ -324,14 +325,32 @@ def test_post_delete(self):
self.client.force_login(self.employer)

response = self.client.post(self.url, data={"action": "delete"})
assert response.url == reverse("approvals:suspension_delete", kwargs={"suspension_id": self.suspension.pk})
self.assertRedirects(
response,
add_url_params(
reverse("approvals:suspension_delete", kwargs={"suspension_id": self.suspension.pk}),
{
"back_url": reverse(
"employees:detail", kwargs={"public_id": self.suspension.approval.user.public_id}
)
},
),
)

def test_post_enddate(self):
self.client.force_login(self.employer)

response = self.client.post(self.url, data={"action": "update_enddate"})
assert response.url == reverse(
"approvals:suspension_update_enddate", kwargs={"suspension_id": self.suspension.pk}
self.assertRedirects(
response,
add_url_params(
reverse("approvals:suspension_update_enddate", kwargs={"suspension_id": self.suspension.pk}),
{
"back_url": reverse(
"employees:detail", kwargs={"public_id": self.suspension.approval.user.public_id}
)
},
),
)

def test_post_enddate_with_invalid_action_parameter(self):
Expand Down Expand Up @@ -388,9 +407,11 @@ def test_context(self):
response = self.client.get(self.url)
assert response.status_code == 200
assert response.context["suspension"] == self.suspension
assert response.context["back_url"] == reverse(
"approvals:suspension_action_choice", kwargs={"suspension_id": self.suspension.id}
assert response.context["secondary_url"] == add_url_params(
reverse("approvals:suspension_action_choice", kwargs={"suspension_id": self.suspension.id}),
{"back_url": reverse("employees:detail", kwargs={"public_id": self.suspension.approval.user.public_id})},
)

assert response.context["reset_url"] == reverse(
"employees:detail", kwargs={"public_id": self.suspension.approval.user.public_id}
)
Expand Down

0 comments on commit 83faa1f

Please sign in to comment.