Skip to content

fix: prevent duplicate moodle_token insert when Moodle rotates token …#400

Merged
y4nder merged 1 commit into
stagingfrom
fix/staging/duplicate-moodle-token-insert
May 12, 2026
Merged

fix: prevent duplicate moodle_token insert when Moodle rotates token …#400
y4nder merged 1 commit into
stagingfrom
fix/staging/duplicate-moodle-token-insert

Conversation

@y4nder

@y4nder y4nder commented May 12, 2026

Copy link
Copy Markdown
Member

…(#399)

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.

…399)

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 308c343 into staging May 12, 2026
2 checks passed
y4nder added a commit that referenced this pull request Jun 3, 2026
* [STAGING] fix: remove [name] redaction from qualitative summary quotes (#394) (#395)

* [STAGING] feat: added raw label on the response for topic labels (#396)

* feat: error log admin diagnostics + multi-role dean login fix (#397) (#398)

Two related changes bundled together — the dean login 500 was the
catalyst for the diagnostic page, so both ship in the same PR.

## Error log admin diagnostics

Captures unhandled 5xx exceptions so they're visible on the admin
console instead of vanishing into a generic 500 response.

- new `error_log` table (entity + migration) — never soft-deleted,
  indexed on status_code / path / user_id / error_name /
  acknowledged_at / occurred_at for filter performance
- `SystemErrorsModule` (`src/modules/system-errors/`) mirrors
  `AuditModule` shape: `ErrorLogService.Emit()` async-enqueues via
  BullMQ, `ErrorLogProcessor` persists, `ErrorLogQueryService` handles
  paginated reads + acknowledge flow, `ErrorLogController` exposes
  `/error-logs` endpoints behind a `@UseJwtGuard(SUPER_ADMIN)`
- `ErrorCaptureFilter` global exception filter: catches everything,
  only persists ≥500, redacts sensitive keys
  (`password`/`token`/`refreshToken`/`authorization` etc.) and caps
  payload depth/breadth/string length, then defers to
  `BaseExceptionFilter` so the wire response is byte-identical to today
- `ErrorLogCleanupJob` daily at 04:00 UTC — 90-day retention via
  `nativeDelete` on `occurred_at`
- 12 new unit tests covering sanitizer behaviour and filter capture
  paths (4xx skipped, 5xx captured, non-http hosts skipped, capture
  failures swallowed without re-throwing)

## Multi-role dean login fix (ucmn-t-67092)

Adds defensive guards to `MoodleUserHydrationService.resolveInstitutionalRoles`
so users with both DEAN and CHAIRPERSON institutional roles can log in
when the populated `moodleCategory` relation is null (soft-delete filter
or drift after Moodle restructure). Seven previously-unguarded
`ir.moodleCategory.moodleCategoryId` dereferences now optional-chain
+ filter; orphaned auto rows are now removed instead of crashing.
Regression tests cover both DEAN-orphan and CHAIRPERSON-orphan shapes.

Single-role deans skip the (DEAN ∧ CHAIRPERSON) cleanup branch, which
is why only the intersection user tripped the TypeError.

## Migration scope (verified safe on staging)

This migration also adds `analysis_pipeline_trigger_check` —
`@Enum(() => PipelineTrigger)` is declared on the entity but the CHECK
was missing from the DB. Verified: 0 violating rows, no name collision.

Does NOT include the CLI-suggested
`drop index uq_analysis_pipeline_active_scope`. That's the FAC-132
partial unique index — MikroORM decorators can't represent it (see
new gotcha entry in `src/entities/CLAUDE.md`) and dropping it would
reintroduce the duplicate-active-pipeline bug it fixed.

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

* 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>

* feat: add enable/disable toggle for Moodle sync cron job (#401) (#402)

Extend the sync schedule endpoint to support an `enabled` flag persisted
via SystemConfig. The scheduler uses CronJob stop/start to fully disable
the cron while keeping manual sync available.

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

---------

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