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

Mise à jour majeure oauth2 to 2.1 #3047

Merged
merged 7 commits into from
Apr 24, 2023
Merged

Mise à jour majeure oauth2 to 2.1 #3047

merged 7 commits into from
Apr 24, 2023

Conversation

fchabouis
Copy link
Contributor

@fchabouis fchabouis commented Mar 22, 2023

J'ai suivi https://github.com/ueberauth/oauth2/blob/master/CHANGELOG.md et j'ai testé en local qu'on arrive toujours à se connecter via data.gouv.fr, ça a l'air de fonctionner.

EDIT @thbar: j'ai pu adapter les tests en conservant la donnée des cassettes, "juste" pour que ça passe, en utilisant le Mock fourni.

Ça ajoute une dépendance à Tesla obligatoire (wrapper HTTP sur différents clients), j'espère qu'on ne le regrettera pas plus tard 😄 selon comment sa maintenance évolue, mais on n'a pas trop le choix en l'état.

Voir:

Changelogs

@fchabouis fchabouis requested a review from a team as a code owner March 22, 2023 17:21
@fchabouis
Copy link
Contributor Author

à rediscuter avec plus de fraicheur dans la tête, les tests fonctionnent avec la maj grâce à des cassettes qui ne correspondent plus forcément à la réalité. Je ne suis pas sûr de ce qu'ils testent réellement.

@AntoineAugusti
Copy link
Member

@fchabouis Ping moi si tu veux en parler à un moment

@thbar
Copy link
Contributor

thbar commented Apr 18, 2023

@fchabouis j'ai pu corriger les tests. Cela dit après vérification du "coverage", la partie authentification oauth en tant que telle n'est je crois pas testée du tout si je ne me trompe pas (/cc @AntoineAugusti), du coup il faut se fier à nos tests "réels" pour le moment. Les tests que j'ai corrigé utilisent des cassettes pour des opérations non auth, avec un token qui est passé dedans.

D'où l'importance aussi de corriger:

@thbar
Copy link
Contributor

thbar commented Apr 24, 2023

Je déploie sur prochainement qui est réparé.

@thbar
Copy link
Contributor

thbar commented Apr 24, 2023

Ça a l'air de fonctionner (j'ai fait un login / logout).

Copy link
Contributor

@thbar thbar left a comment

Choose a reason for hiding this comment

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

J'ai pu tester sur "prochainement", ça a l'air de fonctionner.

@thbar thbar added this pull request to the merge queue Apr 24, 2023
Merged via the queue into master with commit 66f1691 Apr 24, 2023
@thbar thbar deleted the major_update_oauth2 branch April 24, 2023 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants