Problème avec authentification OAuth2 en 4.2, fonctionne en 4.1

Bonjour,
Un utilisateur a reporte un problème sur mon plugin strava pour l’authenfication OAuth2 en 4.2.7.
L’erreur est générée dans la callback appelée par le framework OAuth2, avec la fonction « isConnect() » qui retourne false, alors que je suis connecte a jeedom.
J’ai supprime cette partie de code, et ensuite, lors du’une nouvelle requête, c’est l’identification de session est incorrect.
Je n’ai pas ce problème en 4.1, et donc je pense que c’est lie aux mises a jour sécurité de la 4.2.

Par ailleurs, j’ai fait le test avec le plugin jeedom « google calendar », qui utilise aussi le même framework d’authentification, et j’ai exactement le même problème.

Du coup, je pense que c’est un problème plus général que mon simple plugin. Est-ce que vous quelqu’un aurait une piste, ou des debug supplémentaire a me fournir pour m’aider a solution ce problème.

merci par avance
Benoit

Salut,

Tu as du code à partager?
Sais-tu quel flow OAuth tu suis?
Un diagram de séquence ou tout autre doc?
Un log?

Là, sans rien, ca va ête compliqué :slight_smile:

bonjour Mips, merci pour ta reponse:

Pour google calendar : https://doc.jeedom.com/fr_FR/plugins/organization/gCalendar/

Pour strava :
le framework utilise est https://github.com/thephpleague/oauth2-client

pour la doc: Mentions Légales | strava
le code est public dans : https://github.com/benoit5672/plugin-strava
il s’agit du fichier core/php/authorization.php

Au niveau du flow:
1/ l’authorisation est demande a strava
2/ la page d’acceptation des droits est ouverte sur jeedom
3/ la callback authorization.php appelee sur jeedom en fin d’authorisation, et elle genere l’erreur.
4/ l’application est bien enregistree dans strava, mais pas dans jeedom

c’est cette partie de code qui ne fonctionne plus :

....
if (!isConnect()) {
	echo 'Vous ne pouvez appeler cette page sans être connecté. Veuillez vous connecter <a href=' . network::getNetworkAccess('external') . '/index.php>ici</a> avant et refaire l\'opération de synchronisation';
	die();
}

puis si je supprime le isConnect, ce test ne fonctionne pas non plus (« Invalid State »,

// Check given state against previously stored one to mitigate CSRF attack
//
if (empty($_GET['state']) || ($_GET['state'] !== $_SESSION['oauth2state'])) {

    log::add('strava', 'error', __('Session invalide : _GET=' . $_GET['state'] . ', _SESSION=' . $_SESSION['oauth2state'], __FILE__));

    unset($_SESSION['oauth2state']);
    exit('Invalid state');
}

Benoit

Bonjour,

Ma réponse va pa plaire mais c’est normal c’est une sécurité si tu viens d’un site externe de jeedom tu ne peux pas arriver connecter. Ca évite les liens dans les mails qui te ferais ouvrir ton jeedom si tu cliques dessus sans faire exprès par exemple.

Merci Loic.

mais tu confirmes donc que c’est une différence entre la version 4.1 et 4.2, suite aux améliorations liées a la sécurité ?

Benoit

oui c’est ce que je dis

aie… tu as raison, pas rigolo comme réponse :slight_smile:

A mon tour d’en annoncer une qui ne va pas plaire : , vous avez du coup le même problème dans le plugin google calendar.

je vais voir comment fixer çà de mon cote.

Oui oui on l’a, malheureusement j’ai pas de solution pour le moment et pas le temps de regarder de toute facon.

1 « J'aime »

Moi j’ai dans un plugin le même flow (authorization code grant flow), j’ai le même genre de page.
Le state n’est pas dans la session mais dans le cache, avec une validité limité. J’ai mis 10min, pas plus que le temps pour l’utilisateur de se log sur le système distant.

Et dès qu’un appel est reçu sur cette page, je supprime le state du cache, qu’il corresponde à celui reçu dans la requête ou pas ainsi pas de replay possible.

Après je fetch le token etc et je redirige le user sur la page de config aussi

mais donc jamais je ne vérifie si le user est authentifié sur jeedom → le controle sur le state, ainsi que la validité du code d’authorization suffit à trust le token que tu recois, que le user ait une session valide sur jeedom va pas changer grand chose, de toute façon cette page ne permet rien sur jeedom

@Loic, un flow OAuth c’est quand même assez courant (et de plus en plus), on ne peut qu’encourager les devs à l’utiliser
On pourrait pas déclarer dans le json du plugin une page sur laquelle les redirection externe sont autorisées ainsi on déclare explicitement la page de callback? cela ne sera que plus sécure si en plus on peut valider que le user est connecté.

3 « J'aime »

Faut voir avec jeedom SAS, pour rappel je ne suis pas decideur, ni rapporteur ni rien en faite donc je ne peux pas prendre ce genre de decision. Et au dela de ca faut trouver le dev qui le fait…

merci a tous les 2.

Je viens de faire la modification suivante, qui reprend le code d’exemple de php league:
Je désactive le isConnect(), et je verifie si le session token existe ou non « isset($_SESSION[‹ oauth2state ›]) && ».
Bon, en fait, ça correspond a ne pas le tester, ce qui est pas terrible. J’aime bien ta solution @Mips avec ton mécanisme de cache. je vais voir pour implémenter ca.


/* version 4.2: isConnect returns false on callback (coming from external sites)
if (!isConnect()) {
	echo 'Vous ne pouvez appeler cette page sans être connecté. Veuillez vous connecter <a href=' . network::getNetworkAccess('external') . '/index.php>ici</a> avant et refaire l\'opération de synchronisation';
	die();
}
*/


//
// AUTHORIZATION CALLBACK
//
// As we extend AbstractProvider from League, then we should get our provider
// check the state session compared to the state of the request
//
$provider = $eqLogic->getProvider();

//
// Check given state against previously stored one to mitigate CSRF attack
//
if (empty($_GET['state']) || (isset($_SESSION['oauth2state']) && $_GET['state'] !== $_SESSION['oauth2state'])) {

    log::add('strava', 'error', __('Session invalide : _GET=' . $_GET['state'] . ', _SESSION=' . $_SESSION['oauth2state'], __FILE__));
	if (isset($_SESSION['oauth2state'])) {
    	unset($_SESSION['oauth2state']);
	}
    exit('Invalid state');
}

en tout cas, avec ses modifications, cela fonctionne correctement au niveau OAuth2, donc je peux faire le flow complet. Je vais gerer le token en cache plutot qu’en session dans la version stable.

merci pour votre aide
Benoit

Dans ta méthode connectWithStrava tu changes comme ceci:

    public function connectWithStrava() {
        log::add('strava', 'debug', 'connect with Strava');
        $provider                = $this->getProvider();
        $authorizationUrl        = $provider->getAuthorizationUrl();
        cache::set('strava::state', $provider->getState(), 600);
        return $authorizationUrl;
    }

Et dans ton authorization.php, utilises les fonctions suivantes là où il faut:

Pour tester si la valeur existe:

cache::exist('strava::state')

pour récupérer la valeur:

$state = cache::byKey('strava::state')->getValue();

Pour supprimer la valeur dans le cache:

cache::delete('strava::state');
1 « J'aime »

merci @Mips , c’est du sur mesure. J’apprecie :+1:

1 « J'aime »

Ce sujet a été automatiquement fermé après 24 heures suivant le dernier commentaire. Aucune réponse n’est permise dorénavant.