Skip to content

feat(auth): PKCE (S256) for the device login flow - #1

Merged
Zingzy merged 2 commits into
mainfrom
feat/pkce
Jul 18, 2026
Merged

feat(auth): PKCE (S256) for the device login flow#1
Zingzy merged 2 commits into
mainfrom
feat/pkce

Conversation

@Zingzy

@Zingzy Zingzy commented Jul 18, 2026

Copy link
Copy Markdown
Member

spoo.me device auth now requires PKCE with the S256 method (companion change: spoo-me/spoo#260). This updates the CLI to match:

  • spoo auth login generates a fresh 43-character code verifier per attempt and sends its S256 challenge on the /auth/device/login URL along with code_challenge_method=S256.
  • The token exchange posts {"code", "code_verifier"} to /auth/device/token.
  • Tests cover the new URL params, the request body, and the RFC 7636 Appendix B verifier/challenge vector.

Once the backend change is deployed, older CLI versions will fail new browser logins. Existing sessions and API key logins are unaffected.

Also: removed spoo keys create

Key creation on spoo.me is now an interactive-session action (see spoo-me/spoo#260), so an app token from device login can no longer create API keys. A keys create subcommand would fail every time, so this drops it rather than ship a broken affordance. spoo keys list and spoo keys revoke are unchanged, and the empty-state hint now points to the dashboard. Also removes the now-dead CreateKey client method and the --scopes completion.

The backend now requires code_challenge on /auth/device/login and
code_verifier on /auth/device/token. Generate a 43-char base64url
verifier per login and thread it through the token exchange.
Copilot AI review requested due to automatic review settings July 18, 2026 00:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@Zingzy, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 47 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c5d5472a-f45a-4adb-b298-ba5bf46f3cdd

📥 Commits

Reviewing files that changed from the base of the PR and between d436ffe and f996851.

📒 Files selected for processing (11)
  • internal/api/auth.go
  • internal/api/auth_test.go
  • internal/api/keys.go
  • internal/api/keys_test.go
  • internal/auth/device.go
  • internal/auth/device_test.go
  • internal/cmd/auth.go
  • internal/cmd/completion.go
  • internal/cmd/completion_test.go
  • internal/cmd/keys.go
  • internal/cmd/keys_test.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/pkce

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Key creation now requires an interactive session on spoo.me, so an app
token cannot mint keys. Rather than ship a subcommand that always fails,
drop it: 'keys list' and 'keys revoke' stay, and the empty-state hint
points to the dashboard. Removes the dead CreateKey client method and
--scopes completion.

@Zingzy Zingzy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mergeable, nothing blocking. Clean implementation of the spoo#260 PKCE contract — I verified the crypto and the removal against the backend rather than the description.

  • PKCE is RFC-correct: codeVerifier is 32 random bytes as unpadded base64url = exactly 43 chars (RFC 7636 §4.1), within the backend DTO's 43-128 + charset bound; codeChallengeS256 is unpadded base64url(SHA256), and the login URL carries code_challenge_method=S256 — matching the backend's S256-only gate. The Appendix B vector test (TestCodeChallengeS256Vector) pins the exact vector spoo#260 uses, and TestDeviceFlowReturnsCode asserts the challenge on the auth URL equals S256(verifier), so the two legs can't drift. The verifier threads Run → ExchangeDeviceCode and posts {code, code_verifier}.
  • keys create removal is thorough, not partial: the subcommand, the dead CreateKey client method + CreateKeyRequest, the --scopes completion, and their tests are all gone, and the empty-state hint points at the dashboard. TestKeysCreateCommandRemoved asserts the subcommand no longer resolves. This is the right call — the backend made key minting first-party only, so a keys create over device auth would 403 every time; a never-succeeding subcommand is worse than its absence. keys list/keys revoke are untouched (they ride keys:manage, which the backend kept as list+delete).
  • go test ./... green in the worktree.

Deploy order: backend (spoo#260) must be live first — older CLI builds fail NEW logins only once it deploys, as the PR body says. Good to merge after the backend.

@Zingzy
Zingzy merged commit 697203b into main Jul 18, 2026
2 checks passed
@Zingzy
Zingzy deleted the feat/pkce branch August 16, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants