When adding a prefix/AS to a child, verify that it does not

overlap with a previous allocation to the same child.  This will
avoid raising an exception about the uniqueness constraints on
irdb.models.Child{Net,ASN}

Use django.contrib.formtools to provide a double confirmation for
AS/prefix additions to a child

closes #343


svn path=/trunk/; revision=4864

5 files changed, 170 insertions, 101 deletions

diff --git a/rpkid/portal-gui/settings.py.in b/rpkid/portal-gui/settings.py.in
index 10705ef8..46f6ca60 100644
--- a/rpkid/portal-gui/settings.py.in
+++ b/rpkid/portal-gui/settings.py.in
@@ -86,12 +86,13 @@ INSTALLED_APPS = (
     #'django.contrib.admin',
     #'django.contrib.admindocs',
     'django.contrib.contenttypes',
+    'django.contrib.formtools',
     'django.contrib.sessions',
     'rpki.irdb',
     'rpki.gui.app',
     'rpki.gui.cacheview',
     'rpki.gui.routeview',
-    'south'
+    'south',
 )
 
 TEMPLATE_CONTEXT_PROCESSORS = (
diff --git a/rpkid/rpki/gui/app/forms.py b/rpkid/rpki/gui/app/forms.py
index 68bd0a3f..5429d61e 100644
--- a/rpkid/rpki/gui/app/forms.py
+++ b/rpkid/rpki/gui/app/forms.py
@@ -282,14 +282,13 @@ class ROARequestConfirm(forms.Form):
         return self.cleaned_data
 
 
-def AddASNForm(qs):
+def AddASNForm(child):
     """
-    Generate a form class which only allows specification of ASNs contained
-    within the specified queryset.  `qs` should be a QuerySet of
-    irdb.models.ChildASN.
+    Returns a forms.Form subclass which verifies that the entered ASN range
+    does not overlap with a previous allocation to the specified child, and
+    that the ASN range is within the range allocated to the parent.
 
     """
-
     class _wrapped(forms.Form):
         asns = forms.CharField(label='ASNs', help_text='single ASN or range')
 
@@ -298,21 +297,30 @@ def AddASNForm(qs):
                 r = resource_range_as.parse_str(self.cleaned_data.get('asns'))
             except:
                 raise forms.ValidationError('invalid AS or range')
-            if not qs.filter(min__lte=r.min, max__gte=r.max).exists():
+
+            if not models.ResourceRangeAS.objects.filter(
+                cert__conf=child.issuer,
+                min__lte=r.min,
+                max__gte=r.max).exists():
                 raise forms.ValidationError('AS or range is not delegated to you')
+
+            # determine if the entered range overlaps with any AS already
+            # allocated to this child
+            if child.asns.filter(end_as__gte=r.min, start_as__lte=r.max).exists():
+                raise forms.ValidationError(
+                    'Overlap with previous allocation to this child')
+
             return str(r)
 
     return _wrapped
 
-
-def AddNetForm(qsv4, qsv6):
+def AddNetForm(child):
     """
-    Generate a form class which only allows specification of prefixes contained
-    within the specified queryset.  `qs` should be a QuerySet of
-    irdb.models.ChildNet.
+    Returns a forms.Form subclass which validates that the entered address
+    range is within the resources allocated to the parent, and does not overlap
+    with what is already allocated to the specified child.
 
     """
-
     class _wrapped(forms.Form):
         address_range = forms.CharField(help_text='CIDR or range')
 
@@ -321,19 +329,32 @@ def AddNetForm(qsv4, qsv6):
             try:
                 if ':' in address_range:
                     r = resource_range_ipv6.parse_str(address_range)
-                    if not qsv6.filter(prefix_min__lte=r.min, prefix_max__gte=r.max).exists():
-                        raise forms.ValidationError('IP address range is not delegated to you')
+                    qs = models.ResourceRangeAddressV6
+                    version = 'IPv6'
                 else:
                     r = resource_range_ipv4.parse_str(address_range)
-                    if not qsv4.filter(prefix_min__lte=r.min, prefix_max__gte=r.max).exists():
-                        raise forms.ValidationError('IP address range is not delegated to you')
+                    qs = models.ResourceRangeAddressV4
+                    version = 'IPv4'
             except BadIPResource:
                 raise forms.ValidationError('invalid IP address range')
+
+            if not qs.objects.filter(cert__conf=child.issuer,
+                                     prefix_min__lte=r.min,
+                                     prefix_max__gte=r.max).exists():
+                raise forms.ValidationError('IP address range is not delegated to you')
+
+            # determine if the entered range overlaps with any prefix
+            # already allocated to this child
+            for n in child.address_ranges.filter(version=version):
+                rng = n.as_resource_range()
+                if r.max >= rng.min and r.min <= rng.max:
+                    raise forms.ValidationError(
+                        'Overlap with previous allocation to this child')
+
             return str(r)
 
     return _wrapped
 
-
 def ChildForm(instance):
     """
     Form for editing a Child model.
diff --git a/rpkid/rpki/gui/app/templates/app/child_add_resource_form.html b/rpkid/rpki/gui/app/templates/app/child_add_resource_form.html
deleted file mode 100644
index 98789191..00000000
--- a/rpkid/rpki/gui/app/templates/app/child_add_resource_form.html
+++ /dev/null
@@ -1,16 +0,0 @@
-{% extends "app/app_base.html" %}
-
-{% block content %}
-<div class='page-header'>
-  <h1>Add Resource: {{ object.handle }}</h1>
-</div>
-
-<form method='POST' action='{{ request.get_full_path }}'>
-  {% csrf_token %}
-  {% include "app/bootstrap_form.html" %}
-  <div class='actions'>
-    <input class='btn primary' type='submit' value='Save'>
-    <a class='btn' href='{{ object.get_absolute_url }}'>Cancel</a>
-  </div>
-</form>
-{% endblock content %}
diff --git a/rpkid/rpki/gui/app/templates/app/object_detail.html b/rpkid/rpki/gui/app/templates/app/object_detail.html
index 6a93f644..3927bfd5 100644
--- a/rpkid/rpki/gui/app/templates/app/object_detail.html
+++ b/rpkid/rpki/gui/app/templates/app/object_detail.html
@@ -10,24 +10,68 @@
 {{ object }}
 {% endblock object_detail %}
 
-{% if confirm_delete %}
-<div class='alert-message block-message warning'>
-  <p><strong>Please confirm</strong> that you would like to delete this object.
-  <div class='alert-actions'>
-    <form method='POST' action='{{ request.get_full_path }}'>
-      {% csrf_token %}
-      <input class='btn danger' type='submit' value='Delete'/>
+{% if form %}
+  <h2>{{ form_label }}<h2>
+
+  {% if is_preview %}
+      <h3>Preview</h3>
+  <table>
+  {% for field in form %}
+  <tr>
+  <th>{{ field.label }}:</th>
+  <td>{{ field.data }}</td>
+  </tr>
+  {% endfor %}
+  </table>
+
+  <!-- <p>Security hash: {{ hash_value }}</p> -->
+
+  <!-- this form is used by django.contrib.formtools -->
+  <form action="" method="post">{% csrf_token %}
+  {% for field in form %}{{ field.as_hidden }}
+  {% endfor %}
+  <input type="hidden" name="{{ stage_field }}" value="2" />
+  <input type="hidden" name="{{ hash_field }}" value="{{ hash_value }}" />
+    <div class='actions'>
+      <input class='btn primary' type="submit" value="Save" /></p>
+    </div>
+  </form>
+
+  <h3>Or edit again</h3>
+
+  {% endif %}
+
+  <!-- this form is used by django.contrib.formtools -->
+  <form method='POST' action=''>
+    {% csrf_token %}
+    {% include "app/bootstrap_form.html" %}
+    <div class='actions'>
+      <input class='btn primary' type='submit' value='Preview'>
       <a class='btn' href='{{ object.get_absolute_url }}'>Cancel</a>
-    </form>
-  </div>
-</div>
+    </div>
+    <input type="hidden" name="{{ stage_field }}" value="1" />
+  </form>
+
 {% else %}
-<div class='actions'>
-  {% if can_edit %}
-  <a class='btn' href='{{ object.get_absolute_url }}/edit'>Edit</a>
+  {% if confirm_delete %}
+  <div class='alert-message block-message warning'>
+    <p><strong>Please confirm</strong> that you would like to delete this object.
+    <div class='alert-actions'>
+      <form method='POST' action='{{ request.get_full_path }}'>
+	{% csrf_token %}
+	<input class='btn danger' type='submit' value='Delete'/>
+	<a class='btn' href='{{ object.get_absolute_url }}'>Cancel</a>
+      </form>
+    </div>
+  </div>
+  {% else %}
+  <div class='actions'>
+    {% if can_edit %}
+    <a class='btn' href='{{ object.get_absolute_url }}/edit'>Edit</a>
+    {% endif %}
+    <a class='btn danger' href='{{ object.get_absolute_url }}/delete' title='Permanently delete this object'>Delete</a>
+    {% block actions %}{% endblock actions %}
+  </div>
   {% endif %}
-  <a class='btn danger' href='{{ object.get_absolute_url }}/delete' title='Permanently delete this object'>Delete</a>
-  {% block actions %}{% endblock actions %}
-</div>
 {% endif %}
 {% endblock content %}
diff --git a/rpkid/rpki/gui/app/views.py b/rpkid/rpki/gui/app/views.py
index 0c905070..f72608aa 100644
--- a/rpkid/rpki/gui/app/views.py
+++ b/rpkid/rpki/gui/app/views.py
@@ -31,15 +31,14 @@ from django.utils.http import urlquote
 from django.template import RequestContext
 from django import http
 from django.views.generic.list_detail import object_list, object_detail
-from django.views.generic.create_update import delete_object
 from django.core.urlresolvers import reverse
 from django.contrib.auth.models import User
+from django.contrib.formtools.preview import FormPreview
 
 from rpki.irdb import Zookeeper, ChildASN, ChildNet
 from rpki.gui.app import models, forms, glue, range_list
 from rpki.resource_set import (resource_range_as, resource_range_ipv4,
                                resource_range_ipv6, roa_prefix_ipv4)
-from rpki.exceptions import BadIPResource
 from rpki import sundial
 
 from rpki.gui.cacheview.models import ROAPrefixV4, ROAPrefixV6, ROA
@@ -339,61 +338,81 @@ def child_list(request):
                 'create_label': 'Import'})
 
 
-@handle_required
-def child_add_resource(request, pk, form_class, unused_list, callback,
-                       template_name='app/child_add_resource_form.html'):
-    conf = request.session['handle']
-    child = models.Child.objects.get(issuer=conf, pk=pk)
-    log = request.META['wsgi.errors']
-    if request.method == 'POST':
-        form = form_class(request.POST, request.FILES)
-        if form.is_valid():
-            callback(child, form)
-            Zookeeper(handle=conf.handle, logstream=log).run_rpkid_now()
-            return http.HttpResponseRedirect(child.get_absolute_url())
-    else:
-        form = form_class()
-
-    return render(request, template_name,
-                  {'object': child, 'form': form, 'unused': unused_list})
-
-
-def add_asn_callback(child, form):
-    asns = form.cleaned_data.get('asns')
-    r = resource_range_as.parse_str(asns)
-    child.asns.create(start_as=r.min, end_as=r.max)
-
-
-def child_add_asn(request, pk):
-    conf = request.session['handle']
-    get_object_or_404(models.Child, issuer=conf, pk=pk)
-    qs = models.ResourceRangeAS.objects.filter(cert__conf=conf)
-    return child_add_resource(request, pk, forms.AddASNForm(qs), [],
-                              add_asn_callback)
-
+class ChildAddResourcePreview(FormPreview):
+    """
+    Base class for handling preview of AS/Prefix additions to a child.
+    Subclasses implement the 'done' method to perform actual work on IRDB.
 
-def add_address_callback(child, form):
-    address_range = form.cleaned_data.get('address_range')
-    if ':' in address_range:
-        r = resource_range_ipv6.parse_str(address_range)
-        version = 'IPv6'
-    else:
-        r = resource_range_ipv4.parse_str(address_range)
-        version = 'IPv4'
-    child.address_ranges.create(start_ip=str(r.min), end_ip=str(r.max),
-                                version=version)
+    """
 
+    form_template = 'app/child_detail.html'
+    preview_template = 'app/child_detail.html'
+
+    def __init__(self, *args, **kwargs):
+        """
+        The docstring for FormPreview says we should not redefine this method, but
+        I don't see how we can set extra information in this class otherwise.
+
+        """
+        self.child = kwargs.pop('child')
+        self.logstream = kwargs.pop('logstream')
+        super(ChildAddResourcePreview, self).__init__(*args, **kwargs)
+
+    def get_context(self, *args, **kwargs):
+        """"
+        Override the superclass method to add context variables needed by the
+        form template.
+
+        """
+        d = super(ChildAddResourcePreview, self).get_context(*args, **kwargs)
+        d['object'] = self.child
+        d['form_label'] = 'Add Resource'
+        return d
+
+    def process_preview(self, request, form, context):
+        # set a boolean flag so that the template knows this is a preview
+        context['is_preview'] = True
+
+
+class ChildAddPrefixPreview(ChildAddResourcePreview):
+    def done(self, request, cleaned_data):
+        address_range = cleaned_data.get('address_range')
+        if ':' in address_range:
+            r = resource_range_ipv6.parse_str(address_range)
+            version = 'IPv6'
+        else:
+            r = resource_range_ipv4.parse_str(address_range)
+            version = 'IPv4'
+        self.child.address_ranges.create(start_ip=str(r.min), end_ip=str(r.max),
+                                         version=version)
+        Zookeeper(handle=self.child.issuer.handle, logstream=self.logstream).run_rpkid_now()
+        return http.HttpResponseRedirect(self.child.get_absolute_url())
 
+@handle_required
 def child_add_address(request, pk):
+    logstream = request.META['wsgi.errors']
     conf = request.session['handle']
-    get_object_or_404(models.Child, issuer=conf, pk=pk)
-    qsv4 = models.ResourceRangeAddressV4.objects.filter(cert__conf=conf)
-    qsv6 = models.ResourceRangeAddressV6.objects.filter(cert__conf=conf)
-    return child_add_resource(request, pk,
-                              forms.AddNetForm(qsv4, qsv6),
-                              [],
-                              callback=add_address_callback)
+    child = get_object_or_404(models.Child, issuer=conf, pk=pk)
+    form = forms.AddNetForm(child)
+    preview = ChildAddPrefixPreview(form, child=child, logstream=logstream)
+    return preview(request)
+
+class ChildAddASNPreview(ChildAddResourcePreview):
+    def done(self, request, cleaned_data):
+        asns = cleaned_data.get('asns')
+        r = resource_range_as.parse_str(asns)
+        self.child.asns.create(start_as=r.min, end_as=r.max)
+        Zookeeper(handle=self.child.issuer.handle, logstream=self.logstream).run_rpkid_now()
+        return http.HttpResponseRedirect(self.child.get_absolute_url())
 
+@handle_required
+def child_add_asn(request, pk):
+    logstream = request.META['wsgi.errors']
+    conf = request.session['handle']
+    child = get_object_or_404(models.Child, issuer=conf, pk=pk)
+    form = forms.AddASNForm(child)
+    preview = ChildAddASNPreview(form, child=child, logstream=logstream)
+    return preview(request)
 
 @handle_required
 def child_view(request, pk):