From f70f0f8d62736b3b8781920bbf0aba758ae9ea14 Mon Sep 17 00:00:00 2001
From: Jeremy Stretch <jstretch@digitalocean.com>
Date: Thu, 13 Apr 2017 13:11:23 -0400
Subject: [PATCH] Improved handling of return_url for object edit/delete views;
 removed manual definitions of initial data fields

---
 netbox/circuits/views.py                      |  6 +-
 netbox/dcim/forms.py                          |  2 +-
 netbox/dcim/views.py                          | 21 +++---
 netbox/ipam/views.py                          | 10 ++-
 netbox/secrets/views.py                       |  2 +-
 .../utilities/confirmation_form.html          |  6 +-
 netbox/tenancy/views.py                       |  3 +-
 netbox/utilities/forms.py                     | 13 ++--
 netbox/utilities/views.py                     | 65 ++++++++++---------
 9 files changed, 65 insertions(+), 63 deletions(-)

diff --git a/netbox/circuits/views.py b/netbox/circuits/views.py
index 1ffda899b..110994e81 100644
--- a/netbox/circuits/views.py
+++ b/netbox/circuits/views.py
@@ -95,7 +95,7 @@ class CircuitTypeEditView(PermissionRequiredMixin, ObjectEditView):
     model = CircuitType
     form_class = forms.CircuitTypeForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('circuits:circuittype_list')
 
 
@@ -142,7 +142,6 @@ class CircuitEditView(PermissionRequiredMixin, ObjectEditView):
     permission_required = 'circuits.change_circuit'
     model = Circuit
     form_class = forms.CircuitForm
-    fields_initial = ['provider']
     template_name = 'circuits/circuit_edit.html'
     default_return_url = 'circuits:circuit_list'
 
@@ -230,7 +229,6 @@ class CircuitTerminationEditView(PermissionRequiredMixin, ObjectEditView):
     permission_required = 'circuits.change_circuittermination'
     model = CircuitTermination
     form_class = forms.CircuitTerminationForm
-    fields_initial = ['term_side']
     template_name = 'circuits/circuittermination_edit.html'
 
     def alter_obj(self, obj, request, url_args, url_kwargs):
@@ -238,7 +236,7 @@ class CircuitTerminationEditView(PermissionRequiredMixin, ObjectEditView):
             obj.circuit = get_object_or_404(Circuit, pk=url_kwargs['circuit'])
         return obj
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return obj.circuit.get_absolute_url()
 
 
diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py
index 80326b137..7a4d093c4 100644
--- a/netbox/dcim/forms.py
+++ b/netbox/dcim/forms.py
@@ -1708,7 +1708,7 @@ class IPAddressForm(BootstrapMixin, CustomFieldForm):
 
         self.fields['vrf'].empty_label = 'Global'
 
-        interfaces = device.interfaces.all()
+        interfaces = device.interfaces.order_naturally(method=device.device_type.interface_ordering)
         self.fields['interface'].queryset = interfaces
         self.fields['interface'].required = True
 
diff --git a/netbox/dcim/views.py b/netbox/dcim/views.py
index edaa56d22..6bf5ececd 100644
--- a/netbox/dcim/views.py
+++ b/netbox/dcim/views.py
@@ -124,13 +124,13 @@ class ComponentCreateView(View):
 
 class ComponentEditView(ObjectEditView):
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return obj.device.get_absolute_url()
 
 
 class ComponentDeleteView(ObjectDeleteView):
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return obj.device.get_absolute_url()
 
 
@@ -149,7 +149,7 @@ class RegionEditView(PermissionRequiredMixin, ObjectEditView):
     model = Region
     form_class = forms.RegionForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('dcim:region_list')
 
 
@@ -242,7 +242,7 @@ class RackGroupEditView(PermissionRequiredMixin, ObjectEditView):
     model = RackGroup
     form_class = forms.RackGroupForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('dcim:rackgroup_list')
 
 
@@ -268,7 +268,7 @@ class RackRoleEditView(PermissionRequiredMixin, ObjectEditView):
     model = RackRole
     form_class = forms.RackRoleForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('dcim:rackrole_list')
 
 
