Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ne pas traiter inutilement l'email dans l'implémentation de l'authentification MonComptePro #486

Open
florimondmanca opened this issue Oct 10, 2022 · 2 comments

Comments

@florimondmanca
Copy link
Collaborator

florimondmanca commented Oct 10, 2022

Voir discussion ici : #482 (comment)

Actuellement on collecte l'email du compte Mon Compte Pro car c'est historiquement ce qu'on utilisait comme identifiant lors de la connexion email/mdp.

Il n'est pas certain que ce soit nécessaire. Mon Compte Pro a un scope profile qui devrait contenir des informations comme un ID, un prénom/nom. L'ID pourrait servir d'identifiant unique, et le prénom/nom permettrait de construire un "display name". Dans le cas de la connexion legacy en email/mdp, le display name serait l'email.

Implémentation

Base de données

Cela aurait un impact sur la DB ainsi que la modélisation. En clair, si le scope profile permet bien de récupérer les informations supposées, et à supposer que l'on soit autorisés à l'activer (à discuter avec l'équipe Mon Compte Pro), on devrait passer de :

Account
id*
email
...
DataPassUser
account_id*
PasswordUser
account_id*
password_hash

À :

Account
id*
...
DataPassUser
account_id*
datapass_id*
first_name
last_name
PasswordUser
account_id*
email*
password_hash

Récupération des comptes utilisateur

Dans la plupart du code backend, on récupère un utilisateur par son email. Or chaque account a un id qui est déjà unique.

Il faudrait donc modifier le code pour ne faire référence qu'à account.id.

@johanricher
Copy link
Member

johanricher commented Oct 10, 2022

Aujourd'hui on utilise l'adresse email (récupérée depuis MCP) comme ID unique, ce qui est selon moi une forme d'antipattern. C'est une donnée à caractère personnel dont on n'a pas besoin (puisqu'on n'envoie pas d'emails aux utilisateurs).

Pour les besoins back (invisibles des utilisateurs), utiliser un ID unique fait maison pourrait faire l'affaire. A ma connaissance le seul usage front de l'adresse c'est l'affichage dans le header quand l'utilisateur est connecté. On pourrait l'enlever et ne garder que le bouton "Déconnexion".

@florimondmanca
Copy link
Collaborator Author

florimondmanca commented Oct 10, 2022

Oui ça me semble tout-à-fait pertinent. C'était une erreur de s'appuyer au départ sur l'email comme ID unique, alors qu'on avait déjà un account.id (initialement user.id du temps où on n'avait que l'authentification par email/mdp). Le code le reflète en faisant des opérations comme GetAccountByEmail : ça devrait être GetAccountByID, etc. J'ai mis à jour le ticket avec ces qq autres réflexions.

@florimondmanca florimondmanca changed the title L'authentification par Mon Compte Pro pourrait-elle se passer de traiter l'email ? Revoir l'implémentation de l'authentification Mon Compte Pro pour ne pas traiter inutilement l'email Oct 10, 2022
@florimondmanca florimondmanca changed the title Revoir l'implémentation de l'authentification Mon Compte Pro pour ne pas traiter inutilement l'email Ne pas traiter inutilement l'email dans l'implémentation de l'authentification Mon Compte Pro Oct 10, 2022
@florimondmanca florimondmanca changed the title Ne pas traiter inutilement l'email dans l'implémentation de l'authentification Mon Compte Pro Ne pas traiter inutilement l'email dans l'implémentation de l'authentification MonComptePro Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants