fix(session): ne plus adresser à un serveur le jeton d'un autre

app/src/main/java/app/waveflow/data/remote/CatalogRepository.kt

@@ -5,7 +5,6 @@ import app.waveflow.model.RemoteAlbumDetail
 import app.waveflow.model.RemoteArtist
 import app.waveflow.model.RemoteArtistDetail
 import app.waveflow.model.RemoteSearchResults
-import app.waveflow.model.ServerSession
 
 /**
  * Le catalogue distant, muni d'une session.
@@ -46,42 +45,38 @@ class CatalogRepository(
     /**
      * Exécute [call] avec un jeton valide, en réessayant une fois sur refus.
      *
-     * [ServerSessionRepository.validAccessToken] renouvelle déjà avant
-     * l'échéance, mais un jeton peut être révoqué depuis un autre appareil : il
-     * est alors valide selon l'horloge et refusé par le serveur. Le second essai
-     * repart d'un jeton fraîchement obtenu ; s'il échoue à son tour, c'est que
-     * la session est bel et bien fermée.
+     * [ServerSessionRepository.authorize] renouvelle déjà avant l'échéance,
+     * mais un jeton peut être révoqué depuis un autre appareil : il est alors
+     * valide selon l'horloge et refusé par le serveur. Le second essai repart
+     * d'un jeton fraîchement obtenu ; s'il échoue à son tour, c'est que la
+     * session est bel et bien fermée.
+     *
+     * L'adresse et le jeton viennent du même appel, donc de la même session :
+     * les demander séparément permettrait d'adresser à un serveur le jeton d'un
+     * autre, si l'utilisateur se reconnecte ailleurs entre les deux.
      */
     private suspend fun <T> authorized(call: suspend (String, String) -> T): T {
-        val first = token() ?: throw ServerException.Unauthorized(SESSION_CLOSED)
+        val first = sessionRepository.authorize()
+            ?: throw ServerException.Unauthorized(SESSION_CLOSED)
 
         return try {
-            call(first.first, first.second)
+            call(first.serverUrl, first.accessToken)
         } catch (refused: ServerException.Unauthorized) {
-            val renewed = renewedToken() ?: throw refused
-            call(renewed.first, renewed.second)
+            // Périmer d'abord : sans ça, le second essai réutiliserait le jeton
+            // que le serveur vient de refuser, l'échéance locale le croyant bon.
+            sessionRepository.expireAccessToken(first.accessToken)
+            // Un renouvellement n'est utilisable que sur le serveur du premier
+            // essai : l'appel s'est déroulé sans verrou, et la session a pu
+            // basculer ailleurs entre-temps. Les identifiants n'ont de sens que
+            // pour celui qui les a émis — rejouer ailleurs rendrait une erreur,
+            // ou pire une ressource étrangère portant le même identifiant.
+            val renewed = sessionRepository.authorize()
+                ?.takeIf { it.serverUrl == first.serverUrl }
+                ?: throw refused
+            call(renewed.serverUrl, renewed.accessToken)
         }
     }
 
-    /** Adresse et jeton courants, ou `null` sans session. */
-    private suspend fun token(): Pair<String, String>? {
-        val accessToken = sessionRepository.validAccessToken() ?: return null
-        val url = (sessionRepository.session.value as? ServerSession.Connected)?.serverUrl
-            ?: return null
-        return url to accessToken
-    }
-
-    /**
-     * Force un renouvellement en périmant le jeton courant.
-     *
-     * Sans ça, le second essai réutiliserait celui que le serveur vient de
-     * refuser : l'échéance locale le croit encore bon.
-     */
-    private suspend fun renewedToken(): Pair<String, String>? {
-        sessionRepository.expireAccessToken()
-        return token()
-    }
-
     private companion object {
         const val SESSION_CLOSED = "Aucune session serveur."
     }

app/src/main/java/app/waveflow/data/remote/ServerImageAuthInterceptor.kt

@@ -1,6 +1,5 @@
 package app.waveflow.data.remote
 
