From 6527e23fcdac9f9f3a74746eb20b9d7de249d005 Mon Sep 17 00:00:00 2001 From: Michael Bromley Date: Wed, 15 Nov 2023 13:03:20 +0100 Subject: [PATCH] fix(core): Allow case-sensitive Administrator identifiers Fixes #2485 --- packages/core/e2e/administrator.e2e-spec.ts | 37 ++++++++++++- packages/core/src/common/utils.spec.ts | 55 ++++++++++++++++++- packages/core/src/common/utils.ts | 13 ++++- .../core/src/service/services/user.service.ts | 20 ++++--- 4 files changed, 113 insertions(+), 12 deletions(-) diff --git a/packages/core/e2e/administrator.e2e-spec.ts b/packages/core/e2e/administrator.e2e-spec.ts index 05f8eff8bc..845319fe5a 100644 --- a/packages/core/e2e/administrator.e2e-spec.ts +++ b/packages/core/e2e/administrator.e2e-spec.ts @@ -1,16 +1,21 @@ import { SUPER_ADMIN_USER_IDENTIFIER } from '@vendure/common/lib/shared-constants'; -import { createTestEnvironment } from '@vendure/testing'; +import { createErrorResultGuard, createTestEnvironment, ErrorResultGuard } from '@vendure/testing'; import { fail } from 'assert'; import gql from 'graphql-tag'; import path from 'path'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { initialData } from '../../../e2e-common/e2e-initial-data'; -import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config'; +import { TEST_SETUP_TIMEOUT_MS, testConfig } from '../../../e2e-common/test-config'; import { ADMINISTRATOR_FRAGMENT } from './graphql/fragments'; import * as Codegen from './graphql/generated-e2e-admin-types'; -import { AdministratorFragment, DeletionResult } from './graphql/generated-e2e-admin-types'; +import { + AdministratorFragment, + AttemptLoginDocument, + CurrentUser, + DeletionResult, +} from './graphql/generated-e2e-admin-types'; import { CREATE_ADMINISTRATOR, UPDATE_ADMINISTRATOR } from './graphql/shared-definitions'; import { assertThrowsWithMessage } from './utils/assert-throws-with-message'; @@ -235,6 +240,32 @@ describe('Administrator resolver', () => { expect(activeAdministrator?.firstName).toBe('Thomas'); expect(activeAdministrator?.user.identifier).toBe('neo@metacortex.com'); }); + + it('supports case-sensitive admin identifiers', async () => { + const loginResultGuard: ErrorResultGuard = createErrorResultGuard( + input => !!input.identifier, + ); + const { createAdministrator } = await adminClient.query< + Codegen.CreateAdministratorMutation, + Codegen.CreateAdministratorMutationVariables + >(CREATE_ADMINISTRATOR, { + input: { + emailAddress: 'NewAdmin', + firstName: 'New', + lastName: 'Admin', + password: 'password', + roleIds: ['1'], + }, + }); + + const { login } = await adminClient.query(AttemptLoginDocument, { + username: 'NewAdmin', + password: 'password', + }); + + loginResultGuard.assertSuccess(login); + expect(login.identifier).toBe('NewAdmin'); + }); }); export const GET_ADMINISTRATORS = gql` diff --git a/packages/core/src/common/utils.spec.ts b/packages/core/src/common/utils.spec.ts index 021fc181bd..efb9bfa3e5 100644 --- a/packages/core/src/common/utils.spec.ts +++ b/packages/core/src/common/utils.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { convertRelationPaths } from './utils'; +import { convertRelationPaths, isEmailAddressLike, normalizeEmailAddress } from './utils'; describe('convertRelationPaths()', () => { it('undefined', () => { @@ -44,3 +44,56 @@ describe('convertRelationPaths()', () => { }); }); }); + +describe('normalizeEmailAddress()', () => { + it('should trim whitespace', () => { + expect(normalizeEmailAddress(' test@test.com ')).toBe('test@test.com'); + }); + + it('should lowercase email addresses', async () => { + expect(normalizeEmailAddress('JoeSmith@test.com')).toBe('joesmith@test.com'); + expect(normalizeEmailAddress('TEST@TEST.COM')).toBe('test@test.com'); + expect(normalizeEmailAddress('test.person@TEST.COM')).toBe('test.person@test.com'); + expect(normalizeEmailAddress('test.person+Extra@TEST.COM')).toBe('test.person+extra@test.com'); + expect(normalizeEmailAddress('TEST-person+Extra@TEST.COM')).toBe('test-person+extra@test.com'); + expect(normalizeEmailAddress('我買@屋企.香港')).toBe('我買@屋企.香港'); + }); + + it('ignores surrounding whitespace', async () => { + expect(normalizeEmailAddress(' JoeSmith@test.com')).toBe('joesmith@test.com'); + expect(normalizeEmailAddress('TEST@TEST.COM ')).toBe('test@test.com'); + expect(normalizeEmailAddress(' test.person@TEST.COM ')).toBe('test.person@test.com'); + }); + + it('should not lowercase non-email address identifiers', async () => { + expect(normalizeEmailAddress('Test')).toBe('Test'); + expect(normalizeEmailAddress('Ucj30Da2.!3rAA')).toBe('Ucj30Da2.!3rAA'); + }); +}); + +describe('isEmailAddressLike()', () => { + it('returns true for valid email addresses', () => { + expect(isEmailAddressLike('simple@example.com')).toBe(true); + expect(isEmailAddressLike('very.common@example.com')).toBe(true); + expect(isEmailAddressLike('abc@example.co.uk')).toBe(true); + expect(isEmailAddressLike('disposable.style.email.with+symbol@example.com')).toBe(true); + expect(isEmailAddressLike('other.email-with-hyphen@example.com')).toBe(true); + expect(isEmailAddressLike('fully-qualified-domain@example.com')).toBe(true); + expect(isEmailAddressLike('user.name+tag+sorting@example.com')).toBe(true); + expect(isEmailAddressLike('example-indeed@strange-example.com')).toBe(true); + expect(isEmailAddressLike('example-indeed@strange-example.inininini')).toBe(true); + }); + + it('ignores surrounding whitespace', () => { + expect(isEmailAddressLike(' simple@example.com')).toBe(true); + expect(isEmailAddressLike('very.common@example.com ')).toBe(true); + expect(isEmailAddressLike(' abc@example.co.uk ')).toBe(true); + }); + + it('returns false for invalid email addresses', () => { + expect(isEmailAddressLike('username')).toBe(false); + expect(isEmailAddressLike('823@ee28qje')).toBe(false); + expect(isEmailAddressLike('Abc.example.com')).toBe(false); + expect(isEmailAddressLike('A@b@')).toBe(false); + }); +}); diff --git a/packages/core/src/common/utils.ts b/packages/core/src/common/utils.ts index 08d442d690..e58554ae74 100644 --- a/packages/core/src/common/utils.ts +++ b/packages/core/src/common/utils.ts @@ -65,7 +65,18 @@ export function getAssetType(mimeType: string): AssetType { * upper/lower case. See more discussion here: https://ux.stackexchange.com/a/16849 */ export function normalizeEmailAddress(input: string): string { - return input.trim().toLowerCase(); + return isEmailAddressLike(input) ? input.trim().toLowerCase() : input.trim(); +} + +/** + * This is a "good enough" check for whether the input is an email address. + * From https://stackoverflow.com/a/32686261 + * It is used to determine whether to apply normalization (lower-casing) + * when comparing identifiers in user lookups. This allows case-sensitive + * identifiers for other authentication methods. + */ +export function isEmailAddressLike(input: string): boolean { + return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(input.trim()); } /** diff --git a/packages/core/src/service/services/user.service.ts b/packages/core/src/service/services/user.service.ts index f05e4ea7f2..c0e6b890e7 100644 --- a/packages/core/src/service/services/user.service.ts +++ b/packages/core/src/service/services/user.service.ts @@ -18,7 +18,7 @@ import { VerificationTokenExpiredError, VerificationTokenInvalidError, } from '../../common/error/generated-graphql-shop-errors'; -import { normalizeEmailAddress } from '../../common/index'; +import { isEmailAddressLike, normalizeEmailAddress } from '../../common/index'; import { ConfigService } from '../../config/config.service'; import { TransactionalConnection } from '../../connection/transactional-connection'; import { NativeAuthenticationMethod } from '../../entity/authentication-method/native-authentication-method.entity'; @@ -68,19 +68,25 @@ export class UserService { const entity = userType ?? (ctx.apiType === 'admin' ? 'administrator' : 'customer'); const table = `${this.configService.dbConnectionOptions.entityPrefix ?? ''}${entity}`; - return this.connection + const qb = 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('LOWER(user.identifier) = :identifier', { + .where('user.deletedAt IS NULL'); + + if (isEmailAddressLike(emailAddress)) { + qb.andWhere('LOWER(user.identifier) = :identifier', { identifier: normalizeEmailAddress(emailAddress), - }) - .andWhere('user.deletedAt IS NULL') - .getOne() - .then(result => result ?? undefined); + }); + } else { + qb.andWhere('user.identifier = :identifier', { + identifier: emailAddress, + }); + } + return qb.getOne().then(result => result ?? undefined); } /**