fix(core): Prevent customer data leak via Shop API

Fixes #730

packages/core/e2e/auth.e2e-spec.ts

@@ -11,29 +11,35 @@ import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-conf
 import { ProtectedFieldsPlugin, transactions } from './fixtures/test-plugins/with-protected-field-resolver';
 import {
     CreateAdministrator,
+    CreateCustomerGroup,
     CreateRole,
     ErrorCode,
     GetCustomerList,
+    GetTaxRates,
     Me,
     MutationCreateProductArgs,
     MutationLoginArgs,
     MutationUpdateProductArgs,
     Permission,
+    UpdateTaxRate,
 } from './graphql/generated-e2e-admin-types';
 import {
     ATTEMPT_LOGIN,
     CREATE_ADMINISTRATOR,
+    CREATE_CUSTOMER_GROUP,
     CREATE_PRODUCT,
     CREATE_ROLE,
     GET_CUSTOMER_LIST,
     GET_PRODUCT_LIST,
+    GET_TAX_RATES_LIST,
     ME,
     UPDATE_PRODUCT,
+    UPDATE_TAX_RATE,
 } from './graphql/shared-definitions';
 import { assertThrowsWithMessage } from './utils/assert-throws-with-message';
 
 describe('Authorization & permissions', () => {
-    const { server, adminClient } = createTestEnvironment({
+    const { server, adminClient, shopClient } = createTestEnvironment({
         ...testConfig,
         plugins: [ProtectedFieldsPlugin],
     });
@@ -42,7 +48,7 @@ describe('Authorization & permissions', () => {
         await server.init({
             initialData,
             productsCsvPath: path.join(__dirname, 'fixtures/e2e-products-minimal.csv'),
-            customerCount: 1,
+            customerCount: 5,
         });
         await adminClient.asSuperAdmin();
     }, TEST_SETUP_TIMEOUT_MS);
@@ -239,6 +245,68 @@ describe('Authorization & permissions', () => {
                 { id: 'T_2', amount: -50, description: 'debit' },
             ]);
         });
+
+        // https://github.com/vendure-ecommerce/vendure/issues/730
+        it('protects against deep query data leakage', async () => {
+            await adminClient.asSuperAdmin();
+            const { createCustomerGroup } = await adminClient.query<
+                CreateCustomerGroup.Mutation,
+                CreateCustomerGroup.Variables
+            >(CREATE_CUSTOMER_GROUP, {
+                input: {
+                    name: 'Test group',
+                    customerIds: ['T_1', 'T_2', 'T_3', 'T_4'],
+                },
+            });
+
+            const taxRateName = `Standard Tax ${initialData.defaultZone}`;
+            const { taxRates } = await adminClient.query<GetTaxRates.Query, GetTaxRates.Variables>(
+                GET_TAX_RATES_LIST,
+                {
+                    options: {
+                        filter: {
+                            name: { eq: taxRateName },
+                        },
+                    },
+                },
+            );
+
+            const standardTax = taxRates.items[0];
+            expect(standardTax.name).toBe(taxRateName);
+
+            await adminClient.query<UpdateTaxRate.Mutation, UpdateTaxRate.Variables>(UPDATE_TAX_RATE, {
+                input: {
+                    id: standardTax.id,
+                    customerGroupId: createCustomerGroup.id,
+                },
+            });
+
+            try {
+                const status = await shopClient.query(
+                    gql(`
+                query {
+                  product(id: "T_1") {
+                    variants {
+                      taxRateApplied {
+                        customerGroup {
+                          customers {
+                            items {
+                              id
+                              emailAddress
+                            }
+                          }
+                        }
+                      }
+                    }
+                  }
+                }`),
+                    { id: 'T_1' },
+                );
+                fail(`Should have thrown`);
+            } catch (e) {
+                expect(getErrorCode(e)).toBe('FORBIDDEN');
+            }
+        });
     });
 
     async function assertRequestAllowed<V>(operation: DocumentNode, variables?: V) {

packages/core/e2e/graphql/shared-definitions.ts

@@ -898,3 +898,15 @@ export const REMOVE_PROMOTIONS_FROM_CHANNEL = gql`
         }
     }
 `;
+
+export const GET_TAX_RATES_LIST = gql`
+    query GetTaxRates($options: TaxRateListOptions) {
+        taxRates(options: $options) {
+            items {
+                ...TaxRate
+            }
+            totalItems
+        }
+    }
+    ${TAX_RATE_FRAGMENT}
+`;

packages/core/e2e/tax-rate.e2e-spec.ts

@@ -16,7 +16,7 @@ import {
     GetTaxRates,
     UpdateTaxRate,
 } from './graphql/generated-e2e-admin-types';
-import { UPDATE_TAX_RATE } from './graphql/shared-definitions';
+import { GET_TAX_RATES_LIST, UPDATE_TAX_RATE } from './graphql/shared-definitions';
 
 describe('TaxRate resolver', () => {
     const { server, adminClient, shopClient } = createTestEnvironment(testConfig);
@@ -103,18 +103,6 @@ describe('TaxRate resolver', () => {
     });
 });
 
-export const GET_TAX_RATES_LIST = gql`
-    query GetTaxRates {
-        taxRates {
-            items {
-                ...TaxRate
-            }
-            totalItems
-        }
-    }
-    ${TAX_RATE_FRAGMENT}
-`;
-
 export const GET_TAX_RATE = gql`
     query GetTaxRate($id: ID!) {
         taxRate(id: $id) {

packages/core/src/api/api-internal-modules.ts

@@ -69,6 +69,7 @@ import {
 import { RefundEntityResolver } from './resolvers/entity/refund-entity.resolver';
 import { RoleEntityResolver } from './resolvers/entity/role-entity.resolver';
 import { ShippingLineEntityResolver } from './resolvers/entity/shipping-line-entity.resolver';
+import { TaxRateEntityResolver } from './resolvers/entity/tax-rate-entity.resolver';
 import { UserEntityResolver } from './resolvers/entity/user-entity.resolver';
 import { ShopAuthResolver } from './resolvers/shop/shop-auth.resolver';
 import { ShopCustomerResolver } from './resolvers/shop/shop-customer.resolver';
@@ -130,6 +131,7 @@ export const entityResolvers = [
     RoleEntityResolver,
     ShippingLineEntityResolver,
     UserEntityResolver,
+    TaxRateEntityResolver,
 ];
 
 export const adminEntityResolvers = [

packages/core/src/api/decorators/allow.decorator.ts

@@ -5,8 +5,9 @@ export const PERMISSIONS_METADATA_KEY = '__permissions__';
 
 /**
  * @description
- * Attatches metadata to the resolver defining which permissions are required to execute the
- * operation, using one or more {@link Permission} values.
+ * Attaches metadata to the resolver defining which permissions are required to execute the
+ * operation, using one or more {@link Permission} values. Can be applied to top-level queries
+ * and mutations as well as field resolvers.
  *
  * @example
  * ```TypeScript

packages/core/src/api/resolvers/entity/customer-group-entity.resolver.ts

@@ -1,17 +1,19 @@
 import { Args, Parent, ResolveField, Resolver } from '@nestjs/graphql';
-import { QueryCustomersArgs } from '@vendure/common/lib/generated-types';
+import { Permission, QueryCustomersArgs } from '@vendure/common/lib/generated-types';
 import { PaginatedList } from '@vendure/common/lib/shared-types';
 
 import { CustomerGroup } from '../../../entity/customer-group/customer-group.entity';
 import { Customer } from '../../../entity/customer/customer.entity';
 import { CustomerGroupService } from '../../../service/services/customer-group.service';
 import { RequestContext } from '../../common/request-context';
+import { Allow } from '../../decorators/allow.decorator';
 import { Ctx } from '../../decorators/request-context.decorator';
 
 @Resolver('CustomerGroup')
 export class CustomerGroupEntityResolver {
     constructor(private customerGroupService: CustomerGroupService) {}
 
+    @Allow(Permission.ReadCustomer)
     @ResolveField()
     async customers(
         @Ctx() ctx: RequestContext,

packages/core/src/api/resolvers/entity/tax-rate-entity.resolver.ts

@@ -0,0 +1,29 @@
+import { Parent, ResolveField, Resolver } from '@nestjs/graphql';
+import { Permission } from '@vendure/common/lib/generated-types';
+
+import { Channel } from '../../../entity/channel/channel.entity';
+import { CustomerGroup } from '../../../entity/customer-group/customer-group.entity';
+import { TaxRate } from '../../../entity/tax-rate/tax-rate.entity';
+import { RoleService } from '../../../service/services/role.service';
+import { TaxRateService } from '../../../service/services/tax-rate.service';
+import { RequestContext } from '../../common/request-context';
+import { Allow } from '../../decorators/allow.decorator';
+import { Ctx } from '../../decorators/request-context.decorator';
+
+@Resolver('TaxRate')
+export class TaxRateEntityResolver {
+    constructor(private taxRateService: TaxRateService) {}
+
+    @Allow(Permission.ReadCustomer)
+    @ResolveField()
+    async customerGroup(
+        @Ctx() ctx: RequestContext,
+        @Parent() taxRate: TaxRate,
+    ): Promise<CustomerGroup | undefined> {
+        if (taxRate.customerGroup) {
+            return taxRate.customerGroup;
+        }
+
+        return (await this.taxRateService.findOne(ctx, taxRate.id))?.customerGroup;
+    }
+}