fix(core): Fix Admin/Customer user conflict with external auth

Fixes #926

packages/core/e2e/authentication-strategy.e2e-spec.ts

@@ -12,13 +12,14 @@ import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-conf
 import {
     TestAuthenticationStrategy,
     TestAuthenticationStrategy2,
+    TestSSOStrategyAdmin,
+    TestSSOStrategyShop,
     VALID_AUTH_TOKEN,
 } from './fixtures/test-authentication-strategies';
 import { CURRENT_USER_FRAGMENT } from './graphql/fragments';
 import {
     Authenticate,
     CreateCustomer,
-    CurrentUser,
     CurrentUserFragment,
     CustomerFragment,
     DeleteCustomer,
@@ -31,7 +32,6 @@ import {
 import { Register } from './graphql/generated-e2e-shop-types';
 import { CREATE_CUSTOMER, DELETE_CUSTOMER, GET_CUSTOMER_HISTORY, ME } from './graphql/shared-definitions';
 import { REGISTER_ACCOUNT } from './graphql/shop-definitions';
-import { assertThrowsWithMessage } from './utils/assert-throws-with-message';
 
 describe('AuthenticationStrategy', () => {
     const { server, adminClient, shopClient } = createTestEnvironment(
@@ -41,7 +41,9 @@ describe('AuthenticationStrategy', () => {
                     new NativeAuthenticationStrategy(),
                     new TestAuthenticationStrategy(),
                     new TestAuthenticationStrategy2(),
+                    new TestSSOStrategyShop(),
                 ],
+                adminAuthenticationStrategy: [new NativeAuthenticationStrategy(), new TestSSOStrategyAdmin()],
             },
         }),
     );
@@ -292,6 +294,31 @@ describe('AuthenticationStrategy', () => {
                 },
             ]);
         });
+
+        // https://github.com/vendure-ecommerce/vendure/issues/926
+        it('Customer and Admin external auth does not reuse same User for different strategies', async () => {
+            const emailAddress = '[email protected]';
+            await adminClient.asAnonymousUser();
+            const { authenticate: adminAuth } = await adminClient.query<Authenticate.Mutation>(AUTHENTICATE, {
+                input: {
+                    test_sso_strategy_admin: {
+                        email: emailAddress,
+                    },
+                },
+            });
+            currentUserGuard.assertSuccess(adminAuth);
+
+            const { authenticate: shopAuth } = await shopClient.query<Authenticate.Mutation>(AUTHENTICATE, {
+                input: {
+                    test_sso_strategy_shop: {
+                        email: emailAddress,
+                    },
+                },
+            });
+            currentUserGuard.assertSuccess(shopAuth);
+
+            expect(adminAuth.id).not.toBe(shopAuth.id);
+        });
     });
 
     describe('native auth', () => {

packages/core/e2e/fixtures/test-authentication-strategies.ts

@@ -3,6 +3,7 @@ import {
     ExternalAuthenticationService,
     Injector,
     RequestContext,
+    RoleService,
     User,
 } from '@vendure/core';
 import { DocumentNode } from 'graphql';
@@ -65,6 +66,76 @@ export class TestAuthenticationStrategy implements AuthenticationStrategy<TestAu
     }
 }
 
