Repository navigation
fix: echo the authorize redirect_uri when exchanging the Slack install code - #327
Merged
Merged
Conversation
…l code
Installing Slack from the dashboard died right after the user clicked
Allow, redirecting to /saas/install-error?reason=oauth_failed. Production
logs name the cause exactly:
msg="Failed to exchange OAuth code" error="slack API error: bad_redirect_uri"
BuildInstallURL always sent a redirect_uri on the authorize request, but
exchangeCodeAndFetchUser passed "" to oauth.v2.access, with a comment
claiming the parameter was optional. It is optional only when authorize
did not use one — when it did, Slack requires the exchange to echo the
identical value, and rejects the whole install otherwise.
Both call sites now go through a single installRedirectURI() helper, the
same shape Discord already uses (discord/service.go:256,323) and the same
one Slack *login* uses (auth/slack_service.go:326) — which is why login
kept working and masked this for months.
The reason no test caught it: nothing ever drove a successful exchange.
The only other test through this path deliberately relies on the exchange
failing. The new test asserts the exchange REQUEST, not just the authorize
URL, and was confirmed to fail on the old code with
expected "http://localhost:4000/api/v1/integrations/slack/oauth", actual "".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 5, 2026
Merged
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
Installing Slack from the dashboard (
/dash0/orgs/<org>/integrations/new) fails on production right after the user clicks Allow, bouncing tohttps://www.solidping.io/saas/install-error?reason=oauth_failed.Production logs name the cause exactly:
Root cause
BuildInstallURLalways sent aredirect_urion the authorize request, butexchangeCodeAndFetchUserpassed""tooauth.v2.access, with a comment asserting the parameter was optional.It is optional only when authorize did not use one. When authorize did send one, Slack requires the exchange to echo the identical value and rejects the install otherwise.
Why it went unnoticed
Two reasons, both worth recording:
auth/slack_service.go:326passes the callback URL at both ends, so signing in with Slack kept working and masked a completely broken install path. Discord was never affected either — it already funnels both call sites through oneinstallRedirectURI()helper (discord/service.go:256,323).TestHandleOAuthCallback_StateConsumedOnReuse) deliberately relies on the exchange failing, and the soleredirect_uriassertion was on the authorize URL. The asymmetry was structurally invisible.The fix
Both call sites now go through a single
installRedirectURI()helper — the same shape Discord and Slack login already use, so they cannot drift again.Test
TestHandleOAuthCallback_EchoesAuthorizeRedirectURIAtExchangeasserts the exchange request, not just the authorize URL. It was confirmed to fail on the old code:It also carries a positive control pinning the concrete value, so a helper that returned
""on both sides would still fail.QA
make build-backend lint-back test— all green (lint 0 issues, full backend suite passing).Note for the deploy
This is a production fix; prod has silently lagged releases before, so it is worth confirming the running image actually picks this up rather than assuming the tag deployed.
🤖 Generated with Claude Code