Où vérifier un Bearer Token ?

Bonjour,

J’ai récemment eu une nouvelle dispute avec mon CTO (la deuxième en 1 mois¹).

Je travaille sur un projet PHP (Laravel) et devais mettre en place un QR Code redirigeant vers une page contenant un formulaire de vérification d’identité (sans mot de passe), qui, si le traitement se fait avec succès, renvoie au front un Bearer Token pour pouvoir requêter des fonctionnalités protégées. (Nous devons nous-mêmes gérer cela en mode "custom" et non avec une librairie comme Passport par exemple, pour des raisons techniques que je saute car hors-sujet).

J’ai donc créé un Middleware (abrév. "MW") en amont des routes de ces fonctionnalités protégées, chargé de laisser passer ou de refuser toute requête de ces fonctionnalités protégées en fonction de si le token envoyé est correct ou pas. Ce MW vérifie donc que le token envoyé par le front existe bien en base ET qu’il n’est pas expiré ET qu’il appartient bien à l’utilisateur envoyé par l’appli (cette dernière envoie le type d’utilisateur ainsi que son UUID, ces deux données étant retournées par le traitement du formulaire de vérification d’identité, en plus du bearer token).

Le code ressemble donc à cela :

$locataire = Locataire::query()->where('uuid', $request->uuid')->first();

$token = Token::query()->where('value', $request->value)->where('expi', '>', 'NOW()')->where('locataire_id', $locataire->id)->first();

if (!$token) {
   throw new AuthenticationException('Invalid token.');
}

Le problème selon le CTO et son "argumentation"

Selon lui, je cite, "ça n’a aucun sens" de mettre ça dans un middleware. (Il s’est une nouvelle fois montré très direct voire extrémiste dans ses propos²).

• Pour lui, ce n’est pas une vérification de sécurité qui a sa place dans le middleware car "c’est du requêtage métier" puisque je récupère un locataire (donc une donnée métier). Or d’après lui, aucun MW du projet ne récupère d’objets métier (ce qui est faux puisqu’il en a fait un lui-même).

• Dans le même ordre d’idée, selon lui, il faut que les MW fassent des trucs sans requêter les données métier de la DB, mais uniquement des données "core". (Idem nous avons pourtant des contre-exemples dans notre projet, notamment écrits par lui-même).

• Selon lui, il faudrait que je déplace les notions de locataire et de vérification "token<->locataire" dans chaque contrôleur, dans leur fonction authorize. Pourtant, la fonction authorize que nous déclarons dans les contrôleurs sert juste à établir des autorisations (= permissions à base de rôle du genre "tel utilisateur a le rôle de Lecteur de telle ou telle donnée donc il peut y accéder en lecture"). Moi ce que j’ai fait, ça relève de l’authentification : on vérifie que le token est bien associé au user envoyé par l’appli. C’est donc en amont de la notion d’autorisation.

• Autre argument qu’il avance :

S’il y avait un bug concernant l’appel des routes protégées à cause d’un souci d’exploitation du token ou quoi, où regarderaient les collègues, sachant que la logique "permissive" est historiquement écrite dans la fonction authorize des contrôleurs ?

Réponse que je lui ai donnée : bah vu qu’il s’agit du système de QR Code, on sait bien qu’il y a un MW de sécurité, et de plus on aurait probablement une erreur "Invalid token" ou équivalente. Donc la probabilité est grande pour qu’il regarde dans ce MW et non dans la fonction authorize des middlewares (sachant encore une fois que c’est de l’authentification et non de l’autorisation, de mon point de vue). Puis bon normalement on est censé regarder les back traces des logs d’erreur, c’est le point de départ de tout débogage…

• Dernier argument qu’il avance : là on vérifie le token<->locataire mais quid de quand on généralisera ce système à plein de types d’utilisateurs différents (propriétaires, agences, jsp quoi) ?

Ma réponse : alors déjà je ne savais pas du tout qu’on allait généraliser (ce à quoi il a répondu "il faut TOUJOURS penser à tout et TOUT anticiper", ce qui est en partie contraire au KISS mais passons). Mais de toute façon, comme l’appli envoie le type dans la requête, il suffirait de faire un truc du genre : $user = $request->type::query()->where('uuid', $request->uuid')->first(); au lieu de l’actuel : $locataire = Locataire::query()->where('type', $request->type)->where('uuid', $request->uuid')->first();. Eventuellement, en cas de type plus "complexe", il y aurait un switch (ou match) ou des conditions, ce à quoi il a répondu "Non mais ton MW va faire 10k lignes de long".

• Dernier-Ultime argument : "Non mais arrête de toujours tout remettre en question, tu es clairement têtu, tu dois te conformer à ce que nous avons décidé, je me suis pas fait chier à partir sur l’architecture du authorize pour qu’on fasse une exception ici, c’est tout" (au mot près à 90%). Je n’ai pourtant pas du tout l’habitude de le contredire, au contraire je me tais 90% du temps et je fais juste ce qu’il dit, 90% du temps je m’écrase.

Ma question

Cette vérif de sécurité "token <-> locataire/user" est-elle à faire dans le MW ou dans chaque contrôleur (méthode authorize ou bien méthode asController) ?

¹ : la première "dispute" (enfin j’y ai mis rapidement un terme en m’écrasant au plus tôt) : il voulait récupérer les données d’un jour donné, donc il a écrit : Truc::query()->whereBetween('date', '2022-01-01 00:00:00', '2022-01-01 23:59:59')->get(). Moi je lui ai dit que conceptuellement il manque la dernière seconde (la précision de stockage étant de l’ordre de la milliseconde), donc ma solution était : Truc::query()->where('date', '>=', '2022-01-01 00:00:00')->where('date', '<', '2022-01-02 00:00:00')->get(). C’est légèrement moins lisible mais conceptuellement vrai alors que son truc est légèrement plus lisible mais faux. Il m’a répondu qu’en base, on n’a pas de millisecondes et donc qu’il ne changera "de toutes façons" (je cite) pas sa solution pour la mienne.

² : il a en effet :

• Tendance à me prendre de haut et à être condescendant, à surestimer son avis par rapport au mien. Quand il m’explique son point de vue, c’est toujours sous la forme de questions. Par exemple pour le point " ¹ ", il n’a pas répondu "qu’en base, on n’a pas de millisecondes et donc qu’il ne changera pas sa solution pour la mienne". Mais à la place il a fait exprès pendant 20 minutes de me poser plein de questions en ce sens pour arriver à cette conclusion.

• Il a aussi l’insulte facile puisqu’il y a de nombreux mois il m’avait traité d’hypocrite devant le stagiaire, et hier d’hypocrite devant l’équipe tech, et de têtu avant-hier.

  • Hier il m’a traité d’hypocrite devant les techs parce que j’ai osé ouvrir ma bouche puisqu’il leur expliquait devant moi que j’avais commis une grosse bourde sur le MW en requêtant de la logique métier et que c’était une mauvaise direction technique qui ralentissait toute l’équipe etc etc. Bref il pourrissait mon dev et ma "mauvaise initiative" (je cite), sans leur expliquer précisément ce dont il s’agissait (donc l’authentification, à savoir vérifier que le token est bien associé au bon locataire, ce qui selon moi a toute sa place dans un MW et non dans les fonctions authorize/asController des contrôleurs). Quand j’ai essayé de me défendre devant les techs, il a essayé plusieurs fois de me faire taire en disant "on en a déjà parlé hier tous les deux donc stop" alors même que c’est lui qui a ramené le sujet sur la table.

Il a tendance à balayer d’un revers de mains mes arguments techniques, c’est assez fou, et à faire passer ses pseudo-arguments ("tu dois faire comme ça parce que j’ai décidé, arrête de tout contredire") pour des arguments. Quand il fait cela, il utilise des termes extrémistes du genre "ça n’a aucun sens".

Bref il ne supporte pas que je le contredise. Le pire c’est que je suis souvent là à défaire/refaire du dev car il oublie ce qu’il a dit quelques jours ou semaines auparavant (et il ne me croit pas quand je le lui dis) ! Je ne suis d’ailleurs pas le seul à avoir ce souci ! Bon à ce sujet moi aussi j’oublie parfois des trucs au niveau métier / implémentation technique par exemple, donc on est quittes

PS : j’ajouterais aussi qu’il disait aussi être "constamment contredit" par au moins trois développeurs qui ont quitté la boîte (soit eux-mêmes lors de leur période d’essai soit non-renouvellement de contrat pour le freelance). Je vous laisse libre interprétation de ce PS .

De façon générale il est vrai qu’un MW ne se préoccupe pas trop du métier, mais de la requête en soi. On peut éventuellement gérer l’authentification dans le MW (cette requête vient bien de X ) et ensuite laisser la partie métier gérer l’autorisation (X a le droit d’appeler cette action).

C’est bien ce que j’ai indiqué au CTO et c’est bien ce qui était réalisé avec mon dev.

Pour vérifier que la requête vient de X, il faut vérifier que le token envoyé se situe bien en base, et qu’il est associé à X, et qu’il n’est pas expiré. C’est de l’authentification. => Middleware. Il n’y a pas de logique métier ici. J’accepte de laisser passer la requête, ou non, en fonction de la validité du token. Mais de son point de vue, la vérification comme quoi le token est bien associé à X, c’est du métier, pas un check de sécurité, tout simplement parce que récupérer X depuis la db, c’est du métier (c’est lui qui dit ça hein, pas moi) car X est un objet métier (vu que c’est un Utilisateur (locataire)). (Ce qui contredit l’un de ses propres devs puisqu’il a déjà fait un MW où il réalise ça pour le coup).

La notion d’autorisation ne correspond pas à ça (elle correspond , par ex., à la présence de rôles pour l’utilisateur authentifié, rôles compatibles avec la feature du contrôleur). => à mettre dans le Contrôleur (méthode authorize par ex).

Ici, le CTO confond les deux.

Je suis également d’accord avec le reste de ton message. J’ai effectivement repris ma recherche de job et cette fois j’irai jusqu’au bout avec démission.

Pour la défense de mon CTO, l’équipe produit lui met la pression car ça fait 2 semaines que ces US de QR Code auraient dû être finies, mais :

• la semaine dernière j’étais en arrêt maladie pendant 3/4 jours car grippé

• la semaine d’avant, le CTO avait commencé à traiter mes PR qui étaient prêtes depuis 1 voire 2 semaines avant "cette semaine d’avant", car il était débordé. Cela fait donc 3 ou 4 semaines que j’ai terminé mes US (modulo les retours de PR type refactoring principalement même s’il y a eu 2 sujets un peu plus épineux mais qui ont été traités extrêmement rapidement par moi, i.e. : dans l’heure).

• cette semaine était particulière (séjour à Paris pour Noël).

Ce qui fait que là il est obligé de finir de traiter le reste de mes PR en peu de temps car on doit envoyer en prod les QR Code pour après Noël.

Le PO m’a dit (devant le CTO donc ce dernier est au courant) de ne pas hésiter à rester le soir si besoin car la livraison au 24 décembre est un impératif. Ce que je ferais, même si je compte quitter la boîte. (bon par contre pas sûr d’être chaud pour dépasser les 39h hebdo cette fois-ci donc je m’octroierai moi-même le droit d’éteindre l’ordi pour respecter ces 39h, tant que j’informe le CTO que j’ai fait des heures en plus pour finir le boulot si besoin est …… je sais d’avance qu’il va râler mais je m’en fiche cette fois-ci. Je compte m’affirmer.).

Je ne l’ai pas précisé dans le post mais je ne me suis pas lancé tout seul dans ce dev. Voici ce qui s’est passé :

1. Le CTO m’avait à l’origine donné une direction générale, essayer d’utiliser Passport avec des scopes dans le token (Passport = la lib d’authentification par Bearer JWT Tokens que nous utilisons). Je lui fais part de mes doutes car j’ai déjà utilisé Passport sur des projets perso et Sanctum dans ma boîte précédente, et j’ai pris une demi-matinée pour me renseigner, et je pensais qu’il y aurait des soucis concernant notre cas d’usage particulier. J’ai suggéré une solution custom plutôt.

2. J’essaie cependant d’utiliser Passport, mais je me rends compte que différents points techniques posent bien souci. Je vois que j’ai besoin d’aide. Donc je décide d’en parler au CTO, qui finalement me dit "non mais en fait je ne pense pas qu’il faille utiliser Passport".

3. Avec le CTO et le Lead Dev, nous regardons ça et décidons de partir sur une solution custom, confirmant les problèmes techniques que j’avais rapportés au CTO et qui concernaient l’usage de Passport pour notre besoin précis.

4. Nous décidons ensemble de partir sur une solution custom. Que nous concevons ensemble ; le dev en tant que tel me revient, donc je le fais. Il s’agit ici de générer le token, le sauvegarder en base sur le locataire, de faire un Middleware qui vérifie ce token pour pouvoir accéder aux routes du QR Code après que l’utilisateur a passé avec succès le formulaire de vérification de son identité.

   1. En revanche, je ne sais plus si le fait de vérifier que l’utilisateur requêtant l’app est bien le porteur du jeton a été décidé tous ensemble, ou si c’est moi tout seul qui y ait pensé et qui ait voulu le faire dans le Middleware et non dans le contrôleur.

Sur la question de base « Où vérifier un jeton au porteur ? »

Oui mais en fait les deux liens que tu donnes, dans notre cas précis, ne sont pas utilisables (ni autorisation ni authentification).

Mais sinon si c’est le plus tôt possible bah c’est bien dans le middleware et non dans le contrôleur, mais le CTO ne veut pas de ça, cf ses arguments. Son argument-phare étant que vérifier le porteur du jeton c’est de la logique métier de type autorisation, et non de la logique non-métier de type authentification…

Sur la question des accès à la BDD

Merci, je ne connaissais pas, ni mon Lead ni mon CTO, c’est pour ça qu’on n’a pas été vers ça.

Sur la question des heures supplémentaires

Est-ce que c’est toujours la même entreprise que dans tes derniers messages ?

Je vais essayer de causer le moins possible de tensions, déjà que c’est désormais tendu avec le CTO… C’est dommage car il y a quelques semaines encore, je finissais leS sprintS en avance (y avait toujours une tâche en retard mais c’est dû au fait qu’on m’en rajoutait, car je terminais vraiment mes tâches plus tôt que prévu, je le sais car je les consignais dans mon Agenda Google et je mettais leur durée de dév sur Jira).

Mais sinon oui je suis en train de me former au Java pour étendre drastiquement ma recherche d’emploi ! Un ami datant de la fac veut me pistonner. C’est sûr que je vais pas rester dans cette start-up désormais !

Le mec qui me traite d’hypocrite devant les autres, ou qui se permettrait de le faire au sujet d’un collègue perso je pense qu’il se prendrait rapidement un petit message aux RH sur son comportement, voire au CSE s’il y en a un. On a le droit d’être un con, ça empêche pas d’être poli et respectueux.

Et sinon, vous allez vraiment faire une livraison un 24 décembre ? Sa croit si fort au père Noël chez vous ?

En fait il ne m’a pas traité d’hypocrite (juste de têtu sur Slack en privé). Il a dit un truc du genre "non mais faut arrêter l’hypocrisie" (une fois il y a longtemps en présence du stagiaire et hier matin devant l’équipe tech).

Oui moi je dois absolument livrer le truc avant le 24 (mais en fait je suis en attente de mon CTO depuis au moins 2 ou 3 semaines, moi je traite ses quelques retours très rapidement et ensuite on merge quoi, donc j’ai envie de dire, "j’y peux rien si ça met du temps…" - sauf que si je dis ça, je sais que je vais me faire pourrir). Edit : normalement il n’y aura pas de MEP à Noël mais en janvier, début janvier.

Du coup ça va bien dans mon sens, tu fais comme moi d’ailleurs (verif du porteur du token placée dans le middleware).

Et tu as raison, avec la solution de mon CTO (que j’ai bien entendu appliquée puisqu’il m’y force - je n’ai pas de souci avec ça, c’est sa boîte pas la mienne), il a fallu que je duplique la vérification du porteur du jeton dans chaque contrôleur…

Concernant ton paragraphe sur les permissions (guards policiers authorize) je fais comme toi aussi, donc par exemple : est-ce que l’agence de l’employé est la même que celle du contrat auquel il tente d’accéder et est-ce qu’il a le rôle RENT_READER ?

Le CTO a aussi proposé de mettre la vérif du porteur du token dans le asController au lieu du authorize (le asController est la méthode appelée après les middlewares et après les guards policies et après le authorize donc c’est encore pire). Je reprécise que sa première demande était de mettre ça dans le authorize.

Je n’ose plus rien lui dire perso. Mais oui sinon je suis bien d’accord avec toi !