feat(auth): add S256 PKCE to device authorization flow - #3
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
💤 Files with no reviewable changes (2)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe PR adds PKCE to device authentication, centralizes JWT session cleanup for unauthorized refreshes, preserves sessions during transient failures, and removes API key creation from the extension in favor of dashboard links. ChangesAuthentication and API Key Updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The PR updates device authentication to use PKCE, preserves sessions across transient refresh failures, and removes unsupported in-extension key creation. No actionable merge-blocking risk remains at the current head after normal checks and review. Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant AuthSection
participant LocalStorage
participant AuthServer
participant Background
AuthSection->>LocalStorage: Store PKCE verifier
AuthSection->>AuthServer: Open device-login URL with challenge
AuthServer-->>Background: Return device code
Background->>LocalStorage: Read verifier
Background->>AuthServer: Exchange device code with verifier
Background->>LocalStorage: Store tokens and clear verifier
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Device login now sends code_challenge + code_challenge_method=S256 and the token exchange sends the matching code_verifier, matching the current backend contract. Refresh now clears the session only on a 401 (revoked or expired grant) and keeps it on transient failures. Drops the connected-app API key creation flow, which is first-party only now, in favour of a dashboard link.
f999986 to
c759543
Compare
What changed
The "Sign in with Spoo" device flow had drifted from the current backend contract (live in spoo-me/spoo#260) and from how the CLI and Raycast clients do it. This brings it back in line.
Device login now sends PKCE
GET /auth/device/loginrequirescode_challenge(43-char base64url) andcode_challenge_method=S256. Before, the extension only sentapp_idandstate, so login would now fail with a 400. The flow now generates a 32-byte random verifier as unpadded base64url, derives the S256 challenge with Web Crypto, stores the verifier, and appends both challenge params to the login URL.Token exchange now sends the verifier
POST /auth/device/tokenexpects{ code, code_verifier }. Before, the extension sent only{ code }, which now returns a 422. The background exchange reads the stored verifier, sends it, and clears it on success.Refresh cleanup is now scoped to revocation
Refresh previously cleared the session on any failed response, so a transient server or network blip logged the user out. It now clears the session only on a 401 (revoked grant or expired refresh token) and keeps the tokens on transient failures so a later attempt can retry. This matches how the Raycast client removes tokens on a 401 during refresh.
Dropped connected-app API key creation
Key creation is first-party (dashboard) only now, so creating a key over a connected-app token returns a 403. The Account tab no longer offers in-extension key creation and links to the dashboard instead. Listing and revoking keys still work. This mirrors the CLI, which kept list and revoke and points to the dashboard for creation.
Not changed / no drift found
redirect_uriparam.Backend follow-up (not in this PR)
The
spoo-snapentry in the backend app registry (config/apps.yaml) needs to exist and be active with the scopes this client should get. Noredirect_uriallowlisting is needed because the extension does not use a redirect.Verification
tsc --noEmitcleanbiome check .clean (only pre-existing config-version infos)wxt buildandwxt build -b firefoxboth succeed^[A-Za-z0-9_-]{43}$)Summary by CodeRabbit
New Features
Changes