@@ -379,7 +379,7 @@ class RackReservationEditView(PermissionRequiredMixin, ObjectEditView):
             obj.user = request.user
         return obj
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return obj.rack.get_absolute_url()
 
 
@@ -387,7 +387,7 @@ class RackReservationDeleteView(PermissionRequiredMixin, ObjectDeleteView):
     permission_required = 'dcim.delete_rackreservation'
     model = RackReservation
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return obj.rack.get_absolute_url()
 
 
@@ -412,7 +412,7 @@ class ManufacturerEditView(PermissionRequiredMixin, ObjectEditView):
     model = Manufacturer
     form_class = forms.ManufacturerForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('dcim:manufacturer_list')
 
 
@@ -632,7 +632,7 @@ class DeviceRoleEditView(PermissionRequiredMixin, ObjectEditView):
     model = DeviceRole
     form_class = forms.DeviceRoleForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('dcim:devicerole_list')
 
 
@@ -657,7 +657,7 @@ class PlatformEditView(PermissionRequiredMixin, ObjectEditView):
     model = Platform
     form_class = forms.PlatformForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('dcim:platform_list')
 
 
@@ -750,7 +750,6 @@ class DeviceEditView(PermissionRequiredMixin, ObjectEditView):
     permission_required = 'dcim.change_device'
     model = Device
     form_class = forms.DeviceForm
-    fields_initial = ['site', 'rack', 'position', 'face', 'device_bay']
     template_name = 'dcim/device_edit.html'
     default_return_url = 'dcim:device_list'
 
diff --git a/netbox/ipam/views.py b/netbox/ipam/views.py
index 864909878..2d2e1f044 100644
--- a/netbox/ipam/views.py
+++ b/netbox/ipam/views.py
@@ -244,7 +244,7 @@ class RIREditView(PermissionRequiredMixin, ObjectEditView):
     model = RIR
     form_class = forms.RIRForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('ipam:rir_list')
 
 
@@ -370,7 +370,7 @@ class RoleEditView(PermissionRequiredMixin, ObjectEditView):
     model = Role
     form_class = forms.RoleForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('ipam:role_list')
 
 
@@ -464,7 +464,6 @@ class PrefixEditView(PermissionRequiredMixin, ObjectEditView):
     model = Prefix
     form_class = forms.PrefixForm
     template_name = 'ipam/prefix_edit.html'
-    fields_initial = ['vrf', 'tenant', 'site', 'prefix', 'vlan']
     default_return_url = 'ipam:prefix_list'
 
 
@@ -645,7 +644,6 @@ class IPAddressEditView(PermissionRequiredMixin, ObjectEditView):
     permission_required = 'ipam.change_ipaddress'
     model = IPAddress
     form_class = forms.IPAddressForm
-    fields_initial = ['address', 'vrf']
     template_name = 'ipam/ipaddress_edit.html'
     default_return_url = 'ipam:ipaddress_list'
 
@@ -718,7 +716,7 @@ class VLANGroupEditView(PermissionRequiredMixin, ObjectEditView):
     model = VLANGroup
     form_class = forms.VLANGroupForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('ipam:vlangroup_list')
 
 
@@ -807,7 +805,7 @@ class ServiceEditView(PermissionRequiredMixin, ObjectEditView):
             obj.device = get_object_or_404(Device, pk=url_kwargs['device'])
         return obj
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return obj.device.get_absolute_url()
 
 
diff --git a/netbox/secrets/views.py b/netbox/secrets/views.py
index d67cd18a0..1d27c538a 100644
--- a/netbox/secrets/views.py
+++ b/netbox/secrets/views.py
@@ -30,7 +30,7 @@ class SecretRoleEditView(PermissionRequiredMixin, ObjectEditView):
     model = SecretRole
     form_class = forms.SecretRoleForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('secrets:secretrole_list')
 
 
diff --git a/netbox/templates/utilities/confirmation_form.html b/netbox/templates/utilities/confirmation_form.html
index 8c97056da..16383d6f7 100644
--- a/netbox/templates/utilities/confirmation_form.html
+++ b/netbox/templates/utilities/confirmation_form.html
@@ -6,13 +6,13 @@
 	<div class="col-md-6 col-md-offset-3">
         <form action="." method="post" class="form">
         {% csrf_token %}
