[spip-dev] Ordre des fonctions d'autorisation : surcharges impossibles ?

Dev
1

Bonsoir,

J'ai un pb dans les surcharges de fonctions d'autorisation... SPIP 2.0

Dans inc/autoriser.php (ligne 88) je vois :

  // Chercher une fonction d'autorisation
  // Dans l'ordre on va chercher :
  // autoriser_type_faire,
  // autoriser_type,
  // autoriser_faire,
  // autoriser_defaut ;
  // puis les memes avec _dist

Ne serait-il pas plus logique de faire :

  // Chercher une fonction d'autorisation
  // Dans l'ordre on va chercher :
  // autoriser_type_faire, la meme avec _dist,
  // autoriser_type, la meme avec _dist,
  // autoriser_faire, la meme avec _dist,
  // autoriser_defaut, la meme avec _dist;

Exemple pratique : je voudrais faire fonctionner
  function autoriser_cs_configurer_dist()

Mais celle-ci ne sera jamais atteinte si la fonction autoriser_cs_configurer() n'existe pas ! Comment pouvoir la surcharger dans ce cas ?

Merci d'avance.

Pat

PJ : patch au cas où.

patch_autoriser.patch (1005 Bytes)

2

Ca me semblait plus logique comme ça afin de pouvoir surcharger plus
facilement une _dist du core, mais effectivement du coup ça coince
avec un _dist qui tente d'affiner une pas _dist ...

je ne crois pas qu'il y ait quiconque que ça plante si on change de logique.

Cependant ce que je ne comprends pas c'est qu'on devrait appeler
autoriser_cs_configurer_dist() avant autoriser_configurer_dist() ?

-- Fil

3

Je précise qu'en testant 1.92, c'était bien comme ça avant... Il y a eu sans doute une régression, sauf cause explicite...

J'aimerais poser autoriser_cs_configurer_dist() par défaut dans le plugin, sachant que si qq'un surcharge avec autoriser_cs_configurer(), il faut que ce soit possible. Hors, SPIP 2(mais non 1.92) embraye directement sur autoriser_configurer(), fonction qui existe déjà dans autoriser.php. L'accès à autoriser_cs_configurer() est donc impossible sous SPIP 2.

Pour les appels : les affinages d'abord, du plus fin au plus général, et à chaque fois la surcharge si elle existe...

Pat

Fil a écrit :

4

Pat a écrit :

Hors, SPIP 2(mais non 1.92) embraye directement sur autoriser_configurer(), fonction qui existe déjà dans autoriser.php.

Ceci est une vilaine bêtise, au temps pour moi.
C'est le plugin "Autorité" qui surcharge cette fonction, évidemment SPIP ne surcharge rien.
Mes tests viennent de passer au vert sans ce plugin.

A voir donc si les gros poissons (surcharges les moins affinées) doivent manger les plus petits (surcharges fines) ou non...

Pat

5

En ce qui concerne les surcharges, chacune des _dist ne devrait être appelées amha que si la non _dist associée ne soit pas trouvée. C'est le sens du patch que j'ai proposé.

En ce qui concerne les autorisations, il me semblerait logique de les propager sur tous les niveaux en cas de succès, du plus général au plus particulier.
Donc, pour que l'autorisation 'faire' 'qqchose' soit donnée, il faudrait que :
  1. l'autorisation 'par_defaut' soit positive (ou que la fonction associée ne soit pas trouvée)
  2. puis, idem pour l'autorisation 'faire' ,
  3. puis, idem pour celle de 'qqchose',
  4. et enfin, idem pour celle de 'faire' 'qqchose'

Actuellement (patch appliqué pour faire fonctionner les surcharges), l'autorisation 'faire' 'qqchose' prime sur toutes les autres en les ignorant totalement. Il me semble en effet peu compréhensible de vouloir faire passer autoriser_cs_configurer_dist() alors que autoriser_configurer_dist() interdit de configurer quoi que ce soit.

Pour programmer une exception, alors la porte de sortie est toujours de créer un nouveau 'faire' : autoriser_configurerlecouteau_dist() !

Pat

P.S. : dans un tout autre domaine, SPIP pose le même type de problème au niveau des traitements de balises : en définissant le traitement d'une balise sur un objet particulier, et bien les traitements de cette balise sur l'ensemble de ses objets potentiels sont totalement ignorés, à tord probablement.

Fil a écrit :

6

Bon, en faisant quelques recherches supplémentaires, je m'aperçois que le sujet des autorisations est un vaste et délicat sujet.

Qu'on m'arrête tout de suite si mon propos est stérile...

Le fonction autoriser_defaut() ne devrait être appelée qu'en dernier ressort, si aucune des autorisations appelées n'a été spécifiquement définie. Actuellement, elle n'autorise que les Admins non restreints, ce qui offre une sécurité finalement : vaut mieux ne pas autoriser à n'importe qui, par défaut.

Je propose un patch qui permet la propagation des autorisations, de la plus générale à la plus fine, sachant que autoriser_defaut() n'est appelée qu'en dernier ressort. Les _dist évidemment, restent intimement liées aux non _dist.

Donc, pour que l'autorisation 'faire' 'qqchose' soit donnée, il faudrait que :
      1. l'autorisation de 'faire' soit positive (ou que la fonction
  associée ne soit pas trouvée)
      2. puis, idem pour celle de 'qqchose',
      3. et enfin, idem pour celle de 'faire' 'qqchose'

Pat

Pat a écrit :

patch_propagation_autoriser.patch (2.12 KB)

7

Pat a écrit :

Bon, en faisant quelques recherches supplémentaires, je m'aperçois que le sujet des autorisations est un vaste et délicat sujet.

