Skip to content

fix(auth): tell people when sign-in is rate limited - #9114

Merged
NotThatKindOfDrLiz merged 8 commits into
mainfrom
dw-author-divinemobile-is8444-9b2035fc964503049a2b4e281650d5eae7a517fb
Sep 14, 2026
Merged

NotThatKindOfDrLiz merged 8 commits into
mainfrom
dw-author-divinemobile-is8444-9b2035fc964503049a2b4e281650d5eae7a517fb

Conversation

@dcadenas

@dcadenas dcadenas commented Sep 13, 2026 •

Copy link
Copy Markdown
Member

Description

A 429 during sign-in used to show the same generic "something went wrong" message as any other unrecognized failure, so there was no way to tell "wait and try again" apart from a real error. Sign-up had the same blind spot: a rate-limited registration collapsed into the generic registration failure, and an email that already had an account skipped the recovery options a key conflict gets.

This change classifies an HTTP 429 by status for sign-in and registration. Sign-in shows the existing accountCredentialsRateLimited copy in all 22 locales instead of adding a new string, and a duplicate-email sign-up opens the same sign-in recovery as a key conflict. A wrong password and a non-existent account remain indistinguishable, so nothing here reveals whether an account exists. Change-password, change-email and key export already surface rate limits this way; sign-in was the outlier.

Related Issue: Closes #8444

Inventory

Every user-facing Keycast-backed flow, what the client models today, and what stays intentionally generic. The audit was split out of #8432; the server-side attempt-limiting half remains tracked in divinevideo/keycast#403.

Flow Client distinguishes Intentionally generic
Sign-in 401 credentials, 403 unverified, 400 invalid email, HTTP 429, network 5xx, malformed and unknown responses offer retry only. 401 does not distinguish a missing account.
Account creation key conflict, email already exists, invalid email, HTTP 429, network/server Other codes keep the server description. Registration copy is still hardcoded English. weak_password has no Keycast producer today.
Verification poll complete/pending, duplicate email, expired, temporary (429 retried), network 429 is retried automatically and never shown. Unknown becomes a generic poll failure.
Email-link verification success, duplicate email, expired, temporary, network Opaque terminal responses use invalid-link recovery.
PIN verification success/already complete, invalid, expired, locked, unavailable, duplicate email, server, network Complete. Retry grouping is intentional where the action is the same.
Resend verification success, expired registration, endpoint unavailable, other 4xx, 5xx, network Cooldown and missing or completed registrations all return success, to avoid leaking registration state.
Password-reset request accepted vs HTTP/network failure Accepted always shows neutral "if an account exists" copy.
Password reset success vs failure message Expired and invalid tokens carry no single stable machine code.
Change password wrong password, expired session, weak password, 429, 5xx, network Server and unknown share retry copy.
Change email wrong password, expired session, invalid email, 429, 5xx, network Same, and already-registered target addresses return success by design.
Key export wrong password, expired session, unverified email, no key, 429, 5xx, network Reference for the 429-by-status behavior. Network, server and unknown share reachability copy.
Token exchange (after verification) 200 token response, non-200 JSON error, network/timeout Known limitation, recorded and not fixed: the body is decoded before the status check, so a non-JSON 429/502 reaches the generic auth-completion failure and a JSON error shows the raw server error_description. Retry is the only action either way, and changing it would alter session-recovery semantics.
Account status 200 vs null Null is never read as "no account".
Session exchange, refresh, logout structured OAuth error; refresh distinguishes a rejected token from a network failure Session recovery owns this path, not the sign-in failure copy.
Direct Keycast deletion success, reauthentication, missing account, server, network No production app call site; deletion runs through the separate account-deletion flow.

Out of Scope

  • Localizing registration errors. That needs a SignUpFailureReason enum.
  • Server-side attempt limiting. This change classifies the client response; the server half is divinevideo/keycast#403.
  • Retry-After display.
  • A password-reset token taxonomy. The expired-token path is a success:false 200 body with no machine code, while an invalid token is a 401.
  • Token-exchange error taxonomy and its raw error_description exposure, recorded in the inventory above.

Verification

  • dart format --output=none --set-exit-if-changed lib test integration_test tools: clean (repo-wide).
  • flutter analyze lib test integration_test tools: no issues (repo-wide, the CI analyze job).
  • flutter analyze and flutter test in mobile/packages/keycast_flutter: clean, 323 tests passed.
  • flutter test test/blocs/divine_auth/ test/screens/auth/: 242 tests passed.

Not run locally: the full Mobile CI test job, which is a sharded multi-command script. A plain run over mobile/test/ was stopped at a 900 second timeout under concurrent machine load, so the full-suite signal comes from CI. The generated-files ratchets and the package's coverage workflow are CI-only.

Type of Change

  • Bug fix

@dcadenas
dcadenas requested a review from a team as a code owner September 13, 2026 00:19
@dcadenas
dcadenas requested review from hm21 and rabble and removed request for a team September 13, 2026 00:19
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown

Mobile PR Preview

Preview refreshed for f080fe4

Last refresh: f080fe4 at 2026-09-13 01:00:15 UTC (preview run)

Property Value
Preview URL https://338a4eb5.openvine-app.pages.dev
Pages project openvine-app
Preview branch pr-9114
PR branch dw-author-divinemobile-is8444-9b2035fc964503049a2b4e281650d5eae7a517fb
Commit f080fe4

@NotThatKindOfDrLiz

Copy link
Copy Markdown
Member

Takeover update: I pushed f080fe4f2 to resolve the final review findings.

  • Added one typed registration-failure classification shared by both account-creation entry points.
  • Reused the existing response-error decoder so OAuth error_description consistently wins over a machine error token.
  • Removed stale unsupported registration-code handling and its tests, and added regression coverage for classification and field precedence.

Verification on this head:

  • pre-push analysis passed
  • 303 changed-file tests passed
  • all GitHub checks passed, including four Mobile CI test shards, Generated Files, Analyze, Keycast flutter CI, and service integration tests

@dcadenas please review the takeover changes and merge when you are ready. @rabble and @hm21 remain the requested reviewers.

@NotThatKindOfDrLiz NotThatKindOfDrLiz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved at f080fe4f2. The takeover fixes resolve the parsing duplication, centralize registration failure classification, remove the stale error-code documentation/coverage, and all required checks are green.

@NotThatKindOfDrLiz

Copy link
Copy Markdown
Member

Overlap heads-up: this PR and #9113 both change DivineAuthCubit and DivineAuthState. Please coordinate merge order; whichever lands later should rebase onto fresh origin/main and verify rate-limit messaging remains intact after invite-code onboarding is removed.

@NotThatKindOfDrLiz
NotThatKindOfDrLiz added this pull request to the merge queue Sep 14, 2026
Merged via the queue into main with commit d17001e Sep 14, 2026
23 checks passed
@NotThatKindOfDrLiz
NotThatKindOfDrLiz deleted the dw-author-divinemobile-is8444-9b2035fc964503049a2b4e281650d5eae7a517fb branch September 14, 2026 23:50
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.

task(auth): audit which Keycast responses collapse into a generic sign-in error

2 participants