Skip to content
Merged
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
5 changes: 5 additions & 0 deletions .changeset/heavy-suns-divide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Fixes a bug where billing hooks would attempt to fetch billing information for an organization member with insufficient permissions, resulting in a 403 error.
5 changes: 5 additions & 0 deletions .changeset/thin-swans-punch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/shared': minor
---

Billing hooks now accept a `{ enabled: boolean }` option, that controls the whether or not a request will fire.
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ describe('OrganizationProfile', () => {

render(<OrganizationProfile />, { wrapper });
await waitFor(() => expect(screen.queryByText('Billing')).toBeNull());
expect(fixtures.clerk.billing.getSubscription).toHaveBeenCalled();
expect(fixtures.clerk.billing.getStatements).toHaveBeenCalled();
expect(fixtures.clerk.billing.getSubscription).not.toHaveBeenCalled();
expect(fixtures.clerk.billing.getStatements).not.toHaveBeenCalled();
});

it('does not include Billing when missing billing permission even with paid plans', async () => {
Expand All @@ -109,9 +109,8 @@ describe('OrganizationProfile', () => {
render(<OrganizationProfile />, { wrapper });
await waitFor(() => expect(screen.queryByText('Billing')).toBeNull());

// TODO(@RQ_MIGRATION): Offer a way to disable these, because they fire unnecessary requests.
expect(fixtures.clerk.billing.getSubscription).toHaveBeenCalled();
expect(fixtures.clerk.billing.getStatements).toHaveBeenCalled();
expect(fixtures.clerk.billing.getSubscription).not.toHaveBeenCalled();
expect(fixtures.clerk.billing.getStatements).not.toHaveBeenCalled();
});
it('does not include Billing when organization billing is disabled', async () => {
const { wrapper, fixtures } = await createFixtures(f => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,11 @@ describe('PricingTable - plans visibility', () => {
// Should show plans when signed out
expect(getByRole('heading', { name: 'Test Plan' })).toBeVisible();
});

// Ensure API args reflect user context by default
expect(fixtures.clerk.billing.getPlans).toHaveBeenCalledWith(expect.objectContaining({ for: 'user' }));
// Signed-out users should not fetch subscriptions
expect(fixtures.clerk.billing.getSubscription).not.toHaveBeenCalled();
});

it('shows no plans when user is signed in but subscription is null', async () => {
Expand Down Expand Up @@ -517,7 +522,15 @@ describe('PricingTable - plans visibility', () => {
const { wrapper, fixtures, props } = await createFixtures(f => {
f.withBilling();
f.withOrganizations();
f.withUser({ email_addresses: ['test@clerk.com'], organization_memberships: ['Org1'] });
f.withUser({
email_addresses: ['test@clerk.com'],
organization_memberships: [
{
name: 'Org1',
permissions: ['org:sys_billing:manage'],
},
],
});
});

// Set legacy prop via context provider
Expand Down Expand Up @@ -572,7 +585,15 @@ describe('PricingTable - plans visibility', () => {
const { wrapper, fixtures, props } = await createFixtures(f => {
f.withBilling();
f.withOrganizations();
f.withUser({ email_addresses: ['test@clerk.com'], organization_memberships: ['Org1'] });
f.withUser({
email_addresses: ['test@clerk.com'],
organization_memberships: [
{
name: 'Org1',
permissions: ['org:sys_billing:read'],
},
],
});
});

// Set new prop via context provider
Expand Down
55 changes: 28 additions & 27 deletions packages/clerk-js/src/ui/contexts/components/Plans.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
__experimental_useStatements,
__experimental_useSubscription,
useClerk,
useOrganization,
useSession,
} from '@clerk/shared/react';
import type {
Expand All @@ -15,6 +16,7 @@ import type {
} from '@clerk/shared/types';
import { useCallback, useMemo } from 'react';

import { useProtect } from '@/ui/common/Gate';
import { getClosestProfileScrollBox } from '@/ui/utils/getClosestProfileScrollBox';

import type { LocalizationKey } from '../../localization';
Expand All @@ -28,44 +30,42 @@ export function normalizeFormatted(formatted: string) {
return formatted.endsWith('.00') ? formatted.slice(0, -3) : formatted;
}

// TODO(@COMMERCE): Rename payment sources to payment methods at the API level
export const usePaymentMethods = () => {
const useBillingHookParams = () => {
const subscriberType = useSubscriberTypeContext();
return __experimental_usePaymentMethods({
const { organization } = useOrganization();
const allowBillingRoutes = useProtect(
has =>
has({
permission: 'org:sys_billing:read',
}) || has({ permission: 'org:sys_billing:manage' }),
);

return {
for: subscriberType,
initialPage: 1,
pageSize: 10,
keepPreviousData: true,
});
// If the user is in an organization, only fetch billing data if they have the necessary permissions
enabled: subscriberType === 'organization' ? Boolean(organization) && allowBillingRoutes : true,
};
};

export const usePaymentMethods = () => {
const params = useBillingHookParams();
return __experimental_usePaymentMethods(params);
};

export const usePaymentAttempts = () => {
const subscriberType = useSubscriberTypeContext();
return __experimental_usePaymentAttempts({
for: subscriberType,
initialPage: 1,
pageSize: 10,
keepPreviousData: true,
});
const params = useBillingHookParams();
return __experimental_usePaymentAttempts(params);
};

export const useStatements = (params?: { mode: 'cache' }) => {
const subscriberType = useSubscriberTypeContext();
return __experimental_useStatements({
for: subscriberType,
initialPage: 1,
pageSize: 10,
keepPreviousData: true,
__experimental_mode: params?.mode,
});
export const useStatements = (externalParams?: { mode: 'cache' }) => {
const params = useBillingHookParams();
return __experimental_useStatements({ ...params, __experimental_mode: externalParams?.mode });
};

export const useSubscription = () => {
const subscriberType = useSubscriberTypeContext();
const subscription = __experimental_useSubscription({
for: subscriberType,
keepPreviousData: true,
});
const params = useBillingHookParams();
const subscription = __experimental_useSubscription(params);
const subscriptionItems = useMemo(
() => subscription.data?.subscriptionItems || [],
[subscription.data?.subscriptionItems],
Expand All @@ -85,6 +85,7 @@ export const usePlans = (params?: { mode: 'cache' }) => {
initialPage: 1,
pageSize: 50,
keepPreviousData: true,
enabled: true,
__experimental_mode: params?.mode,
});
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,26 @@ describe('createBillingPaginatedHook', () => {
expect(result.current.isFetching).toBe(false);
});

it('does not fetch when enabled is false', () => {
const { result } = renderHook(() => useDummyAuth({ initialPage: 1, pageSize: 4, enabled: false }), { wrapper });

expect(useFetcherMock).toHaveBeenCalledWith('user');

expect(fetcherMock).not.toHaveBeenCalled();
expect(result.current.isLoading).toBe(false);
expect(result.current.isFetching).toBe(false);
expect(result.current.data).toEqual([]);
});

it('fetches when enabled is true', async () => {
const { result } = renderHook(() => useDummyAuth({ initialPage: 1, pageSize: 4, enabled: true }), { wrapper });

await waitFor(() => expect(result.current.isLoading).toBe(false));
expect(useFetcherMock).toHaveBeenCalledWith('user');
expect(fetcherMock).toHaveBeenCalled();
expect(fetcherMock.mock.calls[0][0]).toStrictEqual({ initialPage: 1, pageSize: 4 });
});

it('authenticated hook: does not fetch when user is null', () => {
mockUser = null;

Expand Down
15 changes: 12 additions & 3 deletions packages/shared/src/react/hooks/createBillingPaginatedHook.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,23 @@ export function createBillingPaginatedHook<TResource extends ClerkResource, TPar
useFetcher,
options,
}: BillingHookConfig<TResource, TParams>) {
type HookParams = PaginatedHookConfig<PagesOrInfiniteOptions> & {
type HookParams = PaginatedHookConfig<
PagesOrInfiniteOptions & {
/**
* If `true`, a request will be triggered when the hook is mounted.
*
* @default true
*/
enabled?: boolean;
}
> & {
for?: ForPayerType;
};

return function useBillingHook<T extends HookParams>(
params?: T,
): PaginatedResources<TResource, T extends { infinite: true } ? true : false> {
const { for: _for, ...paginationParams } = params || ({} as Partial<T>);
const { for: _for, enabled: externalEnabled, ...paginationParams } = params || ({} as Partial<T>);

const safeFor = _for || 'user';

Expand Down Expand Up @@ -90,7 +99,7 @@ export function createBillingPaginatedHook<TResource extends ClerkResource, TPar
? environment?.commerceSettings.billing.organization.enabled
: environment?.commerceSettings.billing.user.enabled;

const isEnabled = !!hookParams && clerk.loaded && !!billingEnabled;
const isEnabled = !!hookParams && clerk.loaded && !!billingEnabled && (externalEnabled ?? true);

const result = usePagesOrInfinite<TParams, ClerkPaginatedResponse<TResource>>(
(hookParams || {}) as TParams,
Expand Down
10 changes: 6 additions & 4 deletions packages/shared/src/react/hooks/useSubscription.rq.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
} from '../contexts';
import type { SubscriptionResult, UseSubscriptionParams } from './useSubscription.types';

const hookName = 'useSubscription';
const HOOK_NAME = 'useSubscription';

/**
* This is the new implementation of useSubscription using React Query.
Expand All @@ -21,7 +21,7 @@ const hookName = 'useSubscription';
* @internal
*/
export function useSubscription(params?: UseSubscriptionParams): SubscriptionResult {
useAssertWrappedByClerkProvider(hookName);
useAssertWrappedByClerkProvider(HOOK_NAME);

const clerk = useClerkInstanceContext();
const user = useUserContext();
Expand All @@ -30,7 +30,7 @@ export function useSubscription(params?: UseSubscriptionParams): SubscriptionRes
// @ts-expect-error `__unstable__environment` is not typed
const environment = clerk.__unstable__environment as unknown as EnvironmentResource | null | undefined;

clerk.telemetry?.record(eventMethodCalled(hookName));
clerk.telemetry?.record(eventMethodCalled(HOOK_NAME));

const isOrganization = params?.for === 'organization';
const billingEnabled = isOrganization
Expand All @@ -49,14 +49,16 @@ export function useSubscription(params?: UseSubscriptionParams): SubscriptionRes
];
}, [user?.id, isOrganization, organization?.id]);

const queriesEnabled = Boolean(user?.id && billingEnabled) && (params?.enabled ?? true);

const query = useClerkQuery({
queryKey,
queryFn: ({ queryKey }) => {
const obj = queryKey[1] as { args: { orgId?: string } };
return clerk.billing.getSubscription(obj.args);
},
staleTime: 1_000 * 60,
enabled: Boolean(user?.id && billingEnabled) && ((params as any)?.enabled ?? true),
enabled: queriesEnabled,
// TODO(@RQ_MIGRATION): Add support for keepPreviousData
});

Expand Down
3 changes: 2 additions & 1 deletion packages/shared/src/react/hooks/useSubscription.swr.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ export function useSubscription(params?: UseSubscriptionParams): SubscriptionRes
const billingEnabled = isOrganization
? environment?.commerceSettings.billing.organization.enabled
: environment?.commerceSettings.billing.user.enabled;
const isEnabled = (params?.enabled ?? true) && billingEnabled;

const swr = useSWR(
billingEnabled
isEnabled
? {
type: 'commerce-subscription',
userId: user?.id,
Expand Down
6 changes: 6 additions & 0 deletions packages/shared/src/react/hooks/useSubscription.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ export type UseSubscriptionParams = {
* Defaults to false.
*/
keepPreviousData?: boolean;
/**
* If `true`, a request will be triggered when the hook is mounted.
*
* @default true
*/
enabled?: boolean;
};

export type SubscriptionResult = {
Expand Down
Loading