Skip to content

fix: prevent duplicate moodle_token insert on Moodle token rotation#399

Merged
y4nder merged 1 commit into
developfrom
fix/moodle-token-rotated-duplicate-key
May 12, 2026
Merged

fix: prevent duplicate moodle_token insert on Moodle token rotation#399
y4nder merged 1 commit into
developfrom
fix/moodle-token-rotated-duplicate-key

Conversation

@y4nder
Copy link
Copy Markdown
Member

@y4nder y4nder commented May 12, 2026

Summary

Diagnosed via the new error-log admin page on staging — login for ucmn-t-67092 and harvie was 500'ing with:

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

`MoodleTokenRepository.UpsertFromMoodle` had two compounding issues:

  1. Rotated-token branch created a duplicate row. When the incoming token string didn't match the stored one, the code kept the existing row alive (flipping `isValid = false`) and called `this.create(...)` to insert a second row with the same `moodleUserId`. The column-level UNIQUE constraint rejected the insert at flush time. Users whose token hadn't rotated hit the in-place-update branch and worked fine — which is why this only surfaced for some users.
  2. Lookup was keyed by `user.id`, not `moodleUserId`. If the local `User` row was recreated with a fresh UUID, or the existing token was soft-deleted (hidden by the global filter), the find returned `null`, the create-path ran, and the unique constraint still saw the orphaned row.

Fix

  • Look up by `moodleUserId` with `filters: { softDelete: false }` — resilient to rotated tokens, soft-deleted rows, and re-created local users.
  • On hit, mutate in place: refresh `token` / `lastValidatedAt` / `isValid` / clear `invalidatedAt` and `deletedAt`, rebind `user` to the current local user. Never create a second row — semantic intent is "one current token per Moodle user", which matches the schema invariant.
  • Defensive precondition: throw if `user.moodleUserId` is missing (mirrors `MoodleToken.Create`).

Test plan

  • 6 new repo unit tests covering: first-login, validation refresh (same token re-presented), rotated token (the failing case), soft-deleted row revival, and the precondition guard
  • Full suite green: `npm run test` — 1154 passing
  • Lint clean on changed files
  • After deploy: `ucmn-t-67092` and `harvie` log in cleanly; verify a fresh entry doesn't reappear on the `/error-logs` page
  • Optional: check staging `moodle_token` for any orphan rows whose `user_id` no longer matches a live user — the fix will quietly reclaim them on next login, but a one-time audit is cheap

Related

🤖 Generated with Claude Code

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>
@y4nder y4nder self-assigned this May 12, 2026
@y4nder y4nder merged commit d66f453 into develop May 12, 2026
2 checks passed
y4nder added a commit that referenced this pull request May 12, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant