Enlève la possibilité de nommer un membre super-utilisateur depuis l'interface de promotion #4142

gllmc · 2017-01-08T20:06:55Z

Cette pull request a été modifiée. Lire les commentaires pour plus de détails.

Nouvelle version

Q	R
Type de modification	correction

Actuellement, il est possible de donner les droits super-user à un utilisateur via l'interface de promotion. Cependant, cela ne donne pas de logs comme le ferait une nomination dans l'administration de Django. Ainsi, cette pull request enlève cette possibilité, mais ajoute un lien vers l'administration de Django du membre.

QA

Vérifier que le formulaire de promotion fonctionne toujours correctement ;
Vérifier qu'on ne peut plus nommer un membre super-utilisateur via ce formulaire ;
Vérifier que le lien vers l'administration de Django fonctionne correctement ;
Vérifier que le test zds.member.tests.tests_views.MemberTests.test_promote_interface passe toujours.

Ancienne version

Q R

Type de modification évolution

Actuellement, il est possible de donner les droits super-user à un utilisateur via l'interface de promotion. Cependant, cela ne lui donne pas accès à l'administration de Django. En effet, cet accès est géré avec le booléen is_staff et pas avec is_superuser. Cette pull request ajoute donc un champ au formulaire de promotion pour donner cet accès.

QA

Donner accès à un utilisateur à l'administration de Django via l'interface de promotion ;

Vérifier que le MP qu'il reçoie mentionne cet accès ;

Vérifier qu'il a effectivement accès à l'administration ;

Supprimer cet accès (toujours avec l'interface de promotion) ;

Vérifier que l'utilisateur n'a plus accès à l'administration ;

Vérifier que le test zds.member.tests.tests_views.MemberTests.test_promote_interface passe toujours.

coveralls · 2017-01-08T20:18:40Z

Coverage increased (+0.01%) to 86.851% when pulling 29cc721 on GCodeur:staff_formulaire_promotion into 42ea44e on zestedesavoir:dev.

vhf · 2017-01-08T21:01:23Z

Merci pour ton travail !

Malheureusement je pense refuser cette modification par mesure de sécurité. En effet, elle renforce fortement le couplage de la sécurité des données de ZdS au code de ZdS.

On part du principe que ce qui fait partie de Django est sûr, sinon on n'utiliserait pas Django. On est abonnés aux bulletins de sécurité, et on déploie tous les patchs de sécurité publiés par Django.

L'admin donne accès à absolument toute notre base de donnée. Le staff ne doit pas avoir accès à l'admin Django. Je crois que je préfère jouer la carte de la prudence et garder la certitude que si quelqu'un a obtenu accès à l'admin, c'est parce qu'un admin l'a fait depuis l'admin et donc cette action est dans les logs de l'admin. Le risque est plus restreint comme ceci. :)

Désolé pour le temps que tu as consacré à cette PR. :(

vhf · 2017-01-08T21:25:15Z

Après discussion avec @GCodeur la situation est différente de ce que je pensais et plus compliquée qu'attendue.

Je vais en discuter avec @gustavi avant qu'on décide d'aller dans un sens ou dans l'autre.

Ce formulaire étant restreint aux superuser, je penche pour enlever la possibilité de modifier l'attribut is_superuser depuis le site, et ajouter un lien vers la page de l'admin concernant cet utilisateur. Ça aurait les avantages suivants :

Ajouter/enlever is_staff (i.e. connexion admin) et is_superuser(i.e. passe-droit) ne peut se faire que dans l'admin et donc on a un log des actions
Modifier les groupes d'un membre continue d'être faisable depuis le profil du membre si on est superuser, donc on est n'est pas obligés d'aller dans l'admin et risque de s'y planter en mettant par exemple is_staff par erreur quand on voulait à la base ajouter le membre au groupe "staff" (très différent !)
Accéder à la page d'admin d'un utilisateur est aisé, alors que la recherche dans l'admin django est une galère

gllmc · 2017-01-08T21:32:56Z

J'en profite pour mettre ici mon petit résumé sur le fonctionnement de is_staff et is_superuser que j'avais donné à victor. :)

Dans le modèle User de Django (donc pas notre modèle Profile qui le complète), il y a deux attributs is_staff et is_superuser. Ceux-ci ont deux fonctions différentes :

is_superuser permet d'accorder tous les droits à l'utilisateur, c'est-à-dire que quand on fait un test sur une permission il sera accordé même si l'utilisateur n'a pas cette permission explicitement (ou de par son groupe).
is_staff quant à lui est un simple booléen qui autorise ou non l'accès à l'admin Django.

Et du coup notre formulaire actuel permet d'assigner is_superuser mais pas is_staff et le but initial de cette pull request était d'y ajouter is_staff. ;)

…staff_formulaire_promotion

… un lien vers l'admin Django

coveralls · 2017-01-15T21:03:50Z

Coverage decreased (-0.01%) to 86.98% when pulling 7094201 on GCodeur:staff_formulaire_promotion into 7d66a6c on zestedesavoir:dev.

gllmc · 2017-01-15T21:08:01Z

Les modifications ont été apportées. Ainsi :

Un lien vers l'administration de Django a été ajouté.
Il n'est plus possible de nommer un utilisateur is_staff ou is_superuser via l'interface de promotion.

J'ai édité le premier message et le titre pour que ce soit plus clair.

La QA peut normalement être faite. :)

…staff_formulaire_promotion

coveralls · 2017-01-15T21:41:16Z

Coverage decreased (-0.03%) to 86.966% when pulling 6b6e98c on GCodeur:staff_formulaire_promotion into 3de5950 on zestedesavoir:dev.

coveralls · 2017-01-27T18:49:47Z

