Skip to content
Closed
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
34 changes: 34 additions & 0 deletions apps/backend/src/api/routes/billing.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ export class BillingController {

@Get('/portal')
async modifyPayment(@GetOrgFromRequest() org: Organization) {
// Admin-granted/never-subscribed orgs have no real `cus_...` customer, so
// there is no Stripe billing portal to open (the UI hides the button; this
// guards the endpoint against a "No such customer" Stripe error).
if (!org.paymentId?.startsWith('cus_')) {
throw new HttpException('No Stripe customer for this organization', 400);
}
const customer = await this._stripeService.getCustomerByOrganizationId(
org.id
);
Expand All @@ -116,6 +122,13 @@ export class BillingController {
@GetUserFromRequest() user: User,
@Body() body: { feedback: string }
) {
// An admin-granted subscription has no Stripe subscription to cancel (its
// paymentId is the user id, not a `cus_...` customer). The UI hides the
// self-service cancel button for these; removal is an admin-only action.
if (!org.paymentId?.startsWith('cus_')) {
throw new HttpException('No Stripe subscription to cancel', 400);
}

await this._notificationService.sendEmail(
process.env.EMAIL_FROM_ADDRESS,
'Subscription Cancelled',
Expand Down Expand Up @@ -168,9 +181,30 @@ export class BillingController {
throw new HttpException('Unauthorized', 400);
}

// Admin-granted subscriptions have no Stripe subscription to cancel; the
// admin panel hides this and offers "Remove Subscription" instead.
if (!org.paymentId?.startsWith('cus_')) {
throw new HttpException(
'Admin-granted subscription; use remove-subscription instead',
400
);
}

return this._stripeService.cancelSubscription(org.id);
}

@Post('/remove-subscription')
async removeSubscription(
@GetUserFromRequest() user: User,
@GetOrgFromRequest() org: Organization
) {
if (!user.isSuperAdmin) {
throw new HttpException('Unauthorized', 400);
}

return this._stripeService.removeAdminGrantedSubscription(org.id);
}

@Get('/chatbase-refund/preview')
chatbaseRefundPreview(@GetOrgFromRequest() org: Organization) {
return this._stripeService.chatbaseRefundPreview(org.id);
Expand Down
171 changes: 169 additions & 2 deletions apps/backend/src/api/routes/users.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
Controller,
Get,
HttpException,
Logger,
Post,
Query,
Req,
Expand All @@ -16,6 +17,8 @@ import { GetOrgFromRequest } from '@gitroom/nestjs-libraries/user/org.from.reque
import { StripeService } from '@gitroom/nestjs-libraries/services/stripe.service';
import { Response, Request } from 'express';
import { AuthService } from '@gitroom/backend/services/auth/auth.service';
import { AuthService as AuthChecker } from '@gitroom/helpers/auth/auth.service';
import { NotificationService } from '@gitroom/nestjs-libraries/database/prisma/notifications/notification.service';
import { OrganizationService } from '@gitroom/nestjs-libraries/database/prisma/organizations/organization.service';
import { CheckPolicies } from '@gitroom/backend/services/auth/permissions/permissions.ability';
import { getCookieUrlFromDomain } from '@gitroom/helpers/subdomain/subdomain.management';
Expand Down Expand Up @@ -44,9 +47,12 @@ export class UsersController {
private _authService: AuthService,
private _orgService: OrganizationService,
private _userService: UsersService,
private _trackService: TrackService
private _trackService: TrackService,
private _notificationService: NotificationService
) {}

private readonly _logger = new Logger(UsersController.name);

@Get('/chatbase-token')
async getChatbaseToken(
@GetUserFromRequest() user: User,
Expand Down Expand Up @@ -122,12 +128,24 @@ export class UsersController {
role: organization?.users[0]?.role,
// @ts-ignore
isLifetime: !!organization?.subscription?.isLifetime,
// An admin-granted subscription stores the user id in `paymentId` rather
// than a real Stripe `cus_...` customer; used to expose an admin-only
// remove action without risking a real paid subscription.
adminGrantedSubscription:
// @ts-ignore
!!organization?.subscription &&
!organization?.paymentId?.startsWith('cus_'),
admin: !!user.isSuperAdmin,
impersonate: !!impersonate,
isTrailing: !process.env.STRIPE_PUBLISHABLE_KEY
? false
: organization?.isTrailing,
allowTrial: organization?.allowTrial,
// A real Stripe customer has a `cus_...` paymentId. Admin-granted
// subscriptions store the user id there instead, and never-subscribed
// orgs have none, so gate billing-portal UI on this to avoid calling
// Stripe with a non-existent customer ("No such customer").
hasStripeCustomer: !!organization?.paymentId?.startsWith('cus_'),
streakSince: organization?.streakSince || null,
publicApi:
// @ts-ignore
Expand All @@ -153,7 +171,7 @@ export class UsersController {
throw new HttpException('Unauthorized', 400);
}

return this._userService.getImpersonateUser(name);
return this._userService.getImpersonateUser(name, user.id);
}

@Post('/impersonate')
Comment on lines 171 to 177

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: When an admin is impersonating a user, the 'Switch User' search incorrectly includes the admin's own account, allowing them to swap credentials with the impersonated user.
Severity: HIGH

Suggested Fix

The original admin's user ID needs to be tracked throughout the impersonation session. The auth middleware should attach the original admin's user object to the request, for example as req.originalUser. This ID should then be passed to getImpersonateUser to ensure the admin's own account is excluded from the search results, preventing the switch.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: apps/backend/src/api/routes/users.controller.ts#L171-L177

Potential issue: When an admin impersonates a user, the auth middleware replaces
`req.user` with the impersonated user's data but does not preserve the original admin's
identity. Consequently, when the admin uses the 'Switch User' feature, the
`getImpersonateUser` function is called with the impersonated user's ID to exclude from
search results. This fails to exclude the admin's own account, allowing it to appear in
the search. If the admin selects their own account, the `POST /user/switch` endpoint
validation passes because it only checks against the impersonated user's ID, resulting
in a successful but unintended credential swap between the admin and the impersonated
user.

Also affects:

  • apps/backend/src/api/routes/users.controller.ts:204-270

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down Expand Up @@ -183,6 +201,155 @@ export class UsersController {
}
}

@Post('/switch')
async switchUser(
@GetUserFromRequest() user: User,
@Body('id') id: string,
@Req() req: Request
) {
if (!user.isSuperAdmin) {
throw new HttpException('Unauthorized', 400);
}

if (!id || id === user.id) {
throw new HttpException('Invalid user to switch to', 400);
}
Comment on lines +206 to +216

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The /user/switch endpoint lacks a check for an active impersonation session, allowing an admin to accidentally swap their own account credentials instead of an impersonated user's.
Severity: CRITICAL

Suggested Fix

Add a guard at the beginning of the switchUser function to verify that an impersonation session is active. Check for the presence of req.cookies.impersonate or req.headers.impersonate and throw an unauthorized exception if it's missing, ensuring the endpoint can only be used during impersonation.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: apps/backend/src/api/routes/users.controller.ts#L204-L216

Potential issue: The `switchUser` endpoint is designed to swap the credentials of an
impersonated user with another's. However, it lacks a check to ensure an impersonation
session is active. If a super admin calls this endpoint directly without an
`impersonate` cookie or header, the `user` object will be the admin's own. The endpoint
will then incorrectly swap the admin's account credentials with the target user's, which
is a destructive and unintended action. The code's intent is clarified by a comment
stating `"user is the impersonated account"`.


// `user` is the impersonated account; `id` is the account whose login it is
// being swapped with. The swap keeps both user ids (and therefore their
// organizations, subscriptions, channels and media) in place and only
// trades the login credentials, so each login now reaches the other's data.
const oldEmail = user.email;
const { kept, switched } = await this._userService.switchUser(user.id, id);

// Audit: `user` is the impersonated account, so read the real admin id from
// the underlying auth token.
const adminId = this.getRequestUserId(req);
this._logger.log(
`User login switch performed${
adminId ? ` by admin ${adminId}` : ''
}: account ${kept.id} login ${oldEmail} -> ${kept.email}; account ${
switched.id
} login ${kept.email} -> ${switched.email}`
);

// Mirror the swap onto Stripe: each org's customer now belongs to whoever
// logs in with the owner's new email.
await this.syncStripeCustomersEmail([kept, switched]);

// If a login ended up on a never-verified email, it must be re-verified, so
// send it a fresh activation link. Failures must not fail the request.
await Promise.all(
[kept, switched].map(async (account) => {
if (account.activated) {
return;
}
try {
await this._authService.resendActivationEmail(account.email);
} catch (err) {
this._logger.error(
`Failed to send activation email to ${account.email}`,
err
);
}
})
);

// Let both accounts know their login changed. The swap is already
// committed, so a notification failure must not fail the request.
try {
await Promise.all([
this.notifyUserOfSwitch(kept.id, kept.email),
this.notifyUserOfSwitch(switched.id, switched.email),
]);
} catch (err) {
this._logger.error('Failed to notify users after login switch', err);
}

return { success: true };
}

private getRequestUserId(req: Request): string | null {
try {
const auth = (req.headers.auth as string) || req.cookies?.auth;
const payload = AuthChecker.verifyJWT(auth) as { id?: string } | null;
return payload?.id || null;
} catch {
return null;
}
}

private async notifyUserOfSwitch(userId: string, email: string) {
const subject = 'Your Postiz login was changed';
const message =
`An administrator changed the login for your Postiz account. ` +
`You can now sign in using ${email}. ` +
`Your subscription and plan were not changed by this switch — if you ` +
`intended to cancel a subscription, please do that separately from your ` +
`billing settings.`;

const organizations = await this._orgService.getOrgsByUserId(userId);
await Promise.all(
organizations.map(async (org) => {
// In-app notifications are organization-scoped, so posting to a shared
// organization would show "your login changed" to co-members who were
// not part of the switch. Only post where this user is the sole member;
// the email below reaches the switched login specifically in every case.
const team = await this._orgService.getTeam(org.id);
if ((team?.users?.length || 0) > 1) {
return;
}
await this._notificationService.inAppNotification(
org.id,
subject,
message,
false,
false,
'info'
);
})
);

if (this._notificationService.hasEmailProvider()) {
await this._notificationService.sendEmail(email, subject, message);
}
}

private async syncStripeCustomersEmail(
accounts: { id: string; email: string }[]
) {
// Map each real Stripe customer to the login email it should now carry.
// Two guards for multi-user organizations:
// - Owner-only: only the org owner's (SUPERADMIN) login drives its billing
// email, so a non-owner member's switch never rewrites a shared org's
// receipts address.
// - Dedupe: each customer is updated once even when both switched accounts
// own the same organization (the first account processed wins), avoiding
// two concurrent, racing updates to the same customer.
// Only real Stripe customers are touched — admin-granted subscriptions store
// the user id in `paymentId` (not a `cus_...` id), so calling Stripe for
// them only produces failed requests.
const emailByCustomer = new Map<string, string>();
for (const account of accounts) {
const organizations = await this._orgService.getOrgsByUserId(account.id);
for (const org of organizations) {
const role = org.users?.[0]?.role;
if (
role === 'SUPERADMIN' &&
org.paymentId?.startsWith('cus_') &&
!emailByCustomer.has(org.paymentId)
) {
emailByCustomer.set(org.paymentId, account.email);
}
}
}
await Promise.all(
[...emailByCustomer].map(([customerId, email]) =>
this._stripeService.updateCustomerEmail(customerId, email)
)
);
}

@Post('/personal')
async changePersonal(
@GetUserFromRequest() user: User,
Expand Down
30 changes: 17 additions & 13 deletions apps/frontend/src/components/billing/main.billing.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -539,21 +539,25 @@ export const MainBillingComponent: FC<{
</div>
{!!subscription?.id && (
<div className="flex justify-center mt-[20px] gap-[10px]">
<Button onClick={updatePayment}>
{t(
'update_payment_method_invoices_history',
'Update Payment Method / Invoices History'
)}
</Button>
{isGeneral && !subscription?.cancelAt && (
<Button
className="bg-red-500"
loading={loading}
onClick={moveToCheckout('FREE')}
>
{t('cancel_subscription_1', 'Cancel subscription')}
{!!(user as any)?.hasStripeCustomer && (
<Button onClick={updatePayment}>
{t(
'update_payment_method_invoices_history',
'Update Payment Method / Invoices History'
)}
</Button>
)}
{isGeneral &&
!subscription?.cancelAt &&
!(user as any)?.adminGrantedSubscription && (
<Button
className="bg-red-500"
loading={loading}
onClick={moveToCheckout('FREE')}
>
{t('cancel_subscription_1', 'Cancel subscription')}
</Button>
)}
</div>
)}
{subscription?.cancelAt && isGeneral && (
Expand Down
Loading
Loading