fix(3cx): adopt a new client secret only after login succeeds

A candidate secret is tried before it replaces the current one, so a revoked secret cannot stick and a later revert to a working secret still takes effect. A failed re-login after 401 returns the original API response.

src/shared/shared/three_cx_client.py

@@ -47,7 +47,6 @@ class ThreeCXClient:
         self._auth_mode = auth_mode
         self._client_id = auth_kwargs.get("client_id")
         self._client_secret = auth_kwargs.get("client_secret")
-        self._retired_secrets: set[str] = set()
         self._token_expires_at = 0.0
         self._auth_lock = threading.RLock()
         self.session = requests.Session()
@@ -68,20 +67,24 @@ class ThreeCXClient:
     def use_client_secret(self, client_secret: str) -> None:
         """Point this client at ``client_secret`` and log in again if needed.
 
-        The compare-and-set runs under ``_auth_lock``. A secret this client has
-        already replaced is ignored, so a slower caller still holding the
-        pre-rotation secret cannot write it back.
+        A different secret is logged in with before it replaces the current one.
+        A candidate that 3CX rejects leaves the working secret in place, so a
+        slower caller still holding a revoked secret cannot clobber a good one,
+        and a later revert to a secret 3CX accepts still takes effect.
         """
         with self._auth_lock:
-            if (
-                client_secret != self._client_secret
-                and client_secret not in self._retired_secrets
-            ):
-                if self._client_secret:
-                    self._retired_secrets.add(self._client_secret)
-                self._client_secret = client_secret
-                self._token_expires_at = 0.0
-            self._refresh_expired_token()
+            if client_secret == self._client_secret:
+                self._refresh_expired_token()
+                return
+            try:
+                self._authenticate_oauth(self._client_id, client_secret)
+            except Exception:
+                logger.warning(
+                    "3CX login with a new client secret failed; keeping the current secret"
+                )
+                self._refresh_expired_token()
+                return
+            self._client_secret = client_secret
 
     def ensure_fresh_token(self) -> None:
         """Fetch a new access token when the current one is missing or near expiry."""
@@ -105,9 +108,15 @@ class ThreeCXClient:
             self.ensure_fresh_token()
         response = self._raw_request(method, url, **kwargs)
         if self._auth_mode == "oauth" and response.status_code == 401:
-            with self._auth_lock:
-                self._token_expires_at = 0.0
-                self._authenticate_oauth(self._client_id, self._client_secret)
+            try:
+                with self._auth_lock:
+                    self._token_expires_at = 0.0
+                    self._authenticate_oauth(self._client_id, self._client_secret)
+            except Exception:
+                logger.warning(
+                    "3CX re-login after 401 failed; returning the original response"
+                )
+                return response
             response = self._raw_request(method, url, **kwargs)
         return response
 

tests/shared/test_three_cx_client.py

@@ -286,15 +286,71 @@ def test_oauth_client_reauths_when_client_secret_changes():
     )
     first = tcx.oauth_client("test.3cx.us", "cid", "old-secret")
     second = tcx.oauth_client("test.3cx.us", "cid", "new-secret")
-    stale = tcx.oauth_client("test.3cx.us", "cid", "old-secret")
-    assert first is second is stale
-    assert stale.session.headers["Authorization"] == "Bearer tok-new"
+    assert first is second
+    assert second.session.headers["Authorization"] == "Bearer tok-new"
     posts = _token_posts()
     assert len(posts) == 2
     assert "client_secret=new-secret" in posts[1].request.body
     tcx._oauth_clients.clear()
 
 
+@responses.activate
+def test_oauth_client_accepts_a_reverted_client_secret():
+    from shared import three_cx_client as tcx
+
+    tcx._oauth_clients.clear()
+    for token in ("tok-a", "tok-b", "tok-a-again"):
+        responses.add(
+            responses.POST,
+            f"{BASE}/connect/token",
+            json={"access_token": token, "expires_in": 3600},
+            status=200,
+        )
+    responses.add(
+        responses.GET,
+        f"{BASE}/xapi/v1/Queues/Pbx.GetByNumber(number='801')",
+        json={"Id": 83},
+        status=200,
+    )
+    client = tcx.oauth_client("test.3cx.us", "cid", "secret-a")
+    tcx.oauth_client("test.3cx.us", "cid", "secret-b")
+    reverted = tcx.oauth_client("test.3cx.us", "cid", "secret-a")
+    assert reverted is client
+    assert reverted.session.headers["Authorization"] == "Bearer tok-a-again"
+    assert reverted.get_queue("801")["Id"] == 83
+    posts = _token_posts()
+    assert len(posts) == 3
+    assert "client_secret=secret-a" in posts[2].request.body
+    tcx._oauth_clients.clear()
+
+
+@responses.activate
+def test_oauth_client_keeps_current_secret_when_candidate_login_fails():
+    from shared import three_cx_client as tcx
+
+    tcx._oauth_clients.clear()
+    responses.add(
+        responses.POST,
+        f"{BASE}/connect/token",
+        json={"access_token": "tok-current", "expires_in": 3600},
+        status=200,
+    )
+    responses.add(
+        responses.POST,
+        f"{BASE}/connect/token",
+        json={"access_token": "tok-new", "expires_in": 3600},
+        status=200,
+    )
+    responses.add(responses.POST, f"{BASE}/connect/token", status=401)
+    client = tcx.oauth_client("test.3cx.us", "cid", "current-secret")
+    tcx.oauth_client("test.3cx.us", "cid", "new-secret")
+    kept = tcx.oauth_client("test.3cx.us", "cid", "revoked-secret")
+    assert kept is client
+    assert kept.session.headers["Authorization"] == "Bearer tok-new"
+    assert kept._client_secret == "new-secret"
+    tcx._oauth_clients.clear()
+
+
 @responses.activate
 def test_oauth_request_retries_once_after_401():
     _stub_oauth()
@@ -321,3 +377,24 @@ def test_oauth_request_retries_once_after_401():
     assert client.get_queue("801")["Id"] == 83
     assert client.session.headers["Authorization"] == "Bearer tok-refreshed"
     assert len(_token_posts()) == 2
+
+
+@responses.activate
+def test_oauth_401_returns_original_response_when_relogin_fails():
+    import pytest
+    from requests import HTTPError
+
+    _stub_oauth()
+    responses.add(responses.POST, f"{BASE}/connect/token", status=401)
+    responses.add(
+        responses.GET,
+        f"{BASE}/xapi/v1/Queues/Pbx.GetByNumber(number='801')",
+        status=401,
+    )
+    client = ThreeCXClient(
+        domain="test.3cx.us", auth_mode="oauth", client_id="c", client_secret="s"
+    )
+    with pytest.raises(HTTPError) as raised:
+        client.get_queue("801")
+    assert "Queues/Pbx.GetByNumber" in str(raised.value)
+    assert raised.value.response.status_code == 401