+        {% for field in form.hidden_fields %}
+            {{ field }}
+        {% endfor %}
             <div class="panel panel-{{ panel_class|default:"danger" }}">
                 <div class="panel-heading">{% block title %}{% endblock %}</div>
                 <div class="panel-body">
                     {% block message %}<p>Are you sure?</p>{% endblock %}
-                    {% for field in form.hidden_fields %}
-                        {{ field }}
-                    {% endfor %}
                     <div class="form-group">
                         <div class="checkbox{% if form.confirm.errors %} has-error{% endif %}">
                             <label for="{{ form.confirm.id_for_label }}">
diff --git a/netbox/tenancy/views.py b/netbox/tenancy/views.py
index f4bb05b69..587b20e72 100644
--- a/netbox/tenancy/views.py
+++ b/netbox/tenancy/views.py
@@ -29,7 +29,7 @@ class TenantGroupEditView(PermissionRequiredMixin, ObjectEditView):
     model = TenantGroup
     form_class = forms.TenantGroupForm
 
-    def get_return_url(self, obj):
+    def get_return_url(self, request, obj):
         return reverse('tenancy:tenantgroup_list')
 
 
@@ -81,7 +81,6 @@ class TenantEditView(PermissionRequiredMixin, ObjectEditView):
     permission_required = 'tenancy.change_tenant'
     model = Tenant
     form_class = forms.TenantForm
-    fields_initial = ['group']
     template_name = 'tenancy/tenant_edit.html'
     default_return_url = 'tenancy:tenant_list'
 
diff --git a/netbox/utilities/forms.py b/netbox/utilities/forms.py
index dd6235f45..f59aa6984 100644
--- a/netbox/utilities/forms.py
+++ b/netbox/utilities/forms.py
@@ -437,13 +437,18 @@ class BootstrapMixin(forms.BaseForm):
                 field.widget.attrs['placeholder'] = field.label
 
 
-class ConfirmationForm(BootstrapMixin, forms.Form):
+class ReturnURLForm(forms.Form):
     """
-    A generic confirmation form. The form is not valid unless the confirm field is checked. An optional return_url can
-    be specified to direct the user to a specific URL after the action has been taken.
+    Provides a hidden return URL field to control where the user is directed after the form is submitted.
+    """
+    return_url = forms.CharField(required=False, widget=forms.HiddenInput())
+
+
+class ConfirmationForm(BootstrapMixin, ReturnURLForm):
+    """
+    A generic confirmation form. The form is not valid unless the confirm field is checked.
     """
     confirm = forms.BooleanField(required=True)
-    return_url = forms.CharField(required=False, widget=forms.HiddenInput())
 
 
 class BulkEditForm(forms.Form):
diff --git a/netbox/utilities/views.py b/netbox/utilities/views.py
index 6f0895734..c80bd4aae 100644
--- a/netbox/utilities/views.py
+++ b/netbox/utilities/views.py
@@ -41,6 +41,23 @@ class CustomFieldQueryset:
             yield obj
 
 
+class GetReturnURLMixin(object):
+    """
+    Provides logic for determining where a user should be redirected after processing a form.
+    """
+    default_return_url = None
+
+    def get_return_url(self, request, obj):
+        query_param = request.GET.get('return_url')
+        if query_param and is_safe_url(url=query_param, host=request.get_host()):
+            return query_param
+        elif obj.pk and hasattr(obj, 'get_absolute_url'):
+            return obj.get_absolute_url()
+        elif self.default_return_url is not None:
+            return reverse(self.default_return_url)
+        return reverse('home')
+
+
 class ObjectListView(View):
     """
     List a series of objects.
@@ -130,21 +147,18 @@ class ObjectListView(View):
         return {}
 
 
-class ObjectEditView(View):
+class ObjectEditView(GetReturnURLMixin, View):
     """
     Create or edit a single object.
 
     model: The model of the object being edited
     form_class: The form used to create or edit the object
