launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/dj14-csrf into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/dj14-csrf/+merge/112549

This is another branch to fix MAAS to be compatible with Django 1.4 (the version Quantal) while keeping the compatibility with Django 1.3 (the version in Precise).

(The only error left when running the test suite with Django 1.4 is related to the API documentation so it's no really critical)

It fixes 3 things:

- The way Django implements csrf protection has changed in Django 1.4 (see [0]).  I've changed the code to use the new way (explicitly create the token) which is also supported by Django 1.3.  When we started with MAAS, we deliberately choose to use the old (deprecated) way to do that because it was more simple.  The reasoning behind the change in Django is that you should explicitly which form uses the csrf protection mecanism in order to avoid the token to be included in a form which will submit to an external site.

- Django's UserChangeForm form, which we use as the base class for EditUserForm (src/maasserver/forms.py), in Django 1.4 (see [1]), defines a custom field 'password' to override the default field automatically created (because that form is a "ModelForm" base on the model class user.   We use that as a base class for the form in MAAS which allows to edit a user (name, email, superuser) and we've excluded the field 'password' by not including it in the list of fields taken from the base model class (see Meta.fields).  But now that the base class overrides the field 'password', we need to get rid of it dynamically.

- The call to syncdb now *must* specify which of the databases if the target database (this is because Django 1.4 added support form multiple databases).

[0] https://docs.djangoproject.com/en/dev/ref/contrib/csrf/
[1] https://code.djangoproject.com/attachment/ticket/16845/mask-password-field.diff
-- 
https://code.launchpad.net/~rvb/maas/dj14-csrf/+merge/112549
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/dj14-csrf into lp:maas.

=== modified file 'src/maas/settings.py'
--- src/maas/settings.py	2012-06-26 13:22:11 +0000
+++ src/maas/settings.py	2012-06-28 12:15:23 +0000
@@ -233,7 +233,6 @@
     'metadataserver.middleware.MetadataErrorsMiddleware',
     'django.middleware.transaction.TransactionMiddleware',
     'django.middleware.csrf.CsrfViewMiddleware',
-    'django.middleware.csrf.CsrfResponseMiddleware',
     'maasserver.middleware.ExceptionLoggerMiddleware',
     'django.contrib.auth.middleware.AuthenticationMiddleware',
     'django.contrib.messages.middleware.MessageMiddleware',

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-06-11 12:56:58 +0000
+++ src/maasserver/forms.py	2012-06-28 12:15:23 +0000
@@ -448,6 +448,14 @@
         fields = (
             'username', 'last_name', 'email', 'is_superuser')
 
+    def __init__(self, *args, **kwargs):
+        super(EditUserForm, self).__init__(*args, **kwargs)
+        # Django 1.4 overrides the field 'password' thus adding it
+        # defacto to the list of the selected fields (Meta.fields).
+        # Here we don't want to use this form to edit the password.
+        if 'password' in self.fields:
+            del self.fields['password']
+
 
 class ConfigForm(Form):
     """A base class for forms that save the content of their fields into

=== modified file 'src/maasserver/templates/maasserver/mac_confirm_delete.html'
--- src/maasserver/templates/maasserver/mac_confirm_delete.html	2012-04-27 08:11:29 +0000
+++ src/maasserver/templates/maasserver/mac_confirm_delete.html	2012-06-28 12:15:23 +0000
@@ -11,7 +11,7 @@
         </h2>
         <p>This action is permanent and can not be undone.</p>
         <p>
-          <form action="." method="post">
+          <form action="." method="post">{% csrf_token %}
             <input type="hidden" name="post" value="yes" />
             <input type="submit" value="Delete MAC address" class="right" />
             <a href="{% url 'node-edit' mac_to_delete.node.system_id %}">Cancel</a>

=== modified file 'src/maasserver/templates/maasserver/node_add_mac.html'
--- src/maasserver/templates/maasserver/node_add_mac.html	2012-04-27 08:11:29 +0000
+++ src/maasserver/templates/maasserver/node_add_mac.html	2012-06-28 12:15:23 +0000
@@ -6,6 +6,7 @@
 
 {% block content %}
   <form action="." method="post" class="block auto-width">
+    {% csrf_token %}
     <ul>
     {% for field in form %}
       {% include "maasserver/form_field.html" %}

=== modified file 'src/maasserver/templates/maasserver/node_confirm_delete.html'
--- src/maasserver/templates/maasserver/node_confirm_delete.html	2012-04-03 14:05:49 +0000
+++ src/maasserver/templates/maasserver/node_confirm_delete.html	2012-06-28 12:15:23 +0000
@@ -13,7 +13,7 @@
         </h2>
         <p>This action is permanent and can not be undone.</p>
         <p>
-          <form action="." method="post">
+          <form action="." method="post">{% csrf_token %}
             <input type="hidden" name="post" value="yes" />
             <input type="submit" value="Delete node" class="right" />
             <a href="{% url 'node-view' node_to_delete.system_id %}">Cancel</a>

=== modified file 'src/maasserver/templates/maasserver/node_edit.html'
--- src/maasserver/templates/maasserver/node_edit.html	2012-06-11 12:25:37 +0000
+++ src/maasserver/templates/maasserver/node_edit.html	2012-06-28 12:15:23 +0000
@@ -31,6 +31,7 @@
 
 {% block content %}
   <form action="." method="post" class="block auto-width">
+    {% csrf_token %}
     <ul>
       {% for field in form %}
         {% include "maasserver/form_field.html" %}

=== modified file 'src/maasserver/templates/maasserver/node_view.html'
--- src/maasserver/templates/maasserver/node_view.html	2012-06-22 07:50:12 +0000
+++ src/maasserver/templates/maasserver/node_view.html	2012-06-28 12:15:23 +0000
@@ -17,7 +17,7 @@
     </a>
   {% if form.action_buttons %}
     <h4>Actions</h4>
-    <form id="node_actions" method="post" action=".">
+    <form id="node_actions" method="post" action=".">{% csrf_token %}
       {% for action in form.action_buttons %}
         <input
           class="secondary

=== modified file 'src/maasserver/templates/maasserver/prefs.html'
--- src/maasserver/templates/maasserver/prefs.html	2012-04-19 09:13:31 +0000
+++ src/maasserver/templates/maasserver/prefs.html	2012-06-28 12:15:23 +0000
@@ -62,7 +62,7 @@
     <div class="clear"></div>
     <div id="profile" class="block size7 first">
       <h2>User details</h2>
-      <form action="." method="post">
+      <form action="." method="post">{% csrf_token %}
         <ul>
           {% for field in profile_form %}
             {% include "maasserver/form_field.html" %}
@@ -74,7 +74,7 @@
     </div>
     <div id="password" class="block size7 first">
       <h2>Password</h2>
-      <form action="." method="post">
+      <form action="." method="post">{% csrf_token %}
         <ul>
           {% for field in password_form %}
             {% include "maasserver/form_field.html" %}

=== modified file 'src/maasserver/templates/maasserver/prefs_add_sshkey.html'
--- src/maasserver/templates/maasserver/prefs_add_sshkey.html	2012-04-05 06:23:59 +0000
+++ src/maasserver/templates/maasserver/prefs_add_sshkey.html	2012-06-28 12:15:23 +0000
@@ -5,6 +5,7 @@
 
 {% block content %}
   <form action="." method="post" class="block auto-width">
+    {% csrf_token %}
     <ul>
     {% for field in form %}
       {% include "maasserver/form_field.html" %}

=== modified file 'src/maasserver/templates/maasserver/prefs_confirm_delete_sshkey.html'
--- src/maasserver/templates/maasserver/prefs_confirm_delete_sshkey.html	2012-03-28 16:00:26 +0000
+++ src/maasserver/templates/maasserver/prefs_confirm_delete_sshkey.html	2012-06-28 12:15:23 +0000
@@ -11,7 +11,7 @@
 	<p style="word-wrap: break-word; width: 700px;">{{ key }}</p>
         <p>This action is permanent and can not be undone.</p>
         <p>
-          <form action="." method="post">
+          <form action="." method="post">{% csrf_token %}
             <input type="hidden" name="post" value="yes" />
             <input type="submit" value="Delete key" class="right" />
             <a href="{% url 'prefs' %}">Cancel</a>

=== modified file 'src/maasserver/templates/maasserver/settings.html'
--- src/maasserver/templates/maasserver/settings.html	2012-04-23 05:57:31 +0000
+++ src/maasserver/templates/maasserver/settings.html	2012-06-28 12:15:23 +0000
@@ -54,6 +54,7 @@
                 </a>
                 <form method="POST"
                       action="{% url 'accounts-del' user_item.username %}">
+                  {% csrf_token %}
                   <input type="hidden" name="username"
                          value="{{ user_item.username }}" />
                 </form>
@@ -71,6 +72,7 @@
     <div id="commissioning" class="block size7 first">
       <h2>Commissioning</h2>
       <form action="{% url "settings" %}" method="post">
+        {% csrf_token %}
         <ul>
         {% for field in commissioning_form %}
           {% include "maasserver/form_field.html" %}
@@ -83,6 +85,7 @@
     <div id="ubuntu" class="block size7 first">
       <h2>Ubuntu</h2>
       <form action="{% url "settings" %}" method="post">
+        {% csrf_token %}
         <ul>
         {% with field=ubuntu_form.update_from %}
           {% include "maasserver/form_field.html" %}
@@ -112,6 +115,7 @@
     <div id="maas_and_network" class="block size7 first">
       <h2>Network Configuration</h2>
       <form action="{% url "settings" %}" method="post">
+        {% csrf_token %}
         <ul>
         {% for field in maas_and_network_form %}
           {% include "maasserver/form_field.html" %}

=== modified file 'src/maasserver/templates/maasserver/settings_add_archive.html'
--- src/maasserver/templates/maasserver/settings_add_archive.html	2012-04-16 05:59:24 +0000
+++ src/maasserver/templates/maasserver/settings_add_archive.html	2012-06-28 12:15:23 +0000
@@ -6,6 +6,7 @@
 
 {% block content %}
   <form action="." method="post" class="block auto-width">
+    {% csrf_token %}
     <ul>
     {% for field in form %}
       {% include "maasserver/form_field.html" %}

=== modified file 'src/maasserver/templates/maasserver/user_add.html'
--- src/maasserver/templates/maasserver/user_add.html	2012-03-05 05:52:32 +0000
+++ src/maasserver/templates/maasserver/user_add.html	2012-06-28 12:15:23 +0000
@@ -6,6 +6,7 @@
 
 {% block content %}
   <form action="." method="post" class="block auto-width first">
+    {% csrf_token %}
     <ul>
     {% for field in form %}
       {% include "maasserver/form_field.html" %}

=== modified file 'src/maasserver/templates/maasserver/user_confirm_delete.html'
--- src/maasserver/templates/maasserver/user_confirm_delete.html	2012-03-02 06:06:51 +0000
+++ src/maasserver/templates/maasserver/user_confirm_delete.html	2012-06-28 12:15:23 +0000
@@ -11,7 +11,7 @@
         </h2>
         <p>This action is permanent and can not be undone.</p>
         <p>
-          <form action="." method="post">
+          <form action="." method="post">{% csrf_token %}
             <input type="hidden" name="post" value="yes" />
             <input type="submit" value="Delete user" class="right" />
             <a href="{% url 'settings' %}">Cancel</a>

=== modified file 'src/maasserver/templates/maasserver/user_edit.html'
--- src/maasserver/templates/maasserver/user_edit.html	2012-04-10 14:32:51 +0000
+++ src/maasserver/templates/maasserver/user_edit.html	2012-06-28 12:15:23 +0000
@@ -9,6 +9,7 @@
   <h2>Settings</h2>
   <div id="profile" class="size7">
     <form action="." method="post" class="block">
+      {% csrf_token %}
       <ul>
       {% for field in profile_form %}
 	{% include "maasserver/form_field.html" %}
@@ -23,6 +24,7 @@
   <h2>Change password</h2>
   <div id="password" class="size7">
     <form action="." method="post" class="block">
+      {% csrf_token %}
       <ul>
       {% for field in password_form %}
 	{% include "maasserver/form_field.html" %}

=== modified file 'src/maasserver/templates/registration/login.html'
--- src/maasserver/templates/registration/login.html	2012-03-22 06:38:06 +0000
+++ src/maasserver/templates/registration/login.html	2012-06-28 12:15:23 +0000
@@ -31,7 +31,7 @@
     <p class="form-errors">Your username and password didn't match. Please try again.</p>
   {% endif %}
 
-  <form method="post" action="{% url 'login' %}">
+  <form method="post" action="{% url 'login' %}">{% csrf_token %}
     <input type="hidden" name="next" value="{{ next }}" />
     <ul>
         {% for field in form %}

=== modified file 'src/maastesting/djangotestcase.py'
--- src/maastesting/djangotestcase.py	2012-06-28 07:05:11 +0000
+++ src/maastesting/djangotestcase.py	2012-06-28 12:15:23 +0000
@@ -84,7 +84,8 @@
         settings.INSTALLED_APPS.append(self.app)
         loading.cache.loaded = False
         # Use Django's 'syncdb' rather than South's.
-        syncdb.Command().handle_noargs(verbosity=0, interactive=False)
+        syncdb.Command().handle_noargs(
+            verbosity=0, interactive=False, database=DEFAULT_DB_ALIAS)
         super(TestModelMixin, self)._pre_setup()
 
     def _post_teardown(self):