diff --git a/app/src/main/java/com/ubisam/ubgitea/core/auth/protocol/OAuthClientConfig.kt b/app/src/main/java/com/ubisam/ubgitea/core/auth/protocol/OAuthClientConfig.kt index c381f8d..fc0b312 100644 --- a/app/src/main/java/com/ubisam/ubgitea/core/auth/protocol/OAuthClientConfig.kt +++ b/app/src/main/java/com/ubisam/ubgitea/core/auth/protocol/OAuthClientConfig.kt @@ -13,7 +13,15 @@ data class OAuthClientConfig( require(clientId.isNotBlank()) { "OAuth client ID cannot be blank" } require(redirectUri.isSecureRedirect()) { "OAuth redirect URI must be HTTPS without query or fragment" } require(scopes.isNotEmpty()) { "At least one OAuth scope is required" } - require(scopes.all { it.isReadOnly }) { "Only read-only OAuth scopes are allowed" } + // Deliberately narrow, not "any write scope goes": WRITE_REPOSITORY is the one specific + // exception, carved out because Gitea's star/unstar endpoints require it (verified against + // Gitea's own route registration -- PUT/DELETE /user/starred/{owner}/{repo} is under + // tokenRequiresScopes(AccessTokenScopeCategoryRepository) with a Write access level) and + // the human product owner explicitly approved requesting it for that feature. Every other + // write scope (e.g. WRITE_USER) is still rejected here exactly as before. + require(scopes.all { it.isReadOnly || it == OAuthScope.WRITE_REPOSITORY }) { + "Only read-only OAuth scopes (plus the narrow write:repository exception for starring) are allowed" + } } val authorizationEndpoint: URI @@ -43,6 +51,9 @@ enum class OAuthScope(val wireValue: String, val isReadOnly: Boolean) { READ_ISSUE("read:issue", true), READ_NOTIFICATION("read:notification", true), WRITE_USER("write:user", false), + // Required for the favorite/star feature: Gitea's star/unstar endpoints need write access to + // the repository scope category, not just read (see the require() above for the source citation). + WRITE_REPOSITORY("write:repository", false), } object UBGiteaOAuthDefaults { @@ -56,6 +67,7 @@ object UBGiteaOAuthDefaults { OAuthScope.READ_ORGANIZATION, OAuthScope.READ_ISSUE, OAuthScope.READ_NOTIFICATION, + OAuthScope.WRITE_REPOSITORY, ), ) } diff --git a/app/src/test/java/com/ubisam/ubgitea/core/auth/protocol/OAuthClientConfigTest.kt b/app/src/test/java/com/ubisam/ubgitea/core/auth/protocol/OAuthClientConfigTest.kt index cdf2650..ec19b93 100644 --- a/app/src/test/java/com/ubisam/ubgitea/core/auth/protocol/OAuthClientConfigTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/core/auth/protocol/OAuthClientConfigTest.kt @@ -17,6 +17,18 @@ class OAuthClientConfigTest { config(setOf(OAuthScope.READ_USER, OAuthScope.READ_REPOSITORY)) } + @Test + fun acceptsWriteRepositoryAsTheOneNarrowExceptionForStarring() { + config(setOf(OAuthScope.READ_USER, OAuthScope.WRITE_REPOSITORY)) + } + + @Test + fun stillRejectsWriteUserEvenAlongsideTheAllowedWriteRepositoryException() { + assertThrows(IllegalArgumentException::class.java) { + config(setOf(OAuthScope.READ_USER, OAuthScope.WRITE_REPOSITORY, OAuthScope.WRITE_USER)) + } + } + @Test fun rejectsEmptyScopes() { assertThrows(IllegalArgumentException::class.java) { config(emptySet()) } diff --git a/docs/development-log.md b/docs/development-log.md index 6ef50e4..ded0f13 100644 --- a/docs/development-log.md +++ b/docs/development-log.md @@ -533,3 +533,16 @@ This is the append-only implementation and handoff record shared by Codex and Cl - Also this turn: while looking into a separately-reported bug (tapping a repository's new favorite star turns it red but doesn't actually favorite it), traced the root cause to a real architectural conflict rather than a simple bug — Gitea's `PUT`/`DELETE /user/starred/{owner}/{repo}` endpoints require `write:repository` OAuth scope (verified against Gitea's own route registration source), but this app's `OAuthClientConfig` has a hard `require(scopes.all { it.isReadOnly })` invariant and only ever requests read-only scopes. Every star/unstar attempt is therefore guaranteed to fail server-side (403), caught generically, and rendered as the reported red "failed" star. Not fixed yet — this needs the human product owner's direction, since the only real fix (requesting `write:repository`) breaks the just-built persistent-login promise from TASK-029 at least once (a scope change requires fresh consent) and changes this app's deliberate read-only security posture; flagged to them rather than decided unilaterally. - Next recommended task: get the human product owner's direction on the star/favorite OAuth-scope conflict (add write scope vs. make favorites a local-only bookmark vs. remove the feature), then continue per the standing direction — still pending from earlier in this session: TASK-030's green theme/footer-height/language-star batch, TASK-031's label/EUC-KR/timeout batch, pinch-zoom/pan, CSV, PDF, and Issues/PR numbered pagination, all awaiting live confirmation. - Next owner: Claude, continuing per the standing direction once the human product owner responds on the OAuth-scope question. + +## 2026-08-27 — TASK-033: request write:repository OAuth scope so starring actually works + +- Agent and role: Claude, direct continuation of the same live session — following up on the star/unstar-always-fails root cause traced during TASK-032. Presented the human product owner with three options (add write:repository scope / make favorites a local-only bookmark / remove the feature); they chose to add the scope. +- Commit/diff: continued on `claude/task-015-notifications`; `core/auth/protocol/OAuthClientConfig.kt` and `OAuthClientConfigTest.kt`, plus `tasks/033-write-repository-oauth-scope-for-starring.md`. +- Feature implemented: `write:repository` is now requested alongside this app's existing read-only OAuth scopes, added as one narrow, explicit exception to the `require(scopes.all { it.isReadOnly })` invariant rather than a blanket relaxation — every other write scope (including the pre-existing, still-unused `WRITE_USER` value) is still rejected, confirmed by a new test that pairs it with the now-allowed `write:repository` and checks it's still rejected. +- How it was implemented: verified directly against Gitea's own route registration source (`routers/api/v1/api.go`) that `PUT`/`DELETE /user/starred/{owner}/{repo}` require Write-level access within the repository scope category — `read:repository` alone can never satisfy it, so this wasn't fixable from the client side any other way. No change was needed to `toggleFavorite`, `FavoriteStar`, or any other favorite-feature code from TASK-018/TASK-030 — all of that was already correctly built against the `RepositoryFavorites` interface; the only thing wrong was the token it was being handed never had permission to actually star anything. +- Files changed: `core/auth/protocol/OAuthClientConfig.kt`, `OAuthClientConfigTest.kt`, `tasks/033-write-repository-oauth-scope-for-starring.md`. +- Checks and results: `./gradlew compileDebugKotlin compileDebugUnitTestKotlin` BUILD SUCCESSFUL; `./gradlew testDebugUnitTest` BUILD SUCCESSFUL, all unit tests passing (new coverage: `write:repository` alone is now accepted; `write:user` is still rejected even alongside the now-allowed `write:repository`; the pre-existing write-scope-rejection test is unaffected); `./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: a device already logged in under the old, read-only-only scope needs one explicit re-login before starring actually works — TASK-029's refresh-token flow keeps a session alive indefinitely for the scope it was originally granted, and a refresh doesn't expand that scope. This is expected and was reported directly to the human product owner rather than silently worked around (e.g. by force-clearing the stored session on their behalf), since that kind of forced logout should be their call. Live confirmation (star actually succeeding after a fresh login) is still pending. +- Next recommended task: ask the human product owner to log out and back in once, then confirm starring/unstarring actually works. Still pending from earlier in this session: TASK-030's green theme/footer-height/language-star batch, TASK-031's label/EUC-KR/timeout batch, TASK-032's swipe-direction fix, pinch-zoom/pan, CSV, PDF, and Issues/PR numbered pagination, all awaiting live confirmation. +- Next owner: Claude, continuing per the standing direction. diff --git a/tasks/033-write-repository-oauth-scope-for-starring.md b/tasks/033-write-repository-oauth-scope-for-starring.md new file mode 100644 index 0000000..4319b7f --- /dev/null +++ b/tasks/033-write-repository-oauth-scope-for-starring.md @@ -0,0 +1,103 @@ +# TASK-033: Request 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:repository` OAuth scope +(verified against Gitea's own route registration source) 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. This task adds +`write:repository` as one narrow, explicit exception to that read-only invariant. + +## Scope + +- Included: `core/auth/protocol/OAuthClientConfig.kt` (new `WRITE_REPOSITORY` scope value, the + read-only `require()` narrowed to allow specifically that one scope, `UBGiteaOAuthDefaults.config` + now requests it), matching test additions in `OAuthClientConfigTest.kt`. +- 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 change to any other OAuth scope (`WRITE_USER` remains + rejected, confirmed by a new test asserting it's still rejected even alongside the now-allowed + `WRITE_REPOSITORY`). + +## 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 one scope this app's own already-built favorite + feature actually needs, and only that one. `require(scopes.all { it.isReadOnly || it == OAuthScope.WRITE_REPOSITORY })` + still rejects every other write scope (including the pre-existing, still-unused `WRITE_USER` + value) — verified by a new test that pairs `WRITE_REPOSITORY` with `WRITE_USER` together and + confirms that combination is still rejected, so a future accidental addition of some other write + scope doesn't silently slip through just because one exception now exists. +- **Verified against Gitea's actual source, not assumed**: Gitea's route registration + (`routers/api/v1/api.go`) puts the whole `/starred/{owner}/{repo}` group under + `tokenRequiresScopes(auth_model.AccessTokenScopeCategoryRepository)`, with the PUT/DELETE methods + specifically requiring Write-level access within that category — `read:repository` alone is + insufficient no matter what, this isn't a bug that could have been fixed any other way from the + client side. +- **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:repository` permission) 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:repository` is requested alongside the app's existing read-only scopes. +- [x] Any other write scope (e.g. `write:user`) is still rejected by `OAuthClientConfig`'s + invariant, even when paired with the now-allowed `write:repository`. +- [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:repository", false)`; the + read-only `require()` narrowed with a source-cited comment; `UBGiteaOAuthDefaults.config`'s + scope set now includes it. +- `OAuthClientConfigTest.kt`: two new tests — `write:repository` alone (with a read scope) is now + accepted; `write:user` is still rejected even when `write:repository` is also present in the same + scope set. +- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL. +- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing (including the two new + ones above; the pre-existing `rejectsWriteScopeInTheProductionOAuthContract` test, which uses + `WRITE_USER` specifically, still passes unchanged). +- `./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); the device's current session still carries the old, read-only-only scope + until the human product owner logs out and back in. + +## Known limitations + +- Requires a manual one-time re-login on the device before starring actually works — see Design + notes. Not yet done in this session. +- 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.