fix: prevent duplicate moodle_token insert when Moodle rotates token (#399) (#400)

Surfaced by the new error log page on staging — login for
`ucmn-t-67092` (and `harvie`) was failing with

  duplicate key value violates unique constraint
  "moodle_token_moodle_user_id_unique"
  Key (moodle_user_id)=(5) already exists

The "rotated token" branch in `MoodleTokenRepository.UpsertFromMoodle`
kept the existing row alive (just flipping `isValid = false`) and then
called `this.create(...)` to insert a *second* row carrying the same
`moodleUserId`. The column-level UNIQUE constraint on `moodle_user_id`
rejected the insert at flush time → unhandled 500. Users whose token
had not yet rotated (string-equal to the stored one) hit the second
branch which updated in place and worked, which is why this was
intermittent.

Two compounding factors:

1. The find was keyed by `user.id`. When the local `User` row was
   recreated with a fresh UUID (or the existing token was soft-deleted),
   the find returned `null`, the create-path ran, and the unique
   constraint still saw the orphaned row.

2. The global soft-delete filter hid candidate rows that would have
   matched, so an in-place mutation never happened.

Fix:

- Look up by `moodleUserId` (the unique key) with
  `filters: { softDelete: false }` so the find is resilient to rotated
  tokens, soft-deleted rows, and re-created local users.
- On hit, mutate the row in place: new token string, refreshed
  validation timestamps, rebind `user` to the current local user, clear
  `deletedAt`. Never create a second row — the unique constraint forbids
  it and the semantic intent ("one current token per Moodle user") is
  preserved.
- Defensive precondition: throw if `user.moodleUserId` is missing,
  mirroring `MoodleToken.Create`.

Adds repo-level unit tests covering: first-login, validation refresh
(same token re-presented), rotated token (the failing case), and
soft-deleted row revival.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: y4nder

src/repositories/moodle-token.repository.spec.ts

@@ -0,0 +1,154 @@
+import { MoodleTokenRepository } from './moodle-token.repository';
+import { MoodleToken } from '../entities/moodle-token.entity';
+import { User } from '../entities/user.entity';
+import type { MoodleTokenResponse } from '../modules/moodle/lib/moodle.types';
+
+// Repo-level unit test exercising the unique-constraint-safe upsert.
+// Mocks the protected `findOne`/`create` surface MikroORM's EntityRepository
+// exposes so we can verify the lookup key + mutation behaviour without a DB.
+describe('MoodleTokenRepository.UpsertFromMoodle', () => {
+  const buildUser = (overrides: Partial<User> = {}): User =>
+    ({
+      id: 'user-uuid-current',
+      moodleUserId: 5,
+      ...overrides,
+    }) as User;
+
+  const buildTokenResponse = (token: string): MoodleTokenResponse =>
+    ({ token }) as MoodleTokenResponse;
+
+  const buildRepo = (
+    findOneResult: MoodleToken | null,
+  ): {
+    repo: MoodleTokenRepository;
+    findOne: jest.Mock;
+    create: jest.Mock;
+  } => {
+    const findOne = jest.fn().mockResolvedValue(findOneResult);
+    const create = jest.fn((data: Partial<MoodleToken>) => data as MoodleToken);
+    const repo = Object.create(
+      MoodleTokenRepository.prototype,
+    ) as MoodleTokenRepository;
+    Object.assign(repo, { findOne, create });
+    return { repo, findOne, create };
+  };
+
+  it('throws when the user has no moodleUserId (defensive precondition)', async () => {
+    const { repo } = buildRepo(null);
+    const user = buildUser({ moodleUserId: undefined });
+
+    await expect(
+      repo.UpsertFromMoodle(user, buildTokenResponse('abc')),
+    ).rejects.toThrow(/moodleUserId/);
+  });
+
+  it('looks up by moodleUserId (not user.id) and includes soft-deleted rows', async () => {
+    const { repo, findOne } = buildRepo(null);
+    const user = buildUser();
+
+    await repo.UpsertFromMoodle(user, buildTokenResponse('abc'));
+
+    expect(findOne).toHaveBeenCalledWith(
+      { moodleUserId: 5 },
+      { filters: { softDelete: false } },
+    );
+  });
+
+  it('creates a new row when no token exists for this moodleUserId', async () => {
+    const { repo, create } = buildRepo(null);
+    const user = buildUser();
+
+    const result = await repo.UpsertFromMoodle(
+      user,
+      buildTokenResponse('new-token'),
+    );
+
+    expect(create).toHaveBeenCalledTimes(1);
+    expect(result.token).toBe('new-token');
+    expect(result.moodleUserId).toBe(5);
+    expect(result.user).toBe(user);
+  });
+
+  it('mutates in place when the same token is re-presented (validation refresh)', async () => {
+    const existing = {
+      id: 'token-uuid',
+      token: 'same-token',
+      moodleUserId: 5,
+      isValid: true,
+      lastValidatedAt: new Date('2026-01-01T00:00:00Z'),
+      invalidatedAt: new Date('2026-01-02T00:00:00Z'),
+      user: { id: 'user-uuid-current' } as User,
+      deletedAt: undefined,
+    } as MoodleToken;
+    const { repo, create } = buildRepo(existing);
+    const user = buildUser();
+
+    const result = await repo.UpsertFromMoodle(
+      user,
+      buildTokenResponse('same-token'),
+    );
+
+    expect(create).not.toHaveBeenCalled();
+    expect(result).toBe(existing);
+    expect(result.token).toBe('same-token');
+    expect(result.isValid).toBe(true);
+    expect(result.invalidatedAt).toBeUndefined();
+    expect(result.lastValidatedAt!.getTime()).toBeGreaterThan(
+      new Date('2026-01-01T00:00:00Z').getTime(),
+    );
+  });
+
+  it('mutates in place on a rotated token (FAC fix: previously created a duplicate row)', async () => {
+    const existing = {
+      id: 'token-uuid',
+      token: 'old-token',
+      moodleUserId: 5,
+      isValid: true,
+      lastValidatedAt: new Date('2026-01-01T00:00:00Z'),
+      user: { id: 'previous-user-uuid' } as User, // intentionally different
+      deletedAt: undefined,
+    } as MoodleToken;
+    const { repo, create } = buildRepo(existing);
+    const user = buildUser({ id: 'user-uuid-current' });
+
+    const result = await repo.UpsertFromMoodle(
+      user,
+      buildTokenResponse('new-rotated-token'),
+    );
+
+    // Critical: no second row created — the unique constraint on moodleUserId
+    // would have rejected it. We update the existing row instead.
+    expect(create).not.toHaveBeenCalled();
+    expect(result).toBe(existing);
+    expect(result.token).toBe('new-rotated-token');
+    expect(result.isValid).toBe(true);
+    expect(result.invalidatedAt).toBeUndefined();
+    // Also rebinds to the current local user so the FK stays consistent
+    // when the local row was re-created with a fresh UUID.
+    expect(result.user).toBe(user);
+  });
+
+  it('revives a soft-deleted token instead of creating a duplicate', async () => {
+    const softDeleted = {
+      id: 'token-uuid',
+      token: 'old-token',
+      moodleUserId: 5,
+      isValid: false,
+      user: { id: 'user-uuid-current' } as User,
+      deletedAt: new Date('2025-12-01T00:00:00Z'),
+    } as MoodleToken;
+    const { repo, create } = buildRepo(softDeleted);
+    const user = buildUser();
+
+    const result = await repo.UpsertFromMoodle(
+      user,
+      buildTokenResponse('fresh-token'),
+    );
+
+    expect(create).not.toHaveBeenCalled();
+    expect(result).toBe(softDeleted);
+    expect(result.token).toBe('fresh-token');
+    expect(result.isValid).toBe(true);
+    expect(result.deletedAt).toBeUndefined();
+  });
+});

src/repositories/moodle-token.repository.ts

@@ -1,31 +1,49 @@
 import { EntityRepository } from '@mikro-orm/postgresql';
 import { MoodleToken } from '../entities/moodle-token.entity';
-import { User } from '../entities/user.entity';
 import { MoodleTokenResponse } from '../modules/moodle/lib/moodle.types';
+import { User } from '../entities/user.entity';
 
 export class MoodleTokenRepository extends EntityRepository<MoodleToken> {
+  /**
+   * Upserts the Moodle token for a user. `moodle_token.moodle_user_id` carries
+   * a column-level UNIQUE constraint, so at most one row exists per Moodle
+   * user. Look up by `moodleUserId` (the unique key) — not `user.id` — so the
+   * lookup survives:
+   *
+   *   1. Rotated tokens. Moodle issues a new token string on the next login;
+   *      we mutate the existing row in place rather than insert a duplicate
+   *      with the same `moodleUserId` (the previous implementation did the
+   *      latter and tripped the unique constraint).
+   *   2. Soft-deleted rows. Postgres enforces UNIQUE on every row, so a
+   *      soft-deleted token still blocks an insert. Including soft-deleted
+   *      rows in the find lets us revive the existing row instead.
+   *   3. Re-created local User. If the local `user` row was rebuilt with a
+   *      fresh UUID, the old token's `user_id` FK no longer matches the
+   *      current user — but `moodleUserId` still does.
+   */
   async UpsertFromMoodle(user: User, moodleTokens: MoodleTokenResponse) {
-    let moodleToken = await this.findOne({
-      user: {
-        id: user.id,
-      },
-    });
+    if (!user.moodleUserId) {
+      throw new Error(
+        'Cannot upsert MoodleToken for user without moodleUserId',
+      );
+    }
 
-    if (moodleToken === null) {
-      // first token
-      moodleToken = this.create(MoodleToken.Create(user, moodleTokens));
-    } else if (moodleToken.token === moodleTokens.token) {
-      // same token
-      moodleToken.lastValidatedAt = new Date();
-      moodleToken.invalidatedAt = undefined;
-      moodleToken.isValid = true;
-    } else {
-      // rotated token
-      moodleToken.isValid = false;
-      moodleToken.invalidatedAt = new Date();
+    const existing = await this.findOne(
+      { moodleUserId: user.moodleUserId },
+      { filters: { softDelete: false } },
+    );
+
+    if (existing === null) {
       return this.create(MoodleToken.Create(user, moodleTokens));
     }
 
-    return moodleToken;
+    existing.user = user;
+    existing.token = moodleTokens.token;
+    existing.lastValidatedAt = new Date();
+    existing.invalidatedAt = undefined;
+    existing.isValid = true;
+    existing.deletedAt = undefined;
+
+    return existing;
   }
 }