-import app.waveflow.model.ServerSession
 import kotlinx.coroutines.runBlocking
 import okhttp3.Interceptor
 import okhttp3.Response
@@ -16,6 +15,17 @@ import okhttp3.Response
  * jaquette venue d'ailleurs pourrait ; lui joindre le jeton reviendrait à le
  * confier à un tiers.
  *
+ * L'origine est comparée à celle que [ServerSessionRepository.authorize] rend
+ * **avec** le jeton, et non à une session lue auparavant : entre les deux
+ * lectures, l'utilisateur peut s'être reconnecté ailleurs, et le jeton du
+ * nouveau serveur partirait à l'ancien.
+ *
+ * Conséquence assumée : une image venue d'ailleurs passe elle aussi par le
+ * verrou de session, et peut déclencher un renouvellement qui n'attendait plus
+ * qu'un appel. Le prix est modeste — le renouvellement était dû — et la
+ * garantie ne tient qu'à ce prix : filtrer avant de demander le jeton, c'est
+ * filtrer sur une session qui n'est peut-être plus celle du jeton obtenu.
+ *
  * L'appel est bloquant : les intercepteurs OkHttp le sont, et s'exécutent sur
  * ses propres fils.
  */
@@ -25,14 +35,7 @@ class ServerImageAuthInterceptor(
 
     override fun intercept(chain: Interceptor.Chain): Response {
         val request = chain.request()
-        val session = sessionRepository.session.value as? ServerSession.Connected
-            ?: return chain.proceed(request)
-
-        if (!request.url.isSameOriginAs(session.serverUrl)) return chain.proceed(request)
-
-        val token = runCatching { runBlocking { sessionRepository.validAccessToken() } }
-            .getOrNull()
-            ?: return chain.proceed(request)
+        val token = tokenFor(request) ?: return chain.proceed(request)
 
         val signed = chain.proceed(request.withBearer(token))
         if (signed.code != HTTP_UNAUTHORIZED) return signed
@@ -41,16 +44,41 @@ class ServerImageAuthInterceptor(
         // le catalogue, on le périme et on rejoue une fois — un second refus
         // veut dire que la session est fermée, et la pochette manquera.
         signed.close()
-        val renewed = runCatching {
-            runBlocking {
-                sessionRepository.expireAccessToken()
-                sessionRepository.validAccessToken()
-            }
-        }.getOrNull() ?: return chain.proceed(request)
+        val renewed = renewedTokenFor(request, refused = token) ?: return chain.proceed(request)
 
         return chain.proceed(request.withBearer(renewed))
     }
 
+    /** Périme le jeton refusé, puis en redemande un — la session en émettra un neuf. */
+    private fun renewedTokenFor(request: okhttp3.Request, refused: String): String? {
+        // Si la péremption échoue, redemander rendrait le même jeton que le
+        // serveur vient de refuser : autant renoncer que rejouer pour rien.
+        obtain { sessionRepository.expireAccessToken(refused) } ?: return null
+        return tokenFor(request)
+    }
+
+    /**
+     * Le jeton à joindre à [request], ou `null` s'il n'y a rien à signer.
+     *
+     * La session est prise d'un bloc — adresse et jeton — puis confrontée à
+     * l'URL demandée : le jeton n'est joint que s'il appartient bien au serveur
+     * auquel la requête s'adresse.
+     */
+    private fun tokenFor(request: okhttp3.Request): String? {
+        val authorized = obtain { sessionRepository.authorize() } ?: return null
+        return authorized.accessToken.takeIf { request.url.isSameOriginAs(authorized.serverUrl) }
+    }
+
+    /**
+     * Exécute [call] en bloquant, sans laisser remonter d'échec.
+     *
+     * Un serveur injoignable ou un refus ne doivent pas faire échouer le
+     * chargement de l'image : la requête partira sans signature, et la pochette
+     * manquera au pire.
+     */
+    private fun <T> obtain(call: suspend () -> T): T? =
+        runCatching { runBlocking { call() } }.getOrNull()
+
     private fun okhttp3.Request.withBearer(token: String) =
         newBuilder().header("Authorization", "Bearer $token").build()
 

app/src/main/java/app/waveflow/data/remote/ServerSessionRepository.kt

@@ -9,6 +9,19 @@ import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 
+/**
+ * Une adresse de serveur et un jeton d'accès **de la même session**.
+ *
+ * Les deux ne valent qu'ensemble : un jeton n'a de sens que pour le serveur qui
+ * l'a émis, et l'envoyer ailleurs reviendrait à le confier à un tiers. Les lire
+ * en deux temps laisserait la session changer entre les deux — d'où cette
+ * paire, rendue en une seule prise du verrou.
+ */
+data class AuthorizedCall(
+    val serverUrl: String,
+    val accessToken: String,
+)
+
 /**
  * La session serveur, et les seuls chemins qui la font changer.
  *
@@ -78,23 +91,30 @@ class ServerSessionRepository(
     }
 
     /**
-     * Un jeton d'accès utilisable, renouvelé si son échéance approche.
+     * De quoi appeler le serveur connecté : son adresse et un jeton d'accès
+     * utilisable, renouvelé si son échéance approche.
      *
      * Renvoie `null` quand il n'y a pas de session, ou quand le renouvellement
      * a été refusé — auquel cas la session est effacée et l'utilisateur devra
      * ressaisir son mot de passe.
      *
+     * Les deux sont rendus ensemble à dessein : un appelant qui lirait
+     * l'adresse d'un côté et le jeton de l'autre pourrait les prendre à deux
+     * sessions différentes, et adresser à l'un le jeton de l'autre.
+     *
      * @throws ServerException.Unreachable si le serveur ne répond pas ; la
      *   session est conservée, l'appelant réessaiera.
      */
-    suspend fun validAccessToken(): String? = mutex.withLock {
+    suspend fun authorize(): AuthorizedCall? = mutex.withLock {
         val current = _session.value as? ServerSession.Connected ?: return@withLock null
-        if (now() < current.accessExpiresAtMs - EXPIRY_MARGIN_MS) return@withLock current.accessToken
+        if (now() < current.accessExpiresAtMs - EXPIRY_MARGIN_MS) {
+            return@withLock AuthorizedCall(current.serverUrl, current.accessToken)
+        }
 
         try {
             val tokens = api.refresh(current.serverUrl, current.refreshToken)
             persist(tokens.toSession(current.serverUrl))
-            tokens.accessToken
+            AuthorizedCall(current.serverUrl, tokens.accessToken)
         } catch (refused: ServerException.Unauthorized) {
             // Le jeton de rafraîchissement est mort : révoqué ailleurs, ou
             // périmé. Rien à réessayer, il faut une nouvelle connexion.
@@ -105,14 +125,19 @@ class ServerSessionRepository(
     }
 
     /**
-     * Marque le jeton d'accès comme périmé.
+     * Marque [refused] comme périmé, si c'est bien le jeton courant.
      *
      * Utile quand le serveur en refuse un que l'horloge locale croit encore
      * bon — révoqué depuis un autre appareil, par exemple. Le prochain
-     * [validAccessToken] renouvellera au lieu de resservir le même.
+     * [authorize] renouvellera au lieu de resservir le même.
+     *
+     * Le jeton refusé est exigé parce qu'un autre appelant a pu renouveler
+     * entre le refus et cet appel : périmer aveuglément jetterait un jeton neuf
+     * et déclencherait un renouvellement pour rien.
      */
-    suspend fun expireAccessToken() = mutex.withLock {
+    suspend fun expireAccessToken(refused: String) = mutex.withLock {
         val current = _session.value as? ServerSession.Connected ?: return@withLock
+        if (current.accessToken != refused) return@withLock
         persist(current.copy(accessExpiresAtMs = 0L))
     }
 

app/src/test/java/app/waveflow/data/remote/CatalogRepositoryTest.kt

@@ -93,6 +93,28 @@ class CatalogRepositoryTest {
         assertEquals(2, catalog.calls)
     }
 
+    @Test
+    fun `un rejeu ne part pas vers un autre serveur que le premier essai`() = runTest {
+        // Le jeton est refusé et, pendant l'appel, l'utilisateur se reconnecte
+        // ailleurs. La paire adresse-jeton reste cohérente, donc rien ne fuit —
+        // mais les identifiants n'ont de sens que pour le serveur qui les a
+        // émis : rejouer sur le nouveau rendrait une erreur, ou pire une
+        // ressource étrangère portant le même identifiant.
+        val catalog = FakeCatalogApi(failuresBeforeSuccess = 1)
+        val sessions = connectedSessionRepository()
+        catalog.pendantLAppel = {
+            // Une seule fois : c'est la bascule qu'on veut, pas une boucle.
+            catalog.pendantLAppel = null
+            sessions.connect("https://ailleurs.test", "autre", "secret")
+        }
+        val repository = CatalogRepository(catalog, sessions)
+
+        val error = runCatching { repository.album("alb-1") }.exceptionOrNull()
+
+        assertTrue("le refus initial doit remonter", error is ServerException.Unauthorized)
+        assertEquals("rien ne doit être rejoué ailleurs", 1, catalog.calls)
+    }
+
     @Test
     fun `une panne reseau n'est pas prise pour un jeton perime`() = runTest {
         val catalog = FakeCatalogApi(failure = ServerException.Unreachable("coupure"))

app/src/test/java/app/waveflow/data/remote/ServerImageAuthInterceptorTest.kt

@@ -7,8 +7,10 @@ import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.test.runTest
 import okhttp3.OkHttpClient
 import okhttp3.Request
+import okhttp3.mockwebserver.Dispatcher
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
+import okhttp3.mockwebserver.RecordedRequest
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNull
@@ -139,6 +141,46 @@ class ServerImageAuthInterceptorTest {
         assertEquals(1, api.refreshCalls)
     }
 
+    @Test
+    fun `un jeton obtenu apres un changement de serveur ne part pas a l'ancien`() = runTest {
+        // La fenêtre : l'intercepteur attend la réponse de l'ancien serveur —
+        // il ne tient alors aucun verrou — et l'utilisateur se reconnecte
+        // ailleurs pendant ce temps. Le jeton qu'il obtiendra ensuite est celui
+        // du nouveau serveur ; le rejeu l'enverrait à l'ancien, qui n'a rien à
+        // en connaître.
+        //
+        // La bascule est déclenchée depuis la réponse elle-même, ce qui la
+        // place exactement dans la fenêtre plutôt que d'espérer l'y croiser.
+        val autre = MockWebServer()
+        autre.start()
+
+        try {
+            val sessions = sessions()
+            var bascule = false
+            server.dispatcher = object : Dispatcher() {
+                override fun dispatch(request: RecordedRequest): MockResponse {
+                    if (!bascule) {
+                        bascule = true
+                        val ailleurs = autre.url("/").toString().trimEnd('/')
+                        runBlocking { sessions.connect(ailleurs, "autre", "secret") }
+                    }
+                    return MockResponse().setResponseCode(401)
+                }
+            }
+
+            fetch(clientWith(sessions), "${url()}/api/v2/artwork/1daf991a")
+
+            assertEquals("Bearer wfa_stocke", server.takeRequest().getHeader("Authorization"))
+            assertNull(
+                "le jeton du nouveau serveur ne doit pas partir à l'ancien",
+                server.takeRequest().getHeader("Authorization"),
+            )
+            assertEquals("le nouveau serveur n'a rien demandé", 0, autre.requestCount)
+        } finally {
+            autre.shutdown()
+        }
+    }
+
     @Test
     fun `un second refus n'est pas rejoue indefiniment`() = runTest {
         server.enqueue(MockResponse().setResponseCode(401))
@@ -164,6 +206,6 @@ class ServerImageAuthInterceptorTest {
 
         fetch(clientWith(sessions), "${url()}/api/v2/artwork/1daf991a")
 
-        assertEquals("wfa_stocke", runBlocking { sessions.validAccessToken() })
+        assertEquals("wfa_stocke", runBlocking { sessions.authorize()?.accessToken })
     }
 }