From 4b23d0eccabcc3932369c90468be8b4f36ec64d0 Mon Sep 17 00:00:00 2001
From: "Hugh A. Miles II" <hughmil3s@gmail.com>
Date: Tue, 13 Apr 2021 10:08:34 -0400
Subject: [PATCH] fix: logs table - user_id is NULL (#14057)

* add user back to session

* add test for logging None on exceptions

* fix this updated test

* reformat

* reformat

* Update log.py
---
 superset/utils/log.py       | 15 +++++++++++++--
 tests/event_logger_tests.py | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/superset/utils/log.py b/superset/utils/log.py
index b48c47c01..b2a7459d2 100644
--- a/superset/utils/log.py
+++ b/superset/utils/log.py
@@ -118,12 +118,23 @@ class AbstractEventLogger(ABC):
 
         duration_ms = int(duration.total_seconds() * 1000) if duration else None
 
+        # Initial try and grab user_id via flask.g.user
         try:
             user_id = g.user.get_id()
-        except Exception as ex:  # pylint: disable=broad-except
-            logging.warning(ex)
+        except Exception:  # pylint: disable=broad-except
             user_id = None
 
+        # Whenever a user is not bounded to a session we
+        # need to add them back before logging to capture user_id
+        if user_id is None:
+            try:
+                session = current_app.appbuilder.get_session
+                session.add(g.user)
+                user_id = g.user.get_id()
+            except Exception as ex:  # pylint: disable=broad-except
+                logging.warning(ex)
+                user_id = None
+
         payload = collect_request_payload()
         if object_ref:
             payload["object_ref"] = object_ref
diff --git a/tests/event_logger_tests.py b/tests/event_logger_tests.py
index becbf4a1c..5727da7c3 100644
--- a/tests/event_logger_tests.py
+++ b/tests/event_logger_tests.py
@@ -21,6 +21,7 @@ from datetime import datetime, timedelta
 from typing import Any, Callable, cast, Dict, Iterator, Optional, Type, Union
 from unittest.mock import patch
 
+from flask import current_app
 from freezegun import freeze_time
 
 from superset import security_manager
@@ -194,3 +195,38 @@ class TestEventLogger(unittest.TestCase):
                 "duration": 5558756000,
             }
         ]
+
+    @patch("superset.utils.log.g", spec={})
+    def test_log_with_context_user_null(self, mock_g):
+        class DummyEventLogger(AbstractEventLogger):
+            def __init__(self):
+                self.records = []
+
+            def log(
+                self,
+                user_id: Optional[int],
+                action: str,
+                dashboard_id: Optional[int],
+                duration_ms: Optional[int],
+                slice_id: Optional[int],
+                referrer: Optional[str],
+                *args: Any,
+                **kwargs: Any,
+            ):
+                self.records.append(
+                    {**kwargs, "user_id": user_id, "duration": duration_ms}
+                )
+
+        logger = DummyEventLogger()
+
+        with app.test_request_context():
+            mock_g.side_effect = Exception("oops")
+            logger.log_with_context(
+                action="foo",
+                duration=timedelta(days=64, seconds=29156, microseconds=10),
+                object_ref={"baz": "food"},
+                log_to_statsd=False,
+                payload_override={"engine": "sqllite"},
+            )
+
+        assert logger.records[0]["user_id"] == None