Implementation for findByEmail using shared read model and member linking

Author: Lan2u

src/authentication/send-log-in-link.ts

@@ -54,7 +54,10 @@ export const sendLogInLink: SendLogInLink = (deps, conf) => emailAddress => {
     deps.logger.error('While looking for email %s we found multiple users!', emailAddress);
     return TE.left(failure('Multiple members associated with that email. Please contact an administrator.')());
   }
-  const email = toEmail(emailAddress)(magicLink.create(conf)(members[0]));
+  // Note that we intentionally use the stored email address rather than the one provided.
+  // This prevents attacks where you specify an email address that somehow matches to an existing user but isn't
+  // actually treated the same by the mailserver(s) so gets routed differently (to the attacker).
+  const email = toEmail(members[0].emailAddress)(magicLink.create(conf)(members[0]));
   return pipe(
     deps.rateLimitSendingOfEmails(email),
     TE.chain(deps.sendEmail),

src/read-models/shared-state/index.ts

@@ -31,6 +31,7 @@ import { ReadonlyRecord } from 'fp-ts/lib/ReadonlyRecord';
 import { TrainingSheetId } from '../../types/training-sheet';
 import { EquipmentId } from '../../types/equipment-id';
 import { getTrainingSheetIdMapping } from './equipment/get';
+import { findByEmail } from './member/get';
 
 export type SharedReadModel = {
   db: BetterSQLite3Database;
@@ -96,6 +97,7 @@ export const initSharedReadModel = (
       get: getMemberFull(readModelDb, linking),
       getAll: getAllMemberFull(readModelDb, linking),
       getAsActor: getMemberAsActorFull(readModelDb, linking),
+      findByEmail: findByEmail(readModelDb, linking),
     },
     equipment: {
       get: getEquipmentFull(readModelDb, linking),

src/read-models/shared-state/member-linking.ts

@@ -1,5 +1,7 @@
 import { MemberNumber } from "../../types/member-number";
 
+// NOTE: MemberLinking is a slightly grim solution to the problem that initially it was assumed each 'person' had 1 member number but
+// actually users might have multiple.
 export class MemberLinking {
   // Stores the linking between member numbers.
   // This is much more effiently stored directly rather than using sqllite as its

src/read-models/shared-state/member/get.ts

@@ -1,13 +1,15 @@
 import {pipe} from 'fp-ts/lib/function';
 import {BetterSQLite3Database} from 'drizzle-orm/better-sqlite3';
-import {inArray, desc} from 'drizzle-orm';
+import {inArray, desc, eq} from 'drizzle-orm';
 import * as O from 'fp-ts/Option';
 import * as RA from 'fp-ts/ReadonlyArray';
 import * as RAE from 'fp-ts/ReadonlyNonEmptyArray';
 import {MemberCoreInfo} from '../return-types';
 import {membersTable} from '../state';
 import {MemberCoreInfoPreMerge, mergeMemberCore} from './merge';
 import {MemberLinking} from '../member-linking';
+import { EmailAddress } from '../../../types';
+import { normaliseEmailAddress } from '../normalise-email-address';
 
 const getMergedMember =
   (db: BetterSQLite3Database) =>
@@ -52,3 +54,29 @@ export const getAllMemberCore = (
   linking: MemberLinking
 ): ReadonlyArray<MemberCoreInfo> =>
   pipe(linking.all(), RA.filterMap(getMergedMemberSet(db)));
+
+export const findByEmail = (
+  db: BetterSQLite3Database,
+  linking: MemberLinking
+) => (email: EmailAddress): ReadonlyArray<MemberCoreInfo> => {
+  // This is a bit grim because member numbers were initially assumed to be uniquely
+  // identify a single member but actually a member can have multiple member numbers.
+  // This means we need to find all the member numbers then group them then
+  // finally use those to actually grab the merged members.
+  // A potential solution would be to introduce a proper primary key that represents a single user
+  // and then have member numbers map to the primary key 1:M.
+  const foundMemberNumbers = db.select({
+      memberNumber: membersTable.memberNumber,
+    })
+    .from(membersTable)
+    .where(eq(membersTable.emailAddress, normaliseEmailAddress(email)))
+    .orderBy(desc(membersTable.memberNumber))
+    .all()
+    .map(row => row.memberNumber);
+  const groupedMemberNumbers = linking.mapAll(foundMemberNumbers);
+  return groupedMemberNumbers.map(
+    getMergedMemberSet(db)
+  ).flatMap(
+    m => O.isSome(m) ? [m.value] : []
+  );
+}

src/read-models/shared-state/normalise-email-address.ts

@@ -0,0 +1,9 @@
+import { EmailAddress } from "../../types";
+
+export const normaliseEmailAddress = (email: EmailAddress): EmailAddress => {
+    const splitPoint = email.lastIndexOf('@');
+    if (splitPoint < 0) {
+        return email;
+    }
+    return (email.substring(0, splitPoint) + '@' + email.substring(splitPoint + 1).toLowerCase()) as EmailAddress;
+};

tests/read-models/members/lookup-by-email.test.ts

@@ -0,0 +1,29 @@
+import { normaliseEmailAddress } from "../../src/read-models/shared-state/normalise-email-address";
+import { EmailAddress } from "../../src/types/email-address";
+
+describe('normaliseEmailAddress', () => {
+  const validEmails = [
+    ['simple@example.com', 'simple@example.com'],
+    ['ALLCAPS@EXAMPLE.COM', 'ALLCAPS@example.com'],
+    ['MiXeD@ExAmPlE.cOm', 'MiXeD@example.com'],
+    ['user.name+tag@domain.co.uk', 'user.name+tag@domain.co.uk'],
+    // Emails with quoted @ in the local part should preserve case in local part
+    ['"quoted@local"@example.com', '"quoted@local"@example.com'],
+    ['"Quoted@Local"@EXAMPLE.COM', '"Quoted@Local"@example.com'],
+  ];
+
+  it.each(validEmails)('normalises valid email %s to %s', (input, expected) => {
+    expect(normaliseEmailAddress(input as EmailAddress)).toBe(expected);
+  });
+
+  const invalidEmails = [
+    ['no-at-sign', 'no-at-sign'],
+    ['', ''],
+    ['@domain.com', '@domain.com'],
+    ['local@', 'local@'],
+  ];
+
+  it.each(invalidEmails)('handles invalid email %s gracefully', (input, expected) => {
+    expect(normaliseEmailAddress(input as EmailAddress)).toBe(expected);
+  });
+});

tests/read-models/normalise-email-address.test.ts

@@ -209,10 +209,14 @@ describe('get-via-shared-read-model', () => {
     pipe(id, framework.sharedReadModel.members.get, getSomeOrFail);
 
   describe('when the member does not exist', () => {
-    it('returns none', () => {
+    it('get returns none', () => {
       const result = framework.sharedReadModel.members.get(memberNumber);
       expect(result).toStrictEqual(O.none);
     });
+    it('findByEmail returns none', () => {
+      const result = framework.sharedReadModel.members.findByEmail(memberEmail);
+      expect(result).toHaveLength(0);
+    })
   });
 
   describe('when the member exists', () => {
@@ -240,6 +244,31 @@ describe('get-via-shared-read-model', () => {
       );
     });
 
+    it('can find member by email', () => {
+      const result = framework.sharedReadModel.members.findByEmail(
+        memberEmail
+      );
+      expect(result).toHaveLength(1);
+      expect(result[0].memberNumber).toEqual(memberNumber);
+      expect(result[0].emailAddress).toEqual('foo@example.com');
+    });
+
+    it('can find member by email with case insensitive domain', () => {
+      const result = framework.sharedReadModel.members.findByEmail(
+        'foo@eXAMple.com' as EmailAddress
+      );
+      expect(result).toHaveLength(1);
+      expect(result[0].memberNumber).toEqual(memberNumber);
+      expect(result[0].emailAddress).toEqual('foo@example.com');
+    });
+
+    it('cannot find a non-existant email', () => {
+      const result = framework.sharedReadModel.members.findByEmail(
+        faker.internet.email() as EmailAddress
+      );
+      expect(result).toHaveLength(0);
+    });
+
     describe('and their name has been recorded', () => {
       const name = faker.person.fullName();
       beforeEach(async () => {