Projet

Général

Profil

Development #41733

BO : Dans la liste de rôles d'un utilisateur, sur la page d'un utilisateur, un rôle ne doit être cliquable que si l'administrateur a le droit de voir cerôle.

Ajouté par Mikaël Ates il y a presque 4 ans. Mis à jour il y a plus de 3 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
-
Catégorie:
-
Version cible:
-
Début:
15 avril 2020
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Dans la liste de rôles d'un utilisateur, sur la page d'un utilisateur, tous les rôles sont cliquables (ce sont des liens vers la page de ce rôle).

Si l'administrateur n'a pas le droit de voir ce rôle, il lui est affiché la page de ce rôle avec le message "Vous n’êtes pas autorisé à voir cette page.".

Il serait préférable que les rôles pour lesquels l'utilisateur n'a pas les droits ne soient pas cliquables (ne soient pas de liens).


Fichiers

Révisions associées

Révision 4fc9450d (diff)
Ajouté par Paul Marillonnet il y a plus de 3 ans

manager: deactivate link for un-viewable roles in user details (#41733)

Historique

#1

Mis à jour par Paul Marillonnet il y a presque 4 ans

Une idée de ce que ça pourrait donner, en construisant une liste de rôles visible passée dans le contexte plutôt que de tester dans le gabarit à chaque itération :

diff --git a/src/authentic2/manager/templates/authentic2/manager/user_detail.html b/src/authentic2/manager/templates/authentic2/manager/user_detail.html
index 1df94dcde..5928c17b1 100644
--- a/src/authentic2/manager/templates/authentic2/manager/user_detail.html
+++ b/src/authentic2/manager/templates/authentic2/manager/user_detail.html
@@ -83,7 +83,7 @@
         {% endif %}
               {% for role in ou_roles %}
                 <li {% if role.description %}title="{{ role.description }}"{% endif %}>
-                <a href="{% url "a2-manager-role-members" pk=role.pk %}">{{ role }}</a></li>
+                {% if role.pk in viewable_roles_pks %}<a href="{% url "a2-manager-role-members" pk=role.pk %}">{{ role }}</a>{% else %}{{ role }}{% endif %}</li>
               {% endfor %}
         {% if multiple_ou %}
               </ul>
diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py
index 64fe239d7..114fec416 100644
--- a/src/authentic2/manager/user_views.py
+++ b/src/authentic2/manager/user_views.py
@@ -324,6 +324,14 @@ class UserDetailView(OtherActionsMixin, BaseDetailView):
             role_qs = role_qs.filter(ou=instance.ou)
         return user.filter_by_perm('a2_rbac.change_role', role_qs).exists()

+    @classmethod
+    def viewable_roles_pks(self, user, instance):
+        role_qs = get_role_model().objects.all()
+        if app_settings.ROLE_MEMBERS_FROM_OU and instance.ou:
+            role_qs = role_qs.filter(ou=instance.ou)
+        return [role.pk for role in user.filter_by_perm(
+            'a2_rbac.view_role', role_qs)]
+
     def get_context_data(self, **kwargs):
         kwargs['default_ou'] = get_default_ou
         roles = self.object.roles_and_parents().order_by('ou__name', 'name')
@@ -334,6 +342,7 @@ class UserDetailView(OtherActionsMixin, BaseDetailView):
         kwargs['roles_by_ou'] = roles_by_ou
         # show modify roles button only if something is possible
         kwargs['can_change_roles'] = self.has_perm_on_roles(self.request.user, self.object)
+        kwargs['viewable_roles_pks'] = self.viewable_roles_pks(self.request.user, self.object)
         user_data = []
         user_data += [data for datas in hooks.call_hooks('manager_user_data', self, self.object)
                       for data in datas]

J'écris un test en fin de journée (i.e. dès que j'ai un peu de temps) pour vérifier que c'est bien une approche viable.

#2

Mis à jour par Benjamin Dauvergne il y a presque 4 ans

C'est mieux de ne pas mettre trop de control flow dans les templates, il vaudrait mieux habiller les objets Role dans roles_by_ou, au passage, d'un attribut .user_accessible et aussi .title et éviter les classmethod quand ce n'est pas nécessaire (ici self.request.user est accessible depuis la méthode, pas besoin de le passer en paramètre).

À noter qu'on peut appeler des méthodes de la vue directement dans les templates depuis un moment via {% view.method %} associé à @cached_property ça peut rendre les modifications à get_context_data() inutiles.

#3

Mis à jour par Paul Marillonnet il y a presque 4 ans

  • Statut changé de Nouveau à Information nécessaire

Benjamin Dauvergne a écrit :

À noter qu'on peut appeler des méthodes de la vue directement dans les templates depuis un moment via {% view.method %} associé à @cached_property ça peut rendre les modifications à get_context_data() inutiles.

Hmm, jamais vu cet usage (qui d'ailleurs n'est employé nulle part ni dans a2 ni dans les autres briques de Publik), et des recherches dans la doc puis dans les sources de Django ne donnent rien. Qu'est-ce que je loupe ?

#4

Mis à jour par Benjamin Dauvergne il y a presque 4 ans

  • Statut changé de Information nécessaire à En cours
#5

Mis à jour par Paul Marillonnet il y a plus de 3 ans

Ok, pigé, et donc on peut s'épargner la propriété en cache pour la vue.
Pour cette histoire d'habiller les objets Role dans roles_by_ou, je n'ai pas cherché à taper un annotate parce que le code initial itère déjà explicitement sur chaque rôle.

#6

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

  • Statut changé de Solution proposée à Solution validée

Ok.

#7

Mis à jour par Paul Marillonnet il y a plus de 3 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit 4fc9450d7153a452ce469c64dd99046f326f5609
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date:   Wed Apr 15 11:14:36 2020 +0200

    manager: deactivate link for un-viewable roles in user details (#41733)
#8

Mis à jour par Frédéric Péters il y a plus de 3 ans

  • Statut changé de Résolu (à déployer) à Solution déployée

Formats disponibles : Atom PDF