Utiliser le SessionNamespace pour sécuriser l’édition des fiches de poste [GEN-2311] - #6044
Merged
Conversation
francoisfreitag
changed the title
francoisfreitag
changed the title
francoisfreitag
commented
Apr 28, 2025
Comment on lines
+292
to
+347
| if edit_session_id: | ||
| session_namespace = SessionNamespace(request.session, JOB_DESCRIPTION_EDIT_SESSION_KIND, edit_session_id) | ||
| if not session_namespace.exists(): | ||
| raise Http404 | ||
| session_data = session_namespace.as_dict() | ||
| else: | ||
| session_namespace = None | ||
| session_data = {} | ||
|
|
||
| # TODO(François): Drop this fallback next week. | ||
| if job_description_id is None and edit_session_id is None: | ||
| session_data = request.session.get(ITOU_SESSION_JOB_DESCRIPTION_KEY, {}) | ||
| job_description_id = session_data.get("pk") | ||
|
|
||
| if job_description_id: | ||
| job_description = get_object_or_404( | ||
| JobDescription.objects.select_related("appellation", "location"), | ||
| pk=job_description_id, | ||
| company=request.current_organization, | ||
| ) | ||
| if job_description_id | ||
| else None | ||
| ) | ||
| else: | ||
| job_description = None | ||
|
|
||
| form = companies_forms.EditJobDescriptionForm( | ||
| request.current_organization, instance=job_description, data=request.POST or None, initial=session_data | ||
| ) | ||
|
|
||
| if request.method == "POST" and form.is_valid(): | ||
| request.session[ITOU_SESSION_JOB_DESCRIPTION_KEY] = {**session_data, **form.cleaned_data} | ||
| return HttpResponseRedirect(reverse("companies_views:edit_job_description_details")) | ||
| session_data.update(form.cleaned_data) | ||
| if session_namespace is None: | ||
| session_namespace = SessionNamespace.create_uuid_namespace( | ||
| request.session, JOB_DESCRIPTION_EDIT_SESSION_KIND, session_data | ||
| ) | ||
| else: | ||
| session_namespace.update(session_data) |
Member
Author
There was a problem hiding this comment.
Suite à la discussion de jeudi, c’est le micmac de création de la SessionNamespace que j’aimerais simplifier.
Si je trouve le temps dans les semaines à venir, je tenterai mon réusinage. :)
xavfernandez
reviewed
May 5, 2025
Comment on lines
+19
to
+25
| path("edit_job_description/<uuid:edit_session_id>", views.edit_job_description, name="edit_job_description"), | ||
| path("edit_job_description/<int:job_description_id>", views.edit_job_description, name="edit_job_description"), | ||
| path( | ||
| "edit_job_description/<int:job_description_id>/<uuid:edit_session_id>", | ||
| views.edit_job_description, | ||
| name="edit_job_description", | ||
| ), |
Contributor
There was a problem hiding this comment.
Est-ce que des URLs en job_description/<int:job_description_id>/edit/..., job_description/create/... ne seraient pas plus jolis ?
Member
Author
There was a problem hiding this comment.
Si certainement, mais pas au milieu de cette PR qui a déjà assez à s’occuper de la rétrocompatibilité sans avoir à ajouter des changements cosmétiques.
Comment on lines
17
to
18
| path("job_description_list", views.job_description_list, name="job_description_list"), | ||
| path("edit_job_description", views.edit_job_description, name="edit_job_description"), |
Contributor
There was a problem hiding this comment.
job_description/list & job_description/create ? 🥹
mais bon ça serait dans une autre PR.
Member
Author
There was a problem hiding this comment.
Je me mets un TODO pour ces URLs, après avoir supprimé la compat’.
xavfernandez
approved these changes
May 5, 2025
Like other views in the system (apply, job_seekers, …), prevent collisions with multiple tabs by specifying the session key in the URL. Clarify URLs by adding the `job_description_id` in their path. The tunnel now starts with `edit_job_description` instead of `update_job_description`. The `update_job_description` is a useless redirect. When edit data exists in the session (using the back_url), `edit_job_description` will use the existing session namespace. Otherwise, it’ll initialize a session namespace. test_empty_session_during_edit was removed, as a SessionNamespace is now required for edit views after the starting view: edit_job_description.
francoisfreitag
force-pushed
the
ff/job-desc-rework
branch
from
May 5, 2025 14:06
3a74266 to
19afc76
Compare
francoisfreitag
enabled auto-merge
May 5, 2025 14:11
francoisfreitag
added this pull request to the merge queue
May 5, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤔 Pourquoi ?
Protéger le parcours des problèmes d’une session modifiée dans des onglets multiples.
🍰 Comment ?
En écrivant l’ID de la session dans l’URL.
J’en ai profité pour ajouter
job_description_iddans les URLs, afin de mieux indiquer si le parcours est une création de fiche de poste ou une mise à jour.test_empty_session_during_edita été supprimé, puisque laSessionNamespacerequiert dorénavant uneedit_session_id.🏝️ Comment tester ?
Créer une fiche de poste.
Mettre à jour une fiche de poste.