fix(core): Prevent data leakage of guest Customer data

Fixes #98

packages/core/e2e/graphql/generated-e2e-shop-types.ts

@@ -2608,7 +2608,23 @@ export type GetCustomerAddressesQuery = { __typename?: 'Query' } & {
         { __typename?: 'Order' } & {
             customer: Maybe<
                 { __typename?: 'Customer' } & {
-                    addresses: Maybe<Array<{ __typename?: 'Address' } & Pick<Address, 'id'>>>;
+                    addresses: Maybe<Array<{ __typename?: 'Address' } & Pick<Address, 'id' | 'streetLine1'>>>;
+                }
+            >;
+        }
+    >;
+};
+
+export type GetCustomerOrdersQueryVariables = {};
+
+export type GetCustomerOrdersQuery = { __typename?: 'Query' } & {
+    activeOrder: Maybe<
+        { __typename?: 'Order' } & {
+            customer: Maybe<
+                { __typename?: 'Customer' } & {
+                    orders: { __typename?: 'OrderList' } & {
+                        items: Array<{ __typename?: 'Order' } & Pick<Order, 'id'>>;
+                    };
                 }
             >;
         }
@@ -2850,6 +2866,19 @@ export namespace GetCustomerAddresses {
     >;
 }
 
+export namespace GetCustomerOrders {
+    export type Variables = GetCustomerOrdersQueryVariables;
+    export type Query = GetCustomerOrdersQuery;
+    export type ActiveOrder = NonNullable<GetCustomerOrdersQuery['activeOrder']>;
+    export type Customer = NonNullable<(NonNullable<GetCustomerOrdersQuery['activeOrder']>)['customer']>;
+    export type Orders = (NonNullable<
+        (NonNullable<GetCustomerOrdersQuery['activeOrder']>)['customer']
+    >)['orders'];
+    export type Items = NonNullable<
+        (NonNullable<(NonNullable<GetCustomerOrdersQuery['activeOrder']>)['customer']>)['orders']['items'][0]
+    >;
+}
+
 export namespace ApplyCouponCode {
     export type Variables = ApplyCouponCodeMutationVariables;
     export type Mutation = ApplyCouponCodeMutation;

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

@@ -349,6 +349,21 @@ export const GET_ACTIVE_ORDER_ADDRESSES = gql`
             customer {
                 addresses {
                     id
+                    streetLine1
+                }
+            }
+        }
+    }
+`;
+
+export const GET_ACTIVE_ORDER_ORDERS = gql`
+    query GetCustomerOrders {
+        activeOrder {
+            customer {
+                orders {
+                    items {
+                        id
+                    }
                 }
             }
         }

packages/core/e2e/shop-order.e2e-spec.ts

@@ -27,6 +27,7 @@ import {
     GetActiveOrderPayments,
     GetAvailableCountries,
     GetCustomerAddresses,
+    GetCustomerOrders,
     GetNextOrderStates,
     GetOrderByCode,
     GetShippingMethods,
@@ -49,6 +50,7 @@ import {
     ADJUST_ITEM_QUANTITY,
     GET_ACTIVE_ORDER,
     GET_ACTIVE_ORDER_ADDRESSES,
+    GET_ACTIVE_ORDER_ORDERS,
     GET_ACTIVE_ORDER_PAYMENTS,
     GET_AVAILABLE_COUNTRIES,
     GET_ELIGIBLE_SHIPPING_METHODS,
@@ -381,10 +383,13 @@ describe('Shop orders', () => {
         });
 
         it('customer default Addresses are not updated before payment', async () => {
-            // TODO: will need to be reworked for https://github.com/vendure-ecommerce/vendure/issues/98
-            const result = await shopClient.query<GetCustomerAddresses.Query>(GET_ACTIVE_ORDER_ADDRESSES);
+            const { activeOrder } = await shopClient.query<GetActiveOrder.Query>(GET_ACTIVE_ORDER);
+            const { customer } = await adminClient.query<GetCustomer.Query, GetCustomer.Variables>(
+                GET_CUSTOMER,
+                { id: activeOrder!.customer!.id },
+            );
 
-            expect(result.activeOrder!.customer!.addresses).toEqual([]);
+            expect(customer!.addresses).toEqual([]);
         });
 
         it('can transition to ArrangingPayment once Customer has been set', async () => {
@@ -965,4 +970,104 @@ describe('Shop orders', () => {
             expect(activeOrder!.lines[1].productVariant.id).toBe('T_2');
         });
     });
+
+    describe('security of customer data', () => {
+        let customers: GetCustomerList.Items[];
+
+        beforeAll(async () => {
+            const result = await adminClient.query<GetCustomerList.Query>(GET_CUSTOMER_LIST);
+            customers = result.customers.items;
+        });
+
+        it('cannot setCustomOrder to existing non-guest Customer', async () => {
+            await shopClient.asAnonymousUser();
+            const { addItemToOrder } = await shopClient.query<
+                AddItemToOrder.Mutation,
+                AddItemToOrder.Variables
+            >(ADD_ITEM_TO_ORDER, {
+                productVariantId: 'T_1',
+                quantity: 1,
+            });
+
+            try {
+                await shopClient.query<SetCustomerForOrder.Mutation, SetCustomerForOrder.Variables>(
+                    SET_CUSTOMER,
+                    {
+                        input: {
+                            emailAddress: customers[0].emailAddress,
+                            firstName: 'Evil',
+                            lastName: 'Hacker',
+                        },
+                    },
+                );
+                fail('Should have thrown');
+            } catch (e) {
+                expect(e.message).toContain(
+                    'Cannot use a registered email address for a guest order. Please log in first',
+                );
+            }
+            const { customer } = await adminClient.query<GetCustomer.Query, GetCustomer.Variables>(
+                GET_CUSTOMER,
+                {
+                    id: customers[0].id,
+                },
+            );
+            expect(customer!.firstName).not.toBe('Evil');
+            expect(customer!.lastName).not.toBe('Hacker');
+        });
+
+        it('guest cannot access Addresses of guest customer', async () => {
+            await shopClient.asAnonymousUser();
+            const { addItemToOrder } = await shopClient.query<
+                AddItemToOrder.Mutation,
+                AddItemToOrder.Variables
+            >(ADD_ITEM_TO_ORDER, {
+                productVariantId: 'T_1',
+                quantity: 1,
+            });
+
+            await shopClient.query<SetCustomerForOrder.Mutation, SetCustomerForOrder.Variables>(
+                SET_CUSTOMER,
+                {
+                    input: {
+                        emailAddress: '[email protected]',
+                        firstName: 'Evil',
+                        lastName: 'Hacker',
+                    },
+                },
+            );
+
+            const { activeOrder } = await shopClient.query<GetCustomerAddresses.Query>(
+                GET_ACTIVE_ORDER_ADDRESSES,
+            );
+
+            expect(activeOrder!.customer!.addresses).toEqual([]);
+        });
+
+        it('guest cannot access Orders of guest customer', async () => {
+            await shopClient.asAnonymousUser();
+            const { addItemToOrder } = await shopClient.query<
+                AddItemToOrder.Mutation,
+                AddItemToOrder.Variables
+            >(ADD_ITEM_TO_ORDER, {
+                productVariantId: 'T_1',
+                quantity: 1,
+            });
+
+            await shopClient.query<SetCustomerForOrder.Mutation, SetCustomerForOrder.Variables>(
+                SET_CUSTOMER,
+                {
+                    input: {
+                        emailAddress: '[email protected]',
+                        firstName: 'Evil',
+                        lastName: 'Hacker',
+                    },
+                },
+            );
+
+            const { activeOrder } = await shopClient.query<GetCustomerOrders.Query>(GET_ACTIVE_ORDER_ORDERS);
+
+            expect(activeOrder!.customer!.orders.items).toEqual([]);
+        });
+    });
 });

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

@@ -8,7 +8,9 @@ import { Order } from '../../../entity/order/order.entity';
 import { CustomerService } from '../../../service/services/customer.service';
 import { OrderService } from '../../../service/services/order.service';
 import { UserService } from '../../../service/services/user.service';
+import { ApiType } from '../../common/get-api-type';
 import { RequestContext } from '../../common/request-context';
+import { Api } from '../../decorators/api.decorator';
 import { Ctx } from '../../decorators/request-context.decorator';
 
 @Resolver('Customer')
@@ -19,7 +21,15 @@ export class CustomerEntityResolver {
         private userService: UserService,
     ) {}
     @ResolveProperty()
-    async addresses(@Ctx() ctx: RequestContext, @Parent() customer: Customer): Promise<Address[]> {
+    async addresses(
+        @Ctx() ctx: RequestContext,
+        @Parent() customer: Customer,
+        @Api() apiType: ApiType,
+    ): Promise<Address[]> {
+        if (apiType === 'shop' && !ctx.activeUserId) {
+            // Guest customers should not be able to see this data
+            return [];
+        }
         return this.customerService.findAddressesByCustomerId(ctx, customer.id);
     }
 
@@ -28,7 +38,12 @@ export class CustomerEntityResolver {
         @Ctx() ctx: RequestContext,
         @Parent() customer: Customer,
         @Args() args: QueryOrdersArgs,
+        @Api() apiType: ApiType,
     ): Promise<PaginatedList<Order>> {
+        if (apiType === 'shop' && !ctx.activeUserId) {
+            // Guest customers should not be able to see this data
+            return { items: [], totalItems: 0 };
+        }
         return this.orderService.findByCustomerId(ctx, customer.id, args.options || undefined);
     }
 

packages/core/src/api/resolvers/shop/shop-order.resolver.ts

@@ -293,7 +293,7 @@ export class ShopOrderResolver {
         if (ctx.authorizedAsOwnerOnly) {
             const sessionOrder = await this.getOrderFromContext(ctx);
             if (sessionOrder) {
-                const customer = await this.customerService.createOrUpdate(args.input);
+                const customer = await this.customerService.createOrUpdate(args.input, true);
                 return this.orderService.addCustomerToOrder(ctx, sessionOrder.id, customer);
             }
         }

packages/core/src/i18n/messages/en.json

@@ -14,6 +14,7 @@
     "cannot-transition-refund-from-to": "Cannot transition Refund from \"{ fromState }\" to \"{ toState }\"",
     "cannot-transition-to-shipping-when-order-is-empty": "Cannot transition Order to the \"ArrangingShipping\" state when it is empty",
     "cannot-transition-to-payment-without-customer": "Cannot transition Order to the \"ArrangingPayment\" state without Customer details",
+    "cannot-use-registered-email-address-for-guest-order":  "Cannot use a registered email address for a guest order. Please log in first",
     "channel-not-found":  "No channel with the token \"{ token }\" exists",
     "country-code-not-valid":  "The countryCode \"{ countryCode }\" was not recognized",
     "coupon-code-expired":  "Coupon code \"{ couponCode }\" has expired",

packages/core/src/service/services/customer.service.ts

@@ -13,7 +13,12 @@ import { ID, PaginatedList } from '@vendure/common/lib/shared-types';
 import { Connection } from 'typeorm';
 
 import { RequestContext } from '../../api/common/request-context';
-import { EntityNotFoundError, InternalServerError, UserInputError } from '../../common/error/errors';
+import {
+    EntityNotFoundError,
+    IllegalOperationError,
+    InternalServerError,
+    UserInputError,
+} from '../../common/error/errors';
 import { ListQueryOptions } from '../../common/types/common-types';
 import { assertFound, idsAreEqual, normalizeEmailAddress } from '../../common/utils';
 import { ConfigService } from '../../config/config.service';
@@ -234,7 +239,10 @@ export class CustomerService {
     /**
      * For guest checkouts, we assume that a matching email address is the same customer.
      */
-    async createOrUpdate(input: Partial<CreateCustomerInput> & { emailAddress: string }): Promise<Customer> {
+    async createOrUpdate(
+        input: Partial<CreateCustomerInput> & { emailAddress: string },
+        throwOnExistingUser: boolean = false,
+    ): Promise<Customer> {
         input.emailAddress = normalizeEmailAddress(input.emailAddress);
         let customer: Customer;
         const existing = await this.connection.getRepository(Customer).findOne({
@@ -243,6 +251,10 @@ export class CustomerService {
             },
         });
         if (existing) {
+            if (existing.user && throwOnExistingUser) {
+                // It is not permitted to modify an existing *registered* Customer
+                throw new IllegalOperationError('error.cannot-use-registered-email-address-for-guest-order');
+            }
             customer = patchEntity(existing, input);
         } else {
             customer = new Customer(input);