Skip to content

Conversation

@issuedat
Copy link
Contributor

@issuedat issuedat commented Dec 1, 2025

Adds support for X (formerly Twitter) v2 as an external OAuth provider.

  • Introduces an oauth_states table to persist the code_verifiers as X mandates the use of PKCE
  • Uses SHA256 challenge for PKCE
  • Updates the GetOAuthToken signature to accept a code_verifier as the second parameter
  • Uses the existing cleanup middleware to delete states
  • Adds the provider as x rather than x_v2 as twitter is already used in old OAuth 1.0a provider to better align with the rebrand
  • The state is a UUIDv4

NOTE: today the flow_states table is overloaded, containing states, auth codes, provider tokens...the goal is to decouple that table eventually and the oauth_states table is the first step towards that.

@issuedat issuedat requested a review from a team as a code owner December 1, 2025 10:41
@coveralls
Copy link

coveralls commented Dec 1, 2025

Pull Request Test Coverage Report for Build 19871055030

Details

  • 254 of 317 (80.13%) changed or added relevant lines in 31 files are covered.
  • 4 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.2%) to 68.435%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/provider/twitter.go 3 4 75.0%
internal/api/provider/slack_oidc.go 2 5 40.0%
internal/models/oauth_client_state.go 17 21 80.95%
internal/api/provider/apple.go 2 7 28.57%
internal/api/provider/slack.go 0 5 0.0%
internal/api/provider/spotify.go 0 5 0.0%
internal/models/errors.go 0 5 0.0%
internal/api/external.go 33 39 84.62%
internal/api/provider/x.go 80 86 93.02%
internal/api/provider/linkedin_oidc.go 0 7 0.0%
Files with Coverage Reduction New Missed Lines %
internal/api/provider/linkedin_oidc.go 1 0.0%
internal/api/provider/slack.go 1 0.0%
internal/api/provider/spotify.go 1 0.0%
internal/api/provider/vercel_marketplace.go 1 0.0%
Totals Coverage Status
Change from base Build 19864408688: 0.2%
Covered Lines: 14641
Relevant Lines: 21394

💛 - Coveralls

Copy link
Contributor

@cstockton cstockton left a comment

Choose a reason for hiding this comment

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

If possible I would like to remove context.Background while we are touching these code paths.

Copy link
Contributor

@cstockton cstockton left a comment

Choose a reason for hiding this comment

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

🚀

@issuedat issuedat merged commit 7f36eb0 into master Dec 2, 2025
6 checks passed
@issuedat issuedat deleted the iat/x-provider branch December 2, 2025 19:53
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.

4 participants