feat/revoke token#1434
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds OAuth token revocation: a revoke path constant and endpoint resolution, a RevokeToken HTTP implementation with error parsing and tests, and integration into authLogoutRun with tests for success, fallback, and revoke failures. ChangesOAuth Token Revocation in Logout
sequenceDiagram
participant User
participant authLogoutRun
participant RevokeToken
participant HTTPClient
participant OAuthServer
User->>authLogoutRun: logout()
authLogoutRun->>authLogoutRun: init HTTP client, resolve app secret
loop per user
alt refresh token present
authLogoutRun->>RevokeToken: revoke(refresh_token, hint=refresh_token)
else access token present
authLogoutRun->>RevokeToken: revoke(access_token, hint=access_token)
else
authLogoutRun->>authLogoutRun: skip revoke
end
RevokeToken->>HTTPClient: POST /oauth/v1/revoke (form)
HTTPClient->>OAuthServer: send request
OAuthServer-->>HTTPClient: status + body
authLogoutRun->>LocalStore: RemoveStoredToken
end
authLogoutRun-->>User: "Logged out"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/auth/revoke.go`:
- Around line 29-48: Wrap all plain error returns in the revoke path with the
repository's typed errors and preserve causes: for transport failures from
http.NewRequest and httpClient.Do return
errs.NewNetworkError(errs.SubtypeNetworkTransport, "token revoke transport
error").WithCause(err); for io.ReadAll and other non-transport failures return
an appropriate typed internal error (e.g. errs.NewInternalError("token revoke
read error")).WithCause(err); and for HTTP error responses (after
logHTTPResponse and formatOAuthErrorBody) return a typed API/validation/internal
error instead of fmt.Errorf while preserving the original body/error as the
cause; update the returns around http.NewRequest, httpClient.Do, io.ReadAll and
the resp.StatusCode >= 400 branch accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7978a337-6774-4938-9c26-62abbae91b9e
📒 Files selected for processing (7)
cmd/auth/logout.gocmd/auth/logout_test.gointernal/auth/device_flow.gointernal/auth/device_flow_test.gointernal/auth/paths.gointernal/auth/revoke.gointernal/auth/revoke_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@019582b8efb4ae8ea3b778594c29f786ff286755🧩 Skill updatenpx skills add larksuite/cli#feat/revoke_token -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1434 +/- ##
==========================================
- Coverage 72.87% 72.86% -0.01%
==========================================
Files 732 738 +6
Lines 69181 69535 +354
==========================================
+ Hits 50413 50666 +253
- Misses 14986 15055 +69
- Partials 3782 3814 +32 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/auth/revoke_test.go`:
- Around line 130-139: The test currently only checks ProblemOf category/code
and message substrings; update the failure-case assertions to also verify typed
metadata and cause preservation by asserting p.Subtype == errs.SubtypeUnknown
(use errs.SubtypeUnknown) and that the original error is wrapped (e.g.,
errors.Unwrap(err) != nil or using errors.Is/As to inspect the cause). Add the
same Subtype and wrapped-cause assertions to the other revoke tests mentioned
(the other error-path blocks that call errs.ProblemOf) so the typed-error
contract is fully asserted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 02fd9487-13d3-47a2-b0aa-c0869ad867bd
📒 Files selected for processing (2)
internal/auth/revoke.gointernal/auth/revoke_test.go
Summary
Improve the
auth logoutflow by revoking stored OAuth tokens before clearing local auth state, while keeping logout output clean even when token revocation fails. This makes logout behavior more complete without surfacing non-blocking revoke warnings to users.Changes
auth logoutbefore removing local stored tokens and clearing logged-in users from configTest Plan
lark-cli auth logoutflow works as expectedRelated Issues
Summary by CodeRabbit
New Features
Tests