Merge branch 'feature-template-fixes' into 'master'

Template ACL fixes

👌 tests

fixes #195
fixes #194
fixes #208

circle/circle/settings/base.py

@@ -260,6 +260,7 @@ THIRD_PARTY_APPS = (
     'taggit',
     'statici18n',
     'django_sshkey',
+    'autocomplete_light',
 )
 
 # Apps specific for this project go here.

circle/dashboard/autocomplete_light_registry.py

+import autocomplete_light
+from django.utils.translation import ugettext as _
+
+from .views import AclUpdateView
+
+
+class AclUserAutocomplete(autocomplete_light.AutocompleteGenericBase):
+    search_fields = (
+        ('^first_name', 'last_name', 'username', '^email', 'profile__org_id'),
+        ('^name', 'groupprofile__org_id'),
+    )
+    autocomplete_js_attributes = {'placeholder': _("Name of group or user")}
+
+    def choices_for_request(self):
+        user = self.request.user
+        self.choices = (AclUpdateView.get_allowed_users(user),
+                        AclUpdateView.get_allowed_groups(user))
+        return super(AclUserAutocomplete, self).choices_for_request()
+
+
+autocomplete_light.register(AclUserAutocomplete)

circle/dashboard/forms.py

@@ -27,6 +27,7 @@ from django.contrib.auth.models import User, Group
 from django.core.validators import URLValidator
 from django.core.exceptions import PermissionDenied, ValidationError
 
+import autocomplete_light
 from crispy_forms.helper import FormHelper
 from crispy_forms.layout import (
     Layout, Div, BaseInput, Field, HTML, Submit, Fieldset, TEMPLATE_PACK,
@@ -44,7 +45,6 @@ from django.core.urlresolvers import reverse_lazy
 
 from django_sshkey.models import UserKey
 from firewall.models import Vlan, Host
-from storage.models import Disk
 from vm.models import (
     InstanceTemplate, Lease, InterfaceTemplate, Node, Trait, Instance
 )
@@ -78,7 +78,7 @@ class VmCustomizeForm(forms.Form):
     amount = forms.IntegerField(min_value=0, initial=1)
 
     disks = forms.ModelMultipleChoiceField(
-        queryset=None, required=True)
+        queryset=None, required=False)
     networks = forms.ModelMultipleChoiceField(
         queryset=None, required=False)
 
@@ -91,8 +91,7 @@ class VmCustomizeForm(forms.Form):
         super(VmCustomizeForm, self).__init__(*args, **kwargs)
 
         # set displayed disk and network list
-        self.fields['disks'].queryset = Disk.get_objects_with_level(
-            'user', self.user).exclude(type="qcow2-snap")
+        self.fields['disks'].queryset = self.template.disks.all()
         self.fields['networks'].queryset = Vlan.get_objects_with_level(
             'user', self.user)
 
@@ -596,8 +595,12 @@ class TemplateForm(forms.ModelForm):
             n = self.instance.interface_set.values_list("vlan", flat=True)
             self.initial['networks'] = n
 
-        self.allowed_fields = (
-            'name', 'access_method', 'description', 'system', 'tags')
+        if self.instance.pk and not self.instance.has_level(self.user,
+                                                            'owner'):
+            self.allowed_fields = ()
+        else:
+            self.allowed_fields = (
+                'name', 'access_method', 'description', 'system', 'tags')
         if self.user.has_perm('vm.change_template_resources'):
             self.allowed_fields += tuple(set(self.fields.keys()) -
                                          set(['raw_data']))
@@ -675,6 +678,11 @@ class TemplateForm(forms.ModelForm):
 
     @property
     def helper(self):
+        submit_kwargs = {}
+        if self.instance.pk and not self.instance.has_level(self.user,
+                                                            'owner'):
+            submit_kwargs['disabled'] = None
+
         helper = FormHelper()
         helper.layout = Layout(
             Field("name"),
@@ -739,7 +747,7 @@ class TemplateForm(forms.ModelForm):
                 Field("tags"),
             ),
         )
-        helper.add_input(Submit('submit', 'Save changes'))
+        helper.add_input(Submit('submit', 'Save changes', **submit_kwargs))
         return helper
 
     class Meta:
@@ -1178,6 +1186,11 @@ class UserCreationForm(OrgUserCreationForm):
         return user
 
 
