Ubisam-MobileGit/tasks/033-write-repository-oauth-scope-for-starring.md
choiminyeong f6ac20c8ed TASK-063: task records say what is actually open
No task waits on a Codex review; the redesign tasks are confirmed on the
product owner's Z Fold7; TASK-016's leftovers are on hold; no biometric
reauthentication.

Agent: main (opus)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-29 20:25:49 +09:00

145 lines
9.9 KiB
Markdown

# TASK-033: Request write:user + write:repository OAuth scope so starring actually works
Status: implemented; all required checks pass; live-installed, requires a one-time re-login on the device to take effect
Owner: Claude
Reviewer: Codex
## Outcome
Every attempt to star/unstar a repository was guaranteed to fail server-side, because Gitea's
`PUT`/`DELETE /user/starred/{owner}/{repo}` endpoints require write-level OAuth scope while this
app's `OAuthClientConfig` only ever requested read-only scopes, enforced by a hard `require()`
invariant. Live-reported as: the new favorite star turns red (this app's failed-action color) but
never actually favorites anything. The human product owner was given three options — add the write
scope, make favorites a local-only bookmark, or remove the feature — and explicitly chose to add
the scope. Which exact scope(s) that endpoint needs took two live-tested corrections to pin down
(see Known limitations) — the endpoint turned out to require **both** `write:user` and
`write:repository` together, not either one alone. This task adds both as narrow, explicit
exceptions to the read-only invariant.
## Scope
- Included: `core/auth/protocol/OAuthClientConfig.kt` (new `WRITE_REPOSITORY`/`WRITE_ORGANIZATION`
scope values alongside the pre-existing `WRITE_USER`, the read-only `require()` narrowed to allow
specifically `WRITE_USER` and `WRITE_REPOSITORY`, `UBGiteaOAuthDefaults.config` now requests
both), matching test additions in `OAuthClientConfigTest.kt`; `core/network/HttpTransport.kt`
(debug-build-only logging of the raw HTTP status + response body on any non-2xx response, added
mid-investigation — see Known limitations); `feature/repositories/RepositoryListViewModel.kt`
(debug-build-only logging of `toggleFavorite`'s failure class/message); `app/build.gradle.kts`
(`testOptions { unitTests { isReturnDefaultValues = true } }`, required once `android.util.Log`
calls landed on code paths existing unit tests exercise).
- Excluded: No change to `toggleFavorite`, `FavoriteStar`, or any other favorite/star UI or
ViewModel logic from TASK-018/TASK-030 — those were already correctly built against the
`RepositoryFavorites` interface; the only thing wrong was the token they were being handed never
had permission to actually do it. No other OAuth scope is allowed by the narrowed invariant
(confirmed by a test that still rejects an unrelated write scope even alongside the two now-allowed
ones).
## Design notes
- **A narrow exception, not a blanket relaxation**: the original `require(scopes.all { it.isReadOnly })`
invariant existed to keep this app's OAuth footprint deliberately minimal — this task doesn't
remove that principle, it carves out exactly the two scopes this app's own already-built favorite
feature actually needs, and only those two. `require(scopes.all { it.isReadOnly || it == OAuthScope.WRITE_USER || it == OAuthScope.WRITE_REPOSITORY })`
still rejects every other write scope — verified by a test that adds an unrelated write scope
(`WRITE_ORGANIZATION`, added purely for this test — the app doesn't request it) alongside the two
now-allowed ones and confirms that combination is still rejected, so a future accidental addition
of some other write scope doesn't silently slip through just because these two exceptions exist.
- **Confirmed live, not just from source reading**: an initial attempt (request `write:repository`
only) was based on a WebFetch-summarized reading of Gitea's route registration source, which
named the wrong scope category — the actual live 403 said `required=[write:user]`. Switching to
`write:user` only then got `required=[write:repository]` right back. Together, both responses
show the endpoint needs both at once — it's nested under `/user/`, which needs `write:user`, but
also runs `repoAssignment()` against the target repository, which separately needs
`write:repository`. This is why the debug-only response-body logging (see Scope) was added
mid-investigation: it's dramatically faster and more reliable than re-reading Gitea's Go route
source through a summarizing tool, which had already produced one incorrect answer.
- **A one-time re-login is required and expected, not a regression of TASK-029**: TASK-029's
proactive refresh-token flow keeps a session alive indefinitely *for the scope it was originally
granted* — a refresh token from before this change carries the old (read-only) scope forward
automatically, and Gitea's refresh flow doesn't expand it. The device's current session therefore
needs one explicit new login (which will show the OAuth consent screen once more, listing the new
write permissions) before starring will actually start working; every login after
that continues to benefit from TASK-029's no-re-consent behavior exactly as before. This is
reported directly to the human product owner rather than silently worked around (e.g. by
force-clearing the stored session), since forcing a logout is a disruptive, irreversible-feeling
action that should be their call, not an automatic side effect of this change.
## Acceptance criteria
- [x] `write:user` and `write:repository` are both requested alongside the app's existing
read-only scopes.
- [x] Any other write scope is still rejected by `OAuthClientConfig`'s invariant, even when paired
with the two now-allowed exceptions.
- [x] Build, unit tests, and lint pass.
- [ ] Live confirmation, after a fresh login on the device, that starring/unstarring a repository
now actually succeeds (star turns filled/primary-colored, not red; the repository shows up
in/disappears from the FAVORITES list) — requires the human product owner to log out and back
in first, not yet done in this session.
## Required checks
```text
./gradlew compileDebugKotlin compileDebugUnitTestKotlin
./gradlew testDebugUnitTest
./gradlew assembleDebug lintDebug
```
## Evidence
- `OAuthClientConfig.kt`: new `OAuthScope.WRITE_REPOSITORY`/`WRITE_ORGANIZATION` values alongside
the pre-existing `WRITE_USER`; the read-only `require()` narrowed to allow `WRITE_USER` and
`WRITE_REPOSITORY` specifically; `UBGiteaOAuthDefaults.config`'s scope set now includes both.
- `HttpTransport.kt`: `requireSuccess()` now logs the raw status + response body in debug builds
before throwing on any non-2xx response.
- `RepositoryListViewModel.kt`: `toggleFavorite`'s catch block now logs the failure class/message
in debug builds.
- `app/build.gradle.kts`: `testOptions { unitTests { isReturnDefaultValues = true } }` added so
those `android.util.Log` calls don't throw inside plain JVM unit tests.
- `OAuthClientConfigTest.kt`: `write:user` + `write:repository` together (with a read scope) are
now accepted; an unrelated write scope (`write:organization`) is still rejected even alongside
both now-allowed exceptions; the base "some write scope is rejected" test now uses
`write:organization` as its example instead of `write:user` (which is no longer rejected).
- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL.
- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing.
- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors.
- Installed on the human product owner's device over wireless `adb` (`adb install -r`, streamed
install succeeded) three times across this investigation, most recently with the final
`write:user` + `write:repository` scope set; the device's current session still carries scope
from an earlier attempt in this same investigation until the human product owner revokes and logs
in again.
## Known limitations
- Requires a manual one-time re-login on the device before starring actually works — see Design
notes.
- **The scope required for starring turned out to need correcting twice, both confirmed by this
instance's own live error responses, not by source reading alone.** First attempt requested only
`write:repository` (based on a WebFetch-summarized reading of Gitea's route registration source,
which turned out to name the wrong scope category) — the live 403 body said
`required=[write:user]`. Second attempt switched to only `write:user` — the live 403 body then
said `required=[write:repository]` right back. Conclusion, confirmed by both responses together:
`/user/starred/{owner}/{repo}` requires **both** scopes at once (it's nested under `/user/`,
which needs `write:user`, but also runs `repoAssignment()` against the target repository, which
needs `write:repository`). The final scope set requests both. **Lesson applied**: added
debug-build-only logging of the raw HTTP status + response body on any non-2xx response
(`HttpTransport.requireSuccess`), since Gitea's own error bodies name the exact missing scope —
far faster and more reliable than re-reading route source through a summarizing tool a third
time. This required adding `testOptions { unitTests { isReturnDefaultValues = true } }` to
`app/build.gradle.kts`, since plain JVM unit tests otherwise throw on any unmocked
`android.util.Log` call.
- **Each scope change requires revoking the existing grant on Gitea's web UI first** (Settings →
Applications → "Authorized OAuth2 Applications" → revoke UbisamPajuApp), or the next login fails
with "denied:server_error" — traced to Gitea 1.23.5's `GrantApplicationOAuth`:
`if grant.Scope != form.Scope { ...ErrorCode: ErrorCodeServerError, ErrorDescription: "a grant exists with different scope" }`.
This is Gitea's own intended behavior (it won't silently widen an existing grant's scope), not a
bug. The human product owner needs to revoke and re-login once more for this final scope set —
not yet confirmed live as of this commit.
- No change was made to retroactively reflect a repository's *already*-starred-on-the-server state
when the ALL list first loads (the pre-existing, unrelated `favoriteAction` limitation noted in
TASK-030 — still applies, unaffected by this task).
## Review findings
None yet — no Codex review pending (Codex retired, 2026-09-29).