Coverage decreased (-0.01%) to 87.001% when pulling 64a201f on GCodeur:staff_formulaire_promotion into fedf98d on zestedesavoir:dev.

DevHugo · 2017-02-01T18:23:23Z

@vhf tu confirme que pour toi, c'est bon ? Je peux faire la QA ?

vhf · 2017-02-01T19:57:50Z

Oui @DevHugo volontiers pour la QA (avec code review comme d'hab hein ;) )

DevHugo · 2017-02-04T17:34:05Z

Je fais ça autour de 20h

DevHugo · 2017-02-04T20:09:19Z

templates/member/profile.html

                    </a>
+                    {% if user.is_staff %}


C'est quoi la règle de gestion pour l'affichage du lien "Administration de Django" ?

Si il a seulement le droit "is_staff" normalement, il peut déjà accéder à l'administration de Django. Il n'a pas besoin d'être "superuser" ET "is_staff" pour accéder à la zone d'administration. Pourquoi donc ces deux conditions sont t-elle imbriquée ?

is_staff donne l'accès à l'administration de Django, mais ne donne aucun droit sur les modèles (c'est-à-dire qu'on peut très bien avoir le droit d'accéder à l'admin Django sans pouvoir rien gérer). On vérifie donc aussi is_superuser afin d'être sûr que l'utilisateur aura le droit d'accéder à cette page de l'admin.

En gros, is_staff n'inclut pas is_superuser, ce sont deux choses bien différentes. ;)

DevHugo · 2017-02-04T20:18:46Z

QA: Ok, j'attend un retour sur la remarque faite plus haut avant de merger.

Cas testé: ceux de la QA.

pierre-24 · 2017-02-08T19:04:55Z

La réponse de @GCodeur me satisfait :)

Permet de donner accès à l'admin Django via l'interface de promtion

29cc721

gllmc mentioned this pull request Jan 10, 2017

Correction de la documentation sur les membres #4147

Merged

gllmc added 2 commits January 15, 2017 21:21

Merge branch 'dev' of https://github.com/zestedesavoir/zds-site into …

92ac50d

…staff_formulaire_promotion

Enlève la possibilité d'ajouter is_staff et is_superuser, mais ajoute…

7094201

… un lien vers l'admin Django

gllmc changed the title ~~Permet de donner accès à l'administration de Django via l'interface de promotion~~ Enlève la possibilité de nommer un membre super-utilisateur depuis l'interface de promotion Jan 15, 2017

gllmc added 2 commits January 15, 2017 22:19

Merge branch 'dev' of https://github.com/zestedesavoir/zds-site into …

81d7dcf

…staff_formulaire_promotion

Documentation

6b6e98c

Merge branch 'dev' into staff_formulaire_promotion

64a201f

gllmc mentioned this pull request Feb 4, 2017

Dernier appel pour la v22 #4173

Closed

vhf requested a review from DevHugo February 4, 2017 16:17

DevHugo reviewed Feb 4, 2017

View reviewed changes

pierre-24 merged commit cb10faf into zestedesavoir:dev Feb 8, 2017

pierre-24 added C-Back Concerne le back-end Django S-Évolution labels Feb 8, 2017

pierre-24 added this to the Version de développement milestone Feb 8, 2017

gllmc deleted the staff_formulaire_promotion branch July 17, 2017 11:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enlève la possibilité de nommer un membre super-utilisateur depuis l'interface de promotion #4142

Enlève la possibilité de nommer un membre super-utilisateur depuis l'interface de promotion #4142

gllmc commented Jan 8, 2017 •

edited

coveralls commented Jan 8, 2017 •

edited

vhf commented Jan 8, 2017

vhf commented Jan 8, 2017 •

edited

gllmc commented Jan 8, 2017 •

edited

coveralls commented Jan 15, 2017 •

edited

gllmc commented Jan 15, 2017

coveralls commented Jan 15, 2017 •

edited

coveralls commented Jan 27, 2017 •

edited

DevHugo commented Feb 1, 2017

vhf commented Feb 1, 2017

DevHugo commented Feb 4, 2017

DevHugo Feb 4, 2017 •

edited

gllmc Feb 4, 2017

DevHugo commented Feb 4, 2017 •

edited

pierre-24 commented Feb 8, 2017

Enlève la possibilité de nommer un membre super-utilisateur depuis l'interface de promotion #4142

Enlève la possibilité de nommer un membre super-utilisateur depuis l'interface de promotion #4142

Conversation

gllmc commented Jan 8, 2017 • edited

Nouvelle version

QA

Ancienne version

QA

coveralls commented Jan 8, 2017 • edited

vhf commented Jan 8, 2017

vhf commented Jan 8, 2017 • edited

gllmc commented Jan 8, 2017 • edited

coveralls commented Jan 15, 2017 • edited

gllmc commented Jan 15, 2017

coveralls commented Jan 15, 2017 • edited

coveralls commented Jan 27, 2017 • edited

DevHugo commented Feb 1, 2017

vhf commented Feb 1, 2017

DevHugo commented Feb 4, 2017

DevHugo Feb 4, 2017 • edited

Choose a reason for hiding this comment

gllmc Feb 4, 2017

Choose a reason for hiding this comment

DevHugo commented Feb 4, 2017 • edited

pierre-24 commented Feb 8, 2017

gllmc commented Jan 8, 2017 •

edited

coveralls commented Jan 8, 2017 •

edited

vhf commented Jan 8, 2017 •

edited

gllmc commented Jan 8, 2017 •

edited

coveralls commented Jan 15, 2017 •

edited

coveralls commented Jan 15, 2017 •

edited

coveralls commented Jan 27, 2017 •

edited

DevHugo Feb 4, 2017 •

edited

DevHugo commented Feb 4, 2017 •

edited