diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt index 6ae07356..fc6ed5e4 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt @@ -117,6 +117,7 @@ object ApiClient { .base .newBuilder() .addInterceptor(authInterceptor) + .addInterceptor(ProfileScopeInterceptor) .addInterceptor(logging) .authenticator(TokenRefreshAuthenticator) .certificatePinner(certificatePinner) diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptor.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptor.kt new file mode 100644 index 00000000..75ec2592 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptor.kt @@ -0,0 +1,77 @@ +package com.m57.hermescontrol.data.remote + +import com.m57.hermescontrol.data.local.AuthManager +import okhttp3.Interceptor +import okhttp3.Response + +/** + * Appends `?profile=` to profile-scoped management endpoints, + * mirroring the desktop/web dashboard contract + * (`web/src/lib/api.ts` → `PROFILE_SCOPED_PREFIXES` + `withManagementProfile`). + * + * Without this, mobile fired all management REST calls with NO profile scope, + * so every profile read/wrote the backend's implicit default profile — + * multi-profile setups silently corrupted config/skills/toolsets/mcp/model/env + * (issue #528). + * + * Rules (copied from the desktop contract): + * - If no profile is active, pass through unchanged (legacy "default" behavior). + * - If the URL already carries an explicit `profile=` query, leave it + * untouched — explicit beats global. + * - Only the endpoint families below honor `?profile=` on the backend; + * everything else (ops, pairing, cron, profiles themselves) is + * machine-global or self-scoped and must NOT be rewritten. + */ +object ProfileScopeInterceptor : Interceptor { + private val PROFILE_SCOPED_PREFIXES = + listOf( + "/api/status", + "/api/gateway", + "/api/analytics", + "/api/skills", + "/api/tools/toolsets", + "/api/config", + "/api/env", + "/api/mcp", + "/api/messaging/platforms", + "/api/messaging/telegram/onboarding", + "/api/messaging/whatsapp/onboarding", + "/api/model/info", + "/api/model/set", + "/api/model/auxiliary", + "/api/model/moa", + "/api/model/options", + ) + + /** + * True only when [path] matches a scoped prefix as a whole path segment — + * e.g. `/api/status` or `/api/status/health`, but NOT `/api/statusXYZ` + * or `/api/gatewayExtra` (Sourcery review, PR #540). + */ + private fun isProfileScopedPath(path: String): Boolean = + PROFILE_SCOPED_PREFIXES.any { prefix -> path == prefix || path.startsWith("$prefix/") } + + override fun intercept(chain: Interceptor.Chain): Response { + val request = chain.request() + val profile = + AuthManager.getSelectedProfileId() + ?: return chain.proceed(request) + + val url = request.url + if (url.queryParameter("profile") != null) { + return chain.proceed(request) // explicit param wins + } + + if (!isProfileScopedPath(url.encodedPath)) { + return chain.proceed(request) + } + + val scopedUrl = + url + .newBuilder() + .addQueryParameter("profile", profile) + .build() + + return chain.proceed(request.newBuilder().url(scopedUrl).build()) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt b/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt index d21aeecc..a1c9d591 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt @@ -43,6 +43,11 @@ class ApiClientTest { every { AuthManager.baseUrl() } returns mockWebServer.url("/").toString() every { AuthManager.getHost() } returns "127.0.0.1" every { AuthManager.getPort() } returns 9119 + + // PR #540 registers ProfileScopeInterceptor in ApiClient.buildService(), + // which calls getSelectedProfileId(). Stub it to null so the interceptor + // short-circuits (no profile scope) and these auth tests stay focused. + every { AuthManager.getSelectedProfileId() } returns null } @AfterEach diff --git a/app/src/test/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptorTest.kt b/app/src/test/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptorTest.kt new file mode 100644 index 00000000..f87675b2 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptorTest.kt @@ -0,0 +1,129 @@ +package com.m57.hermescontrol.data.remote + +import com.m57.hermescontrol.data.local.AuthManager +import io.mockk.every +import io.mockk.mockkObject +import io.mockk.unmockkAll +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test + +class ProfileScopeInterceptorTest { + private lateinit var server: MockWebServer + + @Before + fun setUp() { + mockkObject(AuthManager) + server = MockWebServer() + server.start() + } + + @After + fun tearDown() { + server.shutdown() + unmockkAll() + } + + /** Builds a real client with the interceptor + a stubbed AuthManager profile. */ + private fun clientFor(profile: String?): OkHttpClient { + every { AuthManager.getSelectedProfileId() } returns profile + return OkHttpClient + .Builder() + .addInterceptor(ProfileScopeInterceptor) + .build() + } + + private fun lastRequestedPath(client: OkHttpClient): HttpUrl { + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = Request.Builder().url(server.url("api/config")).build() + client.newCall(req).execute().close() + return server.takeRequest().requestUrl!! + } + + @Test + fun scopedEndpoint_appendsProfile() { + val client = clientFor("work") + val url = lastRequestedPath(client) + assertEquals("work", url.queryParameter("profile")) + assertEquals("/api/config", url.encodedPath) + } + + @Test + fun noActiveProfile_passesThrough() { + val client = clientFor(null) + val url = lastRequestedPath(client) + assertNull(url.queryParameter("profile")) + } + + @Test + fun explicitProfileParam_wins() { + every { AuthManager.getSelectedProfileId() } returns "work" + val client = OkHttpClient.Builder().addInterceptor(ProfileScopeInterceptor).build() + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = + Request + .Builder() + .url(server.url("api/config?profile=other")) + .build() + client.newCall(req).execute().close() + val url = server.takeRequest().requestUrl!! + assertEquals("other", url.queryParameter("profile")) + } + + @Test + fun nonScopedEndpoint_untouched() { + every { AuthManager.getSelectedProfileId() } returns "work" + val client = OkHttpClient.Builder().addInterceptor(ProfileScopeInterceptor).build() + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = Request.Builder().url(server.url("api/cron/jobs")).build() + client.newCall(req).execute().close() + val url = server.takeRequest().requestUrl!! + assertNull(url.queryParameter("profile")) + assertEquals("/api/cron/jobs", url.encodedPath) + } + + @Test + fun skillsEndpoint_isScoped() { + val client = clientFor("alpha") + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = Request.Builder().url(server.url("api/skills")).build() + client.newCall(req).execute().close() + val url = server.takeRequest().requestUrl!! + assertEquals("alpha", url.queryParameter("profile")) + } + + @Test + fun lookalikePath_notScoped() { + // Sourcery review (PR #540): `startsWith` must not match non-segment + // suffixes like /api/statusXYZ or /api/gatewayExtra. + every { AuthManager.getSelectedProfileId() } returns "work" + val client = OkHttpClient.Builder().addInterceptor(ProfileScopeInterceptor).build() + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = Request.Builder().url(server.url("api/statusXYZ")).build() + client.newCall(req).execute().close() + val url = server.takeRequest().requestUrl!! + assertNull(url.queryParameter("profile")) + assertEquals("/api/statusXYZ", url.encodedPath) + } + + @Test + fun scopedSubPath_isScoped() { + // A scoped prefix with a trailing segment (/api/status/health) MUST + // still receive the profile param. + every { AuthManager.getSelectedProfileId() } returns "work" + val client = OkHttpClient.Builder().addInterceptor(ProfileScopeInterceptor).build() + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = Request.Builder().url(server.url("api/status/health")).build() + client.newCall(req).execute().close() + val url = server.takeRequest().requestUrl!! + assertEquals("work", url.queryParameter("profile")) + assertEquals("/api/status/health", url.encodedPath) + } +}