Improve Email Verification

[matmanna]

• cooldown is currently set to 10mins

[matmanna]

@greptileai review por favor?

[greptile-apps]

Greptile Summary

This PR adds email verification flows: users can add email addresses, receive a verification link, resend with a 10-minute cooldown, and unlink emails from the Integrations settings page. The EmailVerificationRequest model and related controller actions are well-structured, and test coverage is solid.

• Deadlock on expiry: add_email uses EmailVerificationRequest.exists? without filtering expired/deleted records, and unlink_email uses .valid which can't see expired requests — together preventing any user whose 30-minute window lapses from ever recovering that email address without admin intervention.
• Stuck expired request: The same .valid filter in unlink_email means expired (but not soft-deleted) pending requests are invisible in the UI and cannot be cancelled.

Confidence Score: 4/5

Not safe to merge as-is — expired verification requests create an irrecoverable deadlock that locks users out of an email address permanently.

Two P1 bugs interact: the unscoped exists? in add_email and the .valid filter in unlink_email together mean that once a 30-minute verification window lapses, there is no UI path for the user to escape. Both fixes are one-liners but the issue affects the primary user path for the new feature.

app/controllers/sessions_controller.rb — lines 194 (add_email exists? check) and 261 (unlink_email .valid scope)

Important Files Changed

Filename	Overview
app/controllers/sessions_controller.rb	Adds add_email, resend_email_verification, unlink_email, and updated token actions; two P1 bugs: exists? check doesn't exclude expired/deleted records, and unlink_email uses .valid which can't reach expired requests — together causing a permanent deadlock state.
app/models/email_verification_request.rb	New model with cooldown, soft-delete, token refresh, and verify! logic; model-level uniqueness correctly scoped to non-deleted rows, but the controller pre-checks (not the model) contain the bugs.
app/controllers/settings/integrations_controller.rb	Adds emails section to section_props, correctly using .valid scope and mapping cooldown data; looks clean.
app/javascript/pages/Users/Settings/Integrations.svelte	Adds email address list UI with resend/remove forms; functional but manually handles CSRF via onMount instead of Inertia form utilities per project rules.
app/javascript/pages/Users/Settings/types.ts	Adds EmailProps type and IntegrationsPageProps; types are accurate and complete for the new email feature.
config/routes.rb	Adds resend_email_verification and related email auth routes as explicit paths rather than resources helper; functional but diverges from the project convention of using resources where possible.
test/controllers/sessions_controller_test.rb	Good test coverage for the new email verification flows including cooldown enforcement, resend after cooldown, unlink, and token verification; edge cases like expired tokens also covered.
app/controllers/settings/base_controller.rb	Adds resend_email_verification_path and unlink_email_path to shared paths_props; straightforward addition, no issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User submits add_email] --> B{EmailAddress.exists?}
    B -- yes --> C[Alert: already associated]
    B -- no --> D{EmailVerificationRequest.exists?\nNO expiry/deleted filter ⚠️}
    D -- yes --> E[Alert: already pending\neven if expired or deleted!]
    D -- no --> F[Create EmailVerificationRequest\nexpires in 30min]
    F --> G[Send verification email]
    G --> H{User clicks link\nbefore 30min?}
    H -- yes --> I[token action:\nEmailVerificationRequest.valid]
    I --> J[verify! → create EmailAddress\n+ soft_delete request]
    H -- no --> K[Request expires]
    K --> L{User tries resend}
    L --> M[resend_email_verification\nuses .valid scope]
    M -- expired not found --> N[Alert: No pending verification]
    K --> O{User tries unlink}
    O --> P[unlink_email\nuses .valid scope ⚠️]
    P -- expired not found --> Q[Alert: Email must exist to be removed]
    K --> R{User tries re-add}
    R --> D
    D -- expired record found --> E
    N & Q & E --> S[User permanently locked out\nfrom this email address]

Loading

Comments Outside Diff (1)

