TASK-033: request write:repository OAuth scope so starring actually works
Gitea's PUT/DELETE /user/starred/{owner}/{repo} require write:repository
token scope (verified against Gitea's route registration source), but
this app's OAuthClientConfig only ever requested read-only scopes,
enforced by a hard require() -- every star/unstar attempt was guaranteed
to fail server-side (403), surfaced as the reported red 'failed' star.
Adds write:repository as one narrow, explicit exception to the read-only
invariant (every other write scope, including the pre-existing unused
write:user, is still rejected). Requires one manual re-login on already
logged-in devices before it takes effect, since a refresh token carries
forward the scope it was originally granted under.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
f3adf007d9
commit
07436d4c83
@ -13,7 +13,15 @@ data class OAuthClientConfig(
|
|||||||
require(clientId.isNotBlank()) { "OAuth client ID cannot be blank" }
|
require(clientId.isNotBlank()) { "OAuth client ID cannot be blank" }
|
||||||
require(redirectUri.isSecureRedirect()) { "OAuth redirect URI must be HTTPS without query or fragment" }
|
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.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
|
val authorizationEndpoint: URI
|
||||||
@ -43,6 +51,9 @@ enum class OAuthScope(val wireValue: String, val isReadOnly: Boolean) {
|
|||||||
READ_ISSUE("read:issue", true),
|
READ_ISSUE("read:issue", true),
|
||||||
READ_NOTIFICATION("read:notification", true),
|
READ_NOTIFICATION("read:notification", true),
|
||||||
WRITE_USER("write:user", false),
|
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 {
|
object UBGiteaOAuthDefaults {
|
||||||
@ -56,6 +67,7 @@ object UBGiteaOAuthDefaults {
|
|||||||
OAuthScope.READ_ORGANIZATION,
|
OAuthScope.READ_ORGANIZATION,
|
||||||
OAuthScope.READ_ISSUE,
|
OAuthScope.READ_ISSUE,
|
||||||
OAuthScope.READ_NOTIFICATION,
|
OAuthScope.READ_NOTIFICATION,
|
||||||
|
OAuthScope.WRITE_REPOSITORY,
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@ -17,6 +17,18 @@ class OAuthClientConfigTest {
|
|||||||
config(setOf(OAuthScope.READ_USER, OAuthScope.READ_REPOSITORY))
|
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
|
@Test
|
||||||
fun rejectsEmptyScopes() {
|
fun rejectsEmptyScopes() {
|
||||||
assertThrows(IllegalArgumentException::class.java) { config(emptySet()) }
|
assertThrows(IllegalArgumentException::class.java) { config(emptySet()) }
|
||||||
|
|||||||
@ -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.
|
- 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 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.
|
- 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.
|
||||||
|
|||||||
103
tasks/033-write-repository-oauth-scope-for-starring.md
Normal file
103
tasks/033-write-repository-oauth-scope-for-starring.md
Normal file
@ -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.
|
||||||
Loading…
x
Reference in New Issue
Block a user