From efacd2ef740c6fe2a23ad6734fee36aa1b8aa566 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:53:26 +0300 Subject: [PATCH 1/7] feat: add instance domain model and static config Introduces Instance and InstancesConfig: what one Open edX platform is and how it is parsed from the INSTANCES block of the bundled config. Nothing reads it yet. Required fields are NAME, API_HOST_URL and OAUTH_CLIENT_ID; a malformed entry is dropped. KEY falls back to a slug of NAME, INSTANCE_NAME is localized with an English fallback, and PLATFORM_NAME defaults to the display name. The schema matches iOS (#672). --- .../org/openedx/core/instance/Instance.kt | 185 ++++++++++++++++++ .../openedx/core/instance/InstancesConfig.kt | 51 +++++ .../core/instance/InstancesConfigTest.kt | 183 +++++++++++++++++ .../instances/bundled_instances.json | 45 +++++ default_config/dev/config.yaml | 49 +++++ 5 files changed, 513 insertions(+) create mode 100644 core/src/main/java/org/openedx/core/instance/Instance.kt create mode 100644 core/src/main/java/org/openedx/core/instance/InstancesConfig.kt create mode 100644 core/src/test/java/org/openedx/core/instance/InstancesConfigTest.kt create mode 100644 core/src/test/resources/instances/bundled_instances.json diff --git a/core/src/main/java/org/openedx/core/instance/Instance.kt b/core/src/main/java/org/openedx/core/instance/Instance.kt new file mode 100644 index 000000000..670b13216 --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/Instance.kt @@ -0,0 +1,185 @@ +package org.openedx.core.instance + +import com.google.gson.JsonElement +import com.google.gson.JsonObject +import java.net.URI +import java.text.Normalizer +import java.util.Locale + +/** + * One Open edX platform the app can sign in to: its own host, OAuth client, branding and + * per-platform overrides of the app-level configuration. + * + * Instances are identified by their stable [key], never by their (possibly localized) + * display name or their position in the catalog. + */ +data class Instance( + /** Machine-readable identifier, e.g. "niepd". Falls back to a slugified [name]. */ + val key: String, + val name: String, + /** Localized display name; falls back to the English entry, then to [name]. */ + val instanceName: String, + val color: String, + val oAuthClientId: String, + val apiHostUrl: String, + val ssoUrl: String?, + val ssoFinishedUrl: String?, + val environmentDisplayName: String?, + val isSwitchInstanceLoginEnabled: Boolean, + /** Remote URL, or the name of a bundled drawable. `null` keeps the default logo. */ + val logoUrl: String?, + /** Remote URL, or the name of a bundled drawable. `null` keeps the default header. */ + val headerBackgroundUrl: String?, + /** `null` derives the palette from [color] instead. */ + val themeColors: InstanceThemeColors?, + /** Defaults to [instanceName] when unset, so it is never blank. */ + val platformName: String, + val isRoundedCorners: Boolean, + val buttonCornersRadius: Double, + /** + * The entry exactly as configured, in UPPER_SNAKE_CASE. Instance-level overrides of + * app-level keys (`AGREEMENT_URLS`, `DISCOVERY`, `UI_COMPONENTS`, ...) are read from here. + */ + val config: JsonObject, +) { + + companion object { + const val KEY = "KEY" + const val NAME = "NAME" + const val INSTANCE_NAME = "INSTANCE_NAME" + const val COLOR = "COLOR" + const val OAUTH_CLIENT_ID = "OAUTH_CLIENT_ID" + const val API_HOST_URL = "API_HOST_URL" + const val SSO_URL = "SSO_URL" + const val SSO_FINISHED_URL = "SSO_FINISHED_URL" + const val SSO_URL_SUCCESSFUL_LOGIN = "SSO_URL_SUCCESSFUL_LOGIN" + const val ENVIRONMENT_DISPLAY_NAME = "ENVIRONMENT_DISPLAY_NAME" + const val IS_SWITCH_INSTANCE_LOGIN_ENABLED = "IS_SWITCH_INSTANCE_LOGIN_ENABLED" + const val PLATFORM_NAME = "PLATFORM_NAME" + const val THEME = "THEME" + const val THEME_LIGHT = "LIGHT" + const val THEME_DARK = "DARK" + const val THEME_LOGO_URL = "LOGO_URL" + const val THEME_HEADER_BACKGROUND_URL = "HEADER_BACKGROUND_URL" + const val THEME_ROUNDED_CORNERS_STYLE = "ROUNDED_CORNERS_STYLE" + const val THEME_BUTTON_CORNERS_RADIUS = "BUTTON_CORNERS_RADIUS" + + private const val DEFAULT_COLOR = "#007AFF" + private const val DEFAULT_BUTTON_CORNERS_RADIUS = 8.0 + private const val DEFAULT_LANGUAGE = "en" + + /** + * Parses one `INSTANCES` entry. Returns `null` when a required field (`NAME`, + * `API_HOST_URL`, `OAUTH_CLIENT_ID`) is missing or `API_HOST_URL` isn't an absolute + * URL, so a malformed entry is dropped instead of producing a half-populated instance. + */ + @Suppress("ReturnCount") + fun fromJson( + json: JsonObject, + languageCode: String = Locale.getDefault().language, + ): Instance? { + val name = json.string(NAME)?.takeIf { it.isNotBlank() } ?: return null + val apiHostUrl = json.string(API_HOST_URL)?.takeIf { it.isAbsoluteUrl() } ?: return null + val oAuthClientId = json.string(OAUTH_CLIENT_ID)?.takeIf { it.isNotBlank() } ?: return null + + val key = json.string(KEY)?.takeIf { it.isNotBlank() } ?: slugify(name) + val localizedNames = json.stringMap(INSTANCE_NAME) + val instanceName = localizedNames[languageCode] + ?: localizedNames[DEFAULT_LANGUAGE] + ?: name + + // Logo and header live inside THEME, as siblings of LIGHT/DARK. The palette maps' + // own field names (accent_color, ...) stay lowercase: they are the theme engine's + // contract, shared with iOS, not part of this schema. + val theme = json.obj(THEME) + val themeColors = theme?.let { + InstanceThemeColors( + light = it.stringMap(THEME_LIGHT), + dark = it.stringMap(THEME_DARK), + ) + } + + return Instance( + key = key, + name = name, + instanceName = instanceName, + color = json.nonBlankString(COLOR) ?: DEFAULT_COLOR, + oAuthClientId = oAuthClientId, + apiHostUrl = apiHostUrl, + ssoUrl = json.nonBlankString(SSO_URL), + ssoFinishedUrl = json.nonBlankString(SSO_FINISHED_URL) + ?: json.nonBlankString(SSO_URL_SUCCESSFUL_LOGIN), + environmentDisplayName = json.nonBlankString(ENVIRONMENT_DISPLAY_NAME), + isSwitchInstanceLoginEnabled = json.boolean(IS_SWITCH_INSTANCE_LOGIN_ENABLED) ?: false, + logoUrl = theme?.nonBlankString(THEME_LOGO_URL), + headerBackgroundUrl = theme?.nonBlankString(THEME_HEADER_BACKGROUND_URL), + themeColors = themeColors, + platformName = json.nonBlankString(PLATFORM_NAME) ?: instanceName, + // Absent or non-boolean means rounded, same as the app-level theme default. + isRoundedCorners = theme?.boolean(THEME_ROUNDED_CORNERS_STYLE) != false, + buttonCornersRadius = theme?.double(THEME_BUTTON_CORNERS_RADIUS) + ?: DEFAULT_BUTTON_CORNERS_RADIUS, + config = json.deepCopy(), + ) + } + + internal fun slugify(name: String): String { + return Normalizer.normalize(name.lowercase(Locale.ROOT), Normalizer.Form.NFD) + .replace("\\p{Mn}+".toRegex(), "") + .replace("[^a-z0-9]+".toRegex(), "-") + .trim('-') + } + + private fun String.isAbsoluteUrl(): Boolean { + return try { + val uri = URI(this) + !uri.scheme.isNullOrEmpty() && !uri.host.isNullOrEmpty() + } catch (_: Exception) { + false + } + } + } +} + +/** + * Per-mode color overrides from an instance's `THEME` block. Keys are the shared palette + * field names (`accent_color`, `text_primary`, ...); values are hex strings. + */ +data class InstanceThemeColors( + val light: Map, + val dark: Map, +) + +internal fun JsonObject.primitive(key: String): JsonElement? { + val element = get(key) ?: return null + return if (element.isJsonPrimitive) element else null +} + +internal fun JsonObject.string(key: String): String? { + val element = primitive(key) ?: return null + return if (element.asJsonPrimitive.isString) element.asString else null +} + +internal fun JsonObject.nonBlankString(key: String): String? = string(key)?.takeIf { it.isNotBlank() } + +internal fun JsonObject.boolean(key: String): Boolean? { + val element = primitive(key) ?: return null + return if (element.asJsonPrimitive.isBoolean) element.asBoolean else null +} + +internal fun JsonObject.double(key: String): Double? { + val element = primitive(key) ?: return null + return if (element.asJsonPrimitive.isNumber) element.asDouble else null +} + +internal fun JsonObject.obj(key: String): JsonObject? { + val element = get(key) ?: return null + return if (element.isJsonObject) element.asJsonObject else null +} + +internal fun JsonObject.stringMap(key: String): Map { + val obj = obj(key) ?: return emptyMap() + return obj.entrySet() + .filter { (_, value) -> value.isJsonPrimitive && value.asJsonPrimitive.isString } + .associate { (entryKey, value) -> entryKey to value.asString } +} diff --git a/core/src/main/java/org/openedx/core/instance/InstancesConfig.kt b/core/src/main/java/org/openedx/core/instance/InstancesConfig.kt new file mode 100644 index 000000000..f96879b10 --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/InstancesConfig.kt @@ -0,0 +1,51 @@ +package org.openedx.core.instance + +import com.google.gson.JsonArray +import com.google.gson.JsonObject +import java.util.Locale + +/** + * The parsed instance catalog: every platform the app can sign in to, in catalog order. + */ +data class InstancesConfig( + val instances: List = emptyList(), +) { + + fun instance(withKey: String): Instance? = instances.firstOrNull { it.key == withKey } + + companion object { + const val INSTANCES = "INSTANCES" + + /** + * Builds the catalog from an `INSTANCES` array. Entries that aren't objects or fail + * [Instance.fromJson] are dropped. + * + * @param fallback values used for the fields an entry omits; the entry always wins. + */ + fun fromJson( + array: JsonArray, + fallback: JsonObject = JsonObject(), + languageCode: String = Locale.getDefault().language, + ): InstancesConfig { + val instances = array.mapNotNull { element -> + if (!element.isJsonObject) return@mapNotNull null + val merged = fallback.deepCopy() + element.asJsonObject.entrySet().forEach { (key, value) -> merged.add(key, value) } + Instance.fromJson(merged, languageCode) + } + return InstancesConfig(instances) + } + + /** The `INSTANCES` array of the bundled `config.json`, or an empty catalog. */ + fun fromAppConfig( + appConfig: JsonObject, + languageCode: String = Locale.getDefault().language, + ): InstancesConfig { + val array = appConfig.get(INSTANCES) + ?.takeIf { it.isJsonArray } + ?.asJsonArray + ?: return InstancesConfig() + return fromJson(array, languageCode = languageCode) + } + } +} diff --git a/core/src/test/java/org/openedx/core/instance/InstancesConfigTest.kt b/core/src/test/java/org/openedx/core/instance/InstancesConfigTest.kt new file mode 100644 index 000000000..75fa89e22 --- /dev/null +++ b/core/src/test/java/org/openedx/core/instance/InstancesConfigTest.kt @@ -0,0 +1,183 @@ +package org.openedx.core.instance + +import com.google.gson.JsonArray +import com.google.gson.JsonObject +import com.google.gson.JsonParser +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class InstancesConfigTest { + + private fun fixture(): JsonObject { + val stream = requireNotNull(javaClass.classLoader?.getResourceAsStream("instances/bundled_instances.json")) + return stream.reader().use { JsonParser.parseReader(it).asJsonObject } + } + + private fun requiredFields(): JsonObject = JsonObject().apply { + addProperty("KEY", "test-instance") + addProperty("NAME", "Test Instance") + addProperty("OAUTH_CLIENT_ID", "client-id") + addProperty("API_HOST_URL", "https://test-instance.example.com") + } + + @Test + fun `bundled catalog parses every field of a complete entry`() { + val catalog = InstancesConfig.fromAppConfig(fixture(), languageCode = "en") + val instance = requireNotNull(catalog.instance("example-instance")) + + assertEquals("Example Instance", instance.name) + assertEquals("Example Instance", instance.instanceName) + assertEquals("example-oauth-client-id", instance.oAuthClientId) + assertEquals("https://example-instance.example.com", instance.apiHostUrl) + assertEquals( + "https://example-instance.example.com/auth/login/tpa-saml/?auth_entry=login", + instance.ssoUrl + ) + assertEquals( + "https://example-instance.example.com/auth/login/tpa-saml/auth/complete/tpa-saml", + instance.ssoFinishedUrl + ) + assertEquals("Example Instance", instance.environmentDisplayName) + assertTrue(instance.isSwitchInstanceLoginEnabled) + assertEquals("exampleInstanceAppLogo", instance.logoUrl) + assertNull(instance.headerBackgroundUrl) + assertEquals(mapOf("accent_color" to "#3C68FF"), instance.themeColors?.light) + assertEquals(mapOf("accent_color" to "#1C355E"), instance.themeColors?.dark) + assertTrue(instance.config.has("UI_COMPONENTS")) + } + + @Test + fun `malformed entries are dropped instead of half populated`() { + val catalog = InstancesConfig.fromAppConfig(fixture(), languageCode = "en") + + assertEquals(listOf("example-instance", "second-universite"), catalog.instances.map { it.key }) + assertNull(catalog.instance("missing-client")) + } + + @Test + fun `each required field is enforced`() { + listOf("NAME", "API_HOST_URL", "OAUTH_CLIENT_ID").forEach { field -> + val json = requiredFields().apply { remove(field) } + assertNull("missing $field must be rejected", Instance.fromJson(json)) + } + val blankName = requiredFields().apply { addProperty("NAME", " ") } + assertNull(Instance.fromJson(blankName)) + val relativeHost = requiredFields().apply { addProperty("API_HOST_URL", "test-instance.example.com") } + assertNull(Instance.fromJson(relativeHost)) + val numericClient = requiredFields().apply { addProperty("OAUTH_CLIENT_ID", 42) } + assertNull(Instance.fromJson(numericClient)) + } + + @Test + fun `key falls back to a slug of the name`() { + val catalog = InstancesConfig.fromAppConfig(fixture(), languageCode = "en") + + assertNotNull(catalog.instance("second-universite")) + assertEquals("acme-u-of-arts", Instance.slugify(" ACME U. of Arts! ")) + } + + @Test + fun `display name follows the language then English then NAME`() { + assertEquals("اسم المُزود", InstancesConfig.fromAppConfig(fixture(), "ar").instances[0].instanceName) + assertEquals("Example Instance", InstancesConfig.fromAppConfig(fixture(), "uk").instances[0].instanceName) + assertEquals("Second Université", InstancesConfig.fromAppConfig(fixture(), "ar").instances[1].instanceName) + } + + @Test + fun `optional fields use defaults when omitted`() { + val instance = requireNotNull(Instance.fromJson(requiredFields())) + + assertEquals("#007AFF", instance.color) + assertEquals("Test Instance", instance.platformName) + assertNull(instance.ssoUrl) + assertNull(instance.ssoFinishedUrl) + assertNull(instance.themeColors) + assertNull(instance.logoUrl) + assertFalse(instance.isSwitchInstanceLoginEnabled) + assertTrue(instance.isRoundedCorners) + assertEquals(8.0, instance.buttonCornersRadius, 0.0) + } + + @Test + fun `blank optional strings count as unset`() { + val json = requiredFields().apply { + addProperty("SSO_URL", "") + addProperty("SSO_URL_SUCCESSFUL_LOGIN", " ") + addProperty("PLATFORM_NAME", "") + add("THEME", JsonParser.parseString("""{"LOGO_URL": "", "HEADER_BACKGROUND_URL": ""}""")) + } + val instance = requireNotNull(Instance.fromJson(json)) + + assertNull(instance.ssoUrl) + assertNull(instance.ssoFinishedUrl) + assertNull(instance.logoUrl) + assertNull(instance.headerBackgroundUrl) + assertEquals("Test Instance", instance.platformName) + } + + @Test + fun `instance level values win when provided`() { + val json = requiredFields().apply { + addProperty("PLATFORM_NAME", "Custom Platform Name") + addProperty("SSO_FINISHED_URL", "https://test-instance.example.com/finished") + addProperty("SSO_URL_SUCCESSFUL_LOGIN", "https://test-instance.example.com/ignored") + add("THEME", JsonParser.parseString("""{"ROUNDED_CORNERS_STYLE": false, "BUTTON_CORNERS_RADIUS": 4}""")) + } + val instance = requireNotNull(Instance.fromJson(json)) + + assertEquals("Custom Platform Name", instance.platformName) + assertEquals("https://test-instance.example.com/finished", instance.ssoFinishedUrl) + assertFalse(instance.isRoundedCorners) + assertEquals(4.0, instance.buttonCornersRadius, 0.0) + assertEquals(InstanceThemeColors(emptyMap(), emptyMap()), instance.themeColors) + } + + @Test + fun `fallback fills omitted fields and never overrides the entry`() { + val fallback = JsonObject().apply { + addProperty("OAUTH_CLIENT_ID", "fallback-client") + addProperty("FEEDBACK_EMAIL_ADDRESS", "fallback@example.com") + } + val entry = requiredFields().apply { remove("OAUTH_CLIENT_ID") } + val withClient = requiredFields().apply { addProperty("KEY", "with-client") } + val array = JsonArray().apply { + add(entry) + add(withClient) + } + + val catalog = InstancesConfig.fromJson(array, fallback) + + assertEquals("fallback-client", catalog.instance("test-instance")?.oAuthClientId) + assertEquals("client-id", catalog.instance("with-client")?.oAuthClientId) + val feedbackEmail = catalog.instance("with-client")?.config?.get("FEEDBACK_EMAIL_ADDRESS")?.asString + assertEquals("fallback@example.com", feedbackEmail) + assertFalse(fallback.has("KEY")) + } + + @Test + fun `identity is the key, not the position in the catalog`() { + val fixture = fixture() + val reversed = JsonArray().apply { + fixture.getAsJsonArray("INSTANCES").reversed().forEach { add(it) } + } + + val original = InstancesConfig.fromAppConfig(fixture, "en") + val reordered = InstancesConfig.fromJson(reversed, languageCode = "en") + + assertEquals(original.instance("example-instance"), reordered.instance("example-instance")) + assertEquals(original.instance("second-universite"), reordered.instance("second-universite")) + } + + @Test + fun `missing or non array INSTANCES gives an empty catalog`() { + assertTrue(InstancesConfig.fromAppConfig(JsonObject()).instances.isEmpty()) + val notArray = JsonObject().apply { addProperty("INSTANCES", "nope") } + assertTrue(InstancesConfig.fromAppConfig(notArray).instances.isEmpty()) + val junk = JsonObject().apply { add("INSTANCES", JsonParser.parseString("""[1, "two", null]""")) } + assertTrue(InstancesConfig.fromAppConfig(junk).instances.isEmpty()) + } +} diff --git a/core/src/test/resources/instances/bundled_instances.json b/core/src/test/resources/instances/bundled_instances.json new file mode 100644 index 000000000..da32373fb --- /dev/null +++ b/core/src/test/resources/instances/bundled_instances.json @@ -0,0 +1,45 @@ +{ + "API_HOST_URL": "https://example-instance.example.com", + "OAUTH_CLIENT_ID": "example-oauth-client-id", + "INSTANCES": [ + { + "KEY": "example-instance", + "NAME": "Example Instance", + "INSTANCE_NAME": { + "ar": "اسم المُزود", + "en": "Example Instance" + }, + "ENVIRONMENT_DISPLAY_NAME": "Example Instance", + "OAUTH_CLIENT_ID": "example-oauth-client-id", + "API_HOST_URL": "https://example-instance.example.com", + "SSO_URL": "https://example-instance.example.com/auth/login/tpa-saml/?auth_entry=login", + "SSO_URL_SUCCESSFUL_LOGIN": "https://example-instance.example.com/auth/login/tpa-saml/auth/complete/tpa-saml", + "IS_SWITCH_INSTANCE_LOGIN_ENABLED": true, + "THEME": { + "LIGHT": { + "accent_color": "#3C68FF" + }, + "DARK": { + "accent_color": "#1C355E" + }, + "LOGO_URL": "exampleInstanceAppLogo" + }, + "UI_COMPONENTS": { + "COURSE_UNIT_PROGRESS_ENABLED": false, + "LOGIN_REGISTRATION_ENABLED": true, + "SAML_SSO_LOGIN_ENABLED": false, + "SAML_SSO_DEFAULT_LOGIN_BUTTON": false + } + }, + { + "NAME": "Second Université", + "OAUTH_CLIENT_ID": "second-client-id", + "API_HOST_URL": "https://second.example.com:8443" + }, + { + "KEY": "missing-client", + "NAME": "Missing Client", + "API_HOST_URL": "https://missing-client.example.com" + } + ] +} diff --git a/default_config/dev/config.yaml b/default_config/dev/config.yaml index f2868eb78..97db15191 100644 --- a/default_config/dev/config.yaml +++ b/default_config/dev/config.yaml @@ -94,3 +94,52 @@ UI_COMPONENTS: COURSE_DROPDOWN_NAVIGATION_ENABLED: false COURSE_UNIT_PROGRESS_ENABLED: false COURSE_DOWNLOAD_QUEUE_SCREEN: false + +# Learning sites ("instances") this build can sign in to. One entry: the app behaves like a +# single-site build, with no site picker. Several entries: the learner picks a site and can stay +# signed in to more than one. NAME, API_HOST_URL and OAUTH_CLIENT_ID are required; every other key +# is optional and falls back to the app-level value above. +INSTANCES: + - KEY: 'localhost' + NAME: 'Localhost' + INSTANCE_NAME: + en: 'Localhost' + ENVIRONMENT_DISPLAY_NAME: 'Localhost' + API_HOST_URL: 'http://localhost:8000' + OAUTH_CLIENT_ID: 'OAUTH_CLIENT_ID' + SSO_URL: '' + SSO_URL_SUCCESSFUL_LOGIN: '' + IS_SWITCH_INSTANCE_LOGIN_ENABLED: false + FEEDBACK_EMAIL_ADDRESS: 'support@example.com' + FAQ_URL: '' + PLATFORM_NAME: 'OpenEdX' + TOKEN_TYPE: 'JWT' + THEME: + LIGHT: + accent_color: '#3C68FF' + DARK: + accent_color: '#3C68FF' + LOGO_URL: '' + HEADER_BACKGROUND_URL: '' + UI_COMPONENTS: + COURSE_DROPDOWN_NAVIGATION_ENABLED: false + COURSE_UNIT_PROGRESS_ENABLED: false + COURSE_DOWNLOAD_QUEUE_SCREEN: false + AGREEMENT_URLS: + PRIVACY_POLICY_URL: '' + COOKIE_POLICY_URL: '' + DATA_SELL_CONSENT_URL: '' + TOS_URL: '' + EULA_URL: '' + SUPPORTED_LANGUAGES: [ ] + DISCOVERY: + TYPE: 'native' + PROGRAM: + TYPE: 'native' + DASHBOARD: + TYPE: 'gallery' + EXPERIMENTAL_FEATURES: + APP_LEVEL_DOWNLOADS: + ENABLED: false + APP_LEVEL_DATES: + ENABLED: false From b15a3e7d7fc5a7a6332cbf94d9312576ee87b48a Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:55:02 +0300 Subject: [PATCH 2/7] feat: add instance selection store (InstanceStore) InstanceStore holds the instance catalog and the selected instance, persists the selection by key and publishes it as a StateFlow. A single-instance catalog selects its only instance automatically. Registered in Koin as InstanceStore and InstanceProvider; nothing consumes it yet. Mirrors iOS #673. --- .../main/java/org/openedx/app/di/AppModule.kt | 10 ++ .../java/org/openedx/core/config/Config.kt | 6 + .../openedx/core/instance/InstanceProvider.kt | 17 +++ .../core/instance/InstanceSelectionStorage.kt | 29 ++++ .../openedx/core/instance/InstanceStore.kt | 74 ++++++++++ .../core/instance/InstanceStoreTest.kt | 135 ++++++++++++++++++ 6 files changed, 271 insertions(+) create mode 100644 core/src/main/java/org/openedx/core/instance/InstanceProvider.kt create mode 100644 core/src/main/java/org/openedx/core/instance/InstanceSelectionStorage.kt create mode 100644 core/src/main/java/org/openedx/core/instance/InstanceStore.kt create mode 100644 core/src/test/java/org/openedx/core/instance/InstanceStoreTest.kt diff --git a/app/src/main/java/org/openedx/app/di/AppModule.kt b/app/src/main/java/org/openedx/app/di/AppModule.kt index 267b73432..64dbc6209 100644 --- a/app/src/main/java/org/openedx/app/di/AppModule.kt +++ b/app/src/main/java/org/openedx/app/di/AppModule.kt @@ -35,6 +35,10 @@ import org.openedx.core.data.storage.CalendarPreferences import org.openedx.core.data.storage.CorePreferences import org.openedx.core.data.storage.InAppReviewPreferences import org.openedx.core.domain.helper.VideoPreviewHelper +import org.openedx.core.instance.InstanceProvider +import org.openedx.core.instance.InstanceSelectionStorage +import org.openedx.core.instance.InstanceStore +import org.openedx.core.instance.SharedPrefsInstanceSelectionStorage import org.openedx.core.module.DownloadWorkerController import org.openedx.core.module.TranscriptManager import org.openedx.core.module.download.DownloadHelper @@ -87,6 +91,12 @@ import org.openedx.core.DatabaseManager as IDatabaseManager val appModule = module { single { Config(get()) } + single { SharedPrefsInstanceSelectionStorage(get()) } + single { + val bundledInstances by lazy { get().getInstancesConfig() } + InstanceStore(get()) { bundledInstances } + } + single { get() } single { PreferencesManager(get(), get()) } single { get() } single { get() } diff --git a/core/src/main/java/org/openedx/core/config/Config.kt b/core/src/main/java/org/openedx/core/config/Config.kt index 1f07f43ad..52ed2cfae 100644 --- a/core/src/main/java/org/openedx/core/config/Config.kt +++ b/core/src/main/java/org/openedx/core/config/Config.kt @@ -6,6 +6,7 @@ import com.google.gson.JsonElement import com.google.gson.JsonObject import com.google.gson.JsonParser import org.openedx.core.domain.model.AgreementUrls +import org.openedx.core.instance.InstancesConfig import java.io.InputStreamReader @Suppress("TooManyFunctions") @@ -146,6 +147,11 @@ class Config(context: Context) { return getBoolean(REGISTRATION_ENABLED, true) } + /** The bundled `INSTANCES` catalog, before any remote catalog replaces it. */ + fun getInstancesConfig(): InstancesConfig { + return InstancesConfig.fromAppConfig(configProperties) + } + private fun getExperimentalFeaturesConfig(): ExperimentalFeaturesConfig { return getObjectOrNewInstance(EXPERIMENTAL_FEATURES, ExperimentalFeaturesConfig::class.java) } diff --git a/core/src/main/java/org/openedx/core/instance/InstanceProvider.kt b/core/src/main/java/org/openedx/core/instance/InstanceProvider.kt new file mode 100644 index 000000000..12b96dba6 --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/InstanceProvider.kt @@ -0,0 +1,17 @@ +package org.openedx.core.instance + +import kotlinx.coroutines.flow.StateFlow + +/** + * Read-only, thread-safe access to the selected instance. + */ +interface InstanceProvider { + /** `null` when nothing is selected, e.g. a build with no `INSTANCES` catalog. */ + val currentInstance: Instance? + + /** Emits the selected instance on every change, including deselection. */ + val currentInstanceFlow: StateFlow + + /** Whether the catalog has more than one instance to switch between. */ + val hasMultipleInstances: Boolean +} diff --git a/core/src/main/java/org/openedx/core/instance/InstanceSelectionStorage.kt b/core/src/main/java/org/openedx/core/instance/InstanceSelectionStorage.kt new file mode 100644 index 000000000..91f2b77b3 --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/InstanceSelectionStorage.kt @@ -0,0 +1,29 @@ +package org.openedx.core.instance + +import android.content.Context +import androidx.core.content.edit + +/** + * Where [InstanceStore] persists the key of the selected instance. + */ +interface InstanceSelectionStorage { + var selectedInstanceKey: String? +} + +class SharedPrefsInstanceSelectionStorage(context: Context) : InstanceSelectionStorage { + + private val prefs = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) + + // Written synchronously: a selection lost to a process kill right after a switch would + // reopen the app on the previous site. + override var selectedInstanceKey: String? + get() = prefs.getString(KEY_SELECTED_INSTANCE, null) + set(value) = prefs.edit(commit = true) { + if (value == null) remove(KEY_SELECTED_INSTANCE) else putString(KEY_SELECTED_INSTANCE, value) + } + + private companion object { + const val PREFS_NAME = "openedx_instance_store" + const val KEY_SELECTED_INSTANCE = "selected_instance_key" + } +} diff --git a/core/src/main/java/org/openedx/core/instance/InstanceStore.kt b/core/src/main/java/org/openedx/core/instance/InstanceStore.kt new file mode 100644 index 000000000..f2a2b8c2e --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/InstanceStore.kt @@ -0,0 +1,74 @@ +package org.openedx.core.instance + +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow + +/** + * Single source of truth for the instance catalog and the selected instance. + * + * The selection is persisted by key, so it survives restarts and catalog reordering. A + * catalog with exactly one instance selects it automatically: there is no picker to choose + * it through. + */ +class InstanceStore( + private val storage: InstanceSelectionStorage, + private val localInstancesConfig: () -> InstancesConfig = { InstancesConfig() }, +) : InstanceProvider { + + private val lock = Any() + private var remoteInstancesConfig: InstancesConfig? = null + private val _currentInstance = MutableStateFlow(null) + + override val currentInstance: Instance? + get() = _currentInstance.value + + override val currentInstanceFlow: StateFlow = _currentInstance.asStateFlow() + + override val hasMultipleInstances: Boolean + get() = instancesConfig.instances.size > 1 + + /** The catalog in effect: the one handed to [updateInstancesConfig], else the bundled one. */ + val instancesConfig: InstancesConfig + get() = synchronized(lock) { remoteInstancesConfig ?: localInstancesConfig() } + + init { + restoreSelection() + } + + fun instance(withKey: String): Instance? = instancesConfig.instance(withKey) + + /** Replaces the catalog in effect and re-resolves the persisted selection against it. */ + fun updateInstancesConfig(instancesConfig: InstancesConfig) { + synchronized(lock) { remoteInstancesConfig = instancesConfig } + restoreSelection() + } + + /** Selects [instance], or clears the selection with `null`, and persists the choice. */ + fun select(instance: Instance?) { + synchronized(lock) { + storage.selectedInstanceKey = instance?.key + _currentInstance.value = instance + } + // A deselect on a single-instance catalog must not leave nothing selected until the + // next launch. + if (instance == null) { + restoreSelection() + } + } + + fun reset() = select(null) + + private fun restoreSelection() { + val catalog = instancesConfig + val resolved = storage.selectedInstanceKey?.let { catalog.instance(it) } + val onlyInstance = catalog.instances.singleOrNull() + if (resolved == null && onlyInstance != null) { + select(onlyInstance) + return + } + // A saved key that the catalog no longer contains resolves to nothing, but stays saved: + // if a later catalog brings that site back, it is selected again. + _currentInstance.value = resolved + } +} diff --git a/core/src/test/java/org/openedx/core/instance/InstanceStoreTest.kt b/core/src/test/java/org/openedx/core/instance/InstanceStoreTest.kt new file mode 100644 index 000000000..f03a9633a --- /dev/null +++ b/core/src/test/java/org/openedx/core/instance/InstanceStoreTest.kt @@ -0,0 +1,135 @@ +package org.openedx.core.instance + +import com.google.gson.JsonObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class InstanceStoreTest { + + private class InMemorySelectionStorage(override var selectedInstanceKey: String? = null) : + InstanceSelectionStorage + + private fun instance(key: String) = requireNotNull( + Instance.fromJson( + JsonObject().apply { + addProperty("KEY", key) + addProperty("NAME", key.uppercase()) + addProperty("OAUTH_CLIENT_ID", "$key-client") + addProperty("API_HOST_URL", "https://$key.example.com") + } + ) + ) + + private val alpha = instance("alpha") + private val beta = instance("beta") + + private fun store( + storage: InstanceSelectionStorage, + vararg instances: Instance, + ) = InstanceStore(storage) { InstancesConfig(instances.toList()) } + + @Test + fun `single instance catalog with no prior selection auto selects it`() { + val storage = InMemorySelectionStorage() + + val store = store(storage, alpha) + + assertEquals(alpha, store.currentInstance) + assertEquals("alpha", storage.selectedInstanceKey) + assertFalse(store.hasMultipleInstances) + } + + @Test + fun `multi instance catalog with no prior selection selects nothing`() { + val store = store(InMemorySelectionStorage(), alpha, beta) + + assertNull(store.currentInstance) + assertTrue(store.hasMultipleInstances) + } + + @Test + fun `persisted selection is restored by key`() { + val store = store(InMemorySelectionStorage("beta"), alpha, beta) + + assertEquals(beta, store.currentInstance) + } + + @Test + fun `single instance catalog replaces a stale saved key`() { + val storage = InMemorySelectionStorage("gone") + + val store = store(storage, alpha) + + assertEquals(alpha, store.currentInstance) + assertEquals("alpha", storage.selectedInstanceKey) + } + + @Test + fun `empty catalog leaves nothing selected`() { + val store = store(InMemorySelectionStorage()) + + assertNull(store.currentInstance) + assertFalse(store.hasMultipleInstances) + } + + @Test + fun `deselecting a single instance catalog selects it again`() { + val store = store(InMemorySelectionStorage(), alpha) + + store.select(null) + + assertEquals(alpha, store.currentInstance) + } + + @Test + fun `deselecting a multi instance catalog stays deselected`() { + val storage = InMemorySelectionStorage() + val store = store(storage, alpha, beta) + store.select(beta) + + store.reset() + + assertNull(store.currentInstance) + assertNull(storage.selectedInstanceKey) + } + + @Test + fun `selection is published to the flow`() { + val store = store(InMemorySelectionStorage(), alpha, beta) + + store.select(alpha) + assertEquals(alpha, store.currentInstanceFlow.value) + store.select(beta) + assertEquals(beta, store.currentInstanceFlow.value) + } + + @Test + fun `a new catalog re-resolves the saved selection`() { + val storage = InMemorySelectionStorage("beta") + val store = store(storage, alpha) + assertEquals(alpha, store.currentInstance) + + storage.selectedInstanceKey = "beta" + store.updateInstancesConfig(InstancesConfig(listOf(beta, alpha))) + + assertEquals(beta, store.currentInstance) + assertEquals(listOf(beta, alpha), store.instancesConfig.instances) + } + + @Test + fun `a site removed from the catalog resolves to nothing but stays saved`() { + val storage = InMemorySelectionStorage() + val store = store(storage, alpha, beta) + store.select(beta) + + store.updateInstancesConfig(InstancesConfig(listOf(alpha, instance("gamma")))) + assertNull(store.currentInstance) + assertEquals("beta", storage.selectedInstanceKey) + + store.updateInstancesConfig(InstancesConfig(listOf(alpha, beta))) + assertEquals(beta, store.currentInstance) + } +} From 16e15deee1058c60a4dbc519e8b223978c5ceb80 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:57:25 +0300 Subject: [PATCH 3/7] feat: add per-instance theme engine InstanceTheme derives a palette from an instance's accent color and its optional THEME.LIGHT/THEME.DARK overrides: only the fields an instance sets change, the rest keep the compiled default, and a missing dark value falls back to the light one. Palette field names are the ones iOS uses (#675), mapped onto AppColors. OpenEdXTheme observes InstanceThemeManager, which holds the selected instance's theme. Nothing applies a theme yet, so every screen still renders the default palette. --- .../openedx/core/ui/theme/InstanceTheme.kt | 124 ++++++++++++++++++ .../java/org/openedx/core/ui/theme/Theme.kt | 15 ++- .../core/ui/theme/InstanceThemeTest.kt | 107 +++++++++++++++ 3 files changed, 240 insertions(+), 6 deletions(-) create mode 100644 core/src/main/java/org/openedx/core/ui/theme/InstanceTheme.kt create mode 100644 core/src/test/java/org/openedx/core/ui/theme/InstanceThemeTest.kt diff --git a/core/src/main/java/org/openedx/core/ui/theme/InstanceTheme.kt b/core/src/main/java/org/openedx/core/ui/theme/InstanceTheme.kt new file mode 100644 index 000000000..c9bb67550 --- /dev/null +++ b/core/src/main/java/org/openedx/core/ui/theme/InstanceTheme.kt @@ -0,0 +1,124 @@ +package org.openedx.core.ui.theme + +import androidx.compose.ui.graphics.Color +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow + +/** + * Branding of one instance: an accent color plus optional per-field palette overrides for + * light and dark mode, and an optional logo and header image. + * + * Takes primitives rather than the instance model, so the theme layer stays independent of + * how instances are configured. + */ +data class InstanceTheme( + /** Stable instance key. Never a display name, which can be renamed or localized. */ + val key: String, + val accentColorHex: String? = null, + val light: Map = emptyMap(), + val dark: Map = emptyMap(), + /** Remote URL, or the name of a bundled drawable. */ + val logoUrl: String? = null, + /** Remote URL, or the name of a bundled drawable. */ + val headerBackgroundUrl: String? = null, +) { + + /** + * The default palette with this instance's colors layered on top. Only the fields the + * instance sets change; everything else keeps the compiled value. A dark override falls + * back to the light one for the same field. + */ + fun applyTo(base: AppColors, isDark: Boolean): AppColors { + val modeAccent = if (isDark) dark[ACCENT_COLOR] ?: light[ACCENT_COLOR] else light[ACCENT_COLOR] + var colors = (modeAccent ?: accentColorHex)?.toColorOrNull()?.let(base::withAccent) ?: base + light.forEach { (field, lightHex) -> + if (field == ACCENT_COLOR) return@forEach + val hex = if (isDark) dark[field] ?: lightHex else lightHex + val color = hex.toColorOrNull() ?: return@forEach + colors = paletteFields[field]?.invoke(colors, color) ?: colors + } + return colors + } + + companion object { + const val ACCENT_COLOR = "accent_color" + + /** + * Palette field names shared with the iOS app, mapped to the colors they override here. + * Unknown names are ignored rather than treated as errors. + */ + internal val paletteFields: Map AppColors> = mapOf( + "accent_x_color" to { c, v -> c.copy(material3 = c.material3.copy(secondary = v)) }, + "alert" to { c, v -> c.copy(material3 = c.material3.copy(error = v)) }, + "background" to { c, v -> c.copy(material3 = c.material3.copy(background = v)) }, + "card_view_background" to { c, v -> c.copy(cardViewBackground = v) }, + "card_view_stroke" to { c, v -> c.copy(cardViewBorder = v) }, + "certificate_foreground" to { c, v -> c.copy(certificateForeground = v) }, + "text_primary" to { c, v -> c.copy(textPrimary = v) }, + "text_secondary" to { c, v -> c.copy(textSecondary = v) }, + "text_input_background" to { c, v -> c.copy(textFieldBackground = v) }, + "text_input_stroke" to { c, v -> c.copy(textFieldBorder = v) }, + "text_input_text_color" to { c, v -> c.copy(textFieldText = v) }, + "text_input_placeholder_color" to { c, v -> c.copy(textFieldHint = v) }, + "warning" to { c, v -> c.copy(warning = v) }, + "success" to { c, v -> c.copy(successGreen = v) }, + "info_color" to { c, v -> c.copy(info = v) }, + "primary_button_text_color" to { c, v -> c.copy(primaryButtonText = v) }, + "secondary_button_border_color" to { c, v -> c.copy(secondaryButtonBorder = v) }, + "secondary_button_text_color" to { c, v -> c.copy(secondaryButtonBorderedText = v) }, + "past_due_timeline_color" to { c, v -> c.copy(datesSectionBarPastDue = v) }, + "today_timeline_color" to { c, v -> c.copy(datesSectionBarToday = v) }, + "this_week_timeline_color" to { c, v -> c.copy(datesSectionBarThisWeek = v) }, + "next_week_timeline_color" to { c, v -> c.copy(datesSectionBarNextWeek = v) }, + "upcoming_timeline_color" to { c, v -> c.copy(datesSectionBarUpcoming = v) }, + ) + } +} + +/** + * The accent drives every color the default palette derives from `primary`: links, primary + * buttons, bordered button text and the unselected tab content. + */ +internal fun AppColors.withAccent(accent: Color): AppColors = copy( + material3 = material3.copy(primary = accent), + textAccent = accent, + textHyperLink = accent, + infoVariant = accent, + primaryButtonBackground = accent, + primaryButtonBorderedText = accent, + secondaryButtonBackground = accent, + secondaryButtonBorderedText = accent, + tabUnselectedBtnContent = accent, +) + +/** Parses `#RGB`, `#RRGGBB` or `#AARRGGBB`. */ +@Suppress("MagicNumber") +internal fun String.toColorOrNull(): Color? { + val hex = trim().removePrefix("#") + val argb = when (hex.length) { + 3 -> "FF" + hex.map { "$it$it" }.joinToString("") + 6 -> "FF$hex" + 8 -> hex + else -> null + } + return argb + ?.takeIf { value -> value.all { Character.digit(it, 16) >= 0 } } + ?.let { Color(it.toLong(16)) } +} + +/** + * Holds the theme of the selected instance; `null` means the compiled default theme. + * [OpenEdXTheme] observes it, so applying a theme restyles every screen. + */ +object InstanceThemeManager { + + private val _theme = MutableStateFlow(null) + val theme: StateFlow = _theme.asStateFlow() + + fun applyTheme(theme: InstanceTheme?) { + _theme.value = theme + } + + fun reset() = applyTheme(null) +} diff --git a/core/src/main/java/org/openedx/core/ui/theme/Theme.kt b/core/src/main/java/org/openedx/core/ui/theme/Theme.kt index ec7997c72..bdeb480f6 100644 --- a/core/src/main/java/org/openedx/core/ui/theme/Theme.kt +++ b/core/src/main/java/org/openedx/core/ui/theme/Theme.kt @@ -9,13 +9,16 @@ import androidx.compose.material3.lightColorScheme import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.ReadOnlyComposable +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember import androidx.compose.runtime.staticCompositionLocalOf internal val LocalAppColors = staticCompositionLocalOf { error("No AppColors provided") } -private val DarkColorPalette = AppColors( +internal val DarkColorPalette = AppColors( material3 = darkColorScheme( primary = dark_primary, onPrimary = dark_onPrimary, @@ -115,7 +118,7 @@ private val DarkColorPalette = AppColors( assignmentCardBorder = dark_assignment_card_border, ) -private val LightColorPalette = AppColors( +internal val LightColorPalette = AppColors( material3 = lightColorScheme( primary = light_primary, onPrimary = light_onPrimary, @@ -223,10 +226,10 @@ val MaterialTheme.appColors: AppColors @OptIn(ExperimentalFoundationApi::class) @Composable fun OpenEdXTheme(darkTheme: Boolean = isSystemInDarkTheme(), content: @Composable () -> Unit) { - val colors = if (darkTheme) { - DarkColorPalette - } else { - LightColorPalette + val instanceTheme by InstanceThemeManager.theme.collectAsState() + val colors = remember(darkTheme, instanceTheme) { + val base = if (darkTheme) DarkColorPalette else LightColorPalette + instanceTheme?.applyTo(base, isDark = darkTheme) ?: base } MaterialTheme( diff --git a/core/src/test/java/org/openedx/core/ui/theme/InstanceThemeTest.kt b/core/src/test/java/org/openedx/core/ui/theme/InstanceThemeTest.kt new file mode 100644 index 000000000..5518bca2e --- /dev/null +++ b/core/src/test/java/org/openedx/core/ui/theme/InstanceThemeTest.kt @@ -0,0 +1,107 @@ +package org.openedx.core.ui.theme + +import androidx.compose.ui.graphics.Color +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Test + +class InstanceThemeTest { + + private val light = LightColorPalette + private val dark = DarkColorPalette + + @Test + fun `no palette derives only the accent colors from the hex`() { + val colors = InstanceTheme(key = "acme", accentColorHex = "#3E578C").applyTo(light, isDark = false) + val accent = Color(0xFF3E578C) + + assertEquals(accent, colors.primary) + assertEquals(accent, colors.textAccent) + assertEquals(accent, colors.textHyperLink) + assertEquals(accent, colors.primaryButtonBackground) + assertEquals(accent, colors.primaryButtonBorderedText) + assertEquals(light.textPrimary, colors.textPrimary) + assertEquals(light.background, colors.background) + } + + @Test + fun `partial palette overrides only the fields it sets`() { + val baseline = InstanceTheme(key = "acme", accentColorHex = "#3E578C").applyTo(light, isDark = false) + val overridden = InstanceTheme( + key = "acme", + accentColorHex = "#3E578C", + light = mapOf("text_primary" to "#18202E"), + ).applyTo(light, isDark = false) + + assertEquals(Color(0xFF18202E), overridden.textPrimary) + assertEquals(baseline.primary, overridden.primary) + assertEquals(baseline.background, overridden.background) + assertEquals(baseline.cardViewBackground, overridden.cardViewBackground) + } + + @Test + fun `palette accent wins over the flat color, per mode`() { + val theme = InstanceTheme( + key = "acme", + accentColorHex = "#007AFF", + light = mapOf("accent_color" to "#3C68FF"), + dark = mapOf("accent_color" to "#1C355E"), + ) + + assertEquals(Color(0xFF3C68FF), theme.applyTo(light, isDark = false).primary) + assertEquals(Color(0xFF1C355E), theme.applyTo(dark, isDark = true).primary) + } + + @Test + fun `missing dark value falls back to the light value`() { + val theme = InstanceTheme(key = "acme", light = mapOf("alert" to "#FF3D71", "accent_color" to "#3C68FF")) + + assertEquals(Color(0xFFFF3D71), theme.applyTo(dark, isDark = true).error) + assertEquals(Color(0xFF3C68FF), theme.applyTo(dark, isDark = true).primary) + } + + @Test + fun `light and dark values resolve independently`() { + val theme = InstanceTheme( + key = "acme", + light = mapOf("background" to "#FFFFFF"), + dark = mapOf("background" to "#0A0A0A"), + ) + + assertEquals(Color(0xFFFFFFFF), theme.applyTo(light, isDark = false).background) + assertEquals(Color(0xFF0A0A0A), theme.applyTo(dark, isDark = true).background) + } + + @Test + fun `unknown fields and bad hex values are ignored`() { + val theme = InstanceTheme( + key = "acme", + accentColorHex = "not-a-color", + light = mapOf("not_a_real_field" to "#000000", "text_primary" to "#12"), + ) + + assertEquals(light, theme.applyTo(light, isDark = false)) + } + + @Test + fun `hex parsing accepts short, long and alpha forms`() { + assertEquals(Color(0xFFFF0000), "#F00".toColorOrNull()) + assertEquals(Color(0xFF3C68FF), " 3C68FF ".toColorOrNull()) + assertEquals(Color(0x803C68FF), "#803C68FF".toColorOrNull()) + assertNull("#GG0000".toColorOrNull()) + assertNull("-12345".toColorOrNull()) + assertNull("".toColorOrNull()) + } + + @Test + fun `manager publishes and resets the theme`() { + val theme = InstanceTheme(key = "acme") + + InstanceThemeManager.applyTheme(theme) + assertSame(theme, InstanceThemeManager.theme.value) + + InstanceThemeManager.reset() + assertNull(InstanceThemeManager.theme.value) + } +} From b8ef84ae1f106778d704ac787215626c73171672 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Mon, 19 Oct 2026 18:00:41 +0300 Subject: [PATCH 4/7] feat: fetch the instance catalog from a remote URL InstanceConfigLoader resolves the catalog the app runs with: the last cached remote catalog, else the bundled INSTANCES list, else nothing. A live fetch with at least one instance replaces that baseline and becomes the cache; a failed, malformed or empty response leaves it untouched. The catalog is fetched from INSTANCES_CATALOG_URL with a plain OkHttp client: no app interceptors, no credentials, a 10 second timeout. Remote snake_case keys are normalized with the same map iOS uses (#676). Nothing calls the loader at launch yet. Unlike iOS, the YAML build configuration stays: the Gradle build already turns it into the bundled config.json the loader reads. --- .../main/java/org/openedx/app/di/AppModule.kt | 8 + .../java/org/openedx/core/config/Config.kt | 6 + .../core/instance/InstanceApiService.kt | 51 ++++ .../core/instance/InstanceConfigLoader.kt | 76 ++++++ .../core/instance/InstancesJsonException.kt | 4 + .../core/instance/InstancesRemoteJson.kt | 77 ++++++ .../core/instance/InstanceConfigLoaderTest.kt | 256 ++++++++++++++++++ .../resources/instances/remote_catalog.json | 34 +++ default_config/dev/config.yaml | 2 + default_config/prod/config.yaml | 2 + default_config/stage/config.yaml | 2 + 11 files changed, 518 insertions(+) create mode 100644 core/src/main/java/org/openedx/core/instance/InstanceApiService.kt create mode 100644 core/src/main/java/org/openedx/core/instance/InstanceConfigLoader.kt create mode 100644 core/src/main/java/org/openedx/core/instance/InstancesJsonException.kt create mode 100644 core/src/main/java/org/openedx/core/instance/InstancesRemoteJson.kt create mode 100644 core/src/test/java/org/openedx/core/instance/InstanceConfigLoaderTest.kt create mode 100644 core/src/test/resources/instances/remote_catalog.json diff --git a/app/src/main/java/org/openedx/app/di/AppModule.kt b/app/src/main/java/org/openedx/app/di/AppModule.kt index 64dbc6209..a713834c1 100644 --- a/app/src/main/java/org/openedx/app/di/AppModule.kt +++ b/app/src/main/java/org/openedx/app/di/AppModule.kt @@ -35,9 +35,14 @@ import org.openedx.core.data.storage.CalendarPreferences import org.openedx.core.data.storage.CorePreferences import org.openedx.core.data.storage.InAppReviewPreferences import org.openedx.core.domain.helper.VideoPreviewHelper +import org.openedx.core.instance.InstanceApiService +import org.openedx.core.instance.InstanceCatalogCache +import org.openedx.core.instance.InstanceConfigLoader import org.openedx.core.instance.InstanceProvider import org.openedx.core.instance.InstanceSelectionStorage import org.openedx.core.instance.InstanceStore +import org.openedx.core.instance.OkHttpInstanceApiService +import org.openedx.core.instance.SharedPrefsInstanceCatalogCache import org.openedx.core.instance.SharedPrefsInstanceSelectionStorage import org.openedx.core.module.DownloadWorkerController import org.openedx.core.module.TranscriptManager @@ -97,6 +102,9 @@ val appModule = module { InstanceStore(get()) { bundledInstances } } single { get() } + single { SharedPrefsInstanceCatalogCache(get()) } + single { OkHttpInstanceApiService(get().getInstancesCatalogUrl()) } + single { InstanceConfigLoader(get(), get(), { get().getInstancesConfig() }) } single { PreferencesManager(get(), get()) } single { get() } single { get() } diff --git a/core/src/main/java/org/openedx/core/config/Config.kt b/core/src/main/java/org/openedx/core/config/Config.kt index 52ed2cfae..6982c2854 100644 --- a/core/src/main/java/org/openedx/core/config/Config.kt +++ b/core/src/main/java/org/openedx/core/config/Config.kt @@ -147,6 +147,11 @@ class Config(context: Context) { return getBoolean(REGISTRATION_ENABLED, true) } + /** Where the remote instance catalog is fetched from; empty means bundled catalog only. */ + fun getInstancesCatalogUrl(): String { + return getString(INSTANCES_CATALOG_URL, "") + } + /** The bundled `INSTANCES` catalog, before any remote catalog replaces it. */ fun getInstancesConfig(): InstancesConfig { return InstancesConfig.fromAppConfig(configProperties) @@ -223,6 +228,7 @@ class Config(context: Context) { private const val BRANCH = "BRANCH" private const val UI_COMPONENTS = "UI_COMPONENTS" private const val PLATFORM_NAME = "PLATFORM_NAME" + private const val INSTANCES_CATALOG_URL = "INSTANCES_CATALOG_URL" } enum class ViewType { diff --git a/core/src/main/java/org/openedx/core/instance/InstanceApiService.kt b/core/src/main/java/org/openedx/core/instance/InstanceApiService.kt new file mode 100644 index 000000000..a97f7a28a --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/InstanceApiService.kt @@ -0,0 +1,51 @@ +package org.openedx.core.instance + +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import okhttp3.CacheControl +import okhttp3.OkHttpClient +import okhttp3.Request +import java.io.IOException +import java.util.concurrent.TimeUnit + +interface InstanceApiService { + /** The catalog's raw JSON. Throws when the request fails or no catalog URL is configured. */ + suspend fun fetchCatalog(): String +} + +class InstanceCatalogNotConfiguredException : IOException("INSTANCES_CATALOG_URL is not configured") + +class InstanceCatalogHttpException(val code: Int) : IOException("Catalog request failed with HTTP $code") + +/** + * Fetches the remote catalog with a plain client of its own. It is a fixed, anonymous endpoint, + * so it deliberately bypasses the app's API client and never carries credentials. + */ +class OkHttpInstanceApiService( + private val catalogUrl: String?, + private val client: OkHttpClient = defaultClient(), +) : InstanceApiService { + + override suspend fun fetchCatalog(): String = withContext(Dispatchers.IO) { + val url = catalogUrl?.takeIf { it.isNotBlank() } ?: throw InstanceCatalogNotConfiguredException() + val request = Request.Builder() + .url(url) + .cacheControl(CacheControl.FORCE_NETWORK) + .header("Accept", "application/json") + .build() + client.newCall(request).execute().use { response -> + if (!response.isSuccessful) throw InstanceCatalogHttpException(response.code) + response.body?.string() ?: throw InstanceCatalogHttpException(response.code) + } + } + + companion object { + private const val TIMEOUT_SECONDS = 10L + + fun defaultClient(): OkHttpClient = OkHttpClient.Builder() + .callTimeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) + .connectTimeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) + .readTimeout(TIMEOUT_SECONDS, TimeUnit.SECONDS) + .build() + } +} diff --git a/core/src/main/java/org/openedx/core/instance/InstanceConfigLoader.kt b/core/src/main/java/org/openedx/core/instance/InstanceConfigLoader.kt new file mode 100644 index 000000000..73a5f12f9 --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/InstanceConfigLoader.kt @@ -0,0 +1,76 @@ +package org.openedx.core.instance + +import android.content.Context +import android.util.Log +import androidx.core.content.edit +import kotlinx.coroutines.CancellationException + +/** Keeps the last remote catalog that replaced the baseline. */ +interface InstanceCatalogCache { + var cachedCatalogJson: String? +} + +class SharedPrefsInstanceCatalogCache(context: Context) : InstanceCatalogCache { + + private val prefs = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) + + override var cachedCatalogJson: String? + get() = prefs.getString(KEY_CATALOG_JSON, null) + set(value) = prefs.edit { putString(KEY_CATALOG_JSON, value) } + + private companion object { + const val PREFS_NAME = "openedx_instance_catalog" + const val KEY_CATALOG_JSON = "cached_catalog_json" + } +} + +/** + * Decides which catalog the app runs with. + * + * The baseline is the last catalog fetched successfully, else the bundled one, else empty. + * Every launch tries a live fetch: a response with at least one instance replaces the baseline + * and becomes the new cache. A failed fetch, or one that returns no instances, leaves the + * baseline untouched, so a reachable but empty endpoint can never wipe a good cache, and an + * offline launch shows the catalog the learner saw last time. + */ +class InstanceConfigLoader( + private val apiService: InstanceApiService, + private val cache: InstanceCatalogCache, + private val bundledConfig: () -> InstancesConfig, + private val logger: (String) -> Unit = { Log.w(TAG, it) }, +) { + + suspend fun load(): InstancesConfig { + val baseline = loadCached() ?: bundledConfig().takeIf { it.instances.isNotEmpty() } ?: InstancesConfig() + return try { + val json = apiService.fetchCatalog() + val fetched = InstancesConfig.fromRemoteJson(json) + if (fetched.instances.isEmpty()) { + baseline + } else { + cache.cachedCatalogJson = json + fetched + } + } catch (e: CancellationException) { + throw e + } catch (@Suppress("TooGenericExceptionCaught") e: Exception) { + // No configured URL is the normal single-catalog setup, not a failure worth logging. + if (e !is InstanceCatalogNotConfiguredException) { + logger("Live catalog fetch failed (${e.message}), using the baseline") + } + baseline + } + } + + /** `null` if nothing was cached, or if the cached payload no longer parses. */ + private fun loadCached(): InstancesConfig? { + val json = cache.cachedCatalogJson ?: return null + return runCatching { InstancesConfig.fromRemoteJson(json) } + .getOrNull() + ?.takeIf { it.instances.isNotEmpty() } + } + + private companion object { + const val TAG = "InstanceConfigLoader" + } +} diff --git a/core/src/main/java/org/openedx/core/instance/InstancesJsonException.kt b/core/src/main/java/org/openedx/core/instance/InstancesJsonException.kt new file mode 100644 index 000000000..5b925856c --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/InstancesJsonException.kt @@ -0,0 +1,4 @@ +package org.openedx.core.instance + +/** The payload is neither a bare instance array nor an object wrapping one. */ +class InstancesJsonException(message: String, cause: Throwable? = null) : Exception(message, cause) diff --git a/core/src/main/java/org/openedx/core/instance/InstancesRemoteJson.kt b/core/src/main/java/org/openedx/core/instance/InstancesRemoteJson.kt new file mode 100644 index 000000000..b03cc1ae5 --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/InstancesRemoteJson.kt @@ -0,0 +1,77 @@ +package org.openedx.core.instance + +import com.google.gson.JsonArray +import com.google.gson.JsonElement +import com.google.gson.JsonObject +import com.google.gson.JsonParseException +import com.google.gson.JsonParser +import java.util.Locale + +/** + * Remote catalogs are sent in snake_case. These are the keys renamed to the bundled schema's + * UPPER_SNAKE_CASE before parsing; every other key passes through unchanged, so a remote field + * outside this map has to be sent in UPPER_SNAKE_CASE already. Shared with iOS. + */ +internal val remoteToConfigKeyMap: Map = mapOf( + "key" to "KEY", + "name" to "NAME", + "color" to "COLOR", + "instance_name" to "INSTANCE_NAME", + "oauth_client_id" to "OAUTH_CLIENT_ID", + "api_host_url" to "API_HOST_URL", + "sso_url" to "SSO_URL", + "sso_url_successful_login" to "SSO_URL_SUCCESSFUL_LOGIN", + "environment_display_name" to "ENVIRONMENT_DISPLAY_NAME", + "is_switch_instance_login_enabled" to "IS_SWITCH_INSTANCE_LOGIN_ENABLED", + "ui_components" to "UI_COMPONENTS", + "course_dropdown_navigation_enabled" to "COURSE_DROPDOWN_NAVIGATION_ENABLED", + "course_unit_progress_enabled" to "COURSE_UNIT_PROGRESS_ENABLED", + "login_registration_enabled" to "LOGIN_REGISTRATION_ENABLED", + "saml_sso_login_enabled" to "SAML_SSO_LOGIN_ENABLED", + "saml_sso_default_login_button" to "SAML_SSO_DEFAULT_LOGIN_BUTTON", + "theme" to "THEME", + "light" to "LIGHT", + "dark" to "DARK", + "logo_url" to "LOGO_URL", + "header_background_url" to "HEADER_BACKGROUND_URL", +) + +/** Renames mapped keys at every nesting depth; unmapped keys and all values stay as they are. */ +internal fun JsonObject.normalizedRemoteKeys(): JsonObject { + val result = JsonObject() + entrySet().forEach { (key, value) -> + val mappedKey = remoteToConfigKeyMap[key.lowercase(Locale.ROOT)] ?: key + result.add(mappedKey, if (value.isJsonObject) value.asJsonObject.normalizedRemoteKeys() else value) + } + return result +} + +/** + * Parses a remote catalog: a bare array, or an object with the array under `instances` or + * `INSTANCES`. Keys are normalized, then entries go through the same rules as the bundled + * catalog. + */ +fun InstancesConfig.Companion.fromRemoteJson( + json: String, + languageCode: String = Locale.getDefault().language, +): InstancesConfig { + val root: JsonElement = try { + JsonParser.parseString(json) + } catch (e: JsonParseException) { + throw InstancesJsonException("Catalog is not valid JSON", e) + } + val array = root.instancesArray() ?: throw InstancesJsonException("Catalog has no instances array") + val normalized = JsonArray().apply { + array.forEach { add(if (it.isJsonObject) it.asJsonObject.normalizedRemoteKeys() else it) } + } + return fromJson(normalized, languageCode = languageCode) +} + +private fun JsonElement.instancesArray(): JsonArray? = when { + isJsonArray -> asJsonArray + isJsonObject -> (asJsonObject.get(InstancesConfig.INSTANCES) ?: asJsonObject.get("instances")) + ?.takeIf { it.isJsonArray } + ?.asJsonArray + + else -> null +} diff --git a/core/src/test/java/org/openedx/core/instance/InstanceConfigLoaderTest.kt b/core/src/test/java/org/openedx/core/instance/InstanceConfigLoaderTest.kt new file mode 100644 index 000000000..6a0d88579 --- /dev/null +++ b/core/src/test/java/org/openedx/core/instance/InstanceConfigLoaderTest.kt @@ -0,0 +1,256 @@ +package org.openedx.core.instance + +import kotlinx.coroutines.runBlocking +import okhttp3.Interceptor +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Test +import java.io.IOException + +class InstanceConfigLoaderTest { + + private fun resource(name: String): String { + val stream = requireNotNull(javaClass.classLoader?.getResourceAsStream("instances/$name")) + return stream.reader().use { it.readText() } + } + + private class FakeApi(private val result: () -> String) : InstanceApiService { + var calls = 0 + override suspend fun fetchCatalog(): String { + calls++ + return result() + } + } + + private class InMemoryCache(override var cachedCatalogJson: String? = null) : InstanceCatalogCache + + private val bundled = InstancesConfig.fromAppConfig( + com.google.gson.JsonParser.parseString( + """{"INSTANCES": [{"KEY": "bundled", "NAME": "Bundled", "OAUTH_CLIENT_ID": "c", + "API_HOST_URL": "https://bundled.example.com"}]}""" + ).asJsonObject + ) + + private val onlineCatalog = """[{"key": "online", "name": "Online", "oauth_client_id": "c", + "api_host_url": "https://online.example.com"}]""" + + private fun loader(api: InstanceApiService, cache: InstanceCatalogCache, bundledConfig: InstancesConfig = bundled) = + InstanceConfigLoader(api, cache, { bundledConfig }, logger = {}) + + // region Remote JSON + + @Test + fun `remote snake_case keys are normalized at every depth`() { + val catalog = InstancesConfig.fromRemoteJson(resource("remote_catalog.json"), languageCode = "en") + val instance = requireNotNull(catalog.instance("niepd")) + + assertEquals("NIEPD", instance.name) + assertEquals("National Institute", instance.instanceName) + assertEquals("#1C355E", instance.color) + assertEquals("niepd-client", instance.oAuthClientId) + assertEquals("https://lms.niepd.example.com", instance.apiHostUrl) + assertEquals("https://lms.niepd.example.com/auth/complete/tpa-saml", instance.ssoFinishedUrl) + assertTrue(instance.isSwitchInstanceLoginEnabled) + assertEquals("https://cdn.example.com/niepd/logo.png", instance.logoUrl) + assertEquals("https://cdn.example.com/niepd/header.png", instance.headerBackgroundUrl) + val uiComponents = instance.config.getAsJsonObject("UI_COMPONENTS") + assertTrue(uiComponents.get("COURSE_UNIT_PROGRESS_ENABLED").asBoolean) + assertFalse(uiComponents.get("LOGIN_REGISTRATION_ENABLED").asBoolean) + } + + @Test + fun `palette field names inside THEME are left as they are`() { + val instance = InstancesConfig.fromRemoteJson(resource("remote_catalog.json")).instances.single() + + assertEquals(mapOf("accent_color" to "#3C68FF", "text_primary" to "#18202E"), instance.themeColors?.light) + assertEquals(mapOf("accent_color" to "#1C355E"), instance.themeColors?.dark) + } + + @Test + fun `unmapped remote keys pass through unchanged`() { + val instance = InstancesConfig.fromRemoteJson(resource("remote_catalog.json")).instances.single() + + assertEquals("help@niepd.example.com", instance.config.get("FEEDBACK_EMAIL_ADDRESS").asString) + assertEquals( + "ignored-because-unmapped@example.com", + instance.config.get("feedback_email_address").asString + ) + } + + @Test + fun `bare and wrapped arrays are both accepted`() { + val entry = """{"key": "a", "name": "A", "oauth_client_id": "c", "api_host_url": "https://a.example.com"}""" + + listOf("[$entry]", """{"instances": [$entry]}""", """{"INSTANCES": [$entry]}""").forEach { json -> + assertEquals(listOf("a"), InstancesConfig.fromRemoteJson(json).instances.map { it.key }) + } + } + + @Test + fun `unrecognized shapes are rejected`() { + listOf("""{"sites": []}""", "\"text\"", "{not json").forEach { json -> + try { + InstancesConfig.fromRemoteJson(json) + fail("$json should be rejected") + } catch (_: InstancesJsonException) { + // expected + } + } + } + + @Test + fun `remote entries follow the same required field rules`() { + val catalog = InstancesConfig.fromRemoteJson(resource("remote_catalog.json")) + + assertNull(catalog.instance("broken")) + } + + // endregion + + // region Loader + + @Test + fun `non empty live catalog replaces the baseline and is cached`() = runBlocking { + val cache = InMemoryCache() + + val result = loader(FakeApi { onlineCatalog }, cache).load() + + assertEquals(listOf("online"), result.instances.map { it.key }) + assertEquals(onlineCatalog, cache.cachedCatalogJson) + } + + @Test + fun `failed fetch falls back to the cache before the bundled catalog`() = runBlocking { + val cache = InMemoryCache(onlineCatalog) + + val result = loader(FakeApi { throw IOException("offline") }, cache).load() + + assertEquals(listOf("online"), result.instances.map { it.key }) + } + + @Test + fun `failed fetch without a cache uses the bundled catalog`() = runBlocking { + val result = loader(FakeApi { throw IOException("timeout") }, InMemoryCache()).load() + + assertEquals(listOf("bundled"), result.instances.map { it.key }) + } + + @Test + fun `failed fetch with nothing cached or bundled gives an empty catalog`() = runBlocking { + val result = loader(FakeApi { throw IOException("offline") }, InMemoryCache(), InstancesConfig()).load() + + assertTrue(result.instances.isEmpty()) + } + + @Test + fun `reachable but empty catalog never wipes a good cache`() = runBlocking { + val cache = InMemoryCache(onlineCatalog) + + listOf("[]", """{"instances": []}""", """[{"key": "x"}]""").forEach { empty -> + val result = loader(FakeApi { empty }, cache).load() + assertEquals(listOf("online"), result.instances.map { it.key }) + } + assertEquals(onlineCatalog, cache.cachedCatalogJson) + } + + @Test + fun `malformed response keeps the baseline`() = runBlocking { + val cache = InMemoryCache(onlineCatalog) + + val result = loader(FakeApi { "502" }, cache).load() + + assertEquals(listOf("online"), result.instances.map { it.key }) + assertEquals(onlineCatalog, cache.cachedCatalogJson) + } + + @Test + fun `unparseable cache is ignored`() = runBlocking { + val result = loader(FakeApi { throw IOException("offline") }, InMemoryCache("{broken")).load() + + assertEquals(listOf("bundled"), result.instances.map { it.key }) + } + + @Test + fun `no configured URL uses the baseline without a network call`() = runBlocking { + val service = OkHttpInstanceApiService( + catalogUrl = "", + client = OkHttpClient.Builder() + .addInterceptor { throw AssertionError("no request expected") } + .build() + ) + + val result = loader(service, InMemoryCache()).load() + + assertEquals(listOf("bundled"), result.instances.map { it.key }) + } + + // endregion + + // region Service + + private fun capturingClient(code: Int, body: String, captured: MutableList) = OkHttpClient.Builder() + .addInterceptor( + Interceptor { chain -> + captured += chain.request() + Response.Builder() + .request(chain.request()) + .protocol(Protocol.HTTP_1_1) + .code(code) + .message("status $code") + .body(body.toResponseBody("application/json".toMediaType())) + .build() + } + ) + .build() + + @Test + fun `catalog request is anonymous and bypasses caches`() = runBlocking { + val captured = mutableListOf() + val service = OkHttpInstanceApiService( + catalogUrl = "https://catalog.example.com/instances.json", + client = capturingClient(200, onlineCatalog, captured), + ) + + assertEquals(onlineCatalog, service.fetchCatalog()) + val request = captured.single() + assertEquals("https://catalog.example.com/instances.json", request.url.toString()) + assertNull(request.header("Authorization")) + assertNull(request.header("Cookie")) + assertTrue(request.cacheControl.noCache) + } + + @Test + fun `non 2xx responses are errors`() = runBlocking { + val service = OkHttpInstanceApiService( + catalogUrl = "https://catalog.example.com/instances.json", + client = capturingClient(503, "{}", mutableListOf()), + ) + + try { + service.fetchCatalog() + fail("503 must throw") + } catch (e: InstanceCatalogHttpException) { + assertEquals(503, e.code) + } + } + + @Test + fun `default client times out after ten seconds`() { + val client = OkHttpInstanceApiService.defaultClient() + + assertEquals(10_000, client.callTimeoutMillis) + assertTrue(client.interceptors.isEmpty()) + assertTrue(client.networkInterceptors.isEmpty()) + } + + // endregion +} diff --git a/core/src/test/resources/instances/remote_catalog.json b/core/src/test/resources/instances/remote_catalog.json new file mode 100644 index 000000000..c9bab44a3 --- /dev/null +++ b/core/src/test/resources/instances/remote_catalog.json @@ -0,0 +1,34 @@ +{ + "instances": [ + { + "key": "niepd", + "name": "NIEPD", + "instance_name": { "en": "National Institute", "ar": "المعهد الوطني" }, + "color": "#1C355E", + "oauth_client_id": "niepd-client", + "api_host_url": "https://lms.niepd.example.com", + "sso_url": "https://lms.niepd.example.com/auth/login/tpa-saml/?auth_entry=login", + "sso_url_successful_login": "https://lms.niepd.example.com/auth/complete/tpa-saml", + "environment_display_name": "NIEPD", + "is_switch_instance_login_enabled": true, + "ui_components": { + "course_unit_progress_enabled": true, + "login_registration_enabled": false, + "saml_sso_login_enabled": true + }, + "theme": { + "light": { "accent_color": "#3C68FF", "text_primary": "#18202E" }, + "dark": { "accent_color": "#1C355E" }, + "logo_url": "https://cdn.example.com/niepd/logo.png", + "header_background_url": "https://cdn.example.com/niepd/header.png" + }, + "FEEDBACK_EMAIL_ADDRESS": "help@niepd.example.com", + "feedback_email_address": "ignored-because-unmapped@example.com" + }, + { + "key": "broken", + "name": "Broken entry without a client", + "api_host_url": "https://broken.example.com" + } + ] +} diff --git a/default_config/dev/config.yaml b/default_config/dev/config.yaml index 97db15191..2101f01d7 100644 --- a/default_config/dev/config.yaml +++ b/default_config/dev/config.yaml @@ -5,6 +5,8 @@ URI_SCHEME: '' FEEDBACK_EMAIL_ADDRESS: 'support@example.com' FAQ_URL: '' OAUTH_CLIENT_ID: 'OAUTH_CLIENT_ID' +# URL of the remote learning site catalog. Leave empty to use only the bundled INSTANCES list. +INSTANCES_CATALOG_URL: '' # Keep empty to hide setting AGREEMENT_URLS: diff --git a/default_config/prod/config.yaml b/default_config/prod/config.yaml index f2868eb78..2f5e900d7 100644 --- a/default_config/prod/config.yaml +++ b/default_config/prod/config.yaml @@ -5,6 +5,8 @@ URI_SCHEME: '' FEEDBACK_EMAIL_ADDRESS: 'support@example.com' FAQ_URL: '' OAUTH_CLIENT_ID: 'OAUTH_CLIENT_ID' +# URL of the remote learning site catalog. Leave empty to use only the bundled INSTANCES list. +INSTANCES_CATALOG_URL: '' # Keep empty to hide setting AGREEMENT_URLS: diff --git a/default_config/stage/config.yaml b/default_config/stage/config.yaml index f2868eb78..2f5e900d7 100644 --- a/default_config/stage/config.yaml +++ b/default_config/stage/config.yaml @@ -5,6 +5,8 @@ URI_SCHEME: '' FEEDBACK_EMAIL_ADDRESS: 'support@example.com' FAQ_URL: '' OAUTH_CLIENT_ID: 'OAUTH_CLIENT_ID' +# URL of the remote learning site catalog. Leave empty to use only the bundled INSTANCES list. +INSTANCES_CATALOG_URL: '' # Keep empty to hide setting AGREEMENT_URLS: From a795b264b3b7e6ddbe433f67a2dafb2aa7cabb8b Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Mon, 19 Oct 2026 18:05:02 +0300 Subject: [PATCH 5/7] feat: route API requests to the selected instance InstanceHostInterceptor re-targets requests built against the app's base URL at the selected instance: scheme, host and port change, path and query stay. Other hosts, and every request made with nothing selected, pass through. Each request is tagged with the instance it was sent for. Token refresh calls the selected instance's /oauth2/access_token with its OAuth client. A 401 for a request sent before a site switch neither refreshes nor logs out, and tokens returned after the learner switched are discarded. The access token is only attached to requests for the active site's host. Mirrors iOS #677. --- app/build.gradle | 2 + .../app/data/networking/HeadersInterceptor.kt | 18 +- .../networking/InstanceHostInterceptor.kt | 57 ++++ .../OauthRefreshTokenAuthenticator.kt | 65 +++-- .../org/openedx/app/di/NetworkingModule.kt | 7 +- .../networking/InstanceAwareNetworkingTest.kt | 270 ++++++++++++++++++ 6 files changed, 397 insertions(+), 22 deletions(-) create mode 100644 app/src/main/java/org/openedx/app/data/networking/InstanceHostInterceptor.kt create mode 100644 app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt diff --git a/app/build.gradle b/app/build.gradle index f3d047808..8b8f7e9e7 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -150,6 +150,8 @@ dependencies { testImplementation "io.mockk:mockk:$mockk_version" testImplementation "io.mockk:mockk-android:$mockk_version" testImplementation "androidx.arch.core:core-testing:$android_arch_version" + // The Android stub of org.json returns defaults; the token authenticator parses error bodies with it. + testImplementation "org.json:json:20240303" } private def setupBranchConfigFields(buildType) { diff --git a/app/src/main/java/org/openedx/app/data/networking/HeadersInterceptor.kt b/app/src/main/java/org/openedx/app/data/networking/HeadersInterceptor.kt index baafe5a86..6feeb9e25 100644 --- a/app/src/main/java/org/openedx/app/data/networking/HeadersInterceptor.kt +++ b/app/src/main/java/org/openedx/app/data/networking/HeadersInterceptor.kt @@ -1,15 +1,18 @@ package org.openedx.app.data.networking +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.Interceptor import okhttp3.Response import org.openedx.core.config.Config import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.instance.InstanceProvider import org.openedx.core.presentation.global.AppData class HeadersInterceptor( private val appData: AppData, private val config: Config, private val preferencesManager: CorePreferences, + private val instanceProvider: InstanceProvider, ) : Interceptor { override fun intercept(chain: Interceptor.Chain): Response = chain.run { @@ -18,7 +21,8 @@ class HeadersInterceptor( .newBuilder().apply { val token = preferencesManager.accessToken - if (token.isNotEmpty()) { + // The token belongs to the selected site; never send it to another host. + if (token.isNotEmpty() && targetsActiveSite(request().url.toString())) { addHeader("Authorization", "${config.getAccessTokenType()} $token") } @@ -29,4 +33,16 @@ class HeadersInterceptor( }.build() ) } + + private fun targetsActiveSite(url: String): Boolean { + val siteUrl = instanceProvider.currentInstance?.apiHostUrl ?: config.getApiHostURL() + val site = siteUrl.toHttpUrlOrNull() + val request = url.toHttpUrlOrNull() + return when { + // An unparseable configured host can't be compared; keep the previous behavior. + site == null -> true + request == null -> false + else -> request.isSameOrigin(site) + } + } } diff --git a/app/src/main/java/org/openedx/app/data/networking/InstanceHostInterceptor.kt b/app/src/main/java/org/openedx/app/data/networking/InstanceHostInterceptor.kt new file mode 100644 index 000000000..8d680ba14 --- /dev/null +++ b/app/src/main/java/org/openedx/app/data/networking/InstanceHostInterceptor.kt @@ -0,0 +1,57 @@ +package org.openedx.app.data.networking + +import okhttp3.HttpUrl +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull +import okhttp3.Interceptor +import okhttp3.Request +import okhttp3.Response +import org.openedx.core.instance.InstanceProvider + +/** + * Which instance a request was sent for, attached as an OkHttp tag when the request starts. + * Token refresh reads it, so a response that arrives after a switch is still handled for the + * site that sent it. + */ +data class RequestInstance(val instanceKey: String?) + +/** + * Re-targets requests built against the app's base URL at the selected instance: scheme, host + * and port change, path and query stay. Requests to any other host (CDNs, third-party SDKs) + * and every request made while no instance is selected pass through untouched. + */ +class InstanceHostInterceptor( + appBaseUrl: String, + private val instanceProvider: InstanceProvider, +) : Interceptor { + + private val appBase: HttpUrl? = appBaseUrl.toHttpUrlOrNull() + + override fun intercept(chain: Interceptor.Chain): Response { + val instance = instanceProvider.currentInstance + val request = chain.request() + val rewritten = rewriteHostIfNeeded(request, instance?.apiHostUrl) + .newBuilder() + .tag(RequestInstance::class.java, RequestInstance(instance?.key)) + .build() + return chain.proceed(rewritten) + } + + internal fun rewriteHostIfNeeded(request: Request, instanceHostUrl: String?): Request { + val target = instanceHostUrl?.toHttpUrlOrNull() + val base = appBase + val url = request.url + if (target == null || base == null || !url.isSameOrigin(base) || url.isSameOrigin(target)) { + return request + } + val newUrl = url.newBuilder() + .scheme(target.scheme) + .host(target.host) + .port(target.port) + .build() + return request.newBuilder().url(newUrl).build() + } +} + +internal fun HttpUrl.isSameOrigin(other: HttpUrl): Boolean { + return scheme == other.scheme && host == other.host && port == other.port +} diff --git a/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt b/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt index a60a3a988..4bec459ef 100644 --- a/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt +++ b/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt @@ -23,25 +23,29 @@ import org.openedx.core.ApiConstants.TOKEN_TYPE_JWT import org.openedx.core.BuildConfig import org.openedx.core.config.Config import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.instance.InstanceProvider import org.openedx.core.system.notifier.app.AppNotifier import org.openedx.core.system.notifier.app.LogoutEvent import org.openedx.core.utils.TimeUtils import retrofit2.Retrofit import retrofit2.converter.gson.GsonConverterFactory import java.io.IOException +import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.TimeUnit class OauthRefreshTokenAuthenticator( private val config: Config, private val preferencesManager: CorePreferences, private val appNotifier: AppNotifier, + private val instanceProvider: InstanceProvider, + private val okHttpClient: OkHttpClient = defaultRefreshClient(), ) : Authenticator, Interceptor { - private val authApi: AuthApi + private val authApis = ConcurrentHashMap() private var lastTokenRefreshRequestTime = 0L override fun intercept(chain: Interceptor.Chain): Response { - if (isTokenExpired()) { + if (isTokenExpired() && isForActiveInstance(chain.request())) { val response = createUnauthorizedResponse(chain) val request = authenticate(chain.connection()?.route(), response) @@ -50,25 +54,33 @@ class OauthRefreshTokenAuthenticator( return chain.proceed(chain.request()) } - init { - val okHttpClient = OkHttpClient.Builder().apply { - writeTimeout(timeout = 60, TimeUnit.SECONDS) - readTimeout(timeout = 60, TimeUnit.SECONDS) - if (BuildConfig.DEBUG) { - addNetworkInterceptor(HttpLoggingInterceptor().setLevel(HttpLoggingInterceptor.Level.BODY)) - } - }.build() - authApi = Retrofit.Builder() - .baseUrl(config.getApiHostURL()) - .client(okHttpClient) - .addConverterFactory(GsonConverterFactory.create(Gson())) - .build() - .create(AuthApi::class.java) + /** Token endpoint of the selected instance, or of the app-level host when none is selected. */ + private fun authApi(): AuthApi { + val baseUrl = instanceProvider.currentInstance?.apiHostUrl ?: config.getApiHostURL() + return authApis.getOrPut(baseUrl) { + Retrofit.Builder() + .baseUrl(baseUrl) + .client(okHttpClient) + .addConverterFactory(GsonConverterFactory.create(Gson())) + .build() + .create(AuthApi::class.java) + } + } + + /** + * A 401 for a request sent before the learner switched sites says nothing about the site + * that is selected now: refreshing it, or logging out, would act on the wrong session. + */ + private fun isForActiveInstance(request: Request): Boolean { + val sentFor = request.tag(RequestInstance::class.java) ?: return true + return sentFor.instanceKey == instanceProvider.currentInstance?.key } @Suppress("ReturnCount") @Synchronized override fun authenticate(route: Route?, response: Response): Request? { + if (!isForActiveInstance(response.request)) return null + val accessToken = preferencesManager.accessToken val refreshToken = preferencesManager.refreshToken @@ -76,7 +88,7 @@ class OauthRefreshTokenAuthenticator( val errorCode = getErrorCode(response.peekBody(Long.MAX_VALUE).string()) ?: return null - return when (errorCode) { + val retry = when (errorCode) { TOKEN_EXPIRED_ERROR_MESSAGE, JWT_TOKEN_EXPIRED -> { handleTokenExpired(response, refreshToken, accessToken) } @@ -91,6 +103,9 @@ class OauthRefreshTokenAuthenticator( else -> null } + // Re-checked after the refresh: a switch during it must not retry this request with the + // new site's token. + return retry?.takeIf { isForActiveInstance(response.request) } } private fun handleDisabledUser(): Request? { @@ -159,13 +174,17 @@ class OauthRefreshTokenAuthenticator( private fun refreshAccessToken(refreshToken: String): AuthResponse? { var authResponse: AuthResponse? = null if (canRequestTokenRefresh()) { - val response = authApi.refreshAccessToken( + val instance = instanceProvider.currentInstance + val response = authApi().refreshAccessToken( ApiConstants.TOKEN_TYPE_REFRESH, - config.getOAuthClientId(), + instance?.oAuthClientId ?: config.getOAuthClientId(), refreshToken, config.getAccessTokenType() ).execute() authResponse = response.body()?.mapToDomain() + // The learner may have switched sites while the refresh was in flight; these tokens + // belong to the site that issued them, not to the one selected now. + if (instance?.key != instanceProvider.currentInstance?.key) return null if (response.isSuccessful && authResponse != null) { val newAccessToken = authResponse.accessToken ?: "" val newRefreshToken = authResponse.refreshToken ?: "" @@ -244,6 +263,14 @@ class OauthRefreshTokenAuthenticator( companion object { private const val HEADER_AUTHORIZATION = "Authorization" + private fun defaultRefreshClient(): OkHttpClient = OkHttpClient.Builder().apply { + writeTimeout(timeout = 60, TimeUnit.SECONDS) + readTimeout(timeout = 60, TimeUnit.SECONDS) + if (BuildConfig.DEBUG) { + addNetworkInterceptor(HttpLoggingInterceptor().setLevel(HttpLoggingInterceptor.Level.BODY)) + } + }.build() + private const val TOKEN_EXPIRED_ERROR_MESSAGE = "token_expired" private const val TOKEN_NONEXISTENT_ERROR_MESSAGE = "token_nonexistent" private const val TOKEN_INVALID_GRANT_ERROR_MESSAGE = "invalid_grant" diff --git a/app/src/main/java/org/openedx/app/di/NetworkingModule.kt b/app/src/main/java/org/openedx/app/di/NetworkingModule.kt index 6360e7fba..412f99a64 100644 --- a/app/src/main/java/org/openedx/app/di/NetworkingModule.kt +++ b/app/src/main/java/org/openedx/app/di/NetworkingModule.kt @@ -7,6 +7,7 @@ import org.openedx.app.data.api.NotificationsApi import org.openedx.app.data.networking.AppUpgradeInterceptor import org.openedx.app.data.networking.HandleErrorInterceptor import org.openedx.app.data.networking.HeadersInterceptor +import org.openedx.app.data.networking.InstanceHostInterceptor import org.openedx.app.data.networking.OauthRefreshTokenAuthenticator import org.openedx.auth.data.api.AuthApi import org.openedx.core.BuildConfig @@ -22,13 +23,15 @@ import java.util.concurrent.TimeUnit val networkingModule = module { - single { OauthRefreshTokenAuthenticator(get(), get(), get()) } + single { OauthRefreshTokenAuthenticator(get(), get(), get(), get()) } + single { InstanceHostInterceptor(get().getApiHostURL(), get()) } single { OkHttpClient.Builder().apply { writeTimeout(60, TimeUnit.SECONDS) readTimeout(60, TimeUnit.SECONDS) - addInterceptor(HeadersInterceptor(get(), get(), get())) + addInterceptor(get()) + addInterceptor(HeadersInterceptor(get(), get(), get(), get())) if (BuildConfig.DEBUG) { addNetworkInterceptor(HttpLoggingInterceptor().setLevel(HttpLoggingInterceptor.Level.BODY)) } diff --git a/app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt b/app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt new file mode 100644 index 000000000..67880402e --- /dev/null +++ b/app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt @@ -0,0 +1,270 @@ +package org.openedx.app.data.networking + +import com.google.gson.JsonObject +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import okhttp3.Interceptor +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import okio.Buffer +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test +import org.openedx.core.config.Config +import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.instance.Instance +import org.openedx.core.instance.InstanceProvider +import org.openedx.core.presentation.global.AppData +import org.openedx.core.system.notifier.app.AppNotifier +import org.openedx.core.system.notifier.app.LogoutEvent + +class InstanceAwareNetworkingTest { + + private class FakeInstanceProvider(instance: Instance?) : InstanceProvider { + private val flow = MutableStateFlow(instance) + override val currentInstance: Instance? get() = flow.value + override val currentInstanceFlow: StateFlow = flow + override val hasMultipleInstances: Boolean = true + fun select(instance: Instance?) { + flow.value = instance + } + } + + private fun instance(key: String, host: String) = requireNotNull( + Instance.fromJson( + JsonObject().apply { + addProperty("KEY", key) + addProperty("NAME", key) + addProperty("OAUTH_CLIENT_ID", "$key-client") + addProperty("API_HOST_URL", host) + } + ) + ) + + private val appBase = "https://app.example.com" + private val alpha = instance("alpha", "https://alpha.example.com") + private val beta = instance("beta", "http://beta.example.com:8080") + + /** Runs [interceptor] on [request] and returns the request it forwarded. */ + private fun forward(interceptor: Interceptor, request: Request): Request { + var forwarded: Request? = null + val client = OkHttpClient.Builder() + .addInterceptor(interceptor) + .addInterceptor { chain -> + forwarded = chain.request() + ok(chain.request(), "{}") + } + .build() + client.newCall(request).execute().close() + return requireNotNull(forwarded) + } + + private fun ok(request: Request, body: String, code: Int = 200) = Response.Builder() + .request(request) + .protocol(Protocol.HTTP_1_1) + .code(code) + .message("status $code") + .body(body.toResponseBody("application/json".toMediaType())) + .build() + + // region InstanceHostInterceptor + + @Test + fun `app base requests are re-targeted at the selected instance, path and query preserved`() { + val interceptor = InstanceHostInterceptor(appBase, FakeInstanceProvider(beta)) + + val forwarded = forward( + interceptor, + Request.Builder().url("https://app.example.com/api/courses/v1/?page=2&size=10").build() + ) + + assertEquals("http://beta.example.com:8080/api/courses/v1/?page=2&size=10", forwarded.url.toString()) + assertEquals(RequestInstance("beta"), forwarded.tag(RequestInstance::class.java)) + } + + @Test + fun `no selected instance passes requests through`() { + val interceptor = InstanceHostInterceptor(appBase, FakeInstanceProvider(null)) + + val forwarded = forward(interceptor, Request.Builder().url("https://app.example.com/api/x").build()) + + assertEquals("https://app.example.com/api/x", forwarded.url.toString()) + assertEquals(RequestInstance(null), forwarded.tag(RequestInstance::class.java)) + } + + @Test + fun `third party hosts are left untouched`() { + val interceptor = InstanceHostInterceptor(appBase, FakeInstanceProvider(alpha)) + + listOf( + "https://cdn.example.net/video.mp4", + "http://app.example.com/api/x", + "https://app.example.com:8443/api/x", + ).forEach { url -> + assertEquals(url, forward(interceptor, Request.Builder().url(url).build()).url.toString()) + } + } + + @Test + fun `requests already built for the instance host stay as they are`() { + val interceptor = InstanceHostInterceptor(appBase, FakeInstanceProvider(alpha)) + val request = Request.Builder().url("https://alpha.example.com/api/x").build() + + assertSame(request, interceptor.rewriteHostIfNeeded(request, alpha.apiHostUrl)) + } + + // endregion + + // region HeadersInterceptor + + private fun headersInterceptor(provider: InstanceProvider): HeadersInterceptor { + val config = mockk { + every { getAccessTokenType() } returns "JWT" + every { getApiHostURL() } returns appBase + } + val preferences = mockk { every { accessToken } returns "token-a" } + val appData = AppData(appName = "app", versionName = "1", applicationId = "org.openedx.app") + return HeadersInterceptor(appData, config, preferences, provider) + } + + @Test + fun `token is sent to the active site only`() { + val interceptor = headersInterceptor(FakeInstanceProvider(alpha)) + + val toSite = forward(interceptor, Request.Builder().url("https://alpha.example.com/api/x").build()) + val elsewhere = forward(interceptor, Request.Builder().url("https://beta.example.com/api/x").build()) + + assertEquals("JWT token-a", toSite.header("Authorization")) + assertNull(elsewhere.header("Authorization")) + } + + @Test + fun `without a selection the app level host gets the token`() { + val interceptor = headersInterceptor(FakeInstanceProvider(null)) + + val forwarded = forward(interceptor, Request.Builder().url("https://app.example.com/api/x").build()) + + assertEquals("JWT token-a", forwarded.header("Authorization")) + } + + // endregion + + // region OauthRefreshTokenAuthenticator + + private class Prefs : CorePreferences by mockk(relaxed = true) { + override var accessToken: String = "old-access" + override var refreshToken: String = "old-refresh" + override var accessTokenExpiresAt: Long = Long.MAX_VALUE + } + + private fun unauthorized(request: Request) = ok(request, """{"detail": "Token has expired."}""", code = 401) + + private fun taggedRequest(url: String, key: String?) = Request.Builder() + .url(url) + .header("Authorization", "JWT old-access") + .tag(RequestInstance::class.java, RequestInstance(key)) + .build() + + @Test + fun `refresh uses the selected instance host and OAuth client`() { + val tokenRequests = mutableListOf() + val refreshClient = OkHttpClient.Builder().addInterceptor { chain -> + tokenRequests += chain.request() + ok( + chain.request(), + """{"access_token": "new-access", "refresh_token": "new-refresh", "expires_in": 3600}""" + ) + }.build() + val prefs = Prefs() + val config = mockk(relaxed = true) { + every { getAccessTokenType() } returns "JWT" + every { getApiHostURL() } returns appBase + every { getOAuthClientId() } returns "app-client" + } + val authenticator = OauthRefreshTokenAuthenticator( + config, + prefs, + mockk(relaxed = true), + FakeInstanceProvider(beta), + refreshClient + ) + + val retry = authenticator.authenticate( + null, + unauthorized(taggedRequest("http://beta.example.com:8080/api/x", "beta")) + ) + + val tokenRequest = tokenRequests.single() + assertEquals("http://beta.example.com:8080/oauth2/access_token/", tokenRequest.url.toString()) + val form = Buffer().also { tokenRequest.body!!.writeTo(it) }.readUtf8() + assertTrue(form, form.contains("client_id=beta-client")) + assertEquals("JWT new-access", retry?.header("Authorization")) + assertEquals("new-access", prefs.accessToken) + } + + @Test + fun `a 401 for a request sent before a switch neither refreshes nor logs out`() { + val refreshClient = OkHttpClient.Builder() + .addInterceptor { throw AssertionError("no refresh expected") } + .build() + val notifier = mockk(relaxed = true) + val prefs = Prefs() + val authenticator = OauthRefreshTokenAuthenticator( + mockk(relaxed = true), + prefs, + notifier, + FakeInstanceProvider(beta), + refreshClient + ) + + val retry = authenticator.authenticate( + null, + unauthorized(taggedRequest("https://alpha.example.com/api/x", "alpha")) + ) + + assertNull(retry) + assertEquals("old-access", prefs.accessToken) + coVerify(exactly = 0) { notifier.send(any()) } + } + + @Test + fun `tokens refreshed while the learner switched sites are discarded`() { + val provider = FakeInstanceProvider(alpha) + val refreshClient = OkHttpClient.Builder().addInterceptor { chain -> + provider.select(beta) + ok( + chain.request(), + """{"access_token": "alpha-new", "refresh_token": "alpha-refresh", "expires_in": 3600}""" + ) + }.build() + val prefs = Prefs() + val authenticator = OauthRefreshTokenAuthenticator( + mockk(relaxed = true) { every { getAccessTokenType() } returns "JWT" }, + prefs, + mockk(relaxed = true), + provider, + refreshClient + ) + + val retry = authenticator.authenticate( + null, + unauthorized(taggedRequest("https://alpha.example.com/api/x", "alpha")) + ) + + assertNull(retry) + assertEquals("old-access", prefs.accessToken) + assertNotNull(provider.currentInstance) + } + + // endregion +} From 4920b7aa152817d40e73a9c4580112c8978e94c0 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Mon, 19 Oct 2026 18:08:41 +0300 Subject: [PATCH 6/7] feat: namespace local storage keys by instance Preferences tied to a learner on one site (tokens, push token, user, profile, server app config, relative dates, calendar sync state) are now stored per instance, so switching sites can't leak one site's session into another. Device-wide settings (video quality, Wi-Fi only downloads, review prompt, What's New) stay shared. StorageNamespace decides where an instance's data lives. The instance whose API host and OAuth client match the app-level configuration keeps the existing unprefixed keys, so an upgraded single-site install stays signed in; that choice is recorded once and never re-derived from a later catalog. Every other instance gets instance..* keys. CorePreferences can now check or clear one instance's session without selecting it, and token refresh saves rotated tokens for the site that issued them even if the learner switched away mid-refresh. Mirrors iOS #678. --- .../OauthRefreshTokenAuthenticator.kt | 11 +- .../app/data/storage/PreferencesManager.kt | 150 +++++++++++------- .../main/java/org/openedx/app/di/AppModule.kt | 16 +- .../networking/InstanceAwareNetworkingTest.kt | 14 +- .../core/data/storage/CorePreferences.kt | 12 ++ .../core/instance/InstanceSelectionStorage.kt | 11 +- .../openedx/core/instance/StorageNamespace.kt | 125 +++++++++++++++ .../core/instance/StorageNamespaceTest.kt | 134 ++++++++++++++++ 8 files changed, 405 insertions(+), 68 deletions(-) create mode 100644 core/src/main/java/org/openedx/core/instance/StorageNamespace.kt create mode 100644 core/src/test/java/org/openedx/core/instance/StorageNamespaceTest.kt diff --git a/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt b/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt index 4bec459ef..10bab6a75 100644 --- a/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt +++ b/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt @@ -182,20 +182,19 @@ class OauthRefreshTokenAuthenticator( config.getAccessTokenType() ).execute() authResponse = response.body()?.mapToDomain() - // The learner may have switched sites while the refresh was in flight; these tokens - // belong to the site that issued them, not to the one selected now. - if (instance?.key != instanceProvider.currentInstance?.key) return null if (response.isSuccessful && authResponse != null) { val newAccessToken = authResponse.accessToken ?: "" val newRefreshToken = authResponse.refreshToken ?: "" val newExpireTime = authResponse.getTokenExpiryTime() if (newAccessToken.isNotEmpty() && newRefreshToken.isNotEmpty()) { - preferencesManager.accessToken = newAccessToken - preferencesManager.refreshToken = newRefreshToken - preferencesManager.accessTokenExpiresAt = newExpireTime + // Saved for the site that issued them even if the learner switched away while + // the refresh was in flight: the server has already rotated the refresh token, + // so dropping these would sign that site out. + preferencesManager.saveTokens(instance?.key, newAccessToken, newRefreshToken, newExpireTime) lastTokenRefreshRequestTime = TimeUtils.getCurrentTime() } + if (instance?.key != instanceProvider.currentInstance?.key) return null } else if (response.code() == 400) { // another refresh already in progress Thread.sleep(REFRESH_TOKEN_THREAD_SLEEP) diff --git a/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt b/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt index 48b0d58a1..87545e3f6 100644 --- a/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt +++ b/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt @@ -24,6 +24,8 @@ import org.openedx.core.domain.model.AppConfig import org.openedx.core.domain.model.CalendarType import org.openedx.core.domain.model.VideoQuality import org.openedx.core.domain.model.VideoSettings +import org.openedx.core.instance.StorageNamespace +import org.openedx.core.instance.StorageNamespaceProvider import org.openedx.core.system.CalendarManager import org.openedx.core.system.notifier.app.AppNotifier import org.openedx.core.system.notifier.app.LogoutEvent @@ -43,6 +45,7 @@ private val Context.dataStore: DataStore by preferencesDataStore( class PreferencesManager( private val context: Context, private val appNotifier: AppNotifier, + private val namespaces: StorageNamespaceProvider, ) : CorePreferences, ProfilePreferences, @@ -61,33 +64,46 @@ class PreferencesManager( @Volatile private var isKeyInvalidated = false + /** + * Keys tied to a learner on one site: they are namespaced per instance, so each site keeps + * its own session, profile and calendar state. Everything in [Keys] is device-wide. + */ + @Suppress("TooManyFunctions") + private object InstanceKeys { + fun accessToken(ns: StorageNamespace) = stringPreferencesKey(ns.key("access_token")) + fun refreshToken(ns: StorageNamespace) = stringPreferencesKey(ns.key("refresh_token")) + fun pushToken(ns: StorageNamespace) = stringPreferencesKey(ns.key("push_token")) + fun expiresIn(ns: StorageNamespace) = longPreferencesKey(ns.key("expires_in")) + fun user(ns: StorageNamespace) = stringPreferencesKey(ns.key("user")) + fun account(ns: StorageNamespace) = stringPreferencesKey(ns.key("account")) + fun appConfig(ns: StorageNamespace) = stringPreferencesKey(ns.key("app_config")) + fun isRelativeDatesEnabled(ns: StorageNamespace) = + booleanPreferencesKey(ns.key("is_relative_dates_enabled")) + fun calendarId(ns: StorageNamespace) = longPreferencesKey(ns.key("calendar_id")) + fun calendarUser(ns: StorageNamespace) = stringPreferencesKey(ns.key("calendar_user")) + fun calendarType(ns: StorageNamespace) = stringPreferencesKey(ns.key("calendar_type")) + fun isCalendarSyncEnabled(ns: StorageNamespace) = + booleanPreferencesKey(ns.key("is_calendar_sync_enabled")) + fun isHideInactiveCourses(ns: StorageNamespace) = + booleanPreferencesKey(ns.key("hide_inactive_courses")) + fun calendarSyncDialogShown(ns: StorageNamespace, courseName: String) = + booleanPreferencesKey(ns.key("calendar_sync_dialog_${courseName.replaceSpace("_")}")) + } + + private val ns: StorageNamespace + get() = namespaces.current + private object Keys { - val ACCESS_TOKEN = stringPreferencesKey("access_token") - val REFRESH_TOKEN = stringPreferencesKey("refresh_token") - val PUSH_TOKEN = stringPreferencesKey("push_token") - val EXPIRES_IN = longPreferencesKey("expires_in") - val USER = stringPreferencesKey("user") - val ACCOUNT = stringPreferencesKey("account") val VIDEO_SETTINGS_WIFI_DOWNLOAD_ONLY = booleanPreferencesKey("video_settings_wifi_download_only") val VIDEO_SETTINGS_STREAMING_QUALITY = stringPreferencesKey("video_settings_streaming_quality") val VIDEO_SETTINGS_DOWNLOAD_QUALITY = stringPreferencesKey("video_settings_download_quality") - val APP_CONFIG = stringPreferencesKey("app_config") val CAN_RESET_APP_DIRECTORY = booleanPreferencesKey("reset_app_directory") - val IS_RELATIVE_DATES_ENABLED = booleanPreferencesKey("is_relative_dates_enabled") - val CALENDAR_ID = longPreferencesKey("calendar_id") - val CALENDAR_USER = stringPreferencesKey("calendar_user") - val CALENDAR_TYPE = stringPreferencesKey("calendar_type") - val IS_CALENDAR_SYNC_ENABLED = booleanPreferencesKey("is_calendar_sync_enabled") - val IS_HIDE_INACTIVE_COURSES = booleanPreferencesKey("hide_inactive_courses") val LAST_WHATS_NEW_VERSION = stringPreferencesKey("last_whats_new_version") val LAST_REVIEW_VERSION = stringPreferencesKey("last_review_version") val WAS_POSITIVE_RATED = booleanPreferencesKey("app_was_positive_rated") - - fun calendarSyncDialogShown(courseName: String) = - booleanPreferencesKey("calendar_sync_dialog_${courseName.replaceSpace("_")}") } private fun getValue(key: Preferences.Key, defaultValue: T): T = @@ -121,47 +137,73 @@ class PreferencesManager( } override suspend fun clearCorePreferences() { + clearSession(namespaces.current) + } + + override fun hasSession(instanceKey: String?): Boolean { + val namespace = namespaces.namespaceFor(instanceKey) + return getValue(InstanceKeys.accessToken(namespace), "").isNotEmpty() && + getValue(InstanceKeys.user(namespace), "").isNotEmpty() + } + + override suspend fun clearSession(instanceKey: String?) { + clearSession(namespaces.namespaceFor(instanceKey)) + } + + private suspend fun clearSession(namespace: StorageNamespace) { dataStore.edit { prefs -> - prefs.remove(Keys.ACCESS_TOKEN) - prefs.remove(Keys.REFRESH_TOKEN) - prefs.remove(Keys.PUSH_TOKEN) - prefs.remove(Keys.EXPIRES_IN) - prefs.remove(Keys.USER) - prefs.remove(Keys.ACCOUNT) + prefs.remove(InstanceKeys.accessToken(namespace)) + prefs.remove(InstanceKeys.refreshToken(namespace)) + prefs.remove(InstanceKeys.pushToken(namespace)) + prefs.remove(InstanceKeys.expiresIn(namespace)) + prefs.remove(InstanceKeys.user(namespace)) + prefs.remove(InstanceKeys.account(namespace)) } } + override fun refreshToken(instanceKey: String?): String { + return getEncryptedString(InstanceKeys.refreshToken(namespaces.namespaceFor(instanceKey)), "") + } + + override fun saveTokens(instanceKey: String?, accessToken: String, refreshToken: String, expiresAt: Long) { + val namespace = namespaces.namespaceFor(instanceKey) + setEncryptedString(InstanceKeys.accessToken(namespace), accessToken) + setEncryptedString(InstanceKeys.refreshToken(namespace), refreshToken) + setValue(InstanceKeys.expiresIn(namespace), expiresAt) + } + override suspend fun clearCalendarPreferences() { + val namespace = ns dataStore.edit { prefs -> - prefs.remove(Keys.CALENDAR_ID) - prefs.remove(Keys.CALENDAR_TYPE) - prefs.remove(Keys.IS_CALENDAR_SYNC_ENABLED) - prefs.remove(Keys.IS_HIDE_INACTIVE_COURSES) + prefs.remove(InstanceKeys.calendarId(namespace)) + prefs.remove(InstanceKeys.calendarType(namespace)) + prefs.remove(InstanceKeys.isCalendarSyncEnabled(namespace)) + prefs.remove(InstanceKeys.isHideInactiveCourses(namespace)) } } override var accessToken: String - get() = getEncryptedString(Keys.ACCESS_TOKEN, "") - set(value) = setEncryptedString(Keys.ACCESS_TOKEN, value) + get() = getEncryptedString(InstanceKeys.accessToken(ns), "") + set(value) = setEncryptedString(InstanceKeys.accessToken(ns), value) override var refreshToken: String - get() = getEncryptedString(Keys.REFRESH_TOKEN, "") - set(value) = setEncryptedString(Keys.REFRESH_TOKEN, value) + get() = getEncryptedString(InstanceKeys.refreshToken(ns), "") + set(value) = setEncryptedString(InstanceKeys.refreshToken(ns), value) override var pushToken: String - get() = getEncryptedString(Keys.PUSH_TOKEN, "") - set(value) = setEncryptedString(Keys.PUSH_TOKEN, value) + get() = getEncryptedString(InstanceKeys.pushToken(ns), "") + set(value) = setEncryptedString(InstanceKeys.pushToken(ns), value) override var accessTokenExpiresAt: Long - get() = getValue(Keys.EXPIRES_IN, 0L) - set(value) = setValue(Keys.EXPIRES_IN, value) + get() = getValue(InstanceKeys.expiresIn(ns), 0L) + set(value) = setValue(InstanceKeys.expiresIn(ns), value) override var user: User? get() { - val json = getEncryptedString(Keys.USER, "") + val json = getEncryptedString(InstanceKeys.user(ns), "") return if (json.isEmpty()) null else Gson().fromJson(json, User::class.java) } - set(value) = setEncryptedString(Keys.USER, Gson().toJson(value)) + set(value) = setEncryptedString(InstanceKeys.user(ns), Gson().toJson(value)) override var videoSettings: VideoSettings get() { @@ -188,25 +230,25 @@ class PreferencesManager( override var appConfig: AppConfig get() { - val json = getValue(Keys.APP_CONFIG, Gson().toJson(AppConfig())) + val json = getValue(InstanceKeys.appConfig(ns), Gson().toJson(AppConfig())) return Gson().fromJson(json, AppConfig::class.java) } - set(value) = setValue(Keys.APP_CONFIG, Gson().toJson(value)) + set(value) = setValue(InstanceKeys.appConfig(ns), Gson().toJson(value)) override var canResetAppDirectory: Boolean get() = getValue(Keys.CAN_RESET_APP_DIRECTORY, true) set(value) = setValue(Keys.CAN_RESET_APP_DIRECTORY, value) override var isRelativeDatesEnabled: Boolean - get() = getValue(Keys.IS_RELATIVE_DATES_ENABLED, true) - set(value) = setValue(Keys.IS_RELATIVE_DATES_ENABLED, value) + get() = getValue(InstanceKeys.isRelativeDatesEnabled(ns), true) + set(value) = setValue(InstanceKeys.isRelativeDatesEnabled(ns), value) override var profile: Account? get() { - val json = getEncryptedString(Keys.ACCOUNT, "") + val json = getEncryptedString(InstanceKeys.account(ns), "") return if (json.isEmpty()) null else Gson().fromJson(json, Account::class.java) } - set(value) = setEncryptedString(Keys.ACCOUNT, Gson().toJson(value)) + set(value) = setEncryptedString(InstanceKeys.account(ns), Gson().toJson(value)) override var lastWhatsNewVersion: String get() = getValue(Keys.LAST_WHATS_NEW_VERSION, "") @@ -229,33 +271,33 @@ class PreferencesManager( set(value) = setValue(Keys.WAS_POSITIVE_RATED, value) override var calendarId: Long - get() = getValue(Keys.CALENDAR_ID, CalendarManager.CALENDAR_DOES_NOT_EXIST) - set(value) = setValue(Keys.CALENDAR_ID, value) + get() = getValue(InstanceKeys.calendarId(ns), CalendarManager.CALENDAR_DOES_NOT_EXIST) + set(value) = setValue(InstanceKeys.calendarId(ns), value) override var calendarUser: String - get() = getValue(Keys.CALENDAR_USER, "") - set(value) = setValue(Keys.CALENDAR_USER, value) + get() = getValue(InstanceKeys.calendarUser(ns), "") + set(value) = setValue(InstanceKeys.calendarUser(ns), value) override var calendarType: CalendarType get() { - val stored = getValue(Keys.CALENDAR_TYPE, CalendarType.LOCAL.name) + val stored = getValue(InstanceKeys.calendarType(ns), CalendarType.LOCAL.name) return runCatching { CalendarType.valueOf(stored) }.getOrDefault(CalendarType.LOCAL) } - set(value) = setValue(Keys.CALENDAR_TYPE, value.name) + set(value) = setValue(InstanceKeys.calendarType(ns), value.name) override var isCalendarSyncEnabled: Boolean - get() = getValue(Keys.IS_CALENDAR_SYNC_ENABLED, true) - set(value) = setValue(Keys.IS_CALENDAR_SYNC_ENABLED, value) + get() = getValue(InstanceKeys.isCalendarSyncEnabled(ns), true) + set(value) = setValue(InstanceKeys.isCalendarSyncEnabled(ns), value) override var isHideInactiveCourses: Boolean - get() = getValue(Keys.IS_HIDE_INACTIVE_COURSES, true) - set(value) = setValue(Keys.IS_HIDE_INACTIVE_COURSES, value) + get() = getValue(InstanceKeys.isHideInactiveCourses(ns), true) + set(value) = setValue(InstanceKeys.isHideInactiveCourses(ns), value) override fun setCalendarSyncEventsDialogShown(courseName: String) { - setValue(Keys.calendarSyncDialogShown(courseName), true) + setValue(InstanceKeys.calendarSyncDialogShown(ns, courseName), true) } override fun isCalendarSyncEventsDialogShown(courseName: String): Boolean { - return getValue(Keys.calendarSyncDialogShown(courseName), false) + return getValue(InstanceKeys.calendarSyncDialogShown(ns, courseName), false) } } diff --git a/app/src/main/java/org/openedx/app/di/AppModule.kt b/app/src/main/java/org/openedx/app/di/AppModule.kt index a713834c1..8bd453326 100644 --- a/app/src/main/java/org/openedx/app/di/AppModule.kt +++ b/app/src/main/java/org/openedx/app/di/AppModule.kt @@ -40,10 +40,12 @@ import org.openedx.core.instance.InstanceCatalogCache import org.openedx.core.instance.InstanceConfigLoader import org.openedx.core.instance.InstanceProvider import org.openedx.core.instance.InstanceSelectionStorage +import org.openedx.core.instance.InstanceStorageNamespaces import org.openedx.core.instance.InstanceStore import org.openedx.core.instance.OkHttpInstanceApiService import org.openedx.core.instance.SharedPrefsInstanceCatalogCache import org.openedx.core.instance.SharedPrefsInstanceSelectionStorage +import org.openedx.core.instance.StorageNamespaceProvider import org.openedx.core.module.DownloadWorkerController import org.openedx.core.module.TranscriptManager import org.openedx.core.module.download.DownloadHelper @@ -96,7 +98,17 @@ import org.openedx.core.DatabaseManager as IDatabaseManager val appModule = module { single { Config(get()) } - single { SharedPrefsInstanceSelectionStorage(get()) } + single { SharedPrefsInstanceSelectionStorage(get()) } + single { get() } + single { + val config = get() + InstanceStorageNamespaces( + instanceStore = get(), + ownerStorage = get(), + appApiHostUrl = config.getApiHostURL(), + appOAuthClientId = config.getOAuthClientId(), + ) + } single { val bundledInstances by lazy { get().getInstancesConfig() } InstanceStore(get()) { bundledInstances } @@ -105,7 +117,7 @@ val appModule = module { single { SharedPrefsInstanceCatalogCache(get()) } single { OkHttpInstanceApiService(get().getInstancesCatalogUrl()) } single { InstanceConfigLoader(get(), get(), { get().getInstancesConfig() }) } - single { PreferencesManager(get(), get()) } + single { PreferencesManager(get(), get(), get()) } single { get() } single { get() } single { get() } diff --git a/app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt b/app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt index 67880402e..a0fc2fdde 100644 --- a/app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt +++ b/app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt @@ -15,7 +15,6 @@ import okhttp3.Response import okhttp3.ResponseBody.Companion.toResponseBody import okio.Buffer import org.junit.Assert.assertEquals -import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertSame import org.junit.Assert.assertTrue @@ -165,6 +164,11 @@ class InstanceAwareNetworkingTest { override var accessToken: String = "old-access" override var refreshToken: String = "old-refresh" override var accessTokenExpiresAt: Long = Long.MAX_VALUE + val savedTokens = mutableMapOf() + + override fun saveTokens(instanceKey: String?, accessToken: String, refreshToken: String, expiresAt: Long) { + savedTokens[instanceKey] = accessToken + } } private fun unauthorized(request: Request) = ok(request, """{"detail": "Token has expired."}""", code = 401) @@ -209,7 +213,7 @@ class InstanceAwareNetworkingTest { val form = Buffer().also { tokenRequest.body!!.writeTo(it) }.readUtf8() assertTrue(form, form.contains("client_id=beta-client")) assertEquals("JWT new-access", retry?.header("Authorization")) - assertEquals("new-access", prefs.accessToken) + assertEquals(mapOf("beta" to "new-access"), prefs.savedTokens) } @Test @@ -238,7 +242,7 @@ class InstanceAwareNetworkingTest { } @Test - fun `tokens refreshed while the learner switched sites are discarded`() { + fun `tokens refreshed while the learner switched sites are kept for the site that issued them`() { val provider = FakeInstanceProvider(alpha) val refreshClient = OkHttpClient.Builder().addInterceptor { chain -> provider.select(beta) @@ -262,8 +266,8 @@ class InstanceAwareNetworkingTest { ) assertNull(retry) - assertEquals("old-access", prefs.accessToken) - assertNotNull(provider.currentInstance) + assertEquals(mapOf("alpha" to "alpha-new"), prefs.savedTokens) + assertEquals("beta", provider.currentInstance?.key) } // endregion diff --git a/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt b/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt index 9e42a5273..4b13957bb 100644 --- a/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt +++ b/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt @@ -15,5 +15,17 @@ interface CorePreferences { var canResetAppDirectory: Boolean var isRelativeDatesEnabled: Boolean + /** Clears the selected instance's session: tokens, user and profile. */ suspend fun clearCorePreferences() + + /** Whether [instanceKey]'s site has a stored session, without selecting it. */ + fun hasSession(instanceKey: String?): Boolean + + /** Clears one instance's session without touching the selected one. */ + suspend fun clearSession(instanceKey: String?) + + fun refreshToken(instanceKey: String?): String + + /** Stores refreshed tokens for the site that issued them, whichever site is selected now. */ + fun saveTokens(instanceKey: String?, accessToken: String, refreshToken: String, expiresAt: Long) } diff --git a/core/src/main/java/org/openedx/core/instance/InstanceSelectionStorage.kt b/core/src/main/java/org/openedx/core/instance/InstanceSelectionStorage.kt index 91f2b77b3..74f99c72c 100644 --- a/core/src/main/java/org/openedx/core/instance/InstanceSelectionStorage.kt +++ b/core/src/main/java/org/openedx/core/instance/InstanceSelectionStorage.kt @@ -10,7 +10,9 @@ interface InstanceSelectionStorage { var selectedInstanceKey: String? } -class SharedPrefsInstanceSelectionStorage(context: Context) : InstanceSelectionStorage { +class SharedPrefsInstanceSelectionStorage(context: Context) : + InstanceSelectionStorage, + LegacyNamespaceOwnerStorage { private val prefs = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE) @@ -22,8 +24,15 @@ class SharedPrefsInstanceSelectionStorage(context: Context) : InstanceSelectionS if (value == null) remove(KEY_SELECTED_INSTANCE) else putString(KEY_SELECTED_INSTANCE, value) } + override var legacyOwnerInstanceKey: String? + get() = prefs.getString(KEY_LEGACY_OWNER, null) + set(value) = prefs.edit(commit = true) { + if (value == null) remove(KEY_LEGACY_OWNER) else putString(KEY_LEGACY_OWNER, value) + } + private companion object { const val PREFS_NAME = "openedx_instance_store" const val KEY_SELECTED_INSTANCE = "selected_instance_key" + const val KEY_LEGACY_OWNER = "legacy_storage_owner_instance_key" } } diff --git a/core/src/main/java/org/openedx/core/instance/StorageNamespace.kt b/core/src/main/java/org/openedx/core/instance/StorageNamespace.kt new file mode 100644 index 000000000..f1d824e0b --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/StorageNamespace.kt @@ -0,0 +1,125 @@ +package org.openedx.core.instance + +import java.net.URI +import java.security.MessageDigest +import java.util.Locale + +/** + * Where one instance's local data lives: preference keys, the database file and the + * downloads folder. + * + * The legacy namespace ([instanceKey] `null`) is the storage a single-site build has always + * used, unprefixed. It belongs to "no instance selected" and to the one instance that is the + * same site as the app-level configuration, so upgrading keeps a signed-in learner's session, + * cached courses and downloads in place. + */ +data class StorageNamespace(val instanceKey: String?) { + + val isLegacy: Boolean get() = instanceKey == null + + /** e.g. `access_token` for the legacy namespace, `instance.acme.access_token` otherwise. */ + fun key(rawKey: String): String = if (instanceKey == null) rawKey else "$PREFIX$instanceKey.$rawKey" + + /** + * A file-system-safe, collision-resistant name segment, or `null` for the legacy namespace. + * The readable part is lossy; the hash keeps two keys that sanitize alike apart. + */ + val fileSegment: String? + get() = instanceKey?.let { key -> + val readable = key.replace(UNSAFE_FILE_CHARS, "_").take(MAX_READABLE_LENGTH) + "${readable}_${sha256(key).take(HASH_LENGTH)}" + } + + companion object { + val LEGACY = StorageNamespace(null) + + private const val PREFIX = "instance." + private const val MAX_READABLE_LENGTH = 32 + private const val HASH_LENGTH = 12 + private val UNSAFE_FILE_CHARS = "[^A-Za-z0-9_-]".toRegex() + + private fun sha256(value: String): String { + val digest = MessageDigest.getInstance("SHA-256").digest(value.toByteArray(Charsets.UTF_8)) + return digest.joinToString("") { "%02x".format(it) } + } + } +} + +/** Resolves instances to the namespace their local data lives under. */ +interface StorageNamespaceProvider { + /** The selected instance's namespace; legacy when nothing is selected. */ + val current: StorageNamespace + + fun namespaceFor(instance: Instance?): StorageNamespace + + /** Resolves by key through the catalog; a key no longer in the catalog keeps its own namespace. */ + fun namespaceFor(instanceKey: String?): StorageNamespace +} + +/** Persists which instance owns the legacy namespace once that has been decided. */ +interface LegacyNamespaceOwnerStorage { + var legacyOwnerInstanceKey: String? +} + +/** + * The instance whose API host and OAuth client are the app-level ones is the site a + * single-site build of this app was signing in to, so it inherits the legacy namespace. That + * decision is recorded the first time it is made and never revisited: a later catalog or + * configuration change can't hand existing data to a different site. + */ +class InstanceStorageNamespaces( + private val instanceStore: InstanceStore, + private val ownerStorage: LegacyNamespaceOwnerStorage, + private val appApiHostUrl: String, + private val appOAuthClientId: String, +) : StorageNamespaceProvider { + + private val lock = Any() + + override val current: StorageNamespace + get() = namespaceFor(instanceStore.currentInstance) + + override fun namespaceFor(instance: Instance?): StorageNamespace { + if (instance == null) return StorageNamespace.LEGACY + val owner = synchronized(lock) { + ownerStorage.legacyOwnerInstanceKey ?: instance.key + .takeIf { isAppLevelSite(instance) } + ?.also { ownerStorage.legacyOwnerInstanceKey = it } + } + return if (owner == instance.key) StorageNamespace.LEGACY else StorageNamespace(instance.key) + } + + override fun namespaceFor(instanceKey: String?): StorageNamespace { + val instance = instanceKey?.let { instanceStore.instance(it) } + val owner = synchronized(lock) { ownerStorage.legacyOwnerInstanceKey } + return when { + instanceKey == null -> StorageNamespace.LEGACY + instance != null -> namespaceFor(instance) + owner == instanceKey -> StorageNamespace.LEGACY + else -> StorageNamespace(instanceKey) + } + } + + private fun isAppLevelSite(instance: Instance): Boolean { + return instance.oAuthClientId == appOAuthClientId && + normalizedOrigin(instance.apiHostUrl) != null && + normalizedOrigin(instance.apiHostUrl) == normalizedOrigin(appApiHostUrl) + } + + private fun normalizedOrigin(url: String): String? = runCatching { + val uri = URI(url.trim()) + val scheme = uri.scheme?.lowercase(Locale.ROOT) ?: return null + val host = uri.host?.lowercase(Locale.ROOT) ?: return null + val port = when { + uri.port != -1 -> uri.port + scheme == "https" -> HTTPS_PORT + else -> HTTP_PORT + } + "$scheme://$host:$port${uri.path.orEmpty().trimEnd('/')}" + }.getOrNull() + + private companion object { + const val HTTPS_PORT = 443 + const val HTTP_PORT = 80 + } +} diff --git a/core/src/test/java/org/openedx/core/instance/StorageNamespaceTest.kt b/core/src/test/java/org/openedx/core/instance/StorageNamespaceTest.kt new file mode 100644 index 000000000..9c84b064e --- /dev/null +++ b/core/src/test/java/org/openedx/core/instance/StorageNamespaceTest.kt @@ -0,0 +1,134 @@ +package org.openedx.core.instance + +import com.google.gson.JsonObject +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class StorageNamespaceTest { + + private class InMemoryStorage( + override var selectedInstanceKey: String? = null, + override var legacyOwnerInstanceKey: String? = null, + ) : InstanceSelectionStorage, LegacyNamespaceOwnerStorage + + private fun instance(key: String, host: String, client: String = "$key-client") = requireNotNull( + Instance.fromJson( + JsonObject().apply { + addProperty("KEY", key) + addProperty("NAME", key) + addProperty("OAUTH_CLIENT_ID", client) + addProperty("API_HOST_URL", host) + } + ) + ) + + private val appSite = instance("app-site", "https://LMS.example.com/", client = "app-client") + private val other = instance("other", "https://other.example.com") + + private fun namespaces(storage: InMemoryStorage, vararg instances: Instance): InstanceStorageNamespaces { + val store = InstanceStore(storage) { InstancesConfig(instances.toList()) } + return InstanceStorageNamespaces(store, storage, "https://lms.example.com:443", "app-client") + } + + @Test + fun `legacy namespace keeps raw keys and has no file segment`() { + assertEquals("access_token", StorageNamespace.LEGACY.key("access_token")) + assertNull(StorageNamespace.LEGACY.fileSegment) + assertTrue(StorageNamespace.LEGACY.isLegacy) + } + + @Test + fun `instance namespace prefixes keys with the instance key`() { + assertEquals("instance.acme.access_token", StorageNamespace("acme").key("access_token")) + } + + @Test + fun `file segments are safe and keep keys that sanitize alike apart`() { + val slash = requireNotNull(StorageNamespace("a/b").fileSegment) + val colon = requireNotNull(StorageNamespace("a:b").fileSegment) + + assertTrue(slash, slash.matches("[A-Za-z0-9_-]+".toRegex())) + assertTrue(slash.startsWith("a_b_")) + assertNotEquals(slash, colon) + assertEquals(slash, StorageNamespace("a/b").fileSegment) + } + + @Test + fun `nothing selected uses the legacy namespace`() { + val namespaces = namespaces(InMemoryStorage(), appSite, other) + + assertEquals(StorageNamespace.LEGACY, namespaces.current) + assertEquals(StorageNamespace.LEGACY, namespaces.namespaceFor(null as String?)) + } + + @Test + fun `the instance matching the app level host and client inherits the legacy namespace`() { + val storage = InMemoryStorage() + val namespaces = namespaces(storage, appSite, other) + + assertEquals(StorageNamespace.LEGACY, namespaces.namespaceFor(appSite)) + assertEquals(StorageNamespace("other"), namespaces.namespaceFor(other)) + assertEquals("app-site", storage.legacyOwnerInstanceKey) + } + + @Test + fun `same host with a different OAuth client is a different site`() { + val sameHost = instance("same-host", "https://lms.example.com", client = "another-client") + val namespaces = namespaces(InMemoryStorage(), sameHost) + + assertEquals(StorageNamespace("same-host"), namespaces.namespaceFor(sameHost)) + } + + @Test + fun `different port or path is a different site`() { + val port = instance("port", "https://lms.example.com:8443", client = "app-client") + val path = instance("path", "https://lms.example.com/tenant", client = "app-client") + val namespaces = namespaces(InMemoryStorage(), port, path) + + assertEquals(StorageNamespace("port"), namespaces.namespaceFor(port)) + assertEquals(StorageNamespace("path"), namespaces.namespaceFor(path)) + } + + @Test + fun `the recorded owner never changes once decided`() { + val storage = InMemoryStorage(legacyOwnerInstanceKey = "other") + val namespaces = namespaces(storage, appSite, other) + + assertEquals(StorageNamespace.LEGACY, namespaces.namespaceFor(other)) + assertEquals(StorageNamespace("app-site"), namespaces.namespaceFor(appSite)) + assertEquals("other", storage.legacyOwnerInstanceKey) + } + + @Test + fun `catalog order does not decide ownership`() { + val first = namespaces(InMemoryStorage(), other, appSite) + val second = namespaces(InMemoryStorage(), appSite, other) + + assertEquals(first.namespaceFor(appSite), second.namespaceFor(appSite)) + assertEquals(first.namespaceFor(other), second.namespaceFor(other)) + } + + @Test + fun `a key no longer in the catalog keeps its own namespace`() { + val storage = InMemoryStorage(legacyOwnerInstanceKey = "app-site") + val namespaces = namespaces(storage, other) + + assertEquals(StorageNamespace("gone"), namespaces.namespaceFor("gone")) + assertEquals(StorageNamespace.LEGACY, namespaces.namespaceFor("app-site")) + } + + @Test + fun `current follows the selection`() { + val storage = InMemoryStorage() + val store = InstanceStore(storage) { InstancesConfig(listOf(appSite, other)) } + val namespaces = InstanceStorageNamespaces(store, storage, "https://lms.example.com", "app-client") + + store.select(other) + assertEquals(StorageNamespace("other"), namespaces.current) + store.select(appSite) + assertEquals(StorageNamespace.LEGACY, namespaces.current) + } +} From 5406ec938044c94005f4ed28bce24ad20c360a54 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Mon, 19 Oct 2026 19:02:14 +0300 Subject: [PATCH 7/7] fix: keep a token refresh on the site that started it The refresh resolved the selected instance twice: once for the tokens and again for the endpoint, so a switch in between could send one site's refresh token to another. The instance is now resolved once per authenticate call and used for the tokens read, the token endpoint and where the result is saved. The minimum interval between refreshes is tracked per site, so refreshing one site no longer holds back another. --- .../OauthRefreshTokenAuthenticator.kt | 54 +++++++++++-------- .../app/data/storage/PreferencesManager.kt | 4 ++ .../networking/InstanceAwareNetworkingTest.kt | 27 ++++++++++ .../core/data/storage/CorePreferences.kt | 2 + 4 files changed, 66 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt b/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt index 10bab6a75..5d8d2c120 100644 --- a/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt +++ b/app/src/main/java/org/openedx/app/data/networking/OauthRefreshTokenAuthenticator.kt @@ -23,6 +23,7 @@ import org.openedx.core.ApiConstants.TOKEN_TYPE_JWT import org.openedx.core.BuildConfig import org.openedx.core.config.Config import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.instance.Instance import org.openedx.core.instance.InstanceProvider import org.openedx.core.system.notifier.app.AppNotifier import org.openedx.core.system.notifier.app.LogoutEvent @@ -42,7 +43,9 @@ class OauthRefreshTokenAuthenticator( ) : Authenticator, Interceptor { private val authApis = ConcurrentHashMap() - private var lastTokenRefreshRequestTime = 0L + + /** Per instance key ("" for none): one site's refresh mustn't hold back another's. */ + private val lastTokenRefreshRequestTimes = ConcurrentHashMap() override fun intercept(chain: Interceptor.Chain): Response { if (isTokenExpired() && isForActiveInstance(chain.request())) { @@ -54,9 +57,9 @@ class OauthRefreshTokenAuthenticator( return chain.proceed(chain.request()) } - /** Token endpoint of the selected instance, or of the app-level host when none is selected. */ - private fun authApi(): AuthApi { - val baseUrl = instanceProvider.currentInstance?.apiHostUrl ?: config.getApiHostURL() + /** Token endpoint of [instance], or of the app-level host when there is none. */ + private fun authApi(instance: Instance?): AuthApi { + val baseUrl = instance?.apiHostUrl ?: config.getApiHostURL() return authApis.getOrPut(baseUrl) { Retrofit.Builder() .baseUrl(baseUrl) @@ -71,18 +74,23 @@ class OauthRefreshTokenAuthenticator( * A 401 for a request sent before the learner switched sites says nothing about the site * that is selected now: refreshing it, or logging out, would act on the wrong session. */ - private fun isForActiveInstance(request: Request): Boolean { + private fun isForActiveInstance(request: Request): Boolean = isFor(request, instanceProvider.currentInstance) + + private fun isFor(request: Request, instance: Instance?): Boolean { val sentFor = request.tag(RequestInstance::class.java) ?: return true - return sentFor.instanceKey == instanceProvider.currentInstance?.key + return sentFor.instanceKey == instance?.key } @Suppress("ReturnCount") @Synchronized override fun authenticate(route: Route?, response: Response): Request? { - if (!isForActiveInstance(response.request)) return null + // Resolved once: the tokens read, the endpoint called and the place the result is saved + // all belong to this site, even if the learner switches while the refresh is in flight. + val instance = instanceProvider.currentInstance + if (!isFor(response.request, instance)) return null - val accessToken = preferencesManager.accessToken - val refreshToken = preferencesManager.refreshToken + val accessToken = preferencesManager.accessToken(instance?.key) + val refreshToken = preferencesManager.refreshToken(instance?.key) if (refreshToken.isEmpty()) return null @@ -90,7 +98,7 @@ class OauthRefreshTokenAuthenticator( val retry = when (errorCode) { TOKEN_EXPIRED_ERROR_MESSAGE, JWT_TOKEN_EXPIRED -> { - handleTokenExpired(response, refreshToken, accessToken) + handleTokenExpired(response, refreshToken, accessToken, instance) } TOKEN_NONEXISTENT_ERROR_MESSAGE, TOKEN_INVALID_GRANT_ERROR_MESSAGE, JWT_INVALID_TOKEN -> { @@ -114,9 +122,14 @@ class OauthRefreshTokenAuthenticator( } // Helper function for handling token expiration logic - private fun handleTokenExpired(response: Response, refreshToken: String, accessToken: String): Request? { + private fun handleTokenExpired( + response: Response, + refreshToken: String, + accessToken: String, + instance: Instance?, + ): Request? { return try { - val newAuth = refreshAccessToken(refreshToken) + val newAuth = refreshAccessToken(refreshToken, instance) if (newAuth != null) { response.request.newBuilder() .header( @@ -125,7 +138,7 @@ class OauthRefreshTokenAuthenticator( ) .build() } else { - val actualToken = preferencesManager.accessToken + val actualToken = preferencesManager.accessToken(instance?.key) if (actualToken != accessToken) { response.request.newBuilder() .header( @@ -165,17 +178,16 @@ class OauthRefreshTokenAuthenticator( return time >= preferencesManager.accessTokenExpiresAt } - private fun canRequestTokenRefresh(): Boolean { - return TimeUtils.getCurrentTime() - lastTokenRefreshRequestTime > - REFRESH_TOKEN_INTERVAL_MINIMUM + private fun canRequestTokenRefresh(instance: Instance?): Boolean { + val last = lastTokenRefreshRequestTimes[instance?.key.orEmpty()] ?: 0L + return TimeUtils.getCurrentTime() - last > REFRESH_TOKEN_INTERVAL_MINIMUM } @Throws(IOException::class) - private fun refreshAccessToken(refreshToken: String): AuthResponse? { + private fun refreshAccessToken(refreshToken: String, instance: Instance?): AuthResponse? { var authResponse: AuthResponse? = null - if (canRequestTokenRefresh()) { - val instance = instanceProvider.currentInstance - val response = authApi().refreshAccessToken( + if (canRequestTokenRefresh(instance)) { + val response = authApi(instance).refreshAccessToken( ApiConstants.TOKEN_TYPE_REFRESH, instance?.oAuthClientId ?: config.getOAuthClientId(), refreshToken, @@ -192,7 +204,7 @@ class OauthRefreshTokenAuthenticator( // the refresh was in flight: the server has already rotated the refresh token, // so dropping these would sign that site out. preferencesManager.saveTokens(instance?.key, newAccessToken, newRefreshToken, newExpireTime) - lastTokenRefreshRequestTime = TimeUtils.getCurrentTime() + lastTokenRefreshRequestTimes[instance?.key.orEmpty()] = TimeUtils.getCurrentTime() } if (instance?.key != instanceProvider.currentInstance?.key) return null } else if (response.code() == 400) { diff --git a/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt b/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt index 87545e3f6..2650776b2 100644 --- a/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt +++ b/app/src/main/java/org/openedx/app/data/storage/PreferencesManager.kt @@ -161,6 +161,10 @@ class PreferencesManager( } } + override fun accessToken(instanceKey: String?): String { + return getEncryptedString(InstanceKeys.accessToken(namespaces.namespaceFor(instanceKey)), "") + } + override fun refreshToken(instanceKey: String?): String { return getEncryptedString(InstanceKeys.refreshToken(namespaces.namespaceFor(instanceKey)), "") } diff --git a/app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt b/app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt index a0fc2fdde..3833b6f81 100644 --- a/app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt +++ b/app/src/test/java/org/openedx/app/data/networking/InstanceAwareNetworkingTest.kt @@ -166,6 +166,10 @@ class InstanceAwareNetworkingTest { override var accessTokenExpiresAt: Long = Long.MAX_VALUE val savedTokens = mutableMapOf() + override fun accessToken(instanceKey: String?) = savedTokens[instanceKey] ?: accessToken + + override fun refreshToken(instanceKey: String?) = refreshToken + override fun saveTokens(instanceKey: String?, accessToken: String, refreshToken: String, expiresAt: Long) { savedTokens[instanceKey] = accessToken } @@ -216,6 +220,29 @@ class InstanceAwareNetworkingTest { assertEquals(mapOf("beta" to "new-access"), prefs.savedTokens) } + @Test + fun `a refresh on one site does not hold back the other site's refresh`() { + val provider = FakeInstanceProvider(alpha) + val tokenHosts = mutableListOf() + val refreshClient = OkHttpClient.Builder().addInterceptor { chain -> + tokenHosts += chain.request().url.host + ok(chain.request(), """{"access_token": "new", "refresh_token": "rotated", "expires_in": 3600}""") + }.build() + val authenticator = OauthRefreshTokenAuthenticator( + mockk(relaxed = true) { every { getAccessTokenType() } returns "JWT" }, + Prefs(), + mockk(relaxed = true), + provider, + refreshClient + ) + + authenticator.authenticate(null, unauthorized(taggedRequest("https://alpha.example.com/api/x", "alpha"))) + provider.select(beta) + authenticator.authenticate(null, unauthorized(taggedRequest("http://beta.example.com:8080/api/x", "beta"))) + + assertEquals(listOf("alpha.example.com", "beta.example.com"), tokenHosts) + } + @Test fun `a 401 for a request sent before a switch neither refreshes nor logs out`() { val refreshClient = OkHttpClient.Builder() diff --git a/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt b/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt index 4b13957bb..6cd94f8a8 100644 --- a/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt +++ b/core/src/main/java/org/openedx/core/data/storage/CorePreferences.kt @@ -24,6 +24,8 @@ interface CorePreferences { /** Clears one instance's session without touching the selected one. */ suspend fun clearSession(instanceKey: String?) + fun accessToken(instanceKey: String?): String + fun refreshToken(instanceKey: String?): String /** Stores refreshed tokens for the site that issued them, whichever site is selected now. */