+export class TestSSOStrategyAdmin implements AuthenticationStrategy<{ email: string }> {
+    readonly name = 'test_sso_strategy_admin';
+    private externalAuthenticationService: ExternalAuthenticationService;
+    private roleService: RoleService;
+
+    init(injector: Injector) {
+        this.externalAuthenticationService = injector.get(ExternalAuthenticationService);
+        this.roleService = injector.get(RoleService);
+    }
+
+    defineInputType(): DocumentNode {
+        return gql`
+            input TestSSOInputAdmin {
+                email: String!
+            }
+        `;
+    }
+
+    async authenticate(ctx: RequestContext, data: { email: string }): Promise<User | false | string> {
+        const { email } = data;
+        const user = await this.externalAuthenticationService.findUser(ctx, this.name, email);
+        if (user) {
+            return user;
+        }
+        const superAdminRole = await this.roleService.getSuperAdminRole();
+        return this.externalAuthenticationService.createAdministratorAndUser(ctx, {
+            strategy: this.name,
+            externalIdentifier: email,
+            emailAddress: email,
+            firstName: 'SSO Admin First Name',
+            lastName: 'SSO Admin Last Name',
+            identifier: email,
+            roles: [superAdminRole],
+        });
+    }
+}
+
+export class TestSSOStrategyShop implements AuthenticationStrategy<{ email: string }> {
+    readonly name = 'test_sso_strategy_shop';
+    private externalAuthenticationService: ExternalAuthenticationService;
+
+    init(injector: Injector) {
+        this.externalAuthenticationService = injector.get(ExternalAuthenticationService);
+    }
+
+    defineInputType(): DocumentNode {
+        return gql`
+            input TestSSOInputShop {
+                email: String!
+            }
+        `;
+    }
+
+    async authenticate(ctx: RequestContext, data: { email: string }): Promise<User | false | string> {
+        const { email } = data;
+        const user = await this.externalAuthenticationService.findUser(ctx, this.name, email);
+        if (user) {
+            return user;
+        }
+        return this.externalAuthenticationService.createCustomerAndUser(ctx, {
+            strategy: this.name,
+            externalIdentifier: email,
+            emailAddress: email,
+            firstName: 'SSO Customer First Name',
+            lastName: 'SSO Customer Last Name',
+            verified: true,
+        });
+    }
+}
+
 export class TestAuthenticationStrategy2 implements AuthenticationStrategy<{ token: string; email: string }> {
     readonly name = 'test_strategy2';
     private externalAuthenticationService: ExternalAuthenticationService;

packages/core/src/service/helpers/external-authentication/external-authentication.service.ts

@@ -4,7 +4,6 @@ import { HistoryEntryType } from '@vendure/common/lib/generated-types';
 import { RequestContext } from '../../../api/common/request-context';
 import { Administrator } from '../../../entity/administrator/administrator.entity';
 import { ExternalAuthenticationMethod } from '../../../entity/authentication-method/external-authentication-method.entity';
-import { Collection } from '../../../entity/collection/collection.entity';
 import { Customer } from '../../../entity/customer/customer.entity';
 import { Role } from '../../../entity/role/role.entity';
 import { User } from '../../../entity/user/user.entity';
@@ -13,7 +12,6 @@ import { ChannelService } from '../../services/channel.service';
 import { CustomerService } from '../../services/customer.service';
 import { HistoryService } from '../../services/history.service';
 import { RoleService } from '../../services/role.service';
-import { UserService } from '../../services/user.service';
 import { TransactionalConnection } from '../../transaction/transactional-connection';
 
 /**
@@ -32,7 +30,6 @@ export class ExternalAuthenticationService {
         private customerService: CustomerService,
         private administratorService: AdministratorService,
         private channelService: ChannelService,
-        private userService: UserService,
     ) {}
 
     /**
@@ -96,7 +93,8 @@ export class ExternalAuthenticationService {
     ): Promise<User> {
         let user: User;
 
-        const existingUser = await this.userService.getUserByEmailAddress(ctx, config.emailAddress);
+        const existingUser = await this.findExistingCustomerUserByEmailAddress(ctx, config.emailAddress);
+
         if (existingUser) {
             user = existingUser;
         } else {
@@ -207,20 +205,34 @@ export class ExternalAuthenticationService {
         strategy: string,
         externalIdentifier: string,
     ): Promise<User | undefined> {
-        const user = await this.connection
+        const usersWithMatchingIdentifier = await this.connection
             .getRepository(ctx, User)
             .createQueryBuilder('user')
             .leftJoinAndSelect('user.authenticationMethods', 'authMethod')
             .andWhere('authMethod.externalIdentifier = :externalIdentifier', { externalIdentifier })
             .andWhere('user.deletedAt IS NULL')
-            .getOne();
+            .getMany();
 
-        const userHasMatchingAuthMethod = !!user?.authenticationMethods.find(m => {
-            return m instanceof ExternalAuthenticationMethod && m.strategy === strategy;
-        });
+        const matchingUser = usersWithMatchingIdentifier.find(user =>
+            user.authenticationMethods.find(
+                m => m instanceof ExternalAuthenticationMethod && m.strategy === strategy,
+            ),
+        );
 
-        if (userHasMatchingAuthMethod) {
-            return user;
-        }
+        return matchingUser;
+    }
+
+    private async findExistingCustomerUserByEmailAddress(ctx: RequestContext, emailAddress: string) {
+        const customer = await this.connection
+            .getRepository(ctx, Customer)
+            .createQueryBuilder('customer')
+            .leftJoinAndSelect('customer.user', 'user')
+            .leftJoin('customer.channels', 'channel')
+            .leftJoinAndSelect('user.authenticationMethods', 'authMethod')
+            .andWhere('customer.emailAddress = :emailAddress', { emailAddress })
+            .andWhere('user.deletedAt IS NULL')
+            .getOne();
+
+        return customer?.user;
     }
 }