Skip to content

Send /oauth/revoke as form-encoded per RFC 7009#28

Merged
ahorovit merged 3 commits into
mainfrom
adin/fix-oauth-revoke
Jun 9, 2026
Merged

Send /oauth/revoke as form-encoded per RFC 7009#28
ahorovit merged 3 commits into
mainfrom
adin/fix-oauth-revoke

Conversation

@ahorovit

@ahorovit ahorovit commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Rewrite ConvertKit::OAuth#revoke_token to POST application/x-www-form-urlencoded instead of application/json. Same host as the rest of the OAuth flow (app.convertkit.com), same method signature for callers — only the Content-Type and a few RFC-7009-shaped semantics around it change. The previous JSON-bodied implementation had been silently no-op'ing every revoke for over a year.

Problem

Kit's /oauth/revoke is a standard RFC 7009 endpoint. Two clauses in the spec combine to make the previous code path silently broken:

  • §2.1 — Request format. Clients MUST POST application/x-www-form-urlencoded. The gem's Connection class sends Content-Type: application/json and a JSON body. Kit's parser cannot read what we send.
  • §2.2 — Server response. When the server cannot identify the supplied token — because the request was unparseable, the token doesn't exist, or the client submitted nothing at all — it MUST still return HTTP 200. The spec is explicit: "The authorization server responds with HTTP status code 200 if the token has been revoked successfully or if the client submitted an invalid token."

So every JSON-bodied revoke from the gem got HTTP 200 back from Kit's edge with no token actually invalidated. revoke_token returned response.success? (which was true), Speckel's caller logged Kit /oauth/revoke succeeded, and Kit's database stayed untouched.

Kit engineer's confirmation:

If the forked gem is posting JSON (or otherwise not form-encoded), that's the bug: we can't parse the token, so per RFC we return 200 without resolving anything, which is why the disconnect looks successful on your side but never lands on ours.

Kit API doc: https://developers.kit.com/api-reference/oauth-token-revocation

Production Evidence

OpenSearch logs, 2026-06-08 23:04–23:07 UTC, network 12165494 — four full disconnect cycles. Each cycle emitted the gem's own success log lines:

ConvertKitService.revoke_token_for_provider: calling Kit /oauth/revoke for kit
ConvertKitService.revoke_token_for_provider: Kit /oauth/revoke succeeded for kit

Kit's engineer confirmed they observed zero parseable revoke requests across the same window. The tokens remained valid server-side.

The post-merge state for the same scenario: identical pre-call log, identical success log when Kit returns 200, but Kit's records reflect actual revocation. Failure modes (non-2xx from Kit) flip from invisible to a warn-level log entry with the HTTP status and body attached.

Solution

The revoke call goes through Ruby's stdlib Net::HTTP.post_form directly. That single method enforces both the URL-encoded body and the Content-Type: application/x-www-form-urlencoded header — there's no way for a future maintainer to silently regress to JSON without seeing both pieces change. token_type_hint: 'access_token' is included in the body per RFC 7009 §2.1's recommendation.

Wire-level result:

POST /oauth/revoke HTTP/1.1
Host: app.convertkit.com
Content-Type: application/x-www-form-urlencoded
Content-Length: <n>

token=<access_token>&client_id=<id>&client_secret=<secret>&token_type_hint=access_token

Error semantics tighten. Non-2xx now raises ConvertKit::OauthError with "Kit /oauth/revoke returned HTTP <code>: <body>". The only caller in production — Speckel's ConvertKitService#revoke_token_for_provider — already wraps in rescue StandardError and emits a warn-level structured log. Its behavior on a real failure changes from silently logging success to logging a warn with HTTP status and body. The success path is unchanged: revoke_token returns true and the caller logs succeeded as before.

get_token and refresh_token are untouched. They still use the gem's Connection class against app.convertkit.com/oauth/token — the connect flow works there today, and this PR doesn't disturb it.

Design Decisions

