Repository: PostHog/posthog
Pull request: Earlier IP anonymization
## Changes

- Under "Project" replace the checkbox "Anonymize client IP data" with "Discard client IP data", which better reflects what this checkbox does. "Anonymize" could also mean e.g. removing 8 random bits.
- Actually hides the IP before the task queues and plugins if the option is flipped, instead of just not storing it in the final ingestion stage
- Resolves https://github.com/PostHog/plugin-server/issues/119

This came out of the needs of a client who wants to use plugins to send events to BigQuery, yet found the IP stored even though they had the flag set.

Draft because:
- Requires corresponding changes to `plugin-server` and `plugin-scaffold`
- I want to test that nothing breaks

Misc question:
- Should we remove the check for `team.anonymize_ips` inside the process event functions after the queues?
  - Pros: gets rid of legacy code. 
  - Cons: there might be an edge case where an IP slips through when updating posthog for in-flight events

## Checklist

- [ ] All querysets/queries filter by Organization, by Team, and by User
- [ ] Django backend tests
- [ ] Jest frontend tests
- [ ] Cypress end-to-end tests


Problem statement:
Respect `team.anonymize_ips` earlier in the pipeline and hide IPs from plugins
We store the IP address of the user calling `posthog.capture` inside the `$ip` property. If this property exists, we use it. If not, we take the IP from the request and put it in the property.

When the event passes through plugins, we pass along the request's IP, all the way until ingestion. If a plugin wants to export the event to another database, it'll also have access to and send the request's IP, not the provided `$ip` property.

Solution: we should override the IP earlier in the pipeline, as to prevent storing PII that we don't want.

Hints from issue discussion:
Somewhat related to this I had a user yesterday who wanted to be able to not capture IPs at all, whereas if they don't set an IP themselves (like `0.0.0.0`) we end up setting it for them if they want it or not.
If `team.anonymize_ips` is set, we also shouldn't capture IPs. Is that not the case?
Ah right - I guess the setting has a bit of a misleading name.

Commit message: flip it around

Patch to review:
diff --git a/ee/clickhouse/process_event.py b/ee/clickhouse/process_event.py
index 77449e9623586..5d3fe5e00e72a 100644
--- a/ee/clickhouse/process_event.py
+++ b/ee/clickhouse/process_event.py
@@ -32,7 +32,7 @@
 def _capture_ee(
     event_uuid: UUID,
     person_uuid: UUID,
-    ip: str,
+    ip: Optional[str],
     site_url: str,
     team_id: int,
     event: str,
@@ -60,7 +60,7 @@ def _capture_ee(
 
     team = Team.objects.select_related("organization").get(pk=team_id)
 
-    if not team.anonymize_ips and "$ip" not in properties:
+    if ip and not team.anonymize_ips and "$ip" not in properties:
         properties["$ip"] = ip
 
     event = sanitize_event_name(event)
@@ -116,7 +116,7 @@ def handle_timestamp(data: dict, now: datetime.datetime, sent_at: Optional[datet
 
     def process_event_ee(
         distinct_id: str,
-        ip: str,
+        ip: Optional[str],
         site_url: str,
         data: dict,
         team_id: int,
@@ -165,7 +165,7 @@ def process_event_ee(
 
     def process_event_ee(
         distinct_id: str,
-        ip: str,
+        ip: Optional[str],
         site_url: str,
         data: dict,
         team_id: int,
@@ -179,7 +179,7 @@ def process_event_ee(
 
 def log_event(
     distinct_id: str,
-    ip: str,
+    ip: Optional[str],
     site_url: str,
     data: dict,
     team_id: int,
diff --git a/frontend/src/scenes/project/Settings/IPCapture.js b/frontend/src/scenes/project/Settings/IPCapture.js
index 04705bfa30bbc..1bebc64a76400 100644
--- a/frontend/src/scenes/project/Settings/IPCapture.js
+++ b/frontend/src/scenes/project/Settings/IPCapture.js
@@ -20,7 +20,7 @@ export function IPCapture() {
                     marginLeft: '10px',
                 }}
             >
-                Anonymize client IP data
+                Do not save client IP data
             </label>
         </div>
     )
diff --git a/posthog/api/capture.py b/posthog/api/capture.py
index af63eb869f157..96a3a1a7b251c 100644
--- a/posthog/api/capture.py
+++ b/posthog/api/capture.py
@@ -204,6 +204,7 @@ def get_event(request):
         _ensure_web_feature_flags_in_properties(event, team, distinct_id)
 
         event_uuid = UUIDT()
+        ip = None if team.anonymize_ips else get_ip_address(request)
 
         if is_ee_enabled():
             log_topics = [KAFKA_EVENTS_WAL]
@@ -214,7 +215,7 @@ def get_event(request):
 
             log_event(
                 distinct_id=distinct_id,
-                ip=get_ip_address(request),
+                ip=ip,
                 site_url=request.build_absolute_uri("/")[:-1],
                 data=event,
                 team_id=team.id,
@@ -228,7 +229,7 @@ def get_event(request):
             if not settings.PLUGIN_SERVER_INGESTION:
                 process_event_ee(
                     distinct_id=distinct_id,
-                    ip=get_ip_address(request),
+                    ip=ip,
                     site_url=request.build_absolute_uri("/")[:-1],
                     data=event,
                     team_id=team.id,
@@ -247,15 +248,7 @@ def get_event(request):
             celery_app.send_task(
                 name=task_name,
                 queue=celery_queue,
-                args=[
-                    distinct_id,
-                    get_ip_address(request),
-                    request.build_absolute_uri("/")[:-1],
-                    event,
-                    team.id,
-                    now.isoformat(),
-                    sent_at,
-                ],
+                args=[distinct_id, ip, request.build_absolute_uri("/")[:-1], event, team.id, now.isoformat(), sent_at,],
             )
     timer.stop("event_endpoint")
     return cors_response(request, JsonResponse({"status": 1}))
diff --git a/posthog/tasks/process_event.py b/posthog/tasks/process_event.py
index ded04f115b834..719393bed5b8b 100644
--- a/posthog/tasks/process_event.py
+++ b/posthog/tasks/process_event.py
@@ -102,7 +102,7 @@ def store_names_and_properties(team: Team, event: str, properties: Dict) -> None
 
 
 def _capture(
-    ip: str,
+    ip: Optional[str],
     site_url: str,
     team_id: int,
     event: str,
@@ -138,7 +138,7 @@ def _capture(
         "ingested_event",
     ).get(pk=team_id)
 
-    if not team.anonymize_ips and "$ip" not in properties:
+    if ip and not team.anonymize_ips and "$ip" not in properties:
         properties["$ip"] = ip
 
     event = sanitize_event_name(event)
@@ -259,7 +259,7 @@ def handle_identify_or_alias(event: str, properties: dict, distinct_id: str, tea
 
 @shared_task(name="posthog.tasks.process_event.process_event", ignore_result=True)
 def process_event(
-    distinct_id: str, ip: str, site_url: str, data: dict, team_id: int, now: str, sent_at: Optional[str],
+    distinct_id: str, ip: Optional[str], site_url: str, data: dict, team_id: int, now: str, sent_at: Optional[str],
 ) -> None:
     properties = data.get("properties", {})
     if data.get("$set"):


Code review comments:
diff_hunk: @@ -20,7 +20,7 @@ export function IPCapture() {
                     marginLeft: '10px',
                 }}
             >
-                Anonymize client IP data
+                Do not save client IP data
path: frontend/src/scenes/project/Settings/IPCapture.js
text: @user1:
How bout "Discard"?

@author:
Could work. "Discard client IP addresses" or "... IP data"?