MedShake / MedShakeEHR-base

Base pour MedShakeEHR
GNU General Public License v3.0
36 stars 21 forks source link

Routes vides dans index.php #135

Open bugeaud opened 1 year ago

bugeaud commented 1 year ago

Bonjour,

De nombreuses traces d'avertissements de tentative d'accès à un table sur un booleen sont présentes dans les logs :

[php:warn] [pid 453] [client 192.168.1.58:39492] PHP Warning: Trying to access array offset on value of type bool in /app/MedShakeEHR-base/public_html/index.php on line 122, referer: http://host/patient/2746/

J'ai rajouté avant cette ligne à l'emplacement https://github.com/MedShake/MedShakeEHR-base/blob/master/public_html/index.php#L122 (induisant un décallage du n° de ligne de +6 ensuite dans les traces)

if(empty($match)){
    error_log("Variable match vide, routes login:" . var_export( msSystem::getRoutes(['login']),1) . " trace=" . print_r(debug_backtrace(), true));
}else{
    error_log("Variable match =". var_export($match,1) );
}

pour permettre de tracer ce qui se passe comme différence de condition entre le cas ou un tableau de route configurées correspondant à la demande est retournée et quand une valeur retournée est vide (bool ?).

Exemple de trace obtenues

[Mon Jan 02 16:27:20.978169 2023] [php:notice] [pid 1200] [client 192.168.27.70:49558] Variable match =array (\n  'target' => 'login/logIn',\n  'params' => \n  array (\n  ),\n  'name' => NULL,\n), referer: http://host/patients/
[Mon Jan 02 16:27:25.943673 2023] [php:notice] [pid 1200] [client 192.168.27.70:49558] Variable match vide, routes login:false trace=Array\n(\n)\n, referer: http://host/patients/
[Mon Jan 02 16:27:25.943776 2023] [php:warn] [pid 1200] [client 192.168.27.70:49558] PHP Warning:  Trying to access array offset on value of type bool in /app/MedShakeEHR-base/public_html/index.php on line 128, referer: http://host/patients/
[Mon Jan 02 16:27:25.943807 2023] [php:warn] [pid 1200] [client 192.168.27.70:49558] PHP Warning:  Trying to access array offset on value of type bool in /app/MedShakeEHR-base/public_html/index.php on line 128, referer: http://host/patients/
[Mon Jan 02 16:27:25.943831 2023] [php:warn] [pid 1200] [client 192.168.27.70:49558] PHP Warning:  Trying to access array offset on value of type bool in /app/MedShakeEHR-base/public_html/index.php on line 128, referer: http://host/patients/
[Mon Jan 02 16:27:25.979956 2023] [php:notice] [pid 1200] [client 192.168.27.70:49558] Variable match =array (\n  'target' => 'login/logIn',\n  'params' => \n  array (\n  ),\n  'name' => NULL,\n), referer: http://host/patients/

La 1e ligne est une trace qui semble correcte, la seconde un exemple de trace erronée car il n'y a pas de trace de routage pour login dans un contexte d'appel direct.

Il semble bien que dans certains cas un simple msSystem::getRoutes(['login']) varie dans sa réponse, ce qui produit que

    if(msConfiguration::getDefaultParameterValue('optionGeActiverApiRest') == 'true') {
      $match = msSystem::getRoutes(['login', 'apiRest']);
    } else {
      $match = msSystem::getRoutes(['login']);
    }

peut retourner 3 cas : les routes pour login, les routes pour login&apiReste ou rien (bool?).

Je ne vois pas dans le code ce qui peut induire cette différence. Les fichiers yaml de route sont statiques (aucun changements entre deux appels). il n'y a pas de routes ajoutées par un module installées.

Une idée ?

MedShake commented 1 year ago

msSystem::getRoutes() ne fait rien d'autre que de renvoyer le résultat de la méthode match() d'Altorouter qui elle-même ne renvoie théoriquement qu'un array ou false : https://github.com/dannyvankooten/AltoRouter/blob/20674b89537080c2257fa69fe766ea7d5d1721cb/AltoRouter.php#L254

Donc effectivement il y a le coup où le retour est false à gérer, car sinon en 122 sur index.php, ça coince.

Par contre en 122, on est a priori dans un cas où l'identification n'est pas présente. Il n'est donc pas logique que les logs produits donne /patients/ en referer ?

On peut donc, il faut donc, gérer le cas ou match est false en 122, mais il faudrait savoir si cela se produit par accident (entrée de l'url sans identification active) ou si ces appels viennent d'une situation d'utilisation normale.

B.

bugeaud commented 1 year ago

J'ai au moins deux cas:

MedShake commented 1 year ago

Je propose en 122 d'ajouter préalablement :

    if($match === false) {
        msTools::redirection('/login/');
    }

Ca ne répond pas à l'origine mais ca traite déjà la réponse à donner.

MedShake commented 1 year ago

Sinon il n'y a logiquement pas 36 scripts qui attaquent en arrière-plan. De mémoire, il n'y en a qu'un seul même : le rafraîchissement de la liste des patients en salle d'attente. Il peut tourner alors même que le menu n'est pas activé. Sinon le plus simple est d'inspecter la page et les appels réseaux. post chargement initial.

bugeaud commented 1 year ago

C'est étrange. Pourquoi un refresh qui dispose du même contexte d'auth dans le user agent tomberait sur ce cas s'il n'y a pas de problème sur le côté serveur aussi ? Je vais tester cet aprem le contournement qui me semble de toute façon nécessaire pour éviter un chemin indésirable mais celà risque de produire des effets de bords si les appels bloqués étaient légitimes.

MedShake commented 1 year ago

C'est bien toute la question effectivement :-) J'ai testé en local ici et pas d'effet de bord visible. Appliqué ce contrôle permettra peut-être de faire sortir la chose plus ouvertement. Toute façon il s'avère nécessaire à terme dans une prochaine version consolidée.