-    fields_initial: A set of fields that will be prepopulated in the form from the request parameters
     template_name: The name of the template
     default_return_url: The name of the URL used to display a list of this object type
     """
     model = None
     form_class = None
-    fields_initial = []
     template_name = 'utilities/obj_edit.html'
-    default_return_url = 'home'
 
     def get_object(self, kwargs):
         # Look up object by slug or PK. Return None if neither was provided.
@@ -159,24 +173,17 @@ class ObjectEditView(View):
         # given some parameter from the request URL.
         return obj
 
-    def get_return_url(self, obj):
-        # Determine where to redirect the user after updating an object (or aborting an update).
-        if obj.pk and hasattr(obj, 'get_absolute_url'):
-            return obj.get_absolute_url()
-        return reverse(self.default_return_url)
-
     def get(self, request, *args, **kwargs):
 
         obj = self.get_object(kwargs)
         obj = self.alter_obj(obj, request, args, kwargs)
-        initial_data = {k: request.GET[k] for k in self.fields_initial if k in request.GET}
-        form = self.form_class(instance=obj, initial=initial_data)
+        form = self.form_class(instance=obj, initial=request.GET)
 
         return render(request, self.template_name, {
             'obj': obj,
             'obj_type': self.model._meta.verbose_name,
             'form': form,
-            'return_url': self.get_return_url(obj),
+            'return_url': self.get_return_url(request, obj),
         })
 
     def post(self, request, *args, **kwargs):
@@ -207,17 +214,22 @@ class ObjectEditView(View):
 
             if '_addanother' in request.POST:
                 return redirect(request.path)
-            return redirect(self.get_return_url(obj))
+
+            return_url = form.cleaned_data.get('return_url')
+            if return_url is not None and is_safe_url(url=return_url, host=request.get_host()):
+                return redirect(return_url)
+            else:
+                return redirect(self.get_return_url(request, obj))
 
         return render(request, self.template_name, {
             'obj': obj,
             'obj_type': self.model._meta.verbose_name,
             'form': form,
-            'return_url': self.get_return_url(obj),
+            'return_url': self.get_return_url(request, obj),
         })
 
 
-class ObjectDeleteView(View):
+class ObjectDeleteView(GetReturnURLMixin, View):
     """
     Delete a single object.
 
@@ -227,7 +239,6 @@ class ObjectDeleteView(View):
     """
     model = None
     template_name = 'utilities/obj_delete.html'
-    default_return_url = 'home'
 
     def get_object(self, kwargs):
         # Look up object by slug if one has been provided. Otherwise, use PK.
@@ -236,24 +247,16 @@ class ObjectDeleteView(View):
         else:
             return get_object_or_404(self.model, pk=kwargs['pk'])
 
-    def get_return_url(self, obj):
-        if obj.pk and hasattr(obj, 'get_absolute_url'):
-            return obj.get_absolute_url()
-        return reverse(self.default_return_url)
-
     def get(self, request, **kwargs):
 
         obj = self.get_object(kwargs)
-        initial_data = {
-            'return_url': request.GET.get('return_url'),
-        }
-        form = ConfirmationForm(initial=initial_data)
+        form = ConfirmationForm(initial=request.GET)
 
         return render(request, self.template_name, {
             'obj': obj,
             'form': form,
             'obj_type': self.model._meta.verbose_name,
-            'return_url': request.GET.get('return_url') or self.get_return_url(obj),
+            'return_url': self.get_return_url(request, obj),
         })
 
     def post(self, request, **kwargs):
@@ -272,17 +275,17 @@ class ObjectDeleteView(View):
             messages.success(request, msg)
             UserAction.objects.log_delete(request.user, obj, msg)
 
-            return_url = form.cleaned_data['return_url']
-            if return_url and is_safe_url(url=return_url, host=request.get_host()):
+            return_url = form.cleaned_data.get('return_url')
+            if return_url is not None and is_safe_url(url=return_url, host=request.get_host()):
                 return redirect(return_url)
             else:
-                return redirect(self.get_return_url(obj))
+                return redirect(self.get_return_url(request, obj))
 
         return render(request, self.template_name, {
             'obj': obj,
             'form': form,
             'obj_type': self.model._meta.verbose_name,
-            'return_url': request.GET.get('return_url') or self.get_return_url(obj),
+            'return_url': self.get_return_url(request, obj),
         })