docs(auth): plan participant privacy migration#5128
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Confidence Score: 5/5This looks safe to merge.
|
| Filename | Overview |
|---|---|
| project/2026-06-16-participant-privacy-auth-plan.md | Adds the main migration plan for participant privacy, authentication, assessment identity, recovery, LTI, and communication flows. |
| project/2026-07-06-pr5128-review-participant-privacy-plan.md | Adds a project review note that records codebase evidence and follow-up work for the migration plan. |
| project/CODEBASE_NOTES.md | Adds auth and LTI notes relevant to participant identity and migration planning. |
Reviews (3): Last reviewed commit: "docs(auth): add evidence-based review of..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ```prisma | ||
| model ParticipantRecoveryCode { | ||
| id String @id @default(uuid()) @db.Uuid | ||
| participantId String @db.Uuid | ||
| codeHash String | ||
| label String? | ||
| usedAt DateTime? | ||
| createdAt DateTime @default(now()) | ||
|
|
||
| participant Participant @relation(fields: [participantId], references: [id], onDelete: Cascade) | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
codeHash missing @unique constraint on ParticipantRecoveryCode
The proposed ParticipantRecoveryCode schema omits a @unique on codeHash. Recovery codes are looked up by hash at redemption time — without a unique index the database cannot enforce single-match semantics, and a query like WHERE codeHash = $hash could return multiple rows across participants. This also slows down lookup and means two code slots could, in theory, end up with the same hash if the generation path has a bug. Add codeHash String @unique (or a composite @@unique([participantId, codeHash]) if per-participant deduplication is the intent).
|
|
||
| ```prisma | ||
| model AssessmentParticipantIdentity { | ||
| id String @id @default(uuid()) @db.Uuid | ||
| courseId String @db.Uuid | ||
| participantId String @db.Uuid | ||
|
|
||
| emailCiphertext Bytes | ||
| emailNonce Bytes | ||
| emailTag Bytes | ||
| emailLookupHash String // HMAC(normalizedEmail), keyed separately from encryption | ||
| emailKeyId String | ||
|
|
||
| matriculationCiphertext Bytes? | ||
| matriculationNonce Bytes? | ||
| matriculationTag Bytes? | ||
| matriculationLookupHash String? | ||
|
|
||
| source AssessmentIdentitySource // EDUID_PRIMARY, EDUID_AFFILIATION, LTI, INVITATION | ||
| verifiedAt DateTime | ||
| retentionUntil DateTime? | ||
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
|
|
||
| @@unique([courseId, participantId]) | ||
| @@unique([courseId, emailLookupHash]) | ||
| } | ||
| ``` |
There was a problem hiding this comment.
matriculationLookupHash lacks a key-id tracking field in both AssessmentParticipantIdentity and ParticipantInvitation
Both models include emailKeyId so the active HMAC key for email lookup hashes can be tracked during rotation, but the parallel matriculationLookupHash field in each model has no corresponding matriculationLookupHashKeyId. When the HMAC key is rotated, there is no way to distinguish rows whose matriculation hash was computed under the old key from rows already rehashed. The same gap appears in ParticipantInvitation (lines 311–327). A rehash job would have to update every row instead of only stale ones, with no safe incremental path. Add matriculationLookupHashKeyId String? alongside matriculationLookupHash in both models.
| Recovery file contents: | ||
|
|
||
| ```json | ||
| { | ||
| "type": "klicker-participant-recovery", | ||
| "version": 1, | ||
| "username": "chosen-name", | ||
| "participantRecoveryId": "public-random-id", | ||
| "codes": ["single-use-code-1", "..."] | ||
| } | ||
| ``` | ||
|
|
||
| Store only hashes. Show/download once. Require recovery code or passkey to reset password if the user is outside LTI. |
There was a problem hiding this comment.
participantRecoveryId but no schema model defines this field
The recovery-file JSON example (line 234) includes a participantRecoveryId field described as a "public-random-id". This implies a separate stable public lookup key distinct from id in ParticipantRecoveryCode, but no such field exists in the proposed schema. Without a dedicated recoveryFileId or equivalent on a ParticipantRecovery parent record, the recovery file cannot be matched back to a participant at redemption time (especially after a code is consumed). Clarify in the schema whether this id lives on the participant row, a new ParticipantRecoveryFile model, or one of the code rows.
| Implementation notes: | ||
|
|
||
| - `@simplewebauthn/browser` and `@simplewebauthn/server` are present in `pnpm-lock.yaml` as transitive dependencies but are not declared in package manifests. Add direct pinned dependencies in the app/package that owns passkeys before implementation. Official docs: https://simplewebauthn.dev/docs/packages/server and https://simplewebauthn.dev/docs/advanced/passkeys | ||
| - Context7 lookup for SimpleWebAuthn failed due quota; official SimpleWebAuthn docs were used instead. |
There was a problem hiding this comment.
Line 62 reads "Context7 lookup for SimpleWebAuthn failed due quota; official SimpleWebAuthn docs were used instead." This is an artifact from the document-generation process and is not meaningful to developers reading the plan. It reveals internal tooling details and may confuse future contributors. It can be removed without loss of information since the surrounding lines already cite the official SimpleWebAuthn docs directly.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| Backfill: | ||
|
|
||
| 1. For each assessment participation: | ||
| - pick best source: `ParticipantAccount.ssoEmail` preferred, else `Participant.email`, else invitation email; |
There was a problem hiding this comment.
Participant.email in the assessment identity backfill does not filter on isEmailValid. The plan's own codebase findings note that createParticipantAccount stores email and marks non-LTI accounts isEmailValid=false until the activation link is clicked. Using an unverified email to populate AssessmentParticipantIdentity would store an unverified address in a store explicitly designed for verified assessment identity — defeating the purpose of the field and potentially matching the wrong invitation during auto-accept.
| - pick best source: `ParticipantAccount.ssoEmail` preferred, else `Participant.email`, else invitation email; | |
| - pick best source: `ParticipantAccount.ssoEmail` preferred, else `Participant.email` only if `isEmailValid=true`, else invitation email; skip (flag and do not backfill) participants where `Participant.email` is present but `isEmailValid=false`; |
Review of PR #5128 against v3 codebase: verifies all cited codebase claims, confirms the six open review-bot findings, adds ten further findings (UX friction, telemetry prerequisite, consent copy, E2E/seed impact, backup alignment, analytics sync), and lays out a phased path to production readiness with per-task verification steps.
|
Added an evidence-based review of this plan in TL;DR: plan is accurate and worth merging after fixes — all 14 spot-checked codebase claims verified, all six greptile findings confirmed valid, plus ten additional findings (mandatory-recovery-at-signup UX friction, missing login-method telemetry as a hard prerequisite, consent-copy conflict at |
1 participant
Summary
project/.Impact
This is documentation-only. It creates a project plan for reducing persisted participant email data while preserving assessment identity requirements and giving existing students a clear migration path. It now also covers how manual and LTI accounts can be linked safely without email, and how duplicate course data should be resolved.
Validation
git diff --checkpassed.pnpm exec prettier --check project/2026-06-16-participant-privacy-auth-plan.md project/CODEBASE_NOTES.mdcould not run because the workspace reportsCommand "prettier" not found.Notes
No runtime or UI changes included.