Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions messages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -2305,6 +2305,8 @@
"store-release-validation-title-track-unknown": "Android store install detected, release track unknown.",
"test-preview": "Test preview",
"stripe-billing-portal-will-be-opened-in-a-new-tab": "Stripe billing portal will be opened in a new tab",
"stripe-checkout-will-be-opened-in-a-new-tab": "Stripe checkout will be opened in a new tab",
"popup-blocked-open-manually": "Your browser blocked the new tab. Confirm to open it.",
"subscribed-events": "Subscribed Events",
"subscribed": "Subscribed",
"subscribed-within-7-days": "Subscribed (within 7 days)",
Expand Down
27 changes: 8 additions & 19 deletions playwright/e2e/subscription-checkout.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,19 +181,6 @@ test.describe('Subscription Checkout', () => {
await page.addInitScript((nextOrgId) => {
localStorage.setItem('capgo_current_org_id', nextOrgId)
}, orgId)
await page.addInitScript(() => {
;(window as Window & { __lastOpenedUrl?: string | null }).__lastOpenedUrl = null
window.open = ((url?: string | URL | null) => {
const normalizedUrl = typeof url === 'string'
? url
: url instanceof URL
? url.toString()
: null
;(window as Window & { __lastOpenedUrl?: string | null }).__lastOpenedUrl = normalizedUrl
return null
}) as typeof window.open
})

await page.login('test@capgo.app', USER_PASSWORD)
await page.goto('/settings/organization/plans')

Expand All @@ -206,14 +193,16 @@ test.describe('Subscription Checkout', () => {
await expect(planCard.getByRole('button', { name: 'Upgrade' })).toBeEnabled()
await planCard.locator('[data-test="plan-action-button"]').click()

// Web checkout opens a confirm dialog with a real target=_blank link.
const confirmLink = page.getByRole('link', { name: 'Confirm' })
await expect(confirmLink).toBeVisible()
const checkoutUrl = await confirmLink.getAttribute('href')
expect(checkoutUrl).toBeTruthy()

const escapedStripeOrigin = STRIPE_EMULATOR_URL.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
let checkoutUrl = ''
await expect.poll(async () => {
checkoutUrl = await page.evaluate(() => (window as Window & { __lastOpenedUrl?: string | null }).__lastOpenedUrl ?? '')
return checkoutUrl
}).toMatch(new RegExp(`${escapedStripeOrigin}/checkout/`))
expect(checkoutUrl!).toMatch(new RegExp(`${escapedStripeOrigin}/checkout/`))

await page.goto(checkoutUrl)
await page.goto(checkoutUrl!)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Reading the Confirm link's href and navigating with page.goto bypasses the actual target=_blank popup this PR is meant to harden, so the new _blank open path and popup-blocker fallback are never e2e-covered. Consider clicking the link and asserting the resulting popup (e.g. const [popup] = await Promise.all([page.waitForEvent('popup'), confirmLink.click()]), then assert popup.url() matches the Stripe origin) so a regression in DialogV2's native-blank handling is caught.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At playwright/e2e/subscription-checkout.spec.ts, line 205:

<comment>Reading the Confirm link's href and navigating with page.goto bypasses the actual target=_blank popup this PR is meant to harden, so the new `_blank` open path and popup-blocker fallback are never e2e-covered. Consider clicking the link and asserting the resulting popup (e.g. `const [popup] = await Promise.all([page.waitForEvent('popup'), confirmLink.click()])`, then assert `popup.url()` matches the Stripe origin) so a regression in DialogV2's native-blank handling is caught.</comment>

<file context>
@@ -206,14 +193,16 @@ test.describe('Subscription Checkout', () => {
+    expect(checkoutUrl!).toMatch(new RegExp(`${escapedStripeOrigin}/checkout/`))
 
-    await page.goto(checkoutUrl)
+    await page.goto(checkoutUrl!)
     await expect(page).toHaveURL(new RegExp(`${escapedStripeOrigin}/checkout/`))
     await page.getByRole('button', { name: /^Pay / }).click()
</file context>

await expect(page).toHaveURL(new RegExp(`${escapedStripeOrigin}/checkout/`))
await page.getByRole('button', { name: /^Pay / }).click()

Expand Down
6 changes: 6 additions & 0 deletions src/components/DialogV2.vue
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ function handleButtonClick(button: DialogV2Button, event?: Event) {
return
}

// Let target=_blank open natively so browsers do not treat it as a blocked popup.
if (button.href && button.target === '_blank') {
close({ ...safeButton, skipNavigation: true })
return
}

const shouldPreventNavigation = button.href && (!mouseEvent || (mouseEvent.button === 0 && !hasModifier))
if (shouldPreventNavigation)
event?.preventDefault()
Expand Down
23 changes: 8 additions & 15 deletions src/pages/settings/organization/Plans.vue
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,6 @@ const isCreditsOnly = computed(() => {
return !org.paying && (org.trial_left ?? 0) <= 0 && (org.credit_available ?? 0) > 0
})

function isSafariBrowser() {
if (Capacitor.getPlatform() !== 'web')
return false
if (typeof navigator === 'undefined')
return false
const ua = navigator.userAgent
return /Version\/[\d.]+/.test(ua) && /Safari\//.test(ua) && !/Chrome|CriOS|FxiOS|OPiOS|Edg|Chromium/.test(ua)
}

async function prefetchStripeCheckoutUrl(plan: Database['public']['Tables']['plans']['Row'], isYear: boolean) {
if (!plan.stripe_id)
return
Expand Down Expand Up @@ -212,16 +203,18 @@ function trackPlanCheckoutStarted(plan: Database['public']['Tables']['plans']['R
}).catch()
}

async function openSafariStripeCheckout(plan: Database['public']['Tables']['plans']['Row'], isYear: boolean) {
async function openWebStripeCheckout(plan: Database['public']['Tables']['plans']['Row'], isYear: boolean) {
const url = await prefetchStripeCheckoutUrl(plan, isYear)
if (!url) {
toast.error('Cannot get your checkout')
return false
}

// Confirm dialog with a real <a href> so checkout opens under a fresh user gesture.
// Avoids popup blockers after the async Stripe session create.
dialogStore.openDialog({
title: t('open-in-new-tab'),
description: 'This will open Stripe to complete checkout.',
description: t('stripe-checkout-will-be-opened-in-a-new-tab'),
buttons: [
{
text: t('button-cancel'),
Expand All @@ -234,7 +227,7 @@ async function openSafariStripeCheckout(plan: Database['public']['Tables']['plan
href: url,
target: '_blank',
rel: 'noopener noreferrer',
handler: () => trackPlanCheckoutStarted(plan, isYear, 'safari_confirm'),
handler: () => trackPlanCheckoutStarted(plan, isYear, 'web_confirm'),
},
],
})
Expand All @@ -255,8 +248,8 @@ async function openChangePlan(plan: Database['public']['Tables']['plans']['Row']
isSubscribeLoading.value[index] = true
if (plan.stripe_id) {
const checkoutIsYearly = hasYearlyDiscount(plan) ? isYearly.value : false
if (isSafariBrowser()) {
const shouldContinue = await openSafariStripeCheckout(plan, checkoutIsYearly)
if (Capacitor.getPlatform() === 'web') {
Comment thread
riderx marked this conversation as resolved.
const shouldContinue = await openWebStripeCheckout(plan, checkoutIsYearly)
if (!shouldContinue) {
isSubscribeLoading.value[index] = false
return
Expand Down Expand Up @@ -592,7 +585,7 @@ function buttonStyle(p: Database['public']['Tables']['plans']['Row']) {
:disabled="isDisabled(p)"
@click="openChangePlan(p, index)"
>
<svg v-if="isSubscribeLoading[index]" class="w-4 h-4 text-white animate-spin" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24">
<svg v-if="isSubscribeLoading[index]" class="w-4 h-4 animate-spin" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24">
<circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4" />
<path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4zm2 5.291A7.962 7.962 0 014 12H0c0 3.042 1.135 5.824 3 7.938l3-2.647z" />
</svg>
Expand Down
39 changes: 34 additions & 5 deletions src/services/stripe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,42 @@ async function presentActionSheetOpen(url: string) {
})
return dialogStore.onDialogDismiss()
}
export function openBlank(link: string) {
async function presentBlockedPopupFallback(url: string) {
const { t } = useI18n()
const dialogStore = useDialogV2Store()

dialogStore.openDialog({
title: t('open-in-new-tab'),
description: t('popup-blocked-open-manually'),
buttons: [
{
text: t('button-cancel'),
role: 'cancel',
},
{
text: t('button-confirm'),
id: 'confirm-button',
role: 'primary',
href: url,
target: '_blank',
rel: 'noopener noreferrer',
},
],
})
return !(await dialogStore.onDialogDismiss())
}

export async function openBlank(link: string) {
console.log('openBlank', link)
if (Capacitor.getPlatform() === 'ios') {
presentActionSheetOpen(link)
return true
// presentActionSheetOpen resolves true when dismissed/canceled
return !(await presentActionSheetOpen(link))
}
return Boolean(globalThis.open(link, '_blank'))
const opened = globalThis.open(link, '_blank')
if (opened)
return true
Comment on lines +62 to +64

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- src/services/stripe.ts ---'
cat -n src/services/stripe.ts
printf '%s\n' '--- window.open usages ---'
rg -n -C 3 'globalThis\.open|window\.open|noopener|opener' src tests .github 2>/dev/null || true
printf '%s\n' '--- relevant package/runtime metadata ---'
rg -n -C 2 '"(lint|test|typecheck|build)"|vue|vite|vitest|playwright' package.json bun.lockb package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || true

Repository: Cap-go/capgo.app

Length of output: 36115


🌐 Web query:

MDN Window open target _blank implicit noopener window.opener browser compatibility

💡 Result:

In modern web browsers, setting target="_blank" on , , or

elements implicitly behaves as if rel="noopener" has been specified [1]. This means the opened browsing context will not have access to the opening window via the window.opener property, which will return null [2][1]. This implicit behavior was standardized to improve security by preventing the opened page from tampering with the original page via the window.opener reference [1][3]. If you explicitly require window.opener access, you must now use rel="opener" on the link [1][3]. Browser Compatibility: This implicit behavior is widely supported in modern browsers [4]. Support for target="_blank" implying rel="noopener" generally aligns with the adoption of this standard in major browser engines (Chromium 88+, Firefox 79+, and Safari 12.1+) [4]. Older browsers do not provide this implicit security, which is why developers previously had to manually add rel="noopener" to secure links [3][5]. Note: This implicit behavior applies to HTML anchor/form elements [1]. When using the window.open JavaScript method, the noopener behavior is not automatic and must still be explicitly requested by including "noopener" in the windowFeatures parameter [6][7].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- browser-target and compatibility configuration ---'
rg -n -i -C 2 'browserslist|browser|target|support|compatib' package.json vite.config.* .browserslistrc tsconfig*.json README.md docs 2>/dev/null || true
printf '%s\n' '--- tests and history for openBlank ---'
rg -n -C 5 'openBlank|presentBlockedPopupFallback|window\.opener' tests src scripts 2>/dev/null || true
git diff --stat
git log --oneline -8 -- src/services/stripe.ts

Repository: Cap-go/capgo.app

Length of output: 50375


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- browser support configuration ---'
for f in package.json vite.config.ts vite.config.js vite.config.mts .browserslistrc; do
  if [ -f "$f" ]; then
    printf '\n[%s]\n' "$f"
    rg -n -i 'browserslist|target|browser|support|compatib' "$f" || true
  fi
done
printf '%s\n' '--- focused openBlank history ---'
git log --oneline -12 -- src/services/stripe.ts
printf '%s\n' '--- focused tests/usages ---'
rg -n -C 4 'openBlank|presentActionSheetOpen|window\.opener' tests src/services src/pages/app/'[app]'.channel.'[channel]'.vue 2>/dev/null || true
printf '%s\n' '--- current diff summary ---'
git diff --stat

Repository: Cap-go/capgo.app

Length of output: 3274


🏁 Script executed (no clone):

node - <<'JS'
'use strict'

// Read-only behavioral model of the proposed operation.
// It checks that the popup-blocked branch still depends on the return value
// and that a successfully returned WindowProxy gets opener cleared.
function openBlankModel(open) {
  const opened = open('https://checkout.example', '_blank')
  if (opened) {
    opened.opener = null
    return true
  }
  return 'fallback'
}

const popup = { opener: { location: 'https://app.example' } }
console.log('opened:', openBlankModel(() => popup))
console.log('opener:', popup.opener)
console.log('blocked:', openBlankModel(() => null))
JS

Length of output: 169


🌐 Web query:

MDN Window.open noopener feature return value WindowProxy opener null

💡 Result:

When the noopener feature is used in the window.open() method, the method returns null instead of a WindowProxy object [1][2]. Additionally, the newly opened window will have its window.opener property set to null [3][4]. This behavior ensures that the new browsing context has no access to the originating window, which is a key security feature to prevent unauthorized access or tampering [4][2]. This behavior is consistent with the HTML specification, which dictates that window.open() returns null when the noopener option is provided, effectively preventing the caller from obtaining a reference to the opened window [5][6][7].

Citations:


Clear opened.opener before returning success. Set opened.opener = null when the popup opens. Keep the null-return fallback intact.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/stripe.ts` around lines 62 - 64, Update the popup-opening flow
around the opened result from globalThis.open to set opened.opener to null
before returning true; preserve the existing null-return fallback unchanged.

// Async callers often lose the user-gesture; offer a confirm link fallback.
return presentBlockedPopupFallback(link)
}
export async function openPortal(orgId: string, t: ComposerTranslation) {
let url = ''
Expand Down Expand Up @@ -70,7 +99,7 @@ export async function openPortal(orgId: string, t: ComposerTranslation) {
handler: async () => {
await prem
if (url)
openBlank(url)
await openBlank(url)
else
toast.error('Cannot open your portal')
},
Expand Down
Loading