From 6fb81e757b1b8bf24c35b419db76eae6372952f1 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Fri, 21 Feb 2025 15:01:36 +0100 Subject: [PATCH] [permissions] Add conditional permission gate on billing's checkoutSession (#10387) Following a conversation with @etiennejouan and @martmull, we are adding a permission gate on billing resolver's checkoutSession, which should only be accessible to entitled users or at workspace creation (when there are no roles yet), when the subscription is incomplete --- .../core-modules/billing/billing.resolver.ts | 56 +++++++++++++++++++ .../billing/services/billing.service.ts | 9 ++- ...ption-incomplete-onboarding-status.util.ts | 5 ++ .../onboarding/onboarding.service.ts | 13 ++--- .../workspace-manager.service.ts | 2 +- .../workspace.integration-spec.ts | 47 ++++++++++++++++ 6 files changed, 121 insertions(+), 11 deletions(-) create mode 100644 packages/twenty-server/src/engine/core-modules/billing/utils/is-subscription-incomplete-onboarding-status.util.ts diff --git a/packages/twenty-server/src/engine/core-modules/billing/billing.resolver.ts b/packages/twenty-server/src/engine/core-modules/billing/billing.resolver.ts index 208d69165..ada797d43 100644 --- a/packages/twenty-server/src/engine/core-modules/billing/billing.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/billing/billing.resolver.ts @@ -18,6 +18,7 @@ import { BillingPlanKey } from 'src/engine/core-modules/billing/enums/billing-pl import { BillingPlanService } from 'src/engine/core-modules/billing/services/billing-plan.service'; import { BillingPortalWorkspaceService } from 'src/engine/core-modules/billing/services/billing-portal.workspace-service'; import { BillingSubscriptionService } from 'src/engine/core-modules/billing/services/billing-subscription.service'; +import { BillingService } from 'src/engine/core-modules/billing/services/billing.service'; import { StripePriceService } from 'src/engine/core-modules/billing/stripe/services/stripe-price.service'; import { BillingPortalCheckoutSessionParameters } from 'src/engine/core-modules/billing/types/billing-portal-checkout-session-parameters.type'; import { formatBillingDatabaseProductToGraphqlDTO } from 'src/engine/core-modules/billing/utils/format-database-product-to-graphql-dto.util'; @@ -25,11 +26,18 @@ import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/featu import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service'; import { User } from 'src/engine/core-modules/user/user.entity'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; +import { AuthUserWorkspaceId } from 'src/engine/decorators/auth/auth-user-workspace-id.decorator'; import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator'; import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; import { SettingsPermissionsGuard } from 'src/engine/guards/settings-permissions.guard'; import { UserAuthGuard } from 'src/engine/guards/user-auth.guard'; import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; +import { + PermissionsException, + PermissionsExceptionCode, + PermissionsExceptionMessage, +} from 'src/engine/metadata-modules/permissions/permissions.exception'; +import { PermissionsService } from 'src/engine/metadata-modules/permissions/permissions.service'; import { PermissionsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter'; @Resolver() @@ -41,6 +49,8 @@ export class BillingResolver { private readonly stripePriceService: StripePriceService, private readonly billingPlanService: BillingPlanService, private readonly featureFlagService: FeatureFlagService, + private readonly billingService: BillingService, + private readonly permissionsService: PermissionsService, ) {} @Query(() => BillingProductPricesOutput) @@ -80,6 +90,7 @@ export class BillingResolver { async checkoutSession( @AuthWorkspace() workspace: Workspace, @AuthUser() user: User, + @AuthUserWorkspaceId() userWorkspaceId: string, @Args() { recurringInterval, @@ -88,6 +99,10 @@ export class BillingResolver { requirePaymentMethod, }: BillingCheckoutSessionInput, ) { + await this.validateCanCheckoutSessionPermissionOrThrow({ + workspaceId: workspace.id, + userWorkspaceId, + }); const isBillingPlansEnabled = await this.featureFlagService.isFeatureEnabled( FeatureFlagKey.IsBillingPlansEnabled, @@ -158,4 +173,45 @@ export class BillingResolver { return plans.map(formatBillingDatabaseProductToGraphqlDTO); } + + private async validateCanCheckoutSessionPermissionOrThrow({ + workspaceId, + userWorkspaceId, + }: { + workspaceId: string; + userWorkspaceId: string; + }) { + const isPermissionsEnabled = await this.featureFlagService.isFeatureEnabled( + FeatureFlagKey.IsPermissionsEnabled, + workspaceId, + ); + + if (!isPermissionsEnabled) { + return; + } + + if ( + await this.billingService.isSubscriptionIncompleteOnboardingStatus( + workspaceId, + ) + ) { + return; + } + + const userHasPermission = + await this.permissionsService.userHasWorkspaceSettingPermission({ + userWorkspaceId, + workspaceId, + _setting: SettingsFeatures.WORKSPACE, + }); + + if (!userHasPermission) { + throw new PermissionsException( + PermissionsExceptionMessage.PERMISSION_DENIED, + PermissionsExceptionCode.PERMISSION_DENIED, + ); + } + + return; + } } diff --git a/packages/twenty-server/src/engine/core-modules/billing/services/billing.service.ts b/packages/twenty-server/src/engine/core-modules/billing/services/billing.service.ts index 438a46e3e..515189344 100644 --- a/packages/twenty-server/src/engine/core-modules/billing/services/billing.service.ts +++ b/packages/twenty-server/src/engine/core-modules/billing/services/billing.service.ts @@ -10,7 +10,6 @@ import { BillingSubscription } from 'src/engine/core-modules/billing/entities/bi import { BillingEntitlementKey } from 'src/engine/core-modules/billing/enums/billing-entitlement-key.enum'; import { BillingSubscriptionService } from 'src/engine/core-modules/billing/services/billing-subscription.service'; import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service'; -import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service'; @Injectable() export class BillingService { @@ -18,7 +17,6 @@ export class BillingService { constructor( private readonly environmentService: EnvironmentService, private readonly billingSubscriptionService: BillingSubscriptionService, - private readonly isFeatureEnabledService: FeatureFlagService, @InjectRepository(BillingSubscription, 'core') private readonly billingSubscriptionRepository: Repository, ) {} @@ -56,4 +54,11 @@ export class BillingService { entitlementKey, ); } + + async isSubscriptionIncompleteOnboardingStatus(workspaceId: string) { + const hasAnySubscription = + await this.hasWorkspaceAnySubscription(workspaceId); + + return !hasAnySubscription; + } } diff --git a/packages/twenty-server/src/engine/core-modules/billing/utils/is-subscription-incomplete-onboarding-status.util.ts b/packages/twenty-server/src/engine/core-modules/billing/utils/is-subscription-incomplete-onboarding-status.util.ts new file mode 100644 index 000000000..ef5ccbc34 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/billing/utils/is-subscription-incomplete-onboarding-status.util.ts @@ -0,0 +1,5 @@ +export const isSubscriptionIncompleteOnboardingStatus = ( + hasAnySubscription: boolean, +) => { + return !hasAnySubscription; +}; diff --git a/packages/twenty-server/src/engine/core-modules/onboarding/onboarding.service.ts b/packages/twenty-server/src/engine/core-modules/onboarding/onboarding.service.ts index f8f813383..fbaff4554 100644 --- a/packages/twenty-server/src/engine/core-modules/onboarding/onboarding.service.ts +++ b/packages/twenty-server/src/engine/core-modules/onboarding/onboarding.service.ts @@ -27,13 +27,6 @@ export class OnboardingService { private readonly userVarsService: UserVarsService, ) {} - private async isSubscriptionIncompleteOnboardingStatus(workspace: Workspace) { - const hasAnySubscription = - await this.billingService.hasWorkspaceAnySubscription(workspace.id); - - return !hasAnySubscription; - } - private isWorkspaceActivationPending(workspace: Workspace) { return ( workspace.activationStatus === WorkspaceActivationStatus.PENDING_CREATION @@ -41,7 +34,11 @@ export class OnboardingService { } async getOnboardingStatus(user: User, workspace: Workspace) { - if (await this.isSubscriptionIncompleteOnboardingStatus(workspace)) { + if ( + await this.billingService.isSubscriptionIncompleteOnboardingStatus( + workspace.id, + ) + ) { return OnboardingStatus.PLAN_REQUIRED; } diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-manager.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-manager.service.ts index 6cb5d8b33..a5dec3a87 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-manager.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-manager.service.ts @@ -258,7 +258,7 @@ export class WorkspaceManagerService { await this.userRoleService.assignRoleToUserWorkspace({ workspaceId, - userWorkspaceId: userWorkspace[0].id, + userWorkspaceId: userWorkspace.id, roleId: adminRole.id, }); } diff --git a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace.integration-spec.ts index 17434de22..0bade6509 100644 --- a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace.integration-spec.ts @@ -4,6 +4,7 @@ import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graph import { updateFeatureFlagFactory } from 'test/integration/graphql/utils/update-feature-flag-factory.util'; import { SEED_APPLE_WORKSPACE_ID } from 'src/database/typeorm-seeds/core/workspaces'; +import { BillingPlanKey } from 'src/engine/core-modules/billing/enums/billing-plan-key.enum'; import { ErrorCode } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; import { PermissionsExceptionMessage } from 'src/engine/metadata-modules/permissions/permissions.exception'; @@ -406,6 +407,52 @@ describe('WorkspaceResolver', () => { }); }); }); + + describe('checkoutSession', () => { + it('should throw a permission error when user does not have permission (member role)', async () => { + const queryData = { + query: ` + mutation CheckoutSession( + $recurringInterval: SubscriptionInterval! + $successUrlPath: String! + $plan: BillingPlanKey! + $requirePaymentMethod: Boolean + ) { + checkoutSession( + recurringInterval: $recurringInterval + successUrlPath: $successUrlPath + plan: $plan + requirePaymentMethod: $requirePaymentMethod + ) { + url + } + } + `, + variables: { + recurringInterval: 'Month', + successUrlPath: '/settings/billing', + plan: BillingPlanKey.PRO, + requirePaymentMethod: true, + }, + }; + + await client + .post('/graphql') + .set('Authorization', `Bearer ${MEMBER_ACCESS_TOKEN}`) + .send(queryData) + .expect(200) + .expect((res) => { + expect(res.body.data).toBeNull(); + expect(res.body.errors).toBeDefined(); + expect(res.body.errors[0].message).toBe( + PermissionsExceptionMessage.PERMISSION_DENIED, + ); + expect(res.body.errors[0].extensions.code).toBe( + ErrorCode.FORBIDDEN, + ); + }); + }); + }); }); describe('lab', () => {