diff --git a/app/src/main/java/org/openedx/app/AppViewModel.kt b/app/src/main/java/org/openedx/app/AppViewModel.kt index bafddb19b..042aa8683 100644 --- a/app/src/main/java/org/openedx/app/AppViewModel.kt +++ b/app/src/main/java/org/openedx/app/AppViewModel.kt @@ -7,20 +7,20 @@ import androidx.fragment.app.FragmentManager import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.LiveData import androidx.lifecycle.viewModelScope -import androidx.room.RoomDatabase -import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import org.openedx.app.deeplink.DeepLink import org.openedx.app.deeplink.DeepLinkRouter +import org.openedx.app.system.push.PushRegistrations import org.openedx.app.system.push.RefreshFirebaseTokenWorker import org.openedx.app.system.push.SyncFirebaseTokenWorker import org.openedx.core.config.Config import org.openedx.core.data.model.User import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.instance.InstanceSessionManager +import org.openedx.core.instance.InstanceStore import org.openedx.core.system.notifier.DownloadFailed import org.openedx.core.system.notifier.DownloadNotifier import org.openedx.core.system.notifier.app.AppNotifier @@ -36,9 +36,10 @@ import org.openedx.foundation.utils.FileUtil class AppViewModel( private val config: Config, private val appNotifier: AppNotifier, - private val room: RoomDatabase, private val preferencesManager: CorePreferences, - private val dispatcher: CoroutineDispatcher, + private val sessionManager: InstanceSessionManager, + private val instanceStore: InstanceStore, + private val pushRegistrations: PushRegistrations, private val analytics: AppAnalytics, private val deepLinkRouter: DeepLinkRouter, private val fileUtil: FileUtil, @@ -69,8 +70,8 @@ class AppViewModel( setUserId(user) - if (user != null && preferencesManager.pushToken.isNotEmpty()) { - SyncFirebaseTokenWorker.schedule(context) + if (preferencesManager.pushToken.isNotEmpty()) { + SyncFirebaseTokenWorker.scheduleForSignedInSites(context, pushRegistrations) } if (canResetAppDirectory) { @@ -80,7 +81,8 @@ class AppViewModel( viewModelScope.launch { appNotifier.notifier.collect { event -> if (event is SignInEvent && config.getFirebaseConfig().isCloudMessagingEnabled) { - SyncFirebaseTokenWorker.schedule(context) + // Sign-in completes only on the site that is still selected. + SyncFirebaseTokenWorker.schedule(context, instanceStore.currentInstance?.key) } else if (event is LogoutEvent) { handleLogoutEvent(event) } @@ -123,10 +125,8 @@ class AppViewModel( if (System.currentTimeMillis() - logoutHandledAt > LOGOUT_EVENT_THRESHOLD) { if (event.isForced) { logoutHandledAt = System.currentTimeMillis() - preferencesManager.clearCorePreferences() - withContext(dispatcher) { - room.clearAllTables() - } + // Signs out of the selected site only; other sites keep their sessions. + sessionManager.logoutCurrentInstance() analytics.logoutEvent(true) _logoutUser.value = Unit } diff --git a/app/src/main/java/org/openedx/app/data/api/NotificationsApi.kt b/app/src/main/java/org/openedx/app/data/api/NotificationsApi.kt index 9106944c3..0c005e076 100644 --- a/app/src/main/java/org/openedx/app/data/api/NotificationsApi.kt +++ b/app/src/main/java/org/openedx/app/data/api/NotificationsApi.kt @@ -1,14 +1,17 @@ package org.openedx.app.data.api +import org.openedx.core.instance.RequestInstance import retrofit2.http.Field import retrofit2.http.FormUrlEncoded import retrofit2.http.POST +import retrofit2.http.Tag interface NotificationsApi { @POST("/api/mobile/v4/notifications/create-token/") @FormUrlEncoded suspend fun syncFirebaseToken( @Field("registration_id") token: String, - @Field("active") active: Boolean = true + @Field("active") active: Boolean, + @Tag instance: RequestInstance, ) } 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 6feeb9e25..e2d03191e 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 @@ -2,10 +2,13 @@ package org.openedx.app.data.networking import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.Interceptor +import okhttp3.Request import okhttp3.Response 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.instance.RequestInstance import org.openedx.core.presentation.global.AppData class HeadersInterceptor( @@ -13,36 +16,44 @@ class HeadersInterceptor( private val config: Config, private val preferencesManager: CorePreferences, private val instanceProvider: InstanceProvider, + private val instanceByKey: (String) -> Instance? = { null }, ) : Interceptor { override fun intercept(chain: Interceptor.Chain): Response = chain.run { + val request = request() proceed( - request() - .newBuilder().apply { - val token = preferencesManager.accessToken + request.newBuilder().apply { + // The token of the site the request belongs to, and only to that site's host. + val site = request.tag(RequestInstance::class.java) + ?: RequestInstance(instanceProvider.currentInstance?.key) + val token = preferencesManager.accessToken(site.instanceKey) + if (token.isNotEmpty() && request.header(AUTHORIZATION) == null && targets(request, site)) { + addHeader(AUTHORIZATION, "${config.getAccessTokenType()} $token") + } - // 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") - } + addHeader("Accept", "application/json") - addHeader("Accept", "application/json") - - val httpAgent = System.getProperty("http.agent") ?: "" - addHeader("User-Agent", "$httpAgent ${appData.appUserAgent}") - }.build() + val httpAgent = System.getProperty("http.agent") ?: "" + addHeader("User-Agent", "$httpAgent ${appData.appUserAgent}") + }.build() ) } - private fun targetsActiveSite(url: String): Boolean { - val siteUrl = instanceProvider.currentInstance?.apiHostUrl ?: config.getApiHostURL() - val site = siteUrl.toHttpUrlOrNull() - val request = url.toHttpUrlOrNull() + private fun targets(request: Request, site: RequestInstance): Boolean { + val siteUrl = when (val key = site.instanceKey) { + null -> config.getApiHostURL() + // A site no longer in the catalog has no known host: send its token nowhere. + else -> instanceByKey(key)?.apiHostUrl ?: return false + } + val siteHost = siteUrl.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) + siteHost == null -> true + else -> request.url.isSameOrigin(siteHost) } } + + private companion object { + const val AUTHORIZATION = "Authorization" + } } 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 index 8d680ba14..76012b687 100644 --- a/app/src/main/java/org/openedx/app/data/networking/InstanceHostInterceptor.kt +++ b/app/src/main/java/org/openedx/app/data/networking/InstanceHostInterceptor.kt @@ -5,14 +5,9 @@ import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.Interceptor import okhttp3.Request import okhttp3.Response +import org.openedx.core.instance.Instance 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?) +import org.openedx.core.instance.RequestInstance /** * Re-targets requests built against the app's base URL at the selected instance: scheme, host @@ -22,16 +17,19 @@ data class RequestInstance(val instanceKey: String?) class InstanceHostInterceptor( appBaseUrl: String, private val instanceProvider: InstanceProvider, + private val instanceByKey: (String) -> Instance? = { null }, ) : 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) + // A request pinned to a site by its caller goes there; any other goes to the selected one. + val pinned = request.tag(RequestInstance::class.java) + val target = if (pinned != null) pinned.instanceKey?.let(instanceByKey) else instanceProvider.currentInstance + val rewritten = rewriteHostIfNeeded(request, target?.apiHostUrl) .newBuilder() - .tag(RequestInstance::class.java, RequestInstance(instance?.key)) + .tag(RequestInstance::class.java, pinned ?: RequestInstance(target?.key)) .build() return chain.proceed(rewritten) } 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 5d8d2c120..70e524000 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 @@ -25,6 +25,7 @@ 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.instance.RequestInstance import org.openedx.core.system.notifier.app.AppNotifier import org.openedx.core.system.notifier.app.LogoutEvent import org.openedx.core.utils.TimeUtils 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 2650776b2..d7ba7c4d6 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 @@ -72,11 +72,11 @@ class PreferencesManager( 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 lastSignedInUsername(ns: StorageNamespace) = stringPreferencesKey(ns.key("last_signed_in_username")) fun isRelativeDatesEnabled(ns: StorageNamespace) = booleanPreferencesKey(ns.key("is_relative_dates_enabled")) fun calendarId(ns: StorageNamespace) = longPreferencesKey(ns.key("calendar_id")) @@ -94,6 +94,7 @@ class PreferencesManager( get() = namespaces.current private object Keys { + val PUSH_TOKEN = stringPreferencesKey("push_token") val VIDEO_SETTINGS_WIFI_DOWNLOAD_ONLY = booleanPreferencesKey("video_settings_wifi_download_only") val VIDEO_SETTINGS_STREAMING_QUALITY = @@ -154,7 +155,6 @@ class PreferencesManager( dataStore.edit { prefs -> 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)) @@ -176,6 +176,14 @@ class PreferencesManager( setValue(InstanceKeys.expiresIn(namespace), expiresAt) } + override fun saveAccessToken(instanceKey: String?, accessToken: String) { + setEncryptedString(InstanceKeys.accessToken(namespaces.namespaceFor(instanceKey)), accessToken) + } + + override fun saveUser(instanceKey: String?, user: User?) { + setEncryptedString(InstanceKeys.user(namespaces.namespaceFor(instanceKey)), Gson().toJson(user)) + } + override suspend fun clearCalendarPreferences() { val namespace = ns dataStore.edit { prefs -> @@ -195,8 +203,8 @@ class PreferencesManager( set(value) = setEncryptedString(InstanceKeys.refreshToken(ns), value) override var pushToken: String - get() = getEncryptedString(InstanceKeys.pushToken(ns), "") - set(value) = setEncryptedString(InstanceKeys.pushToken(ns), value) + get() = getEncryptedString(Keys.PUSH_TOKEN, "") + set(value) = setEncryptedString(Keys.PUSH_TOKEN, value) override var accessTokenExpiresAt: Long get() = getValue(InstanceKeys.expiresIn(ns), 0L) @@ -247,6 +255,14 @@ class PreferencesManager( get() = getValue(InstanceKeys.isRelativeDatesEnabled(ns), true) set(value) = setValue(InstanceKeys.isRelativeDatesEnabled(ns), value) + override fun lastSignedInUsername(instanceKey: String?): String { + return getValue(InstanceKeys.lastSignedInUsername(namespaces.namespaceFor(instanceKey)), "") + } + + override fun setLastSignedInUsername(instanceKey: String?, username: String) { + setValue(InstanceKeys.lastSignedInUsername(namespaces.namespaceFor(instanceKey)), username) + } + override var profile: Account? get() { val json = getEncryptedString(InstanceKeys.account(ns), "") 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 6bdf62233..401334918 100644 --- a/app/src/main/java/org/openedx/app/di/AppModule.kt +++ b/app/src/main/java/org/openedx/app/di/AppModule.kt @@ -1,7 +1,6 @@ package org.openedx.app.di import androidx.appcompat.app.AppCompatActivity -import androidx.room.RoomDatabase import com.google.android.play.core.review.ReviewManagerFactory import com.google.gson.Gson import com.google.gson.GsonBuilder @@ -16,7 +15,6 @@ import org.openedx.app.BuildConfig import org.openedx.app.PluginManager import org.openedx.app.data.storage.PreferencesManager import org.openedx.app.deeplink.DeepLinkRouter -import org.openedx.app.room.AppDatabase import org.openedx.app.room.DatabaseManager import org.openedx.app.room.InstanceDatabaseProvider import org.openedx.app.room.InstanceScopedCalendarDao @@ -25,6 +23,7 @@ import org.openedx.app.room.InstanceScopedDashboardDao import org.openedx.app.room.InstanceScopedDatesDao import org.openedx.app.room.InstanceScopedDiscoveryDao import org.openedx.app.room.InstanceScopedDownloadDao +import org.openedx.app.system.push.PushRegistrations import org.openedx.auth.presentation.AgreementProvider import org.openedx.auth.presentation.AuthAnalytics import org.openedx.auth.presentation.AuthRouter @@ -41,12 +40,14 @@ import org.openedx.core.data.storage.CorePreferences import org.openedx.core.data.storage.CourseDao import org.openedx.core.data.storage.InAppReviewPreferences import org.openedx.core.domain.helper.VideoPreviewHelper +import org.openedx.core.instance.DefaultInstanceSessionManager import org.openedx.core.instance.InstanceApiService import org.openedx.core.instance.InstanceCatalogCache import org.openedx.core.instance.InstanceConfigLoader import org.openedx.core.instance.InstanceDatabases import org.openedx.core.instance.InstanceProvider import org.openedx.core.instance.InstanceSelectionStorage +import org.openedx.core.instance.InstanceSessionManager import org.openedx.core.instance.InstanceStorageNamespaces import org.openedx.core.instance.InstanceStore import org.openedx.core.instance.OkHttpInstanceApiService @@ -129,6 +130,17 @@ val appModule = module { single { SharedPrefsInstanceCatalogCache(get()) } single { OkHttpInstanceApiService(get().getInstancesCatalogUrl()) } single { InstanceConfigLoader(get(), get(), { get().getInstancesConfig() }) } + single { PushRegistrations(get(), get(), get(), get()) } + single { + DefaultInstanceSessionManager( + get(), + get(), + get(), + get(), + get(), + pushTokenUnregistrar = get() + ) + } single { PreferencesManager(get(), get(), get()) } single { get() } single { get() } @@ -142,7 +154,7 @@ val appModule = module { single { ReviewManagerFactory.create(get()) } single { CalendarManager(get(), get()) } single { DownloadDialogManager(get(), get(), get(), get()) } - single { DatabaseManager(get(), get(), get(), get()) } + single { DatabaseManager(get(), get()) } single { get() } single { ImageProcessor(get()) } @@ -184,9 +196,6 @@ val appModule = module { single { InstanceDatabaseProvider(androidApplication(), get(), get()) } single { get() } - // Resolved on every injection: the database belongs to whichever instance is selected. - factory { get().current } - factory { get() } single { InstanceScopedDiscoveryDao(get()) } single { InstanceScopedCourseDao(get()) } 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 412f99a64..b4ea69c02 100644 --- a/app/src/main/java/org/openedx/app/di/NetworkingModule.kt +++ b/app/src/main/java/org/openedx/app/di/NetworkingModule.kt @@ -14,6 +14,7 @@ import org.openedx.core.BuildConfig import org.openedx.core.config.Config import org.openedx.core.data.api.CookiesApi import org.openedx.core.data.api.CourseApi +import org.openedx.core.instance.InstanceStore import org.openedx.discovery.data.api.DiscoveryApi import org.openedx.discussion.data.api.DiscussionApi import org.openedx.profile.data.api.ProfileApi @@ -24,14 +25,17 @@ import java.util.concurrent.TimeUnit val networkingModule = module { single { OauthRefreshTokenAuthenticator(get(), get(), get(), get()) } - single { InstanceHostInterceptor(get().getApiHostURL(), get()) } + single { + val store = get() + InstanceHostInterceptor(get().getApiHostURL(), store) { store.instance(it) } + } single { OkHttpClient.Builder().apply { writeTimeout(60, TimeUnit.SECONDS) readTimeout(60, TimeUnit.SECONDS) addInterceptor(get()) - addInterceptor(HeadersInterceptor(get(), get(), get(), get())) + addInterceptor(HeadersInterceptor(get(), get(), get(), get()) { get().instance(it) }) if (BuildConfig.DEBUG) { addNetworkInterceptor(HttpLoggingInterceptor().setLevel(HttpLoggingInterceptor.Level.BODY)) } diff --git a/app/src/main/java/org/openedx/app/di/ScreenModule.kt b/app/src/main/java/org/openedx/app/di/ScreenModule.kt index 2ee5a6284..129c5b61c 100644 --- a/app/src/main/java/org/openedx/app/di/ScreenModule.kt +++ b/app/src/main/java/org/openedx/app/di/ScreenModule.kt @@ -2,7 +2,6 @@ package org.openedx.app.di import android.content.res.Resources import org.koin.core.module.dsl.viewModel -import org.koin.core.qualifier.named import org.koin.dsl.module import org.openedx.app.AppViewModel import org.openedx.app.MainViewModel @@ -90,9 +89,10 @@ val screenModule = module { AppViewModel( config = get(), appNotifier = get(), - room = get(), preferencesManager = get(), - dispatcher = get(named("IODispatcher")), + sessionManager = get(), + instanceStore = get(), + pushRegistrations = get(), analytics = get(), deepLinkRouter = get(), fileUtil = get(), @@ -103,7 +103,7 @@ val screenModule = module { } viewModel { MainViewModel(get(), get(), get(), get(), get()) } - factory { AuthRepository(get(), get(), get()) } + factory { AuthRepository(get(), get(), get(), get(), get()) } factory { AuthInteractor(get()) } factory { Validator() } @@ -205,7 +205,7 @@ val screenModule = module { ) } - factory { ProfileRepository(get(), get(), get(), get(), get()) } + factory { ProfileRepository(get(), get(), get(), get(), get(), get()) } factory { ProfileInteractor(get()) } viewModel { ProfileViewModel( @@ -250,7 +250,6 @@ val screenModule = module { get(), get(), get(), - get(), ) } viewModel { diff --git a/app/src/main/java/org/openedx/app/room/DatabaseManager.kt b/app/src/main/java/org/openedx/app/room/DatabaseManager.kt index 0c3087abf..b1be65c97 100644 --- a/app/src/main/java/org/openedx/app/room/DatabaseManager.kt +++ b/app/src/main/java/org/openedx/app/room/DatabaseManager.kt @@ -4,23 +4,28 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import org.openedx.core.DatabaseManager -import org.openedx.core.data.storage.CourseDao -import org.openedx.core.module.db.DownloadDao -import org.openedx.dashboard.data.DashboardDao -import org.openedx.discovery.data.storage.DiscoveryDao +import org.openedx.core.instance.StorageNamespaceProvider class DatabaseManager( - private val courseDao: CourseDao, - private val dashboardDao: DashboardDao, - private val downloadDao: DownloadDao, - private val discoveryDao: DiscoveryDao + private val databases: InstanceDatabaseProvider, + private val namespaces: StorageNamespaceProvider, ) : DatabaseManager { override fun clearTables() { + // Resolved now, not when the coroutine runs: a switch in between must not clear the + // other site's cache. + clear(databases.current) + } + + override fun clearTables(instanceKey: String?) { + clear(databases.database(namespaces.namespaceFor(instanceKey))) + } + + private fun clear(database: AppDatabase) { CoroutineScope(Dispatchers.IO).launch { - courseDao.clearCachedData() - dashboardDao.clearCachedData() - downloadDao.clearOfflineProgress() - discoveryDao.clearCachedData() + database.courseDao().clearCachedData() + database.dashboardDao().clearCachedData() + database.downloadDao().clearOfflineProgress() + database.discoveryDao().clearCachedData() } } } diff --git a/app/src/main/java/org/openedx/app/system/push/OpenEdXFirebaseMessagingService.kt b/app/src/main/java/org/openedx/app/system/push/OpenEdXFirebaseMessagingService.kt index 52caf4de7..339cffb02 100644 --- a/app/src/main/java/org/openedx/app/system/push/OpenEdXFirebaseMessagingService.kt +++ b/app/src/main/java/org/openedx/app/system/push/OpenEdXFirebaseMessagingService.kt @@ -20,6 +20,7 @@ import org.openedx.core.data.storage.CorePreferences class OpenEdXFirebaseMessagingService : FirebaseMessagingService() { private val preferences: CorePreferences by inject() + private val registrations: PushRegistrations by inject() private val config: Config by inject() override fun onMessageReceived(message: RemoteMessage) { @@ -37,9 +38,8 @@ class OpenEdXFirebaseMessagingService : FirebaseMessagingService() { override fun onNewToken(token: String) { super.onNewToken(token) preferences.pushToken = token - if (preferences.user != null) { - SyncFirebaseTokenWorker.schedule(this) - } + // The device token changed: every signed-in site needs the new one. + SyncFirebaseTokenWorker.scheduleForSignedInSites(this, registrations) } private fun handlePushNotification(message: RemoteMessage) { diff --git a/app/src/main/java/org/openedx/app/system/push/PushRegistrations.kt b/app/src/main/java/org/openedx/app/system/push/PushRegistrations.kt new file mode 100644 index 000000000..02402c79a --- /dev/null +++ b/app/src/main/java/org/openedx/app/system/push/PushRegistrations.kt @@ -0,0 +1,51 @@ +package org.openedx.app.system.push + +import org.openedx.app.data.api.NotificationsApi +import org.openedx.core.config.Config +import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.instance.InstanceStore +import org.openedx.core.instance.PushTokenUnregistering +import org.openedx.core.instance.RequestInstance + +/** + * The device has one push token; each site the learner is signed in to registers it with its + * own backend. Registering and unregistering are pinned to one site, so they reach that site's + * host with that site's session even if the learner switches sites while they run. + */ +class PushRegistrations( + private val api: NotificationsApi, + private val preferences: CorePreferences, + private val config: Config, + private val instanceStore: InstanceStore, +) : PushTokenUnregistering { + + private val isEnabled: Boolean + get() = config.getFirebaseConfig().isCloudMessagingEnabled + + /** The sites with a session: catalog sites, or the single-site session (`null`) with no catalog. */ + fun signedInSites(): List { + val catalog = instanceStore.instancesConfig.instances + return if (catalog.isEmpty()) { + if (preferences.hasSession(null)) listOf(null) else emptyList() + } else { + catalog.map { it.key }.filter { preferences.hasSession(it) } + } + } + + /** Registers this device's token with one site; does nothing for a site without a session. */ + suspend fun register(instanceKey: String?): Boolean { + val token = preferences.pushToken + if (!isEnabled || token.isEmpty() || !preferences.hasSession(instanceKey)) return false + api.syncFirebaseToken(token, active = true, instance = RequestInstance(instanceKey)) + return true + } + + override suspend fun unregisterPushToken(instanceKey: String?) { + val token = preferences.pushToken + if (!isEnabled || token.isEmpty() || !preferences.hasSession(instanceKey)) return + // Best effort: signing out must not fail because the site is unreachable. + runCatching { + api.syncFirebaseToken(token, active = false, instance = RequestInstance(instanceKey)) + } + } +} diff --git a/app/src/main/java/org/openedx/app/system/push/RefreshFirebaseTokenWorker.kt b/app/src/main/java/org/openedx/app/system/push/RefreshFirebaseTokenWorker.kt index 0f37f36e3..cbd8fe1ee 100644 --- a/app/src/main/java/org/openedx/app/system/push/RefreshFirebaseTokenWorker.kt +++ b/app/src/main/java/org/openedx/app/system/push/RefreshFirebaseTokenWorker.kt @@ -12,19 +12,27 @@ import org.koin.core.component.KoinComponent import org.koin.core.component.inject import org.openedx.core.data.storage.CorePreferences +/** + * Rotates the device's push token after a sign-out, so a backend that missed the unregistration + * can no longer reach this device. The token is device-wide, so this is device-level work with + * one unique name. It rotates only when no site is still signed in: rotating would cut off the + * sites that still use the token. Any site signed in by the time the new token arrives gets it. + */ class RefreshFirebaseTokenWorker(context: Context, params: WorkerParameters) : CoroutineWorker(context, params), KoinComponent { private val preferences: CorePreferences by inject() + private val registrations: PushRegistrations by inject() override suspend fun doWork(): Result { - FirebaseMessaging.getInstance().deleteToken().await() + if (registrations.signedInSites().isNotEmpty()) return Result.success() + FirebaseMessaging.getInstance().deleteToken().await() val newPushToken = FirebaseMessaging.getInstance().getToken().await() - preferences.pushToken = newPushToken + SyncFirebaseTokenWorker.scheduleForSignedInSites(applicationContext, registrations) return Result.success() } diff --git a/app/src/main/java/org/openedx/app/system/push/SyncFirebaseTokenWorker.kt b/app/src/main/java/org/openedx/app/system/push/SyncFirebaseTokenWorker.kt index 6c45a3ab4..953314843 100644 --- a/app/src/main/java/org/openedx/app/system/push/SyncFirebaseTokenWorker.kt +++ b/app/src/main/java/org/openedx/app/system/push/SyncFirebaseTokenWorker.kt @@ -6,40 +6,43 @@ import androidx.work.ExistingWorkPolicy import androidx.work.OneTimeWorkRequest import androidx.work.WorkManager import androidx.work.WorkerParameters +import androidx.work.workDataOf import org.koin.core.component.KoinComponent import org.koin.core.component.inject -import org.openedx.app.data.api.NotificationsApi -import org.openedx.core.data.storage.CorePreferences +/** Registers the device's push token with one site, the one it was scheduled for. */ class SyncFirebaseTokenWorker(context: Context, params: WorkerParameters) : CoroutineWorker(context, params), KoinComponent { - private val preferences: CorePreferences by inject() - private val api: NotificationsApi by inject() + private val registrations: PushRegistrations by inject() override suspend fun doWork(): Result { - if (preferences.user != null && preferences.pushToken.isNotEmpty()) { - api.syncFirebaseToken(preferences.pushToken) - - return Result.success() - } - return Result.failure() + val instanceKey = inputData.getString(KEY_INSTANCE_KEY) + registrations.register(instanceKey) + return Result.success() } companion object { private const val WORKER_TAG = "SyncFirebaseTokenWorker" + private const val KEY_INSTANCE_KEY = "instance_key" - fun schedule(context: Context) { + /** One unique work per site: a sync for one site never replaces another site's. */ + fun schedule(context: Context, instanceKey: String?) { val work = OneTimeWorkRequest .Builder(SyncFirebaseTokenWorker::class.java) .addTag(WORKER_TAG) + .setInputData(workDataOf(KEY_INSTANCE_KEY to instanceKey)) .build() WorkManager.getInstance(context).beginUniqueWork( - WORKER_TAG, + "$WORKER_TAG:${instanceKey.orEmpty()}", ExistingWorkPolicy.REPLACE, work ).enqueue() } + + fun scheduleForSignedInSites(context: Context, registrations: PushRegistrations) { + registrations.signedInSites().forEach { schedule(context, it) } + } } } diff --git a/app/src/test/java/org/openedx/AppViewModelTest.kt b/app/src/test/java/org/openedx/AppViewModelTest.kt index d661a1c17..c2a914b83 100644 --- a/app/src/test/java/org/openedx/AppViewModelTest.kt +++ b/app/src/test/java/org/openedx/AppViewModelTest.kt @@ -27,10 +27,12 @@ import org.openedx.app.AppAnalytics import org.openedx.app.AppViewModel import org.openedx.app.data.storage.PreferencesManager import org.openedx.app.deeplink.DeepLinkRouter -import org.openedx.app.room.AppDatabase +import org.openedx.app.system.push.PushRegistrations import org.openedx.core.CoreMocks import org.openedx.core.config.Config import org.openedx.core.config.FirebaseConfig +import org.openedx.core.instance.InstanceSessionManager +import org.openedx.core.instance.InstanceStore import org.openedx.core.system.notifier.DownloadNotifier import org.openedx.core.system.notifier.app.AppNotifier import org.openedx.core.system.notifier.app.LogoutEvent @@ -47,7 +49,9 @@ class AppViewModelTest { private val config = mockk() private val notifier = mockk() - private val room = mockk() + private val sessionManager = mockk() + private val instanceStore = mockk(relaxed = true) + private val pushRegistrations = mockk(relaxed = true) private val preferencesManager = mockk() private val analytics = mockk() private val fileUtil = mockk() @@ -78,9 +82,10 @@ class AppViewModelTest { val viewModel = AppViewModel( config, notifier, - room, preferencesManager, - dispatcher, + sessionManager, + instanceStore, + pushRegistrations, analytics, deepLinkRouter, fileUtil, @@ -103,10 +108,9 @@ class AppViewModelTest { every { notifier.notifier } returns flow { emit(LogoutEvent(true)) } - coEvery { preferencesManager.clearCorePreferences() } returns Unit + coEvery { sessionManager.logoutCurrentInstance() } returns Unit every { analytics.setUserIdForSession(any()) } returns Unit every { preferencesManager.user } returns CoreMocks.mockUser - every { room.clearAllTables() } returns Unit every { analytics.logoutEvent(true) } returns Unit every { preferencesManager.canResetAppDirectory } returns false every { preferencesManager.pushToken } returns "" @@ -115,9 +119,10 @@ class AppViewModelTest { val viewModel = AppViewModel( config, notifier, - room, preferencesManager, - dispatcher, + sessionManager, + instanceStore, + pushRegistrations, analytics, deepLinkRouter, fileUtil, @@ -142,10 +147,9 @@ class AppViewModelTest { emit(LogoutEvent(true)) emit(LogoutEvent(true)) } - coEvery { preferencesManager.clearCorePreferences() } returns Unit + coEvery { sessionManager.logoutCurrentInstance() } returns Unit every { analytics.setUserIdForSession(any()) } returns Unit every { preferencesManager.user } returns CoreMocks.mockUser - every { room.clearAllTables() } returns Unit every { analytics.logoutEvent(true) } returns Unit every { preferencesManager.canResetAppDirectory } returns false every { preferencesManager.pushToken } returns "" @@ -154,9 +158,10 @@ class AppViewModelTest { val viewModel = AppViewModel( config, notifier, - room, preferencesManager, - dispatcher, + sessionManager, + instanceStore, + pushRegistrations, analytics, deepLinkRouter, fileUtil, @@ -172,10 +177,9 @@ class AppViewModelTest { advanceUntilIdle() verify(exactly = 1) { analytics.logoutEvent(true) } - coVerify(exactly = 1) { preferencesManager.clearCorePreferences() } + coVerify(exactly = 1) { sessionManager.logoutCurrentInstance() } verify(exactly = 1) { analytics.setUserIdForSession(any()) } verify(exactly = 1) { preferencesManager.user } - verify(exactly = 1) { room.clearAllTables() } verify(exactly = 1) { analytics.logoutEvent(true) } } } 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 3833b6f81..46f0ed0c9 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 @@ -23,6 +23,7 @@ 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.instance.RequestInstance import org.openedx.core.presentation.global.AppData import org.openedx.core.system.notifier.app.AppNotifier import org.openedx.core.system.notifier.app.LogoutEvent @@ -126,14 +127,18 @@ class InstanceAwareNetworkingTest { // region HeadersInterceptor + private val catalog = listOf(alpha, beta).associateBy { it.key } + 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 preferences = mockk { + every { accessToken(any()) } answers { "token-${firstArg() ?: "app"}" } + } val appData = AppData(appName = "app", versionName = "1", applicationId = "org.openedx.app") - return HeadersInterceptor(appData, config, preferences, provider) + return HeadersInterceptor(appData, config, preferences, provider) { catalog[it] } } @Test @@ -143,7 +148,7 @@ class InstanceAwareNetworkingTest { 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")) + assertEquals("JWT token-alpha", toSite.header("Authorization")) assertNull(elsewhere.header("Authorization")) } @@ -153,7 +158,41 @@ class InstanceAwareNetworkingTest { val forwarded = forward(interceptor, Request.Builder().url("https://app.example.com/api/x").build()) - assertEquals("JWT token-a", forwarded.header("Authorization")) + assertEquals("JWT token-app", forwarded.header("Authorization")) + } + + @Test + fun `a request pinned to a site goes there with that site's token whichever site is selected`() { + val provider = FakeInstanceProvider(beta) + val chain = OkHttpClient.Builder() + .addInterceptor(InstanceHostInterceptor(appBase, provider) { catalog[it] }) + .addInterceptor(headersInterceptor(provider)) + var forwarded: Request? = null + chain.addInterceptor { + forwarded = it.request() + ok(it.request(), "{}") + } + val request = Request.Builder() + .url("https://app.example.com/api/mobile/v4/notifications/create-token/") + .tag(RequestInstance::class.java, RequestInstance("alpha")) + .build() + + chain.build().newCall(request).execute().close() + + assertEquals("alpha.example.com", forwarded?.url?.host) + assertEquals("JWT token-alpha", forwarded?.header("Authorization")) + assertEquals(RequestInstance("alpha"), forwarded?.tag(RequestInstance::class.java)) + } + + @Test + fun `a request pinned to a site no longer in the catalog carries no token`() { + val interceptor = headersInterceptor(FakeInstanceProvider(beta)) + val request = Request.Builder() + .url("https://app.example.com/api/x") + .tag(RequestInstance::class.java, RequestInstance("gone")) + .build() + + assertNull(forward(interceptor, request).header("Authorization")) } // endregion diff --git a/app/src/test/java/org/openedx/app/system/push/PushRegistrationsTest.kt b/app/src/test/java/org/openedx/app/system/push/PushRegistrationsTest.kt new file mode 100644 index 000000000..5a04e642a --- /dev/null +++ b/app/src/test/java/org/openedx/app/system/push/PushRegistrationsTest.kt @@ -0,0 +1,99 @@ +package org.openedx.app.system.push + +import com.google.gson.JsonObject +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Test +import org.openedx.app.data.api.NotificationsApi +import org.openedx.core.config.Config +import org.openedx.core.config.FirebaseConfig +import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.instance.Instance +import org.openedx.core.instance.InstanceSelectionStorage +import org.openedx.core.instance.InstanceStore +import org.openedx.core.instance.InstancesConfig +import org.openedx.core.instance.RequestInstance +import java.io.IOException + +class PushRegistrationsTest { + + private fun site(key: String) = requireNotNull( + Instance.fromJson( + JsonObject().apply { + addProperty("KEY", key) + addProperty("NAME", key) + addProperty("OAUTH_CLIENT_ID", "$key-client") + addProperty("API_HOST_URL", "https://$key.example.com") + } + ) + ) + + private val alpha = site("alpha") + private val beta = site("beta") + private val api = mockk(relaxed = true) + private val preferences = mockk { + every { pushToken } returns "device-token" + every { hasSession(any()) } answers { firstArg() in setOf("alpha", "beta") } + } + private val config = mockk { + every { getFirebaseConfig() } returns FirebaseConfig(isCloudMessagingEnabled = true) + } + + private fun store(vararg sites: Instance) = InstanceStore( + object : InstanceSelectionStorage { + override var selectedInstanceKey: String? = null + } + ) { InstancesConfig(sites.toList()) } + + @Test + fun `unregistering targets the site it is for, not the selected one`() = runBlocking { + val store = store(alpha, beta) + store.select(beta) + + PushRegistrations(api, preferences, config, store).unregisterPushToken("alpha") + + coVerify { api.syncFirebaseToken("device-token", active = false, instance = RequestInstance("alpha")) } + coVerify(exactly = 0) { api.syncFirebaseToken(any(), any(), RequestInstance("beta")) } + } + + @Test + fun `an unreachable site does not fail the sign-out`() = runBlocking { + coEvery { api.syncFirebaseToken(any(), any(), any()) } throws IOException("offline") + + PushRegistrations(api, preferences, config, store(alpha)).unregisterPushToken("alpha") + } + + @Test + fun `registering is pinned to its site and skipped without a session`() = runBlocking { + val registrations = PushRegistrations(api, preferences, config, store(alpha, beta, site("gamma"))) + + registrations.register("beta") + registrations.register("gamma") + + coVerify { api.syncFirebaseToken("device-token", active = true, instance = RequestInstance("beta")) } + coVerify(exactly = 0) { api.syncFirebaseToken(any(), any(), RequestInstance("gamma")) } + } + + @Test + fun `nothing is sent when cloud messaging is off`() = runBlocking { + every { config.getFirebaseConfig() } returns FirebaseConfig(isCloudMessagingEnabled = false) + + PushRegistrations(api, preferences, config, store(alpha)).unregisterPushToken("alpha") + + coVerify(exactly = 0) { api.syncFirebaseToken(any(), any(), any()) } + } + + @Test + fun `signed in sites come from the catalog, or the single-site session without one`() { + assertEquals( + listOf("alpha", "beta"), + PushRegistrations(api, preferences, config, store(alpha, site("gamma"), beta)).signedInSites() + ) + every { preferences.hasSession(null) } returns true + assertEquals(listOf(null), PushRegistrations(api, preferences, config, store()).signedInSites()) + } +} diff --git a/auth/src/main/java/org/openedx/auth/data/api/AuthApi.kt b/auth/src/main/java/org/openedx/auth/data/api/AuthApi.kt index 673168c57..a9cc4f20f 100644 --- a/auth/src/main/java/org/openedx/auth/data/api/AuthApi.kt +++ b/auth/src/main/java/org/openedx/auth/data/api/AuthApi.kt @@ -6,6 +6,7 @@ import org.openedx.auth.data.model.RegistrationFields import org.openedx.auth.data.model.ValidationFields import org.openedx.core.ApiConstants import org.openedx.core.data.model.User +import org.openedx.core.instance.RequestInstance import retrofit2.Call import retrofit2.http.Field import retrofit2.http.FieldMap @@ -13,6 +14,7 @@ import retrofit2.http.FormUrlEncoded import retrofit2.http.GET import retrofit2.http.POST import retrofit2.http.Path +import retrofit2.http.Tag interface AuthApi { @@ -24,6 +26,7 @@ interface AuthApi { @Field("token_type") tokenType: String, @Field("asymmetric_jwt") isAsymmetricJwt: Boolean = true, @Path("auth_type") authType: String, + @Tag instance: RequestInstance, ): AuthResponse @FormUrlEncoded @@ -35,6 +38,7 @@ interface AuthApi { @Field("password") password: String, @Field("token_type") tokenType: String, @Field("asymmetric_jwt") isAsymmetricJwt: Boolean = true, + @Tag instance: RequestInstance, ): AuthResponse @FormUrlEncoded @@ -59,7 +63,7 @@ interface AuthApi { suspend fun validateRegistrationFields(@FieldMap fields: Map): ValidationFields @GET(ApiConstants.GET_USER_PROFILE) - suspend fun getProfile(): User + suspend fun getProfile(@Tag instance: RequestInstance): User @FormUrlEncoded @POST(ApiConstants.URL_PASSWORD_RESET) diff --git a/auth/src/main/java/org/openedx/auth/data/repository/AuthRepository.kt b/auth/src/main/java/org/openedx/auth/data/repository/AuthRepository.kt index 31a869703..1f807aec5 100644 --- a/auth/src/main/java/org/openedx/auth/data/repository/AuthRepository.kt +++ b/auth/src/main/java/org/openedx/auth/data/repository/AuthRepository.kt @@ -8,49 +8,64 @@ import org.openedx.core.ApiConstants import org.openedx.core.config.Config import org.openedx.core.data.storage.CorePreferences import org.openedx.core.domain.model.RegistrationField +import org.openedx.core.instance.InstanceProvider +import org.openedx.core.instance.InstanceSessionManager +import org.openedx.core.instance.InstanceSwitchedException +import org.openedx.core.instance.RequestInstance import org.openedx.core.system.EdxError +/** + * Sign-in belongs to the site selected when it starts. Every request is pinned to that site, + * every credential is written to that site's storage, and if the learner selects another site + * before it finishes, it stops with [InstanceSwitchedException] instead of completing. + */ class AuthRepository( private val config: Config, private val api: AuthApi, private val preferencesManager: CorePreferences, + private val sessionManager: InstanceSessionManager, + private val instanceProvider: InstanceProvider, ) { suspend fun login( username: String, password: String, ) { + val site = currentSite() api.getAccessToken( - ApiConstants.GRANT_TYPE_PASSWORD, - config.getOAuthClientId(), - username, - password, - config.getAccessTokenType(), + grantType = ApiConstants.GRANT_TYPE_PASSWORD, + clientId = config.getOAuthClientId(), + username = username, + password = password, + tokenType = config.getAccessTokenType(), + instance = site, ) .mapToDomain() - .processAuthResponse() + .processAuthResponse(site) } suspend fun ssoLogin( jwtToken: String ) { - if (preferencesManager.accessToken.isBlank()) { - preferencesManager.accessToken = jwtToken + val site = currentSite() + if (preferencesManager.accessToken(site.instanceKey).isBlank()) { + preferencesManager.saveAccessToken(site.instanceKey, jwtToken) } - val user = api.getProfile() - preferencesManager.user = user + saveProfile(site) } suspend fun socialLogin(token: String?, authType: AuthType) { require(!token.isNullOrBlank()) { "Token is null" } + val site = currentSite() api.exchangeAccessToken( accessToken = token, clientId = config.getOAuthClientId(), tokenType = config.getAccessTokenType(), - authType = authType.postfix + authType = authType.postfix, + instance = site, ) .mapToDomain() - .processAuthResponse() + .processAuthResponse(site) } suspend fun getRegistrationFields(): List { @@ -69,14 +84,32 @@ class AuthRepository( return api.passwordReset(email).success } - private suspend fun AuthResponse.processAuthResponse() { + private fun currentSite() = RequestInstance(instanceProvider.currentInstance?.key) + + private fun ensureStillSelected(site: RequestInstance) { + if (instanceProvider.currentInstance?.key != site.instanceKey) { + throw InstanceSwitchedException(site.instanceKey) + } + } + + private suspend fun AuthResponse.processAuthResponse(site: RequestInstance) { if (error != null) { throw EdxError.UnknownException(error!!) } - preferencesManager.accessToken = accessToken ?: "" - preferencesManager.refreshToken = refreshToken ?: "" - preferencesManager.accessTokenExpiresAt = getTokenExpiryTime() - val user = api.getProfile() - preferencesManager.user = user + ensureStillSelected(site) + preferencesManager.saveTokens( + site.instanceKey, + accessToken.orEmpty(), + refreshToken.orEmpty(), + getTokenExpiryTime(), + ) + saveProfile(site) + } + + private suspend fun saveProfile(site: RequestInstance) { + val user = api.getProfile(site) + ensureStillSelected(site) + preferencesManager.saveUser(site.instanceKey, user) + user.username?.let { sessionManager.onSignedIn(site.instanceKey, it) } } } diff --git a/auth/src/test/java/org/openedx/auth/data/repository/AuthRepositoryInstanceTest.kt b/auth/src/test/java/org/openedx/auth/data/repository/AuthRepositoryInstanceTest.kt new file mode 100644 index 000000000..5b1f7c806 --- /dev/null +++ b/auth/src/test/java/org/openedx/auth/data/repository/AuthRepositoryInstanceTest.kt @@ -0,0 +1,113 @@ +package org.openedx.auth.data.repository + +import com.google.gson.JsonObject +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.fail +import org.junit.Test +import org.openedx.auth.data.api.AuthApi +import org.openedx.auth.data.model.AuthResponse +import org.openedx.core.config.Config +import org.openedx.core.data.model.User +import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.instance.Instance +import org.openedx.core.instance.InstanceProvider +import org.openedx.core.instance.InstanceSessionManager +import org.openedx.core.instance.InstanceSwitchedException +import org.openedx.core.instance.RequestInstance + +class AuthRepositoryInstanceTest { + + private class Selection(instance: Instance?) : InstanceProvider { + val flow = MutableStateFlow(instance) + override val currentInstance: Instance? get() = flow.value + override val currentInstanceFlow: StateFlow = flow + override val hasMultipleInstances = true + } + + private fun site(key: String) = requireNotNull( + Instance.fromJson( + JsonObject().apply { + addProperty("KEY", key) + addProperty("NAME", key) + addProperty("OAUTH_CLIENT_ID", "$key-client") + addProperty("API_HOST_URL", "https://$key.example.com") + } + ) + ) + + private val alpha = site("alpha") + private val beta = site("beta") + private val selection = Selection(alpha) + private val api = mockk() + private val preferences = mockk(relaxed = true) + private val sessionManager = mockk(relaxed = true) + private val config = mockk { + every { getOAuthClientId() } returns "alpha-client" + every { getAccessTokenType() } returns "JWT" + } + private val tokens = AuthResponse("access-a", "JWT", 3600, null, null, "refresh-a") + private val learner = User(1, "learner_a", "a@example.com", "Learner A") + + private fun repository() = AuthRepository(config, api, preferences, sessionManager, selection) + + @Test + fun `sign-in pins every request and write to the site it started on`() = runTest { + coEvery { + api.getAccessToken(any(), any(), any(), any(), any(), any(), RequestInstance("alpha")) + } returns tokens + coEvery { api.getProfile(RequestInstance("alpha")) } returns learner + + repository().login("learner_a", "secret") + + verify { preferences.saveTokens("alpha", "access-a", "refresh-a", any()) } + verify { preferences.saveUser("alpha", learner) } + verify { sessionManager.onSignedIn("alpha", "learner_a") } + } + + @Test + fun `a sign-in answer that arrives after another site was selected writes nothing`() = runTest { + coEvery { api.getAccessToken(any(), any(), any(), any(), any(), any(), any()) } answers { + selection.flow.value = beta + tokens + } + + try { + repository().login("learner_a", "secret") + fail("the sign-in must not complete on another site") + } catch (e: InstanceSwitchedException) { + assertEquals("alpha", e.instanceKey) + } + + verify(exactly = 0) { preferences.saveTokens(any(), any(), any(), any()) } + verify(exactly = 0) { preferences.saveUser(any(), any()) } + verify(exactly = 0) { sessionManager.onSignedIn(any(), any()) } + coVerify(exactly = 0) { api.getProfile(any()) } + } + + @Test + fun `a switch while the profile loads leaves the new site without a user`() = runTest { + coEvery { api.getAccessToken(any(), any(), any(), any(), any(), any(), any()) } returns tokens + coEvery { api.getProfile(RequestInstance("alpha")) } answers { + selection.flow.value = beta + learner + } + + try { + repository().login("learner_a", "secret") + fail("the sign-in must not complete on another site") + } catch (_: InstanceSwitchedException) { + // expected + } + + verify(exactly = 0) { preferences.saveUser(any(), any()) } + verify(exactly = 0) { preferences.saveTokens("beta", any(), any(), any()) } + } +} diff --git a/core/src/main/java/org/openedx/core/DatabaseManager.kt b/core/src/main/java/org/openedx/core/DatabaseManager.kt index d7bc7d025..6be731f33 100644 --- a/core/src/main/java/org/openedx/core/DatabaseManager.kt +++ b/core/src/main/java/org/openedx/core/DatabaseManager.kt @@ -1,5 +1,9 @@ package org.openedx.core interface DatabaseManager { + /** Clears the selected site's cached course data. */ fun clearTables() + + /** Clears one site's cached course data, whichever site is selected. */ + fun clearTables(instanceKey: String?) } 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 6cd94f8a8..f858c0d99 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 @@ -7,6 +7,8 @@ import org.openedx.core.domain.model.VideoSettings interface CorePreferences { var accessToken: String var refreshToken: String + + /** This device's push token. Device-wide: each signed-in site registers the same token. */ var pushToken: String var accessTokenExpiresAt: Long var user: User? @@ -30,4 +32,13 @@ interface CorePreferences { /** Stores refreshed tokens for the site that issued them, whichever site is selected now. */ fun saveTokens(instanceKey: String?, accessToken: String, refreshToken: String, expiresAt: Long) + + fun saveAccessToken(instanceKey: String?, accessToken: String) + + fun saveUser(instanceKey: String?, user: User?) + + /** Who signed in to [instanceKey]'s site last. Survives logout, unlike [user]. */ + fun lastSignedInUsername(instanceKey: String?): String + + fun setLastSignedInUsername(instanceKey: String?, username: String) } diff --git a/core/src/main/java/org/openedx/core/instance/InstanceSessionManager.kt b/core/src/main/java/org/openedx/core/instance/InstanceSessionManager.kt new file mode 100644 index 000000000..6a91f5a25 --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/InstanceSessionManager.kt @@ -0,0 +1,122 @@ +package org.openedx.core.instance + +import org.openedx.core.DatabaseManager +import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.module.DownloadWorkerController +import org.openedx.core.system.AppCookieManager +import org.openedx.core.ui.theme.InstanceTheme +import org.openedx.core.ui.theme.InstanceThemeManager + +/** Stops one site's backend from sending push notifications to this device. */ +interface PushTokenUnregistering { + /** Needs the site's session, so it runs before the session is cleared. */ + suspend fun unregisterPushToken(instanceKey: String?) +} + +interface InstanceSessionManager { + /** + * Makes [instance] the active site. Nothing is torn down: the outgoing site keeps its + * session, cached content and downloads for when the learner comes back. + */ + suspend fun switchActiveInstance(instance: Instance) + + /** + * Signs out of the site selected when it is called: cancels downloads, unregisters push, + * clears its tokens and user, then clears the selection unless the learner has switched to + * another site in the meantime. Cached courses and downloaded files stay, so signing back in + * to the same site is fast. + */ + suspend fun logoutCurrentInstance() + + /** + * Signs out of [instance]. The selected site goes through [logoutCurrentInstance]; any other + * site has push unregistered and loses its stored session, and the active one is untouched. + */ + suspend fun logout(instance: Instance) + + /** + * [logout] by key, for a sign-out that started on one site and must finish there even if + * the learner switched in between. `null` is the no-instance (single-site) session. + */ + suspend fun logout(instanceKey: String?) + + /** + * Records who signed in to [instanceKey]'s site. When it isn't the learner who used that site + * last, that learner's cached course data is cleared first. + */ + fun onSignedIn(instanceKey: String?, username: String) + + /** + * Applies the selected instance's branding, or the default theme when nothing is selected. + * Restoring a saved selection at launch doesn't go through [switchActiveInstance], so launch + * calls this once the catalog is resolved. + */ + fun applyThemeForCurrentInstance() +} + +class DefaultInstanceSessionManager( + private val instanceStore: InstanceStore, + private val preferences: CorePreferences, + private val downloadWorkerController: DownloadWorkerController, + private val databaseManager: DatabaseManager, + private val cookieManager: AppCookieManager, + private val pushTokenUnregistrar: PushTokenUnregistering? = null, +) : InstanceSessionManager { + + override suspend fun switchActiveInstance(instance: Instance) { + // Web content signs in with session cookies fetched for one site; the next page has to + // fetch the new site's (refetching replaces the old ones). + cookieManager.invalidateSessionCookie() + instanceStore.select(instance) + applyThemeForCurrentInstance() + } + + override suspend fun logoutCurrentInstance() { + signOut(instanceStore.currentInstance?.key, isSelected = true) + } + + override suspend fun logout(instance: Instance) = logout(instance.key) + + override suspend fun logout(instanceKey: String?) { + signOut(instanceKey, isSelected = instanceStore.currentInstance?.key == instanceKey) + } + + /** + * The site is fixed when the sign-out starts: unregistering push waits on the network, and the + * learner can switch sites meanwhile. Only that site's session is cleared, and the selection, + * web cookies and theme are reset only while it is still the selected site. + */ + private suspend fun signOut(instanceKey: String?, isSelected: Boolean) { + if (isSelected) downloadWorkerController.cancelUnfinishedDownloads() + // Before the tokens go: unregistering is an authenticated call. + pushTokenUnregistrar?.unregisterPushToken(instanceKey) + preferences.clearSession(instanceKey) + if (isSelected && instanceStore.currentInstance?.key == instanceKey) { + cookieManager.clearWebViewCookie() + // Last, so nothing observes a half signed-out site. + instanceStore.select(null) + applyThemeForCurrentInstance() + } + } + + override fun onSignedIn(instanceKey: String?, username: String) { + val previous = preferences.lastSignedInUsername(instanceKey) + if (previous.isNotEmpty() && previous != username) { + databaseManager.clearTables(instanceKey) + } + preferences.setLastSignedInUsername(instanceKey, username) + } + + override fun applyThemeForCurrentInstance() { + InstanceThemeManager.applyTheme(instanceStore.currentInstance?.toTheme()) + } +} + +fun Instance.toTheme() = InstanceTheme( + key = key, + accentColorHex = color, + light = themeColors?.light.orEmpty(), + dark = themeColors?.dark.orEmpty(), + logoUrl = logoUrl, + headerBackgroundUrl = headerBackgroundUrl, +) diff --git a/core/src/main/java/org/openedx/core/instance/InstanceSwitchedException.kt b/core/src/main/java/org/openedx/core/instance/InstanceSwitchedException.kt new file mode 100644 index 000000000..5a0f4870f --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/InstanceSwitchedException.kt @@ -0,0 +1,8 @@ +package org.openedx.core.instance + +/** + * The learner selected another site while an operation that belongs to one site was waiting on + * the network. Its results were kept away from the newly selected site. + */ +class InstanceSwitchedException(val instanceKey: String?) : + IllegalStateException("The selected site changed during an operation for $instanceKey") diff --git a/core/src/main/java/org/openedx/core/instance/RequestInstance.kt b/core/src/main/java/org/openedx/core/instance/RequestInstance.kt new file mode 100644 index 000000000..70288e2fd --- /dev/null +++ b/core/src/main/java/org/openedx/core/instance/RequestInstance.kt @@ -0,0 +1,12 @@ +package org.openedx.core.instance + +/** + * Which instance a request belongs to, as an OkHttp tag. + * + * Requests built through the app's API client are tagged with the selected instance when they + * start. A caller that must stay on one site across suspensions (sign-in, sign-out, push + * registration) passes the tag itself through a Retrofit `@Tag` parameter: the request then goes + * to that site's host with that site's token, whichever site is selected by the time it is sent. + * A `null` key means no instance: the app-level host and the legacy session. + */ +data class RequestInstance(val instanceKey: String?) diff --git a/core/src/main/java/org/openedx/core/module/DownloadWorkerController.kt b/core/src/main/java/org/openedx/core/module/DownloadWorkerController.kt index d2e0e8167..54a734700 100644 --- a/core/src/main/java/org/openedx/core/module/DownloadWorkerController.kt +++ b/core/src/main/java/org/openedx/core/module/DownloadWorkerController.kt @@ -84,11 +84,27 @@ class DownloadWorkerController( updateList() - if (downloadTaskList.isEmpty()) { + // Only when the worker was on one of these: it may be busy with another site's queue, + // which this site has no say over. + if (hasDownloading && downloadTaskList.isEmpty()) { workManager.cancelAllWorkByTag(DownloadWorker.WORKER_TAG) } } + /** + * Cancels every download the selected site hasn't finished: the active one and the queue. + * Finished files stay. + */ + suspend fun cancelUnfinishedDownloads() { + val unfinished = downloadDao.readAllData() + .map { it.mapToDomain() } + .filter { + it.downloadedState == DownloadedState.WAITING || it.downloadedState == DownloadedState.DOWNLOADING + } + .map { it.id } + if (unfinished.isNotEmpty()) removeModels(unfinished) + } + suspend fun removeModels() { fileDownloader.cancelDownloading() workManager.cancelAllWorkByTag(DownloadWorker.WORKER_TAG) diff --git a/core/src/main/java/org/openedx/core/system/AppCookieManager.kt b/core/src/main/java/org/openedx/core/system/AppCookieManager.kt index 7df19c627..6f24c701a 100644 --- a/core/src/main/java/org/openedx/core/system/AppCookieManager.kt +++ b/core/src/main/java/org/openedx/core/system/AppCookieManager.kt @@ -36,6 +36,14 @@ class AppCookieManager(private val config: Config, private val api: CookiesApi) authSessionCookieExpiration = -1 } + /** + * Forgets that the session cookies are fresh, so the next web page fetches them again, for + * the site selected then. Cheap enough for the main thread: it doesn't touch the WebView. + */ + fun invalidateSessionCookie() { + authSessionCookieExpiration = -1 + } + fun isSessionCookieMissingOrExpired(): Boolean { return authSessionCookieExpiration < System.currentTimeMillis() } diff --git a/core/src/test/java/org/openedx/core/instance/InstanceSessionManagerTest.kt b/core/src/test/java/org/openedx/core/instance/InstanceSessionManagerTest.kt new file mode 100644 index 000000000..a458a7af1 --- /dev/null +++ b/core/src/test/java/org/openedx/core/instance/InstanceSessionManagerTest.kt @@ -0,0 +1,228 @@ +package org.openedx.core.instance + +import com.google.gson.JsonObject +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.coVerifyOrder +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.openedx.core.DatabaseManager +import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.module.DownloadWorkerController +import org.openedx.core.system.AppCookieManager +import org.openedx.core.ui.theme.InstanceThemeManager + +class InstanceSessionManagerTest { + + private class InMemorySelection(override var selectedInstanceKey: String? = null) : InstanceSelectionStorage + + private fun instance(key: String, color: String = "#123456") = requireNotNull( + Instance.fromJson( + JsonObject().apply { + addProperty("KEY", key) + addProperty("NAME", key) + addProperty("COLOR", color) + addProperty("OAUTH_CLIENT_ID", "$key-client") + addProperty("API_HOST_URL", "https://$key.example.com") + } + ) + ) + + private val alpha = instance("alpha", "#3C68FF") + private val beta = instance("beta", "#1C355E") + private val store = InstanceStore(InMemorySelection()) { InstancesConfig(listOf(alpha, beta)) } + private val preferences = mockk(relaxed = true) + private val downloads = mockk(relaxed = true) + private val databaseManager = mockk(relaxed = true) + private val push = mockk(relaxed = true) + private val cookies = mockk(relaxed = true) + + private fun manager(pushTokenUnregistrar: PushTokenUnregistering? = push) = + DefaultInstanceSessionManager(store, preferences, downloads, databaseManager, cookies, pushTokenUnregistrar) + + @After + fun tearDown() { + InstanceThemeManager.reset() + } + + @Test + fun `switching selects the site and tears nothing down`() = runBlocking { + manager().switchActiveInstance(beta) + + assertEquals(beta, store.currentInstance) + assertEquals("beta", InstanceThemeManager.theme.value?.key) + verify { cookies.invalidateSessionCookie() } + coVerify(exactly = 0) { preferences.clearCorePreferences() } + coVerify(exactly = 0) { preferences.clearSession(any()) } + coVerify(exactly = 0) { downloads.cancelUnfinishedDownloads() } + verify(exactly = 0) { databaseManager.clearTables() } + } + + @Test + fun `logout ends the session in order and keeps cached content`() = runBlocking { + store.select(alpha) + + manager().logoutCurrentInstance() + + coVerifyOrder { + downloads.cancelUnfinishedDownloads() + push.unregisterPushToken("alpha") + preferences.clearSession("alpha") + } + assertNull(store.currentInstance) + assertNull(InstanceThemeManager.theme.value) + verify { cookies.clearWebViewCookie() } + verify(exactly = 0) { databaseManager.clearTables() } + } + + @Test + fun `logout completes without push unregistration wired`() = runBlocking { + store.select(alpha) + + manager(pushTokenUnregistrar = null).logoutCurrentInstance() + + coVerify { preferences.clearSession("alpha") } + assertNull(store.currentInstance) + } + + @Test + fun `clearing the selection happens after the session is cleared`() = runBlocking { + store.select(alpha) + var selectedWhileClearing: Instance? = null + coEvery { preferences.clearSession("alpha") } answers { selectedWhileClearing = store.currentInstance } + + manager().logoutCurrentInstance() + + assertEquals(alpha, selectedWhileClearing) + } + + @Test + fun `logging out of another site leaves the active one alone`() = runBlocking { + store.select(alpha) + + manager().logout(beta) + + coVerify { push.unregisterPushToken("beta") } + coVerify { preferences.clearSession("beta") } + coVerify(exactly = 0) { preferences.clearCorePreferences() } + coVerify(exactly = 0) { downloads.cancelUnfinishedDownloads() } + assertEquals(alpha, store.currentInstance) + } + + @Test + fun `logging out of the active site is a full logout`() = runBlocking { + store.select(beta) + + manager().logout(beta) + + coVerify { preferences.clearSession("beta") } + assertNull(store.currentInstance) + } + + @Test + fun `switching sites while push unregisters signs out only the site that started it`() = runBlocking { + val sessions = mutableSetOf("alpha", "beta") + every { preferences.hasSession(any()) } answers { firstArg() in sessions } + coEvery { preferences.clearSession(any()) } answers { sessions.remove(firstArg()) } + val unregisterStarted = CompletableDeferred() + val unregisterDone = CompletableDeferred() + coEvery { push.unregisterPushToken("alpha") } coAnswers { + unregisterStarted.complete(Unit) + unregisterDone.await() + } + val manager = manager() + manager.switchActiveInstance(alpha) + + val logout = launch { manager.logoutCurrentInstance() } + unregisterStarted.await() + manager.switchActiveInstance(beta) + unregisterDone.complete(Unit) + logout.join() + + assertFalse(preferences.hasSession("alpha")) + assertTrue(preferences.hasSession("beta")) + coVerify(exactly = 0) { preferences.clearSession("beta") } + coVerify(exactly = 0) { preferences.clearCorePreferences() } + assertEquals(beta, store.currentInstance) + assertEquals("beta", InstanceThemeManager.theme.value?.key) + verify(exactly = 0) { cookies.clearWebViewCookie() } + } + + @Test + fun `signing out of another site while it becomes selected leaves the selection alone`() = runBlocking { + val unregisterStarted = CompletableDeferred() + val unregisterDone = CompletableDeferred() + coEvery { push.unregisterPushToken("beta") } coAnswers { + unregisterStarted.complete(Unit) + unregisterDone.await() + } + val manager = manager() + manager.switchActiveInstance(alpha) + + val logout = launch { manager.logout(beta) } + unregisterStarted.await() + manager.switchActiveInstance(beta) + unregisterDone.complete(Unit) + logout.join() + + coVerify { preferences.clearSession("beta") } + coVerify(exactly = 0) { downloads.cancelUnfinishedDownloads() } + assertEquals(beta, store.currentInstance) + verify(exactly = 0) { cookies.clearWebViewCookie() } + } + + @Test + fun `a different learner signing in clears the previous learner's cache on that site`() { + every { preferences.lastSignedInUsername("beta") } returns "first" + + manager().onSignedIn("beta", "second") + + verify { databaseManager.clearTables("beta") } + verify { preferences.setLastSignedInUsername("beta", "second") } + } + + @Test + fun `the same learner signing back in keeps the cache`() { + every { preferences.lastSignedInUsername("beta") } returns "first" + manager().onSignedIn("beta", "first") + every { preferences.lastSignedInUsername("alpha") } returns "" + manager().onSignedIn("alpha", "first") + + verify(exactly = 0) { databaseManager.clearTables(any()) } + } + + @Test + fun `a sign-out that started on another site finishes there only`() = runBlocking { + store.select(beta) + + manager().logout("alpha") + + coVerifyOrder { + push.unregisterPushToken("alpha") + preferences.clearSession("alpha") + } + coVerify(exactly = 0) { preferences.clearCorePreferences() } + assertEquals(beta, store.currentInstance) + } + + @Test + fun `theme follows the selected instance and resets without one`() { + store.select(beta) + manager().applyThemeForCurrentInstance() + assertEquals("#1C355E", InstanceThemeManager.theme.value?.accentColorHex) + + store.select(null) + manager().applyThemeForCurrentInstance() + assertNull(InstanceThemeManager.theme.value) + } +} diff --git a/profile/src/main/java/org/openedx/profile/data/api/ProfileApi.kt b/profile/src/main/java/org/openedx/profile/data/api/ProfileApi.kt index 1f206ed77..87386decf 100644 --- a/profile/src/main/java/org/openedx/profile/data/api/ProfileApi.kt +++ b/profile/src/main/java/org/openedx/profile/data/api/ProfileApi.kt @@ -3,6 +3,7 @@ package org.openedx.profile.data.api import okhttp3.RequestBody import okhttp3.ResponseBody import org.openedx.core.ApiConstants +import org.openedx.core.instance.RequestInstance import org.openedx.profile.data.model.Account import retrofit2.Response import retrofit2.http.Body @@ -16,6 +17,7 @@ import retrofit2.http.PATCH import retrofit2.http.POST import retrofit2.http.Path import retrofit2.http.Query +import retrofit2.http.Tag interface ProfileApi { @@ -24,7 +26,8 @@ interface ProfileApi { suspend fun revokeAccessToken( @Field("client_id") clientId: String?, @Field("token") token: String?, - @Field("token_type_hint") tokenTypeHint: String? + @Field("token_type_hint") tokenTypeHint: String?, + @Tag instance: RequestInstance, ): ResponseBody @GET("/api/user/v1/accounts/{username}") diff --git a/profile/src/main/java/org/openedx/profile/data/repository/ProfileRepository.kt b/profile/src/main/java/org/openedx/profile/data/repository/ProfileRepository.kt index 561d73c05..668052ef5 100644 --- a/profile/src/main/java/org/openedx/profile/data/repository/ProfileRepository.kt +++ b/profile/src/main/java/org/openedx/profile/data/repository/ProfileRepository.kt @@ -3,9 +3,11 @@ package org.openedx.profile.data.repository import okhttp3.MediaType.Companion.toMediaType import okhttp3.RequestBody.Companion.asRequestBody import org.openedx.core.ApiConstants -import org.openedx.core.DatabaseManager import org.openedx.core.config.Config import org.openedx.core.data.storage.CorePreferences +import org.openedx.core.instance.InstanceProvider +import org.openedx.core.instance.InstanceSessionManager +import org.openedx.core.instance.RequestInstance import org.openedx.profile.data.api.ProfileApi import org.openedx.profile.data.storage.ProfilePreferences import org.openedx.profile.domain.model.Account @@ -16,7 +18,8 @@ class ProfileRepository( private val api: ProfileApi, private val profilePreferences: ProfilePreferences, private val corePreferences: CorePreferences, - private val databaseManager: DatabaseManager + private val sessionManager: InstanceSessionManager, + private val instanceProvider: InstanceProvider, ) { suspend fun getAccount(): Account { @@ -53,16 +56,24 @@ class ProfileRepository( suspend fun deactivateAccount(password: String) = api.deactivateAccount(password) + /** + * Signs out of the site selected when it starts. The revoke request is pinned to that site, + * and if the learner selects another site while it is in flight, only the original site is + * signed out: the new selection and its session stay. + */ suspend fun logout() { + val siteKey = instanceProvider.currentInstance?.key + val clientId = config.getOAuthClientId() + val refreshToken = corePreferences.refreshToken(siteKey) try { api.revokeAccessToken( - config.getOAuthClientId(), - corePreferences.refreshToken, - ApiConstants.TOKEN_TYPE_REFRESH + clientId, + refreshToken, + ApiConstants.TOKEN_TYPE_REFRESH, + RequestInstance(siteKey), ) } finally { - corePreferences.clearCorePreferences() - databaseManager.clearTables() + sessionManager.logout(siteKey) } } } diff --git a/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsViewModel.kt b/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsViewModel.kt index 4c418f705..6b453b7dd 100644 --- a/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsViewModel.kt +++ b/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsViewModel.kt @@ -17,7 +17,6 @@ import org.openedx.core.AppUpdateState import org.openedx.core.CalendarRouter import org.openedx.core.R import org.openedx.core.config.Config -import org.openedx.core.module.DownloadWorkerController import org.openedx.core.presentation.global.AppData import org.openedx.core.system.AppCookieManager import org.openedx.core.system.notifier.app.AppNotifier @@ -40,7 +39,6 @@ class SettingsViewModel( private val resourceManager: ResourceManager, private val interactor: ProfileInteractor, private val cookieManager: AppCookieManager, - private val workerController: DownloadWorkerController, private val analytics: ProfileAnalytics, private val profileRouter: ProfileRouter, private val calendarRouter: CalendarRouter, @@ -73,7 +71,6 @@ class SettingsViewModel( logProfileEvent(ProfileAnalyticsEvent.LOGOUT_CLICKED) viewModelScope.launch { try { - workerController.removeModels() withContext(Dispatchers.IO) { interactor.logout() } diff --git a/profile/src/test/java/org/openedx/profile/data/repository/ProfileRepositoryLogoutTest.kt b/profile/src/test/java/org/openedx/profile/data/repository/ProfileRepositoryLogoutTest.kt new file mode 100644 index 000000000..5be927ada --- /dev/null +++ b/profile/src/test/java/org/openedx/profile/data/repository/ProfileRepositoryLogoutTest.kt @@ -0,0 +1,81 @@ +package org.openedx.profile.data.repository + +import com.google.gson.JsonObject +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.test.runTest +import okhttp3.ResponseBody.Companion.toResponseBody +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.instance.InstanceSessionManager +import org.openedx.core.instance.RequestInstance +import org.openedx.profile.data.api.ProfileApi + +class ProfileRepositoryLogoutTest { + + private class Selection(instance: Instance?) : InstanceProvider { + val flow = MutableStateFlow(instance) + override val currentInstance: Instance? get() = flow.value + override val currentInstanceFlow: StateFlow = flow + override val hasMultipleInstances = true + } + + private fun site(key: String) = requireNotNull( + Instance.fromJson( + JsonObject().apply { + addProperty("KEY", key) + addProperty("NAME", key) + addProperty("OAUTH_CLIENT_ID", "$key-client") + addProperty("API_HOST_URL", "https://$key.example.com") + } + ) + ) + + private val selection = Selection(site("alpha")) + private val api = mockk() + private val preferences = mockk(relaxed = true) { + every { refreshToken("alpha") } returns "refresh-a" + } + private val sessionManager = mockk(relaxed = true) + private val config = mockk { every { getOAuthClientId() } returns "alpha-client" } + + private fun repository() = ProfileRepository( + config, + api, + mockk(relaxed = true), + preferences, + sessionManager, + selection + ) + + @Test + fun `a sign-out that was in flight during a switch signs out only the site it started on`() = runTest { + coEvery { api.revokeAccessToken(any(), any(), any(), any()) } answers { + selection.flow.value = site("beta") + "".toResponseBody() + } + + repository().logout() + + coVerify { api.revokeAccessToken("alpha-client", "refresh-a", any(), RequestInstance("alpha")) } + coVerify { sessionManager.logout("alpha") } + coVerify(exactly = 0) { sessionManager.logoutCurrentInstance() } + coVerify(exactly = 0) { sessionManager.logout("beta") } + } + + @Test + fun `a failed revoke still signs the site out`() = runTest { + coEvery { api.revokeAccessToken(any(), any(), any(), any()) } throws java.io.IOException("offline") + + runCatching { repository().logout() } + + coVerify { sessionManager.logout("alpha") } + } +}