Qu'on m'arrête tout de suite si mon propos est stérile...

Le fonction autoriser_defaut() ne devrait être appelée qu'en dernier ressort, si aucune des autorisations appelées n'a été spécifiquement définie.

et c'est normalement le cas

Actuellement, elle n'autorise que les Admins non restreints, ce qui offre une sécurité finalement : vaut mieux ne pas autoriser à n'importe qui, par défaut.

oui, c'est l'idée

Je propose un patch qui permet la propagation des autorisations, de la plus générale à la plus fine, sachant que autoriser_defaut() n'est appelée qu'en dernier ressort. Les _dist évidemment, restent intimement liées aux non _dist.

et normalement c'est le cas

Donc, pour que l'autorisation 'faire' 'qqchose' soit donnée, il faudrait que :
     1. l'autorisation de 'faire' soit positive (ou que la fonction
associée ne soit pas trouvée)
     2. puis, idem pour celle de 'qqchose',
     3. et enfin, idem pour celle de 'faire' 'qqchose'

pas sur de comprendre, mais du coup, en regardant le code de autoriser_dist, j'avoue que je ne suis pas fan de l'ordre de selection des fonctions, du moins entre fonctions dist et non-dist:
$fonctions = $type
        ? array (
            'autoriser_'.$type.'_'.$faire,
            'autoriser_'.$type,
            'autoriser_'.$faire,
            'autoriser_defaut',
            'autoriser_'.$type.'_'.$faire.'_dist',
            'autoriser_'.$type.'_dist',
            'autoriser_'.$faire.'_dist',
            'autoriser_defaut_dist'
        )
        : array (
            'autoriser_'.$faire,
            'autoriser_defaut',
            'autoriser_'.$faire.'_dist',
            'autoriser_defaut_dist'
        );

ca me paraitrait quand meme plus logique de faire :
$fonctions = $type
        ? array (
            'autoriser_'.$type.'_'.$faire,
            'autoriser_'.$type.'_'.$faire.'_dist',
            'autoriser_'.$faire,
            'autoriser_'.$faire.'_dist',
            'autoriser_'.$type,
            'autoriser_'.$type.'_dist',
            'autoriser_defaut',
            'autoriser_defaut_dist'
        )
        : array (
            'autoriser_'.$faire,
            'autoriser_'.$faire.'_dist',
            'autoriser_defaut',
            'autoriser_defaut_dist'
        );

quand je surcharge une fonction en mettant ma fonction "non-dist", je ne m'attend pas à péter toute la chaine d'autorisation...

mes 2 sous.

Stephane

8

Stephane a écrit :

Pat a écrit :

Donc, pour que l'autorisation 'faire' 'qqchose' soit donnée, il faudrait que :
     1. l'autorisation de 'faire' soit positive (ou que la fonction
associée ne soit pas trouvée)
     2. puis, idem pour celle de 'qqchose',
     3. et enfin, idem pour celle de 'faire' 'qqchose'

pas sur de comprendre

En gros, je propose de propager la demande d'autorisation du plus général au plus précis, et non plus de se satisfaire une seule fonction (à savoir la plus fine)... Si tout le monde est au vert, alors on autorise !

mais du coup, en regardant le code de autoriser_dist, j'avoue que je ne suis pas fan de l'ordre de selection quand je surcharge une fonction en mettant ma fonction "non-dist", je ne m'attend pas à péter toute la chaine d'autorisation...

C'est très exactement le teneur de mon premier patch !
Mon second patch (concernant la propagation des autorisations) prend aussi en compte cette idée...

Pat

9

Cédric

Stephane a écrit :

Pat a écrit :

Donc, pour que l'autorisation 'faire' 'qqchose' soit donnée, il faudrait que :
    1. l'autorisation de 'faire' soit positive (ou que la fonction
associée ne soit pas trouvée)
    2. puis, idem pour celle de 'qqchose',
    3. et enfin, idem pour celle de 'faire' 'qqchose'

pas sur de comprendre

En gros, je propose de propager la demande d'autorisation

Dans spip, quand on veut proposer ce type de fonctionnalité, on me fait avec un pipeline.

Cette question avait été posée au moment de la création de l' api autoriser.
On peut se la reposer, mais sans casser l'existant, et en utilisant un mécanisme standard de spip si possible
Cédric

10

cedric.morin@yterium.com a écrit :

Dans spip, quand on veut proposer ce type de fonctionnalité, on me fait avec un pipeline.

Cette question avait été posée au moment de la création de l' api autoriser.
On peut se la reposer, mais sans casser l'existant, et en utilisant un mécanisme standard de spip si possible

oui mais la ou je rejoins Pat, c'est dans l'ordre de prise en compte des fonctions _dist
La, tel que c'est fait, si je surcharge autoriser_defaut, c'est cette fonction qui ser prise en compte à la place de TOUTES les fonctions _dist, et non pas à la place de autoriser_defaut_dist comme on pourrait s'y attendre

Je sens bien l'intention qu'il y a derriere (surcharger l'ensemble des fonctions d'un objet ou d'un type avec une seule fonction), mais ca me parait contre intuitif et surtout très limitant quant aux possibilités de surcharge et d'organisations des fonctions d'autorisation (qui peuvnt s'appeler en cascade en fonction des droits)

Mais bon, je ne sais pas si ca poserait réellement probleme à des organisations existantes et puis comme j'ai tendance à surcharger autoriser... je ne vais pas me battre pour ce changement.

Stephane.

11

Bonjour,
Je republie mon patch car une erreur de frappe (et d'inattention!) y figurait.
Pat

Pat a écrit :

patch_propagation_autoriser.patch (2.18 KB)