Audit: track downloads separately from page views

Author: stsewd

readthedocs/audit/models.py

@@ -22,10 +22,15 @@ def new(self, action, user=None, request=None, **kwargs):
         other fields will be auto-populated from that information.
         """
 
-        actions_requiring_user = (AuditLog.PAGEVIEW, AuditLog.AUTHN, AuditLog.LOGOUT)
+        actions_requiring_user = (
+            AuditLog.PAGEVIEW,
+            AuditLog.DOWNLOAD,
+            AuditLog.AUTHN,
+            AuditLog.LOGOUT,
+        )
         if action in actions_requiring_user and (not user or not request):
             raise TypeError(f'A user and a request is required for the {action} action.')
-        if action == AuditLog.PAGEVIEW and 'project' not in kwargs:
+        if action in (AuditLog.PAGEVIEW, AuditLog.DOWNLOAD) and 'project' not in kwargs:
             raise TypeError(f'A project is required for the {action} action.')
 
         # Don't save anonymous users.
@@ -62,12 +67,14 @@ class AuditLog(TimeStampedModel):
     """
 
     PAGEVIEW = 'pageview'
+    DOWNLOAD = 'download'
     AUTHN = 'authentication'
     AUTHN_FAILURE = 'authentication-failure'
     LOGOUT = 'log-out'
 
     CHOICES = (
         (PAGEVIEW, 'Page view'),
+        (DOWNLOAD, 'Download'),
         (AUTHN, 'Authentication'),
         (AUTHN_FAILURE, 'Authentication failure'),
         (LOGOUT, 'Log out'),

readthedocs/proxito/tests/test_full.py

@@ -335,6 +335,29 @@ def test_track_html_files_only(self, is_audit_enabled):
         self.assertIn('x-accel-redirect', resp)
         self.assertEqual(AuditLog.objects.all().count(), 1)
 
+    @mock.patch.object(ServeDocsMixin, '_is_audit_enabled')
+    def test_track_downloads(self, is_audit_enabled):
+        is_audit_enabled.return_value = True
+
+        self.project.versions.update(
+            has_pdf=True,
+            has_epub=True,
+            has_htmlzip=True,
+        )
+
+        self.assertEqual(AuditLog.objects.all().count(), 0)
+        url = '/_/downloads/en/latest/pdf/'
+        host = 'project.dev.readthedocs.io'
+        resp = self.client.get(url, HTTP_HOST=host)
+        self.assertIn('x-accel-redirect', resp)
+        self.assertEqual(AuditLog.objects.all().count(), 1)
+
+        log = AuditLog.objects.last()
+        self.assertEqual(log.user, None)
+        self.assertEqual(log.project, self.project)
+        self.assertEqual(log.resource, url)
+        self.assertEqual(log.action, AuditLog.DOWNLOAD)
+
 
 @override_settings(
     PYTHON_MEDIA=False,

readthedocs/proxito/views/mixins.py

@@ -54,7 +54,12 @@ def _serve_docs(
         or "docs-celeryproject-org-kombu-en-stable.pdf")
         """
 
-        self._track_pageview(final_project, path, request)
+        self._track_pageview(
+            project=final_project,
+            path=path,
+            request=request,
+            download=download,
+        )
 
         if settings.PYTHON_MEDIA:
             return self._serve_docs_python(
@@ -71,14 +76,15 @@ def _serve_docs(
             download=download,
         )
 
-    def _track_pageview(self, project, path, request):
+    def _track_pageview(self, project, path, request, download):
         """Create an audit log of the page view if audit is enabled."""
         # Remove any query args (like the token access from AWS).
         path_only = urlparse(path).path
         track_file = path_only.endswith(('.html', '.pdf', '.epub', '.zip'))
         if track_file and self._is_audit_enabled(project):
+            action = AuditLog.DOWNLOAD if download else AuditLog.PAGEVIEW
             AuditLog.objects.new(
-                action=AuditLog.PAGEVIEW,
+                action=action,
                 user=request.user,
                 request=request,
                 project=project,