From 5f343617599dcf1fa2c889d97eb65a0983984be4 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Wed, 17 Apr 2019 00:32:30 +0100
Subject: [PATCH] ApiCSPReport: Log user ID instead of name, and limit urls to
 origin

These reports often contain false-positives from gadgets and
browser extensions that use a cross-domain requests for retreiving
information from a web API. (E.g. not for fetching executable JS
code, or for sending data elsewhere.)

Those API requests aren't static like "/foo.js?v2" but rather
dynamic, like /query/input+from+user, containing information about
what the user was reading, who or what they interacted with on
the wiki and/or text they entered or selected specifically.
(e.g. investigating user behaviour, counter-vandalism,
Google Translate tools, WHOIS gadgets, etc.)

Details of such action don't need to be recorded, and shown on
Logstash dashboards by default in the 'message' field. In fact,
I don't think it is needed for anything by default. If there's a
security problem, I imagine the origin suffices for a CSP block
and/or to start investigating.

Same for the user name. I don't want to see "[enwiki] John, referer
/wiki/Topic_read, chrome-extension/xyz, vandal-query.org/George".

These now log: "[enwiki] user_id 123, referer /wiki/Topic_read,
chrome-extension/xyz, vandal-query.org"

The user name still available when purposely investigating (via
public tools) by resolving the user ID.

Bug: T207900
Change-Id: Ic9855400c8cfedfa92b6659a4ad29c4dc28fb256
---
 includes/api/ApiCSPReport.php | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/includes/api/ApiCSPReport.php b/includes/api/ApiCSPReport.php
index 6271128171..79fe43d9b6 100644
--- a/includes/api/ApiCSPReport.php
+++ b/includes/api/ApiCSPReport.php
@@ -54,7 +54,7 @@ class ApiCSPReport extends ApiBase {
 			// XXX Is it ok to put untrusted data into log??
 			'csp-report' => $report,
 			'method' => __METHOD__,
-			'user' => $this->getUser()->getName(),
+			'user_id' => $this->getUser()->getId() || 'logged-out',
 			'user-agent' => $userAgent,
 			'source' => $this->getParameter( 'source' ),
 		] );
@@ -176,15 +176,32 @@ class ApiCSPReport extends ApiBase {
 			$flagText = '[' . implode( ', ', $flags ) . ']';
 		}
 
-		$blockedFile = $report['blocked-uri'] ?? 'n/a';
+		$blockedOrigin = isset( $report['blocked-uri'] )
+			? $this->originFromUrl( $report['blocked-uri'] )
+			: 'n/a';
 		$page = $report['document-uri'] ?? 'n/a';
-		$line = isset( $report['line-number'] ) ? ':' . $report['line-number'] : '';
+		$line = isset( $report['line-number'] )
+			? ':' . $report['line-number']
+			: '';
 		$warningText = $flagText .
-			' Received CSP report: <' . $blockedFile .
-			'> blocked from being loaded on <' . $page . '>' . $line;
+			' Received CSP report: <' . $blockedOrigin . '>' .
+			' blocked from being loaded on <' . $page . '>' . $line;
 		return $warningText;
 	}
 
+	/**
+	 * @param string $url
+	 * @return string
+	 */
+	private function originFromUrl( $url ) {
+		$bits = wfParseUrl( $url );
+		unset( $bits['user'], $bits['pass'], $bits['query'], $bits['fragment'] );
+		$bits['path'] = '';
+		$serverUrl = wfAssembleUrl( $bits );
+		// e.g. "https://example.org" from "https://example.org/foo/b?a#r"
+		return $serverUrl;
+	}
+
 	/**
 	 * Stop processing the request, and output/log an error
 	 *
-- 
2.20.1