OAuth Provider #4749
OAuth Provider #4749
Conversation
️✅ 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. 🦉 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. |
victormours
force-pushed
the
oauth-provider
branch
3 times, most recently
from
4457543
to
5d084e2
Compare
victormours
force-pushed
the
oauth-provider
branch
from
0c91a46
to
531860d
Compare
There was a problem hiding this comment.
commentaire edité car posté trop vite, woups!
félicitations pour cette PR monstre @victormours 👍
la description est limpide et c’est parfait de pouvoir se projeter aussi loin que tu l’as déjà fait dans les étapes à venir.
Je n’ai pas formellement accepté la PR puisqu’elle n’est pas vouée à être mergée mais plutôt qu’on va en extraire des PRs successives.
Je n’ai aucun retour bloquant.
Je n’ai pas testé en local, je n’ai pas cherché à le faire je me dis que ça pourrait être plus efficace de le faire tous les deux en synchrone et éventuellement le documenter.
Dans la description de la PR je n’ai pas compris cette phrase :
L'introspection de tokens ne fonctionne pas parce que le token qui authentifie la requête doit être différent de celui qui est introspecté)
|
|
||
| Ci-dessous le processus d'autorisation par lequel un agent de demarches-simplifiees.fr lie son compte à RDV-SP : | ||
|
|
||
| ```mermaid |
There was a problem hiding this comment.
un grand merci à @francois-ferrandis qui a rédigé cette doc 😍
There was a problem hiding this comment.
C'était rigolo et je suis fier d'avoir mon nom au générique. 🍿
lib/omniauth-rdv-service-public/lib/omniauth/strategies/rdv_service_public.rb
Outdated
Show resolved
Hide resolved
Co-authored-by: Adrien Di Pasquale <[email protected]>
Co-authored-by: Adrien Di Pasquale <[email protected]>
| # Use DeviseTokenAuth | ||
| authenticate_api_v1_agent_with_token_auth! | ||
| else |
There was a problem hiding this comment.
Dommage qu'on ait pas de moyen de déterminer quand il faut tenter une auth Doorkeep, ça signifie qu'on va tenter cette auth par défaut, pour toutes les requêtes qui ne passent pas dans les if / elsif. C'est pas forcément un problème, je vérifier aussi que j'ai bien compris.
J'ai initialement été surpris que le if doorkeeper_token n'ait pas de else, mais en fait le else est explicite : si on a pas de doorkeeper_token, c'est que le doorkeeper_authorize! a échoué et a déjà appelé un render d'erreur, c'est ça ?
| @@ -4,4 +4,8 @@ def index | |||
| agents = agents.joins(:organisations).where(organisations: { id: current_organisation.id }) if current_organisation.present? | |||
| render_collection(agents.order(:created_at)) | |||
| end | |||
|
|
|||
| def me | |||
| render_record current_agent | |||
There was a problem hiding this comment.
On dévoile l'attribut inclusion_connect_open_id_sub de l'agent, c'est OK (je pense que oui) ?
There was a problem hiding this comment.
je crois que oui (et on le fait déjà par ailleurs)
| return "https://#{ENV['FRANCECONNECT_HOST']}/api/v1/logout" \ | ||
| if @connected_with_franceconnect | ||
| if @connected_with_franceconnect | ||
| return "https://#{ENV['FRANCECONNECT_HOST']}/api/v1/logout" | ||
| end |
| class FakeOauthClient < Sinatra::Base | ||
| use OmniAuth::Builder do | ||
| provider :rdv_service_public, "fake_app_id", "fake_app_secret", | ||
| scope: "write", base_url: Capybara.app_host |
There was a problem hiding this comment.
La cerise sur le gâteau (dans une autre PR) serait de pouvoir utiliser cette app Sinatra en dev, mais j'y suis pas arrivé en moins de 15 minutes (la route http://127.0.0.1:4567/omniauth/rdvservicepublic n'est pas définie, je ne sais pas pourquoi).
There was a problem hiding this comment.
oui, j'ai eu des difficultés à faire ça moi aussi. J'en suis arrivé à la conclusion que le préfixe omniauth n'est pas exactement le même (la route devient http://127.0.0.1:4567/auth/rdvservicepublic. Je suis pas sûr exactement de l'explication, et omniauth n'aide vraiment pas à débugger ça.
There was a problem hiding this comment.
Trop cool @victormours 🙏 🚀 !
J'ai testé le flow avec rdv-i ça marche nickel :).
Je t'ai laissé des mini remarques, je te laisserai juger si elles sont pertinentes ou non vis à vis de vos pratiques.
lib/omniauth-rdv-service-public/lib/omniauth/strategies/rdv_service_public.rb
Show resolved
Hide resolved
| ' En continuant, vous allez permettre à <b>#{@pre_auth.client.application.name}</b> d'accéder à votre compte <b>#{current_domain.name}</b> | ||
| ' lié à l'adresse #{current_agent.email} | ||
|
|
||
| - @pre_auth.scopes.each do |scope| |
There was a problem hiding this comment.
Pareil ici peut-être que de la documentation ou un lien quelque part vers ce qu'est @pre_auth (sur la classe par exemple) peut être utile
Contexte : l'intégration à Démarches Simplifiées
Dans le cadre de notre financement par le FTAP, on a prévu de faire une intégration avec Démarches Simplifiées pour permettre à l'instructeur d'un dossier sur DS de proposer un rendez-vous à l'usager qui a envoyé le dossier.
Cette intégration nécessite que Démarches Simplifiées appelle notre api, par exemple vérifier l'existence de plages d'ouverture, ou pour envoyer l'invitation (une fois qu'on aura un endpoint qui permettra de faire cette action).
Voir la maquette de l'intégration : https://www.figma.com/board/Vuy7UCLaLWpoY1u30AEaKG/Piste-d'int%C3%A9gration-RDV-%26-DS?node-id=0-1&node-type=canvas&t=eQKAZwYgYYkh8ERT-0
Piste d'intégration RDV & DS.pdf
Le problème, c'est que l'authentification à notre api actuelle ne fonctionne pas très bien pour ce cas d'usage. On a deux modes d'authentification :
devise_token_auth: l'endpoint/api/v1/auth/sign_inpermet d'envoyer un email/mot de passe et d'obtenir un token d'api en échange. Si on utilisait ce mécanisme pour l'intégration à DS, il faudrait demander aux agents d'entrer leur email/mot de passe de RDV Service Public sur le front de DS, ce qui n'est pas acceptable (c'est la pire dé-sensibilitation au risque de phishing qu'on puisse imaginer). Cela ne permet pas non plus d'authentifier les agents qui utilisent ProConnect)app/controllers/api/v1/agent_auth_base_controller.rb:95), et ajoute ce hash en header des requêtes. C'est ce qu'utilise RDV Insertion pour les agents qui utilisent ProConnect. Le problème de cette approche est qu'il n'y a pas vraiment de vérification que l'agent utilisé a bien donné la permission à ce qu'on utilise l'api en son nom, et ça permet à l'application cliente de lire et modifier des données pour tous les agents de l'application. C'est déjà discutable pour RDV Insertion, et pas généralisable à d'autres applications (impossible à justifier d'un point de vue RGPD).On a donc besoin d'un autre mode d'authentification des agents à notre api, et on a la chance d'être dans un cas très classique pour lequel un standard a émergé : l'OAuth.
La place de l'oauth dans cette intégration
En proposant aux agents qui utilisent Démarches Simplifiées et RDV Service Public de connecter les deux applications par Oauth, on permet à la fois de fournir un mode d'authentification viable, et d'expliciter l'intégration aux agents.
Le fait de rendre notre application visible à l'agent permet aussi d'éviter les demandes qu'on a eu de la part de certains partenaires potentiel de cacher toute notre application derrière leur front et de recoder toute notre application en api.
Le parcours d'OAuth permettra à Démarches Simplifiées de récupérer un token d'api au nom de l'agent.
Le plan d'ouverture de l'oauth
C'est un gros projet, donc on va le découper :
MVP : Mise en place du parcours d'OAuth pour RDV Insertion
Pour cette première étape, on ne change pas l'authentification à l'api, mais on met en place le parcours d'oauth pour les agents.
Ça nous permet directement de remplacer la manière dont certains agents se connectent à RDV Insertion en entrant leur email/mot de passe RDV Solidarités sur le front de RDV Insertion. Cette PR chez RDV Insertion permettra de faire le changement une fois qu'on aura déployé l'oauth sur RDV Service Public gip-inclusion/rdv-insertion#2426
Critère d'acceptation (aka "definition of done") :
V1 : Authentification à l'api par token récupérés pendant l'Oauth par RDV Insertion
Une fois que les agents seront authentifiés par Oauth, on pourra commencer à utiliser les token pour les appels faits par RDV Insertion, et à voir comment on gère les refresh_token.
A partir de ce moment, on pourra aussi désactiver le bouton ProConnect sur RDV Insertion, puisque la connexion à ProConnect pourra se faire directement sur RDV Solidarités.
Réutilisation pour Démarches Simplifiées
Une fois que ça marchera pour RDV Insertion, ça devrait être réutilisable à l'identique pour commencer à travailler sur l'intégration avec DS
Le périmètre de cette PR dans ce plan
Le but de cette PR est de permettre des premiers tests utilisateurs du parcours d'OAuth avec RDV Insertion. Elle se concentre plutôt sur les aspects techniques, et le but est donc de valider les différents choix qui sont faits ici.
NB : Pour faire les strong migrations correctement, on ne peut pas merger directement cette PR. On va d'abord extraire la migration de création des tables dans une autre PR avant de merger celle-ci
La mise en prod du code de cette PR
Plan de mise en ligne :
forcedans l'urlSolution :
Rappel sur l'oauth https://auth0.com/docs/get-started/authentication-and-authorization-flow/authorization-code-flow
Choix d'implémentation majeurs
Doorkeeper
On utilise la gem doorkeeper pour l'implémentation. Ça semble être un standard, et gitlab s'en sert aussi. On a essayé d'en faire l'usage le plus simple possible.
gem omniauth dans lib
endpoint de
/meDe manière similaire à ce que fait Github, on propose un endpoint
/api/v1/agents/mepour les infos de l'agent connecté. C'est la manière la plus standard que j'ai trouvé pour faire ça. L'introspection de tokens ne fonctionne pas parce que le token qui authentifie la requête doit être différent de celui qui est introspecté)Choix de configuration de doorkeeper
Choix d'implémentation mineurs
Tester
En local, on peut directement tester avec RDV Insertion en étant sur la bonne branche et en relançant les seeds.
Scénarios à tester dans le navigateur
Prochaines étapes après cette PR
Tests utilisateurs et amélioration de la page pour accepter l'autorisation
skip_authorizationdans l'initializerHors scope