Two live-confirmed 403 responses (not source reading, which had already
given one wrong answer) showed /user/starred/{owner}/{repo} requires
BOTH scopes at once: requesting write:repository alone got
required=[write:user]; requesting write:user alone then got
required=[write:repository]. Both are now requested.
Added debug-build-only HTTP status/body logging (HttpTransport
.requireSuccess) and toggleFavorite failure logging, which is how this
was actually diagnosed -- ground truth from the server beats re-reading
Go route source through a summarizing tool. Required testOptions {
unitTests { isReturnDefaultValues = true } } in build.gradle.kts so
those Log calls don't throw inside plain JVM unit tests.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
145 lines
9.9 KiB
Markdown
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 — pending Codex/human review.
|