TASK-029: OAuth refresh-token support, stop re-prompting for consent
The app captured Gitea's OAuth refresh token on login but never used it -- an expired access token forced a full new browser-based login (and consent screen) every time. Now silently exchanges the stored refresh token for a new access token in the background, proactively before it goes stale, shared via one AuthSession singleton across all five features' API clients instead of each independently reading a possibly-stale token. Verified Gitea's refresh_token grant shape against its own source (routers/web/auth/oauth2_provider.go) before implementing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
bb83c215d3
commit
50d1b41f47
@ -0,0 +1,35 @@
|
||||
package com.ubisam.ubgitea.core.auth.android
|
||||
|
||||
import android.app.Application
|
||||
import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults
|
||||
import com.ubisam.ubgitea.core.network.OkHttpTransport
|
||||
import com.ubisam.ubgitea.data.auth.AuthApi
|
||||
import com.ubisam.ubgitea.data.auth.TokenVault
|
||||
import com.ubisam.ubgitea.data.repositories.BearerTokenProvider
|
||||
|
||||
/**
|
||||
* One shared [TokenVault] and one shared [RefreshingTokenProvider] per process, rather than each
|
||||
* of the app's ViewModel.Factory classes independently constructing its own disconnected instance
|
||||
* the way they did before this existed. Sharing matters for two reasons: a refresh triggered by
|
||||
* one feature is immediately visible (via the shared vault) to every other feature's next API
|
||||
* call instead of each staying stuck with its own stale copy; and the shared provider's internal
|
||||
* mutex actually prevents concurrent refreshes app-wide only if every feature goes through the
|
||||
* same instance.
|
||||
*/
|
||||
object AuthSession {
|
||||
@Volatile private var tokenVaultInstance: TokenVault? = null
|
||||
@Volatile private var tokenProviderInstance: BearerTokenProvider? = null
|
||||
|
||||
fun tokenVault(application: Application): TokenVault =
|
||||
tokenVaultInstance ?: synchronized(this) {
|
||||
tokenVaultInstance ?: AndroidKeystoreTokenVault(application).also { tokenVaultInstance = it }
|
||||
}
|
||||
|
||||
fun tokenProvider(application: Application): BearerTokenProvider =
|
||||
tokenProviderInstance ?: synchronized(this) {
|
||||
tokenProviderInstance ?: RefreshingTokenProvider(
|
||||
tokenVault = tokenVault(application),
|
||||
gateway = AuthApi(UBGiteaOAuthDefaults.config, OkHttpTransport()),
|
||||
).also { tokenProviderInstance = it }
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,60 @@
|
||||
package com.ubisam.ubgitea.core.auth.android
|
||||
|
||||
import com.ubisam.ubgitea.data.auth.AuthenticationGateway
|
||||
import com.ubisam.ubgitea.data.auth.TokenVault
|
||||
import com.ubisam.ubgitea.data.repositories.BearerTokenProvider
|
||||
import kotlinx.coroutines.CancellationException
|
||||
import kotlinx.coroutines.sync.Mutex
|
||||
import kotlinx.coroutines.sync.withLock
|
||||
|
||||
/**
|
||||
* Wraps the stored session so every API call gets a currently-valid access token without the user
|
||||
* ever needing to reopen the browser and re-approve the app. Gitea's `refresh_token` grant never
|
||||
* re-prompts for consent, unlike a fresh `authorization_code` flow does — that's the entire reason
|
||||
* this exists: previously, an expired access token (however that expiry was noticed — a stale
|
||||
* clock check or a live 401) went straight to clearing the session and demanding a full new login,
|
||||
* even though a perfectly usable refresh token was sitting right there unused.
|
||||
*
|
||||
* Refreshing a little *before* actual expiry ([REFRESH_SKEW_MILLIS]), not only reactively after,
|
||||
* matters because this provider is meant to be shared (see [AuthSession]) across every feature's
|
||||
* independent API client — proactively refreshing here means whichever feature happens to make the
|
||||
* next call already sees a fresh token, rather than each one separately racing a token that's
|
||||
* already gone stale. The [Mutex] ensures concurrent callers across features share one in-flight
|
||||
* refresh instead of each independently trying to consume the same (rotating, single-use) refresh
|
||||
* token, which would make every refresh but the first one fail.
|
||||
*/
|
||||
class RefreshingTokenProvider(
|
||||
private val tokenVault: TokenVault,
|
||||
private val gateway: AuthenticationGateway,
|
||||
private val now: () -> Long = System::currentTimeMillis,
|
||||
) : BearerTokenProvider {
|
||||
private val mutex = Mutex()
|
||||
|
||||
override suspend fun accessToken(): String = mutex.withLock {
|
||||
val tokens = tokenVault.read() ?: return@withLock ""
|
||||
val refreshToken = tokens.refreshToken
|
||||
val expiresAt = tokens.expiresAtEpochMillis
|
||||
val needsRefresh = expiresAt != null && expiresAt - REFRESH_SKEW_MILLIS <= now()
|
||||
if (!needsRefresh || refreshToken == null) return@withLock tokens.accessToken
|
||||
|
||||
try {
|
||||
val refreshed = gateway.refreshToken(refreshToken, now())
|
||||
tokenVault.store(refreshed)
|
||||
refreshed.accessToken
|
||||
} catch (failure: CancellationException) {
|
||||
throw failure
|
||||
} catch (_: Exception) {
|
||||
// Refresh itself failed (revoked/expired refresh token, offline, server error) — fall
|
||||
// back to the stale access token rather than blocking the caller on this failure. The
|
||||
// 401 that stale token then draws from the actual API call drives the app's existing
|
||||
// AUTHENTICATION_EXPIRED handling, which is the correct outcome once refreshing
|
||||
// genuinely can't succeed — this provider does not itself decide "give up," it only
|
||||
// ever tries to avoid needing to.
|
||||
tokens.accessToken
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val REFRESH_SKEW_MILLIS = 60_000L
|
||||
}
|
||||
}
|
||||
@ -28,6 +28,11 @@ data class AuthenticatedUser(
|
||||
|
||||
interface AuthenticationGateway {
|
||||
suspend fun exchangeCode(authorizationCode: String, verifier: String, nowEpochMillis: Long): TokenSet
|
||||
// Gitea's refresh_token grant (routers/web/auth/oauth2_provider.go's handleRefreshToken)
|
||||
// never re-prompts for consent, unlike a fresh authorization_code flow through the browser —
|
||||
// this is what lets a session outlive a single short-lived access token without the user
|
||||
// ever seeing the "Authorize this application?" screen again.
|
||||
suspend fun refreshToken(refreshToken: String, nowEpochMillis: Long): TokenSet
|
||||
suspend fun getAuthenticatedUser(accessToken: String): AuthenticatedUser
|
||||
}
|
||||
|
||||
@ -52,13 +57,40 @@ class AuthApi(
|
||||
"code_verifier" to verifier,
|
||||
),
|
||||
).requireSuccess()
|
||||
val dto = parse<TokenResponseDto>(response.body)
|
||||
return parseTokenResponse(response.body, nowEpochMillis, fallbackRefreshToken = null)
|
||||
}
|
||||
|
||||
override suspend fun refreshToken(refreshToken: String, nowEpochMillis: Long): TokenSet {
|
||||
require(refreshToken.isNotBlank()) { "Refresh token cannot be blank" }
|
||||
val response = transport.postForm(
|
||||
url = config.baseUri.resolve("/login/oauth/access_token").toString(),
|
||||
fields = mapOf(
|
||||
"grant_type" to "refresh_token",
|
||||
// No client_secret: this is a public/PKCE client, and Gitea only requires
|
||||
// client_secret validation for confidential clients (routers/web/auth/
|
||||
// oauth2_provider.go), matching exchangeCode's existing authorization_code call
|
||||
// above, which likewise never sends one.
|
||||
"client_id" to config.clientId,
|
||||
"refresh_token" to refreshToken,
|
||||
),
|
||||
).requireSuccess()
|
||||
// Gitea rotates refresh tokens (grant.Counter-based reuse detection) and is expected to
|
||||
// return a new one each call; falling back to the just-used token if a response somehow
|
||||
// omits it means the next refresh attempt is still possible rather than refresh support
|
||||
// silently breaking, at the cost of that fallback attempt itself possibly failing if the
|
||||
// old token really was invalidated — no worse than not having refresh support at all.
|
||||
return parseTokenResponse(response.body, nowEpochMillis, fallbackRefreshToken = refreshToken)
|
||||
}
|
||||
|
||||
private fun parseTokenResponse(body: String, nowEpochMillis: Long, fallbackRefreshToken: String?): TokenSet {
|
||||
val dto = parse<TokenResponseDto>(body)
|
||||
val expiresAt = dto.expiresIn
|
||||
?.takeIf { it > 0 }
|
||||
?.let { seconds -> nowEpochMillis.saturatingAdd(seconds.saturatingMilliseconds()) }
|
||||
val accessToken = dto.accessToken?.takeIf(String::isNotBlank)
|
||||
?: throw ApiFailure.InvalidResponse()
|
||||
return TokenSet(accessToken, dto.refreshToken?.takeIf(String::isNotBlank), expiresAt)
|
||||
val refreshToken = dto.refreshToken?.takeIf(String::isNotBlank) ?: fallbackRefreshToken
|
||||
return TokenSet(accessToken, refreshToken, expiresAt)
|
||||
}
|
||||
|
||||
override suspend fun getAuthenticatedUser(accessToken: String): AuthenticatedUser {
|
||||
|
||||
@ -4,11 +4,10 @@ import android.app.Application
|
||||
import androidx.lifecycle.ViewModel
|
||||
import androidx.lifecycle.ViewModelProvider
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import com.ubisam.ubgitea.core.auth.android.AndroidKeystoreTokenVault
|
||||
import com.ubisam.ubgitea.core.auth.android.AuthSession
|
||||
import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults
|
||||
import com.ubisam.ubgitea.core.network.ApiFailure
|
||||
import com.ubisam.ubgitea.core.network.OkHttpTransport
|
||||
import com.ubisam.ubgitea.data.repositories.BearerTokenProvider
|
||||
import com.ubisam.ubgitea.data.repositories.GiteaRepositoryBrowserApi
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryCoordinates
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryPath
|
||||
@ -235,13 +234,12 @@ class CodeBrowserViewModel(
|
||||
class Factory(private val application: Application) : ViewModelProvider.Factory {
|
||||
override fun <T : ViewModel> create(modelClass: Class<T>): T {
|
||||
require(modelClass.isAssignableFrom(CodeBrowserViewModel::class.java))
|
||||
val tokenVault = AndroidKeystoreTokenVault(application)
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
return CodeBrowserViewModel(
|
||||
GiteaRepositoryBrowserApi(
|
||||
UBGiteaOAuthDefaults.config,
|
||||
OkHttpTransport(),
|
||||
BearerTokenProvider { tokenVault.read()?.accessToken.orEmpty() },
|
||||
AuthSession.tokenProvider(application),
|
||||
),
|
||||
) as T
|
||||
}
|
||||
|
||||
@ -4,12 +4,11 @@ import android.app.Application
|
||||
import androidx.lifecycle.ViewModel
|
||||
import androidx.lifecycle.ViewModelProvider
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import com.ubisam.ubgitea.core.auth.android.AndroidKeystoreTokenVault
|
||||
import com.ubisam.ubgitea.core.auth.android.AuthSession
|
||||
import com.ubisam.ubgitea.core.network.ApiFailure
|
||||
import com.ubisam.ubgitea.core.network.OkHttpTransport
|
||||
import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults
|
||||
import com.ubisam.ubgitea.data.history.GiteaHistoryApi
|
||||
import com.ubisam.ubgitea.data.repositories.BearerTokenProvider
|
||||
import com.ubisam.ubgitea.data.repositories.GiteaRepositoryBrowserApi
|
||||
import com.ubisam.ubgitea.domain.history.CommitDetail
|
||||
import com.ubisam.ubgitea.domain.history.DiffRow
|
||||
@ -237,8 +236,7 @@ class HistoryViewModel(
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
override fun <T : ViewModel> create(modelClass: Class<T>): T {
|
||||
require(modelClass.isAssignableFrom(HistoryViewModel::class.java))
|
||||
val tokenVault = AndroidKeystoreTokenVault(application)
|
||||
val tokenProvider = BearerTokenProvider { tokenVault.read()?.accessToken.orEmpty() }
|
||||
val tokenProvider = AuthSession.tokenProvider(application)
|
||||
val browser = GiteaRepositoryBrowserApi(UBGiteaOAuthDefaults.config, OkHttpTransport(), tokenProvider)
|
||||
val history = GiteaHistoryApi(UBGiteaOAuthDefaults.config, OkHttpTransport(), tokenProvider)
|
||||
return HistoryViewModel(browser, history) as T
|
||||
|
||||
@ -4,12 +4,11 @@ import android.app.Application
|
||||
import androidx.lifecycle.ViewModel
|
||||
import androidx.lifecycle.ViewModelProvider
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import com.ubisam.ubgitea.core.auth.android.AndroidKeystoreTokenVault
|
||||
import com.ubisam.ubgitea.core.auth.android.AuthSession
|
||||
import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults
|
||||
import com.ubisam.ubgitea.core.network.ApiFailure
|
||||
import com.ubisam.ubgitea.core.network.OkHttpTransport
|
||||
import com.ubisam.ubgitea.data.notifications.GiteaNotificationsApi
|
||||
import com.ubisam.ubgitea.data.repositories.BearerTokenProvider
|
||||
import com.ubisam.ubgitea.domain.notifications.NotificationSummary
|
||||
import com.ubisam.ubgitea.domain.notifications.NotificationsRepository
|
||||
import java.io.IOException
|
||||
@ -98,11 +97,10 @@ class NotificationsViewModel(private val repository: NotificationsRepository) :
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
override fun <T : ViewModel> create(modelClass: Class<T>): T {
|
||||
require(modelClass.isAssignableFrom(NotificationsViewModel::class.java))
|
||||
val tokenVault = AndroidKeystoreTokenVault(application)
|
||||
val repository = GiteaNotificationsApi(
|
||||
UBGiteaOAuthDefaults.config,
|
||||
OkHttpTransport(),
|
||||
BearerTokenProvider { tokenVault.read()?.accessToken.orEmpty() },
|
||||
AuthSession.tokenProvider(application),
|
||||
)
|
||||
return NotificationsViewModel(repository) as T
|
||||
}
|
||||
|
||||
@ -5,7 +5,7 @@ import com.ubisam.ubgitea.BuildConfig
|
||||
import androidx.lifecycle.ViewModel
|
||||
import androidx.lifecycle.ViewModelProvider
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import com.ubisam.ubgitea.core.auth.android.AndroidKeystoreTokenVault
|
||||
import com.ubisam.ubgitea.core.auth.android.AuthSession
|
||||
import com.ubisam.ubgitea.core.auth.android.PendingAuthStore
|
||||
import com.ubisam.ubgitea.core.auth.android.PendingAuthTransactionStore
|
||||
import com.ubisam.ubgitea.core.auth.protocol.OAuthAuthorizationRequestFactory
|
||||
@ -15,7 +15,6 @@ import com.ubisam.ubgitea.core.network.ApiFailure
|
||||
import com.ubisam.ubgitea.core.network.OkHttpTransport
|
||||
import com.ubisam.ubgitea.data.auth.AuthApi
|
||||
import com.ubisam.ubgitea.data.auth.AuthenticationGateway
|
||||
import com.ubisam.ubgitea.data.auth.TokenSet
|
||||
import com.ubisam.ubgitea.data.auth.TokenVault
|
||||
import com.ubisam.ubgitea.data.repositories.BearerTokenProvider
|
||||
import com.ubisam.ubgitea.data.repositories.GiteaRepositoryBrowserApi
|
||||
@ -143,6 +142,10 @@ class RepositoryListViewModel(
|
||||
private val browserApi: GiteaRepositoryBrowser,
|
||||
private val favoritesApi: RepositoryFavorites,
|
||||
private val organizationsApi: OrganizationsRepository,
|
||||
// Defaulted to the old no-refresh behavior (read whatever's stored, as-is) so every existing
|
||||
// caller/test keeps compiling and behaving identically; production wiring (Factory, below)
|
||||
// explicitly passes the shared refresh-aware provider instead.
|
||||
private val tokenProvider: BearerTokenProvider = BearerTokenProvider { tokenVault.read()?.accessToken.orEmpty() },
|
||||
private val now: () -> Long = System::currentTimeMillis,
|
||||
) : ViewModel() {
|
||||
private val requestFactory = OAuthAuthorizationRequestFactory(UBGiteaOAuthDefaults.config)
|
||||
@ -356,7 +359,7 @@ class RepositoryListViewModel(
|
||||
val tokens = authApi.exchangeCode(code, verifier, now())
|
||||
tokenVault.store(tokens)
|
||||
hasAuthenticatedSession = true
|
||||
loadRepositories(tokens)
|
||||
loadRepositories()
|
||||
} catch (failure: Throwable) {
|
||||
if (failure is CancellationException) throw failure
|
||||
if (failure is ApiFailure.AuthenticationExpired) {
|
||||
@ -381,15 +384,13 @@ class RepositoryListViewModel(
|
||||
return
|
||||
}
|
||||
hasAuthenticatedSession = true
|
||||
if (tokens.expiresAtEpochMillis?.let { it <= now() } == true) {
|
||||
tokenVault.clear()
|
||||
activeAccountName = null
|
||||
hasAuthenticatedSession = false
|
||||
mutableState.value = RepositoryListUiState.Error(RepositoryListError.AUTHENTICATION_EXPIRED)
|
||||
return
|
||||
}
|
||||
// No more "already past its stated expiry -> fail immediately" short-circuit here: tokens
|
||||
// now go through tokenProvider (refresh-aware in production), which silently renews an
|
||||
// expired-but-refreshable token before the request below is even made. Genuine expiry —
|
||||
// no refresh token available, or the refresh attempt itself failing — still surfaces as
|
||||
// the exact same ApiFailure.AuthenticationExpired from the API call, caught below.
|
||||
try {
|
||||
loadRepositories(tokens)
|
||||
loadRepositories()
|
||||
} catch (failure: Throwable) {
|
||||
if (failure is CancellationException) throw failure
|
||||
if (failure is ApiFailure.AuthenticationExpired) {
|
||||
@ -405,8 +406,8 @@ class RepositoryListViewModel(
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun loadRepositories(tokens: TokenSet) {
|
||||
val user = authApi.getAuthenticatedUser(tokens.accessToken)
|
||||
private suspend fun loadRepositories() {
|
||||
val user = authApi.getAuthenticatedUser(tokenProvider.accessToken())
|
||||
val accountName = user.displayName ?: user.login
|
||||
activeAccountName = accountName
|
||||
loadRepositoryPage(accountName, page = 1, mode = RepositoryListMode.ALL, organization = null)
|
||||
@ -622,20 +623,20 @@ class RepositoryListViewModel(
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
override fun <T : ViewModel> create(modelClass: Class<T>): T {
|
||||
require(modelClass.isAssignableFrom(RepositoryListViewModel::class.java))
|
||||
val tokenVault = AndroidKeystoreTokenVault(application)
|
||||
val authTransport = OkHttpTransport()
|
||||
val tokenVault = AuthSession.tokenVault(application)
|
||||
val browser = GiteaRepositoryBrowserApi(
|
||||
UBGiteaOAuthDefaults.config,
|
||||
OkHttpTransport(),
|
||||
BearerTokenProvider { tokenVault.read()?.accessToken.orEmpty() },
|
||||
AuthSession.tokenProvider(application),
|
||||
)
|
||||
return RepositoryListViewModel(
|
||||
tokenVault = tokenVault,
|
||||
pendingAuthStore = PendingAuthStore(application),
|
||||
authApi = AuthApi(UBGiteaOAuthDefaults.config, authTransport),
|
||||
authApi = AuthApi(UBGiteaOAuthDefaults.config, OkHttpTransport()),
|
||||
browserApi = browser,
|
||||
favoritesApi = browser,
|
||||
organizationsApi = browser,
|
||||
tokenProvider = AuthSession.tokenProvider(application),
|
||||
) as T
|
||||
}
|
||||
}
|
||||
|
||||
@ -4,11 +4,10 @@ import android.app.Application
|
||||
import androidx.lifecycle.ViewModel
|
||||
import androidx.lifecycle.ViewModelProvider
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import com.ubisam.ubgitea.core.auth.android.AndroidKeystoreTokenVault
|
||||
import com.ubisam.ubgitea.core.auth.android.AuthSession
|
||||
import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults
|
||||
import com.ubisam.ubgitea.core.network.ApiFailure
|
||||
import com.ubisam.ubgitea.core.network.OkHttpTransport
|
||||
import com.ubisam.ubgitea.data.repositories.BearerTokenProvider
|
||||
import com.ubisam.ubgitea.data.workitems.GiteaWorkItemsApi
|
||||
import com.ubisam.ubgitea.domain.model.Page
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryCoordinates
|
||||
@ -241,8 +240,7 @@ class WorkItemsViewModel(private val repository: WorkItemsRepository) : ViewMode
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
override fun <T : ViewModel> create(modelClass: Class<T>): T {
|
||||
require(modelClass.isAssignableFrom(WorkItemsViewModel::class.java))
|
||||
val tokenVault = AndroidKeystoreTokenVault(application)
|
||||
val tokenProvider = BearerTokenProvider { tokenVault.read()?.accessToken.orEmpty() }
|
||||
val tokenProvider = AuthSession.tokenProvider(application)
|
||||
val repository = GiteaWorkItemsApi(UBGiteaOAuthDefaults.config, OkHttpTransport(), tokenProvider)
|
||||
return WorkItemsViewModel(repository) as T
|
||||
}
|
||||
|
||||
60
app/src/main/java/com/ubisam/ubgitea/ui/theme/Color.kt
Normal file
60
app/src/main/java/com/ubisam/ubgitea/ui/theme/Color.kt
Normal file
@ -0,0 +1,60 @@
|
||||
package com.ubisam.ubgitea.ui.theme
|
||||
|
||||
import androidx.compose.ui.graphics.Color
|
||||
|
||||
// Gitea's own logo/brand green is #609926 (verified against Gitea's source:
|
||||
// web_src/css/themes/theme-gitea-light.css's --color-logo). The Material3 primary role below is a
|
||||
// deeper tone of that same hue rather than the exact logo value, purely for contrast: white text
|
||||
// on #609926 sits around a 3.4:1 contrast ratio, under WCAG's 4.5:1 threshold for normal-size
|
||||
// button labels. Every other color here is a hand-built, standard-shaped Material3 tonal
|
||||
// palette (primary/secondary/tertiary, each with a container + on-* pair, plus neutrals) grown
|
||||
// from that same green family, so the whole app reads as one deliberate "Gitea green" theme
|
||||
// instead of Compose's unthemed default (Material3's baseline purple, which is what every screen
|
||||
// in this app looked like before this file existed with real values).
|
||||
|
||||
// Light theme
|
||||
val GreenPrimaryLight = Color(0xFF3B6A1F)
|
||||
val OnGreenPrimaryLight = Color(0xFFFFFFFF)
|
||||
val GreenPrimaryContainerLight = Color(0xFFBCEF8B)
|
||||
val OnGreenPrimaryContainerLight = Color(0xFF0C2000)
|
||||
val SageSecondaryLight = Color(0xFF55624A)
|
||||
val OnSageSecondaryLight = Color(0xFFFFFFFF)
|
||||
val SageSecondaryContainerLight = Color(0xFFD9E7C8)
|
||||
val OnSageSecondaryContainerLight = Color(0xFF131F0B)
|
||||
val TealTertiaryLight = Color(0xFF386661)
|
||||
val OnTealTertiaryLight = Color(0xFFFFFFFF)
|
||||
val TealTertiaryContainerLight = Color(0xFFBBECE5)
|
||||
val OnTealTertiaryContainerLight = Color(0xFF00201D)
|
||||
val BackgroundLight = Color(0xFFFCFDF6)
|
||||
val OnBackgroundLight = Color(0xFF1A1C16)
|
||||
val SurfaceVariantLight = Color(0xFFDFE4D3)
|
||||
val OnSurfaceVariantLight = Color(0xFF43483C)
|
||||
val OutlineLight = Color(0xFF73796B)
|
||||
val OutlineVariantLight = Color(0xFFC3C8B8)
|
||||
val InverseSurfaceLight = Color(0xFF2F312A)
|
||||
val InverseOnSurfaceLight = Color(0xFFF1F1E9)
|
||||
val InversePrimaryLight = Color(0xFFA1D474)
|
||||
|
||||
// Dark theme — tones flip per Material3 convention (lighter primary for legibility on a dark
|
||||
// surface; the light theme's primary becomes the dark theme's inversePrimary, and vice versa).
|
||||
val GreenPrimaryDark = Color(0xFFA1D474)
|
||||
val OnGreenPrimaryDark = Color(0xFF1B3700)
|
||||
val GreenPrimaryContainerDark = Color(0xFF2C4F0A)
|
||||
val OnGreenPrimaryContainerDark = Color(0xFFBCEF8B)
|
||||
val SageSecondaryDark = Color(0xFFBDCBAB)
|
||||
val OnSageSecondaryDark = Color(0xFF28331E)
|
||||
val SageSecondaryContainerDark = Color(0xFF3E4A32)
|
||||
val OnSageSecondaryContainerDark = Color(0xFFD9E7C8)
|
||||
val TealTertiaryDark = Color(0xFFA0D0C9)
|
||||
val OnTealTertiaryDark = Color(0xFF003733)
|
||||
val TealTertiaryContainerDark = Color(0xFF1E4E49)
|
||||
val OnTealTertiaryContainerDark = Color(0xFFBBECE5)
|
||||
val BackgroundDark = Color(0xFF12140F)
|
||||
val OnBackgroundDark = Color(0xFFE2E3D9)
|
||||
val SurfaceVariantDark = Color(0xFF43483C)
|
||||
val OnSurfaceVariantDark = Color(0xFFC3C8B8)
|
||||
val OutlineDark = Color(0xFF8D9285)
|
||||
val OutlineVariantDark = Color(0xFF43483C)
|
||||
val InverseSurfaceDark = Color(0xFFE2E3D9)
|
||||
val InverseOnSurfaceDark = Color(0xFF2F312A)
|
||||
val InversePrimaryDark = Color(0xFF3B6A1F)
|
||||
@ -0,0 +1,121 @@
|
||||
package com.ubisam.ubgitea.core.auth.android
|
||||
|
||||
import com.ubisam.ubgitea.data.auth.AuthenticatedUser
|
||||
import com.ubisam.ubgitea.data.auth.AuthenticationGateway
|
||||
import com.ubisam.ubgitea.data.auth.TokenSet
|
||||
import com.ubisam.ubgitea.data.auth.TokenVault
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
|
||||
class RefreshingTokenProviderTest {
|
||||
@Test
|
||||
fun `returns the stored token unchanged when it is not close to expiry`() = runTest {
|
||||
val vault = FakeVault(TokenSet("access", "refresh", expiresAtEpochMillis = 200_000L))
|
||||
val gateway = FakeGateway()
|
||||
val provider = RefreshingTokenProvider(vault, gateway, now = { 1_000L })
|
||||
|
||||
val token = provider.accessToken()
|
||||
|
||||
assertEquals("access", token)
|
||||
assertEquals(0, gateway.refreshCalls)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `refreshes and persists a new token set once expiry is within the skew window`() = runTest {
|
||||
val vault = FakeVault(TokenSet("stale-access", "refresh", expiresAtEpochMillis = 1_030_000L))
|
||||
val gateway = FakeGateway(refreshed = TokenSet("fresh-access", "fresh-refresh", 2_000_000L))
|
||||
val provider = RefreshingTokenProvider(vault, gateway, now = { 1_000_000L })
|
||||
|
||||
val token = provider.accessToken()
|
||||
|
||||
assertEquals("fresh-access", token)
|
||||
assertEquals(1, gateway.refreshCalls)
|
||||
assertEquals("fresh-access", vault.stored?.accessToken)
|
||||
assertEquals("fresh-refresh", vault.stored?.refreshToken)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `refreshes proactively before the token has actually expired`() = runTest {
|
||||
// expiresAtEpochMillis is still in the future relative to now, but within the skew window.
|
||||
val vault = FakeVault(TokenSet("stale-access", "refresh", expiresAtEpochMillis = 1_030_000L))
|
||||
val gateway = FakeGateway(refreshed = TokenSet("fresh-access", "fresh-refresh", 2_000_000L))
|
||||
val provider = RefreshingTokenProvider(vault, gateway, now = { 1_000_000L })
|
||||
|
||||
provider.accessToken()
|
||||
|
||||
assertEquals(1, gateway.refreshCalls)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `falls back to the stale token when there is no refresh token to use`() = runTest {
|
||||
val vault = FakeVault(TokenSet("stale-access", refreshToken = null, expiresAtEpochMillis = 500L))
|
||||
val gateway = FakeGateway()
|
||||
val provider = RefreshingTokenProvider(vault, gateway, now = { 1_000L })
|
||||
|
||||
val token = provider.accessToken()
|
||||
|
||||
assertEquals("stale-access", token)
|
||||
assertEquals(0, gateway.refreshCalls)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `falls back to the stale token when the refresh attempt itself fails`() = runTest {
|
||||
val vault = FakeVault(TokenSet("stale-access", "refresh", expiresAtEpochMillis = 500L))
|
||||
val gateway = FakeGateway(refreshFailure = RuntimeException("offline"))
|
||||
val provider = RefreshingTokenProvider(vault, gateway, now = { 1_000L })
|
||||
|
||||
val token = provider.accessToken()
|
||||
|
||||
assertEquals("stale-access", token)
|
||||
// store() is never reached on a failed refresh, so the vault is left exactly as it
|
||||
// was — not cleared, not overwritten.
|
||||
assertEquals("stale-access", vault.stored?.accessToken)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `returns empty string when nothing is stored at all`() = runTest {
|
||||
val vault = FakeVault(null)
|
||||
val provider = RefreshingTokenProvider(vault, FakeGateway())
|
||||
|
||||
assertEquals("", provider.accessToken())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a token with no expiry is never considered due for refresh`() = runTest {
|
||||
val vault = FakeVault(TokenSet("access", "refresh", expiresAtEpochMillis = null))
|
||||
val gateway = FakeGateway()
|
||||
val provider = RefreshingTokenProvider(vault, gateway, now = { Long.MAX_VALUE })
|
||||
|
||||
val token = provider.accessToken()
|
||||
|
||||
assertEquals("access", token)
|
||||
assertEquals(0, gateway.refreshCalls)
|
||||
}
|
||||
|
||||
private class FakeVault(initial: TokenSet?) : TokenVault {
|
||||
var stored: TokenSet? = initial
|
||||
override suspend fun store(tokens: TokenSet) { stored = tokens }
|
||||
override suspend fun read(): TokenSet? = stored
|
||||
override suspend fun clear() { stored = null }
|
||||
}
|
||||
|
||||
private class FakeGateway(
|
||||
private val refreshed: TokenSet? = null,
|
||||
private val refreshFailure: Throwable? = null,
|
||||
) : AuthenticationGateway {
|
||||
var refreshCalls = 0
|
||||
|
||||
override suspend fun exchangeCode(authorizationCode: String, verifier: String, nowEpochMillis: Long): TokenSet =
|
||||
error("Unexpected exchangeCode call")
|
||||
|
||||
override suspend fun refreshToken(refreshToken: String, nowEpochMillis: Long): TokenSet {
|
||||
refreshCalls++
|
||||
refreshFailure?.let { throw it }
|
||||
return refreshed ?: error("No refreshed token configured")
|
||||
}
|
||||
|
||||
override suspend fun getAuthenticatedUser(accessToken: String): AuthenticatedUser =
|
||||
error("Unexpected getAuthenticatedUser call")
|
||||
}
|
||||
}
|
||||
@ -27,6 +27,45 @@ class AuthApiTest {
|
||||
assertFalse(transport.postedFields.keys.any { it.contains("secret", ignoreCase = true) })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `token refresh sends grant_type refresh_token and never a client secret`() = runTest {
|
||||
val transport = RecordingTransport(
|
||||
postResponse = HttpResponse(200, """{"access_token":"new-access","refresh_token":"new-refresh","expires_in":60}""", emptyMap()),
|
||||
)
|
||||
|
||||
val tokens = AuthApi(UBGiteaOAuthDefaults.config, transport).refreshToken("old-refresh", 1_000L)
|
||||
|
||||
assertEquals("new-access", tokens.accessToken)
|
||||
assertEquals("new-refresh", tokens.refreshToken)
|
||||
assertEquals(61_000L, tokens.expiresAtEpochMillis)
|
||||
assertEquals("refresh_token", transport.postedFields["grant_type"])
|
||||
assertEquals("old-refresh", transport.postedFields["refresh_token"])
|
||||
assertFalse(transport.postedFields.keys.any { it.contains("secret", ignoreCase = true) })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `token refresh falls back to the token just used when the response omits a new one`() = runTest {
|
||||
val transport = RecordingTransport(
|
||||
postResponse = HttpResponse(200, """{"access_token":"new-access","expires_in":60}""", emptyMap()),
|
||||
)
|
||||
|
||||
val tokens = AuthApi(UBGiteaOAuthDefaults.config, transport).refreshToken("old-refresh", 1_000L)
|
||||
|
||||
assertEquals("old-refresh", tokens.refreshToken)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `blank refresh token is rejected before any request`() = runTest {
|
||||
val transport = RecordingTransport()
|
||||
|
||||
try {
|
||||
AuthApi(UBGiteaOAuthDefaults.config, transport).refreshToken(" ", 1_000L)
|
||||
fail("Expected IllegalArgumentException")
|
||||
} catch (_: IllegalArgumentException) {
|
||||
// Expected.
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `authenticated user maps display name`() = runTest {
|
||||
val transport = RecordingTransport(
|
||||
|
||||
@ -62,6 +62,28 @@ class RepositoryListViewModelTest {
|
||||
assertEquals(listOf("v1"), references.tags.map { it.name })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `restoring the session authenticates through the injected token provider, not the raw stored token`() = runTest(dispatcher.scheduler) {
|
||||
// The stored token is deliberately stale/wrong here; a token-provider-backed refresh (as
|
||||
// production wiring uses) is what's expected to supply the actually-used access token.
|
||||
val authGateway = FakeAuthGateway()
|
||||
val browser = FakeBrowser(repositories = listOf(page1Of(repo("repo"))))
|
||||
val viewModel = RepositoryListViewModel(
|
||||
tokenVault = FakeTokenVault(TokenSet("stale-stored-access", null, null)),
|
||||
pendingAuthStore = FakePendingStore(),
|
||||
authApi = authGateway,
|
||||
browserApi = browser,
|
||||
favoritesApi = browser,
|
||||
organizationsApi = browser,
|
||||
tokenProvider = { "provider-issued-access" },
|
||||
now = { 1_000L },
|
||||
)
|
||||
advanceUntilIdle()
|
||||
|
||||
assertTrue(viewModel.state.value is RepositoryListUiState.Content)
|
||||
assertEquals("provider-issued-access", authGateway.lastAuthenticatedUserToken)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `selecting tag updates one selected ref and back restores repository snapshot`() = runTest(dispatcher.scheduler) {
|
||||
val viewModel = viewModel(FakeBrowser(repositories = listOf(page1Of(repo("repo")))))
|
||||
@ -493,9 +515,15 @@ class RepositoryListViewModelTest {
|
||||
}
|
||||
|
||||
private class FakeAuthGateway : AuthenticationGateway {
|
||||
var lastAuthenticatedUserToken: String? = null
|
||||
override suspend fun exchangeCode(authorizationCode: String, verifier: String, nowEpochMillis: Long) =
|
||||
TokenSet("access", null, null)
|
||||
override suspend fun getAuthenticatedUser(accessToken: String) = AuthenticatedUser(1, "account", null)
|
||||
override suspend fun refreshToken(refreshToken: String, nowEpochMillis: Long) =
|
||||
TokenSet("refreshed-access", refreshToken, null)
|
||||
override suspend fun getAuthenticatedUser(accessToken: String): AuthenticatedUser {
|
||||
lastAuthenticatedUserToken = accessToken
|
||||
return AuthenticatedUser(1, "account", null)
|
||||
}
|
||||
}
|
||||
|
||||
private class FakeBrowser(
|
||||
|
||||
@ -478,3 +478,17 @@ This is the append-only implementation and handoff record shared by Codex and Cl
|
||||
- Known limitations: Neither the PDF rendering path nor the shared gesture code has a dedicated unit test — both are Android-framework/pure-UI concerns with no non-UI logic to isolate (`PdfRenderer` specifically isn't available in a plain JVM unit test without Robolectric). Not yet confirmed against a real `.pdf` file on the live device — no repository browsed this session had one readily reachable, same as the still-outstanding CSV table confirmation. Re-opening and fully re-rendering the file on every page turn hasn't been performance-checked against a real large, many-page PDF.
|
||||
- Next recommended task: Get live confirmation of the PDF viewer, the CSV table, pinch-zoom/pan/double-tap, and the Issues/PR numbered pagination — everything built this session that's still pending a live look — otherwise continue per the standing direction. No other items remain queued.
|
||||
- Next owner: Claude, continuing per the standing direction.
|
||||
|
||||
## 2026-08-27 — TASK-029: OAuth refresh-token support
|
||||
|
||||
- Agent and role: Claude, responding directly to a live complaint — "그리고 애플리케이션 승인 안하고 로그인 쭉 이어서 되어있게 해주면 안되나?" (can't login just stay continuous without having to approve the application) — asked immediately after a separate feature request (push notifications for favorited repos, see feasibility note below) revealed *why* it mattered to the human product owner: reliable long-lived auth is a prerequisite for any kind of background checking.
|
||||
- Commit/diff: continued on `claude/task-015-notifications`; `data/auth/AuthApi.kt`, new `core/auth/android/RefreshingTokenProvider.kt` and `core/auth/android/AuthSession.kt`, `feature/repositories/RepositoryListViewModel.kt`, and the `Factory` classes of all five ViewModels, plus matching tests and `tasks/029-oauth-token-refresh.md`.
|
||||
- Bug found while investigating: the app already captured Gitea's OAuth refresh token on login (it was part of the persisted `TokenSet`) but never actually used it anywhere — `RepositoryListViewModel.restoreSession()` cleared the whole session and forced a brand-new browser-based login (Custom Tab open, Gitea login, "Authorize this application?" consent screen) the instant the access token's stated expiry passed, every single time, discarding a perfectly usable refresh token each time.
|
||||
- How it was implemented: verified Gitea's actual `refresh_token` grant shape directly against its source (`routers/web/auth/oauth2_provider.go`'s `handleRefreshToken`) before implementing, per this project's established convention, rather than assuming the standard OAuth2 shape applied unmodified. Added `AuthenticationGateway.refreshToken` (shares the existing `exchangeCode` response-parsing code) and a new `RefreshingTokenProvider` that proactively refreshes a token 60 seconds ahead of its stated expiry (not reactively after a failed call) — proactive specifically because the provider is now *shared* across all five features' independent API clients via a new `AuthSession` singleton, replacing what used to be five separately-constructed, mutually-unaware `AndroidKeystoreTokenVault`/`BearerTokenProvider` pairs; sharing means a refresh triggered by one feature's next call is immediately visible to every other feature too, and the provider's internal `Mutex` actually prevents concurrent refreshes only because every feature now goes through the same instance. `RepositoryListViewModel.restoreSession()`'s old "already past its stated expiry → clear and fail immediately" clock check was removed outright (not adjusted) since it fired *before* the now-refresh-capable API call ever got a chance to run — the exact same fallback behavior (clear + `AUTHENTICATION_EXPIRED`) still happens today, just driven by a real failed API call instead of a client-side guess made too early.
|
||||
- Files changed: see `tasks/029-oauth-token-refresh.md`'s Scope section for the full list.
|
||||
- Checks and results: `./gradlew compileDebugKotlin compileDebugUnitTestKotlin` BUILD SUCCESSFUL (after finding and fixing one missed `loadRepositories(tokens)` call site broken by the signature change); `./gradlew testDebugUnitTest` BUILD SUCCESSFUL, all unit tests passing (after finding and fixing two bugs in the new tests' own fixture data — not the implementation — flagged by two initially-failing assertions); `./gradlew assembleDebug lintDebug` BUILD SUCCESSFUL, 0 lint errors. Installed on the human product owner's device over wireless `adb`.
|
||||
- Review findings and resolutions: None from a separate review pass.
|
||||
- Known limitations: Proactive-refresh only — no "catch a live 401 and retry once" fallback at the transport layer (a token revoked server-side for a reason other than ordinary expiry still surfaces as a normal 401, correctly, just not silently retried). Not yet confirmed against a real long-idle session on the device — the fix targets a problem that only manifests after enough real time passes for a token to actually expire, and this session's live testing so far has been continuous active use rather than a multi-hour gap.
|
||||
- Also this turn: gave the human product owner a feasibility assessment (not yet implemented) for a separately-requested feature — a notification when a favorited repository receives a push. Gitea's own server-side notification system doesn't fire for plain pushes (only issue/PR activity), so this isn't reachable via the existing `/notifications` integration; it would need a new periodic background check (Android `WorkManager`, ~15-minute minimum interval) comparing each favorited repo's last-push time against a locally remembered value, plus a new notification permission and a new dependency. Told the human product owner it's feasible but sized as its own separate task rather than folding into this batch; this refresh-token work is a direct prerequisite for it either way.
|
||||
- Next recommended task: Continue the still-queued visual redesign (Gitea-green theme, pagination footer height-consistency fix) and repository-row enhancements (favorite/star toggle, primary language display) requested in the same live session, then circle back to live-confirm everything still pending a look.
|
||||
- Next owner: Claude, continuing per the standing direction.
|
||||
|
||||
171
tasks/029-oauth-token-refresh.md
Normal file
171
tasks/029-oauth-token-refresh.md
Normal file
@ -0,0 +1,171 @@
|
||||
# TASK-029: OAuth refresh-token support (stop re-prompting for consent every session)
|
||||
|
||||
Status: implemented; all required checks pass; live device verification pending
|
||||
Owner: Claude
|
||||
Reviewer: Codex
|
||||
|
||||
## Outcome
|
||||
|
||||
The app previously captured Gitea's OAuth refresh token on login but never actually used it: once
|
||||
the access token's stated expiry passed, `RepositoryListViewModel.restoreSession()` cleared the
|
||||
session outright and forced the user through a brand-new `authorization_code` flow — browser open,
|
||||
Gitea login, and (per the human product owner's direct complaint, "애플리케이션 승인 안하고 로그인
|
||||
쭉 이어서 되어있게 해주면 안되나?") the "Authorize this application?" consent screen — every time.
|
||||
The app now silently exchanges the stored refresh token for a new access token in the background
|
||||
before it's needed, never opening a browser or showing any Gitea UI, which is what the standard
|
||||
OAuth `refresh_token` grant is specifically designed to do without ever re-prompting for consent.
|
||||
|
||||
## Scope
|
||||
|
||||
- Included: `data/auth/AuthApi.kt` (`AuthenticationGateway.refreshToken`, its `AuthApi`
|
||||
implementation, shared token-response parsing extracted from `exchangeCode`), new
|
||||
`core/auth/android/RefreshingTokenProvider.kt` (the actual refresh-before-stale logic, shared
|
||||
Mutex for single-flight refresh), new `core/auth/android/AuthSession.kt` (one shared
|
||||
`TokenVault`/`RefreshingTokenProvider` per process, replacing five independent
|
||||
`AndroidKeystoreTokenVault`/`BearerTokenProvider` constructions), `feature/repositories/RepositoryListViewModel.kt`
|
||||
(`tokenProvider` param, `restoreSession()`/`loadRepositories()` no longer short-circuit on a
|
||||
stale clock check), and the `Factory` classes of `CodeBrowserViewModel`, `HistoryViewModel`,
|
||||
`WorkItemsViewModel`, `NotificationsViewModel`, `RepositoryListViewModel` (all five now share
|
||||
`AuthSession`'s vault/provider instead of each building its own disconnected one).
|
||||
- Excluded: No change to the initial `authorization_code` login flow itself (TASK-026's Custom
|
||||
Tabs change is untouched), no change to what's stored or how (still the same
|
||||
`AndroidKeystoreTokenVault` encryption, just one shared instance instead of five), no reactive
|
||||
"retry the request once after a 401" fallback — this task is proactive-refresh only (see Known
|
||||
limitations).
|
||||
|
||||
## Design notes
|
||||
|
||||
- **Verified against Gitea's own source before implementing**, per this project's established
|
||||
convention for anything not directly provable from the client: `routers/web/auth/oauth2_provider.go`'s
|
||||
`handleRefreshToken` confirms the `refresh_token` grant reads `grant_type`, `refresh_token`, and
|
||||
`client_id` (no `client_secret` needed for a public/PKCE client — only confidential clients are
|
||||
validated against one), and reuses the same `NewAccessTokenResponse` response shape as the
|
||||
existing, already-working `authorization_code` exchange. Gitea's own refresh-token reuse
|
||||
detection (`grant.Counter`-based) confirms it rotates refresh tokens on each use, which is why
|
||||
the response's `refresh_token` (when present) always replaces the one just used, not the other
|
||||
way around.
|
||||
- **Proactive, not reactive**: `RefreshingTokenProvider` refreshes a token that's within 60 seconds
|
||||
of its stated expiry *before* handing it out, rather than waiting for an API call to actually
|
||||
fail with a 401 first. This matters specifically because the refresh-aware provider is shared
|
||||
(see below) across five independent feature API clients — proactively refreshing means whichever
|
||||
feature happens to make the next call already sees a fresh token, instead of each one separately
|
||||
racing a token that's already gone stale and only some of them noticing.
|
||||
- **One shared `AuthSession` singleton, not five independent instances**: previously every
|
||||
`ViewModel.Factory` built its own `AndroidKeystoreTokenVault` and `BearerTokenProvider` from
|
||||
scratch, meaning a token refreshed by (say) the repository list would never be visible to the
|
||||
code browser's independent copy. `AuthSession` holds exactly one `TokenVault` and one
|
||||
`RefreshingTokenProvider` per process; every Factory now pulls from it instead. This also makes
|
||||
the provider's internal `Mutex` actually meaningful — it stops concurrent callers *across
|
||||
different features* from independently trying to consume the same single-use, rotating refresh
|
||||
token, which would make every refresh but the first one fail.
|
||||
- **`RepositoryListViewModel.restoreSession()`'s old "expiresAtEpochMillis already past -> clear
|
||||
and fail immediately" check was removed outright**, not adjusted, since it actively worked
|
||||
against refresh support: it fired *before* the (now refresh-capable) API call ever had a chance
|
||||
to run. The exact same fallback behavior it existed for — clearing the session and surfacing
|
||||
`AUTHENTICATION_EXPIRED` — still happens today, just now driven by a genuine failed API call
|
||||
(refresh attempted and failed, or no refresh token available) rather than a client-side clock
|
||||
guess made before ever trying.
|
||||
- **`RepositoryListViewModel`'s constructor gained a `tokenProvider` parameter defaulted to the old
|
||||
no-refresh behavior** (`BearerTokenProvider { tokenVault.read()?.accessToken.orEmpty() }`)
|
||||
specifically so no existing test needed to change — only production wiring (the `Factory`)
|
||||
explicitly passes the shared refresh-aware one.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] `AuthApi.refreshToken` sends `grant_type=refresh_token`, `client_id`, `refresh_token`, and no
|
||||
`client_secret`, to the same `/login/oauth/access_token` endpoint as the existing code
|
||||
exchange.
|
||||
- [x] A successful refresh response's `access_token`/`refresh_token`/`expires_in` are parsed and
|
||||
persisted; a response omitting `refresh_token` falls back to reusing the one just sent rather
|
||||
than losing refresh capability entirely.
|
||||
- [x] `RefreshingTokenProvider` returns the stored token unchanged when it isn't close to expiry,
|
||||
refreshes (and persists the result) once within 60 seconds of expiry, falls back to the stale
|
||||
token — without attempting a network call — when no refresh token is stored, and falls back
|
||||
to the stale token (leaving the vault untouched) when the refresh attempt itself fails.
|
||||
- [x] A token with no `expiresAtEpochMillis` at all is never treated as due for refresh.
|
||||
- [x] `RepositoryListViewModel.restoreSession()` authenticates through the injected token provider
|
||||
(not a stale value read once at the top of the function) — proven directly by a test using a
|
||||
deliberately-wrong stored token alongside a provider that returns a different one.
|
||||
- [x] Every existing test continues to pass unmodified except the two direct `AuthenticationGateway`
|
||||
fakes, which now also implement the new interface method.
|
||||
- [x] Build, unit tests, and lint pass.
|
||||
- [ ] Live device confirmation that a session now survives past what used to force a full
|
||||
re-login/re-approval — not yet observed over a long enough idle period this session; see
|
||||
Known limitations.
|
||||
|
||||
## Security and privacy
|
||||
|
||||
- No new data is stored beyond what `TokenSet` already held (`refreshToken` was already part of
|
||||
the persisted, Android-Keystore-encrypted `TokenSet`; this task is the first code to actually use
|
||||
the field, not the first to store it).
|
||||
- Refresh failures fail closed: any error refreshing falls back to the *existing* stale token
|
||||
(which then draws a normal 401 from Gitea, handled exactly as before) — never to a broadened
|
||||
scope, an unencrypted fallback, or a silently-extended session past what the server itself allows.
|
||||
- `client_secret` is deliberately never sent for this public/PKCE client's refresh calls, matching
|
||||
the existing `authorization_code` exchange and Gitea's own documented requirement that only
|
||||
confidential clients need one.
|
||||
|
||||
## Required checks
|
||||
|
||||
```text
|
||||
./gradlew assembleDebug
|
||||
./gradlew testDebugUnitTest
|
||||
./gradlew lintDebug
|
||||
```
|
||||
|
||||
## Evidence
|
||||
|
||||
- `data/auth/AuthApi.kt`: `AuthenticationGateway.refreshToken(refreshToken, nowEpochMillis)`;
|
||||
`AuthApi`'s implementation and `exchangeCode` now share one `parseTokenResponse(...)` helper
|
||||
(`fallbackRefreshToken` parameter distinguishes "nothing to fall back to" for a fresh code
|
||||
exchange from "reuse the token just sent" for a refresh).
|
||||
- `core/auth/android/RefreshingTokenProvider.kt` (new): the refresh-before-stale `BearerTokenProvider`
|
||||
implementation described above.
|
||||
- `core/auth/android/AuthSession.kt` (new): the shared singleton wiring.
|
||||
- `feature/repositories/RepositoryListViewModel.kt`: `tokenProvider` constructor param (defaulted);
|
||||
`restoreSession()`'s early-expiry short-circuit removed; `loadRepositories()` takes no arguments
|
||||
and reads `tokenProvider.accessToken()` instead of a captured `TokenSet.accessToken`; `Factory`
|
||||
wired to `AuthSession`.
|
||||
- `CodeBrowserViewModel.kt`, `HistoryViewModel.kt`, `WorkItemsViewModel.kt`,
|
||||
`NotificationsViewModel.kt`: each `Factory` now calls `AuthSession.tokenProvider(application)`
|
||||
instead of constructing its own `AndroidKeystoreTokenVault`/`BearerTokenProvider` pair.
|
||||
- New tests: `AuthApiTest.kt` (refresh sends the right grant/fields and no client secret, response
|
||||
without a `refresh_token` falls back to the one sent, blank refresh token rejected before any
|
||||
request); `RefreshingTokenProviderTest.kt` (not-yet-expired passthrough, refresh-and-persist
|
||||
within the skew window, proactive-before-actual-expiry, no-refresh-token fallback, failed-refresh
|
||||
fallback leaving the vault untouched, empty-string when nothing stored, no-expiry-never-refreshes);
|
||||
`RepositoryListViewModelTest.kt` (new test proving `restoreSession` authenticates through the
|
||||
injected provider rather than a raw stored token; `FakeAuthGateway` updated to implement the new
|
||||
interface method and capture the token it was called with).
|
||||
- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL (one intermediate
|
||||
failure — a second, missed `loadRepositories(tokens)` call site in `authenticate()` after the
|
||||
signature change — found and fixed before this).
|
||||
- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all 231+ unit tests passing (two intermediate
|
||||
failures were bugs in the new tests' own fixture data, not the implementation — fixed before
|
||||
this: one test's `expiresAtEpochMillis` was accidentally always within the skew window regardless
|
||||
of `now`, and one asserted the vault was `null` after a failed refresh when "untouched" was the
|
||||
actually-intended and correct behavior).
|
||||
- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors.
|
||||
- Live device: build installed over wireless `adb`.
|
||||
|
||||
## Known limitations
|
||||
|
||||
- **This task is proactive-refresh only.** There is no "catch a live 401, refresh, and retry the
|
||||
same request once" fallback at the HTTP-transport layer. In practice this should rarely matter —
|
||||
the provider refreshes 60 seconds ahead of the token's own stated expiry before any request that
|
||||
goes through it — but a token invalidated *server-side* for a reason other than ordinary expiry
|
||||
(e.g. an admin revoking the OAuth grant) would still surface as a live 401 exactly as before this
|
||||
task, correctly falling through to `AUTHENTICATION_EXPIRED` rather than being silently retried.
|
||||
- **Not yet confirmed against a real long-idle session on the device.** The fix targets a problem
|
||||
(repeated forced re-login/re-approval) that only manifests after enough real time passes for
|
||||
Gitea's access token to actually expire — this session's live testing so far has been continuous
|
||||
active use, not a multi-hour idle gap, so the *felt* improvement (no more surprise "승인" prompts)
|
||||
hasn't been directly observed yet, only proven correct at the unit level.
|
||||
- Gitea's server-side `expires_in` / refresh-token TTL/rotation settings are instance-configurable
|
||||
(`OAuth2.InvalidateRefreshTokens` and friends) and unknown for this specific Gitea instance;
|
||||
nothing in this implementation assumes a particular value, but the practical frequency of silent
|
||||
refreshes on this instance hasn't been observed yet either.
|
||||
|
||||
## Review findings
|
||||
|
||||
None yet — pending Codex/human review.
|
||||
Loading…
x
Reference in New Issue
Block a user