+class AclUserAddForm(forms.Form):
+    name = forms.CharField(widget=autocomplete_light.TextWidget(
+        'AclUserAutocomplete', attrs={'class': 'form-control'}))
+
+
 class UserKeyForm(forms.ModelForm):
     name = forms.CharField(required=True, label=_('Name'))
     key = forms.CharField(

circle/dashboard/models.py

@@ -142,6 +142,11 @@ class Profile(Model):
     def __unicode__(self):
         return self.get_display_name()
 
+    class Meta:
+        permissions = (
+            ('use_autocomplete', _('Can use autocomplete.')),
+        )
+
 
 class FutureMember(Model):
     org_id = CharField(max_length=64, help_text=_(

circle/dashboard/templates/base.html

@@ -70,6 +70,7 @@
   <script src="//code.jquery.com/jquery-1.11.1.min.js"></script>
   <script src="//netdna.bootstrapcdn.com/bootstrap/3.0.0/js/bootstrap.min.js"></script>
   <script src="{{ STATIC_URL }}jsi18n/{{ LANGUAGE_CODE }}/djangojs.js"></script>
+  {% include 'autocomplete_light/static.html' %}
 
   {% block extra_script %}
   {% endblock %}

circle/dashboard/templates/dashboard/_disk-list-element.html

@@ -10,10 +10,12 @@
   <div class="label label-danger"{% if user.is_superuser %} title="{{ d.get_latest_activity_result }}"{% endif %}>{% trans "failed" %}</div>
   {% endif %}
 {% else %}<span class="disk-list-disk-percentage" data-disk-pk="{{ d.pk }}">{{ d.get_download_percentage }}</span>%{% endif %}
+{% if is_owner != False %}
 <a href="{% url "dashboard.views.disk-remove" pk=d.pk %}?next={{ request.path }}"
   data-disk-pk="{{ d.pk }}" class="btn btn-xs btn-danger pull-right disk-remove"
   {% if not long_remove %}title="{% trans "Remove" %}"{% endif %}
   >
   <i class="fa fa-times"></i>{% if long_remove %} {% trans "Remove" %}{% endif %}
 </a>
+{% endif %}
 <div style="clear: both;"></div>

circle/dashboard/templates/dashboard/_manage_access.html

+{% load i18n %}
+<form action="{{ acl.url }}" method="post">{% csrf_token %}
+    <table class="table table-striped table-with-form-fields" id="{{table_id}}">
+    <thead>
+        <tr>
+        <th></th>
+        <th>{% trans "Who" %}</th>
+        <th>{% trans "What" %}</th>
+        <th><i class="fa fa-times"></i></th>
+        </tr>
+    </thead>
+    <tbody>
+        {% for i in acl.users %}
+        <tr>
+        <td>
+            <i class="fa fa-user"></i>
+        </td>
+        <td>
+            <a href="{% url "dashboard.views.profile" username=i.user.username %}"
+            title="{{ i.user.username }}">
+            {% include "dashboard/_display-name.html" with user=i.user show_org=True %}
+            </a>
+        </td>
+        <td>
+            <select class="form-control" name="perm-u-{{i.user.id}}"{% if i.level not in acl.allowed_levels %} disabled{% endif %}>
+            {% for id, name in acl.levels %}
+            <option{%if id = i.level%} selected="selected"{%endif%}
+            {% if id not in acl.allowed_levels %} disabled{% endif %}
+                value="{{id}}">{{name}}</option>
+            {% endfor %}
+            </select>
+        </td>
+        <td>
+            <input type="checkbox" name="remove-u-{{i.user.id}}" title="{% trans "Remove" %}"/>
+        </td>
+        </tr>
+        {% endfor %}
+        {% for i in acl.groups %}
+        <tr>
+        <td><i class="fa fa-group"></i></td>
+        <td>
+            <a href="{% url "dashboard.views.group-detail" pk=i.group.pk %}">
+            {{i.group}}
+            </a>
+        </td>
+        <td>
+            <select class="form-control" name="perm-g-{{i.group.id}}{% if i.level not in acl.allowed_levels %} disabled{% endif %}">
+            {% for id, name in acl.levels %}
+            <option{%if id = i.level%} selected="selected"{%endif%}
+            {% if id not in acl.allowed_levels %} disabled{% endif %}
+                value="{{id}}">{{name}}</option>
+            {% endfor %}
+            </select>
+        </td>
+        <td>
+            <input type="checkbox" name="remove-g-{{i.group.id}}" title="{% trans "Remove" %}"/>
+        </td>
+        </tr>
+        {% endfor %}
+        <tr><td><i class="fa fa-plus"></i></td>
+            <td>{{aclform.name }}</td>
+            <td><select class="form-control" name="level">
+                    {% for id, name in acl.levels %}
+                    {% if id in acl.allowed_levels %}
+                    <option value="{{id}}">{{name}}</option>
+                    {% endif %}
+                    {% endfor %}
+            </select></td><td></td>
+        </tr>
+    </tbody>
+    </table>
+    <div class="form-actions">
+    <button type="submit" class="btn btn-success">{% trans "Save" %}</button>
+    </div>
+</form>

circle/dashboard/templates/dashboard/group-detail.html

@@ -104,76 +104,7 @@
 
   <hr />
 <h3 id="group-detail-perm-header">{% trans "Access permissions"|capfirst %}</h3>
-  <form action="{{acl.url}}" method="post">{% csrf_token %}
-  <table class="table table-striped table-with-form-fields table-bordered" id="group-detail-perm-table">
-    <thead>
-      <tr>
-        <th></th><th>{% trans "Who" %}</th><th>{% trans "What" %}</th><th>{% trans "Remove" %}</th>
-      </tr>
-    </thead>
-    <tbody>
-    {% for i in acl.users %}
-    <tr>
-      <td>
-        <i class="fa fa-user"></i>
-      </td>
-      <td>
-        <a href="{% url "dashboard.views.profile" username=i.user.username %}" title="{{ i.user.username }}"
-          >{% include "dashboard/_display-name.html" with user=i.user show_org=True %}</a>
-      </td>
-      <td>
-        <select class="form-control" name="perm-u-{{i.user.id}}">
-        {% for id, name in acl.levels %}
-          <option{%if id = i.level%} selected="selected"{%endif%} value="{{id}}">{{name}}</option>
-        {% endfor %}
-        </select>
-      </td>
-      <td class="user-remove"><a data-group_pk="{{ group.pk }}" data-member_pk="{{i.user.pk }}" href="{% url "dashboard.views.remove-acluser" member_pk=i.user.pk group_pk=group.pk %}" class="real-link delete-from-group btn btn-link btn-xs"><i class="fa fa-times"><span class="sr-only">{% trans "remove" %}</span></i></a></td>
-    </tr>
-    {% endfor %}
-
-    {% for i in acl.groups %}
-      <tr>
-        <td>
-          <i class="fa fa-group"></i>
-        </td>
-        <td>
-          <a href="{% url "dashboard.views.group-detail" pk=i.group.pk %}">{{ i.group }}</a>
-        </td>
-        <td>
-          <select class="form-control" name="perm-g-{{ i.group.pk }}">
-          {% for id, name in acl.levels %}
-            <option{%if id = i.level%} selected="selected"{%endif%} value="{{id}}">{{name}}</option>
-          {% endfor %}
-          </select>
-        </td>
-        <td class="user-remove"><a data-group_pk="{{ i.pk }}"data-member_pk="{{i.group.pk }}" href="{% url "dashboard.views.remove-aclgroup" member_pk=i.group.pk group_pk=group.pk %}" class="real-link delete-from-group btn btn-link btn-xs"><i class="fa fa-times"><span class="sr-only">{% trans "remove" %}</span></i></a>
-        </td>
-      </tr>
-      {% endfor %}
-      <tr>
-        <td><i class="fa fa-plus"></i></td>
-        <td>
-          <input type="text" class="form-control" name="perm-new-name"
-          placeholder="{% trans "Name of group or user" %}">
-        </td>
-        <td>
-          <select class="form-control" name="perm-new">
-            {% for id, name in acl.levels %}
-              <option value="{{id}}">{{name}}</option>
-            {% endfor %}
-          </select>
-        </td>
-        <td></td>
-      </tr>
-  </tbody>
-  </table>
-
-  <div class="form-actions">
-    <button type="submit" class="btn btn-success">{% trans "Save" %}</button>
-  </div>
-  </form>
-
+{% include "dashboard/_manage_access.html" with table_id="group-detail-perm-table" %}
 {% if user.is_superuser %}
   <hr />
 

circle/dashboard/templates/dashboard/template-edit.html

@@ -29,75 +29,7 @@
         <h4 class="no-margin"><i class="fa fa-group"></i> {% trans "Manage access"  %}</h4>
       </div>
       <div class="panel-body">
-        <form action="{% url "dashboard.views.template-acl" pk=object.pk %}" method="post">{% csrf_token %}
-          <table class="table table-striped table-with-form-fields" id="template-access-table">
-            <thead>
-              <tr>
-                <th></th>
-                <th>{% trans "Who" %}</th>
-                <th>{% trans "What" %}</th>
-                <th><i class="fa fa-times"></i></th>
-              </tr>
-            </thead>
-            <tbody>
-              {% for i in acl.users %}
-              <tr>
-                <td>
-                  <i class="fa fa-user"></i>
-                </td>
-                <td>
-                  <a href="{% url "dashboard.views.profile" username=i.user.username %}"
-                    title="{{ i.user.username }}">
-                    {% include "dashboard/_display-name.html" with user=i.user show_org=True %}
-                  </a>
-                </td>
-                <td>
-                  <select class="form-control" name="perm-u-{{i.user.id}}">
-                    {% for id, name in acl.levels %}
-                    <option{%if id = i.level%} selected="selected"{%endif%} value="{{id}}">{{name}}</option>
-                    {% endfor %}
-                  </select>
-                </td>
-                <td>
-                  <input type="checkbox" name="remove-u-{{i.user.id}}" title="{% trans "Remove" %}"/>
-                </td>
-              </tr>
-              {% endfor %}
-              {% for i in acl.groups %}
-              <tr>
-                <td><i class="fa fa-group"></i></td>
-                <td>
-                  <a href="{% url "dashboard.views.group-detail" pk=i.group.pk %}">
-                    {{i.group}}
-                  </a>
-                </td>
-                <td>
-                  <select class="form-control" name="perm-g-{{i.group.id}}">
-                    {% for id, name in acl.levels %}
-                    <option{%if id = i.level%} selected="selected"{%endif%} value="{{id}}">{{name}}</option>
-                    {% endfor %}
-                  </select>
-                </td>
-                <td>
-                  <input type="checkbox" name="remove-g-{{i.group.id}}" title="{% trans "Remove" %}"/>
-                </td>
-              </tr>
-              {% endfor %}
-              <tr><td><i class="fa fa-plus"></i></td>
-                  <td><input type="text" class="form-control" name="perm-new-name"
-                      placeholder="{% trans "Name of group or user" %}"></td>
-                  <td><select class="form-control" name="perm-new">
-                          {% for id, name in acl.levels %}
-                          <option value="{{id}}">{{name}}</option>
-                          {% endfor %}
-                  </select></td><td></td>
-              </tr>
-            </tbody>
-          </table>
-          <div class="form-actions">
-            <button type="submit" class="btn btn-success">{% trans "Save" %}</button>
-          </div>
-        </form>
+        {% include "dashboard/_manage_access.html" with table_id="template-access-table" %}
       </div>
     </div>
 

circle/dashboard/templates/dashboard/vm-detail/access.html

@@ -14,64 +14,4 @@
   {% endif %}
 </p>
 <h3>{% trans "Permissions"|capfirst %}</h3>
-  <form action="{{acl.url}}" method="post">{% csrf_token %}
-  <table class="table table-striped table-with-form-fields" id="vm-access-table">
-    <thead><tr>
-        <th></th>
-        <th>{% trans "Who" %}</th>
-        <th>{% trans "What" %}</th>
-        <th>{% trans "Remove" %}</th>
-    </tr></thead>
-    <tbody>
-        {% for i in acl.users %}
-        <tr>
-          <td><i class="fa fa-user"></i></td>
-          <td>
-            <a href="{% url "dashboard.views.profile" username=i.user.username %}" title="{{ i.user.username }}"
-              >{% include "dashboard/_display-name.html" with user=i.user show_org=True %}</a>
-          </td>
-          <td>
-            <select class="form-control" name="perm-u-{{i.user.id}}">
-              {% for id, name in acl.levels %}
-                <option{%if id = i.level%} selected="selected"{%endif%} value="{{id}}">{{name}}</option>
-              {% endfor %}
-            </select>
-          </td>
-          <td>
-            <input type="checkbox" name="remove-u-{{i.user.id}}"/>
-          </td>
-        </tr>
-        {% endfor %}
-        {% for i in acl.groups %}
-        <tr>
-          <td><i class="fa fa-group"></i></td>
-          <td>
-            <a href="{% url "dashboard.views.group-detail" pk=i.group.pk %}"
-              >{{ i.group.name }}</a>
-          </td>
-          <td>
-            <select class="form-control" name="perm-g-{{i.group.id}}">
-              {% for id, name in acl.levels %}
-              <option{%if id = i.level%} selected="selected"{%endif%} value="{{id}}">{{name}}</option>
-              {% endfor %}
-            </select></td>
-          <td>
-            <input type="checkbox" name="remove-g-{{i.group.id}}"/>
-          </td>
-        </tr>
-        {% endfor %}
-        <tr><td><i class="fa fa-plus"></i></td>
-            <td><input type="text" class="form-control" name="perm-new-name"
-                placeholder="{% trans "Name of group or user" %}"></td>
-            <td><select class="form-control" name="perm-new">
-                    {% for id, name in acl.levels %}
-                    <option value="{{id}}">{{name}}</option>
-                    {% endfor %}
-            </select></td><td></td>
-        </tr>
-  </tbody>
-  </table>
-  <div class="form-actions">
-    <button type="submit" class="btn btn-success">{% trans "Save" %}</button>
-  </div>
-  </form>
+{% include "dashboard/_manage_access.html" with table_id="vm-access-table" %}

circle/dashboard/tests/test_mockedviews.py

@@ -30,6 +30,7 @@ from django.utils import baseconv
 from ..models import Profile
 from ..views import InstanceActivityDetail, InstanceActivity
 from ..views import vm_ops, Instance, UnsubscribeFormView
+from ..views import AclUpdateView
 from .. import views
 
 
@@ -429,6 +430,79 @@ class RenewViewTest(unittest.TestCase):
                           view.as_view()(request, pk=1234)['location'])
 
 
+class AclUpdateViewTest(unittest.TestCase):
+    def test_has_next_level(self):
+        data = {None: 'user', 'user': 'operator', 'operator': 'owner',
+                'owner': 'owner'}
+        for k, v in data.items():
+            inst = MagicMock(spec=Instance)
+            inst.has_level.return_value = True
+            inst.ACL_LEVELS = Instance.ACL_LEVELS
+
+            self.assertTrue(AclUpdateView.has_next_level('dummy', inst, k))
+            inst.has_level.assert_called_with('dummy', v)
+
+    def test_set_level_mod_owner(self):
+        with patch('dashboard.views.messages') as msg:
+            request = FakeRequestFactory(POST={})
+
+            inst = MagicMock(spec=Instance)
+            inst.owner = request.user
+
+            v = AclUpdateView()
+            v.instance = inst
+            v.request = request
+            v.get_level = MagicMock(return_value='owner')
+            v.check_auth = MagicMock(side_effect=Exception(''))
+
+            v.set_level(request.user, 'user')
+            v.get_level.assert_called_with(request.user)
+            assert not v.check_auth.called
+            assert msg.warning.called
+
+    def test_set_level_permitted(self):
+        data = (('user', 'owner', ('user', 'operator', 'owner'), False),
+                (None, None, ('user', ), True),
+                ('user', None, ('user', ), True),
+                (None, 'user', ('user', ), True),
+                ('operator', 'owner', ('user', 'operator'), True),
+                (None, 'user', ('user', 'operator'), False))
+
+        for old_level, new_level, allowed_levels, fail in data:
+            with patch('dashboard.views.messages') as msg:
+                def has_level(user, level):
+                    return level in allowed_levels
+
+                request = FakeRequestFactory(POST={})
+
+                inst = MagicMock(spec=Instance)
+                inst.has_level.side_effect = has_level
+                inst.ACL_LEVELS = Instance.ACL_LEVELS
+
+                v = AclUpdateView()
+                v.instance = inst
+                v.request = request
+                v.is_owner = True
+                v.get_level = MagicMock(return_value=old_level)
+
+                v.set_level(request.user, new_level)
+
+                v.get_level.assert_called_with(request.user)
+                assert (new_level == old_level) ^ inst.has_level.called
+                assert fail ^ inst.set_level.called
+                assert fail ^ msg.success.called
+
+    def test_readd(self):
+        request = FakeRequestFactory(POST={'name': 'user0', 'level': 'user'})
+        with patch('dashboard.views.messages') as msg:
+            with patch.object(AclUpdateView, 'get_object') as go:
+                view = AclUpdateView.as_view()
+                inst = MagicMock(spec=Instance)
+                go.return_value = inst
+                view(request)
+                assert msg.warning.called
+
+
 def FakeRequestFactory(user=None, **kwargs):
     ''' FakeRequestFactory, FakeMessages and FakeRequestContext are good for
     mocking out django views; they are MUCH faster than the Django test client.

circle/dashboard/tests/test_views.py

@@ -27,7 +27,6 @@ from django.contrib.auth import authenticate
 from vm.models import Instance, InstanceTemplate, Lease, Node, Trait
 from vm.operations import WakeUpOperation
 from ..models import Profile
-from storage.models import Disk
 from firewall.models import Vlan, Host, VlanGroup
 from mock import Mock, patch
 from django_sshkey.models import UserKey
@@ -249,7 +248,6 @@ class VmDetailTest(LoginMixin, TestCase):
     def test_use_unpermitted_template(self):
         c = Client()
         self.login(c, 'user1')
-        Disk.objects.get(id=1).set_level(self.u1, 'user')
         Vlan.objects.get(id=1).set_level(self.u1, 'user')
         response = c.post('/dashboard/vm/create/',
                           {'template': 1,
@@ -261,7 +259,6 @@ class VmDetailTest(LoginMixin, TestCase):
     def test_use_permitted_template(self):
         c = Client()
         self.login(c, 'user1')
-        Disk.objects.get(id=1).set_level(self.u1, 'user')
         InstanceTemplate.objects.get(id=1).set_level(self.u1, 'user')
         Vlan.objects.get(id=1).set_level(self.u1, 'user')
         response = c.post('/dashboard/vm/create/',
@@ -293,7 +290,6 @@ class VmDetailTest(LoginMixin, TestCase):
         self.login(c, 'user1')
         tmpl = InstanceTemplate.objects.get(id=1)
         tmpl.set_level(self.u1, 'owner')
-        tmpl.disks.get().set_level(self.u1, 'owner')
         Vlan.objects.get(id=1).set_level(self.u1, 'user')
         kwargs = tmpl.__dict__.copy()
         kwargs.update(name='t1', lease=1, disks=1, raw_data='tst1')
@@ -591,7 +587,6 @@ class VmDetailTest(LoginMixin, TestCase):
             'template': 1,
             'cpu_priority': 1, 'cpu_count': 1, 'ram_size': 1,
             'network': [],
-            'disks': [Disk.objects.get(id=1).pk],
         })
 
         self.assertEqual(response.status_code, 302)
@@ -1248,7 +1243,7 @@ class GroupDetailTest(LoginMixin, TestCase):
         acl_users = len(gp.get_users_with_level())
         response = c.post('/dashboard/group/' +
                           str(self.g1.pk) + '/acl/',
-                          {'perm-new-name': 'user3', 'perm-new': 'owner'})
+                          {'name': 'user3', 'level': 'owner'})
         self.assertEqual(acl_users, len(gp.get_users_with_level()))
         self.assertEqual(response.status_code, 302)
 
@@ -1259,9 +1254,9 @@ class GroupDetailTest(LoginMixin, TestCase):
         acl_users = len(gp.get_users_with_level())
         response = c.post('/dashboard/group/' +
                           str(self.g1.pk) + '/acl/',
-                          {'perm-new-name': 'user3', 'perm-new': 'owner'})
+                          {'name': 'user3', 'level': 'owner'})
         self.assertEqual(acl_users, len(gp.get_users_with_level()))
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 302)
 
     def test_superuser_add_acluser_to_group(self):
         c = Client()
@@ -1270,7 +1265,7 @@ class GroupDetailTest(LoginMixin, TestCase):
         acl_users = len(gp.get_users_with_level())
         response = c.post('/dashboard/group/' +
                           str(self.g1.pk) + '/acl/',
-                          {'perm-new-name': 'user3', 'perm-new': 'owner'})
+                          {'name': 'user3', 'level': 'owner'})
         self.assertEqual(acl_users + 1, len(gp.get_users_with_level()))
         self.assertEqual(response.status_code, 302)
 
@@ -1281,7 +1276,7 @@ class GroupDetailTest(LoginMixin, TestCase):
         acl_users = len(gp.get_users_with_level())
         response = c.post('/dashboard/group/' +
                           str(self.g1.pk) + '/acl/',
-                          {'perm-new-name': 'user3', 'perm-new': 'owner'})
+                          {'name': 'user3', 'level': 'owner'})
         self.assertEqual(acl_users + 1, len(gp.get_users_with_level()))
         self.assertEqual(response.status_code, 302)
 
@@ -1291,7 +1286,7 @@ class GroupDetailTest(LoginMixin, TestCase):
         acl_groups = len(gp.get_groups_with_level())
         response = c.post('/dashboard/group/' +
                           str(self.g1.pk) + '/acl/',
-                          {'perm-new-name': 'group2', 'perm-new': 'owner'})
+                          {'name': 'group2', 'level': 'owner'})
         self.assertEqual(acl_groups, len(gp.get_groups_with_level()))
         self.assertEqual(response.status_code, 302)
 
@@ -1302,9 +1297,9 @@ class GroupDetailTest(LoginMixin, TestCase):
         acl_groups = len(gp.get_groups_with_level())
         response = c.post('/dashboard/group/' +
                           str(self.g1.pk) + '/acl/',
-                          {'perm-new-name': 'group2', 'perm-new': 'owner'})
+                          {'name': 'group2', 'level': 'owner'})
         self.assertEqual(acl_groups, len(gp.get_groups_with_level()))
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 302)
 
     def test_superuser_add_aclgroup_to_group(self):
         c = Client()
@@ -1313,7 +1308,7 @@ class GroupDetailTest(LoginMixin, TestCase):
         acl_groups = len(gp.get_groups_with_level())
         response = c.post('/dashboard/group/' +
                           str(self.g1.pk) + '/acl/',
-                          {'perm-new-name': 'group2', 'perm-new': 'owner'})
+                          {'name': 'group2', 'level': 'owner'})
         self.assertEqual(acl_groups + 1, len(gp.get_groups_with_level()))
         self.assertEqual(response.status_code, 302)
 
@@ -1324,7 +1319,7 @@ class GroupDetailTest(LoginMixin, TestCase):
         acl_groups = len(gp.get_groups_with_level())
         response = c.post('/dashboard/group/' +
                           str(self.g1.pk) + '/acl/',
-                          {'perm-new-name': 'group2', 'perm-new': 'owner'})
+                          {'name': 'group2', 'level': 'owner'})
         self.assertEqual(acl_groups + 1, len(gp.get_groups_with_level()))
         self.assertEqual(response.status_code, 302)
 
@@ -1366,84 +1361,6 @@ class GroupDetailTest(LoginMixin, TestCase):
         self.assertEqual(user_in_group - 1, self.g1.user_set.count())
         self.assertEqual(response.status_code, 302)
 
-    def test_anon_remove_acluser_from_group(self):
-        c = Client()
-        gp = self.g1.profile
-        acl_users = len(gp.get_users_with_level())
-        response = c.post('/dashboard/group/' + str(self.g1.pk) +
-                          '/remove/acl/user/' + str(self.u4.pk) + '/')
-        self.assertEqual(acl_users, len(gp.get_users_with_level()))
-        self.assertEqual(response.status_code, 302)
-
-    def test_unpermitted_remove_acluser_from_group(self):
-        c = Client()
-        self.login(c, 'user3')
-        gp = self.g1.profile
-        acl_users = len(gp.get_users_with_level())
-        response = c.post('/dashboard/group/' + str(self.g1.pk) +
-                          '/remove/acl/user/' + str(self.u4.pk) + '/')
-        self.assertEqual(acl_users, len(gp.get_users_with_level()))
-        self.assertEqual(response.status_code, 403)
-
-    def test_superuser_remove_acluser_from_group(self):
-        c = Client()
-        gp = self.g1.profile
-        self.login(c, 'superuser')
-        acl_users = len(gp.get_users_with_level())
-        response = c.post('/dashboard/group/' + str(self.g1.pk) +
-                          '/remove/acl/user/' + str(self.u4.pk) + '/')
-        self.assertEqual(acl_users - 1, len(gp.get_users_with_level()))
-        self.assertEqual(response.status_code, 302)
-
-    def test_permitted_remove_acluser_from_group(self):
-        c = Client()
-        gp = self.g1.profile
-        self.login(c, 'user0')
-        acl_users = len(gp.get_users_with_level())
-        response = c.post('/dashboard/group/' + str(self.g1.pk) +
-                          '/remove/acl/user/' + str(self.u4.pk) + '/')
-        self.assertEqual(acl_users - 1, len(gp.get_users_with_level()))
-        self.assertEqual(response.status_code, 302)
-
-    def test_anon_remove_aclgroup_from_group(self):
-        c = Client()
-        gp = self.g1.profile
-        acl_groups = len(gp.get_groups_with_level())
-        response = c.post('/dashboard/group/' + str(self.g1.pk) +
-                          '/remove/acl/group/' + str(self.g3.pk) + '/')
-        self.assertEqual(acl_groups, len(gp.get_groups_with_level()))
-        self.assertEqual(response.status_code, 302)
-
-    def test_unpermitted_remove_aclgroup_from_group(self):
-        c = Client()
-        self.login(c, 'user3')
-        gp = self.g1.profile
-        acl_groups = len(gp.get_groups_with_level())
-        response = c.post('/dashboard/group/' + str(self.g1.pk) +
-                          '/remove/acl/group/' + str(self.g3.pk) + '/')
-        self.assertEqual(acl_groups, len(gp.get_groups_with_level()))
-        self.assertEqual(response.status_code, 403)
-
-    def test_superuser_remove_aclgroup_from_group(self):
-        c = Client()
-        gp = self.g1.profile
-        acl_groups = len(gp.get_groups_with_level())
-        self.login(c, 'superuser')
-        response = c.post('/dashboard/group/' + str(self.g1.pk) +
-                          '/remove/acl/group/' + str(self.g3.pk) + '/')
-        self.assertEqual(acl_groups - 1, len(gp.get_groups_with_level()))
-        self.assertEqual(response.status_code, 302)
-
-    def test_permitted_remove_aclgroup_from_group(self):
-        c = Client()
-        gp = self.g1.profile
-        acl_groups = len(gp.get_groups_with_level())
-        self.login(c, 'user0')
-        response = c.post('/dashboard/group/' + str(self.g1.pk) +
-                          '/remove/acl/group/' + str(self.g3.pk) + '/')
-        self.assertEqual(acl_groups - 1, len(gp.get_groups_with_level()))
-        self.assertEqual(response.status_code, 302)
-
     def test_unpermitted_user_add_wo_group_perm(self):
         user_count = self.g1.user_set.count()
         c = Client()
@@ -1772,8 +1689,8 @@ class AclViewTest(LoginMixin, TestCase):
 
         resp = c.post("/dashboard/vm/1/acl/", {
             'remove-u-%d' % self.u1.pk: "",
-            'perm-new-name': "",
-            'perm-new': "",
+            'name': "",
+            'level': "",
         })
         self.assertFalse((self.u1, "user") in inst.get_users_with_level())
         self.assertEqual(resp.status_code, 302)
@@ -1786,11 +1703,11 @@ class AclViewTest(LoginMixin, TestCase):
 
         resp = c.post("/dashboard/vm/1/acl/", {
             'remove-u-%d' % self.u1.pk: "",
-            'perm-new-name': "",
-            'perm-new': "",
+            'name': "",
+            'level': "",
         })
         self.assertTrue((self.u1, "user") in inst.get_users_with_level())
-        self.assertEqual(resp.status_code, 403)
+        self.assertEqual(resp.status_code, 302)
 
     def test_instance_original_owner_access_revoke(self):
         c = Client()
@@ -1800,8 +1717,8 @@ class AclViewTest(LoginMixin, TestCase):
         inst.set_level(self.ut, "owner")
         resp = c.post("/dashboard/vm/1/acl/", {
             'remove-u-%d' % self.ut.pk: "",
-            'perm-new-name': "",
-            'perm-new': "",
+            'name': "",
+            'level': "",
         })
         self.assertEqual(self.ut, Instance.objects.get(id=1).owner)
         self.assertTrue((self.ut, "owner") in inst.get_users_with_level())
@@ -1816,8 +1733,8 @@ class AclViewTest(LoginMixin, TestCase):
 
         resp = c.post("/dashboard/template/1/acl/", {
             'remove-u-%d' % self.u1.pk: "",
-            'perm-new-name': "",
-            'perm-new': "",
+            'name': "",
+            'level': "",
         })
         self.assertFalse((self.u1, "user") in tmpl.get_users_with_level())
         self.assertEqual(resp.status_code, 302)
@@ -1830,11 +1747,11 @@ class AclViewTest(LoginMixin, TestCase):
 
         resp = c.post("/dashboard/template/1/acl/", {
             'remove-u-%d' % self.u1.pk: "",
-            'perm-new-name': "",
-            'perm-new': "",
+            'name': "",
+            'level': "",
         })
         self.assertTrue((self.u1, "user") in tmpl.get_users_with_level())
-        self.assertEqual(resp.status_code, 403)
+        self.assertEqual(resp.status_code, 302)
 
     def test_template_original_owner_access_revoke(self):
         c = Client()
@@ -1844,8 +1761,8 @@ class AclViewTest(LoginMixin, TestCase):
         tmpl.set_level(self.ut, "owner")
         resp = c.post("/dashboard/template/1/acl/", {
             'remove-u-%d' % self.ut.pk: "",
-            'perm-new-name': "",
-            'perm-new': "",
+            'name': "",
+            'level': "",
         })
         self.assertEqual(self.ut, InstanceTemplate.objects.get(id=1).owner)
         self.assertTrue((self.ut, "owner") in tmpl.get_users_with_level())

circle/dashboard/urls.py

@@ -18,6 +18,7 @@
 from __future__ import absolute_import
 from django.conf.urls import patterns, url, include
 
+import autocomplete_light
 from vm.models import Instance
 from .views import (
     AclUpdateView, FavouriteView, GroupAclUpdateView, GroupDelete,
@@ -30,7 +31,7 @@ from .views import (
     TransferOwnershipView, vm_activity, VmCreate, VmDelete, VmDetailView,
     VmDetailVncTokenView, VmGraphView, VmList, VmMassDelete,
     DiskRemoveView, get_disk_download_status, InterfaceDeleteView,
-    GroupRemoveAclUserView, GroupRemoveAclGroupView, GroupRemoveUserView,
+    GroupRemoveUserView,
     GroupRemoveFutureUserView,
     GroupCreate, GroupProfileUpdate,
     TemplateChoose,
@@ -43,7 +44,10 @@ from .views import (
     LeaseAclUpdateView,
 )
 
+autocomplete_light.autodiscover()
+
 urlpatterns = patterns(
+
     '',
     url(r'^$', IndexView.as_view(), name="dashboard.index"),
     url(r'^lease/(?P<pk>\d+)/$', LeaseDetail.as_view(),
@@ -151,12 +155,6 @@ urlpatterns = patterns(
         name="dashboard.views.profile"),
     url(r'^profile/(?P<username>[^/]+)/use_gravatar/$', toggle_use_gravatar),
 
-    url(r'^group/(?P<group_pk>\d+)/remove/acl/user/(?P<member_pk>\d+)/$',
-        GroupRemoveAclUserView.as_view(),
-        name="dashboard.views.remove-acluser"),
-    url(r'^group/(?P<group_pk>\d+)/remove/acl/group/(?P<member_pk>\d+)/$',
-        GroupRemoveAclGroupView.as_view(),
-        name="dashboard.views.remove-aclgroup"),
     url(r'^group/(?P<group_pk>\d+)/remove/user/(?P<member_pk>\d+)/$',
         GroupRemoveUserView.as_view(),
         name="dashboard.views.remove-user"),
@@ -181,4 +179,5 @@ urlpatterns = patterns(
     url(r'^sshkey/create/$',
         UserKeyCreate.as_view(),
         name="dashboard.views.userkey-create"),
+    url(r'^autocomplete/', include('autocomplete_light.urls')),
 )

circle/dashboard/views.py

@@ -28,14 +28,13 @@ import requests
 from django.conf import settings
 from django.contrib.auth.models import User, Group
 from django.contrib.auth.views import login, redirect_to_login
-from django.contrib.messages import warning
 from django.contrib.messages.views import SuccessMessageMixin
 from django.core.exceptions import (
     PermissionDenied, SuspiciousOperation,
 )
 from django.core import signing
 from django.core.urlresolvers import reverse, reverse_lazy
-from django.db.models import Count
+from django.db.models import Count, Q
 from django.http import HttpResponse, HttpResponseRedirect, Http404
 from django.shortcuts import redirect, render, get_object_or_404
 from django.views.decorators.http import require_GET, require_POST
@@ -62,7 +61,7 @@ from .forms import (
     UserCreationForm, GroupProfileUpdateForm, UnsubscribeForm,
     VmSaveForm, UserKeyForm, VmRenewForm,
     CirclePasswordChangeForm, VmCreateDiskForm, VmDownloadDiskForm,
-    TraitsForm, RawDataForm, GroupPermissionForm
+    TraitsForm, RawDataForm, GroupPermissionForm, AclUserAddForm
 )
 
 from .tables import (
@@ -219,27 +218,6 @@ class IndexView(LoginRequiredMixin, TemplateView):
         return context
 
 
-def get_vm_acl_data(obj):
-    levels = obj.ACL_LEVELS
-    users = obj.get_users_with_level()
-    users = [{'user': u, 'level': l} for u, l in users]
-    groups = obj.get_groups_with_level()
-    groups = [{'group': g, 'level': l} for g, l in groups]
-    return {'users': users, 'groups': groups, 'levels': levels,
-            'url': reverse('dashboard.views.vm-acl', args=[obj.pk])}
-
-
-def get_group_acl_data(obj):
-    aclobj = obj.profile
-    levels = aclobj.ACL_LEVELS
-    users = aclobj.get_users_with_level()
-    users = [{'user': u, 'level': l} for u, l in users]
-    groups = aclobj.get_groups_with_level()
-    groups = [{'group': g, 'level': l} for g, l in groups]
-    return {'users': users, 'groups': groups, 'levels': levels,
-            'url': reverse('dashboard.views.group-acl', args=[obj.pk])}
-
-
 class CheckedDetailView(LoginRequiredMixin, DetailView):
     read_level = 'user'
 
@@ -303,7 +281,9 @@ class VmDetailView(CheckedDetailView):
             pk__in=Interface.objects.filter(
                 instance=self.get_object()).values_list("vlan", flat=True)
         ).all()
-        context['acl'] = get_vm_acl_data(instance)
+        context['acl'] = AclUpdateView.get_acl_data(
+            instance, self.request.user, 'dashboard.views.vm-acl')
+        context['aclform'] = AclUserAddForm()
         context['os_type_icon'] = instance.os_type.replace("unknown",
                                                            "question")
         # ipv6 infos
@@ -979,7 +959,10 @@ class GroupDetailView(CheckedDetailView):
         context['users'] = self.object.user_set.all()
         context['future_users'] = FutureMember.objects.filter(
             group=self.object)
-        context['acl'] = get_group_acl_data(self.object)
+        context['acl'] = AclUpdateView.get_acl_data(
+            self.object.profile, self.request.user,
+            'dashboard.views.group-acl')
+        context['aclform'] = AclUserAddForm()
         context['group_profile_form'] = GroupProfileUpdate.get_form_object(
             self.request, self.object.profile)
 
@@ -1022,7 +1005,7 @@ class GroupDetailView(CheckedDetailView):
                 FutureMember.objects.get_or_create(org_id=name,
                                                    group=self.object)
             else:
-                warning(request, _('User "%s" not found.') % name)
+                messages.warning(request, _('User "%s" not found.') % name)
 
     def __add_list(self, request):
         if not self.get_has_level()(request.user, 'operator'):
@@ -1067,120 +1050,169 @@ class GroupPermissionsView(SuperuserRequiredMixin, UpdateView):
 
 
 class AclUpdateView(LoginRequiredMixin, View, SingleObjectMixin):
+    def send_success_message(self, whom, old_level, new_level):
+        if old_level and new_level:
+            msg = _("Acl user/group %(w)s successfully modified.")
+        elif not old_level and new_level:
+            msg = _("Acl user/group %(w)s successfully added.")
+        elif old_level and not new_level:
+            msg = _("Acl user/group %(w)s successfully removed.")
+        if msg:
+            messages.success(self.request, msg % {'w': whom})
+
+    def get_level(self, whom):
+        for u, level in self.acl_data:
+            if u == whom:
+                return level
+        return None
 
-    def post(self, request, *args, **kwargs):
-        instance = self.get_object()
-        if not (instance.has_level(request.user, "owner") or
-                getattr(instance, 'owner', None) == request.user):
-            logger.warning('Tried to set permissions of %s by non-owner %s.',
-                           unicode(instance), unicode(request.user))
-            raise PermissionDenied()
-        self.set_levels(request, instance)
-        self.remove_levels(request, instance)
-        self.add_levels(request, instance)
-        return redirect("%s#access" % instance.get_absolute_url())
-
-    def set_levels(self, request, instance):
-        for key, value in request.POST.items():
-            m = re.match('perm-([ug])-(\d+)', key)
+    @classmethod
+    def get_acl_data(cls, obj, user, url):
+        levels = obj.ACL_LEVELS
+        allowed_levels = list(l for l in OrderedDict(levels)
+                              if cls.has_next_level(user, obj, l))
+        is_owner = 'owner' in allowed_levels
+
+        allowed_users = cls.get_allowed_users(user)
+        allowed_groups = cls.get_allowed_groups(user)
+
+        user_levels = list(
+            {'user': u, 'level': l} for u, l in obj.get_users_with_level()
+            if is_owner or u == user or u in allowed_users)
+
+        group_levels = list(
+            {'group': g, 'level': l} for g, l in obj.get_groups_with_level()
+            if is_owner or g in allowed_groups)
+
+        return {'users': user_levels,
+                'groups': group_levels,
+                'levels': levels,
+                'allowed_levels': allowed_levels,
+                'url': reverse(url, args=[obj.pk])}
+
+    @classmethod
+    def has_next_level(self, user, instance, level):
+        levels = OrderedDict(instance.ACL_LEVELS).keys()
+        next_levels = dict(zip([None] + levels, levels + levels[-1:]))
+        # {None: 'user', 'user': 'operator', 'operator: 'owner',
+        #  'owner: 'owner'}
+        next_level = next_levels[level]
+        return instance.has_level(user, next_level)
+
+    @classmethod
+    def get_allowed_groups(cls, user):
+        if user.has_perm('dashboard.use_autocomplete'):
+            return Group.objects.all()
+        else:
+            profiles = GroupProfile.get_objects_with_level('owner', user)
+            return Group.objects.filter(groupprofile__in=profiles).distinct()
+
+    @classmethod
+    def get_allowed_users(cls, user):
+        if user.has_perm('dashboard.use_autocomplete'):
+            return User.objects.all()
+        else:
+            groups = cls.get_allowed_groups(user)
+            return User.objects.filter(
+                Q(groups__in=groups) | Q(pk=user.pk)).distinct()
+
+    def check_auth(self, whom, old_level, new_level):
+        if isinstance(whom, Group):
+            if (not self.is_owner and whom not in
+                    AclUpdateView.get_allowed_groups(self.request.user)):
+                return False
+        elif isinstance(whom, User):
+            if (not self.is_owner and whom not in
+                    AclUpdateView.get_allowed_users(self.request.user)):
+                return False
+        return (
+            AclUpdateView.has_next_level(self.request.user,
+                                         self.instance, new_level) and
+            AclUpdateView.has_next_level(self.request.user,
+                                         self.instance, old_level))
+
+    def set_level(self, whom, new_level):
+        user = self.request.user
+        old_level = self.get_level(whom)
+        if old_level == new_level:
+            return
+
+        if getattr(self.instance, "owner", None) == whom:
+            logger.info("Tried to set owner's acl level for %s by %s.",
+                        unicode(self.instance), unicode(user))
+            msg = _("The original owner cannot be removed, however "
+                    "you can transfer ownership.")
+            if not getattr(self, 'hide_messages', False):
+                messages.warning(self.request, msg)
+        elif self.check_auth(whom, old_level, new_level):
+            logger.info(
+                u"Set %s's acl level for %s to %s by %s.", unicode(whom),
+                unicode(self.instance), new_level, unicode(user))
+            if not getattr(self, 'hide_messages', False):
+                self.send_success_message(whom, old_level, new_level)
+            self.instance.set_level(whom, new_level)
+        else:
+            logger.warning(
+                u"Tried to set %s's acl_level for %s (%s->%s) by %s.",
+                unicode(whom), unicode(self.instance), old_level, new_level,
+                unicode(user))
+
+    def set_or_remove_levels(self):
+        for key, value in self.request.POST.items():
+            m = re.match('(perm|remove)-([ug])-(\d+)', key)
             if m:
-                typ, id = m.groups()
-                entity = {'u': User, 'g': Group}[typ].objects.get(id=id)
-                if getattr(instance, "owner", None) == entity:
-                    logger.info("Tried to set owner's acl level for %s by %s.",
-                                unicode(instance), unicode(request.user))
-                    continue
-                instance.set_level(entity, value)
-                logger.info("Set %s's acl level for %s to %s by %s.",
-                            unicode(entity), unicode(instance),
-                            value, unicode(request.user))
-
-    def remove_levels(self, request, instance):
-        for key, value in request.POST.items():
-            if key.startswith("remove"):
-                typ = key[7:8]  # len("remove-")
-                id = key[9:]  # len("remove-x-")
+                cmd, typ, id = m.groups()
+                if cmd == 'remove':
+                    value = None
                 entity = {'u': User, 'g': Group}[typ].objects.get(id=id)
-                if getattr(instance, "owner", None) == entity:
-                    logger.info("Tried to remove owner from %s by %s.",
-                                unicode(instance), unicode(request.user))
-                    msg = _("The original owner cannot be removed, however "
-                            "you can transfer ownership.")
-                    messages.warning(request, msg)
-                    continue
-                instance.set_level(entity, None)
-                logger.info("Revoked %s's access to %s by %s.",
-                            unicode(entity), unicode(instance),
-                            unicode(request.user))
-
-    def add_levels(self, request, instance):
-        name = request.POST['perm-new-name']
-        value = request.POST['perm-new']
-        if not name:
+                self.set_level(entity, value)
+
+    def add_levels(self):
+        name = self.request.POST.get('name', None)
+        level = self.request.POST.get('level', None)
+        if not name or not level:
             return
         try:
             entity = search_user(name)
+            if self.instance.object_level_set.filter(users__in=[entity]):
+                messages.warning(
+                    self.request, _('User "%s" has already '
+                                    'access to this object.') % name)
+                return
         except User.DoesNotExist:
             entity = None
             try:
                 entity = Group.objects.get(name=name)
+                if self.instance.object_level_set.filter(groups__in=[entity]):
+                    messages.warning(
+                        self.request, _('Group "%s" has already '
+                                        'access to this object.') % name)
+                    return
             except Group.DoesNotExist:
-                warning(request, _('User or group "%s" not found.') % name)
+                messages.warning(
+                    self.request, _('User or group "%s" not found.') % name)
                 return
+        self.set_level(entity, level)
 
-        instance.set_level(entity, value)
-        logger.info("Set %s's new acl level for %s to %s by %s.",
-                    unicode(entity), unicode(instance),
-                    value, unicode(request.user))
+    def post(self, request, *args, **kwargs):
+        self.instance = self.get_object()
+        self.is_owner = self.instance.has_level(request.user, 'owner')
+        self.acl_data = (self.instance.get_users_with_level() +
+                         self.instance.get_groups_with_level())
+        self.set_or_remove_levels()
+        self.add_levels()
+        return redirect("%s#access" % self.instance.get_absolute_url())
 
 
 class TemplateAclUpdateView(AclUpdateView):
     model = InstanceTemplate
 
-    def post(self, request, *args, **kwargs):
-        template = self.get_object()
-        if not (template.has_level(request.user, "owner") or
-                getattr(template, 'owner', None) == request.user):
-            logger.warning('Tried to set permissions of %s by non-owner %s.',
-                           unicode(template), unicode(request.user))
-            raise PermissionDenied()
-
-        name = request.POST['perm-new-name']
-        if (User.objects.filter(username=name).count() +
-                Group.objects.filter(name=name).count() < 1
-                and len(name) > 0):
-            warning(request, _('User or group "%s" not found.') % name)
-        else:
-            self.set_levels(request, template)
-            self.add_levels(request, template)
-            self.remove_levels(request, template)
-
-            post_for_disk = request.POST.copy()
-            post_for_disk['perm-new'] = 'user'
-            request.POST = post_for_disk
-            for d in template.disks.all():
-                self.set_levels(request, d)
-                self.add_levels(request, d)
-                self.remove_levels(request, d)
-
-        return redirect(template)
-
 
 class GroupAclUpdateView(AclUpdateView):
     model = Group
 
-    def post(self, request, *args, **kwargs):
-        instance = self.get_object().profile
-        if not (instance.has_level(request.user, "owner") or
-                getattr(instance, 'owner', None) == request.user):
-            logger.warning('Tried to set permissions of %s by non-owner %s.',
-                           unicode(instance), unicode(request.user))
-            raise PermissionDenied()
-
-        self.set_levels(request, instance)
-        self.add_levels(request, instance)
-        return redirect(reverse("dashboard.views.group-detail",
-                                kwargs=self.kwargs))
+    def get_object(self):
+        return super(GroupAclUpdateView, self).get_object().profile
 
 
 class TemplateChoose(LoginRequiredMixin, TemplateView):
@@ -1327,8 +1359,11 @@ class TemplateDetail(LoginRequiredMixin, SuccessMessageMixin, UpdateView):
     def get_context_data(self, **kwargs):
         obj = self.get_object()
         context = super(TemplateDetail, self).get_context_data(**kwargs)
-        context['acl'] = get_vm_acl_data(obj)
+        context['acl'] = AclUpdateView.get_acl_data(
+            obj, self.request.user, 'dashboard.views.template-acl')
         context['disks'] = obj.disks.all()
+        context['is_owner'] = obj.has_level(self.request.user, 'owner')
+        context['aclform'] = AclUserAddForm()
         return context
 
     def get_success_url(self):
@@ -1627,34 +1662,6 @@ class GroupRemoveFutureUserView(GroupRemoveUserView):
         return _("Future user successfully removed from group.")
 
 
-class GroupRemoveAclUserView(GroupRemoveUserView):
-
-    def remove_member(self, pk):
-        container = self.get_object().profile
-        container.set_level(User.objects.get(pk=pk), None)
-
-    def get_success_message(self):
-        return _("Acl user successfully removed from group.")
-
-
-class GroupRemoveAclGroupView(GroupRemoveUserView):
-
-    def get_context_data(self, **kwargs):
-        context = super(GroupRemoveUserView, self).get_context_data(**kwargs)
-        try:
-            context['member'] = Group.objects.get(pk=self.member_pk)
-        except User.DoesNotExist:
-            raise Http404()
-        return context
-
-    def remove_member(self, pk):
-        container = self.get_object().profile
-        container.set_level(Group.objects.get(pk=pk), None)
-
-    def get_success_message(self):
-        return _("Acl group successfully removed from group.")
-
-
 class GroupDelete(CheckedDetailView, DeleteView):
 
     """This stuff deletes the group.
@@ -1781,13 +1788,12 @@ class VmCreate(LoginRequiredMixin, TemplateView):
             }
             networks = [InterfaceTemplate(vlan=l, managed=l.managed)
                         for l in post['networks']]
-            disks = post['disks']
 
             ikwargs.update({
                 'template': template,
                 'owner': user,
                 'networks': networks,
-                'disks': disks,
+                'disks': list(template.disks.all()),
             })
 
             amount = post['amount']
@@ -2319,7 +2325,8 @@ class LeaseDetail(LoginRequiredMixin, SuperuserRequiredMixin,
     def get_context_data(self, *args, **kwargs):
         obj = self.get_object()
         context = super(LeaseDetail, self).get_context_data(*args, **kwargs)
-        context['acl'] = get_vm_acl_data(obj)
+        context['acl'] = AclUpdateView.get_acl_data(
+            obj, self.request.user, 'dashboard.views.lease-acl')
         return context
 
     def get_success_url(self):
@@ -2792,12 +2799,11 @@ class DiskRemoveView(DeleteView):
 
     def delete(self, request, *args, **kwargs):
         disk = self.get_object()
-        if not disk.has_level(request.user, 'owner'):
-            raise PermissionDenied()
-
-        disk = self.get_object()
         app = disk.get_appliance()
 
+        if not app.has_level(request.user, 'owner'):
+            raise PermissionDenied()
+
         app.remove_disk(disk=disk, user=request.user)
         disk.destroy()
 
@@ -2818,7 +2824,7 @@ class DiskRemoveView(DeleteView):
 @require_GET
 def get_disk_download_status(request, pk):
     disk = Disk.objects.get(pk=pk)
-    if not disk.has_level(request.user, 'owner'):
+    if not disk.get_appliance().has_level(request.user, 'owner'):
         raise PermissionDenied()
 
     return HttpResponse(

circle/network/templates/network/vlan-edit.html

@@ -23,61 +23,7 @@
     <div class="page-header">
       <h3>{% trans "Manage access" %}</h3>
     </div>
-    <form action="{% url "network.vlan-acl" vid=vlan_vid %}" method="post">{% csrf_token %}
-      <table class="table table-striped table-with-form-fields" id="vlan-access-table">
-        <thead>
-          <tr>
-            <th></th>
-            <th>{% trans "Who" %}</th>
-            <th>{% trans "What" %}</th>
-            <th><i class="fa fa-times"></i></th>
-        </tr></thead>
-        <tbody>
-            {% for i in acl.users %}
-            <tr>
-              <td><i class="fa fa-user"></i></td><td>{{i.user}}</td>
-              <td>
-                <select class="form-control" name="perm-u-{{i.user.id}}">
-                  {% for id, name in acl.levels %}
-                  <option{%if id = i.level%} selected="selected"{%endif%} value="{{id}}">{{name}}</option>
-                  {% endfor %}
-                </select>
-              </td>
-              <td>
-                <input type="checkbox" name="remove-u-{{i.user.id}}" title="{% trans "Remove" %}"/>
-              </td>
-            </tr>
-            {% endfor %}
-            {% for i in acl.groups %}
-            <tr>
-              <td><i class="fa fa-group"></i></td><td>{{i.group}}</td>
-              <td>
-                <select class="form-control" name="perm-g-{{i.group.id}}">
-                  {% for id, name in acl.levels %}
-                  <option{%if id = i.level%} selected="selected"{%endif%} value="{{id}}">{{name}}</option>
-                  {% endfor %}
-                </select>
-              </td>
-              <td>
-                <input type="checkbox" name="remove-g-{{i.group.id}}" title="{% trans "Remove" %}"/>
-              </td>
-            </tr>
-            {% endfor %}
-            <tr><td><i class="fa fa-plus"></i></td>
-                <td><input type="text" class="form-control" name="perm-new-name"
-                    placeholder="{% trans "Name of group or user" %}"></td>
-                <td><select class="form-control" name="perm-new">
-                        {% for id, name in acl.levels %}
-                        <option value="{{id}}">{{name}}</option>
-                        {% endfor %}
-                </select></td><td></td>
-            </tr>
-      </tbody>
-      </table>
-      <div class="form-actions">
-        <button type="submit" class="btn btn-success">{% trans "Save" %}</button>
-      </div>
-    </form>
+    {% include "dashboard/_manage_access.html" with table_id="vlan-access-table" %}
   </div>
 </div>
 {% endblock %}

circle/network/tests/test_views.py

@@ -63,9 +63,9 @@ class VlanAclTest(LoginMixin, TestCase):
         vlan = Vlan.objects.get(vid=1)
         self.assertEqual([], vlan.get_users_with_level())
 
-        resp = c.post("/network/vlans/1/acl/", {
-            'perm-new-name': "user1",
-            'perm-new': "user",
+        resp = c.post("/network/vlans/2/acl/", {
+            'name': "user1",
+            'level': "user",
         })
 
         vlan = Vlan.objects.get(vid=1)
@@ -80,10 +80,10 @@ class VlanAclTest(LoginMixin, TestCase):
         vlan.set_level(self.u1, "user")
         self.assertTrue((self.u1, "user") in vlan.get_users_with_level())
 
-        resp = c.post("/network/vlans/1/acl/", {
+        resp = c.post("/network/vlans/2/acl/", {
             'perm-u-%d' % self.u1.pk: "operator",
-            'perm-new': "",
-            'perm-new-name': "",
+            'level': "",
+            'name': "",
         })
 
         self.assertTrue((self.u1, "operator") in vlan.get_users_with_level())
@@ -97,10 +97,10 @@ class VlanAclTest(LoginMixin, TestCase):
         vlan.set_level(self.u1, "user")
         self.assertTrue((self.u1, "user") in vlan.get_users_with_level())
 
-        resp = c.post("/network/vlans/1/acl/", {
+        resp = c.post("/network/vlans/2/acl/", {
             'remove-u-%d' % self.u1.pk: "",
-            'perm-new': "",
-            'perm-new-name': "",
+            'level': "",
+            'name': "",
         })
 
         self.assertTrue((self.u1, "user") not in vlan.get_users_with_level())

circle/network/urls.py

@@ -84,7 +84,7 @@ urlpatterns = patterns(
     url('^vlans/$', VlanList.as_view(), name='network.vlan_list'),
     url('^vlans/create$', VlanCreate.as_view(), name='network.vlan_create'),
     url('^vlans/(?P<vid>\d+)/$', VlanDetail.as_view(), name='network.vlan'),
-    url('^vlans/(?P<vid>\d+)/acl/$', VlanAclUpdateView.as_view(),
+    url('^vlans/(?P<pk>\d+)/acl/$', VlanAclUpdateView.as_view(),
         name='network.vlan-acl'),
     url('^vlans/delete/(?P<vid>\d+)/$', VlanDelete.as_view(),
         name="network.vlan_delete"),

circle/network/views.py

@@ -42,6 +42,7 @@ from operator import itemgetter
 from itertools import chain
 import json
 from dashboard.views import AclUpdateView
+from dashboard.forms import AclUserAddForm
 
 
 class SuccessMessageMixin(FormMixin):
@@ -629,19 +630,8 @@ class VlanList(LoginRequiredMixin, SuperuserRequiredMixin, SingleTableView):
     table_pagination = False
 
 
-def get_vlan_acl_data(obj):
-    levels = obj.ACL_LEVELS
-    users = obj.get_users_with_level()
-    users = [{'user': u, 'level': l} for u, l in users]
-    groups = obj.get_groups_with_level()
-    groups = [{'group': g, 'level': l} for g, l in groups]
-    return {'users': users, 'groups': groups, 'levels': levels}
-
-
 class VlanAclUpdateView(AclUpdateView):
     model = Vlan
-    slug_field = "vid"
-    slug_url_kwarg = "vid"
 
 
 class VlanDetail(LoginRequiredMixin, SuperuserRequiredMixin,
@@ -662,7 +652,9 @@ class VlanDetail(LoginRequiredMixin, SuperuserRequiredMixin,
 
         context['host_list'] = SmallHostTable(q)
         context['vlan_vid'] = self.kwargs.get('vid')
-        context['acl'] = get_vlan_acl_data(self.get_object())
+        context['acl'] = AclUpdateView.get_acl_data(
+            self.object, self.request.user, 'network.vlan-acl')
+        context['aclform'] = AclUserAddForm()
         return context
 
     success_url = reverse_lazy('network.vlan_list')

circle/storage/models.py

@@ -31,7 +31,6 @@ from django.utils.translation import ugettext_lazy as _
 from model_utils.models import TimeStampedModel
 from sizefield.models import FileSizeField
 
-from acl.models import AclBase
 from .tasks import local_tasks, storage_tasks
 from celery.exceptions import TimeoutError
 from common.models import WorkerNotFound
@@ -76,15 +75,10 @@ class DataStore(Model):
                     destroyed__isnull=False) if disk.is_deletable]
 
 
-class Disk(AclBase, TimeStampedModel):
+class Disk(TimeStampedModel):
 
     """A virtual disk.
     """
-    ACL_LEVELS = (
-        ('user', _('user')),          # see all details
-        ('operator', _('operator')),
-        ('owner', _('owner')),        # superuser, can delete, delegate perms
-    )
     TYPES = [('qcow2-norm', 'qcow2 normal'), ('qcow2-snap', 'qcow2 snapshot'),
              ('iso', 'iso'), ('raw-ro', 'raw read-only'), ('raw-rw', 'raw')]
     name = CharField(blank=True, max_length=100, verbose_name=_("name"))
@@ -225,15 +219,14 @@ class Disk(AclBase, TimeStampedModel):
         return any(i.state != 'STOPPED' for i in self.instance_set.all())
 
     def get_appliance(self):
-        """Return an Instance or InstanceTemplate object where the disk is used
+        """Return the Instance or InstanceTemplate object where the disk
+        is used
         """
-        instance = self.instance_set.all()
-        template = self.template_set.all()
-        app = list(instance) + list(template)
-        if len(app) > 0:
-            return app[0]
-        else:
-            return None
+        from vm.models import Instance
+        try:
+            return self.instance_set.get()
+        except Instance.DoesNotExist:
+            return self.template_set.get()
 
     def get_exclusive(self):
         """Get an instance of the disk for exclusive usage.

circle/vm/models/instance.py

@@ -405,13 +405,6 @@ class Instance(AclBase, VirtualMachineDescModel, StatusModel, OperatedMixin,
         """
         disks = template.disks.all() if disks is None else disks
 
-        for disk in disks:
-            if not disk.has_level(owner, 'user'):
-                raise PermissionDenied()
-            elif (disk.type == 'qcow2-snap'
-                  and not disk.has_level(owner, 'owner')):
-                raise PermissionDenied()
-
         networks = (template.interface_set.all() if networks is None
                     else networks)
 

requirements/base.txt

@@ -4,6 +4,7 @@ billiard==3.3.0.17
 bpython==0.12
 celery==3.1.11
 Django==1.6.3
+django-autocomplete-light==1.4.14
 django-braces==1.4.0
 django-celery==3.1.10
 django-crispy-forms==1.4.0