* Fixed some permission issues. see #204

* The 'limits' extension has no need to modify the DomainAdmins group and a domainadmin resources must not include the 'domains' limit. see #105

admin/fixtures/initial_data.json

@@ -1 +1 @@
-[{"pk": 1, "model": "auth.group", "fields": {"name": "DomainAdmins", "permissions": [["add_mailbox", "admin", "mailbox"], ["change_mailbox", "admin", "mailbox"], ["delete_mailbox", "admin", "mailbox"], ["add_alias", "admin", "alias"], ["change_alias", "admin", "alias"], ["delete_alias", "admin", "alias"], ["view_domain", "admin", "domain"], ["view_mailboxes", "admin", "mailbox"], ["view_aliases", "admin", "alias"], ["view_domaliases", "admin", "domainalias"], ["add_domainalias", "admin", "domainalias"], ["change_domainalias", "admin", "domainalias"], ["delete_domainalias", "admin", "domainalias"]]}},{"pk": 2, "model": "auth.group", "fields": {"name": "SimpleUsers", "permissions": []}},{"pk": 1, "model": "auth.user", "fields": {"username": "admin", "first_name": "", "last_name": "", "is_active": true, "is_superuser": true, "is_staff": false, "last_login": "2012-01-26 15:19:25", "groups": [], "user_permissions": [], "password": "{CRYPT}dTTsGDkA5ZHKg", "email": "", "date_joined": "2012-01-26 15:19:25"}}]
+[{"pk": 1, "model": "auth.group", "fields": {"name": "DomainAdmins", "permissions": [["add_user", "auth", "user"], ["change_user", "auth", "user"], ["delete_user", "auth", "user"], ["add_mailbox", "admin", "mailbox"], ["change_mailbox", "admin", "mailbox"], ["delete_mailbox", "admin", "mailbox"], ["add_alias", "admin", "alias"], ["change_alias", "admin", "alias"], ["delete_alias", "admin", "alias"], ["view_mailboxes", "admin", "mailbox"], ["view_aliases", "admin", "alias"], ["view_domaliases", "admin", "domainalias"], ["add_domainalias", "admin", "domainalias"], ["change_domainalias", "admin", "domainalias"], ["delete_domainalias", "admin", "domainalias"]]}},{"pk": 2, "model": "auth.group", "fields": {"name": "SimpleUsers", "permissions": []}},{"pk": 1, "model": "auth.user", "fields": {"username": "admin", "first_name": "", "last_name": "", "is_active": true, "is_superuser": true, "is_staff": false, "last_login": "2012-01-26 15:19:25", "groups": [], "user_permissions": [], "password": "{CRYPT}dTTsGDkA5ZHKg", "email": "", "date_joined": "2012-01-26 15:19:25"}}]

admin/fixtures/test_content.json

@@ -1 +1,2 @@
-[{"pk": 2, "model": "admin.User", "fields": {"username": "user@test.com", "is_active": "True", "is_superuser": "False", "password": "{CLEAN}toto"}},{"pk": 1, "model": "admin.ObjectDates", "fields": {"creation": "2011-09-07 09:27:07.579603"}}, {"pk": 1, "model": "admin.Domain", "fields": {"name" : "test.com", "quota" : "100", "dates": "1"}}, {"pk": 1, "model": "admin.Mailbox", "fields": {"address": "user", "domain": "1", "dates": "1", "uid": "1001", "gid": "1001", "path": "user", "quota": "10", "user": "2"}}]
+[{"pk": 2, "model": "auth.user", "fields": {"username": "user@test.com", "is_active": "True", "is_superuser": "False", "password": "{PLAIN}toto", "groups": [2]}},{"pk": 1, "model": "admin.ObjectDates", "fields": {"creation": "2011-09-07 09:27:07.579603"}}, {"pk": 1, "model": "admin.Domain", "fields": {"name" : "test.com", "quota" : "100", "dates": "1"}}, {"pk": 1, "model": "admin.Mailbox", "fields": {"address": "user", "domain": "1", "dates": "1", "uid": "1001", "gid": "1001", "path": "user", "quota": "10", "user": "2"}}]
+        

admin/forms.py

@@ -166,7 +166,7 @@ class DlistForm(forms.ModelForm, DynamicForm):
             except Domain.DoesNotExist:
                 domain = None
             if domain:
-                if not user.is_owner(domain):
+                if not user.can_access(domain):
                     raise PermDeniedException(v)
                 try:
                     mb = Mailbox.objects.get(domain=domain, address=local_part)
@@ -244,15 +244,13 @@ class AccountFormGeneral(forms.ModelForm):
             account.save()
             if self.cleaned_data.has_key("role"):
                 role = self.cleaned_data["role"]
-            else:
-                role = "SimpleUsers"
-            account.groups.clear()
-            if role == "SuperAdmins":
-                account.is_superuser = True
-            else:
-                account.is_superuser = False
-                account.groups.add(Group.objects.get(name=role))
-            account.save()
+                account.groups.clear()
+                if role == "SuperAdmins":
+                    account.is_superuser = True
+                else:
+                    account.is_superuser = False
+                    account.groups.add(Group.objects.get(name=role))
+                account.save()
         return account
 
 class AccountFormGeneralPwd(AccountFormGeneral):
@@ -438,7 +436,7 @@ class AccountForm(TabForms):
     def check_perms(self, account):
         if account.is_superuser:
             return False
-        return account.has_perm("admin.view_domain")
+        return account.has_perm("auth.add_user")
 
     def _before_is_valid(self, form):
         if form["id"] == "general":

admin/models.py

@@ -43,14 +43,21 @@ class User(DUser):
 
     def delete(self, fromuser, keep_mb_dir, *args, **kwargs):
         """Custom delete method
+
+        To check permissions properly, we need to make a distinction
+        between 2 cases:
+
+        * If the user owns a mailbox, the check is made on that object
+          (useful for domain admins)
+
+        * Otherwise, the check is made on the user
         """
         from modoboa.lib.permissions import \
             get_object_owner, grant_access_to_object, ungrant_access_to_object
 
-        if not fromuser.can_access(self):
-            raise PermDeniedException(_("You don't have access to this account"))
         if fromuser == self:
             raise AdminError(_("You can't delete your own account"))
+
         if self.has_mailbox:
             mb = self.mailbox_set.all()[0]
             if not fromuser.can_access(mb.domain):
@@ -58,6 +65,8 @@ class User(DUser):
             events.raiseEvent("DeleteMailbox", mb)
             ungrant_access_to_object(mb)
             mb.delete(keepdir=keep_mb_dir)
+        elif not fromuser.can_access(self):
+            raise PermDeniedException(_("You don't have access to this account"))
 
         owner = get_object_owner(self)
         for ooentry in self.objectaccess_set.all():
