Merge branch 'feature-tx-ownership' into 'master'

Feature: Transer Ownership Fixes #39

Merge branch 'feature-tx-ownership' into 'master'
Feature: Transer Ownership Fixes #39
031f3fcc · Őry Máté · 9f818d85 · e62e9383 · 031f3fcc · 031f3fcc
Commit 031f3fcc authored Mar 02, 2014 by Őry Máté
8 changed files
--- a/circle/dashboard/templates/dashboard/_notifications-timeline.html
+++ b/circle/dashboard/templates/dashboard/_notifications-timeline.html
@@ -10,7 +10,7 @@
    </span>
    <div style="clear: both;"></div>
    <div class="notification-message-text">
-      {{ n.message }}
+      {{ n.message|safe }}
    </div>
  </li>
 {% empty %}

--- a/circle/dashboard/templates/dashboard/notifications/ownership-accepted.html
+++ b/circle/dashboard/templates/dashboard/notifications/ownership-accepted.html
+{%load i18n%}
+{%blocktrans with instance=instance.name user=user.name%}
+Your ownership offer of {{instance}} has been accepted by {{user}}.
+{%endblocktrans%}
--- a/circle/dashboard/templates/dashboard/notifications/ownership-offer.html
+++ b/circle/dashboard/templates/dashboard/notifications/ownership-offer.html
+{%load i18n%}
+{%blocktrans with instance=instance.name user=user.name%}
+{{user}} offered you to take the ownership of his/her virtual machine
+called {{instance}}.{%endblocktrans%} 
+<a href="{{token}}" class="btn btn-success btn-small">{%trans "Accept"%}</a>
--- a/circle/dashboard/templates/dashboard/vm-detail/access.html
+++ b/circle/dashboard/templates/dashboard/vm-detail/access.html
 {% load i18n %}
 <h3>{% trans "Owner" %}</h3>
 <p>
+  {% if user == instance.owner %}
  {% blocktrans %}You are the current owner of this instance.{% endblocktrans %}
-  <a href="#" class="btn btn-link">{% trans "Transfer ownership..." %}</a>
+  {% else %}
+  {% blocktrans with owner=instance.owner %}
+    The current owner of this instance is {{owner}}.
+  {% endblocktrans %}
+  {% endif %}
+  {% if user == instance.owner or user.is_superuser %}
+  <a href="{% url "dashboard.views.vm-transfer-ownership" instance.pk %}"
+      class="btn btn-link">{% trans "Transfer ownership..." %}</a>
+  {% endif %}
 </p>
 <h3>{% trans "Permissions"|capfirst %}</h3>
  <form action="{{acl.url}}" method="post">{% csrf_token %}

--- a/circle/dashboard/templates/dashboard/vm-detail/tx-owner.html
+++ b/circle/dashboard/templates/dashboard/vm-detail/tx-owner.html
@@ -13,6 +13,7 @@
        <div class="pull-right">
          <form action="" method="POST">
            {% csrf_token %}
+            E-mail address or identifier of user:
            <input name="name">
            <input type="submit">
          </form>

--- a/circle/dashboard/tests/test_views.py
+++ b/circle/dashboard/tests/test_views.py
 from django.test import TestCase
 from django.test.client import Client
 from django.contrib.auth.models import User, Group
+from django.core.exceptions import SuspiciousOperation
 from vm.models import Instance, InstanceTemplate, Lease, Node
+from ..models import Profile
 from storage.models import Disk
 from firewall.models import Vlan
-class VmDetailTest(TestCase):
+class LoginMixin(object):
+    def login(self, client, username, password='password'):
+        response = client.post('/accounts/login/', {'username': username,
+                                                    'password': password})
+        self.assertNotEqual(response.status_code, 403)
+class VmDetailTest(LoginMixin, TestCase):
    fixtures = ['test-vm-fixture.json']
    def setUp(self):
@@ -32,11 +41,6 @@ class VmDetailTest(TestCase):
        self.us.delete()
        self.g1.delete()
-    def login(self, client, username, password='password'):
-        response = client.post('/accounts/login/', {'username': username,
-                                                    'password': password})
-        self.assertNotEqual(response.status_code, 403)
    def test_404_vm_page(self):
        c = Client()
        self.login(c, 'user1')
@@ -236,7 +240,7 @@ class VmDetailTest(TestCase):
        assert self.u1.notification_set.get().status == 'read'
-class VmDetailVncTest(TestCase):
+class VmDetailVncTest(LoginMixin, TestCase):
    fixtures = ['test-vm-fixture.json', 'node.json']
    def setUp(self):
