Skip to content

P0: fix(auth): ignore incomplete OTP submissions - #216

Draft
aspiers wants to merge 2 commits into
mainfrom
split-pr165/ignore-incomplete-otp-submit
Draft

P0: fix(auth): ignore incomplete OTP submissions#216
aspiers wants to merge 2 commits into
mainfrom
split-pr165/ignore-incomplete-otp-submit

Conversation

@aspiers

@aspiers aspiers commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Stop empty or partial verification codes before they reach better-auth. This avoids misleading invalid-code errors and prevents incomplete submissions from spending a verification attempt.

Verify is disabled until every OTP slot is filled, so an incomplete code is visibly un-submittable rather than a click that silently does nothing. The submit-time guard remains as a backstop for the paths that reach submit without the button (Enter, autofill, requestSubmit()).

Changes

  • Disable Verify until every OTP slot is filled, driven by the filled-slot count
  • Keep the submit-handler completeness guard as a backstop
  • Focus the first unfilled slot when a bypassing submit is ignored
  • Never re-enable Verify mid-verify or after the flow is aborted — including on the failed-verify path, which clears the boxes
  • Re-sync at load so a bfcache-restored page can't strand the button disabled
  • Fade the disabled button to 0.6 opacity (reads as inert; keeps the label at ~4.9:1, WCAG AA) and stop :hover brightening it
  • Add unit and Cucumber coverage

Testing

  • pnpm format:check
  • pnpm lint
  • pnpm typecheck
  • pnpm test
  • pnpm test:coverage

Screenshots

Before: submitting a partial code sent it for verification and showed a misleading invalid-code error. The preview response was controlled to isolate this state.

Before: partial OTP reported as invalid

After: clicking Verify with only part of the code entered sends no verification request, leaves the controls editable, and shows no false error.

After: incomplete OTP remains editable

Notes

  • Rebased onto current main at a6cf09d; conflicts with the newer “Use different email” coverage were resolved by retaining both behaviors.
  • The completeness guard now applies cleanly to the segmented OTP boxes already on main.
  • Current rebased SHA aa1abc7 passed formatting, lint, typecheck, production build, unit tests, coverage, Cucumber definition validation, and the full deployed E2E suite in run 30652050690.
  • Focused extraction and review of work originally proposed in [WIP] many usability fixes (needs untangling) #165.
  • Follow-up commit c0f0867 disables the button (previously the guard was submit-time only, leaving Verify enabled but inert). All 17 checks green on c0f0867; the deployed E2E suite ran 73 scenarios / 494 steps, all passing, including both new @verify-incomplete-otp scenarios.

@changeset-bot

changeset-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 48964db

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
epds-demo Ready Ready Preview Aug 4, 2026 10:10pm

Request Review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7d2503f8-1cbe-4ace-8e3e-a0fd3d103157

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@railway-app

railway-app Bot commented Jul 30, 2026

Copy link
Copy Markdown

🚅 Deployed to the pr-80d44f-216 environment in ePDS

Service Status Web Updated (UTC)
@certified-app/demo untrusted ✅ Success (View Logs) Web Aug 4, 2026 at 10:10 pm
@certified-app/demo ✅ Success (View Logs) Web Aug 4, 2026 at 10:10 pm
@certified-app/pds-core ✅ Success (View Logs) Web Aug 4, 2026 at 10:10 pm
@certified-app/auth-service ✅ Success (View Logs) Web Aug 4, 2026 at 10:10 pm

@coveralls-official

coveralls-official Bot commented Jul 30, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 30955306853

Coverage remained the same at 57.909%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 3106
Covered Lines: 1789
Line Coverage: 57.6%
Relevant Branches: 1945
Covered Branches: 1136
Branch Coverage: 58.41%
Branches in Coverage %: Yes
Coverage Strength: 9.86 hits per line

💛 - Coveralls

@aspiers aspiers changed the title fix(auth): ignore incomplete OTP submissions P0: fix(auth): ignore incomplete OTP submissions Jul 30, 2026
Comment thread packages/auth-service/src/__tests__/login-page.test.ts Outdated
Comment thread features/passwordless-authentication.feature Outdated

@aspiers aspiers left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention is good here but very flawed testing.

@aspiers

aspiers commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

The PR-attached E2E check is red only because its SHA-specific Railway deployment lookup timed out before the preview deployment was discoverable. The same commit passed the full deployed suite when run explicitly against ePDS / ePDS-pr-216: https://github.com/hypercerts-org/ePDS/actions/runs/30549081436

@aspiers
aspiers force-pushed the split-pr165/ignore-incomplete-otp-submit branch from e244ebd to ce5653a Compare July 30, 2026 15:39
@railway-app
railway-app Bot temporarily deployed to ePDS / ePDS-pr-216 July 30, 2026 15:40 Destroyed
@aspiers aspiers closed this Jul 31, 2026
@aspiers aspiers reopened this Jul 31, 2026
@railway-app
railway-app Bot temporarily deployed to ePDS / pr-80d44f-216 July 31, 2026 11:51 Destroyed
@aspiers

aspiers commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

(reply generated by OpenAI Codex)

Closing and reopening the PR created a fresh Railway preview for the unchanged current SHA. The required check has now run the full E2E suite and passed: https://github.com/hypercerts-org/ePDS/actions/runs/30557766403

aspiers and others added 2 commits August 4, 2026 23:02
The incomplete-submit guard silently swallowed the click, so Verify
looked live but did nothing until every box was filled. Render the
button disabled and drive its state from the filled-slot count, so an
incomplete code is visibly un-submittable rather than a dead click.

The submit-handler guard stays as a backstop: Enter, autofill and
requestSubmit() all reach submit without going through the button.

Sync from updateHiddenCode()/clearOtpBoxes() — the two chokepoints where
the code value changes — plus once at load, so a bfcache-restored page
that repopulates the boxes without firing input can't strand the button
disabled. The helper bails while verifying or after the flow is aborted;
those owners disabled the button for reasons unrelated to length. That
also replaces the unconditional re-enable on the failed-verify path,
which would otherwise re-enable Verify over the boxes it just cleared.

Drop the disabled button to 0.6 opacity so it reads as inert; 0.6 is the
floor that keeps the white label at ~4.9:1 on the default brand colour
(WCAG AA). Stop :hover brightening a disabled button, which read as
clickable.

The e2e steps now submit the form directly rather than clicking, since a
disabled button is never actionable, and assert the disabled state.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Aug 4, 2026

Copy link
Copy Markdown

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