Enforce email verification via MustVerifyEmail on User model - #220
Merged
Conversation
The 'verified' route middleware guarding /dashboard, /endpoints, /events, and /deliveries has been structurally dead code: User never implemented Illuminate\Contracts\Auth\MustVerifyEmail, and Fortify's emailVerification feature was disabled, so the middleware silently let every request through regardless of verification status. The rest of the app already assumed this feature was live (a full VerifyEmail Vue page, a resend-link control in profile settings, and an EmailVerificationTest suite that skipped every case), so this wires it up rather than removing the middleware: - Implement MustVerifyEmail on App\Models\User. - Enable Features::emailVerification() in config/fortify.php. - Add EmailVerificationEnforcementTest covering that an unverified user is redirected away from the dashboard and a verified user can still reach it. Fixes #171 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LmKzHHTMtJob3d8jvRe6bz
The full test suite intermittently failed with a UniqueConstraintViolationException on events(user_id, name) once email verification enforcement added more factory-created events per test run, surfacing a latent bug: - WebhookSeeder generated names as `$eventType.'_'.time().'_'.rand(1000,9999)`, which can collide when multiple events of the same type are seeded within the same second. - EventFactory::definition() returned a name straight from a fixed pool with no uniqueness guarantee, so any test creating more than one event per user via the factory without an explicit name override could collide. Replace the seeder's timestamp+rand suffix with Str::ulid(), and append a Str::random(6) suffix in the factory default. Fixes #100 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LmKzHHTMtJob3d8jvRe6bz
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
1 participant
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.
What was broken
routes/web.phpwraps/dashboard,/endpoints,/events, and/deliveriesin Laravel's'verified'middleware group. That middleware (Illuminate\Auth\Middleware\EnsureEmailIsVerified) only takes action when$request->user() instanceof MustVerifyEmail— otherwise it's a silent no-op.App\Models\Usernever implementedMustVerifyEmail(the import was commented out), andFeatures::emailVerification()was commented out inconfig/fortify.php. So the'verified'middleware guarding every core dashboard route was structurally dead: it could never enforce anything, for any user, regardless of config changes to the route file alone. A reader ofroutes/web.phpwould reasonably assume unverified users are blocked from the dashboard — they never were.The rest of the app already assumed this feature was wired up: a full
VerifyEmailVue page, a resend-verification-link control in profile settings, Fortify actions that check$user instanceof MustVerifyEmail, a factoryunverified()state, and a completeEmailVerificationTestsuite — every case of which was silently skipped because the feature flag was off.What changed
App\Models\Usernow implementsIlluminate\Contracts\Auth\MustVerifyEmail.Features::emailVerification()is enabled inconfig/fortify.php.EmailVerificationEnforcementTest, asserting an unverified user is redirected away from/dashboardand a verified user can still reach it.No new routes/views/notifications were needed — Fortify auto-registers the verification prompt/notification/confirmation routes once the feature flag is on, and Jetstream's scaffolding for it was already in place.
Incidental fix
Running the full suite repeatedly surfaced a pre-existing flaky-test bug (tracked separately in #100):
WebhookSeedergenerated event names as$type.'_'.time().'_'.rand(1000,9999), andEventFactory::definition()returned event names straight from a fixed pool with no uniqueness guarantee. Both can collide against theevents(user_id, name)unique constraint when more than one event is created for the same user within the same second — which happened often enough once this PR added more per-test event creation to intermittently fail CI. Fixed both to generate collision-safe names (Str::ulid()in the seeder, a random suffix in the factory default), which fully resolves #100 as well.Fixes #171
Fixes #100
🤖 Generated with Claude Code
https://claude.ai/code/session_01LmKzHHTMtJob3d8jvRe6bz
Generated by Claude Code