@@ -244,11 +248,6 @@ class VmDetailVncTest(TestCase):
        self.u1.set_password('password')
        self.u1.save()
-    def login(self, client, username, password='password'):
-        response = client.post('/accounts/login/', {'username': username,
-                                                    'password': password})
-        self.assertNotEqual(response.status_code, 403)
    def test_permitted_vm_console(self):
        c = Client()
        self.login(c, 'user1')
@@ -268,3 +267,70 @@ class VmDetailVncTest(TestCase):
        inst.set_level(self.u1, 'user')
        response = c.get('/dashboard/vm/1/vnctoken/')
        self.assertEqual(response.status_code, 403)
+class TransferOwnershipViewTest(LoginMixin, TestCase):
+    fixtures = ['test-vm-fixture.json']
+    def setUp(self):
+        self.u1 = User.objects.create(username='user1')
+        self.u1.set_password('password')
+        self.u1.save()
+        Profile.objects.create(user=self.u1)
+        self.u2 = User.objects.create(username='user2', is_staff=True)
+        self.u2.set_password('password')
+        self.u2.save()
+        Profile.objects.create(user=self.u2)
+        self.us = User.objects.create(username='superuser', is_superuser=True)
+        self.us.set_password('password')
+        self.us.save()
+        Profile.objects.create(user=self.us)
+        inst = Instance.objects.get(pk=1)
+        inst.owner = self.u1
+        inst.save()
+    def test_non_owner_offer(self):
+        c2 = self.u2.notification_set.count()
+        c = Client()
+        self.login(c, 'user2')
+        with self.assertRaises(SuspiciousOperation):
+            c.post('/dashboard/vm/1/tx/')
+        self.assertEqual(self.u2.notification_set.count(), c2)
+    def test_owned_offer(self):
+        c2 = self.u2.notification_set.count()
+        c = Client()
+        self.login(c, 'user1')
+        response = c.get('/dashboard/vm/1/tx/')
+        assert response.status_code == 200
+        response = c.post('/dashboard/vm/1/tx/', {'name': 'user2'})
+        self.assertEqual(self.u2.notification_set.count(), c2 + 1)
+    def test_transfer(self):
+        c = Client()
+        self.login(c, 'user1')
+        response = c.post('/dashboard/vm/1/tx/', {'name': 'user2'})
+        url = response.context['token']
+        c = Client()
+        self.login(c, 'user2')
+        response = c.post(url)
+        self.assertEquals(Instance.objects.get(pk=1).owner.pk, self.u2.pk)
+    def test_transfer_token_used_by_others(self):
+        c = Client()
+        self.login(c, 'user1')
+        response = c.post('/dashboard/vm/1/tx/', {'name': 'user2'})
+        url = response.context['token']
+        response = c.post(url)  # token is for user2
+        assert response.status_code == 403
+        self.assertEquals(Instance.objects.get(pk=1).owner.pk, self.u1.pk)
+    def test_transfer_by_superuser(self):
+        c = Client()
+        self.login(c, 'superuser')
+        response = c.post('/dashboard/vm/1/tx/', {'name': 'user2'})
+        url = response.context['token']
+        c = Client()
+        self.login(c, 'user2')
+        response = c.post(url)
+        self.assertEquals(Instance.objects.get(pk=1).owner.pk, self.u2.pk)
--- a/circle/dashboard/urls.py
+++ b/circle/dashboard/urls.py
@@ -57,7 +57,7 @@ urlpatterns = patterns(
    url(r'^node/list/$', NodeList.as_view(), name='dashboard.views.node-list'),
    url(r'^node/(?P<pk>\d+)/$', NodeDetailView.as_view(),
        name='dashboard.views.node-detail'),
-    url(r'^tx/$', TransferOwnershipConfirmView.as_view(),
+    url(r'^tx/(?P<key>.*)/?$', TransferOwnershipConfirmView.as_view(),
        name='dashboard.views.vm-transfer-ownership-confirm'),
    url(r'^node/delete/(?P<pk>\d+)/$', NodeDelete.as_view(),
        name="dashboard.views.delete-node"),

--- a/circle/dashboard/views.py
+++ b/circle/dashboard/views.py
@@ -37,7 +37,7 @@ from vm.models import (Instance, InstanceTemplate, InterfaceTemplate,
                       InstanceActivity, Node, instance_activity, Lease,
                       Interface, NodeActivity)
 from firewall.models import Vlan, Host, Rule
-from dashboard.models import Favourite
+from dashboard.models import Favourite, Profile
 logger = logging.getLogger(__name__)
@@ -1490,7 +1490,12 @@ class TransferOwnershipView(LoginRequiredMixin, DetailView):
        try:
            new_owner = User.objects.get(username=request.POST['name'])
        except User.DoesNotExist:
-            raise Http404()
+            new_owner = User.objects.get(email=request.POST['name'])
+        except User.DoesNotExist:
+            new_owner = User.objects.get(profile__org_id=request.POST['name'])
+        except User.DoesNotExist:
+            messages.error(request, _('Can not find specified user.'))
+            return self.get(request, *args, **kwargs)
        except KeyError:
            raise SuspiciousOperation()
@@ -1501,29 +1506,41 @@ class TransferOwnershipView(LoginRequiredMixin, DetailView):
        token = signing.dumps((obj.pk, new_owner.pk),
                              salt=TransferOwnershipConfirmView.get_salt())
-        return HttpResponse("%s?key=%s" % (
+        token_path = reverse(
-            reverse('dashboard.views.vm-transfer-ownership-confirm'), token),
+            'dashboard.views.vm-transfer-ownership-confirm', args=[token])
-            content_type="text/plain")
+        try:
+            new_owner.profile.notify(
+                _('Ownership offer'),
+                'dashboard/notifications/ownership-offer.html',
+                {'instance': obj, 'token': token_path})
+        except Profile.DoesNotExist:
+            messages.error(request, _('Can not notify selected user.'))
+        else:
+            messages.success(request,
+                             _('User %s is notified about the offer.') % (
+                                 unicode(new_owner), ))
+        return redirect(reverse_lazy("dashboard.views.detail",
+                                     kwargs={'pk': obj.pk}))
 class TransferOwnershipConfirmView(LoginRequiredMixin, View):
+    """User can accept an ownership offer."""
    max_age = 3 * 24 * 3600
-    success_message = _("Ownership successfully transferred.")
+    success_message = _("Ownership successfully transferred to you.")
    @classmethod
    def get_salt(cls):
        return unicode(cls)
-    def get(self, request, *args, **kwargs):
+    def get(self, request, key, *args, **kwargs):
        """Confirm ownership transfer based on token.
        """
-        try:
-            key = request.GET['key']
        logger.debug('Confirm dialog for token %s.', key)
+        try:
            instance, new_owner = self.get_instance(key, request.user)
-        except KeyError:
+        except PermissionDenied:
-            raise Http404()
-        except PermissionDenied():
            messages.error(request, _('This token is for an other user.'))
            raise
        except SuspiciousOperation:
@@ -1533,16 +1550,10 @@ class TransferOwnershipConfirmView(LoginRequiredMixin, View):
                      "dashboard/confirm/base-transfer-ownership.html",
                      dictionary={'instance': instance, 'key': key})
-    def post(self, request, *args, **kwargs):
+    def post(self, request, key, *args, **kwargs):
        """Really transfer ownership based on token.
        """
-        try:
-            key = request.POST['key']
        instance, owner = self.get_instance(key, request.user)
-        except KeyError:
-            logger.debug('Posted to %s without key field.',
-                         unicode(self.__class__))
-            raise SuspiciousOperation()
        old = instance.owner
        with instance_activity(code_suffix='ownership-transferred',
@@ -1553,6 +1564,11 @@ class TransferOwnershipConfirmView(LoginRequiredMixin, View):
        messages.success(request, self.success_message)
        logger.info('Ownership of %s transferred from %s to %s.',
                    unicode(instance), unicode(old), unicode(request.user))
+        if old.profile:
+            old.profile.notify(
+                _('Ownership accepted'),
+                'dashboard/notifications/ownership-accepted.html',
+                {'instance': instance})
        return HttpResponseRedirect(instance.get_absolute_url())
    def get_instance(self, key, user):
@@ -1562,15 +1578,7 @@ class TransferOwnershipConfirmView(LoginRequiredMixin, View):
            instance, new_owner = (
                signing.loads(key, max_age=self.max_age,
                              salt=self.get_salt()))
-        except signing.BadSignature as e:
+        except (signing.BadSignature, ValueError, TypeError) as e:
-            logger.error('Tried invalid token. Token: %s, user: %s. %s',
-                         key, unicode(user), unicode(e))
-            raise SuspiciousOperation()
-        except ValueError as e:
-            logger.error('Tried invalid token. Token: %s, user: %s. %s',
-                         key, unicode(user), unicode(e))
-            raise SuspiciousOperation()
-        except TypeError as e:
            logger.error('Tried invalid token. Token: %s, user: %s. %s',
                         key, unicode(user), unicode(e))
            raise SuspiciousOperation()