Embedded Stripe checkout with redirect mode#1231
Open
manager-hub wants to merge 4 commits intodevelopfrom
Open
Embedded Stripe checkout with redirect mode#1231manager-hub wants to merge 4 commits intodevelopfrom
manager-hub wants to merge 4 commits intodevelopfrom
Conversation
- Add embedded mode parameter to checkout session creation - Return client_secret for embedded mode instead of sessionId - Use return_url instead of success_url/cancel_url for embedded mode - Look up promotion codes properly when coupon param is provided Backend supports both modes - controlled by frontend request param. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Security improvements: - Add TRUSTED_FRONTEND_URL based on SERVER_URL environment mapping - Replace req.headers.origin with trusted URL to prevent URL spoofing - Add null check for session.client_secret in embedded mode - Return 500 error with log if client_secret is missing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add session.url to response for redirect mode (stripe.redirectToCheckout removed in v8) - Add null check for session.url with error logging - Keep sessionId in response for backwards compatibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
sqlite3 requires node-gyp to compile native bindings, which needs Python and build tools. This fixes the CI build failure: "gyp ERR! find Python - python3 is not in PATH or produced an error" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe pull request updates system dependencies in Docker build environments by adding Python 3 and build-essential packages, and enhances the Stripe subscription controller to support embedded checkout mode with a configurable trusted frontend URL mechanism for redirect handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/Frontend
participant Server as Server
participant Stripe as Stripe API
alt Embedded Mode
Client->>Server: POST /checkout (embedded: true)
Server->>Server: Validate schema, set ui_mode: 'embedded'
Server->>Stripe: Create checkout session (embedded)
Stripe-->>Server: Return clientSecret
Server-->>Client: Return clientSecret
Client->>Client: Initialize Stripe embedded form
else Redirect Mode
Client->>Server: POST /checkout (embedded: false)
Server->>Server: Validate schema, set success/cancel URLs
Server->>Stripe: Create checkout session (redirect)
Stripe-->>Server: Return sessionId & url
Server-->>Client: Return session url
Client->>Stripe: Redirect to checkout
Stripe->>Client: Redirect to success/cancel URL
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![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
🤖 Fix all issues with AI agents
In `@desci-server/src/controllers/stripe/subscription.ts`:
- Around line 232-234: The portal return URL uses the user-supplied returnUrl
directly when creating portalSession (stripe.billingPortal.sessions.create) —
validate the origin the same way as for the other URL: parse returnUrl, ensure
its origin equals TRUSTED_FRONTEND_URL (or matches allowed trusted origins), and
only pass the user returnUrl to stripe.billingPortal.sessions.create if it
passes validation; otherwise fall back to the trusted default
(`${TRUSTED_FRONTEND_URL}/settings/subscription`). Update the logic around
portalSession creation in subscription.ts to perform this origin check before
constructing the request.
- Around line 71-79: The code accepts user-supplied
returnUrl/successUrl/cancelUrl and uses them directly, bypassing
TRUSTED_FRONTEND_URL; update the session URL assignment in the block that sets
sessionConfig (the embedded branch and the else branch) to either (1) always use
TRUSTED_FRONTEND_URL and ignore returnUrl/successUrl/cancelUrl, or (2) validate
any provided returnUrl/successUrl/cancelUrl against an allowlist rule (e.g.,
must startWith TRUSTED_FRONTEND_URL) and only use them when they pass
validation, otherwise fall back to the TRUSTED_FRONTEND_URL defaults; ensure you
reference the variables returnUrl, successUrl, cancelUrl, embedded,
sessionConfig and TRUSTED_FRONTEND_URL when implementing the change so no
user-provided URL can bypass the trusted origin.
🧹 Nitpick comments (3)
desci-server/Dockerfile.ci (1)
4-4: Consider adding--no-install-recommendsto reduce image size.
build-essentialin particular pulls a large set of recommended packages. Adding--no-install-recommendscan significantly reduce the final image size and attack surface. Also consider cleaning up the apt cache in the same layer.Proposed fix
-RUN apt-get -qy update && apt-get -qy install openssl curl socat jq dumb-init python3 build-essential +RUN apt-get -qy update && apt-get -qy --no-install-recommends install openssl curl socat jq dumb-init python3 build-essential \ + && rm -rf /var/lib/apt/lists/*desci-server/Dockerfile (1)
5-5: Same--no-install-recommendssuggestion as inDockerfile.ci.Proposed fix
-RUN apt-get -qy update && apt-get -qy install openssl curl socat jq python3 build-essential +RUN apt-get -qy update && apt-get -qy --no-install-recommends install openssl curl socat jq python3 build-essential \ + && rm -rf /var/lib/apt/lists/*desci-server/src/controllers/stripe/subscription.ts (1)
16-24:TRUSTED_FRONTEND_URLis evaluated once at module load — env changes require a restart.This is fine for typical deployments but worth noting. The nested ternary is a bit hard to scan; a lookup map would be clearer.
Optional: use a map for readability
-const TRUSTED_FRONTEND_URL = - process.env.FRONTEND_URL || - (process.env.SERVER_URL === 'https://nodes-api.desci.com' - ? 'https://sciweave.com' - : process.env.SERVER_URL === 'https://nodes-api-dev.desci.com' - ? 'https://dev.sciweave.com' - : 'http://localhost:3000'); +const SERVER_TO_FRONTEND: Record<string, string> = { + 'https://nodes-api.desci.com': 'https://sciweave.com', + 'https://nodes-api-dev.desci.com': 'https://dev.sciweave.com', +}; + +const TRUSTED_FRONTEND_URL = + process.env.FRONTEND_URL || + SERVER_TO_FRONTEND[process.env.SERVER_URL ?? ''] || + 'http://localhost:3000';
Comment on lines
+71
to
+79
| // Use embedded mode or redirect mode based on request | ||
| // Use trusted frontend URL instead of req.headers.origin to prevent URL spoofing | ||
| if (embedded) { | ||
| sessionConfig.ui_mode = 'embedded'; | ||
| sessionConfig.return_url = returnUrl || `${TRUSTED_FRONTEND_URL}/settings/subscription/complete?session_id={CHECKOUT_SESSION_ID}`; | ||
| } else { | ||
| sessionConfig.success_url = successUrl || `${TRUSTED_FRONTEND_URL}/settings/subscription?success=true`; | ||
| sessionConfig.cancel_url = cancelUrl || `${TRUSTED_FRONTEND_URL}/settings/subscription?canceled=true`; | ||
| } |
There was a problem hiding this comment.
User-supplied URLs bypass the trusted-URL protection.
The comment on Line 72 states the intent is to "prevent URL spoofing," but successUrl, cancelUrl, and returnUrl are accepted from the request body and used as-is when provided. An attacker can supply arbitrary redirect targets, which is the exact vector TRUSTED_FRONTEND_URL was introduced to mitigate.
Either remove the user-supplied URL overrides entirely (always use TRUSTED_FRONTEND_URL), or validate that any provided URLs match an allowlist (e.g., must start with TRUSTED_FRONTEND_URL).
Example: validate URLs against trusted origin
+ const isUrlTrusted = (url: string) => url.startsWith(TRUSTED_FRONTEND_URL);
+
if (embedded) {
sessionConfig.ui_mode = 'embedded';
- sessionConfig.return_url = returnUrl || `${TRUSTED_FRONTEND_URL}/settings/subscription/complete?session_id={CHECKOUT_SESSION_ID}`;
+ const resolvedReturnUrl = returnUrl && isUrlTrusted(returnUrl) ? returnUrl : `${TRUSTED_FRONTEND_URL}/settings/subscription/complete?session_id={CHECKOUT_SESSION_ID}`;
+ sessionConfig.return_url = resolvedReturnUrl;
} else {
- sessionConfig.success_url = successUrl || `${TRUSTED_FRONTEND_URL}/settings/subscription?success=true`;
- sessionConfig.cancel_url = cancelUrl || `${TRUSTED_FRONTEND_URL}/settings/subscription?canceled=true`;
+ sessionConfig.success_url = successUrl && isUrlTrusted(successUrl) ? successUrl : `${TRUSTED_FRONTEND_URL}/settings/subscription?success=true`;
+ sessionConfig.cancel_url = cancelUrl && isUrlTrusted(cancelUrl) ? cancelUrl : `${TRUSTED_FRONTEND_URL}/settings/subscription?canceled=true`;
}🤖 Prompt for AI Agents
In `@desci-server/src/controllers/stripe/subscription.ts` around lines 71 - 79,
The code accepts user-supplied returnUrl/successUrl/cancelUrl and uses them
directly, bypassing TRUSTED_FRONTEND_URL; update the session URL assignment in
the block that sets sessionConfig (the embedded branch and the else branch) to
either (1) always use TRUSTED_FRONTEND_URL and ignore
returnUrl/successUrl/cancelUrl, or (2) validate any provided
returnUrl/successUrl/cancelUrl against an allowlist rule (e.g., must startWith
TRUSTED_FRONTEND_URL) and only use them when they pass validation, otherwise
fall back to the TRUSTED_FRONTEND_URL defaults; ensure you reference the
variables returnUrl, successUrl, cancelUrl, embedded, sessionConfig and
TRUSTED_FRONTEND_URL when implementing the change so no user-provided URL can
bypass the trusted origin.
Comment on lines
232
to
+234
| const portalSession = await stripe.billingPortal.sessions.create({ | ||
| customer: subscription.stripeCustomerId, | ||
| return_url: returnUrl || `${req.headers.origin}/settings/subscription`, | ||
| return_url: returnUrl || `${TRUSTED_FRONTEND_URL}/settings/subscription`, |
There was a problem hiding this comment.
Same URL validation concern applies to the portal returnUrl.
The user-supplied returnUrl is used directly here too. Apply the same trusted-origin validation as suggested above.
🤖 Prompt for AI Agents
In `@desci-server/src/controllers/stripe/subscription.ts` around lines 232 - 234,
The portal return URL uses the user-supplied returnUrl directly when creating
portalSession (stripe.billingPortal.sessions.create) — validate the origin the
same way as for the other URL: parse returnUrl, ensure its origin equals
TRUSTED_FRONTEND_URL (or matches allowed trusted origins), and only pass the
user returnUrl to stripe.billingPortal.sessions.create if it passes validation;
otherwise fall back to the trusted default
(`${TRUSTED_FRONTEND_URL}/settings/subscription`). Update the logic around
portalSession creation in subscription.ts to perform this origin check before
constructing the request.
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
1 participant
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
req.headers.originTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores