Hello,
une 1ère mise en application de l’idée partagée plus haut :
1 « J'aime »
Bonjour
Mon avis :
- soit on définit les dossiers directement maintenant un public et un download et on ne laisse pas le choix au dev : c’est moins flexible mais ce demande moins de boulot côté core et c’est plus simple a sécuriser
- soit dans l’idée de ta fonction (mais dans l’autre sens) on défini un champs dans le json.info ou un attribut de class qui donne les dossier public : c’est plus souple mais ça va être nettement plus lourd a déployer
1 « J'aime »
Salut,
Moi je trouve l’option deux plus intéressante: gardons la souplesse et ca sera plus « lourd » uniquement coté plugin. Et en même temps le boulot sera pas dingue, de toute façon on doit faire qlqch pour s’adapter à la sécu, donc juste déclarer la liste des dossiers dans le info.json (ou en attribut, mais est-ce la bonne place?) ca reste très facile.
Et d’expérience (professionnelle), sur le long terme, le niveau sécu du code sera plus élevé si le développeur doit faire explicitement qlqch (déclarer ses dossiers publics explicitement) plutôt que d’avoir des comportements par défaut (où par défaut on ouvre); c’est toujours plus safe de travailler en mode « liste blanche ».
Par contre coté vérification, peut-être que je pousse un peu mais je limiterais un peu plus dans le cas ou c’est une apikey d’un plugin: on ne devrait en principe n’accéder qu’aux dossiers publics du plugin et pas ceux des autres plugins, on ne pourrait accéder à tous les dossiers publics de tous les plugins uniquement en tant qu’utilisateur (via IHM ou via apikey); mais c’est plus complex à gérer
edit: et en fait question nommage, on ne devrait pas appeler ca un « dossier public » mais « download folder » si j’ai bien suivi l’idée de Loic qui voudrait p-e rajouter un « vrai » dossier public à un moment? (a voir pour quel usage?)
1 « J'aime »
Ok je vois si c’est ce que vous voulez ca me vas, par contre pour faire ca ca va me prendre quelques mois pas sur de pouvoir l’integrer en 4.2… C’est pour ca que je proposais l’autre solution.
Dans tous les cas il va avoir un dossier public c’est pour tout ce qui doit etre accessible sans aucun restriction (demande de certains dev)
nouvelles modifs dans PR dispo du coup 
je suis donc parti sur le fichier info.json, où ca me semble plus cohérent d’avoir qlq chose ici qu’un attribut dans la classe qui hérite de eqLogic.
il faudra donc ajouter une clé whiteListFolders dans le fichier qui est un tableau de chemin « public » pour le plugin :
"whiteListFolders": [
"/data/img",
"/data/configs",
"../data/../img2",
"/pouet/coco"
],
(dans cette exemple, seuls les 2 premiers items seront réelement pris en compte : le ../../ sera transformé en /img2 qui n’existe pas dans le plugin et le /pouet/coco n’existe pas non plus)
j’ai initié quelque chose aussi
mais de base la fonction apiAccess n’accepte que les clé api d’utilisateur OU celle du core, donc les qlq lignes ne servent à rien dans l’immédiat
à moins d’ajouter l’autorisation des clé api des plugins
Ok je vois alors :
- faudrait faire le pr en alpha (c’est du détails)
- si c’est un plugin qui arrive avec une clef api il faut qu’il le précise dans l’url 'ex plugin=toto), comme pour tous les appels api en faite. Ca permet de pas a avoir a chercher le plugin et d’etre sur que c’est bien sa clef api qui est utilisé
- pour la partie plugin info faut utiliser la class (faut rajouter dans celle-ci la nouvelle clef whiteListFolders), en plus tu n’auras pas besoin de parcourir tous les plugins vu que que le plugin passe un paramètre plugin lors de l’appel
- faut vérifier que le plugin est bien dans son répertoire et pas celui d’un autre je pense
Merci en tout cas pour le boulot fait
voilà Monsieur !
fait ! 
ba évidemment, jsuis c*n !.. 
c’est corrigé !
déjà le cas !
j’ai fait qlq tests, un peu dans tous les sens, ça a plutôt l’air d’être OK.
je me suis également permis de mettre qlq logs (dans api, uniquement en debug) pour avoir qlq infos, au cas où …!
Je pense tu peux simplifier avec un truc du genre :
if(!isConnect() && !jeedom::apiAccess($apikey, init('plugin')){
throw new Exception(__('401 - Accès non autorisé', __FILE__));
}
Si plugin est fournis il va chercher pour la clef api sinon il va chercher pour la clef api core ou user.
c’est l’inverse, il commence par le user, puis passe au plugin si indiqué sinon core.
c’est plus « simple » en effet, mais là ca permet d’avoir une log distinct et voir où ca ne passe pas, d’où le split
Oui mais avoir 2 passage different c’est un risque de faille de securité multiplié par deux…
c’est fait !
Merci j’ai fait le merge, je vais surement retravailler 2/3 trucs mais juste de la mise en forme.
Merci pour ton aide
top !
par contre il ne faudra pas oublier de préciser que c’est un control strict des paths (en tout cas sur ce que j’ai fait)
donc avec en whiteList /data/img (pour les non admin & api plugin) :
- accéder à
/data/img/photo.png
- accéder à
/data/img/custom/photo.png
(les admins ont eux accès à tout ce qui est autorisé par les .htaccess)
autre question : quid des accès aux répertoires du core ? est ce qu’il y a un équivalent de info.json ?
(faudra également penser à modifier le plugin template)
Salut,
Voila j’ai poussé des modifications, normalement j’ai rien cassé mais faut verifier quand meme.
J’ai juste un soucis avec ca :
$current = realpath($rootPath . '/' . getAbsolutePath($folder));
A mon avis getAbsolutePath ne sert a rien avec le realpath qui va tout remettre en absolue
eh ben non ! justement … c’est là où ca sert à être sur de rester dans le répertoire du plugin !
avec le pluginId ‹ monPlugin ›, on obtient en $rootPath = /var/www/html/plugins/monPlugin
deux exemples :
$folder = '../../data/img'
$current1 = realpath($rootPath . '/' . getAbsolutePath($folder));
$current2 = realpath($rootPath . '/' . $folder);
echo 'with getAbsolutePath => ' . $current1;
echo 'remove getAbsolutePath => ' . $current2;
dans ce cas :
with getAbsolutePath => /var/www/html/plugins/monPlugin/data/img
remove getAbsolutePath => /var/www/html/data/img
on évite que le plugin définisse un autre répertoire qui ne lui « appartient pas »
ca me semble donc au contraire essentiel (enfin d’un point de vue « sécurité » !)
Ok dans ce cas un :
$folder = ‹ …/…/data/img ›;
$folder = str_replace(array(‹ …/ ›,‹ /… ›,‹ ./ ›,‹ /. ›),‹ ›,$folder);
Ne serait pas plus simple ?
Up to you.
Mais tu n auras pas le meme resultat avec ../data/../img
ca va le transformer en
data/img
Donc pour moi ca semble le meme resultat et on evite une fonction en plus avec une boucle.
ba non ! getAbsolutePath('../data/../img') donne img, donc différent de data/img
à la limite on peut faire un truc bcp plus simple, et bcp plus secure histoire d’éviter d’avoir un mauvais dossier en whiteList parce que le développeur pensait pouvoir remonter dans l’arborescence.
Il est censé connaitre son plugin, donc pourquoi pas simplement skipper les items qui auraient ‹ …/ › :
if (strpos($folder, '../') !== false) continue ;
$current = realpath($rootPath . '/' . $folder);
1 « J'aime »
La je suis pour oui c’est plus simple pour tout le monde par contre je ferais plus violent pour etre sur :
if (strpos($folder, '..') !== false) continue ;
2 « J'aime »