Ubisam-MobileGit/tasks/029-oauth-token-refresh.md
choiminyeong 50d1b41f47 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>
2026-08-27 22:52:03 +09:00

172 lines
12 KiB
Markdown

# 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.