empêcher le contrôle des redirections sur les mails d'enregistrement et de réinitialisation de mot de passe (#76835) #171

Open
bdauvergne wants to merge 2 commits from wip/76835-open-redirection into main
Owner
No description provided.
bdauvergne added 2 commits 2023-11-09 14:24:46 +01:00
gitea/authentic/pipeline/head There was a failure building this commit Details
924cb1c73b
misc: review next_url management on passwords and registration views (#76835)
All forms used on a view handling a next_url must inherit from
NextUrlFormMixin and the view from NextUrlViewMixin or
SuccessUrlViewMixin (the latter if the view will redirect itself to this
url).

NextUrlViewMixin automatically create a form class inheriting from
NextUrlFormMixin if needed. The form class only need to declare
NextUrlFormMixin when it has a direct use of the next_url field, for
example PasswordResetForm needs it to make the reset link.

The new behaviour is more Django-esque as success_url is used as the
next_url of last resort as implemented in Django generic views.

The password change done views was removed as it's not the flow we
wanted, after password change people are by default returned to the
account_management view.

Next url support is removed from the password change view, there is no
need for it, we should always return to the account management page.
Dead code was also removed (get_context_data()).
bdauvergne force-pushed wip/76835-open-redirection from 924cb1c73b to e792b7175b 2023-11-09 17:39:29 +01:00 Compare
bdauvergne force-pushed wip/76835-open-redirection from e792b7175b to 390b057fef 2023-11-09 18:30:01 +01:00 Compare
bdauvergne force-pushed wip/76835-open-redirection from 390b057fef to 741ef113e9 2023-11-09 19:30:01 +01:00 Compare
bdauvergne force-pushed wip/76835-open-redirection from 741ef113e9 to 9dbfb87b8b 2023-11-09 19:31:02 +01:00 Compare
bdauvergne changed title from WIP: wip/76835-open-redirection to wip/76835-open-redirection 2023-11-09 19:31:08 +01:00
pmarillonnet reviewed 2023-11-14 15:42:33 +01:00
pmarillonnet left a comment
Owner

J’avoue n’avoir pas trop compris pourquoi le mixin de gestion de la next_url dans les vues vient altérer l’héritage du formulaire correspondant.
Est-ce que les formulaires ne sont-ils pas suffisamment identifiés à l’avance pour leur appliquer cet héritage directement, sans passer par functools ?

J’avoue n’avoir pas trop compris pourquoi le mixin de gestion de la next_url dans les vues vient altérer l’héritage du formulaire correspondant. Est-ce que les formulaires ne sont-ils pas suffisamment identifiés à l’avance pour leur appliquer cet héritage directement, sans passer par `functools` ?
bdauvergne changed title from wip/76835-open-redirection to wip/76835-open-redirection (#76385) 2023-12-20 17:27:27 +01:00
bdauvergne changed title from wip/76835-open-redirection (#76385) to wip/76835-open-redirection (#76835) 2023-12-20 17:27:42 +01:00
Author
Owner

J’avoue n’avoir pas trop compris pourquoi le mixin de gestion de la next_url dans les vues vient altérer l’héritage du formulaire correspondant.
Est-ce que les formulaires ne sont-ils pas suffisamment identifiés à l’avance pour leur appliquer cet héritage directement, sans passer par functools ?

C'est pour éviter que les gens aient à se souvenir d'appliquer cet héritage, j'aurai pu bêtement mettre un assert. Le functools n'est pas nécessaire, c'est un peu gratuit, mais dans l'absolu ça évite des soucis de type différent quand on débug avec des types systématiquement recréé (on pourrait avoir deux form issu du même get_form() avec form1.__class__ is not form2.__class__). Je veux bien l'enlever, je peux virer la génération de type (possible que certaines Form ne soient pas uniquement utilisé dans une vue avec next_url et il faudra quand même une bidouille au niveau de la View.

> J’avoue n’avoir pas trop compris pourquoi le mixin de gestion de la next_url dans les vues vient altérer l’héritage du formulaire correspondant. > Est-ce que les formulaires ne sont-ils pas suffisamment identifiés à l’avance pour leur appliquer cet héritage directement, sans passer par `functools` ? C'est pour éviter que les gens aient à se souvenir d'appliquer cet héritage, j'aurai pu bêtement mettre un assert. Le functools n'est pas nécessaire, c'est un peu gratuit, mais dans l'absolu ça évite des soucis de type différent quand on débug avec des types systématiquement recréé (on pourrait avoir deux form issu du même get_form() avec `form1.__class__ is not form2.__class__`). Je veux bien l'enlever, je peux virer la génération de type (possible que certaines Form ne soient pas uniquement utilisé dans une vue avec next_url et il faudra quand même une bidouille au niveau de la View.
bdauvergne changed title from wip/76835-open-redirection (#76835) to empêcher le contrôle des redirections sur les mails d'enregistrement et de réinitialisation de mot de passe (#76835) 2023-12-20 17:33:46 +01:00
bdauvergne force-pushed wip/76835-open-redirection from 9dbfb87b8b to d246432e72 2024-01-16 13:17:05 +01:00 Compare
bdauvergne requested review from pmarillonnet 2024-01-16 17:24:51 +01:00
bdauvergne force-pushed wip/76835-open-redirection from d246432e72 to 0fb65107ac 2024-01-29 12:50:03 +01:00 Compare
All checks were successful
gitea/authentic/pipeline/head This commit looks good
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b wip/76835-open-redirection main
git pull origin wip/76835-open-redirection

Step 2:

Merge the changes and update on Gitea.
git checkout main
git merge --no-ff wip/76835-open-redirection
git push origin main
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/authentic#171
No description provided.