Fix(OTP Login): on fresh installs and load settings from the documents table - #1056
Merged
Conversation
LukeSeers
changed the title
Migration 0005_otp_codes.sql was added but the migrations bundle (migrations-bundle.ts) — the actual source MigrationService reads, not the .sql files — was not regenerated. On a fresh install 0005 never applied: otp_codes was never created and /auth/otp/request 500'd for a real user with `no such table: otp_codes`, and migrations.test.ts failed CI (bundle stopped at 0004 while the test expects 0005). Regenerate the bundle to include 0005, and retag the OTP regression spec @smoke so CI actually selects it (the migrations/otp change never triggers the @auth-only selection). Also correct the stale spec docstring (settings now come from the documents table, not `plugins`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
loadOtpSettings hand-rolled raw SQL against the documents table and a dead fallback to the dropped legacy `plugins` table (R4 duplication). PluginService.getPlugin already reads the same plugin document (type_id='plugin', slug='otp-login') and returns its settings. Reuse it and drop the dead legacy path. Behavior unchanged: never throws, missing document or unparseable settings resolves to DEFAULT_SETTINGS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # packages/core/src/db/migrations-bundle.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix the OTP login plugin returning 500 errors on fresh installs. The legacy
pluginsandotp_codestables were dropped from migrations when plugin settings moved to thedocumentstable (#858), but the OTP plugin still tried to read its settings from the oldpluginstable. On a fresh database, every/auth/otp/*request therefore failed withD1_ERROR: no such table: plugins.The
pluginstable is intentionally gone; plugin settings are now stored in thedocumentstable (type_id='plugin',slug='otp-login'). This PR updates the OTP plugin to read its settings from thedocumentstable, restores only theotp_codesmigration that the OTP service still requires, and adds regression tests.Fixes #
Changes
otp-login-plugin/index.ts): added aloadOtpSettings(db)helper that reads settings from the plugin's row in thedocumentstable (type_id='plugin',slug='otp-login',tenant_id='default',is_current_draft=1,deleted_at IS NULL). It falls back to a guarded lookup in the legacypluginstable and then to defaults, so a missing legacy table no longer causes an error. Both/auth/otp/requestand/auth/otp/verifynow use this helper, so settings saved through the admin UI are correctly applied.packages/core/migrations/0005_otp_codes.sql: restores theotp_codestable and indexes used byOTPServicefor request, verify, and resend operations. This is the only legacy table still required by the OTP service.migrations-bundle.ts), which now contains 5 migrations, and synced the sandbox copies inmy-sonicjs-app/migrations/.migrations.test.tsfor the new migration list (0001–0005).tests/e2e/97-otp-request-route.spec.tsfor the OTP request and verify flow.Testing
Verified end-to-end against a fresh local D1 database with no
pluginstable:POST /auth/otp/request(unknown email) → 200POST /auth/otp/request(known user) → 200 + dev codePOST /auth/otp/verify(correct code) → 200, authenticated user returnedPOST /auth/otp/verify(wrong code) → 401POST /auth/otp/request(invalid email) → 400documentstable (codeExpiryMinutes: 3) → correctly honored (expiresIn: 180instead of the 600 default)Unit Tests
migrations.test.ts)no-event-without-dispatch-site.test.ts, which also fails on a clean base)E2E Tests
tests/e2e/97-otp-request-route.spec.ts)Screenshots/Videos
N/A — no UI changes.
Checklist