fix(salesforce): preserve the caller's OAuth state alongside the PKCE verifier - #100
Merged
Merged
Conversation
… verifier getAuthorizationUri() overwrote the `state` query parameter with the encrypted PKCE verifier and never read `this.state`, the value Frigg passes through from the adopter. In OAuth 2.0 `state` belongs to the client, and adopters rely on it: Clockwork serves every firm from one bounce endpoint and resolves the firm hostname out of `state`, so the Salesforce callback landed on "unknown firm" for every connect attempt. HubSpot's module appends `this.state` and works; this module was the outlier. The verifier still has to survive the round trip, so it is now appended behind `~` rather than replacing the caller's value. `~` is unreserved in RFC 3986 and absent from base64url, so it cannot appear inside the verifier. restoreVerifierFromState() reads the trailing segment and falls back to the whole string, which keeps authorizations that were in flight before this change working and leaves callers who pass no state exactly where they were. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Cut the comments added by the state fix down to the repo's style; the rationale stays in the PR description. Behaviour is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
|
🚀 PR was released in |
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
2 participants
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.
Summary
getAuthorizationUri()overwrote the OAuthstateparameter with the encrypted PKCE verifier and never readthis.state, the value Frigg passes through from the adopter.statebelongs to the client; adopters that use it could not route their own callback.Clockwork is the case that surfaces it: one bounce endpoint serves every firm and resolves the firm hostname from
state. Every Salesforce connect attempt ended on "Authorization state is missing or refers to an unknown firm." The same request against the HubSpot module returns the caller's state verbatim.statesentstatereturnedhubspottestfirma.MYNONCE123testfirma.MYNONCE123salesforcetestfirma.MYNONCE123iFEOtKPJEl6iXUHo.J9Ubkra…Change
~instead of replacing the caller's state.~is unreserved per RFC 3986 (survives the provider redirect unescaped) and is not in the base64url alphabet, so it cannot occur inside the verifier.restoreVerifierFromState()reads the trailing segment and falls back to the whole string.Strictly additive: a caller that passes no state gets the bare verifier exactly as before, and an authorization already in flight when this ships still completes. Adopters that never read
statesee no behavioural change.Tests
Five new cases in
test/api.test.js: caller state preserved, first-dot split still yields the hostname, verifier round-trips out of the composed state, no-caller-state emits the bare verifier, legacy bare-verifier state still restores. The jsforce mock now setscodeVerifierwhenuseVerifieris on, matching the real library — previously no test reached the state branch at all.Suite: 23 passing (was 18), same 17 pre-existing failures that need a live org. The two
getAuthorizationUriscope assertions fail at baseline too (the mock supplies no scope) and are unrelated.Adopter side
Clockwork's
oauth_state_noncenow stops at the delimiter — a one-line change, already written.🤖 Generated with Claude Code
📦 Published PR as canary version:
Canary Versions✨ Test out this PR locally via:
npm install @friggframework/api-module-salesforce@1.0.3-canary.100.ffcd0f5.0 # or yarn add @friggframework/api-module-salesforce@1.0.3-canary.100.ffcd0f5.0Version
Published prerelease version:
@friggframework/api-module-salesforce@2.0.0-next.8Changelog
🐛 Bug Fix
@friggframework/api-module-salesforce@friggframework/api-module-microsoft-teams,@friggframework/api-module-slack,@friggframework/api-module-42matters,@friggframework/api-module-asana,@friggframework/api-module-attio,@friggframework/api-module-clio,@friggframework/api-module-connectwise,@friggframework/api-module-contentful,@friggframework/api-module-contentstack,@friggframework/api-module-crossbeam,@friggframework/api-module-deel,@friggframework/api-module-fathom,@friggframework/api-module-fireflies,@friggframework/api-module-frigg-scale-test,@friggframework/api-module-frontify,@friggframework/api-module-gong,@friggframework/api-module-google-calendar,@friggframework/api-module-google-drive,@friggframework/api-module-helpscout,@friggframework/api-module-hubspot,@friggframework/api-module-ironclad,@friggframework/api-module-linear,@friggframework/api-module-otter,@friggframework/api-module-pipedrive,@friggframework/api-module-quo,@friggframework/api-module-reevo,@friggframework/api-module-salesforce,@friggframework/api-module-stripe,@friggframework/api-module-unbabel-projects,@friggframework/api-module-unbabel,@friggframework/api-module-zoho-crm,@friggframework/api-module-zoomAuthors: 2