1. app/controllers/sessions_controller.rb, line 194-196 (link)

   Deadlock on expired verification requests

   EmailVerificationRequest.exists?(email: email) matches all records, including soft-deleted and expired ones. Once a 30-minute request expires, the user is completely locked out: resend_email_verification uses .valid (which filters expires_at > Time.current), unlink_email also uses .valid for its pending-request lookup, and this line blocks re-adding. There is no UI path to escape this state.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: app/controllers/sessions_controller.rb
   Line: 194-196
   
   Comment:
   **Deadlock on expired verification requests**
   
   `EmailVerificationRequest.exists?(email: email)` matches all records, including soft-deleted and expired ones. Once a 30-minute request expires, the user is completely locked out: `resend_email_verification` uses `.valid` (which filters `expires_at > Time.current`), `unlink_email` also uses `.valid` for its pending-request lookup, and this line blocks re-adding. There is no UI path to escape this state.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: app/controllers/sessions_controller.rb
Line: 194-196

Comment:
**Deadlock on expired verification requests**

`EmailVerificationRequest.exists?(email: email)` matches all records, including soft-deleted and expired ones. Once a 30-minute request expires, the user is completely locked out: `resend_email_verification` uses `.valid` (which filters `expires_at > Time.current`), `unlink_email` also uses `.valid` for its pending-request lookup, and this line blocks re-adding. There is no UI path to escape this state.

```suggestion
    if EmailVerificationRequest.valid.exists?(email: email)
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: app/controllers/sessions_controller.rb
Line: 260-261

Comment:
**Expired pending requests cannot be removed**

The `.valid` scope requires `expires_at > Time.current`, so an expired (but non-deleted) verification request is invisible here and can't be cancelled. Combined with the `add_email` check above, users who miss the 30-minute window end up permanently blocked from that email address in the UI.

```suggestion
      pending_request = current_user.email_verification_requests.where(deleted_at: nil).find_by(email: email)
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: app/javascript/pages/Users/Settings/Integrations.svelte
Line: 200-219

Comment:
**Manual CSRF handling instead of Inertia form utilities**

The resend and remove forms (and others in this file) manually fetch the CSRF token via `onMount` + `document.querySelector`. The project rules prefer Inertia's `useForm` / `router.post` utilities, which handle CSRF, validation state, and redirects automatically. This is a pattern across the entire component that could be addressed as a follow-up.

**Rule Used:** What: Always use Inertia's `<Form>` or `useForm` u... ([source](https://app.greptile.com/review/custom-context?memory=03426195-806a-4980-ad3d-f1c3db3c22fb))

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "feat: improve email verification flows" | Re-trigger Greptile}

[greptile-apps]

Expired pending requests cannot be removed

The .valid scope requires expires_at > Time.current, so an expired (but non-deleted) verification request is invisible here and can't be cancelled. Combined with the add_email check above, users who miss the 30-minute window end up permanently blocked from that email address in the UI.

Suggested change

	unless email_record
	redirect_to my_settings_path, alert: "Email must exist to be unlinked"
	pending_request = current_user.email_verification_requests.valid.find_by(email: email)
	pending_request = current_user.email_verification_requests.where(deleted_at: nil).find_by(email: email)

Prompt To Fix With AI

This is a comment left during a code review.
Path: app/controllers/sessions_controller.rb
Line: 260-261

Comment:
**Expired pending requests cannot be removed**

The `.valid` scope requires `expires_at > Time.current`, so an expired (but non-deleted) verification request is invisible here and can't be cancelled. Combined with the `add_email` check above, users who miss the 30-minute window end up permanently blocked from that email address in the UI.

```suggestion
      pending_request = current_user.email_verification_requests.where(deleted_at: nil).find_by(email: email)
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Manual CSRF handling instead of Inertia form utilities

The resend and remove forms (and others in this file) manually fetch the CSRF token via onMount + document.querySelector. The project rules prefer Inertia's useForm / router.post utilities, which handle CSRF, validation state, and redirects automatically. This is a pattern across the entire component that could be addressed as a follow-up.

Rule Used: What: Always use Inertia's <Form> or useForm u... (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: app/javascript/pages/Users/Settings/Integrations.svelte
Line: 200-219

Comment:
**Manual CSRF handling instead of Inertia form utilities**

The resend and remove forms (and others in this file) manually fetch the CSRF token via `onMount` + `document.querySelector`. The project rules prefer Inertia's `useForm` / `router.post` utilities, which handle CSRF, validation state, and redirects automatically. This is a pattern across the entire component that could be addressed as a follow-up.

**Rule Used:** What: Always use Inertia's `<Form>` or `useForm` u... ([source](https://app.greptile.com/review/custom-context?memory=03426195-806a-4980-ad3d-f1c3db3c22fb))

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!