diff --git a/AGENTS.md b/AGENTS.md index f9095c832..2935e010f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -91,7 +91,7 @@ Type-safe navigation using `@Serializable` sealed interface `GithubStoreGraph` i ### Key Cross-Cutting Concerns - **Auth flow:** GitHub device-flow OAuth. Primary path goes through backend proxy (`/v1/auth/device/start`, `/v1/auth/device/poll`); falls back to direct GitHub only on infrastructure errors (5xx, timeouts). HTTP 4xx and GitHub's negative 200-bodies never trigger fallback. Backend rate limits (10 starts/hr, 200 polls/hr per IP) are hard — do not add retry loops. -- **`X-GitHub-Token` header:** Only sent on `/v1/search` and `/v1/search/explore`. Never on other endpoints, never logged. +- **`X-GitHub-Token` header:** Forwarded on every backend passthrough route — `/v1/search`, `/v1/search/explore`, `/v1/repo/{owner}/{name}`, `/v1/releases/{owner}/{name}`, `/v1/readme/{owner}/{name}`, `/v1/user/{username}`. Backend re-sends as `Authorization: token $token` so upstream GitHub calls run under the user's 5000/hr OAuth quota; without it the request falls back to the shared 60/hr anonymous bucket and a single 4xx can poison the backend's 15-min negative cache for everyone. DB-only routes (`/v1/categories`, `/v1/topics`, `/v1/events`, `/v1/auth/device/*`, `/v1/badge/*`) never get the header. Sourced via `BackendApiClient.currentUserGithubToken()` (`private`), never logged. 401 from passthrough routes ≠ session expired — `AuthenticationStateImpl` debounces consecutive 401s under the same token before clearing the session. - **Platform branching:** Source sets are `commonMain` (shared), `androidMain` (Android), `jvmMain` (Desktop). Some features (apps, installation, Shizuku) are Android-only. - **Shizuku (Android):** Optional silent install via AIDL service. Falls back to standard installer on failure. diff --git a/CLAUDE.md b/CLAUDE.md index f42e3dc54..6d1a2aac3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -159,7 +159,7 @@ Custom Gradle plugins in `build-logic/convention/` standardize module setup: - **Gradle properties:** Config cache enabled, build cache enabled, 4GB Gradle heap, 3GB Kotlin daemon heap - **Code style:** Official Kotlin style (`kotlin.code.style=official`) - **Desktop logs:** `CrashReporter` (installed as the first line of `DesktopApp.main`) tees `System.out`/`System.err` to a rotating `session.log` and writes `crash-.log` on uncaught exceptions. Paths: `~/Library/Logs/GitHub-Store/` (macOS), `%LOCALAPPDATA%/GitHub-Store/logs/` (Windows), `$XDG_STATE_HOME/GitHub-Store/logs/` (Linux). Android uses Logcat — no CrashReporter. -- **`X-GitHub-Token` header:** Forwarded to the backend *only* on `/v1/search` and `/v1/search/explore` (so the backend's live GitHub passthrough runs under the user's own 5000/hr quota). Sourced from `TokenStore.currentToken()` via `BackendApiClient.currentUserGithubToken()` — the helper is `private` so other endpoints can't leak it accidentally. Never sent on other endpoints (`/v1/categories`, `/v1/topics`, `/v1/repo`, `/v1/events`), never logged (no Ktor `Logging` plugin installed). +- **`X-GitHub-Token` header:** **Client-side**, `BackendApiClient.getRepo` / `getReleases` / `getReadme` / `getUser` always attach the header when a token exists in `TokenStore.currentToken()` (sourced through the `private` helper `currentUserGithubToken()`); they don't gate it on what the backend will do with it. **Backend-side**, the same header is consumed on every passthrough route — `/v1/search`, `/v1/search/explore`, `/v1/repo/{owner}/{name}` (only when the lazy-fetch DB-miss path actually hits GitHub upstream — cached hits don't need it), `/v1/releases/{owner}/{name}`, `/v1/readme/{owner}/{name}`, `/v1/user/{username}` — and re-sent as `Authorization: token $token` to `api.github.com`. The upstream call then runs under the user's own 5000/hr OAuth quota; the per-token bucket is per-user (not per-POP), so a logged-in user always gets their own headroom. Without the header the request falls back to the 60/hr-per-IP anonymous bucket, and a popular repo's releases endpoint can poison the backend's 15-min negative cache (`negativeTtlSeconds = 900`, key `releases:{owner}/{name}?page=…&per_page=…` is shared across users) on a single quota burst. DB-only routes never read the header: `/v1/categories`, `/v1/topics`, `/v1/events`, `/v1/auth/device/start`, `/v1/auth/device/poll`, `/v1/badge/...` — and the client doesn't send it on those either (the per-route `httpClient.get` block decides). Never logged (no Ktor `Logging` plugin installed). **Status-code semantics** on passthrough routes: backend remaps GitHub-upstream 401 (rejected token) into a `502` so the client never sees a "session expired" 401 on these endpoints; `502` therefore means *either* "GitHub unreachable" *or* "upstream rejected our auth" and is handled the same way — fall back to direct GitHub via `shouldFallbackToGithubOrRethrow`. `429` from these routes means the backend exhausted both the user's token bucket and its own pool retries; the client must **not** fall back to direct GitHub on `429` (same wall, same token), only back off and retry later — `shouldFallbackToGithubOrRethrow` already returns `false` for the entire 4xx range. The client's `UnauthorizedInterceptor` only installs on `createGitHubHttpClient` (direct GitHub calls), and `AuthenticationStateImpl` additionally debounces consecutive 401s under the same token (token snapshot threaded through from the request, reset on any non-401 response) so a single transient direct-GitHub 401 can't sign the user out. - **Device-flow auth proxy:** `feature/auth` calls `/v1/auth/device/start` and `/v1/auth/device/poll` on the backend as the primary path so users on networks that throttle `github.com` (China, corporate filters) can still complete login. Each session picks one `AuthPath` (`Backend` or `Direct`) at start and sticks to it; `AuthenticationRepositoryImpl` only escalates `Backend → Direct` on infrastructure errors (`HttpRequestTimeoutException`, `SocketTimeoutException`, `ConnectTimeoutException`, `BackendHttpException` with 5xx). HTTP 4xx and GitHub's valid-but-negative 200-bodies (`authorization_pending`, `slow_down`, `access_denied`, `expired_token`, `bad_verification_code`) are real answers, never cause fallback. `AuthenticationViewModel` persists `auth_path` in `SavedStateHandle` so activity recreation resumes on the same path. The Direct path still requires `BuildKonfig.GITHUB_CLIENT_ID` — both paths use the same OAuth App, so client-side `GITHUB_CLIENT_ID` must match the backend's `GITHUB_OAUTH_CLIENT_ID`. Shared backend constants live in `core/data/network/BackendEndpoints.kt` (`BACKEND_ORIGIN`, `BACKEND_BASE_URL`). Backend responses carry `X-Request-ID` — `GitHubAuthApi` embeds it in every error message via `asRequestIdTag()` so bug reports can cite the ID and it maps straight to backend logs. Backend rate limits (10 starts/hr, 200 polls/hr per source IP) are hard — do not add retry loops on top of Ktor's existing `HttpRequestRetry(maxRetries = 2)`. ## Coding Conventions diff --git a/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/GitHubClientProvider.kt b/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/GitHubClientProvider.kt index 89b8b419e..c42c34937 100644 --- a/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/GitHubClientProvider.kt +++ b/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/GitHubClientProvider.kt @@ -32,7 +32,6 @@ class GitHubClientProvider( tokenStore = tokenStore, rateLimitRepository = rateLimitRepository, authenticationState = authenticationState, - scope = scope, proxyConfig = proxyConfigFlow.value, ) @@ -50,7 +49,6 @@ class GitHubClientProvider( tokenStore = tokenStore, rateLimitRepository = rateLimitRepository, authenticationState = authenticationState, - scope = scope, proxyConfig = proxyConfig, ) val previous = currentClient diff --git a/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.kt b/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.kt index edf52263a..afcae952d 100644 --- a/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.kt +++ b/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.kt @@ -10,7 +10,6 @@ import io.ktor.client.statement.HttpResponse import io.ktor.http.* import io.ktor.serialization.kotlinx.json.* import io.ktor.util.network.UnresolvedAddressException -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.serialization.json.Json import zed.rainxch.core.data.data_source.TokenStore @@ -29,7 +28,6 @@ fun createGitHubHttpClient( tokenStore: TokenStore, rateLimitRepository: RateLimitRepository, authenticationState: AuthenticationState? = null, - scope: CoroutineScope? = null, proxyConfig: ProxyConfig = ProxyConfig.System, ): HttpClient { val json = @@ -43,10 +41,9 @@ fun createGitHubHttpClient( this.rateLimitRepository = rateLimitRepository } - if (authenticationState != null && scope != null) { + if (authenticationState != null) { install(UnauthorizedInterceptor) { this.authenticationState = authenticationState - this.scope = scope } } diff --git a/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/interceptor/UnauthorizedInterceptor.kt b/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/interceptor/UnauthorizedInterceptor.kt index 80de543a6..3d371c5d5 100644 --- a/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/interceptor/UnauthorizedInterceptor.kt +++ b/core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/interceptor/UnauthorizedInterceptor.kt @@ -3,18 +3,15 @@ package zed.rainxch.core.data.network.interceptor import io.ktor.client.HttpClient import io.ktor.client.plugins.HttpClientPlugin import io.ktor.client.statement.HttpReceivePipeline +import io.ktor.http.HttpHeaders import io.ktor.util.AttributeKey -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.launch import zed.rainxch.core.domain.repository.AuthenticationState class UnauthorizedInterceptor( private val authenticationState: AuthenticationState, - private val scope: CoroutineScope, ) { class Config { var authenticationState: AuthenticationState? = null - var scope: CoroutineScope? = null } companion object Plugin : HttpClientPlugin { @@ -28,10 +25,6 @@ class UnauthorizedInterceptor( requireNotNull(config.authenticationState) { "AuthenticationState must be provided" }, - scope = - requireNotNull(config.scope) { - "CoroutineScope must be provided" - }, ) } @@ -40,13 +33,27 @@ class UnauthorizedInterceptor( scope: HttpClient, ) { scope.receivePipeline.intercept(HttpReceivePipeline.After) { + val tokenKey = extractBearerToken(subject.call.request.headers[HttpHeaders.Authorization]) if (subject.status.value == 401) { - plugin.scope.launch { - plugin.authenticationState.notifySessionExpired() - } + plugin.authenticationState.notifySessionExpired(tokenKey) + } else { + plugin.authenticationState.notifyRequestSucceeded(tokenKey) } proceedWith(subject) } } + + private fun extractBearerToken(headerValue: String?): String? { + if (headerValue.isNullOrEmpty()) return null + val trimmed = headerValue.trim() + val withoutScheme = when { + trimmed.startsWith("Bearer ", ignoreCase = true) -> + trimmed.substring("Bearer ".length) + trimmed.startsWith("token ", ignoreCase = true) -> + trimmed.substring("token ".length) + else -> trimmed + } + return withoutScheme.trim().takeIf { it.isNotEmpty() } + } } } diff --git a/core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AuthenticationStateImpl.kt b/core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AuthenticationStateImpl.kt index f6a29a820..d514633a6 100644 --- a/core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AuthenticationStateImpl.kt +++ b/core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/AuthenticationStateImpl.kt @@ -1,5 +1,6 @@ package zed.rainxch.core.data.repository +import co.touchlab.kermit.Logger import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharedFlow @@ -9,7 +10,10 @@ import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import zed.rainxch.core.data.data_source.TokenStore import zed.rainxch.core.domain.repository.AuthenticationState +import kotlin.time.Clock +import kotlin.time.ExperimentalTime +@OptIn(ExperimentalTime::class) class AuthenticationStateImpl( private val tokenStore: TokenStore, ) : AuthenticationState { @@ -18,20 +22,77 @@ class AuthenticationStateImpl( private val sessionExpiredMutex = Mutex() + private var _failingTokenSnapshot: String? = null + private var _firstFailureAtMillis: Long = 0L + private var _consecutiveFailures: Int = 0 + override fun isUserLoggedIn(): Flow = tokenStore .tokenFlow() - .map { - it != null - } + .map { it != null } override suspend fun isCurrentlyUserLoggedIn(): Boolean = tokenStore.currentToken() != null - override suspend fun notifySessionExpired() { + override suspend fun notifySessionExpired(tokenKey: String?) { + if (tokenKey.isNullOrEmpty()) return sessionExpiredMutex.withLock { - if (tokenStore.currentToken() == null) return@withLock + val now = Clock.System.now().toEpochMilliseconds() + if (tokenKey != _failingTokenSnapshot || + now - _firstFailureAtMillis > FAILURE_WINDOW_MS + ) { + _failingTokenSnapshot = tokenKey + _firstFailureAtMillis = now + _consecutiveFailures = 1 + } else { + _consecutiveFailures += 1 + } + + if (_consecutiveFailures < REQUIRED_CONSECUTIVE_FAILURES) { + Logger.w(TAG) { + "notifySessionExpired: 401 count=$_consecutiveFailures (need " + + "$REQUIRED_CONSECUTIVE_FAILURES); deferring sign-out" + } + return@withLock + } + + val current = tokenStore.currentToken()?.accessToken + if (current != tokenKey) { + Logger.w(TAG) { + "notifySessionExpired: stored token rotated since the failing " + + "request; skipping clear" + } + resetCounter() + return@withLock + } + + Logger.w(TAG) { + "notifySessionExpired: $_consecutiveFailures consecutive 401s within " + + "window; clearing token" + } tokenStore.clear() + resetCounter() _sessionExpiredEvent.emit(Unit) } } + + override suspend fun notifyRequestSucceeded(tokenKey: String?) { + if (tokenKey.isNullOrEmpty()) return + sessionExpiredMutex.withLock { + if (tokenKey == _failingTokenSnapshot) { + resetCounter() + } + } + } + + private fun resetCounter() { + _failingTokenSnapshot = null + _firstFailureAtMillis = 0L + _consecutiveFailures = 0 + } + + private companion object { + const val TAG = "AuthState" + const val REQUIRED_CONSECUTIVE_FAILURES = 2 + const val FAILURE_WINDOW_MS = 60_000L + } } diff --git a/core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/AuthenticationState.kt b/core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/AuthenticationState.kt index f5dcf1192..8e2803b84 100644 --- a/core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/AuthenticationState.kt +++ b/core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/AuthenticationState.kt @@ -10,5 +10,7 @@ interface AuthenticationState { val sessionExpiredEvent: SharedFlow - suspend fun notifySessionExpired() + suspend fun notifySessionExpired(tokenKey: String?) + + suspend fun notifyRequestSucceeded(tokenKey: String?) } diff --git a/core/presentation/src/commonMain/composeResources/values/strings.xml b/core/presentation/src/commonMain/composeResources/values/strings.xml index 2e5b2ff8a..78a67a32a 100644 --- a/core/presentation/src/commonMain/composeResources/values/strings.xml +++ b/core/presentation/src/commonMain/composeResources/values/strings.xml @@ -350,6 +350,7 @@ You've used all %1$d free API requests. Resets in %1$d minutes 💡 Sign in to get 5,000 requests per hour instead of 60! + Releases are temporarily unavailable. Please try again in a bit. Sign In OK Close diff --git a/feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt b/feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt index 064189cce..25a9f4c7c 100644 --- a/feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt +++ b/feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt @@ -81,6 +81,7 @@ import zed.rainxch.githubstore.core.presentation.res.failed_to_open_app import zed.rainxch.githubstore.core.presentation.res.failed_to_share_link import zed.rainxch.githubstore.core.presentation.res.failed_to_uninstall import zed.rainxch.githubstore.core.presentation.res.installer_saved_downloads +import zed.rainxch.githubstore.core.presentation.res.releases_unavailable_temporarily import zed.rainxch.githubstore.core.presentation.res.link_copied_to_clipboard import zed.rainxch.githubstore.core.presentation.res.rate_limit_exceeded import zed.rainxch.githubstore.core.presentation.res.removed_from_favourites @@ -884,7 +885,20 @@ class DetailsViewModel( it.copy(isRetryingReleases = false, releasesLoadFailed = true) } } catch (t: Throwable) { + // The detailed cause ("HTTP 403", network error, parse + // failure) only matters for telemetry — for the user, + // "the release list isn't available right now, try + // again in a bit" is the entire signal. Surface the + // friendly message via snackbar; keep the raw cause in + // logs so support / bug reports can still trace it. logger.warn("Retry failed to load releases: ${t.message}") + viewModelScope.launch { + _events.send( + DetailsEvent.OnMessage( + getString(Res.string.releases_unavailable_temporarily), + ), + ) + } _state.update { it.copy(isRetryingReleases = false, releasesLoadFailed = true) }