diff --git a/app/src/main/java/com/runicgateway/app/core/auth/sso/EncryptedPendingSsoStore.kt b/app/src/main/java/com/runicgateway/app/core/auth/sso/EncryptedPendingSsoStore.kt new file mode 100644 index 0000000..3548ff2 --- /dev/null +++ b/app/src/main/java/com/runicgateway/app/core/auth/sso/EncryptedPendingSsoStore.kt @@ -0,0 +1,61 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.core.auth.sso + +import android.content.Context +import android.content.SharedPreferences +import androidx.security.crypto.EncryptedSharedPreferences +import androidx.security.crypto.MasterKey +import dagger.hilt.android.qualifiers.ApplicationContext +import javax.inject.Inject +import javax.inject.Singleton + +/** + * [PendingSsoStore] backed by Jetpack Security's [EncryptedSharedPreferences] + * (Tink/AES-256-GCM), so the PKCE verifier is encrypted at rest for the brief + * window a flow is in progress. Separate prefs file from the session token store — + * this holds only the transient SSO handshake, cleared as soon as the callback is + * consumed. Lazy, so a device that never signs in via SSO pays no keystore cost. + */ +@Singleton +class EncryptedPendingSsoStore @Inject constructor( + @param:ApplicationContext private val context: Context, +) : PendingSsoStore { + + private val prefs: SharedPreferences by lazy { + val masterKey = MasterKey.Builder(context) + .setKeyScheme(MasterKey.KeyScheme.AES256_GCM) + .build() + EncryptedSharedPreferences.create( + context, + PREFS_NAME, + masterKey, + EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, + EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM, + ) + } + + override fun save(state: String, verifier: String) { + prefs.edit() + .putString(KEY_STATE, state) + .putString(KEY_VERIFIER, verifier) + .apply() + } + + override fun load(): PendingSso? { + val state = prefs.getString(KEY_STATE, null) ?: return null + val verifier = prefs.getString(KEY_VERIFIER, null) ?: return null + return PendingSso(state = state, verifier = verifier) + } + + override fun clear() { + prefs.edit().clear().apply() + } + + private companion object { + const val PREFS_NAME = "runic_sso_pending" + const val KEY_STATE = "state" + const val KEY_VERIFIER = "verifier" + } +} diff --git a/app/src/main/java/com/runicgateway/app/core/auth/sso/PendingSsoStore.kt b/app/src/main/java/com/runicgateway/app/core/auth/sso/PendingSsoStore.kt new file mode 100644 index 0000000..822db83 --- /dev/null +++ b/app/src/main/java/com/runicgateway/app/core/auth/sso/PendingSsoStore.kt @@ -0,0 +1,24 @@ +/* + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.runicgateway.app.core.auth.sso + +/** + * Persists the in-flight SSO `{state, verifier}` (PKCE Layer B + CSRF state) across + * the Custom-Tab round trip so the exchange survives process death — a low-memory + * device can evict the app while the Custom Tab is foreground, and the callback then + * returns to a fresh process (PLAN.md §4.2). Kept behind an interface so + * [SsoAuthManager] stays framework-free and unit-tests on the JVM with a fake. + * + * Exactly one flow is pending at a time; [save] overwrites any prior. The verifier + * is a bearer-equivalent secret for the one-time code, so the production impl + * ([EncryptedPendingSsoStore]) encrypts it at rest, mirroring the token store. + */ +interface PendingSsoStore { + fun save(state: String, verifier: String) + fun load(): PendingSso? + fun clear() +} + +/** The stashed CSRF state + PKCE verifier for the current SSO attempt. */ +data class PendingSso(val state: String, val verifier: String) diff --git a/app/src/main/java/com/runicgateway/app/core/auth/sso/SsoAuthManager.kt b/app/src/main/java/com/runicgateway/app/core/auth/sso/SsoAuthManager.kt index b3ae095..a3dcc50 100644 --- a/app/src/main/java/com/runicgateway/app/core/auth/sso/SsoAuthManager.kt +++ b/app/src/main/java/com/runicgateway/app/core/auth/sso/SsoAuthManager.kt @@ -13,7 +13,6 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import java.io.IOException -import java.util.concurrent.atomic.AtomicReference import javax.inject.Inject import javax.inject.Singleton @@ -34,20 +33,22 @@ import javax.inject.Singleton * parses the callback `Uri` (the Android edge) and hands the raw params here, * so this class stays free of framework types and unit-tests on the JVM. * - * The pending `{state, verifier}` lives only in memory: if the process is killed - * while the Custom Tab is foreground it is lost and the exchange **fails closed** - * (the user simply retries) — never a security downgrade. + * The pending `{state, verifier}` is persisted via [PendingSsoStore] (encrypted at + * rest), so the exchange survives the process being evicted while the Custom Tab is + * foreground — the callback can land in a fresh process and still complete. It is + * cleared the moment [complete] consumes it, so a lost/duplicate callback still + * **fails closed** as [Failure.STATE_MISMATCH] rather than double-exchanging. * * Threading: [buildStartUrl] runs on the UI thread; [complete] runs on the - * activity's coroutine scope after a deep link. The pending holder is an - * [AtomicReference] and [outcome] a [StateFlow], so a ViewModel/activity recreation - * while the Custom Tab is open cannot drop a result. + * activity's coroutine scope after a deep link. [outcome] is a [StateFlow], so a + * ViewModel/activity recreation while the Custom Tab is open cannot drop a result. */ @Singleton class SsoAuthManager @Inject constructor( private val ssoApi: SsoApi, private val sessionManager: SessionManager, private val baseUrlHolder: BaseUrlHolder, + private val pendingStore: PendingSsoStore, ) { /** Why an SSO attempt ended, for a friendly inline message on the login screen. */ @@ -75,10 +76,6 @@ class SsoAuthManager @Inject constructor( data class Failed(val reason: Failure) : Outcome } - private data class Pending(val state: String, val verifier: String) - - private val pending = AtomicReference(null) - /** * The host this build baked an App Link intent-filter for (`BuildConfig.APP_LINK_HOST`, * empty on the generic multi-tenant build — see docs/android/APP_LINKS.md). @@ -97,16 +94,16 @@ class SsoAuthManager @Inject constructor( /** * Build the `/auth/mobile/sso/start` URL for [providerId] and stash the pending - * PKCE verifier + CSRF state. Returns null when no shard site is configured yet - * (the caller then keeps the website hand-off fallback). Also resets [outcome] - * to [Outcome.Idle] so a stale prior result can't fire against the new attempt. + * PKCE verifier + CSRF state (persisted so it survives process death). Returns + * null when no shard site is configured yet. Also resets [outcome] to + * [Outcome.Idle] so a stale prior result can't fire against the new attempt. */ fun buildStartUrl(providerId: String): String? { val base = baseUrlHolder.current ?: return null val verifier = Pkce.newVerifier() val challenge = Pkce.challengeOf(verifier) val state = Pkce.newState() - pending.set(Pending(state = state, verifier = verifier)) + pendingStore.save(state = state, verifier = verifier) _outcome.value = Outcome.Idle return base.newBuilder() .addPathSegments("api/v1/auth/mobile/sso/start") @@ -158,7 +155,8 @@ class SsoAuthManager @Inject constructor( * single-uses the code). */ suspend fun complete(state: String?, code: String?, error: String?) { - val stashed = pending.getAndSet(null) + val stashed = pendingStore.load() + pendingStore.clear() // CSRF: the callback must echo the exact state we generated at /start. if (stashed == null || state.isNullOrEmpty() || state != stashed.state) { diff --git a/app/src/main/java/com/runicgateway/app/core/web/WebsiteUrls.kt b/app/src/main/java/com/runicgateway/app/core/web/WebsiteUrls.kt index a4d3185..55c66f7 100644 --- a/app/src/main/java/com/runicgateway/app/core/web/WebsiteUrls.kt +++ b/app/src/main/java/com/runicgateway/app/core/web/WebsiteUrls.kt @@ -26,12 +26,8 @@ class WebsiteUrls @Inject constructor( /** Forgot / reset password (the flow built on the backend before app work, §8). */ fun forgotPassword(): String? = resolve(FORGOT) - /** The website login page — carries the SSO provider buttons (§4.2). */ - fun login(): String? = resolve(LOGIN) - private companion object { const val REGISTER = "account/register" const val FORGOT = "account/forgot" - const val LOGIN = "account/login" } } diff --git a/app/src/main/java/com/runicgateway/app/data/repository/AuthRepository.kt b/app/src/main/java/com/runicgateway/app/data/repository/AuthRepository.kt index a0c859e..79a3207 100644 --- a/app/src/main/java/com/runicgateway/app/data/repository/AuthRepository.kt +++ b/app/src/main/java/com/runicgateway/app/data/repository/AuthRepository.kt @@ -13,6 +13,7 @@ import com.runicgateway.app.data.api.dto.MobileTokenResponse import com.runicgateway.app.data.api.dto.SsoProviderDto import com.runicgateway.app.data.api.dto.TotpRequiredError import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.delay import kotlinx.serialization.json.Json import retrofit2.Response import java.io.IOException @@ -34,17 +35,40 @@ class AuthRepository @Inject constructor( private val json: Json, ) { + /** The three outcomes of SSO provider discovery, so the login screen can tell a + * shard that offers no SSO ([None]) apart from a discovery that failed + * ([Unavailable], offer a retry) — the old "empty on any failure" conflation hid + * a broken call behind a dead website hand-off (§4.2). */ + sealed interface SsoDiscovery { + /** At least one enabled provider — render a native button per entry. */ + data class Available(val providers: List) : SsoDiscovery + + /** Discovery succeeded but the shard has no SSO providers configured. */ + data object None : SsoDiscovery + + /** The discovery call failed (offline / server error) — surface a retry. */ + data object Unavailable : SsoDiscovery + } + /** - * The shard's enabled SSO providers for the native login buttons (§4.2). Public - * discovery, never secrets. Returns an empty list on any failure — the login - * screen then keeps the website hand-off fallback rather than showing nothing. + * Discover the shard's enabled SSO providers for the native login buttons (§4.2). + * Public discovery, never secrets. Retries once before reporting [Unavailable], + * so a single transient blip doesn't strand the user. */ - suspend fun ssoProviders(): List = try { - ssoApi.providers() - } catch (e: CancellationException) { - throw e - } catch (_: Exception) { - emptyList() + suspend fun ssoProviders(): SsoDiscovery { + var lastFailed = false + repeat(2) { attempt -> + try { + val providers = ssoApi.providers() + return if (providers.isEmpty()) SsoDiscovery.None else SsoDiscovery.Available(providers) + } catch (e: CancellationException) { + throw e + } catch (_: Exception) { + lastFailed = true + if (attempt == 0) delay(DISCOVERY_RETRY_DELAY_MS) + } + } + return if (lastFailed) SsoDiscovery.Unavailable else SsoDiscovery.None } /** Outcome of a login attempt (§4.1). */ @@ -140,4 +164,8 @@ class AuthRepository @Inject constructor( } catch (_: Exception) { false } + + private companion object { + const val DISCOVERY_RETRY_DELAY_MS = 400L + } } diff --git a/app/src/main/java/com/runicgateway/app/di/StorageModule.kt b/app/src/main/java/com/runicgateway/app/di/StorageModule.kt index 05cc4ff..96c9c91 100644 --- a/app/src/main/java/com/runicgateway/app/di/StorageModule.kt +++ b/app/src/main/java/com/runicgateway/app/di/StorageModule.kt @@ -5,13 +5,15 @@ package com.runicgateway.app.di import com.runicgateway.app.core.auth.EncryptedTokenStore import com.runicgateway.app.core.auth.TokenStore +import com.runicgateway.app.core.auth.sso.EncryptedPendingSsoStore +import com.runicgateway.app.core.auth.sso.PendingSsoStore import dagger.Binds import dagger.Module import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent import javax.inject.Singleton -/** Binds the at-rest token store to its EncryptedSharedPreferences impl (§4.3). */ +/** Binds the at-rest stores to their EncryptedSharedPreferences impls (§4.3). */ @Module @InstallIn(SingletonComponent::class) abstract class StorageModule { @@ -19,4 +21,8 @@ abstract class StorageModule { @Binds @Singleton abstract fun bindTokenStore(impl: EncryptedTokenStore): TokenStore + + @Binds + @Singleton + abstract fun bindPendingSsoStore(impl: EncryptedPendingSsoStore): PendingSsoStore } diff --git a/app/src/main/java/com/runicgateway/app/ui/auth/LoginScreen.kt b/app/src/main/java/com/runicgateway/app/ui/auth/LoginScreen.kt index a6e01dc..cc1159f 100644 --- a/app/src/main/java/com/runicgateway/app/ui/auth/LoginScreen.kt +++ b/app/src/main/java/com/runicgateway/app/ui/auth/LoginScreen.kt @@ -164,26 +164,42 @@ fun LoginScreen( } // ── Native SSO (§4.2, M9): a button per enabled provider that opens the - // Custom-Tab bridge and returns the user signed in. Falls back to the - // website login hand-off when the shard exposes no providers. - if (state.ssoProviders.isNotEmpty()) { - state.ssoProviders.forEach { provider -> - OutlinedButton( - onClick = { viewModel.onSsoProviderClick(provider) }, - enabled = !state.submitting, - modifier = Modifier - .fillMaxWidth() - .padding(top = 12.dp), + // Custom-Tab bridge and returns the user signed in. No website-login + // fallback — that page isn't mobile-formatted and can't deep-link the + // session back; a failed discovery offers a retry instead. + when { + state.ssoProviders.isNotEmpty() -> { + state.ssoProviders.forEach { provider -> + OutlinedButton( + onClick = { viewModel.onSsoProviderClick(provider) }, + enabled = !state.submitting, + modifier = Modifier + .fillMaxWidth() + .padding(top = 12.dp), + ) { + Text(stringResource(R.string.login_sso_provider, provider.name)) + } + } + } + + state.ssoDiscovering -> { + Text( + text = stringResource(R.string.login_sso_loading), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(top = 12.dp), + ) + } + + state.ssoUnavailable -> { + TextButton( + onClick = { viewModel.discoverSsoProviders() }, + modifier = Modifier.padding(top = 4.dp), ) { - Text(stringResource(R.string.login_sso_provider, provider.name)) - } - } - } else { - viewModel.ssoLoginUrl?.let { url -> - TextButton(onClick = { WebHandoff.open(context, url) }) { - Text(stringResource(R.string.login_sso)) + Text(stringResource(R.string.login_sso_retry)) } } + // else: discovery succeeded with no providers — this shard offers no SSO. } // ── Website hand-offs (§4.2): open the site's own pages in a Custom Tab ── diff --git a/app/src/main/java/com/runicgateway/app/ui/auth/LoginViewModel.kt b/app/src/main/java/com/runicgateway/app/ui/auth/LoginViewModel.kt index 0839e0d..3080407 100644 --- a/app/src/main/java/com/runicgateway/app/ui/auth/LoginViewModel.kt +++ b/app/src/main/java/com/runicgateway/app/ui/auth/LoginViewModel.kt @@ -10,6 +10,7 @@ import com.runicgateway.app.core.web.WebsiteUrls import com.runicgateway.app.data.api.dto.SsoProviderDto import com.runicgateway.app.data.repository.AuthRepository import com.runicgateway.app.data.repository.AuthRepository.LoginResult +import com.runicgateway.app.data.repository.AuthRepository.SsoDiscovery import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -43,8 +44,12 @@ class LoginViewModel @Inject constructor( val submitting: Boolean = false, val error: LoginError? = null, val signedIn: Boolean = false, - /** The shard's enabled SSO providers (§4.2); empty → website hand-off fallback. */ + /** The shard's enabled SSO providers (§4.2); empty until discovery resolves. */ val ssoProviders: List = emptyList(), + /** True while discovery is in flight — the screen shows a spinner, not an empty gap. */ + val ssoDiscovering: Boolean = true, + /** True when discovery failed (offline/server) — offer a retry rather than a dead end. */ + val ssoUnavailable: Boolean = false, /** A `/auth/mobile/sso/start` URL the screen should open in a Custom Tab, once. */ val ssoLaunchUrl: String? = null, ) @@ -53,11 +58,7 @@ class LoginViewModel @Inject constructor( val state: StateFlow = _state.asStateFlow() init { - // Discover the native SSO providers to render buttons for (§4.2). - viewModelScope.launch { - val providers = authRepository.ssoProviders() - if (providers.isNotEmpty()) _state.update { it.copy(ssoProviders = providers) } - } + discoverSsoProviders() // Consume the SSO bridge outcome: a returned callback completes here even if // this ViewModel was recreated while the Custom Tab was foreground (§4.2). viewModelScope.launch { @@ -85,8 +86,30 @@ class LoginViewModel @Inject constructor( val registerUrl: String? get() = websiteUrls.register() val forgotPasswordUrl: String? get() = websiteUrls.forgotPassword() - /** Website login hand-off — the fallback when native SSO discovery is empty (§4.2). */ - val ssoLoginUrl: String? get() = websiteUrls.login() + /** + * Discover the shard's native SSO providers (§4.2). A failure surfaces a retry + * affordance instead of the old dead website-login hand-off, which was never + * mobile-formatted and could not deep-link the session back. + */ + fun discoverSsoProviders() { + _state.update { it.copy(ssoDiscovering = true, ssoUnavailable = false) } + viewModelScope.launch { + when (val result = authRepository.ssoProviders()) { + is SsoDiscovery.Available -> + _state.update { + it.copy(ssoProviders = result.providers, ssoDiscovering = false, ssoUnavailable = false) + } + SsoDiscovery.None -> + _state.update { + it.copy(ssoProviders = emptyList(), ssoDiscovering = false, ssoUnavailable = false) + } + SsoDiscovery.Unavailable -> + _state.update { + it.copy(ssoProviders = emptyList(), ssoDiscovering = false, ssoUnavailable = true) + } + } + } + } /** * Begin a native SSO flow for [provider]: mint PKCE + state and surface the diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index cab1716..b753bc4 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -60,9 +60,10 @@ Sign in Create an account Forgot your password? - Sign in with Google or Discord (on the website) Sign in with %1$s + Loading sign-in options… + Couldn\'t load sign-in options. Tap to retry. Incorrect username or password. That code didn\'t match. Try the current code. Too many attempts. Please try again shortly. diff --git a/app/src/test/java/com/runicgateway/app/core/auth/sso/SsoAuthManagerTest.kt b/app/src/test/java/com/runicgateway/app/core/auth/sso/SsoAuthManagerTest.kt index 0f01ebe..c7af12b 100644 --- a/app/src/test/java/com/runicgateway/app/core/auth/sso/SsoAuthManagerTest.kt +++ b/app/src/test/java/com/runicgateway/app/core/auth/sso/SsoAuthManagerTest.kt @@ -37,6 +37,14 @@ class SsoAuthManagerTest { override fun clear() { stored = null } } + /** In-memory stand-in for the encrypted pending-SSO store (survives across + * manager instances the way the on-disk store survives process death). */ + private class FakePendingSsoStore(var pending: PendingSso? = null) : PendingSsoStore { + override fun save(state: String, verifier: String) { pending = PendingSso(state, verifier) } + override fun load(): PendingSso? = pending + override fun clear() { pending = null } + } + /** Records the exchange it was called with and returns a scripted response. */ private class FakeSsoApi( private val exchangeResult: () -> Response, @@ -67,10 +75,11 @@ class SsoAuthManagerTest { api: SsoApi, session: SessionManager, base: String? = "https://shard.example.com/", + store: PendingSsoStore = FakePendingSsoStore(), ): SsoAuthManager { val holder = BaseUrlHolder() if (base != null) holder.set(base.toHttpUrl()) - return SsoAuthManager(api, session, holder) + return SsoAuthManager(api, session, holder, store) } /** Build a start URL and pull the generated `state` back out of it. */ @@ -135,6 +144,25 @@ class SsoAuthManagerTest { assertEquals(SsoAuthManager.Outcome.Failed(SsoAuthManager.Failure.STATE_MISMATCH), mgr.outcome.value) } + @Test fun `pending survives process death — a fresh manager on the same store completes`() = runTest { + // Persist the pending on one instance, then throw that instance away. + val store = FakePendingSsoStore() + val session = SessionManager(FakeTokenStore()) + val started = managerWith(FakeSsoApi { Response.success(tokenPair()) }, session, store = store) + val state = startAndState(started) + + // A brand-new manager (simulating the app relaunched after eviction) reads the + // persisted pending and completes the exchange — the old in-memory holder would + // have lost it and failed STATE_MISMATCH. + val api = FakeSsoApi { Response.success(tokenPair()) } + val revived = managerWith(api, session, store = store) + revived.complete(state = state, code = "auth-code-1", error = null) + + assertEquals(1, api.exchangeCalls) + assertTrue(session.state.value is Session.SignedIn) + assertEquals(SsoAuthManager.Outcome.Success, revived.outcome.value) + } + @Test fun `error callback maps to a declined sign-in and does not exchange`() = runTest { val api = FakeSsoApi { Response.success(tokenPair()) } val mgr = managerWith(api, SessionManager(FakeTokenStore()))