fix(async): synchronize token refresh to avoid race condition (#708)

src/keycloak/openid_connection.py

@@ -30,6 +30,7 @@
 
 from __future__ import annotations
 
+import asyncio
 from datetime import datetime, timedelta, timezone
 from typing import TYPE_CHECKING, Any
 
@@ -143,6 +144,7 @@ def __init__(
         self.custom_headers = custom_headers
         self.headers = {**self.headers, "Content-Type": "application/json"}
         self.cert = cert
+        self._a_refresh_lock = asyncio.Lock()
 
         if not self.grant_type:
             if username and password:
@@ -530,24 +532,33 @@ async def a_refresh_token(self) -> None:
 
         :raises KeycloakPostError: In case the refresh token request failed.
         """
-        refresh_token = self.token.get("refresh_token", None) if self.token else None
-        if refresh_token is None:
-            await self.a_get_token()
-        else:
-            try:
-                self.token = await self.keycloak_openid.a_refresh_token(refresh_token)
-            except KeycloakPostError as e:
-                list_errors = [
-                    b"Refresh token expired",
-                    b"Token is not active",
-                    b"Session not active",
-                ]
-                if e.response_code == HTTP_BAD_REQUEST and any(
-                    err in (e.response_body or b"") for err in list_errors
-                ):
-                    await self.a_get_token()
-                else:
-                    raise
+        current_token = self.token.get("access_token") if self.token else None
+
+        async with self._a_refresh_lock:
+            new_token = self.token.get("access_token") if self.token else None
+            if (current_token is None and new_token is not None) or (
+                current_token is not None and current_token != new_token
+            ):
+                return
+
+            refresh_token = self.token.get("refresh_token", None) if self.token else None
+            if refresh_token is None:
+                await self.a_get_token()
+            else:
+                try:
+                    self.token = await self.keycloak_openid.a_refresh_token(refresh_token)
+                except KeycloakPostError as e:
+                    list_errors = [
+                        b"Refresh token expired",
+                        b"Token is not active",
+                        b"Session not active",
+                    ]
+                    if e.response_code == HTTP_BAD_REQUEST and any(
+                        err in (e.response_body or b"") for err in list_errors
+                    ):
+                        await self.a_get_token()
+                    else:
+                        raise
 
     async def a__refresh_if_required(self) -> None:
         """Refresh the token if it is expired."""

tests/test_keycloak_admin.py

@@ -1284,14 +1284,14 @@ def test_clients(admin: KeycloakAdmin, realm: str) -> None:
             resource_id="invalid_resource_id",
             payload={"name": "temp-updated-resource"},
         )
-    assert err.match("404: b''"), err
+    assert err.match("404: b.*"), err
     cz_res = admin.delete_client_authz_resource(
         client_id=auth_client_id, resource_id=temp_resource_id
     )
     assert cz_res == {}
     with pytest.raises(KeycloakGetError) as err:
         admin.get_client_authz_resource(client_id=auth_client_id, resource_id=temp_resource_id)
-    assert err.match("404: b''")
+    assert err.match("404: b.*")
 
     # Authz policies
     res = admin.get_client_authz_policies(client_id=auth_client_id)
@@ -1335,7 +1335,7 @@ def test_clients(admin: KeycloakAdmin, realm: str) -> None:
     assert cz_res == {}
     with pytest.raises(KeycloakGetError) as err:
         admin.get_client_authz_policy(client_id=auth_client_id, policy_id=res["id"])
-    assert err.match("404: b''")
+    assert err.match("404: b.*")
 
     res = admin.create_client_authz_policy(
         client_id=auth_client_id,
@@ -2671,7 +2671,7 @@ def test_auth_flows(admin: KeycloakAdmin, realm: str) -> None:
     # Test copying
     with pytest.raises(KeycloakPostError) as err:
         admin.copy_authentication_flow(payload={}, flow_alias="bad")
-    assert ('b\'{"error":"Flow not found"' in str(err)) or err.match("404: b''")
+    assert ('b\'{"error":"Flow not found"' in str(err)) or err.match("404: b.*")
 
     res = admin.copy_authentication_flow(payload={"newName": "test-browser"}, flow_alias="browser")
     assert res == b"", res
@@ -2708,7 +2708,7 @@ def test_auth_flows(admin: KeycloakAdmin, realm: str) -> None:
 
     with pytest.raises(KeycloakGetError) as err:
         admin.get_authentication_flow_executions(flow_alias="bad")
-    assert ('b\'{"error":"Flow not found"' in str(err)) or err.match("404: b''")
+    assert ('b\'{"error":"Flow not found"' in str(err)) or err.match("404: b.*")
     exec_id = res[0]["id"]
 
     res = admin.get_authentication_flow_execution(execution_id=exec_id)
@@ -4986,7 +4986,7 @@ async def test_a_clients(admin: KeycloakAdmin, realm: str) -> None:
             resource_id="invalid_resource_id",
             payload={"name": "temp-updated-resource"},
         )
-    assert err.match("404: b''"), err
+    assert err.match("404: b.*"), err
     res = await admin.a_delete_client_authz_resource(
         client_id=auth_client_id,
         resource_id=temp_resource_id,
@@ -4997,7 +4997,7 @@ async def test_a_clients(admin: KeycloakAdmin, realm: str) -> None:
             client_id=auth_client_id,
             resource_id=temp_resource_id,
         )
-    assert err.match("404: b''")
+    assert err.match("404: b.*")
 
     # Authz policies
     res = await admin.a_get_client_authz_policies(client_id=auth_client_id)
@@ -5041,7 +5041,7 @@ async def test_a_clients(admin: KeycloakAdmin, realm: str) -> None:
     assert res3 == {}
     with pytest.raises(KeycloakGetError) as err:
         await admin.a_get_client_authz_policy(client_id=auth_client_id, policy_id=res["id"])
-    assert err.match("404: b''")
+    assert err.match("404: b.*")
 
     res = await admin.a_create_client_authz_policy(
         client_id=auth_client_id,
@@ -6510,7 +6510,7 @@ async def test_a_auth_flows(admin: KeycloakAdmin, realm: str) -> None:
     # Test copying
     with pytest.raises(KeycloakPostError) as err:
         await admin.a_copy_authentication_flow(payload={}, flow_alias="bad")
-    assert ('b\'{"error":"Flow not found"' in str(err)) or err.match("404: b''")
+    assert ('b\'{"error":"Flow not found"' in str(err)) or err.match("404: b.*")
 
     res = await admin.a_copy_authentication_flow(
         payload={"newName": "test-browser"},
@@ -6555,7 +6555,7 @@ async def test_a_auth_flows(admin: KeycloakAdmin, realm: str) -> None:
 
     with pytest.raises(KeycloakGetError) as err:
         await admin.a_get_authentication_flow_executions(flow_alias="bad")
-    assert ('b\'{"error":"Flow not found"' in str(err)) or err.match("404: b''")
+    assert ('b\'{"error":"Flow not found"' in str(err)) or err.match("404: b.*")
     exec_id = res[0]["id"]
 
     res = await admin.a_get_authentication_flow_execution(execution_id=exec_id)

tests/test_keycloak_uma.py

@@ -148,12 +148,12 @@ def test_uma_resource_sets(uma: KeycloakUMA) -> None:
     assert res == {}, res
     with pytest.raises(KeycloakGetError) as err:
         uma.resource_set_read(created_resource["_id"])
-    err.match("404: b''")
+    err.match("404: b.*")
 
     # Test delete fail
     with pytest.raises(KeycloakDeleteError) as err:
         uma.resource_set_delete(resource_id=created_resource["_id"])
-    assert err.match("404: b''")
+    assert err.match("404: b.*")
 
 
 def test_uma_policy(uma: KeycloakUMA, admin: KeycloakAdmin) -> None:
@@ -457,12 +457,12 @@ async def test_a_uma_resource_sets(uma: KeycloakUMA) -> None:
     assert res == {}, res
     with pytest.raises(KeycloakGetError) as err:
         await uma.a_resource_set_read(created_resource["_id"])
-    err.match("404: b''")
+    err.match("404: b.*")
 
     # Test delete fail
     with pytest.raises(KeycloakDeleteError) as err:
         await uma.a_resource_set_delete(resource_id=created_resource["_id"])
-    assert err.match("404: b''")
+    assert err.match("404: b.*")
 
 
 @pytest.mark.asyncio