From 3f9d613c3da68e69afe5addf1aafc9c60265bd74 Mon Sep 17 00:00:00 2001 From: Hugo Levy-Falk Date: Mon, 9 Sep 2019 12:03:27 +0200 Subject: [PATCH 1/3] Fix #217 --- ...1_optionaluser_allow_archived_connexion.py | 20 +++++++++++++++++++ preferences/models.py | 4 ++++ .../preferences/display_preferences.html | 2 ++ users/models.py | 3 ++- 4 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 preferences/migrations/0061_optionaluser_allow_archived_connexion.py diff --git a/preferences/migrations/0061_optionaluser_allow_archived_connexion.py b/preferences/migrations/0061_optionaluser_allow_archived_connexion.py new file mode 100644 index 00000000..505f940b --- /dev/null +++ b/preferences/migrations/0061_optionaluser_allow_archived_connexion.py @@ -0,0 +1,20 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.23 on 2019-09-09 09:50 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('preferences', '0060_auto_20190712_1821'), + ] + + operations = [ + migrations.AddField( + model_name='optionaluser', + name='allow_archived_connexion', + field=models.BooleanField(default=False, help_text='If True, archived users are allowed to connect.'), + ), + ] diff --git a/preferences/models.py b/preferences/models.py index be8b8ec2..98374e77 100644 --- a/preferences/models.py +++ b/preferences/models.py @@ -123,6 +123,10 @@ class OptionalUser(AclMixin, PreferencesModel): help_text=_("If True, all new created and connected users are active." " If False, only when a valid registration has been paid.") ) + allow_archived_connexion = models.BooleanField( + default=False, + help_text=_("If True, archived users are allowed to connect.") + ) class Meta: permissions = ( diff --git a/preferences/templates/preferences/display_preferences.html b/preferences/templates/preferences/display_preferences.html index 3e71c817..626cb1a3 100644 --- a/preferences/templates/preferences/display_preferences.html +++ b/preferences/templates/preferences/display_preferences.html @@ -125,6 +125,8 @@ with this program; if not, write to the Free Software Foundation, Inc., {% trans "All users are active by default" %} {{ useroptions.all_users_active|tick }} + {% trans "Allow archived users to log-in" %} + {{ useroptions.allow_archived_connexion|tick }} diff --git a/users/models.py b/users/models.py index 68719e0f..6c19d873 100755 --- a/users/models.py +++ b/users/models.py @@ -333,7 +333,8 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, @property def is_active(self): """ Renvoie si l'user est à l'état actif""" - return self.state == self.STATE_ACTIVE or self.state == self.STATE_NOT_YET_ACTIVE + allow_archived = OptionalUser.get_cached_value('allow_archived_connexion') + return self.state == self.STATE_ACTIVE or self.state == self.STATE_NOT_YET_ACTIVE or (allow_archived and self.state in (self.STATE_ARCHIVE, self.STATE_FULL_ARCHIVE)) def set_active(self): """Enable this user if he subscribed successfully one time before From a9ebe331dde703f9bec84f97ef2ebfc8f301f79b Mon Sep 17 00:00:00 2001 From: Hugo Levy-Falk Date: Mon, 9 Sep 2019 12:16:37 +0200 Subject: [PATCH 2/3] Fix #54 --- re2o/acl.py | 15 +++++++++++++-- users/models.py | 15 ++++++++++----- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/re2o/acl.py b/re2o/acl.py index e445a28c..75857dbf 100644 --- a/re2o/acl.py +++ b/re2o/acl.py @@ -41,6 +41,8 @@ from re2o.utils import get_group_having_permission def acl_error_message(msg, permissions): """Create an error message for msg and permissions.""" + if permissions is None: + return msg groups = ", ".join([ g.name for g in get_group_having_permission(*permissions) ]) @@ -76,9 +78,11 @@ def acl_base_decorator(method_name, *targets, on_instance=True): permission was granted. This is to allow you to run ACL tests on fields only. If the method exists, it has to return a 2-tuple `(can, reason, permissions)` with `can` being a boolean stating - whether the access is granted, `reason` a message to be + whether the access is granted, `reason` an arror message to be displayed if `can` equals `False` (can be `None`) and `permissions` - a list of permissions needed for access (can be `None`). + a list of permissions needed for access (can be `None`). If can is + True and permission is not `None`, a warning message will be + displayed. *targets: The targets. Targets are specified like a sequence of models and fields names. As an example ``` @@ -172,10 +176,17 @@ ModelC) yield can_change_fct(request.user, *args, **kwargs) error_messages = [] + warning_messages = [] for target, fields in group_targets(): for can, msg, permissions in process_target(target, fields): if not can: error_messages.append(acl_error_message(msg, permissions)) + elif msg: + warning_messages.append(acl_error_message(msg, permissions)) + + if warning_messages: + for msg in warning_messages: + messages.warning(request, msg) if error_messages: for msg in error_messages: diff --git a/users/models.py b/users/models.py index 6c19d873..faacc57f 100755 --- a/users/models.py +++ b/users/models.py @@ -859,18 +859,23 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, user_request one of its member, or if user_request is self, or if user_request has the 'cableur' right. """ + if self.state in (self.STATE_ARCHIVE, self.STATE_FULL_ARCHIVE): + warning_message = _("This user is archived.") + else: + warning_message = None + if self.is_class_club and user_request.is_class_adherent: if (self == user_request or user_request.has_perm('users.change_user') or user_request.adherent in self.club.administrators.all()): - return True, None, None + return True, warning_message, None else: return False, _("You don't have the right to edit this club."), ('users.change_user',) else: if self == user_request: - return True, None, None + return True, warning_message, None elif user_request.has_perm('users.change_all_users'): - return True, None, None + return True, warning_message, None elif user_request.has_perm('users.change_user'): if self.groups.filter(listright__critical=True): return ( @@ -886,9 +891,9 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, ('users.change_all_users', ) ) else: - return True, None, None + return True, warning_message, None elif user_request.has_perm('users.change_all_users'): - return True, None, None + return True, warning_message, None else: return ( False, From 8df5a05d89a0acee2a68c251e0da04fb7b8e2e31 Mon Sep 17 00:00:00 2001 From: Hugo Levy-Falk Date: Mon, 9 Sep 2019 14:40:40 +0200 Subject: [PATCH 3/3] Do not display unnecessary warnings. --- cotisations/models.py | 23 +++++++++++--------- machines/models.py | 20 ++++++++++------- re2o/mixins.py | 25 +++++++++++++--------- tickets/models.py | 5 +++-- users/models.py | 50 ++++++++++++++++++++++++++----------------- 5 files changed, 73 insertions(+), 50 deletions(-) diff --git a/cotisations/models.py b/cotisations/models.py index 32326983..006e8d69 100644 --- a/cotisations/models.py +++ b/cotisations/models.py @@ -246,9 +246,10 @@ class Facture(BaseInvoice): def can_change_control(user_request, *_args, **_kwargs): """ Returns True if the user can change the 'controlled' status of this invoice """ + can = user_request.has_perm('cotisations.change_facture_control') return ( - user_request.has_perm('cotisations.change_facture_control'), - _("You don't have the right to edit the \"controlled\" state."), + can, + _("You don't have the right to edit the \"controlled\" state.") if not can else None, ('cotisations.change_facture_control',) ) @@ -746,11 +747,12 @@ class Article(RevMixin, AclMixin, models.Model): A boolean stating if usage is granted and an explanation message if the boolean is `False`. """ + can = self.available_for_everyone \ + or user.has_perm('cotisations.buy_every_article') \ + or user.has_perm('cotisations.add_facture') return ( - self.available_for_everyone - or user.has_perm('cotisations.buy_every_article') - or user.has_perm('cotisations.add_facture'), - _("You can't buy this article."), + can, + _("You can't buy this article.") if not can else None, ('cotisations.buy_every_article', 'cotisations.add_facture') ) @@ -902,11 +904,12 @@ class Paiement(RevMixin, AclMixin, models.Model): A boolean stating if usage is granted and an explanation message if the boolean is `False`. """ + can = self.available_for_everyone \ + or user.has_perm('cotisations.use_every_payment') \ + or user.has_perm('cotisations.add_facture') return ( - self.available_for_everyone - or user.has_perm('cotisations.use_every_payment') - or user.has_perm('cotisations.add_facture'), - _("You can't use this payment method."), + can, + _("You can't use this payment method.") if not can else None, ('cotisations.use_every_payment', 'cotisations.add_facture') ) diff --git a/machines/models.py b/machines/models.py index 11a4498b..68c7c58b 100644 --- a/machines/models.py +++ b/machines/models.py @@ -105,9 +105,10 @@ class Machine(RevMixin, FieldPermissionModelMixin, models.Model): A tuple with a boolean stating if edition is allowed and an explanation message. """ + can = user_request.has_perm('machines.change_machine_user') return ( - user_request.has_perm('machines.change_machine_user'), - _("You don't have the right to change the machine's user."), + can, + _("You don't have the right to change the machine's user.") if not can else None, ('machines.change_machine_user',) ) @@ -803,9 +804,10 @@ class Extension(RevMixin, AclMixin, models.Model): restrictions :param user_request: instance user qui fait l'edition :return: True ou False avec la raison de l'échec le cas échéant""" + can = user_request.has_perm('machines.use_all_extension') return ( - user_request.has_perm('machines.use_all_extension'), - _("You cannot use all extensions."), + can, + _("You cannot use all extensions.") if not can else None, ('machines.use_all_extension',) ) @@ -1294,9 +1296,10 @@ class Interface(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): def can_change_machine(user_request, *_args, **_kwargs): """Check if a user can change the machine associated with an Interface object """ + can = user_request.has_perm('machines.change_interface_machine') return ( - user_request.has_perm('machines.change_interface_machine'), - _("Permission required to edit the machine."), + can, + _("Permission required to edit the machine.") if not can else None, ('machines.change_interface_machine',) ) @@ -1421,10 +1424,11 @@ class Ipv6List(RevMixin, AclMixin, FieldPermissionModelMixin, models.Model): @staticmethod def can_change_slaac_ip(user_request, *_args, **_kwargs): """ Check if a user can change the slaac value """ + can = user_request.has_perm('machines.change_ipv6list_slaac_ip') return ( - user_request.has_perm('machines.change_ipv6list_slaac_ip'), + can, _("Permission required to change the SLAAC value of an IPv6" - " address"), + " address") if not can else None, ('machines.change_ipv6list_slaac_ip',) ) diff --git a/re2o/mixins.py b/re2o/mixins.py index 9d7f7b2f..dfa6e987 100644 --- a/re2o/mixins.py +++ b/re2o/mixins.py @@ -105,10 +105,11 @@ class AclMixin(object): :param user_request: instance utilisateur qui fait la requête :return: soit True, soit False avec la raison de l'échec""" permission = cls.get_modulename() + '.add_' + cls.get_classname() + can = user_request.has_perm(permission) return ( - user_request.has_perm(permission), + can, _("You don't have the right to create a %s object.") - % cls.get_classname(), + % cls.get_classname() if not can else None, (permission,) ) @@ -119,10 +120,11 @@ class AclMixin(object): :param user_request: Utilisateur qui fait la requête :return: soit True, soit False avec la raison de l'échec""" permission = self.get_modulename() + '.change_' + self.get_classname() + can = user_request.has_perm(permission) return ( - user_request.has_perm(permission), + can, _("You don't have the right to edit a %s object.") - % self.get_classname(), + % self.get_classname() if not can else None, (permission,) ) @@ -133,10 +135,11 @@ class AclMixin(object): :param user_request: Utilisateur qui fait la requête :return: soit True, soit False avec la raison de l'échec""" permission = self.get_modulename() + '.delete_' + self.get_classname() + can = user_request.has_perm(permission) return ( - user_request.has_perm(permission), + can, _("You don't have the right to delete a %s object.") - % self.get_classname(), + % self.get_classname() if not can else None, (permission,) ) @@ -147,10 +150,11 @@ class AclMixin(object): :param user_request: instance user qui fait l'edition :return: True ou False avec la raison de l'échec le cas échéant""" permission = cls.get_modulename() + '.view_' + cls.get_classname() + can = user_request.has_perm(permission) return ( - user_request.has_perm(permission), + can, _("You don't have the right to view every %s object.") - % cls.get_classname(), + % cls.get_classname() if not can else None, (permission,) ) @@ -161,10 +165,11 @@ class AclMixin(object): :param user_request: instance user qui fait l'edition :return: True ou False avec la raison de l'échec le cas échéant""" permission = self.get_modulename() + '.view_' + self.get_classname() + can = user_request.has_perm(permission) return ( - user_request.has_perm(permission), + can, _("You don't have the right to view a %s object.") - % self.get_classname(), + % self.get_classname() if not can else None, (permission,) ) diff --git a/tickets/models.py b/tickets/models.py index a3fe5e7a..e9acaf9b 100644 --- a/tickets/models.py +++ b/tickets/models.py @@ -86,9 +86,10 @@ class Ticket(AclMixin, models.Model): @staticmethod def can_view_all(user_request, *_args, **_kwargs): """ Check that the user has access to the list of all tickets""" + can = user_request.has_perm('tickets.view_tickets') return( - user_request.has_perm('tickets.view_tickets'), - _("You don't have the right to view the list of tickets."), + can, + _("You don't have the right to view the list of tickets.") if not can else None, ('tickets.view_tickets',) ) diff --git a/users/models.py b/users/models.py index faacc57f..66d53a96 100755 --- a/users/models.py +++ b/users/models.py @@ -968,9 +968,10 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, :returns: a message and a boolean which is True if the user has the right to change a state """ + can = user_request.has_perm('users.change_user_state') return ( - user_request.has_perm('users.change_user_state'), - _("Permission required to change the state."), + can, + _("Permission required to change the state.") if not can else None, ('users.change_user_state',) ) @@ -999,9 +1000,10 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, :returns: a message and a boolean which is True if the user has the right to change a redirection """ + can = OptionalUser.get_cached_value('local_email_accounts_enabled') return ( - OptionalUser.get_cached_value('local_email_accounts_enabled'), - _("Local email accounts must be enabled."), + can, + _("Local email accounts must be enabled.") if not can else None, None ) @@ -1013,9 +1015,10 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, :returns: a message and a boolean which is True if the user has the right to change internal address """ + can = OptionalUser.get_cached_value('local_email_accounts_enabled') return ( - OptionalUser.get_cached_value('local_email_accounts_enabled'), - _("Local email accounts must be enabled."), + can, + _("Local email accounts must be enabled.") if not can else None, None ) @@ -1027,9 +1030,10 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, :returns: a message and a boolean which is True if the user has the right to change a force """ + can = user_request.has_perm('users.change_user_force') return ( - user_request.has_perm('users.change_user_force'), - _("Permission required to force the move."), + can, + _("Permission required to force the move.") if not can else None, ('users.change_user_force',) ) @@ -1041,9 +1045,10 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, :returns: a message and a boolean which is True if the user has the right to change a group """ + can = user_request.has_perm('users.change_user_grou') return ( - user_request.has_perm('users.change_user_groups'), - _("Permission required to edit the user's groups of rights."), + can, + _("Permission required to edit the user's groups of rights.") if not can else None, ('users.change_user_groups') ) @@ -1054,9 +1059,10 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, :param user_request: The user who request :returns: a message and a boolean which is True if permission is granted. """ + can = user_request.is_superuser return ( - user_request.is_superuser, - _("'superuser' right required to edit the superuser flag."), + can, + _("'superuser' right required to edit the superuser flag.") if not can else None, [] ) @@ -1099,9 +1105,10 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, :return: True if the user can view the list and an explanation message. """ + can = user_request.has_perm('use.view_user') return ( - user_request.has_perm('users.view_user'), - _("You don't have the right to view the list of users."), + can, + _("You don't have the right to view the list of users.") if not can else None, ('users.view_user',) ) @@ -1113,9 +1120,10 @@ class User(RevMixin, FieldPermissionModelMixin, AbstractBaseUser, :return: True if user_request has the right 'bureau', and a message. """ + can = user_request.has_perm('users.delete_user') return ( - user_request.has_perm('users.delete_user'), - _("You don't have the right to delete this user."), + can, + _("You don't have the right to delete this user.") if not can else None, ('users.delete_user',) ) @@ -1209,9 +1217,10 @@ class Adherent(User): OptionalUser.get_cached_value('self_adhesion')): return True, None, None else: + can = user_request.has_perm('users.add_user') return ( - user_request.has_perm('users.add_user'), - _("You don't have the right to create a user."), + can, + _("You don't have the right to create a user.") if not can else None, ('users.add_user',) ) @@ -1265,9 +1274,10 @@ class Club(User): if OptionalUser.get_cached_value('all_can_create_club'): return True, None, None else: + can = user_request.has_perm('users.add_user') return ( - user_request.has_perm('users.add_user'), - _("You don't have the right to create a club."), + can, + _("You don't have the right to create a club.") if not can else None, ('users.add_user',) )