Skip to content

fix(oauth): send client_id on token revocation - #249

Draft
big-kahuna-burger wants to merge 6 commits into
mainfrom
oauth-revocation-includes-client-id
Draft

fix(oauth): send client_id on token revocation#249
big-kahuna-burger wants to merge 6 commits into
mainfrom
oauth-revocation-includes-client-id

Conversation

@big-kahuna-burger

@big-kahuna-burger big-kahuna-burger commented Aug 14, 2026

Copy link
Copy Markdown

Summary

Send client_id on OAuth token revocation by going through OAuthClient.RevokeToken instead of a raw form POST.

Depends on dash0hq/dash0-api-client-go#31, which adds ClientId to OAuthRevocationRequest. Until that ships, go.mod has a local redirect:

replace github.com/dash0hq/dash0-api-client-go => ../dash0-api-client-go

CI cannot resolve that path and will fail. Drop the replace and bump the client version once PR 31 is released on upstream.

Why

The authorization server now requires client_id on POST /oauth/revoke (RFC 7009 §2.1). Without it, logout, re-login, and config profiles update --oauth=false left the refresh token valid server-side.

Also in this PR

  • oauth.RevokeRequest so API URL / client ID / refresh token cannot be swapped at call sites
  • httptest handlers no longer FailNow on the server goroutine
  • SamplingModeAdaptive / SamplingModeDisabled (needed to compile against the client PR, which is based on current client main)

Tests

  • Revoke request body includes client_id
  • --oauth=false revoke path
  • Re-login revokes the old token with the old client's ID, not the new one
  • Session-expired error message, interactive + agent mode

@big-kahuna-burger
big-kahuna-burger requested a review from a team as a code owner August 14, 2026 17:55

@mmanciop mmanciop left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think at least parts of this change needs to go into dash0-api-client-go so that the Terraform provider can also benefit from it

@big-kahuna-burger

Copy link
Copy Markdown
Author

I think at least parts of this change needs to go into dash0-api-client-go so that the Terraform provider can also benefit from it

Agree, let me handle that

Comment thread internal/oauth/revoke.go Outdated
Comment thread internal/oauth/revoke.go Outdated
Comment thread internal/oauth/revoke.go Outdated
Comment thread internal/oauth/revoke.go Outdated
Comment thread internal/config/config_cmd_test.go Outdated
Comment thread internal/login/integration_test.go Outdated
big-kahuna-burger and others added 2 commits August 17, 2026 20:22
Send client_id through the typed RevokeToken path instead of a raw form POST, and point go.mod at the sibling dash0-api-client-go checkout until that PR ships.

Co-authored-by: Cursor <cursoragent@cursor.com>
addressed review comment to provide a RevokeRequest struct
@big-kahuna-burger

Copy link
Copy Markdown
Author

Thanks @mmanciop for guidance on how to land this and @nicolastakashi for detailed review. Addressed everything as far as I can see.

Both:

go test -v ./internal/oauth/
go test -v ./internal/config/ -run 'OAuthFalse|Revoke'
go test -v -tags=integration ./internal/login/ -run 'Revoke|Logout'

and

go test ./...
go test -tags=integration ./...
make lint-go
make build

Exit with 0 - logs not shown here for brevity, but you can simply checkout and run next to a local version of api client from the upstream PR: dash0hq/dash0-api-client-go#31

@mmanciop
mmanciop marked this pull request as draft August 18, 2026 08:42
@mmanciop

Copy link
Copy Markdown
Member

Converting to draft until dash0hq/dash0-api-client-go#31 ships

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.

3 participants