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
47 changes: 43 additions & 4 deletions ghost/core/core/server/services/gifts/gift-bookshelf-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ type BookshelfDocument<T> = {

type BookshelfModel<T> = {
add(data: Partial<T>, unfilteredOptions?: unknown): Promise<T>;
findOne(data: Record<string, unknown>, unfilteredOptions?: unknown): Promise<BookshelfDocument<T> | null>;
edit(data: Partial<T>, unfilteredOptions?: unknown): Promise<BookshelfDocument<T>>;
findOne(data: Record<string, unknown>, options: {require: true}): Promise<BookshelfDocument<T>>;
findOne(data: Record<string, unknown>, options?: {require?: false}): Promise<BookshelfDocument<T> | null>;
};

type GiftRow = {
id: string;
token: string;
buyer_email: string;
buyer_member_id: string | null;
Expand Down Expand Up @@ -73,11 +76,47 @@ export class GiftBookshelfRepository implements GiftRepository {
});
}

async update(gift: Gift): Promise<void> {
const existing = await this.model.findOne({token: gift.token}, {require: true});

const id = existing.toJSON().id;

await this.model.edit({
token: gift.token,
buyer_email: gift.buyerEmail,
buyer_member_id: gift.buyerMemberId,
redeemer_member_id: gift.redeemerMemberId,
tier_id: gift.tierId,
cadence: gift.cadence,
duration: gift.duration,
currency: gift.currency,
amount: gift.amount,
stripe_checkout_session_id: gift.stripeCheckoutSessionId,
stripe_payment_intent_id: gift.stripePaymentIntentId,
consumes_at: gift.consumesAt,
expires_at: gift.expiresAt,
status: gift.status,
purchased_at: gift.purchasedAt,
redeemed_at: gift.redeemedAt,
consumed_at: gift.consumedAt,
expired_at: gift.expiredAt,
refunded_at: gift.refundedAt
}, {id});
}

async getByToken(token: string): Promise<Gift | null> {
const model = await this.model.findOne({
token
}, {require: false});
return this.toGift(
await this.model.findOne({token}, {require: false})
);
}

async getByPaymentIntentId(paymentIntentId: string): Promise<Gift | null> {
return this.toGift(
await this.model.findOne({stripe_payment_intent_id: paymentIntentId}, {require: false})
);
}

private toGift(model: BookshelfDocument<GiftRow> | null): Gift | null {
if (!model) {
return null;
}
Expand Down
2 changes: 2 additions & 0 deletions ghost/core/core/server/services/gifts/gift-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,7 @@ import type {Gift} from './gift';
export interface GiftRepository {
existsByCheckoutSessionId(checkoutSessionId: string): Promise<boolean>;
create(gift: Gift): Promise<void>;
update(gift: Gift): Promise<void>;
getByToken(token: string): Promise<Gift | null>;
getByPaymentIntentId(paymentIntentId: string): Promise<Gift | null>;
}
21 changes: 21 additions & 0 deletions ghost/core/core/server/services/gifts/gift-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,27 @@
return true;
}

