Unittested and refactored the newnm procedure

backend/tests/test_views.py

@@ -8,31 +8,23 @@ from __future__ import division
 from __future__ import unicode_literals
 from django.test import TestCase
 from django.core.urlresolvers import reverse
+from backend.models import Person
 from backend import const
 from backend.unittest import PersonFixtureMixin
 
+
 class TestNewnm(PersonFixtureMixin, TestCase):
+    # Use an old, not yet revoked key of mine
+    new_person_fingerprint = "66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB"
+
     @classmethod
     def __add_extra_tests__(cls):
-        for person in ("dc", "dc_ga", "dm", "dm_ga"):
+        for person in ("pending", "dc", "dc_ga", "dm", "dm_ga"):
             cls._add_method(cls._test_non_dd, person)
 
         for person in ("dd_nu", "dd_u", "fd", "dam"):
             cls._add_method(cls._test_dd, person)
 
-        ## pending account
-        #cls.person.create("pending", status=const.STATUS_DC, expires=now() + datetime.timedelta(days=1), pending="12345", alioth=True)
-
-#    def _test_get_allowed(self, person):
-#        client = self.make_test_client(person)
-#        response = client.get(reverse("plant_obtouch_list"))
-#        self.assertEquals(response.status_code, 200)
-#
-#    def _test_get_forbidden(self, person):
-#        client = self.make_test_client(person)
-#        response = client.get(reverse("plant_obtouch_list"))
-#        self.assertPermissionDenied(response)
-
     def make_test_client(self, person):
         """
         Override the default make_test_client to allow sso-logged-in people
@@ -43,6 +35,28 @@ class TestNewnm(PersonFixtureMixin, TestCase):
         else:
             return super(TestNewnm, self).make_test_client(person)
 
+    def assertPostForbidden(self, person):
+        person = self.persons[person]
+        client = self.make_test_client(person)
+
+        # Posting to newnm to create a new record is forbidden
+        response = client.post(reverse("public_newnm"), data={"fpr": self.new_person_fingerprint, "sc_ok": "yes", "dmup_ok": "yes", "cn": "test", "email": "new_person@example.org"})
+        self.assertPermissionDenied(response)
+
+        # Trying to resend newnm challenge is forbidden
+        if person:
+            response = client.get(reverse("public_newnm_resend_challenge", kwargs={"key": person.lookup_key}))
+        else:
+            response = client.get(reverse("public_newnm_resend_challenge", kwargs={"key": self.persons["pending"].lookup_key}))
+        self.assertPermissionDenied(response)
+
+        # Trying to confirm the account is forbidden
+        if person:
+            response = client.get(reverse("public_newnm_confirm", kwargs={"nonce": person.pending}))
+        else:
+            response = client.get(reverse("public_newnm_confirm", kwargs={"nonce": self.persons["pending"].pending}))
+        self.assertPermissionDenied(response)
+
     def test_require_login(self):
         client = self.make_test_client(None)
         response = client.get(reverse("public_newnm"))
@@ -55,9 +69,10 @@ class TestNewnm(PersonFixtureMixin, TestCase):
         self.assertNotContains(response, "Not only you have an entry, but you are also")
         self.assertNotContains(response, "Apply for an entry in the system")
         self.assertNotContains(response, "Submit disabled because you already have an entry in the system")
+        self.assertPostForbidden(None)
 
     def test_no_person(self):
-        client = self.make_test_client("new_person@example.org")
+        client = self.make_test_client("new_person-guest@users.alioth.debian.org")
         response = client.get(reverse("public_newnm"))
         self.assertEquals(response.status_code, 200)
         self.assertEquals(response.context["person"], None)
@@ -69,6 +84,30 @@ class TestNewnm(PersonFixtureMixin, TestCase):
         self.assertContains(response, "Apply for an entry in the system")
         self.assertNotContains(response, "Submit disabled because you already have an entry in the system")
 
+        # A new Person is created on POST
+        response = client.post(reverse("public_newnm"), data={"fpr": self.new_person_fingerprint, "sc_ok": "yes", "dmup_ok": "yes", "cn": "test", "email": "new_person@example.org"})
+        self.assertRedirectMatches(response, reverse("public_newnm_resend_challenge", kwargs={"key": "new_person@example.org"}))
+        new_person = Person.lookup("new_person@example.org")
+        self.assertEquals(new_person.status, const.STATUS_DC)
+        self.assertIsNotNone(new_person.expires)
+        self.assertIsNotNone(new_person.pending)
+
+        # The new person can resend the challenge email
+        response = client.get(reverse("public_newnm_resend_challenge", kwargs={"key": "new_person@example.org"}))
+        self.assertRedirectMatches(response, new_person.get_absolute_url())
+
+        # The new person has a page in the system
+        response = client.get(new_person.get_absolute_url())
+        self.assertEquals(response.status_code, 200)
+
+        # The new person can confirm its record
+        response = client.get(reverse("public_newnm_confirm", kwargs={"nonce": new_person.pending}))
+        self.assertRedirectMatches(response, new_person.get_absolute_url())
+        new_person = Person.objects.get(pk=new_person.pk)
+        self.assertEquals(new_person.status, const.STATUS_DC)
+        self.assertIsNotNone(new_person.expires)
+        self.assertEquals(new_person.pending, "")
+
     def _test_non_dd(self, person):
         client = self.make_test_client(person)
         response = client.get(reverse("public_newnm"))
@@ -81,6 +120,7 @@ class TestNewnm(PersonFixtureMixin, TestCase):
         self.assertNotContains(response, "Not only you have an entry, but you are also")
         self.assertNotContains(response, "Apply for an entry in the system")
         self.assertNotContains(response, "Submit disabled because you already have an entry in the system")
+        self.assertPostForbidden(None)
 
     def _test_dd(self, person):
         client = self.make_test_client(person)
@@ -94,3 +134,4 @@ class TestNewnm(PersonFixtureMixin, TestCase):
         self.assertContains(response, "Not only you have an entry, but you are also")
         self.assertContains(response, "Apply for an entry in the system")
         self.assertContains(response, "Submit disabled because you already have an entry in the system")
+        self.assertPostForbidden(None)

backend/unittest.py

@@ -140,19 +140,17 @@ class TestBase(object):
             kw["SSL_CLIENT_S_DN_CN"] = sso_username
         return Client(**kw)
 
-    #def assertPermissionDenied(self, response):
-    #    if response.status_code == 403:
-    #        pass
-    #    elif response.status_code == 302:
-    #        self.assertRedirectMatches(response, reverse("login"))
-    #    else:
-    #        self.fail("response has status code {} instead of a 403 Forbidden or a 302 Redirect".format(response.status_code))
-
-    #def assertRedirectMatches(self, response, target):
-    #    if response.status_code != 302:
-    #        self.fail("response has status code {} instead of a Redirect".format(response.status_code))
-    #    if target and not re.search(target, response["Location"]):
-    #        self.fail("response redirects to {} which does not match {}".format(response["Location"], target))
+    def assertPermissionDenied(self, response):
+        if response.status_code == 403:
+            pass
+        else:
+            self.fail("response has status code {} instead of a 403 Forbidden".format(response.status_code))
+
+    def assertRedirectMatches(self, response, target):
+        if response.status_code != 302:
+            self.fail("response has status code {} instead of a Redirect".format(response.status_code))
+        if target and not re.search(target, response["Location"]):
+            self.fail("response redirects to {} which does not match {}".format(response["Location"], target))
 
 
 class PersonFixtureMixin(TestBase):

django_dacs/auth.py

+# coding: utf8
+from __future__ import print_function
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import unicode_literals
 import django.contrib.auth.backends
 import django.contrib.auth.middleware
 from django.conf import settings

public/urls.py

@@ -10,8 +10,8 @@ from . import views
 urlpatterns = [
     url(r'^$', RedirectView.as_view(url="/", permanent=True), name="public_index"),
     url(r'^newnm$', views.Newnm.as_view(), name="public_newnm"),
-    url(r'^newnm/resend_challenge/(?P<key>[^/]+)$', views.newnm_resend_challenge, name="public_newnm_resend_challenge"),
-    url(r'^newnm/confirm/(?P<nonce>[^/]+)$', views.newnm_confirm, name="public_newnm_confirm"),
+    url(r'^newnm/resend_challenge/(?P<key>[^/]+)$', views.NewnmResendChallenge.as_view(), name="public_newnm_resend_challenge"),
+    url(r'^newnm/confirm/(?P<nonce>[^/]+)$', views.NewnmConfirm.as_view(), name="public_newnm_confirm"),
     url(r'^processes$', views.Processes.as_view(), name="processes"),
     url(r'^managers$', views.Managers.as_view(), name="managers"),
     url(r'^people(?:/(?P<status>\w+))?$', views.People.as_view(), name="people"),

public/views.py

@@ -27,7 +27,7 @@ from django.core.urlresolvers import reverse
 from django.core.exceptions import PermissionDenied
 from django.utils.translation import ugettext as _
 from django.utils.timezone import now
-from django.views.generic import TemplateView
+from django.views.generic import TemplateView, View
 from django.views.generic.edit import FormView
 import backend.models as bmodels
 import backend.email as bemail
@@ -830,6 +830,9 @@ class Newnm(VisitorMixin, FormView):
         return redirect("public_newnm_resend_challenge", key=self.request.user.lookup_key)
 
     def form_valid(self, form):
+        if self.visitor is not None: raise PermissionDenied
+        if self.request.sso_username is None: raise PermissionDenied
+
         person = form.save(commit=False)
         person.username = self.request.sso_username
         person.status = const.STATUS_DC
@@ -877,40 +880,40 @@ class Newnm(VisitorMixin, FormView):
         )
         return ctx
 
-def newnm_resend_challenge(request, key):
+
+class NewnmResendChallenge(VisitorMixin, View):
     """
     Send/resend the encrypted email nonce for people who just requested a new
     Person record
     """
+    def get(self, request, key=None, *args, **kw):
+        from keyring.models import UserKey
 
-    # Check someone is logged in, to avoid casual crawling
-    if not request.user.is_authenticated():
-        raise PermissionDenied()
+        if self.visitor is None: raise PermissionDenied()
 
-    from keyring.models import UserKey
-    person = bmodels.Person.lookup_or_404(key)
+        # Deal gracefully with someone clicking the reconfirm link after they have
+        # already confirmed
+        if not self.visitor.pending: return redirect(self.visitor.get_absolute_url())
 
-    # Deal gracefully with someone clicking the reconfirm link after they have
-    # already confirmed
-    if not person.pending: return redirect(person.get_absolute_url())
+        confirm_url = request.build_absolute_uri(reverse("public_newnm_confirm", kwargs=dict(nonce=self.visitor.pending)))
+        plaintext = "Please visit {} to confirm your application at {}\n".format(
+                confirm_url,
+                request.build_absolute_uri(self.visitor.get_absolute_url()))
+        key = UserKey(self.visitor.fpr)
+        key.refresh()
+        encrypted = key.encrypt(plaintext.encode("utf8"))
+        bemail.send_nonce("notification_mails/newperson.txt", self.visitor, encrypted_nonce=encrypted)
+        return redirect(self.visitor.get_absolute_url())
 
-    confirm_url = request.build_absolute_uri(reverse("public_newnm_confirm", kwargs=dict(nonce=person.pending)))
-    plaintext = "Please visit {} to confirm your application at {}\n".format(
-            confirm_url,
-            request.build_absolute_uri(person.get_absolute_url()))
-    key = UserKey(person.fpr)
-    key.refresh()
-    encrypted = key.encrypt(plaintext.encode("utf8"))
-    bemail.send_nonce("notification_mails/newperson.txt", person, encrypted_nonce=encrypted)
-    return redirect(person.get_absolute_url())
 
-def newnm_confirm(request, nonce):
+class NewnmConfirm(VisitorMixin, View):
     """
     Confirm a pending Person object, given its nonce
     """
-    person = get_object_or_404(bmodels.Person, pending=nonce)
-    person.pending = ""
-    person.expires = now() + datetime.timedelta(days=30)
-    person.save(audit_author=person, audit_notes="confirmed pending subscription")
-    return redirect(person.get_absolute_url())
-
+    def get(self, request, nonce, *args, **kw):
+        if self.visitor is None: raise PermissionDenied
+        if self.visitor.pending != nonce: raise PermissionDenied
+        self.visitor.pending = ""
+        self.visitor.expires = now() + datetime.timedelta(days=30)
+        self.visitor.save(audit_author=self.visitor, audit_notes="confirmed pending subscription")
+        return redirect(self.visitor.get_absolute_url())