@@ -700,7 +709,8 @@ class Alias(DatesAware):
         Internal and external addresses are mixed into a single list.
         """
         result = map(lambda mb: mb.full_address, self.mboxes.all())
-        result += self.extmboxes.split(',')
+        if self.extmboxes != "":
+            result += self.extmboxes.split(',')
         return result
 
     def ui_disabled(self, user):

admin/templatetags/admin_extras.py

@@ -19,22 +19,19 @@ def admin_menu(selection, user):
         entries += [
             {"name" : "domains",
              "url" : reverse(admin.views.domains),
-             "label" : _("Domains"),
-             "img" : static_url("pics/domains.png")}
+             "label" : _("Domains")}
             ]
     entries += \
         events.raiseQueryEvent("AdminMenuDisplay", "top_menu", user)
-    if user.has_perm("auth.add_user") or user.has_perm("admin.add_mailbox"):
+    if user.has_perm("auth.add_user") or user.has_perm("admin.add_alias"):
         entries += [
             {"name" : "identities",
              "url" : reverse(admin.views.identities),
              "label" : _("Identities")},
-             
             ]
     if user.is_superuser:
         entries += [
             {"name" : "settings",
-             "img" : static_url("pics/settings.png"),
              "label" : _("Modoboa"),
              "url" : reverse(admin.views.viewparameters)}
             ]

admin/tests.py

@@ -1,3 +1,16 @@
+# coding: utf-8
+"""
+
+Liste des tests effectués
+=========================
+
+Permissions
+-----------
+
+* Vérifier qu'un admin de domaine qui modifie son propre compte laisse
+  le rôle intact
+
+"""
 from django.test import TestCase
 from django.test.client import Client
 from django.utils import simplejson
@@ -141,20 +154,19 @@ class PermissionsTestCase(TestCase):
         self.clt.logout()
 
     def test_domain_admins(self):
-        response = self.clt.post("/modoboa/admin/permissions/domainadmin/promote/",
-                                 {"name" : self.user.username})
+        response = self.clt.post("/modoboa/admin/accounts/edit/%d/" % self.user.id,
+                                 {"role" : "DomainAdmins"})
         self.assertEqual(response.status_code, 200)
         obj = simplejson.loads(response.content)
         self.assertEqual(obj["status"], "ok")
-        self.assertEqual(self.user.belongs_to_group('DomainAdmins'), True)
+        self.assertEqual(self.user.group == "DomainAdmins", True)
 
-        response = self.clt.get("/modoboa/admin/permissions/delete/",
-                                {"role" : "domain_admins", 
-                                 "selection" : self.user.id})
+        response = self.clt.get("/modoboa/admin/accounts/edit/%d/" % self.user.id,
+                                {"role" : "SimpleUsers"})
         self.assertEqual(response.status_code, 200)
         obj = simplejson.loads(response.content)
         self.assertEqual(obj["status"], "ok")
-        self.assertEqual(self.user.belongs_to_group('DomainAdmins'), False)
+        self.assertEqual(self.user.group == 'DomainAdmins', False)
 
     def test_superusers(self):
         response = self.clt.post("/modoboa/admin/permissions/add/",

admin/views.py

@@ -37,7 +37,7 @@ def domains(request):
                           objects=domains)
 
 @login_required
-@permission_required("admin.view_domains")
+@permission_required("auth.add_user")
 def domains_list(request):
     doms = map(lambda dom: dom.name, request.user.get_domains())
     return ajax_simple_response(doms)
@@ -245,7 +245,7 @@ def deldlist(request):
     return ajax_response(request)
 
 @login_required
-@user_passes_test(lambda u: u.has_perm("auth.add_user") or u.has_perm("admin.add_mailbox"))
+@user_passes_test(lambda u: u.has_perm("auth.add_user") or u.has_perm("admin.add_alias"))
 def identities(request, tplname='admin/identities.html'):
     accounts_list = []
     for account in User.objects.all():
@@ -253,17 +253,19 @@ def identities(request, tplname='admin/identities.html'):
                 account.has_mailbox and request.user.can_access(account.mailbox_set.all()[0].domain):
             accounts_list += [account]
     for mbalias in Alias.objects.all():
-        if len(mbalias.get_recipients()) > 2:
+        if len(mbalias.get_recipients()) >= 2:
             accounts_list += [mbalias]
     return render_listing(request, "identities", tplname, objects=accounts_list)
 
 @login_required
+@permission_required("auth.add_user")
 def accounts_list(request):
     accs = User.objects.filter(is_superuser=False).exclude(groups__name='SimpleUsers')
     res = map(lambda a: a.username, accs.all())
     return ajax_simple_response(res)
 
 @login_required
+@permission_required("auth.add_user")
 @transaction.commit_on_success
 def newaccount(request, tplname='admin/newaccount.html'):
     from modoboa.lib.formutils import CreationWizard
@@ -315,6 +317,7 @@ def newaccount(request, tplname='admin/newaccount.html'):
     return _render(request, tplname, ctx)
 
 @login_required
+@permission_required("auth.change_user")
 @transaction.commit_on_success
 def editaccount(request, accountid, tplname="common/tabforms.html"):
     account = User.objects.get(pk=accountid)
@@ -352,8 +355,7 @@ def editaccount(request, accountid, tplname="common/tabforms.html"):
     return _render(request, tplname, ctx)
 
 @login_required
-@user_passes_test(lambda u: u.has_perm("auth.delete_user") or \
-                            u.has_perm("admin.delete_mailbox"))
+@permission_required("auth.delete_user")
 @transaction.commit_on_success
 def delaccount(request):
     selection = request.GET["selection"].split(",")
@@ -367,7 +369,7 @@ def delaccount(request):
     return ajax_response(request)
 
 @login_required
-@permission_required("auth.view_permissions")
+@permission_required("auth.add_account")
 @check_domain_access
 def search_account(request):
     search = request.POST.get("search", None)
@@ -518,6 +520,7 @@ def importdata(request, typ):
             })
 
 @login_required
+@user_passes_test(lambda u: u.has_perm("auth.add_user") or u.has_perm("auth.add_alias"))
 @transaction.commit_on_success
 def import_identities(request):
     if request.method == "POST":
@@ -536,6 +539,7 @@ def import_identities(request):
     return _render(request, "admin/importform.html", ctx)
 
 @login_required
+@permission_required("admin.add_domain")
 @transaction.commit_on_success
 def import_domains(request):
     if request.method == "POST":

extensions/limits/__init__.py

@@ -24,18 +24,20 @@ def init():
 
     ct = ContentType.objects.get(app_label="admin", model="domain")
     dagrp = Group.objects.get(name="DomainAdmins")
-    for pname in ["view_domains", "add_domain", "change_domain", "delete_domain"]:
-        perm = Permission.objects.get(content_type=ct, codename=pname)
-        dagrp.permissions.add(perm)
-    dagrp.save()
 
     grp = Group(name="Resellers")
     grp.save()
     grp.permissions.add(*dagrp.permissions.all())
+
     ct = ContentType.objects.get_for_model(Permission)
-    grp.permissions.add(Permission.objects.get(
-            content_type=ct, codename="view_permissions"
-            ))
+    for pname in ["view_permissions"]:
+        perm = Permission.objects.get(content_type=ct, codename=pname)
+        grp.permissions.add(perm)
+
+    ct = ContentType.objects.get_for_model(Domain)
+    for pname in ["view_domains", "add_domain", "change_domain", "delete_domain"]:
+        perm = Permission.objects.get(content_type=ct, codename=pname)
+        grp.permissions.add(perm)
     grp.save()
 
     for user in User.objects.filter(groups__name='DomainAdmins'):
@@ -45,12 +47,6 @@ def init():
             pass
 
 def destroy():
-    ct = ContentType.objects.get(app_label="admin", model="domain")
-    grp = Group.objects.get(name="DomainAdmins")
-    for pname in ["view_domains", "add_domain", "change_domain", "delete_domain"]:
-        perm = Permission.objects.get(content_type=ct, codename=pname)
-        grp.permissions.remove(perm)
-    grp.save()
     Group.objects.get(name="Resellers").delete()
 
 def infos():

extensions/limits/forms.py

@@ -26,8 +26,9 @@ class ResourcePoolForm(forms.Form):
             del kwargs["instance"]
         super(ResourcePoolForm, self).__init__(*args, **kwargs)
         if hasattr(self, "account"):
-            if not self.account.belongs_to_group("Resellers"):
+            if self.account.group != "Resellers":
                 del self.fields["domain_admins_limit"]
+                del self.fields["domains_limit"]
             self.load_from_user(self.account)
 
     def check_limit_value(self, lname):

static/js/dynamic_input.js

@@ -1,6 +1,5 @@
 (function($) {
     var DynamicInput = function(element, options) {
-        console.log(element);
         this.$element = $(element);
         this.options = $.extend({}, $.fn.dynamic_input.defaults, options);
         this.nextid = 1;

static/js/utils.js

@@ -210,7 +210,7 @@ function simple_ajax_request(e) {
     var $this = $(this);
     e.preventDefault();
     $.ajax({
-        url: $this.attr("herf"),
+        url: $this.attr("href"),
         dataType: 'json',
         success: function(data) {
             if (data.status == "ok") {

templates/admin/domains.html

@@ -8,9 +8,6 @@
             question: gettext("Delete this domain?"),
             checkboxes: {keepdir: gettext("Do not delete domain directory")}
         });
-
-        {% url admin.views.mailboxes as view_url %}
-        //bindRows("{{ view_url }}", "{{ current_page }}");
     });
   </script>
 {% endblock %}

templates/userprefs/preferences.html

@@ -25,7 +25,7 @@ $(document).ready(function() {
     {% include "common/parameters.html" %}
   </form>
 {% endblock %}
-{% block footer %}
+{% block footerright %}
 <div class="pull-right padded5">
   <a href="#" class="btn btn-primary" id="update" name="update">{% trans 'Save' %}</a>
 </div>