Decision Context
Stay on app.convertkit.com/oauth/revoke rather than move to the canonical api.kit.com/v4/oauth/revoke Kit's docs put all OAuth endpoints at api.kit.com/v4/oauth/*, but the gem and Speckel currently use app.convertkit.com for authorize and token. Moving revoke alone would split a single OAuth flow across two hosts. The Content-Type fix is what unblocks the actual bug; the host name is cosmetic in comparison. When Kit deprecates the legacy host, all three OAuth endpoints will migrate together in a coordinated change with proper end-to-end verification.
Net::HTTP.post_form directly, not a patched Connection Stdlib enforces both the form encoding and the Content-Type header in a single call. Patching Connection to accept a per-request Content-Type override would add coupling for one caller's benefit, and stdlib here exactly mirrors the snippet Kit's engineer recommended — useful when a future Kit-side debugger reads our wire traffic.
Raise on non-2xx instead of returning a boolean The previous response.success? return was the precise mechanism that hid this bug for over a year — an unparseable-request 200 looked identical to a real-revoke 200. Failures should be loud. The only caller already has rescue StandardError and structured-warn logging, so this immediately surfaces real failures in our existing log infrastructure with no caller-side change.

Blast Radius

Pure gem-internal change. Method signature is unchanged: ConvertKit::OAuth.new(id, secret).revoke_token(token) works exactly as before. The only production caller (ConvertKitService#revoke_token_for_provider in Speckel) already wraps the call in rescue StandardError, so failures land in existing log infrastructure with no caller-side change required.

What to watch after deploy:

  • mighty-logger lines labelled oauth,kit with message: "Kit /oauth/revoke failed". Pre-merge: never observed (the silent-200 path masked all real failures). Post-merge: any actual rejection from Kit (wrong client_id, expired credentials) becomes visible at warn level with the HTTP status and body attached.
  • Successful revokes still look identical in our logs — the visible change is on Kit's side, not ours.

Rollback Plan

Revert this PR via GitHub's Revert button. Returns to the silent-no-op state that's been running for over a year — a known steady state, not the state we want, but stable.

Test plan

Verifiable by agent before merging

  • Full rspec suite under Ruby 2.7.7 (gem's .ruby-version): 140 examples, 0 failures, 99.51% coverage.
  • Full rspec suite under Ruby 3.4.7 (Speckel's Ruby): 140 examples, 2 failures. Both are pre-existing kwargs-strictness mock mismatches on main (Purchases#create, Subscribers#bulk_create); neither file is touched by this PR.
  • oauth_spec.rb itself: 16/16 pass on both Rubies.
  • Spec verifies the exact URL (https://app.convertkit.com/oauth/revoke), exact form parameters (token, client_id, client_secret, token_type_hint=access_token), true return on 2xx, and ConvertKit::OauthError raise with HTTP status and body on non-2xx.

Cannot be verified before merge

  • Confirm Kit's server actually invalidates the token end-to-end — requires this gem to ship into Speckel, a live disconnect against a real Kit account, and Kit's engineer (or Kit's logs) confirming the revoke landed. Local mocks prove the request leaves the gem well-formed; only a round-trip proves Kit's side processes it.

ahorovit added 3 commits June 8, 2026 18:07
Kit's /oauth/revoke is a standard RFC 7009 endpoint hosted at
https://api.kit.com/v4/oauth/revoke and requires
application/x-www-form-urlencoded. The gem's Connection class POSTs
application/json, which the endpoint cannot parse; per RFC 7009 §2.2
it returns 200 to unparseable requests, so the previous implementation
logged successes that never actually revoked tokens on Kit's side.

Route revoke_token directly through Net::HTTP.post_form so the
Content-Type is enforced by the standard library. token_type_hint is
included per the RFC §2.1 recommendation. Non-2xx responses now raise
ConvertKit::OauthError with the status and body so caller-side logging
sees real failures.

API docs: https://developers.kit.com/api-reference/oauth-token-revocation
Kit's docs name api.kit.com/v4/oauth/revoke as the canonical endpoint,
but get_token and refresh_token continue to live at app.convertkit.com,
and Speckel's authorize URL also points at app.convertkit.com. Splitting
revoke onto the new host alone would introduce host asymmetry within a
single OAuth flow with no functional benefit — the form-encoded
Content-Type fix is what unblocks the bug, not the host change.

Stay on app.convertkit.com for now. When Kit deprecates the legacy host
we can migrate authorize, token, and revoke together in a coordinated
change with proper end-to-end verification.
The earlier introduction of REVOKE_URL was over-engineering — the gem
already has a URL constant for the host. Restore REVOKE_PATH to match
the original constant name and compose the revoke URI inline via
URI.join(URL, REVOKE_PATH). Shrinks the diff against main to just the
require, the method body, and the comment.
@ahorovit ahorovit self-assigned this Jun 9, 2026
@ahorovit ahorovit marked this pull request as ready for review June 9, 2026 04:22
@ahorovit ahorovit merged commit 70229c5 into main Jun 9, 2026
1 check passed
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