Skip to content

Commit

Permalink
fix(core): Allow case-sensitive Administrator identifiers
Browse files Browse the repository at this point in the history
Fixes #2485
  • Loading branch information
michaelbromley committed Nov 15, 2023
1 parent cb13de4 commit 6527e23
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 12 deletions.
37 changes: 34 additions & 3 deletions packages/core/e2e/administrator.e2e-spec.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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<CurrentUser> = 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`
Expand Down
55 changes: 54 additions & 1 deletion packages/core/src/common/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, it } from 'vitest';

import { convertRelationPaths } from './utils';
import { convertRelationPaths, isEmailAddressLike, normalizeEmailAddress } from './utils';

describe('convertRelationPaths()', () => {
it('undefined', () => {
Expand Down Expand Up @@ -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);
});
});
13 changes: 12 additions & 1 deletion packages/core/src/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

/**
Expand Down
20 changes: 13 additions & 7 deletions packages/core/src/service/services/user.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}

/**
Expand Down

0 comments on commit 6527e23

Please sign in to comment.