feat: allow organization owners access to shared billing portal#23451
feat: allow organization owners access to shared billing portal#23451anikdhabal merged 8 commits intomainfrom
Conversation
- Modify stripepayment portal endpoint to accept teamId parameter - Use team's subscription ID to get customer ID from Stripe subscription - Validate user permissions before allowing team billing access - Update platform and regular billing views to pass teamId - Maintain backward compatibility with individual user billing Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
WalkthroughThe UI adds NextAuth useSession to BillingView and introduces getTeamIdFromContext to derive a teamId from the URL (/settings/teams/{id}/billing) or from the session for organization billing, updating billingHref to include teamId and returnTo when present. The portal API adds a team-scoped flow: if teamId is provided, it verifies admin access via TeamRepository.getTeamByIdIfUserIsAdmin, extracts subscriptionId from team.metadata (teamMetadataSchema), loads it with getSubscriptionFromId, creates a Stripe portal session, and issues a 302 redirect. Otherwise it falls back to the user-centric flow returning JSON { url }. Multiple 400 validations are added. New repository method and subscription helper are exported. Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
keithwillcode
added
core
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
In this endpoint we were only showing the portal if the requesting user id matched the specific customer id tied to the subscription on Stripe. Now we determine if the requesting user is an owner/admin of the team then redirect them to the billing portal.
dosubot
bot
added
billing
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/app-store/stripepayment/api/portal.ts (1)
12-71: Ensure all callers includeteamIdor adjust handler for fallback.
- apps/web/modules/settings/billing/billing-view.tsx: line 71 uses
/api/integrations/stripepayment/portal?returnTo=…withoutteamId, which will trigger the “Team ID is required” 400 from the handler.- apps/web/modules/settings/platform/billing/billing-view.tsx always supplies
teamIdand is unaffected.
🧹 Nitpick comments (4)
packages/app-store/stripepayment/lib/subscriptions.ts (1)
31-33: Add explicit return type; minor cleanup.Typing improves safety for callers and avoids accidental structural changes later.
+import type Stripe from "stripe"; -export async function getSubscriptionFromId(subscriptionId: string) { - return await stripe.subscriptions.retrieve(subscriptionId); +export async function getSubscriptionFromId(subscriptionId: string): Promise<Stripe.Subscription> { + return stripe.subscriptions.retrieve(subscriptionId); }apps/web/modules/settings/platform/billing/billing-view.tsx (1)
44-45: URL-encode returnTo to avoid malformed query strings.Prevents issues with special characters in the path.
- const billingHref = `/api/integrations/stripepayment/portal?teamId=${userOrgId}&returnTo=${WEBAPP_URL}${returnTo}`; + const billingHref = `/api/integrations/stripepayment/portal?teamId=${userOrgId}&returnTo=${encodeURIComponent( + WEBAPP_URL + returnTo + )}`;packages/app-store/stripepayment/api/portal.ts (1)
18-20: Harden teamId parsing.Avoid falsy checks; validate integer explicitly.
- const userId = req.session.user.id; - const teamId = req.query.teamId ? parseInt(req.query.teamId as string) : null; + const userId = req.session.user.id; + const rawTeamId = Array.isArray(req.query.teamId) ? req.query.teamId[0] : req.query.teamId; + const parsedTeamId = rawTeamId ? Number.parseInt(rawTeamId, 10) : NaN; + const teamId = Number.isNaN(parsedTeamId) ? null : parsedTeamId;apps/web/modules/settings/billing/billing-view.tsx (1)
50-65: Path-based context detection works; consider minor hardening.Optional: use encodeURIComponent for returnTo and guard for session load for org path.
- if (pathname.includes("/organizations/billing")) { - const orgId = session.data?.user?.org?.id; + if (pathname.includes("/organizations/billing")) { + const orgId = session.data?.user?.org?.id; return typeof orgId === "number" ? orgId.toString() : null; }And for the URL:
- const billingHref = teamId - ? `/api/integrations/stripepayment/portal?teamId=${teamId}&returnTo=${WEBAPP_URL}${returnTo}` - : `/api/integrations/stripepayment/portal?returnTo=${WEBAPP_URL}${returnTo}`; + const billingHref = teamId + ? `/api/integrations/stripepayment/portal?teamId=${teamId}&returnTo=${encodeURIComponent( + WEBAPP_URL + returnTo + )}` + : `/api/integrations/stripepayment/portal?returnTo=${encodeURIComponent(WEBAPP_URL + returnTo)}`;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/web/modules/settings/billing/billing-view.tsx(2 hunks)apps/web/modules/settings/platform/billing/billing-view.tsx(1 hunks)packages/app-store/stripepayment/api/portal.ts(1 hunks)packages/app-store/stripepayment/lib/subscriptions.ts(1 hunks)packages/lib/server/repository/team.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/app-store/stripepayment/api/portal.tspackages/lib/server/repository/team.tspackages/app-store/stripepayment/lib/subscriptions.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/app-store/stripepayment/api/portal.tspackages/lib/server/repository/team.tspackages/app-store/stripepayment/lib/subscriptions.tsapps/web/modules/settings/billing/billing-view.tsxapps/web/modules/settings/platform/billing/billing-view.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/app-store/stripepayment/api/portal.tspackages/lib/server/repository/team.tspackages/app-store/stripepayment/lib/subscriptions.tsapps/web/modules/settings/billing/billing-view.tsxapps/web/modules/settings/platform/billing/billing-view.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
apps/web/modules/settings/billing/billing-view.tsxapps/web/modules/settings/platform/billing/billing-view.tsx
🧬 Code graph analysis (5)
packages/app-store/stripepayment/api/portal.ts (1)
packages/app-store/stripepayment/lib/subscriptions.ts (1)
getSubscriptionFromId(31-33)
packages/lib/server/repository/team.ts (1)
packages/platform/libraries/index.ts (1)
MembershipRole(98-98)
packages/app-store/stripepayment/lib/subscriptions.ts (1)
packages/app-store/_utils/stripe.ts (1)
stripe(72-74)
apps/web/modules/settings/billing/billing-view.tsx (1)
packages/lib/constants.ts (1)
WEBAPP_URL(12-18)
apps/web/modules/settings/platform/billing/billing-view.tsx (1)
packages/lib/constants.ts (1)
WEBAPP_URL(12-18)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (3)
packages/app-store/stripepayment/api/portal.ts (2)
59-69: Non-issue: safe redirect usage looks good.Use of getSafeRedirectUrl protects against open redirects; construction of the portal session is standard.
12-14: Method allowance and redirect flow LGTM.Allowing GET/POST and returning 302 aligns with existing client usage.
Also applies to: 66-71
apps/web/modules/settings/billing/billing-view.tsx (1)
3-3: Import and usage of useSession LGTM.Keeps org context available where needed.
Also applies to: 46-47
E2E results are ready! |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/app-store/stripepayment/api/portal.ts (4)
19-24: Do not require teamId; keep backward compatibility with user-level billing.Returning 400 when teamId is absent breaks existing flows that open the individual portal. Make teamId optional and derive a hasTeamScope flag. This also tightens parsing.
Apply this diff:
const userId = req.session.user.id; - const teamId = req.query.teamId ? parseInt(req.query.teamId as string) : null; - - if (!teamId) { - return res.status(400).json({ message: "Team ID is required" }); - } + const teamIdRaw = req.query.teamId; + const teamId = + typeof teamIdRaw === "string" && /^\d+$/.test(teamIdRaw) ? Number(teamIdRaw) : null; + const hasTeamScope = typeof teamId === "number" && Number.isInteger(teamId) && teamId > 0;
38-48: Return 403 when teamId is provided but caller isn’t admin; fall back to user portal only when no teamId.Current code silently falls back to user billing even if a non-admin explicitly requested team billing. Enforce authorization on team-scoped requests; only use the user path when there’s no team scope. Add basic Stripe error handling.
Apply this diff:
- if (!team) { - const customerId = await getStripeCustomerIdFromUserId(userId); - if (!customerId) return res.status(404).json({ message: "CustomerId not found" }); - - const portalSession = await stripe.billingPortal.sessions.create({ - customer: customerId, - return_url, - }); - - return res.status(200).json({ url: portalSession.url }); - } + if (hasTeamScope && !team) { + return res + .status(403) + .json({ message: "Not authorized to manage this team's billing" }); + } + // No team scope: fall back to individual user billing (backward compatible) + if (!hasTeamScope) { + const customerId = await getStripeCustomerIdFromUserId(userId); + if (!customerId) return res.status(404).json({ message: "CustomerId not found" }); + let portalSession; + try { + portalSession = await stripe.billingPortal.sessions.create({ + customer: customerId, + return_url, + }); + } catch { + return res.status(502).json({ message: "Failed to create portal session" }); + } + return res.status(200).json({ url: portalSession.url }); + }
60-64: Catch Stripe retrieval errors for subscription lookup.Wrap getSubscriptionFromId to avoid unhandled exceptions and return an upstream-failure status when Stripe errors.
Apply this diff:
- const subscription = await getSubscriptionFromId(teamMetadataParsed.data.subscriptionId); - - if (!subscription) { - return res.status(400).json({ message: "Subscription not found" }); - } + let subscription; + try { + subscription = await getSubscriptionFromId(teamMetadataParsed.data.subscriptionId); + } catch { + return res.status(502).json({ message: "Failed to retrieve subscription" }); + } + if (!subscription) { + return res.status(400).json({ message: "Subscription not found" }); + }
66-72: Handle Stripe expandable customer field safely.subscription.customer can be a string or an object; don’t assert string.
Apply this diff:
- if (!subscription.customer) { - return res.status(400).json({ message: "Subscription customer not found" }); - } - - const customerId = subscription.customer as string; - - if (!customerId) return res.status(400).json({ message: "CustomerId not found in stripe" }); + const customerId = + typeof subscription.customer === "string" + ? subscription.customer + : subscription.customer?.id; + if (!customerId) + return res.status(400).json({ message: "Subscription customer not found" });
🧹 Nitpick comments (1)
packages/app-store/stripepayment/api/portal.ts (1)
74-79: Optional: guard the team-portal creation with try/catch.Mirror the user-path handling to return 502 if Stripe session creation fails.
I can provide a follow-up diff if you’d like this added.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/app-store/stripepayment/api/portal.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/app-store/stripepayment/api/portal.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/app-store/stripepayment/api/portal.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/app-store/stripepayment/api/portal.ts
🧬 Code graph analysis (1)
packages/app-store/stripepayment/api/portal.ts (3)
packages/lib/constants.ts (1)
WEBAPP_URL(12-18)packages/app-store/_utils/stripe.ts (1)
stripe(72-74)packages/app-store/stripepayment/lib/subscriptions.ts (1)
getSubscriptionFromId(31-33)
🔇 Additional comments (3)
packages/app-store/stripepayment/api/portal.ts (3)
5-7: LGTM: Correct prisma default import and repository/schema usage.Default-importing prisma aligns with repo conventions and fixes the earlier compile error; TeamRepository and teamMetadataSchema imports look right.
11-11: LGTM: Subscription helper import.Importing getSubscriptionFromId here keeps Stripe access centralized.
50-58: LGTM: Zod-validated team metadata.safeParse + explicit 400s for invalid metadata and missing subscriptionId are good guards.
| const teamRepository = new TeamRepository(prisma); | ||
| const team = await teamRepository.getTeamByIdIfUserIsAdmin({ | ||
| teamId, | ||
| userId, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Only query TeamRepository when team scope is present.
Avoid hitting the DB when teamId isn’t provided; set team conditionally.
Apply this diff:
- const teamRepository = new TeamRepository(prisma);
- const team = await teamRepository.getTeamByIdIfUserIsAdmin({
- teamId,
- userId,
- });
+ let team: any = null;
+ if (hasTeamScope) {
+ const teamRepository = new TeamRepository(prisma);
+ team = await teamRepository.getTeamByIdIfUserIsAdmin({
+ teamId: teamId!,
+ userId,
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const teamRepository = new TeamRepository(prisma); | |
| const team = await teamRepository.getTeamByIdIfUserIsAdmin({ | |
| teamId, | |
| userId, | |
| }); | |
| // Only fetch the team if we have a valid team scope | |
| let team: any = null; | |
| if (hasTeamScope) { | |
| const teamRepository = new TeamRepository(prisma); | |
| team = await teamRepository.getTeamByIdIfUserIsAdmin({ | |
| teamId: teamId!, | |
| userId, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/app-store/stripepayment/api/portal.ts around lines 26 to 30, the
code always queries TeamRepository regardless of whether teamId is provided;
change it to only construct TeamRepository/getTeamByIdIfUserIsAdmin and await
the call when teamId (or team scope) is present, otherwise set team to undefined
(or null) so no DB hit occurs; implement a conditional branch that checks for
teamId before calling the repository and assigns the result to the team variable
only when present.
anikdhabal
deleted the
devin/allow-org-owners-billing-access-1756486446
branch
4 participants
What does this PR do?
This PR enables organization owners and admins to access their shared Stripe billing portal instead of being limited to individual user billing. When a user accesses billing from within an organization context, the system now uses the organization's shared Stripe customer ID to create the billing portal session.
Key Changes:
/api/integrations/stripepayment/portalendpoint to accept optionalteamIdparameterteamIdwhen in organization contexthttps://www.loom.com/share/7c8334ec9d0a4199a584430d52971b9e
Link to Devin run: https://app.devin.ai/sessions/0276b9035821431f9cc890b5b5cfca7b
Requested by: @joeauyeung
How should this be tested?
Test Scenarios:
Environment Setup:
Critical Review Areas
🔍 High Priority Review Items:
portal.ts:20-27): Verify the permission check correctly identifies organization owners vs just team adminsportal.ts:31-40): Review Stripe API error handling for edge cases (timeouts, invalid subscriptions)portal.ts:17): TheparseInt(req.query.teamId as string)conversion needs validationbilling-view.tsx:48): The pathname splitting logic could be fragile with URL changes🛡️ Security Considerations:
Mandatory Tasks (DO NOT REMOVE)
Checklist