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

[FEATURE] Déconnecter l'utilisateur s'il utilise un refresh token avec un scope incorrect (PIX-14237) #10101

Merged
merged 6 commits into from
Sep 19, 2024

Conversation

bpetetot
Copy link
Contributor

@bpetetot bpetetot commented Sep 11, 2024

🦄 Problème

Actuellement, si un refresh token n'appartient pas au scope de l'application, il est quand même accepté pour régénérer un access token.

🤖 Proposition

Utiliser le scope pour vérifier qu'il est le même que celui du refresh token.
Pour cela, il est nécessaire de mettre à jour la librairie ember-simple-auth en version 6.1.0 qui inclut un correctif permettant d'envoyer le scope dans le endpoint de refresh.

Pour plus de détails, voir la PR mainmatter/ember-simple-auth#2813

💯 Pour tester

⚠️ Pour réaliser ces tests, bien penser à ne pas avoir une configuration de Pix Api avec des valeurs de REFRESH_TOKEN_LIFESPAN trop petites, sinon vous aurez beaucoup de mal à pouvoir conclure quoi que ce soit. Aussi voici des valeurs conseillées pour ce test à mettre dans votre .env :

REFRESH_TOKEN_LIFESPAN_MON_PIX=7d
REFRESH_TOKEN_LIFESPAN_PIX_ORGA=7d
REFRESH_TOKEN_LIFESPAN_PIX_CERTIF=7d
REFRESH_TOKEN_LIFESPAN_PIX_ADMIN=7d
  1. Se connecter sur Pix Admin (avec superadmin)

    • Copier le refresh token depuis le local storage
  2. Se connecter sur Pix App (avec superadmin)

    • Remplacer le refresh token par celui de Pix App dans le local storage
    • Changer le timestamp d'access token dans le local storage avec un timestamp dans le passé pour l'expirer (ex: 1726065420825)

Vous devez être déconnecté 🎉

  1. Retourner sur Pix Admin
    • Changer le timestamp d'access token dans le local storage avec un timestamp dans le passé pour l'expirer (ex: 1726065420825)

l'access token doit être correctement renouvellé (voir s'il a changé dans le local storage)

@bpetetot bpetetot self-assigned this Sep 11, 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 :

Copy link

gitguardian bot commented Sep 11, 2024

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@bpetetot bpetetot force-pushed the pix-14237-upgrade-ember-simple-auth branch 3 times, most recently from e4ad21e to a822035 Compare September 11, 2024 18:19
@bpetetot bpetetot marked this pull request as ready for review September 11, 2024 18:19
@bpetetot bpetetot requested a review from a team as a code owner September 11, 2024 18:19
Copy link
Contributor

@er-lim er-lim left a comment

Choose a reason for hiding this comment

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

Test en RA sur firefox :

  • Scénario testé ✅
  • D'autres tests de non régression... ✅
  • Un scénario particulier testé :
    • se connecter sur pix-orga (donc génération token avec comme scope pix-orga)
    • enlever le champ scope du local storage et changer l'expires_at
    • un access_token est généré même sans scope. Semble OK par la rapport à la spec (voir la mention plus bas) ✅

Refresh tokens are issued to the client by the authorization server and are used to obtain a new access token when the current access token becomes invalid or expires, or to obtain additional access tokens with identical or narrower scope (access tokens may have a shorter lifetime and fewer permissions than authorized by the resource owner)

@bpetetot bpetetot force-pushed the pix-14237-upgrade-ember-simple-auth branch from a822035 to 498c6a8 Compare September 16, 2024 12:22
Copy link
Contributor

@mariannebost mariannebost 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é sur Chrome ✅

Copy link
Contributor

@lego-technix lego-technix 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é fonctionnellement avec succès avec Firefox

@bpetetot bpetetot force-pushed the pix-14237-upgrade-ember-simple-auth branch from 498c6a8 to 6ac16e8 Compare September 19, 2024 08:25
@bpetetot bpetetot added Tech Review OK Func Review OK PO validated functionally the PR 🚀 Ready to Merge and removed 👀 Tech Review Needed 👀 Func Review Needed Need PO validation for this functionally labels Sep 19, 2024
@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-14237-upgrade-ember-simple-auth branch from 6ac16e8 to b7537f3 Compare September 19, 2024 08:44
@pix-service-auto-merge pix-service-auto-merge merged commit ba30a47 into dev Sep 19, 2024
6 of 7 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-14237-upgrade-ember-simple-auth branch September 19, 2024 08:50
@bpetetot bpetetot changed the title [FEATURE] Déconnecter l'utilisateur s'il utilise un refresh token avec un scope incorrect [FEATURE] Déconnecter l'utilisateur s'il utilise un refresh token avec un scope incorrect (PIX-14237) Sep 20, 2024
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.

6 participants