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

[TECH] Rendre le code lié à l'authentification des users plus clair et corriger un test erroné (PIX-15878) #10907

Merged
merged 7 commits into from
Dec 30, 2024

Conversation

lego-technix
Copy link
Contributor

@lego-technix lego-technix commented Dec 28, 2024

🎄 Problème

Avant le travail de confinement des access tokens utilisateurs, des points gênants ont été identifiés :

  • dissémination de la validation dans la gestion des Access Tokens et Refresh Tokens
  • présence d’un test totalement erroné sur la génération d’Access Tokens pour utilisateur
  • tests sur les Access Tokens et Refresh Tokens utilisateurs pas très bien agencés

🎁 Proposition

  • Pour la route /api/token le code assurant la validation du grant_type dissémine la logique de validation entre la route (avec la validation joi) et le controller (avec des if-else dans tokenController.createToken). Il faut choisir : soit la validation complète du grant_type est à faire au niveau dans la validation joi au niveau de la route, soit la validation complète du grant_type est à faire au niveau du controller. La philosophie actuelle du code est de plutôt mettre toute la validation de la payload (avec son grant_type donc) dans la route. Cela a du sens car ce sont des paramètres plutôt techniques (très lié au domaine de l'authentification technique) que métier. Le refactor proposé est donc de rapatrier toute la logique de validation dans la définition de la route.
  • Suppression du test erroné
  • Petite réorganisation minimum des autres tests

🧦 Remarques

RAS

🎅 Pour tester

  • Vérifier que l'authentification utilisateur par mot de passe fonctionne (en se connectant par exemple sur Pix App avec le compte superadmin@example.net)
  • Vérifier que la génération d'un nouvel Access Token à partir d'un Refresh Tokens se produit bien sans erreur (en se connectant par exemple sur Pix App avec le compte superadmin@example.net en ayant au préalable dans le .env configuré ACCESS_TOKEN_LIFESPAN=10s de manière à ne pas attendre trop longtemps la génération d'un nouvel Access Token à partir du Refresh Token)
  • Vérifier qu'aucun grant_type autre que password et refresh_token n'est autorisé et que le script ci-dessous renvoie une erreur 400 (remplacer XXX par le bon mot de passe) :
curl -X POST \
--data 'grant_type=wrongGrantType' \
--data 'username=superadmin@example.net' \
--data 'password=XXX' \
http://localhost:3000/api/token

@lego-technix lego-technix self-assigned this Dec 28, 2024
@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@lego-technix lego-technix marked this pull request as ready for review December 28, 2024 17:45
@lego-technix lego-technix requested a review from a team as a code owner December 28, 2024 17:45
@lego-technix lego-technix changed the title [TECH] Refactor better api token code and tests [TECH] Refactor better api token code and tests (PIX-15878) Dec 30, 2024
@lego-technix lego-technix force-pushed the refactor-better-api-token-code-and-tests branch from 7c01838 to 2c1a51a Compare December 30, 2024 10:56
@lego-technix lego-technix changed the title [TECH] Refactor better api token code and tests (PIX-15878) [TECH] Rendre le code lié à l'authentification des users plus clair et corriger un test erroné (PIX-15878) Dec 30, 2024
Copy link
Contributor

@EmmanuelleBonnemay EmmanuelleBonnemay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lu et testé avec succès

@pix-service-auto-merge pix-service-auto-merge force-pushed the refactor-better-api-token-code-and-tests branch from 2c1a51a to 23f9918 Compare December 30, 2024 15:38
@pix-service-auto-merge pix-service-auto-merge merged commit f00e1c9 into dev Dec 30, 2024
7 of 8 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the refactor-better-api-token-code-and-tests branch December 30, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants