Conversation
🦋 Changeset detectedLatest commit: 4136984 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughA KYC link allowlist feature is introduced for Bridge onboarding, adding schema support for a Changes
Sequence DiagramsequenceDiagram
participant Client
participant GetProvider as getProvider()
participant MaybeKycLink as maybeKycLink()
participant BridgeAPI as Bridge API
participant ErrorHandler as Sentry/Error Handler
Client->>GetProvider: Request provider info (customerId, redirectURL)
GetProvider->>GetProvider: Check onboarding status
alt User in ONBOARDING
GetProvider->>MaybeKycLink: Check if KYC link needed
alt Conditions match (missing/issues in allowlist)
MaybeKycLink->>BridgeAPI: getKycLink(customerId, redirectUri, endorsement)
alt Success
BridgeAPI-->>MaybeKycLink: KYC URL
MaybeKycLink-->>GetProvider: kycLink populated
else Failure
BridgeAPI-->>MaybeKycLink: Error
MaybeKycLink->>ErrorHandler: captureException
MaybeKycLink-->>GetProvider: kycLink undefined
end
else Conditions don't match or blocked
MaybeKycLink-->>GetProvider: kycLink undefined
end
GetProvider-->>Client: Provider info with kycLink
else Other status
GetProvider-->>Client: Provider info without kycLink
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a KYC link allowlist for bridge onboarding, allowing the system to conditionally provide a KYC URL based on specific customer requirements and endorsement statuses. Key changes include the addition of a getKycLink utility, logic to evaluate complex requirement structures (using all_of and any_of), and extensive test coverage for various onboarding scenarios. A technical issue was identified in the getKycLink function where manual URI encoding of the redirect URI leads to double-encoding, as URLSearchParams handles this automatically.
| export function getKycLink(customerId: string, redirectUri?: string, endorsement?: (typeof Endorsements)[number]) { | ||
| const params = new URLSearchParams(); | ||
| if (endorsement) params.set("endorsement", endorsement); | ||
| if (redirectUri) params.set("redirect_uri", encodeURIComponent(redirectUri)); |
There was a problem hiding this comment.
URLSearchParams.set automatically handles URL encoding for both keys and values. Manually calling encodeURIComponent on redirectUri here will result in double-encoding. Additionally, encoding should be centralized within the function that constructs the request rather than at each call site to prevent omissions and ensure consistency.
| if (redirectUri) params.set("redirect_uri", encodeURIComponent(redirectUri)); | |
| if (redirectUri) params.set("redirect_uri", redirectUri); |
References
- When a value is used to construct a request URL, encode it within the function that makes the request rather than at each call site to centralize the security logic and prevent future omissions.
|
✅ All tests passed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 33b527bb-897b-46d0-bf9d-c2084782b01d
📒 Files selected for processing (4)
.changeset/bright-owls-glow.mdserver/api/ramp.tsserver/test/utils/bridge.test.tsserver/utils/ramps/bridge.ts
| it("returns ONBOARDING without kycLink when nested missing has no matching targets", async () => { | ||
| vi.spyOn(globalThis, "fetch").mockResolvedValueOnce( | ||
| fetchResponse({ | ||
| ...activeCustomer, | ||
| status: "incomplete", | ||
| endorsements: [ | ||
| { | ||
| name: "base", | ||
| status: "approved", | ||
| requirements: { | ||
| complete: [], | ||
| pending: [], | ||
| missing: { | ||
| all_of: ["address_of_residence", { any_of: ["first_name", "source_of_funds_questionnaire"] }], | ||
| }, | ||
| issues: [], | ||
| }, | ||
| }, | ||
| ], | ||
| }), | ||
| ); | ||
|
|
||
| const result = await bridge.getProvider({ credentialId: "cred-1", customerId: "cust-1" }); | ||
|
|
||
| expect(result.status).toBe("ONBOARDING"); | ||
| expect(result.kycLink).toBeUndefined(); |
There was a problem hiding this comment.
This negative case still matches the allowlist.
source_of_funds_questionnaire in this fixture is one of the allowed missing requirements, so containsRequirement() in server/utils/ramps/bridge.ts will try to fetch a KYC link here. The test only passes because the default fetch mock turns that extra request into kycLink: undefined, which makes this a false positive.
🧪 Suggested fix
missing: {
- all_of: ["address_of_residence", { any_of: ["first_name", "source_of_funds_questionnaire"] }],
+ all_of: ["address_of_residence", { any_of: ["first_name", "last_name"] }],
},
},
],
@@
const result = await bridge.getProvider({ credentialId: "cred-1", customerId: "cust-1" });
expect(result.status).toBe("ONBOARDING");
expect(result.kycLink).toBeUndefined();
+ expect(globalThis.fetch).toHaveBeenCalledTimes(1);Please apply the same “only one fetch call” assertion to the other negative KYC-link cases above as well.
| export function getKycLink(customerId: string, redirectUri?: string, endorsement?: (typeof Endorsements)[number]) { | ||
| const params = new URLSearchParams(); | ||
| if (endorsement) params.set("endorsement", endorsement); | ||
| if (redirectUri) params.set("redirect_uri", encodeURIComponent(redirectUri)); | ||
| return request( | ||
| object({ url: string() }), | ||
| `/customers/${customerId}/kyc_link${String(params) ? `?${String(params)}` : ""}`, | ||
| ).then((result) => result.url); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, confirm the code at lines 160-167
head -n 170 server/utils/ramps/bridge.ts | tail -n 20Repository: exactly/exa
Length of output: 776
🏁 Script executed:
# Search for usages of getKycLink to see how it's called
rg "getKycLink" --type ts --type tsx -B 2 -A 2Repository: exactly/exa
Length of output: 82
🏁 Script executed:
# Look for test files related to this module
fd -e test.ts -e spec.ts | xargs grep -l "getKycLink\|bridge" 2>/dev/null | head -5Repository: exactly/exa
Length of output: 211
🏁 Script executed:
# Check the structure of URLSearchParams usage in the codebase
rg "URLSearchParams" --type ts --type tsx -B 1 -A 1 | head -50Repository: exactly/exa
Length of output: 82
🏁 Script executed:
# Check if there are tests for getKycLink
cat server/test/utils/bridge.test.tsRepository: exactly/exa
Length of output: 50367
🏁 Script executed:
# Check how getKycLink is used/called in the codebase
rg "getKycLink" --type ts -B 3 -A 3Repository: exactly/exa
Length of output: 1520
🏁 Script executed:
# Also check if there's any documentation or comments about URL encoding in bridge.ts
head -n 200 server/utils/ramps/bridge.ts | tail -n 100Repository: exactly/exa
Length of output: 4069
🏁 Script executed:
# Check how redirectURL flows through getProvider and into getKycLink
rg "redirectURL\|redirectUri" server/utils/ramps/bridge.ts -B 2 -A 2Repository: exactly/exa
Length of output: 37
🏁 Script executed:
# Look for any integration tests or actual usage of redirectURL
rg "redirectURL" server/test -B 2 -A 2 | head -50Repository: exactly/exa
Length of output: 458
🏁 Script executed:
# Let's search for the specific test that uses redirectURL to see what it validates
rg "appends redirect URL with provider param" server/test/utils/bridge.test.ts -A 10Repository: exactly/exa
Length of output: 604
🏁 Script executed:
# Check how getProvider passes redirectURL to getKycLink
rg "getKycLink\|redirectURL" server/utils/ramps/bridge.ts -B 3 -A 3Repository: exactly/exa
Length of output: 37
🏁 Script executed:
# Let's view the full getProvider function to see how redirectURL flows to getKycLink
sed -n '214,280p' server/utils/ramps/bridge.tsRepository: exactly/exa
Length of output: 2650
🏁 Script executed:
# Search for where getKycLink is called with redirectUri
rg "getKycLink" server/utils/ramps/bridge.ts -A 1 -B 1Repository: exactly/exa
Length of output: 360
🏁 Script executed:
# Let's verify URLSearchParams encoding behavior with a test
node <<'EOF'
const params = new URLSearchParams();
const redirectUri = "https://app.example.com/callback";
// First, what happens with pre-encoding
params.set("redirect_uri", encodeURIComponent(redirectUri));
console.log("With pre-encoding:", String(params));
console.log("Decoded value:", decodeURIComponent(params.get("redirect_uri")));
// What should happen without pre-encoding
const params2 = new URLSearchParams();
params2.set("redirect_uri", redirectUri);
console.log("\nWithout pre-encoding:", String(params2));
console.log("Decoded value:", decodeURIComponent(params2.get("redirect_uri")));
EOFRepository: exactly/exa
Length of output: 308
Remove pre-encoding of redirect_uri before passing to URLSearchParams.
Line 163 double-encodes the redirect URI. URLSearchParams.set() already percent-encodes values, so calling encodeURIComponent() first causes double-encoding: a redirect like https://app.example.com/callback becomes https%253A%252F%252Fapp.example.com%252Fcallback. Bridge receives the encoded literal string instead of the actual URL, causing the post-KYC redirect to fail.
🛠️ Proposed fix
export function getKycLink(customerId: string, redirectUri?: string, endorsement?: (typeof Endorsements)[number]) {
const params = new URLSearchParams();
if (endorsement) params.set("endorsement", endorsement);
- if (redirectUri) params.set("redirect_uri", encodeURIComponent(redirectUri));
+ if (redirectUri) params.set("redirect_uri", redirectUri);
return request(
object({ url: string() }),
`/customers/${customerId}/kyc_link${String(params) ? `?${String(params)}` : ""}`,
).then((result) => result.url);
}1 participant
Summary by CodeRabbit
New Features
Tests