fix(core): Fix auth of admin and customer users with the same email (#2016)

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

@@ -1,6 +1,6 @@
 /* tslint:disable:no-non-null-assertion */
 import { SUPER_ADMIN_USER_IDENTIFIER, SUPER_ADMIN_USER_PASSWORD } from '@vendure/common/lib/shared-constants';
-import { createTestEnvironment } from '@vendure/testing';
+import { createErrorResultGuard, createTestEnvironment, ErrorResultGuard } from '@vendure/testing';
 import { DocumentNode } from 'graphql';
 import gql from 'graphql-tag';
 import path from 'path';
@@ -10,9 +10,12 @@ import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-conf
 
 import { ProtectedFieldsPlugin, transactions } from './fixtures/test-plugins/with-protected-field-resolver';
 import {
+    AttemptLogin,
     CreateAdministrator,
+    CreateCustomer,
     CreateCustomerGroup,
     CreateRole,
+    CurrentUserFragment,
     ErrorCode,
     GetCustomerList,
     GetTaxRates,
@@ -26,6 +29,7 @@ import {
 import {
     ATTEMPT_LOGIN,
     CREATE_ADMINISTRATOR,
+    CREATE_CUSTOMER,
     CREATE_CUSTOMER_GROUP,
     CREATE_PRODUCT,
     CREATE_ROLE,
@@ -184,6 +188,99 @@ describe('Authorization & permissions', () => {
         });
     });
 
+    describe('administrator and customer users with the same email address', () => {
+        const emailAddress = 'same-email@test.com';
+        const adminPassword = 'admin-password';
+        const customerPassword = 'customer-password';
+
+        const loginErrorGuard: ErrorResultGuard<CurrentUserFragment> = createErrorResultGuard(
+            input => !!input.identifier,
+        );
+
+        beforeAll(async () => {
+            await adminClient.asSuperAdmin();
+
+            await adminClient.query<CreateAdministrator.Mutation, CreateAdministrator.Variables>(
+                CREATE_ADMINISTRATOR,
+                {
+                    input: {
+                        emailAddress,
+                        firstName: 'First',
+                        lastName: 'Last',
+                        password: adminPassword,
+                        roleIds: ['1'],
+                    },
+                },
+            );
+
+            await adminClient.query<CreateCustomer.Mutation, CreateCustomer.Variables>(CREATE_CUSTOMER, {
+                input: {
+                    emailAddress,
+                    firstName: 'First',
+                    lastName: 'Last',
+                },
+                password: customerPassword,
+            });
+        });
+
+        beforeEach(async () => {
+            await adminClient.asAnonymousUser();
+            await shopClient.asAnonymousUser();
+        });
+
+        it('can log in as an administrator', async () => {
+            const loginResult = await adminClient.query<AttemptLogin.Mutation, AttemptLogin.Variables>(
+                ATTEMPT_LOGIN,
+                {
+                    username: emailAddress,
+                    password: adminPassword,
+                },
+            );
+
+            loginErrorGuard.assertSuccess(loginResult.login);
+            expect(loginResult.login.identifier).toEqual(emailAddress);
+        });
+
+        it('can log in as a customer', async () => {
+            const loginResult = await shopClient.query<AttemptLogin.Mutation, AttemptLogin.Variables>(
+                ATTEMPT_LOGIN,
+                {
+                    username: emailAddress,
+                    password: customerPassword,
+                },
+            );
+
+            loginErrorGuard.assertSuccess(loginResult.login);
+            expect(loginResult.login.identifier).toEqual(emailAddress);
+        });
+
+        it('cannot log in as an administrator using a customer password', async () => {
+            const loginResult = await adminClient.query<AttemptLogin.Mutation, AttemptLogin.Variables>(
+                ATTEMPT_LOGIN,
+                {
+                    username: emailAddress,
+                    password: customerPassword,
+                },
+            );
+
+            loginErrorGuard.assertErrorResult(loginResult.login);
+            expect(loginResult.login.errorCode).toEqual(ErrorCode.INVALID_CREDENTIALS_ERROR);
+        });
+
+        it('cannot log in as a customer using an administrator password', async () => {
+            const loginResult = await shopClient.query<AttemptLogin.Mutation, AttemptLogin.Variables>(
+                ATTEMPT_LOGIN,
+                {
+                    username: emailAddress,
+                    password: adminPassword,
+                },
+            );
+
+            loginErrorGuard.assertErrorResult(loginResult.login);
+            expect(loginResult.login.errorCode).toEqual(ErrorCode.INVALID_CREDENTIALS_ERROR);
+        });
+    });
+
     describe('protected field resolvers', () => {
         let readCatalogAdmin: { identifier: string; password: string };
         let transactionsAdmin: { identifier: string; password: string };

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

@@ -20,6 +20,7 @@ import { CUSTOMER_FRAGMENT } from './graphql/fragments';
 import {
     AddNoteToCustomer,
     CreateAddress,
+    CreateAdministrator,
     CreateCustomer,
     CustomerFragment,
     DeleteCustomer,
@@ -48,6 +49,7 @@ import {
 } from './graphql/generated-e2e-shop-types';
 import {
     CREATE_ADDRESS,
+    CREATE_ADMINISTRATOR,
     CREATE_CUSTOMER,
     DELETE_CUSTOMER,
     DELETE_CUSTOMER_NOTE,
@@ -139,16 +141,51 @@ describe('Customer resolver', () => {
     });
 
     it('customer resolver resolves User', async () => {
+        const emailAddress = 'same-email@test.com';
+
+        // Create an administrator with the same email first in order to ensure the right user is resolved.
+        // This test also validates that a customer can be created with the same identifier
+        // of an existing administrator
+        const { createAdministrator } = await adminClient.query<
+            CreateAdministrator.Mutation,
+            CreateAdministrator.Variables
+        >(CREATE_ADMINISTRATOR, {
+            input: {
+                emailAddress,
+                firstName: 'First',
+                lastName: 'Last',
+                password: '123',
+                roleIds: ['1'],
+            },
+        });
+
+        expect(createAdministrator.emailAddress).toEqual(emailAddress);
+
+        const { createCustomer } = await adminClient.query<CreateCustomer.Mutation, CreateCustomer.Variables>(
+            CREATE_CUSTOMER,
+            {
+                input: {
+                    emailAddress,
+                    firstName: 'New',
+                    lastName: 'Customer',
+                },
+                password: 'test',
+            },
+        );
+
+        customerErrorGuard.assertSuccess(createCustomer);
+        expect(createCustomer.emailAddress).toEqual(emailAddress);
+
         const { customer } = await adminClient.query<
             GetCustomerWithUser.Query,
             GetCustomerWithUser.Variables
         >(GET_CUSTOMER_WITH_USER, {
-            id: firstCustomer.id,
+            id: createCustomer.id,
         });
 
         expect(customer!.user).toEqual({
-            id: 'T_2',
-            identifier: firstCustomer.emailAddress,
+            id: createCustomer.user?.id,
+            identifier: emailAddress,
             verified: true,
         });
     });

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

@@ -208,6 +208,10 @@ export const ATTEMPT_LOGIN = gql`
     mutation AttemptLogin($username: String!, $password: String!, $rememberMe: Boolean) {
         login(username: $username, password: $password, rememberMe: $rememberMe) {
             ...CurrentUser
+            ... on ErrorResult {
+                errorCode
+                message
+            }
         }
     }
     ${CURRENT_USER_FRAGMENT}

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

@@ -56,7 +56,7 @@ export class CustomerEntityResolver {
             return customer.user;
         }
 
-        return this.userService.getUserByEmailAddress(ctx, customer.emailAddress);
+        return this.userService.getUserByEmailAddress(ctx, customer.emailAddress, 'customer');
     }
 }
 

packages/core/src/config/auth/native-authentication-strategy.ts

@@ -30,12 +30,15 @@ export class NativeAuthenticationStrategy implements AuthenticationStrategy<Nati
 
     private connection: TransactionalConnection;
     private passwordCipher: import('../../service/helpers/password-cipher/password-cipher').PasswordCipher;
+    private userService: import('../../service/services/user.service').UserService;
 
     async init(injector: Injector) {
         this.connection = injector.get(TransactionalConnection);
-        // This is lazily-loaded to avoid a circular dependency
+        // These are lazily-loaded to avoid a circular dependency
         const { PasswordCipher } = await import('../../service/helpers/password-cipher/password-cipher');
+        const { UserService } = await import('../../service/services/user.service');
         this.passwordCipher = injector.get(PasswordCipher);
+        this.userService = injector.get(UserService);
     }
 
     defineInputType(): DocumentNode {
@@ -48,7 +51,7 @@ export class NativeAuthenticationStrategy implements AuthenticationStrategy<Nati
     }
 
     async authenticate(ctx: RequestContext, data: NativeAuthenticationData): Promise<User | false> {
-        const user = await this.getUserFromIdentifier(ctx, data.username);
+        const user = await this.userService.getUserByEmailAddress(ctx, data.username);
         if (!user) {
             return false;
         }
@@ -59,13 +62,6 @@ export class NativeAuthenticationStrategy implements AuthenticationStrategy<Nati
         return user;
     }
 
-    private getUserFromIdentifier(ctx: RequestContext, identifier: string): Promise<User | undefined> {
-        return this.connection.getRepository(ctx, User).findOne({
-            where: { identifier, deletedAt: null },
-            relations: ['roles', 'roles.channels', 'authenticationMethods'],
-        });
-    }
-
     /**
      * Verify the provided password against the one we have for the given user.
      */

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

@@ -227,12 +227,11 @@ export class CustomerService {
                 deletedAt: null,
             },
         });
-        const existingUser = await this.connection.getRepository(ctx, User).findOne({
-            where: {
-                identifier: input.emailAddress,
-                deletedAt: null,
-            },
-        });
+        const existingUser = await this.userService.getUserByEmailAddress(
+            ctx,
+            input.emailAddress,
+            'customer',
+        );
 
         if (existingCustomer && existingUser) {
             // Customer already exists, bring to this Channel
@@ -326,6 +325,7 @@ export class CustomerService {
                     const existingUserWithEmailAddress = await this.userService.getUserByEmailAddress(
                         ctx,
                         input.emailAddress,
+                        'customer',
                     );
 
                     if (

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

@@ -48,14 +48,24 @@ export class UserService {
         });
     }
 
-    async getUserByEmailAddress(ctx: RequestContext, emailAddress: string): Promise<User | undefined> {
-        return this.connection.getRepository(ctx, User).findOne({
-            where: {
-                identifier: emailAddress,
-                deletedAt: null,
-            },
-            relations: ['roles', 'roles.channels', 'authenticationMethods'],
-        });
+    async getUserByEmailAddress(
+        ctx: RequestContext,
+        emailAddress: string,
+        userType?: 'administrator' | 'customer',
+    ): Promise<User | undefined> {
+        const entity = userType ?? (ctx.apiType === 'admin' ? 'administrator' : 'customer');
+        const table = `${this.configService.dbConnectionOptions.entityPrefix ?? ''}${entity}`;
+
+        return this.connection
+            .getRepository(ctx, User)
+            .createQueryBuilder('user')
+            .innerJoin(table, table, `${table}.userId = user.id`)
+            .leftJoinAndSelect('user.roles', 'roles')
+            .leftJoinAndSelect('roles.channels', 'channels')
+            .leftJoinAndSelect('user.authenticationMethods', 'authenticationMethods')
+            .where('user.identifier = :identifier', { identifier: emailAddress })
+            .andWhere('user.deletedAt IS NULL')
+            .getOne();
     }
 
     /**