Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

verification skip for OTP in dev/docker was broken

  1. Local development without email service = no verification
    required
  2. Any production deployment = verification always required
  3. Any setup with email service = verification required (secure
    default)

Type of Change

  • Bug fix

Testing

Tested manually.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Sep 20, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
docs Ready Ready Preview Comment Sep 20, 2025 6:14pm
sim Ready Ready Preview Comment Sep 20, 2025 6:14pm

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR fixes a critical bug in the OTP (One-Time Password) verification system for development environments. The main issue was that the verification skip logic wasn't working correctly in local development and Docker setups without email services configured.

The changes introduce a more sophisticated email service detection mechanism by adding a hasEmailService() function to the mailer module that checks for both Resend and Azure Communication Services availability. This replaces the previous simple environment variable check that only looked for Resend API keys.

The core fix updates the verification logic from checking only production status to checking both production status AND email service availability. The new logic implements three distinct behaviors:

  1. Local development without email service = no verification required
  2. Production deployment = verification always required
  3. Any setup with email service = verification required (secure default)

Key technical changes include:

  • Adding hasEmailService() function in apps/sim/lib/email/mailer.ts that returns true if either Resend or Azure email clients are configured
  • Updating the auth system in apps/sim/lib/auth.ts to use requireEmailVerification: isProd && hasEmailService() instead of just checking production status
  • Refactoring the verification hook from hasResendKey to hasEmailService parameter with updated skip logic !isProduction && !hasEmailService
  • Setting explicit NODE_ENV values in Docker compose files for proper environment detection
  • Adding fallback logging for development environments to display OTP codes in console when no email service is configured

The changes maintain security by ensuring verification is never skipped in production while providing a smooth development experience when email services aren't configured locally.

Confidence score: 4/5

  • This PR addresses a legitimate bug with well-structured fixes across multiple related files
  • The logic change from OR to AND condition (!isProduction && !hasEmailService) properly fixes the security vulnerability where verification could be bypassed in production
  • The centralized email service detection and consistent parameter renaming improve code maintainability and clarity across the verification system

7 files reviewed, 1 comment

Edit Code Review Bot Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit 93f9293 into staging Sep 20, 2025
6 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/verification branch September 23, 2025 15:57
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.

2 participants