Closed Bug 1110904 Opened 11 years ago Closed 11 years ago

Log IP addresses for each edit

Categories

(developer.mozilla.org Graveyard :: Editing, enhancement)

Product:
developer.mozilla.org Graveyard
Component:
Editing
Platform:
All
Other
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: sheppy, Unassigned)

References

Details

(Whiteboard: [specification][type:feature])

What problems would this solve? =============================== In order to enable us to better control spam, we need to be able to ban IP addresses. To do that, we have to know what IP addresses to ban. Who would use this? =================== Administrators and webops personnel. What would users see? ===================== Admins should be able to see when viewing revision history the IP address of the contributor. What would users do? What would happen as a result? =================================================== The IP should appear in the list of revisions when viewing revision history. Is there anything else we should know? ====================================== (We probably will need other tools but this is a start; recording the info is a needed first step. Other stuff we would likely want is a way to search to find all the edits done by a given IP)
Per threads with legal, it looks like we will: 1. Update TOS links in the footer and account creation page (bug 1110989 and bug 1111002) 2. Put this feature behind a waffle flag 3. Only retain IP addresses for a maximum of 30 days
Depends on: 1110989, 1111002
cc'ing :jezdez to keep him in the loop. If the spam spike continues I may start working on this ASAP.
Sounds half sensible, two question areas come to mind though: - Do we even have access to the original IP of the user behind the loadbalancer? How will we handle proxies, VPNs, office gateways and other situation of shared internet connection? - How will purging the IP addresses after 30 days work? By mass changing revisions? If yes, I'd like to strongly suggest to *not* use Revision.save() as that actually saves the document as well and will trigger a happy dependency chain with the rendering engine. A DOS waiting to happen. Instead it *must* use update() to set the IP field to None (or whatever null value). If no, there should be a separate IPLogEntry model that has a OneToOneField with the revision. Then use the on_delete parameter as on_delete=models.SET_NULL that sets the value to null automatically when a IPLogEntry instance is deleted. The IPLogEntry model should therefor have a easy to query datetime field, e.g. timestamp. I prefer the later.
Thanks for the input :jezdez. The spam attack seems to have stalled for now at least. I wrote the code for the blocker bugs here, but haven't started on this quite yet ... 1. We can start with the first IP in X-Forwarded-For to get the originating IP from well-behaved intermediate hosts. It's still vulnerable to IP spoofing, [1] which would be a more sophisticated spammer. If we find that's the case, we can improve the code with trusted proxies and/or valid IP matching like Rails does. [2] If it's still an issue, I'd want to escalate to WebAppSec and/or OpSec. 2. I was thinking of adding an IP field to the Revision model, but I like your IPLogEntry model much better. So, I haven't started yet, but with this approach I'm ready to start when/if we need to. [1] http://esd.io/blog/flask-apps-heroku-real-ip-spoofing.html [2] https://github.com/rails/rails/blob/0c7a283140e750800a9479f6b1736ca1f7005948/actionpack/lib/action_dispatch/middleware/remote_ip.rb
Sent https://github.com/mozilla/kuma/pull/2960 ... still need to create the management command that will clean out all IP addresses that are 30 days old.
Severity: normal → enhancement
Component: General → Editing
Depends on: 1113818
All blockers are fixed. We can now do this via the django admin site and the 'store_revision_ips' waffle switch.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Product: developer.mozilla.org → developer.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.