Improve the management of user permissions #28
Comments
La notions de LS/Ban temporaire n'est pas une notions gérée par django. Si on veut gérer le fait qu'un utilisation soit sanctionné un moment et que cette sanction prenne fin toute seule (sans devoir passer manuellement ou encore sans qu'un bot serveur fasse les réactivation) on est obligé de gérer une date de fin de sanction. Et là je ne vois pas comment se passer autrement des deux dates et des methodes can_<read/write>_now(). Après c'est surement faisable en annotations. Mais je ne m'y suis pas encore penché. |
|
J'ai un peu testé, les permissions avec Django sont vraiment élégantes. Par exemple, si nous voulons implémenter la lecture seule et le bannissement, il suffirait de : 1/ Rajouter les permissions dans la classe class Profile(models.Model):
'''Represents an user profile'''
class Meta:
verbose_name = 'Profil'
verbose_name_plural = 'Profils'
permissions = (
("moderation", u"Modérer un membre"),
("show_ip", u"Afficher les IP d'un membre"),
("can_read", "Possibilité de lecture"),
("can_write", "Possibilité d'écriture"),
)2/ Ajouter ou supprimer ces permissions sur les utilisateurs (à faire sur la classe myuser.user_permissions = [permission_list]
myuser.user_permissions.add(permission, permission, ...)
myuser.user_permissions.remove(permission, permission, ...)
myuser.user_permissions.clear()3/ Obliger l'utilisateur à avoir la permission pour accéder à la vue voulue : from django.contrib.auth.decorators import permission_required
@permission_required('member.can_read')
def topic(request):
# Do somethingA savoir qu'il existe aussi des méthodes pour savoir si l'utilisateur à les permissions : myuser.has_perm('member.can_read')Voilà, c'est qu'une idée mais c'est quand même vachement plus élégant que des tests au début de toutes les méthodes concernées par la permission. Après, c'est vrai que ça nécessite de réfléchir à comment mettre fin à la sanction de l'utilisateur. Je pense qu'il serait possible de trouver facilement une solution. Genre gérer les permissions à la re-connexion de l'utilisateur. Voilà, qu'en penses-tu ? |
|
C'est classe. Le 20 décembre 2013 10:20, Gerard notifications@github.com a écrit :
|
Tu as bien saisi le comportement de Django, et c'est ainsi que je gère les permissions pour tout le reste. Le seul problème posé ici c'est que si on veut faire de la sanction temporaire, ça devient tout de suite moins trivial. L'idéal serait de surcharger la méthode d'une annotation particulière mais encore une fois, c'est un autre paire de manche. Et je n'ai pas envie de gérer les de-ban manuellement ni par tâches cron. |
|
Ça pose vraiment un problème les tâches cron pour ce faire ? Le 20 décembre 2013 10:29, dralliw notifications@github.com a écrit :
|
Disons, que tu es quand même obligé de stocké ta date de fin de sanction et que ta cron tache dois parcourir (tous les jours? heures ?) tes membres sanctionné. Je trouve ça un peu lourd à faire par le serveur, qu'une simple surcharge de méthode (dans la mesure du possible). |
|
Ce genre de modifs, c'est du confort et ça rentre donc dans le scope d'une version ultérieure. Pour le moment on a un truc qui marche, c'est le principal. En bref, vous gâchez du temps de cerveau disponible de Willard, laissez-le travailler sur le modules des tutos !! :D |
|
Je me suis un peu re-penché sur cette issue et je dois dire que c'est plutôt simple de créer ses propres annotations. Pour ceux qui ne savent pas comment faire et pour reprendre l'exemple de mon premier poste, voici le code qui permet de créer une annotation def can_read_now(func):
'''Decorator to check that the user can read now'''
def _can_read_now(request, *args, **kwargs):
profile = Profile.objects.filter(user__pk=request.user.pk).all()
if len(profile)>0:
if not profile[0].can_read_now():
raise Http404
return func(request, *args, **kwargs)
return _can_read_nowDonc pour une méthode décorée comme l' @can_read_now
def index(request):
passL'attribut Dans cette méthode privée, il suffit d'y mettre le comportement qu'on veut répéter pour toutes les méthodes annotées @can_read_now. J'aurais aimé avoir des avis si cela vous semble une solution viable et avoir vos avis pour savoir où je dois placer ces décorateurs. |
|
Le decorateur c'est propre et classe mais il reste le prob du cron pour le coté temporaire des sancontions, non ? |
|
Pour l'exemple ci-dessus, je ne mentionnais pas une application pour les sanctions temporaires. Si tu veux parler des sanctions temporaires, à mon sens, il faudrait une table propre aux sanctions avec l'utilisateur, le type de bannissement et la date de fin. A partir de là, ça serait très simple de les gérer dans un décorateur. :) |
Oh mais elle est juste magnifique cette solution. Je savais bien qu'on pouvais gérer ça proprement à coup de décorateur, mais je ne m'y étais pas encore penché. Cette solution règle bien le problème de la répétition du code dans les fonctions, et permet de gérer les sanctions temporaires avec le modèles existant (on ne peut pas faire de sanctions temporaires sans rajouter des dates de fin de validité dans le modèle). Pour l'implémentation, il faudrait créer un fichier |
|
J'intègre la gestion des sanctions dans le PR ou simplement la répétition sur la lecture/écriture ou non ? Dans le second cas, je peux déjà soumettre. :) |
|
Si tu peux ne pas l'intégrer, c'est que c'est deux choses différentes, du Le 2 janvier 2014 14:48, Gerard notifications@github.com a écrit :
|
|
Ce n'est pas si différent que ça. C'est juste que j'ai pas trop de temps en ce moment. Du coup, soit je soumets mon PR maintenant avec les annotations @can_read_now et @can_read_and_write_now ou plus tard (je ne sais pas quand), avec une gestion plus poussée des sanctions. |
Il y'a plusieurs chose à gérer :
Donc, en principe, je ne vois pas de modification particulière à faire pour la gestion "poussée" des sanctions, et donc, une PR pour décorer un peu les fonctions adéquates serait suffisante selon moi. |
|
Oui, j'ai tendance à vouloir bien tout diviser mais j'oubliais que les jointures étaient couteuses pour un système comme celui-ci. Du coup, j'étais partie là pour terminer la possibilité de lire ou d'écrire pour un utilisateur quand il est sanctionné et je me suis posé plusieurs questions :
|
ça c'est ce que j'appelle une grosse connerie de ma part. En effet, la méthode get est plus adaptée ici.
Pour moi
Pour les LS, déjà je vois toutes les fonctions qui sont annotées en tant que @login_required dans la views.py du forum. Pour le Ban, j'ai envie de dire, il faut déjà annoter la méthode de connexion (interdiction de se connecter si on est ban). |
Pour le ban Ok tu peux l'empecher de se connecter a condition de le deconnecter au moment de l'application de la sanction |
Toutafè |
|
J'ai intégré les dernières modifications. Juste 2 questions :
Après ces réponses, je fais un nouveau PR de la branche permission. |
|
En effet si tu deconnecte pour le ban, pas besoin d'annoter ainsi. En fait cela aurait une certaine logique si tu aitorisais par exemple a un banni de consulter ces mp. La, en effet, ne t'embete pas. |
from django.contrib.auth import logout
def logout_view(request):
'''Log out user'''
logout(request)
request.session.clear()
J'ai dis "empêcher de se connecter", c'est une proposition, pas forcément décidée. On peut aussi dire que le membre ban doit pouvoir lire ses MP. Quelque soit la décision prise au final, le |
|
La méthode
|
|
Ah si tu comptes déconnecter un utilisateur a distance, ça ne marchera pas. Faudra le déconnecter s'il essaye d'accéder a une vue quelconque (et dans ce cas c'est lui qui invoque le logout(request)). Et donc décorer toutes les autres vues que tu souhaites interdire (ce qui résoudre le dilemme de l'utilité de En somme, tu trouveras du code pas utilisé (mais dont je sais qu'on en aura besoin, a moyen/long terme) si tu regarde bien, mais l'avantage de ce genre de pratique est que le jour ou j'en ai besoin, je n'ai plus a me replacer dans le contexte du problème en plus d'avoir déjà un squelette de la façon de faire. |
|
Wow, c'est un peu chaud comme solution ça. Je vais devoir annoter absolument toutes les méthodes des vues des différents modules. C'est bizarre qu'il n'existe aucune solution pour déconnecter un utilisateur mais si nous n'avons pas le choix ... |
|
Si jamais vous ne l'avez pas lu, y'a quelques pistes ici : |
|
J'avais vu cette solution mais ça me semblait vraiment couteux de lister toutes les sessions pour récupérer la bonne et la supprimer. Puis, je ne suis pas un as du systèmes des sessions, mais que se passe-t-il lorsqu'un utilisateur est connecté avec plusieurs appareils ? Dans tous les cas, cette solution me semble un peu du bricolage. |
|
La solution avec la classe |
|
Cette solution semble pas mal mais elle permet encore une action de l'utilisateur avant de le déconnecter, je ne comprends pas trop comment l'intégrer dans les settings et il semble y avoir des problèmes si l'utilisateur est connecté depuis plusieurs endroits. |
|
S'il est connecté depuis plusieurs endroits, alors il aura le droit à autant de dernières actions. Si jamais on ne trouve pas mieux sans aller lister les sessions ouvertes, ça me paraît acceptable comme compromis. |
ghost
assigned
|
Cette issue a déjà été corrigée, je ferme donc. |
J'ai l'impression que la gestion des permissions liées aux sanctions pourraient être améliorées.
Actuellement, c'est géré par 2 booléens et 2 dates associées à ces booléens. Cependant, j'ai cru comprendre qu'un système de permissions était implémenté automatiquement dans Django à travers les
Meta.Il faudrait penser à migrer le système actuel par ce système intégré au framework. Ca rendrait le code vraiment plus joli que de gérer des booléens.
Et, si j'ai bien compris le module du forum, on aurait plus qu'à utiliser des annotations sur les différentes méthodes de
views.pypour exécuter ou non la méthode. Les instructions suivantes deviendront inutiles :The text was updated successfully, but these errors were encountered: