Connexion : Nouvelle page pour se connecter à un compte existant suite à un conflit d'email lors de l'enregistrement [GEN-1569] #4865
Conversation
tonial
left a comment
tonial
left a comment
There was a problem hiding this comment.
Nice !
J'ai fait quelques remarques mais dans l'ensemble c'est super !
calummackervoy
force-pushed
the
calum/bloquer-remplacement-sso-error-message
branch
2 times, most recently
from
ef7b850 to
da8b4b3
Compare
calummackervoy
added
the
1-recette-jetable
|
🥁 La recette jetable est prête ! 👉 Je veux tester cette PR ! |
calummackervoy
force-pushed
the
calum/bloquer-remplacement-sso-error-message
branch
from
da8b4b3 to
9a598c7
Compare
calummackervoy
force-pushed
the
calum/bloquer-remplacement-sso-error-message
branch
3 times, most recently
from
697b899 to
b07996f
Compare
|
@tonial j'ai ajouté un bouton de retour sur la page pour le cas de rédirection (demande d'une relecture UX) dans le dernier commit. Si tout a l'air bon pour toi je les squasherai et merger |
in our login views we use this format for images fix: source.srcset static hash
calummackervoy
force-pushed
the
calum/bloquer-remplacement-sso-error-message
branch
from
b07996f to
0df7f9a
Compare
allows users to login to their accounts (given a public_id) with only their sign-in method of choice displayed. This will be used to redirect users to the correct sign-in method when there is an error relating to the use of a different sign-in method on the same email fix(middleware.py): support login urls with params feat(LoginExistingUser): support for ProConnect feat(LoginExistingUser): support for return button fix(login_existing_user.html): spacing
allows us to remove code duplication between identity providers. Makes more obvious to new readers that certain IDPs serve only certain user kinds refactor(openid_connect): rename utility function
calummackervoy
force-pushed
the
calum/bloquer-remplacement-sso-error-message
branch
2 times, most recently
from
2a2768c to
0ec338e
Compare
…isting login for better UX, when the user encounters that the email address is in use by another account, we provide them with the option to login via one link to the existing account fix(tests): remove unittest_compatibility changes from another PR fix(test_callback_redirect_on_email_in_use_exception): typo
calummackervoy
force-pushed
the
calum/bloquer-remplacement-sso-error-message
branch
from
0ec338e to
82cfdf2
Compare
francoisfreitag
left a comment
francoisfreitag
left a comment
There was a problem hiding this comment.
Autrement ça m’a l’air de bien fonctionner 👍
| login_routes = [reverse(f"login:{url.name}") for url in login_urls.urlpatterns] + [reverse("account_login")] | ||
| skip_middleware_conditions = [ | ||
| request.path in login_routes, | ||
| request.path.startswith("/login/"), |
There was a problem hiding this comment.
Je ne comprends pas pourquoi ce changement est meilleur ? Le fait qu’on inclue actuellement sous le chemin /login ne garantie pas qu’on continue de le faire. Énumérer les vues de l’app login me paraît plus sûr ?
Exemple au hasard, si on décide de renommer nos URLs pour être en français, /connexion ne sera pas exclue du middleware, alors que la méthode actuelle devrait continuer à fonctionner.
There was a problem hiding this comment.
Je vois le souci, on ne peut pas reverse la nouvelle vue, car on n’a pas d’UUID, et request.resolver_match n’est apparemment pas encore disponible. Comme on a déjà plein de startswith, j’imagine qu’un de plus ne pose pas tant de problème.
There was a problem hiding this comment.
C'est vrai mais il y a déjà des autres lignes impliquées par cette contrainte, e.g. request.path.startswith("/signup/siae/join")
Je l'ai ajouté avec la nouvelle vue (login/existing/xyz) - le paramètre xyz empêche l'utilisation du list simple, et je voulais éviter augmenter le compte des lookups - il me semblait un bon compromis entre ces deux concernes ?
calummackervoy
deleted the
calum/bloquer-remplacement-sso-error-message
branch
🤔 Pourquoi ?
Dans le cadre de la refonte de parcours d'inscription, quand les utilisateurs se connecte avec un nouveau compte qui partage un email avec un autre compte, on affiche un erreur. Dans ce PR on affiche un erreur standardisé, qui permet l'utilisateur à se connecter au compte existant (via une page dédiée) ou de s'inscrire avec un email différent depuis la page signup.
🚨 À vérifier
🏝️ Comment tester
💻 Captures d'écran
Example :
En cliquant sur « se connecter avec ce compte » :
En cliquant sur « retour » :