async refundGift(paymentIntentId: string): Promise<boolean> {
const gift = await this.giftRepository.getByPaymentIntentId(paymentIntentId);

if (!gift) {
return false;
}

const refunded = gift.markRefunded();

if (!refunded) {
return true;
}

await this.giftRepository.update(gift);

// TODO: if the gift was already redeemed/consumed, we should also

Check warning on line 185 in ghost/core/core/server/services/gifts/gift-service.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Complete the task associated to this "TODO" comment.

See more on https://sonarcloud.io/project/issues?id=TryGhost_Ghost&issues=AZ1z8BW7-rAd27ODl7tT&open=AZ1z8BW7-rAd27ODl7tT&pullRequest=27321
// downgrade the recipient member back to free.
Comment on lines +185 to +186
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.

⚠️ Potential issue | 🟠 Major

Refunded redeemed gifts can keep paid access (missing downgrade path).

Line 185 leaves a functional gap: if a gift was already redeemed/consumed, refunding only the gift record can leave member entitlement inconsistent with billing state. Please implement the downgrade/reconciliation path (with tests) before release.

I can help draft the downgrade flow and the corresponding unit/e2e test cases if you want me to open a follow-up issue outline.

🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis

[warning] 185-185: Complete the task associated to this "TODO" comment.

See more on https://sonarcloud.io/project/issues?id=TryGhost_Ghost&issues=AZ1z8BW7-rAd27ODl7tT&open=AZ1z8BW7-rAd27ODl7tT&pullRequest=27321

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ghost/core/core/server/services/gifts/gift-service.ts` around lines 185 -
186, When a gift refund is processed in gift-service.ts, add a reconciliation
path that detects if the gift has been redeemed/consumed and, if so, revokes the
granted entitlement from the recipient member and downgrades them back to free;
locate the refund handler (e.g., the method that marks a gift refunded or
processesRefund/refundGift) and after marking the gift refunded call the
members/subscription API (e.g., MemberService.revokePaidAccess or
SubscriptionService.downgradeMemberToFree) to remove the paid access, persist
the change, and emit the appropriate events/audit log; add unit tests and an e2e
test that simulates a redeemed gift being refunded and asserts the member loses
paid access and billing/subscription state is consistent.


return true;
}

async getRedeemableGiftByToken({token, currentMember}: {token: string; currentMember?: {status: string} | null}) {
if (!this.labsService.isSet('giftSubscriptions')) {
throw new errors.BadRequestError({
Expand Down
11 changes: 11 additions & 0 deletions ghost/core/core/server/services/gifts/gift.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,15 @@ export class Gift {

return {redeemable: true};
}

markRefunded(): boolean {
if (this.isRefunded()) {
return false;
}

this.status = 'refunded';
this.refundedAt = new Date();

return true;
Comment on lines +144 to +152
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.

⚠️ Potential issue | 🟠 Major

Refunds can leave the gift in conflicting terminal states.

markRefunded() only updates status/refundedAt. If a charge is refunded after the gift was already redeemed, consumed, or expired, those timestamps remain set, and checkRedeemable() will still return 'redeemed', 'consumed', or 'expired' because those checks run before isRefunded(). Please make the refund transition exclusive, or give refunded precedence when evaluating redeemability.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ghost/core/core/server/services/gifts/gift.ts` around lines 144 - 152,
markRefunded() currently only sets status and refundedAt which can leave
redeemedAt/consumedAt/expiredAt set and cause checkRedeemable() to still return
'redeemed'/'consumed'/'expired'; update markRefunded() to make the refund
transition exclusive by clearing other terminal timestamps (e.g., redeemedAt,
consumedAt, expiredAt) and any related flags when setting status = 'refunded'
and refundedAt, and/or modify checkRedeemable() to give isRefunded() precedence
(evaluate refunded before redeemed/consumed/expired) so refunded state always
wins.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
const logging = require('@tryghost/logging');

/**
* Handles `charge.refunded` webhook events
*
* When a charge is refunded in Stripe, this service delegates to the
* appropriate handler based on the type of charge.
*/
module.exports = class ChargeRefundedEventService {
/**
* @param {object} deps
* @param {object} deps.giftService
*/
constructor(deps) {
this.deps = deps;
}

/**
* Handles a `charge.refunded` event
*
* Extracts the payment intent ID from the charge and delegates to
* type-specific handlers.
*
* @param {import('stripe').Stripe.Charge} charge
*/
async handleEvent(charge) {
// payment_intent can be a string ID or an expanded PaymentIntent object
const raw = charge.payment_intent;
const paymentIntentId = typeof raw === 'string' ? raw : raw?.id;

if (!paymentIntentId) {
logging.info('charge.refunded: no payment_intent on charge, skipping');

return;
}

// One-time payments (gifts, donations) have no invoice
if (charge.invoice === null) {
await this.handleGiftRefundEvent(paymentIntentId);
}
}

/**
* Handles a refund for a gift subscription purchase
*
* Looks up the gift by payment intent ID and marks it as refunded.
* If no gift matches, the refund is for something else and is ignored.
*
* @param {string} paymentIntentId
* @private
*/
async handleGiftRefundEvent(paymentIntentId) {
const refunded = await this.deps.giftService.refundGift(paymentIntentId);

if (!refunded) {
logging.info(`charge.refunded: no gift found for payment_intent ${paymentIntentId}, skipping`);
}
}
};
10 changes: 9 additions & 1 deletion ghost/core/core/server/services/stripe/stripe-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const {StripeLiveEnabledEvent, StripeLiveDisabledEvent} = require('./events');
const SubscriptionEventService = require('./services/webhook/subscription-event-service');
const InvoiceEventService = require('./services/webhook/invoice-event-service');
const CheckoutSessionEventService = require('./services/webhook/checkout-session-event-service');
const ChargeRefundedEventService = require('./services/webhook/charge-refunded-event-service');
const memberWelcomeEmailService = require('../member-welcome-emails/service');

/**
Expand Down Expand Up @@ -134,11 +135,18 @@ module.exports = class StripeService {
}
});

const chargeRefundedEventService = new ChargeRefundedEventService({
get giftService() {
return giftService.service;
}
});

const webhookController = new WebhookController({
webhookManager,
subscriptionEventService,
invoiceEventService,
checkoutSessionEventService
checkoutSessionEventService,
chargeRefundedEventService
});

this.models = models;
Expand Down
14 changes: 13 additions & 1 deletion ghost/core/core/server/services/stripe/webhook-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,21 @@ module.exports = class WebhookController {
* @param {import('./services/webhook/checkout-session-event-service')} deps.checkoutSessionEventService
* @param {import('./services/webhook/subscription-event-service')} deps.subscriptionEventService
* @param {import('./services/webhook/invoice-event-service')} deps.invoiceEventService
* @param {import('./services/webhook/charge-refunded-event-service')} deps.chargeRefundedEventService
*/
constructor(deps) {
this.checkoutSessionEventService = deps.checkoutSessionEventService;
this.subscriptionEventService = deps.subscriptionEventService;
this.invoiceEventService = deps.invoiceEventService;
this.chargeRefundedEventService = deps.chargeRefundedEventService;
this.webhookManager = deps.webhookManager;
this.handlers = {
'customer.subscription.deleted': this.subscriptionEvent,
'customer.subscription.updated': this.subscriptionEvent,
'customer.subscription.created': this.subscriptionEvent,
'invoice.payment_succeeded': this.invoiceEvent,
'checkout.session.completed': this.checkoutSessionEvent
'checkout.session.completed': this.checkoutSessionEvent,
'charge.refunded': this.chargeRefundedEvent
};
}

Expand Down Expand Up @@ -145,4 +148,13 @@ module.exports = class WebhookController {
async checkoutSessionEvent(session) {
await this.checkoutSessionEventService.handleEvent(session);
}

/**
* Delegates `charge.refunded` events to the `chargeRefundedEventService`
* @param {import('stripe').Stripe.Charge} charge
* @private
*/
async chargeRefundedEvent(charge) {
await this.chargeRefundedEventService.handleEvent(charge);
}
};
3 changes: 2 additions & 1 deletion ghost/core/core/server/services/stripe/webhook-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ module.exports = class WebhookManager {
'customer.subscription.deleted',
'customer.subscription.updated',
'customer.subscription.created',
'invoice.payment_succeeded'
'invoice.payment_succeeded',
'charge.refunded'
];

/**
Expand Down
114 changes: 107 additions & 7 deletions ghost/core/test/e2e-api/members/gift-subscriptions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,28 @@ describe('Gift Subscriptions', function () {
assert.equal(gift.get('status'), 'purchased');
});

it('Handles Stripe webhook idempotency', async function () {
it('Includes gift token in the purchase success URL', async function () {
const paidTier = await getPaidTier();

await membersAgent.post('/api/create-stripe-checkout-session/')
.body({
type: 'gift',
tierId: paidTier.id,
cadence: 'month',
customerEmail: 'url-test-buyer@example.com',
metadata: {}
})
.expectStatus(200);

const checkoutSession = getLatestCheckoutSession();
const successUrl = checkoutSession.success_url;

assert.ok(successUrl, 'Should have a success URL');
assert.ok(successUrl.includes('stripe=gift-purchase-success'), 'Success URL should contain stripe=gift-purchase-success');
assert.ok(successUrl.includes(`gift_token=${checkoutSession.metadata.gift_token}`), 'Success URL should contain the gift token');
});

it('Handles Stripe webhook idempotency for gift purchases', async function () {
const paidTier = await getPaidTier();

await membersAgent.post('/api/create-stripe-checkout-session/')
Expand Down Expand Up @@ -258,24 +279,103 @@ describe('Gift Subscriptions', function () {
await expectGiftCheckoutError({customerEmail: 'not-an-email'});
});

it('Includes gift token in the success URL', async function () {
it('Marks gift as refunded when Stripe charge.refunded webhook is received', async function () {
const paidTier = await getPaidTier();

await membersAgent.post('/api/create-stripe-checkout-session/')
.body({
type: 'gift',
tierId: paidTier.id,
cadence: 'month',
customerEmail: 'url-test-buyer@example.com',
customerEmail: 'refund-buyer@example.com',
metadata: {}
})
.expectStatus(200);

const checkoutSession = getLatestCheckoutSession();
const successUrl = checkoutSession.success_url;
const paymentIntentId = 'pi_refund_test_789';

assert.ok(successUrl, 'Should have a success URL');
assert.ok(successUrl.includes('stripe=gift-purchase-success'), 'Success URL should contain stripe=gift-purchase-success');
assert.ok(successUrl.includes(`gift_token=${checkoutSession.metadata.gift_token}`), 'Success URL should contain the gift token');
// Complete the gift purchase via webhook
await stripeMocker.sendWebhook({
type: 'checkout.session.completed',
data: {
object: {
id: checkoutSession.id,
mode: 'payment',
amount_total: paidTier.monthly_price,
currency: paidTier.currency.toLowerCase(),
customer: checkoutSession.customer,
metadata: toWebhookMetadata(checkoutSession.metadata),
payment_intent: paymentIntentId
}
}
});

await DomainEvents.allSettled();

// Verify the gift was created
const gift = await models.Gift.findOne({
token: checkoutSession.metadata.gift_token
}, {require: true});

assert.equal(gift.get('status'), 'purchased');

// Send charge.refunded webhook
await stripeMocker.sendWebhook({
type: 'charge.refunded',
data: {
object: {
id: 'ch_refund_test',
payment_intent: paymentIntentId,
invoice: null
}
}
});

await DomainEvents.allSettled();

// Verify the gift is now refunded
const refundedGift = await models.Gift.findOne({
token: checkoutSession.metadata.gift_token
}, {require: true});

assert.equal(refundedGift.get('status'), 'refunded');
assert.ok(refundedGift.get('refunded_at'));
});

it('Ignores Stripe charge.refunded webhook for non-gift charges', async function () {
// Send a charge.refunded webhook with a payment_intent that doesn't match any gift
await stripeMocker.sendWebhook({
type: 'charge.refunded',
data: {
object: {
id: 'ch_non_gift',
payment_intent: 'pi_non_gift_charge',
invoice: null
}
}
});

await DomainEvents.allSettled();

// No error thrown, webhook handled gracefully
});

it('Ignores Stripe charge.refunded webhook for subscription charges', async function () {
// Send a charge.refunded webhook with an invoice (subscription charge)
await stripeMocker.sendWebhook({
type: 'charge.refunded',
data: {
object: {
id: 'ch_sub_refund',
payment_intent: 'pi_sub_charge',
invoice: 'in_123'
}
}
});

await DomainEvents.allSettled();

// No error thrown, webhook handled gracefully
});
});
Loading
Loading