Skip to content

Improve Email Verification#1134

Open
matmanna wants to merge 6 commits intohackclub:mainfrom
matmanna:email-verification
Open

Improve Email Verification#1134
matmanna wants to merge 6 commits intohackclub:mainfrom
matmanna:email-verification

Conversation

@matmanna
Copy link
Copy Markdown
Member

@matmanna matmanna commented Apr 5, 2026

  • fixes bugs where email verification requests which expire softlocks adding a new email
  • adds rerequests
    • cooldown is currently set to 10mins

#1139 needs to be merged for tests to pass on this pr :pf:

@matmanna
Copy link
Copy Markdown
Member Author

@greptileai review por favor?

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 10, 2026

Greptile Summary

This PR improves email verification by fixing the bug where expired (but undeleted) pending requests blocked users from re-adding the same email, and adds a resend-verification feature with a 10-minute cooldown. Tests are included for the new resend and unlink-pending-email paths.

One UX inconsistency worth addressing: for expired verification requests where the cooldown has also elapsed, the Resend button is disabled but labeled "Resend soon", implying a short wait will help — when in reality no wait will ever enable it (the server rejects resends for expired tokens via the .valid scope). The user must instead use the "Remove" button and re-add the address.

Confidence Score: 5/5

Safe to merge — the core logic is correct, all paths are scoped to the current user, and tests cover the new flows.

All findings are P2 (UX label wording). The bug fixes are correct, resend cooldown is properly enforced, and the soft-delete approach consistently prevents the softlock. Tests were added for the new behaviour.

app/javascript/pages/Users/Settings/Integrations.svelte — minor label wording for expired tokens.

Important Files Changed

Filename Overview
app/models/email_verification_request.rb Adds RESEND_COOLDOWN constant, resend_available?/resend_cooldown_seconds helpers, refresh_for_resend!, and fixes the expired scope to also exclude soft-deleted records. Logic is sound.
app/controllers/sessions_controller.rb Adds resend_email_verification action (scoped to current_user, valid scope, cooldown guard) and extends unlink_email to soft-delete pending requests. Also fixes add_email to filter by deleted_at: nil. All paths are correctly scoped to current_user.
app/controllers/settings/integrations_controller.rb Refactors email list to include pending (non-deleted) verification requests alongside verified addresses, with per-entry pending/expired/can_resend metadata. Logic is correct.
app/javascript/pages/Users/Settings/Integrations.svelte Renders pending emails with Unverified/Expired badges and a Resend form. Minor UX issue: button label "Resend soon" is misleading for expired tokens.
app/javascript/pages/Users/Settings/types.ts Adds resend_email_verification_path to PathsProps and expands EmailProps with pending/expired/can_resend/resend_cooldown_seconds fields. Types match the controller output.
config/routes.rb Adds post /auth/email/resend_verification route, consistent with adjacent email auth routes.
app/controllers/settings/base_controller.rb Adds resend_email_verification_path to shared paths_props — straightforward one-liner addition.
test/controllers/sessions_controller_test.rb Adds four tests covering resend cooldown, resend after cooldown, unlink pending, and unlink expired pending. Good coverage of the happy paths and the unlock-expired-email flow.

Sequence Diagram

sequenceDiagram
    participant U as User (Browser)
    participant S as SessionsController
    participant M as EmailVerificationRequest
    participant Mail as EmailVerificationMailer

    U->>S: POST /auth/email/add {email}
    S->>M: where(deleted_at: nil).exists?(email)
    alt Already pending/verified
        S-->>U: redirect — already pending
    else
        S->>M: create!(email, token, expires_at: +30min)
        S->>Mail: deliver verification email
        S-->>U: redirect — check your email
    end

    U->>S: POST /auth/email/resend_verification {email}
    S->>M: valid.find_by(email) [scope: expires_at > now AND deleted_at IS NULL]
    alt Not found (expired or deleted)
        S-->>U: redirect — no pending verification
    else Found
        S->>M: resend_available?
        alt Within 10-min cooldown
            S-->>U: redirect — please wait N minutes
        else Cooldown passed
            S->>M: refresh_for_resend! (new token, expires_at: +30min)
            S->>Mail: deliver verification email
            S-->>U: redirect — verification email resent
        end
    end

    U->>S: DELETE /auth/email/unlink {email}
    alt Verified email exists
        S->>M: find any associated verification request
        S->>M: email_record.destroy!
        S->>M: soft_delete! verification request (if any)
        S-->>U: redirect — email unlinked
    else Only pending request exists
        S->>M: where(deleted_at: nil).find_by(email)
        S->>M: soft_delete!
        S-->>U: redirect — pending email removed
    end

    U->>S: GET /auth/token/:token
    S->>M: valid.find_by(token)
    S->>M: verify! (creates EmailAddress, soft_deletes request)
    S-->>U: redirect — email verified
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: app/javascript/pages/Users/Settings/Integrations.svelte
Line: 225-229

Comment:
**Misleading button label for expired tokens**

When a verification request is expired and its cooldown has also elapsed, `resend_cooldown_seconds` is `0` (clamped), so `formatCooldown(0)` returns `""`, and the button falls back to `"Resend soon"`. This implies waiting will eventually enable the button, but `resend_email_verification` uses the `.valid` scope and will always reject expired tokens regardless of the cooldown. The user actually needs to **Remove** the request and re-add the address.

Consider showing `"Expired"` (or hiding the button entirely) when `email.expired` is true:

```suggestion
                  {email.can_resend
                    ? "Resend"
                    : email.expired
                      ? "Expired"
                      : formatCooldown(email.resend_cooldown_seconds) ||
                        "Resend soon"}
```

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

Reviews (5): Last reviewed commit: "Merge branch 'main' into email-verificat..." | Re-trigger Greptile

Comment thread app/controllers/sessions_controller.rb Outdated
Comment thread app/javascript/pages/Users/Settings/Integrations.svelte
@matmanna
Copy link
Copy Markdown
Member Author

@greptileai I forgor to do ts again

@matmanna matmanna marked this pull request as ready for review April 17, 2026 21:13
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