fix(auth): stop signin auth-watcher from racing navigation past validate() - #4534
Conversation
…ate() watch.auth was the sole navigator on signin: it pushed the redirect route as soon as signin() flipped authStore.auth to true. That pre-flush watcher job lands on an earlier microtask than validate()'s own `await authStore.signin()` continuation, decoupling navigation from the flow that triggered it. Any future step added after the awaited signin() (MFA challenge, forced password rotation, consent interstitial) would be silently skipped before it could run - reproducing #4437's bug class (fixed for signup in #4528) on this view. Moves navigation into validate()'s success path, explicitly after authStore.signin() resolves and authStore.auth is true, preserving the redirect-query behavior byte-for-byte (including the startsWith('/') open-redirect guard). Removes the now-unused watch.auth handler and auth computed. OAuth's full-page href redirects (handled in created()) are unaffected. Adds a regression suite that runs the real Pinia auth store (the existing suite mocks a static { auth: false } store, so the watcher never fired and the race was untested) covering the default-route branch, the redirect-query branch, the non-"/"-prefixed redirect guard, and the failed-signin (no navigation) branch. Closes #4533
Cuts the fill-email/fill-password/validate/flushPromises sequence that repeated verbatim across all 4 tests into one submitSignin() helper. Keeps each scenario as its own named it() block (each documents a distinct, security-relevant redirect case) rather than collapsing into a test.each table.
|
Warning Review limit reached
Next review available in: 17 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4534 +/- ##
=======================================
Coverage 99.58% 99.58%
=======================================
Files 37 37
Lines 1457 1457
Branches 455 455
=======================================
Hits 1451 1451
Misses 6 6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 17 minutes. |
1 participant
Summary
watch: { auth(auth) {...} }reactive handler and intovalidate()'s success path, explicitly afterawait authStore.signin()resolves andauthStore.authis confirmed true.validate()'s ownawait authStore.signin()continuation, so navigation could run beforevalidate()'s success path even started — decoupling navigation from the flow that triggered it. Any future step added after the awaitedsignin()(e.g. MFA) would have been silently skipped. Same bug class already fixed on the siblingsignup.view.vuein PR fix(auth): stop signup auth-watcher from racing past email verification #4528 / issue 🐛 fix(auth): signup auth-watcher races validate() → skips email-verification gate #4437 (there,validate()already navigated in every branch, so the watcher was purely redundant; here it was the SOLE navigator, so the fix relocates the call rather than just deleting it).Scope
authnonelowValidation
npm run lintnpm run test:unitnpm run buildGuardrails check
.env*,secrets/**, keys, tokens)Notes for reviewers
redirect.startsWith('/')) is preserved byte-for-byte — only the trigger point for navigation moved, not the redirect validation logic.auth.signin.view.race.unit.tests.js) covers 4 scenarios: default route, redirect-query, non-/-prefixed redirect ignored (open-redirect guard), and failed signin (no navigation).