Merge lp:~widelands-dev/widelands-website/cleanup_ip_address_field into lp:widelands-website

[kaputtnik]

Remove the use of an IPAddressField from wiki, wlimages and pybb

As far i can see the IPAddressField was stored without a specific reason, except allowing to post anonymously, which we don't allow at all.

This branch removes that field from wiki, wlimages and pybb.

Removing this field from threadedcomments will be done in a different branch, because the changes are too big for this branch.

[GunChleoc (gunchleoc)]

2 comments that might or might not need attention.

[kaputtnik (franku)]

Thanks for the review :-)

The classes are just used to show collapsed sections. They show up as "More options (Show)" like here: https://wl.widelands.org/admin/news/post/188/change/

Sorry for the remaining print statement. I'll remove it and merge this then.

[lp:~widelands-dev/widelands-website/cleanup_ip_address_field]

505. By kaputtnik on 2018-10-03

removed superfluous print statement

[kaputtnik (franku)]

The changes are applied, but not committed yet. Will do it this evening...

=== modified file 'pybb/admin.py'
--- pybb/admin.py	2017-04-29 19:52:28 +0000
+++ pybb/admin.py	2018-10-03 20:25:29 +0000
@@ -84,7 +84,7 @@
         ),
         (_('Additional options'), {
             'classes': ('collapse',),
-            'fields': (('created', 'updated'), 'user_ip')
+            'fields': (('created', 'updated'),)
         }
         ),
         (_('Message'), {

=== modified file 'pybb/forms.py'
--- pybb/forms.py	2018-04-07 09:48:02 +0000
+++ pybb/forms.py	2018-10-03 20:25:29 +0000
@@ -25,7 +25,6 @@
         self.user = kwargs.pop('user', None)
         self.topic = kwargs.pop('topic', None)
         self.forum = kwargs.pop('forum', None)
-        self.ip = kwargs.pop('ip', None)
         super(AddPostForm, self).__init__(*args, **kwargs)
 
         if self.topic:
@@ -54,7 +53,7 @@
             topic_is_new = False
             topic = self.topic
 
-        post = Post(topic=topic, user=self.user, user_ip=self.ip,
+        post = Post(topic=topic, user=self.user,
                     markup=self.cleaned_data['markup'],
                     body=self.cleaned_data['body'])
         post.save(*args, **kwargs)

=== added file 'pybb/migrations/0003_remove_post_user_ip.py'
--- pybb/migrations/0003_remove_post_user_ip.py	1970-01-01 00:00:00 +0000
+++ pybb/migrations/0003_remove_post_user_ip.py	2018-10-03 20:25:29 +0000
@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.12 on 2018-10-02 19:31
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('pybb', '0002_auto_20161001_2046'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='post',
+            name='user_ip',
+        ),
+    ]

=== modified file 'pybb/models.py'
--- pybb/models.py	2018-04-08 14:40:17 +0000
+++ pybb/models.py	2018-10-03 20:25:29 +0000
@@ -203,7 +203,6 @@
     body = models.TextField(_('Message'))
     body_html = models.TextField(_('HTML version'))
     body_text = models.TextField(_('Text version'))
-    user_ip = models.GenericIPAddressField(_('User IP'), default='')
     hidden = models.BooleanField(_('Hidden'), blank=True, default=False)
 
     class Meta:

=== modified file 'pybb/views.py'
--- pybb/views.py	2018-04-08 14:40:17 +0000
+++ pybb/views.py	2018-10-03 20:25:29 +0000
@@ -20,7 +20,6 @@
 from pybb import settings as pybb_settings
 from pybb.orm import load_related
 
-from wl_utils import get_real_ip
 from check_input.models import SuspiciousInput
 
 try:
@@ -154,7 +153,7 @@
         quote = quote_text(post.body, post.user, 'markdown')
 
     form = build_form(AddPostForm, request, topic=topic, forum=forum,
-                      user=request.user, ip=get_real_ip(request),
+                      user=request.user,
                       initial={'markup': 'markdown', 'body': quote})
 
     if form.is_valid():

=== modified file 'templates/wiki/edit.html'
--- templates/wiki/edit.html	2017-01-15 12:03:04 +0000
+++ templates/wiki/edit.html	2018-10-03 20:25:29 +0000
@@ -77,7 +77,6 @@
 		{{ form.content_type }}
 		{{ form.object_id }}
 		{{ form.action }}
-		{{ form.user_ip }}
 
 		<div>
 			<input id="id_preview" type="button" value="{% trans "Preview" %}" />

=== modified file 'templates/wiki/history.html'
--- templates/wiki/history.html	2016-04-26 16:10:04 +0000
+++ templates/wiki/history.html	2018-10-03 20:25:29 +0000
@@ -99,11 +99,7 @@
 					{{ change.modified|custom_date:user }}
 				</td>
 				<td>
-				{% if change.is_anonymous_change %}
-					{{ change.editor_ip }}
-				{% else %}
 					{{ change.editor|user_link }}
-				{% endif %}
 				</td>
 				<td class="italic">
 					{% if change.comment %}'{{ change.comment }}'{% endif %}

=== modified file 'wiki/admin.py'
--- wiki/admin.py	2017-12-03 11:43:47 +0000
+++ wiki/admin.py	2018-10-03 20:25:29 +0000
@@ -21,11 +21,7 @@
     list_filter = ('title',)
     ordering = ['-last_update']
     fieldsets = (
-        (None, {'fields': ('title', 'content', 'markup')}),
-        ('Creator', {'fields': ('creator', 'creator_ip'),
-                     'classes': ('collapse', 'wide')}),
-        # ('Group', {'fields': ('object_id', 'content_type'),
-        #              'classes': ('collapse', 'wide')}),
+        (None, {'fields': ('title', 'content', 'creator', 'markup')}),
     )
     raw_id_fields = ('creator',)
     inlines = [InlineImages]
@@ -41,13 +37,11 @@
     list_filter = ('article__title',)
     ordering = ('-modified',)
     fieldsets = (
-        ('Article', {'fields': ('article',)}),
+        ('Article', {'fields': ('article', 'editor')}),
         ('Differences', {'fields': ('old_title', 'old_markup',
                                     'content_diff')}),
         ('Other', {'fields': ('comment', 'modified', 'revision', 'reverted'),
                    'classes': ('collapse', 'wide')}),
-        ('Editor', {'fields': ('editor', 'editor_ip'),
-                    'classes': ('collapse', 'wide')}),
     )
     raw_id_fields = ('editor',)
 

=== modified file 'wiki/forms.py'
--- wiki/forms.py	2017-04-29 19:56:32 +0000
+++ wiki/forms.py	2018-10-03 20:25:29 +0000
@@ -22,7 +22,6 @@
     summary = forms.CharField(widget=forms.Textarea)
 
     comment = forms.CharField(required=False)
-    user_ip = forms.CharField(widget=forms.HiddenInput)
 
     content_type = forms.ModelChoiceField(
         queryset=ContentType.objects.all(),
@@ -35,8 +34,7 @@
 
     class Meta:
         model = Article
-        exclude = ('creator', 'creator_ip',
-                   'group', 'created_at', 'last_update')
+        exclude = ('creator', 'group', 'created_at', 'last_update')
 
     def clean_title(self):
         """Check for some errors regarding the title:
@@ -92,7 +90,6 @@
 
     def save(self, *args, **kwargs):
         # 0 - Extra data
-        editor_ip = self.cleaned_data['user_ip']
         comment = self.cleaned_data['comment']
 
         # 2 - Save the Article
@@ -102,7 +99,6 @@
         editor = getattr(self, 'editor', None)
         group = getattr(self, 'group', None)
         if self.is_new:
-            article.creator_ip = editor_ip
             if editor is not None:
                 article.creator = editor
                 article.group = group
@@ -113,6 +109,6 @@
         # 4 - Create new revision
         changeset = article.new_revision(
             self.old_content, self.old_title, self.old_markup,
-            comment, editor_ip, editor)
+            comment, editor)
 
         return article, changeset

=== added file 'wiki/migrations/0003_auto_20180918_0836.py'
--- wiki/migrations/0003_auto_20180918_0836.py	1970-01-01 00:00:00 +0000
+++ wiki/migrations/0003_auto_20180918_0836.py	2018-10-03 20:25:29 +0000
@@ -0,0 +1,27 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.12 on 2018-09-18 08:36
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('wiki', '0002_auto_20161218_1056'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='article',
+            options={'default_permissions': ('change', 'add'), 'ordering': ['title'], 'verbose_name': 'Article', 'verbose_name_plural': 'Articles'},
+        ),
+        migrations.RemoveField(
+            model_name='article',
+            name='creator_ip',
+        ),
+        migrations.RemoveField(
+            model_name='changeset',
+            name='editor_ip',
+        ),
+    ]

=== modified file 'wiki/models.py'
--- wiki/models.py	2018-05-02 06:33:13 +0000
+++ wiki/models.py	2018-10-03 20:25:29 +0000
@@ -53,8 +53,6 @@
                               null=True, blank=True)
     creator = models.ForeignKey(User, verbose_name=_('Article Creator'),
                                 null=True)
-    creator_ip = models.GenericIPAddressField(_('IP Address of the Article Creator'),
-                                              blank=True, null=True)
     created_at = models.DateTimeField(default=datetime.now)
     last_update = models.DateTimeField(blank=True, null=True)
 
@@ -93,7 +91,7 @@
         return self.images.all()
 
     def new_revision(self, old_content, old_title, old_markup,
-                     comment, editor_ip, editor):
+                     comment, editor):
         """Create a new ChangeSet with the old content."""
 
         content_diff = diff(self.content, old_content)
@@ -101,7 +99,6 @@
         cs = ChangeSet.objects.create(
             article=self,
             comment=comment,
-            editor_ip=editor_ip,
             editor=editor,
             old_title=old_title,
             old_markup=old_markup,
@@ -109,10 +106,10 @@
 
         return cs
 
-    def revert_to(self, revision, editor_ip, editor=None):
+    def revert_to(self, revision, editor=None):
         """Revert the article to a previuos state, by revision number."""
         changeset = self.changeset_set.get(revision=revision)
-        changeset.reapply(editor_ip, editor)
+        changeset.reapply(editor)
 
     def compare(self, from_revision, to_revision):
         """Compares to revisions of this article."""
@@ -139,10 +136,9 @@
 
     article = models.ForeignKey(Article, verbose_name=_(u"Article"))
 
-    # Editor identification -- logged or anonymous
+    # Editor identification -- logged
     editor = models.ForeignKey(User, verbose_name=_(u'Editor'),
                                null=True)
-    editor_ip = models.GenericIPAddressField(_(u"IP Address of the Editor"))
 
     # Revision number, starting from 1
     revision = models.IntegerField(_(u"Revision Number"))
@@ -185,7 +181,7 @@
     def is_anonymous_change(self):
         return self.editor is None
 
-    def reapply(self, editor_ip, editor):
+    def reapply(self, editor):
         """Return the Article to this revision."""
 
         # XXX Would be better to exclude reverted revisions
@@ -218,7 +214,8 @@
             old_content=old_content, old_title=old_title,
             old_markup=old_markup,
             comment='Reverted to revision #%s' % self.revision,
-            editor_ip=editor_ip, editor=editor)
+            editor=editor
+            )
 
         self.save()
 

=== modified file 'wiki/views.py'
--- wiki/views.py	2018-04-08 14:40:17 +0000
+++ wiki/views.py	2018-10-03 20:25:29 +0000
@@ -300,15 +300,13 @@
             return redirect(new_article)
 
     elif request.method == 'GET':
-        user_ip = get_real_ip(request)
 
         # TODO(Franku): Never worked IMHO
         # lock = cache.get(title, None)
         # if lock is None:
         #     lock = ArticleEditLock(title, request)
         # lock.create_message(request)
-
-        initial = {'user_ip': user_ip}
+        initial = {}
         if group_slug is not None:
             initial.update({'content_type': group_ct.id,
                             'object_id': group.id})
@@ -493,9 +491,9 @@
         except Article.DoesNotExist:
             # No existing article found -> reverting possible
             if request.user.is_authenticated:
-                article.revert_to(revision, get_real_ip(request), request.user)
+                article.revert_to(revision, request.user)
             else:
-                article.revert_to(revision, get_real_ip(request))
+                article.revert_to(revision)
             return redirect(article)
         # An article with this name exists
         messages.error(

=== modified file 'wlimages/admin.py'
--- wlimages/admin.py	2016-12-13 18:28:51 +0000
+++ wlimages/admin.py	2018-10-03 20:25:29 +0000
@@ -31,7 +31,7 @@
     actions = [delete_with_file]
     fieldsets = (
         (None, {'fields': ('image', 'name', 'date_submitted', 'revision')}),
-        (_('Upload data:'), {'fields': ('user', 'editor_ip')}),
+        (_('Uploaded by:'), {'fields': ('user',)}),
         (_('Content object:'), {
          'fields': (('content_type', 'content_object'), 'object_id')}),
     )

=== added file 'wlimages/migrations/0003_remove_image_editor_ip.py'
--- wlimages/migrations/0003_remove_image_editor_ip.py	1970-01-01 00:00:00 +0000
+++ wlimages/migrations/0003_remove_image_editor_ip.py	2018-10-03 20:25:29 +0000
@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.12 on 2018-09-18 09:01
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('wlimages', '0002_remove_image_url'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='image',
+            name='editor_ip',
+        ),
+    ]

=== modified file 'wlimages/models.py'
--- wlimages/models.py	2016-12-13 18:28:51 +0000
+++ wlimages/models.py	2018-10-03 20:25:29 +0000
@@ -24,6 +24,7 @@
     def create(self, **keyw):
         """Makes sure that no image/revision pair is already in the
         database."""
+
         if 'name' not in keyw or 'revision' not in keyw:
             raise IntegrityError('needs name and revision as keywords')
 
@@ -32,12 +33,12 @@
 
         return super(ImageManager, self).create(**keyw)
 
-    def create_and_save_image(self, user, image, content_type, object_id, ip):
+    def create_and_save_image(self, user, image, content_type, object_id):
         # Use Django's get_valid_name() to get a safe filename
         storage = FileSystemStorage()
         safe_filename = storage.get_valid_name(image.name)
         im = self.create(content_type=content_type, object_id=object_id,
-                         user=user, revision=1, name=image.name, editor_ip=ip)
+                         user=user, revision=1, name=image.name)
         path = '%swlimages/%s' % (MEDIA_ROOT, safe_filename)
 
         destination = open(path, 'wb')
@@ -69,8 +70,6 @@
 
     # User Field
     user = models.ForeignKey(User)
-    editor_ip = models.GenericIPAddressField(
-        _('IP address'), null=True, blank=True)
 
     # Date Fields
     date_submitted = models.DateTimeField(

=== modified file 'wlimages/views.py'
--- wlimages/views.py	2018-04-03 05:18:03 +0000
+++ wlimages/views.py	2018-10-03 20:25:29 +0000
@@ -4,7 +4,6 @@
 from django.shortcuts import get_object_or_404, render
 
 from models import Image
-from wl_utils import get_real_ip
 from forms import UploadImageForm
 
 
@@ -31,7 +30,7 @@
         form = UploadImageForm(request.POST, request.FILES)
         if form.is_valid():  # All validation rules pass
             Image.objects.create_and_save_image(user=request.user, image=request.FILES['imagename'],
-                                                content_type=ContentType.objects.get(pk=content_type), object_id=object_id, ip=get_real_ip(request))
+                                                content_type=ContentType.objects.get(pk=content_type), object_id=object_id)
             return HttpResponseRedirect(next)  # Redirect after POST
     else:
         form = UploadImageForm()  # An unbound form