Move email lookup to shared read model (#176)

* Remove unused email lookup

* Remove old email edit handling that was never fully implemented

* use shared read model to get members by email

* Implementation for findByEmail using shared read model and member linking

* Fix mapAll in MemberLinking

* Remove unused

* Added more login link tests

* OpenAI codex

* Add more tests

* Log the normalised email when sending login link

Author: Lan2u

.codex/config.toml

@@ -0,0 +1,6 @@
+# Repo-local Codex defaults for the Makespace members app.
+
+model = "gpt-5.4"
+model_reasoning_effort = "high"
+approval_policy = "on-request"
+sandbox_mode = "workspace-write"

AGENTS.md

@@ -0,0 +1,90 @@
+# AGENTS.md
+
+Guidance for coding agents working in this repository.
+
+## Quick start
+
+- Runtime: Node.js 20
+- Package manager: Bun
+- Language: TypeScript
+- Test runner: Jest with `ts-jest`
+- Linting: ESLint
+- Primary workflow entrypoint: `make`
+
+Prefer these commands:
+
+- `make check` to run all the verification steps
+- `make test` to run all tests
+- `npx jest path/to/test.test.ts` to run a single test file
+- `make lint` to run ESLint
+- `make typecheck` to run check types
+- `make unused-exports` to detect unused exports
+- `make start` to start the local app stack
+- `make populate-local-dev` to seed local app stack with data
+
+Local app stack endpoints:
+
+- App: `http://localhost:8080`
+- Mailcatcher: `http://localhost:1080`
+
+## Architecture
+
+This app is built around event sourcing.
+
+- Commands live in `src/commands/`
+- Read models live in `src/read-models/`
+- Queries live in `src/queries/`
+- Authentication flows live in `src/authentication/`
+- Background sync code lives in `src/sync-worker/`
+
+Important boundaries:
+
+- Commands validate authorization and business rules, then emit domain events.
+- Commands should only use the events passed to them. They should not query read models directly.
+- Read models project events into queryable state.
+- Queries read from read models and render HTTP responses.
+- Communication across the app should happen via events, not by coupling command code to read-model code.
+
+## Project patterns
+
+- Prefer existing fp-ts patterns such as `TaskEither`, `Option`, and `pipe`.
+- Use existing io-ts types and decoders where applicable.
+- Use `deps.logger` for logging instead of `console.log`.
+- Follow the repo’s current style rather than introducing a new abstraction or library unless necessary.
+- Keep changes small and targeted.
+
+Authentication notes:
+
+- Login is passwordless and handled by magic link flows under `src/authentication/`.
+- Member numbers are linked to email addresses through domain events.
+- Be careful with authorization and session-related changes.
+
+## Testing guidance
+
+Tests live under `tests/` and mirror the source layout.
+
+- Use `.test.ts` files.
+- Command tests should assert on resulting events.
+- Read-model tests should feed events into the read model and assert on query results.
+- It is acceptable in read-model tests to use `command.process()` only to generate events for setup.
+- Avoid calling read models from command tests.
+- Tests should follow a behaviour driven testing style
+
+Helpful test utilities already used in the repo:
+
+- `faker` for test data
+
+## Working norms for agents
+
+- Prefer `rg` for searching the codebase.
+- Before broader verification, run the smallest relevant check for the files you changed when practical.
+- If you change behavior, add or update tests in the matching area under `tests/`.
+- Preserve the command/read-model separation when adding features.
+- Avoid speculative refactors unless they are necessary for the task.
+- Do not overwrite unrelated local changes.
+
+## Useful references
+
+- `README.md` for local setup and operational context
+- `CLAUDE.md` for repository guidance already written for coding assistants
+- `Makefile` for the canonical development, test, and lint commands

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

@@ -1,11 +1,9 @@
-import * as E from 'fp-ts/Either';
 import * as TE from 'fp-ts/TaskEither';
-import {flow, pipe} from 'fp-ts/lib/function';
+import {pipe} from 'fp-ts/lib/function';
 import {Dependencies} from '../dependencies';
 import {Email, EmailAddress, Failure, failure} from '../types';
 import {Config} from '../configuration';
 import {magicLink} from '.';
-import {readModels} from '../read-models';
 import mjml2html from 'mjml';
 
 const toEmail =
@@ -42,37 +40,25 @@ const toEmail =
     `).html,
   });
 
-const lookupMemberNumber = (emailAddress: string) =>
-  flow(
-    readModels.members.lookupByCaseInsensitiveEmail(emailAddress),
-    members => {
-      switch (members.length) {
-        case 0:
-          return E.left(failure('No member associated with that email')());
-        case 1:
-          return E.right(members[0]);
-        default:
-          return E.left(
-            failure(
-              'Multiple members associated with that email with diffrent capitalization. This is very likely to be a mistake.'
-            )()
-          );
-      }
-    }
-  );
-
-type SendLogInLink = (
-  deps: Dependencies,
+export const sendLogInLink = (
+  deps: Pick<Dependencies, 'sendEmail' | 'rateLimitSendingOfEmails' | 'sharedReadModel' | 'logger'>,
   conf: Config
-) => (emailAddress: EmailAddress) => TE.TaskEither<Failure, string>;
-
-export const sendLogInLink: SendLogInLink = (deps, conf) => emailAddress =>
-  pipe(
-    deps.getAllEvents(),
-    TE.chainEitherK(lookupMemberNumber(emailAddress)),
-    TE.map(magicLink.create(conf)),
-    TE.map(toEmail(emailAddress)),
-    TE.chain(deps.rateLimitSendingOfEmails),
+) => (emailAddress: EmailAddress): TE.TaskEither<Failure, string> => {
+  const members = deps.sharedReadModel.members.findByEmail(emailAddress);
+  if (members.length === 0) {
+    return TE.left(failure('No member associated with that email')());
+  }
+  if (members.length > 1) {
+    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.')());
+  }
+  // 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),
-    TE.map(() => `Sent login link to ${emailAddress}`)
+    TE.map(() => `Sent login link to ${members[0].emailAddress}`)
   );
+}

src/read-models/members/index.ts

@@ -1,8 +1,6 @@
-import {lookupByCaseInsensitiveEmail} from './lookup-by-email';
 import {getFailedImports} from './get-failed-imports';
 
 export const members = {
-  lookupByCaseInsensitiveEmail,
   getFailedImports,
 };
 

src/read-models/members/lookup-by-email.ts

@@ -7,15 +7,15 @@ import * as O from 'fp-ts/Option';
 import {createTables} from './state';
 import {BetterSQLite3Database, drizzle} from 'drizzle-orm/better-sqlite3';
 import Database from 'better-sqlite3';
-import {Area, Equipment, Member} from './return-types';
+import {Area, Equipment, Member, MemberCoreInfo} from './return-types';
 
 import {Client} from '@libsql/client';
 import {asyncRefresh} from './async-refresh';
 import {updateState} from './update-state';
 import {Logger} from 'pino';
 import {asyncApplyExternalEventSources} from './async-apply-external-event-sources';
 import {UUID} from 'io-ts-types';
-import {User} from '../../types';
+import {EmailAddress, User} from '../../types';
 import {getAllEquipmentFull, getEquipmentFull} from './equipment/helpers';
 import {getAllAreaFull, getAreaFull} from './area/helpers';
 import {
@@ -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;
@@ -44,6 +45,7 @@ export type SharedReadModel = {
     get: (memberNumber: number) => O.Option<Member>;
     getAll: () => ReadonlyArray<Member>;
     getAsActor: (user: User) => (memberNumber: number) => O.Option<Member>;
+    findByEmail: (email: EmailAddress) => ReadonlyArray<MemberCoreInfo>;
   };
   equipment: {
     get: (id: UUID) => O.Option<Equipment>;
@@ -95,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/index.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
@@ -43,6 +45,6 @@ export class MemberLinking {
   }
 
   mapAll(all: readonly MemberNumber[]): ReadonlySet<MemberNumber>[] {
-    return Array.from(new Set(all.map(this.map)));
+    return Array.from(new Set(all.map(this.map.bind(this))));
   }
 }

src/read-models/shared-state/member-linking.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/member/get.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;
+};

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

@@ -16,6 +16,7 @@ import {and, eq, inArray, sql} from 'drizzle-orm';
 import {isOwnerOfAreaContainingEquipment} from './area/helpers';
 import {pipe} from 'fp-ts/lib/function';
 import {MemberLinking} from './member-linking';
+import { normaliseEmailAddress } from './normalise-email-address';
 
 const revokeSuperuser = (db: BetterSQLite3Database, memberNumber: number) =>
   db
@@ -33,7 +34,7 @@ export const updateState =
         db.insert(membersTable)
           .values({
             memberNumber: event.memberNumber,
-            emailAddress: event.email,
+            emailAddress: normaliseEmailAddress(event.email),
             gravatarHash: gravatarHashFromEmail(event.email),
             name: O.fromNullable(event.name),
             formOfAddress: O.fromNullable(event.formOfAddress),
@@ -285,7 +286,7 @@ export const updateState =
           .set({
             status,
           })
-          .where(eq(membersTable.emailAddress, event.email))
+          .where(eq(membersTable.emailAddress, normaliseEmailAddress(event.email)))
           .run();
         break;
       }