From 425ad473f89af05e40c8d34f2934591546b858ee Mon Sep 17 00:00:00 2001
From: Beto Dealmeida <beto@preset.io>
Date: Fri, 17 Apr 2026 20:52:09 +0000
Subject: [PATCH] fix(oauth2): silence lock acquisition errors on token refresh

When a dashboard has multiple charts sharing an OAuth2-enabled DB
with an expired token, simultaneous requests race for the distributed
lock. Only the first acquires it; the rest should silently return None.

Added raise_on_giveup=False and giveup_log_level=logging.DEBUG to the
@backoff.on_exception decorator on get_oauth2_access_token so that
failed lock acquisition is no longer logged at ERROR level and does
not propagate as an exception.

Also adds a unit test covering this scenario.
---
 superset/utils/oauth2.py               |  2 ++
 tests/unit_tests/utils/oauth2_tests.py | 39 ++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/superset/utils/oauth2.py b/superset/utils/oauth2.py
index 76b671b4109a..62e6559a1b6f 100644
--- a/superset/utils/oauth2.py
+++ b/superset/utils/oauth2.py
@@ -82,6 +82,8 @@ def generate_code_challenge(code_verifier: str) -> str:
     factor=10,
     base=2,
     max_tries=5,
+    raise_on_giveup=False,
+    giveup_log_level=logging.DEBUG,
 )
 def get_oauth2_access_token(
     config: OAuth2ClientConfig,
diff --git a/tests/unit_tests/utils/oauth2_tests.py b/tests/unit_tests/utils/oauth2_tests.py
index a320a8a23e85..1ca9a4eb21b3 100644
--- a/tests/unit_tests/utils/oauth2_tests.py
+++ b/tests/unit_tests/utils/oauth2_tests.py
@@ -338,6 +338,45 @@ def test_encode_decode_oauth2_state(
     assert decoded["user_id"] == 2
 
 
+def test_get_oauth2_access_token_lock_not_acquired_no_error_log(
+    mocker: MockerFixture,
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """
+    Test that when a distributed lock can't be acquired, no error is logged and
+    the function returns None instead of raising.
+
+    This scenario occurs when a dashboard with multiple charts from the same
+    OAuth2-enabled DB has an expired token: simultaneous requests compete for
+    the lock, and only the first one wins. The rest should silently return None.
+    """
+    import logging
+
+    from superset.exceptions import AcquireDistributedLockFailedException
+
+    mocker.patch("time.sleep")  # avoid backoff delays in tests
+
+    db = mocker.patch("superset.utils.oauth2.db")
+    db_engine_spec = mocker.MagicMock()
+    token = mocker.MagicMock()
+    token.access_token = "access-token"  # noqa: S105
+    token.access_token_expiration = datetime(2024, 1, 1)
+    token.refresh_token = "refresh-token"  # noqa: S105
+    db.session.query().filter_by().one_or_none.return_value = token
+
+    mocker.patch(
+        "superset.utils.oauth2.refresh_oauth2_token",
+        side_effect=AcquireDistributedLockFailedException("Lock not available"),
+    )
+
+    with freeze_time("2024-01-02"):
+        with caplog.at_level(logging.DEBUG):
+            result = get_oauth2_access_token({}, 1, 1, db_engine_spec)
+
+    assert result is None
+    assert not any(record.levelno >= logging.ERROR for record in caplog.records)
+
+
 def test_get_oauth2_redirect_uri_from_config(mocker: MockerFixture) -> None:
     """
     Test that get_oauth2_redirect_uri returns the configured value when set.