Skip to content

Add an option to require email verification on sign up #739

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 23 commits into
base: dev
Choose a base branch
from

Conversation

fomalhautb
Copy link
Contributor

@fomalhautb fomalhautb commented Jul 3, 2025


Important

Adds an option to require email verification on sign-up, updating configurations, handlers, and UI components to support this feature.

  • Behavior:
    • Adds emailVerificationRequired option to project configuration in schema.prisma and config.tsx.
    • Implements throwEmailVerificationRequiredErrorIfNeeded function in verification-code-handler.tsx to enforce email verification.
    • Updates sign-in and sign-up routes to check for email verification requirement.
  • UI:
    • Adds email verification setting in page-client.tsx.
    • Updates email verification template in email-verification.tsx to include OTP.
  • API:
    • Modifies verifyEmail in client-interface.ts to return tokens on success.
    • Updates OAuth and sign-in methods to handle email verification redirects.
  • Misc:
    • Adds emailVerificationRedirectUrl to OAuth flows in auth.ts and client-app-impl.ts.
    • Updates project and admin configurations to include emailVerificationRequired.

This description was created by Ellipsis for 03ed9e0. You can customize this summary. It will automatically update as commits are pushed.

Copy link

vercel bot commented Jul 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stack-backend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 11, 2025 11:47am
stack-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 11, 2025 11:47am
stack-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 11, 2025 11:47am
stack-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 11, 2025 11:47am

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.

PR Summary

Implements required email verification option for user sign-up across the stack-auth system, with admin configuration controls and secure verification flows.

  • Added requiresEmailVerification boolean field to ProjectUser model with Prisma migration and CRUD layer integration
  • Implemented email verification handlers in sign-in/sign-up flows with both OTP and magic link support
  • Enhanced OAuthModel to check email verification requirements before token generation
  • Added EmailVerificationRequired error type and handler in client app implementation
  • Created admin UI toggle in dashboard for configuring email verification requirements per project

22 files reviewed, 6 comments
Edit PR Review Bot Settings | Greptile

Copy link

recurseml bot commented Jul 3, 2025

✨ No issues found! Your code is sparkling clean! ✨

🗒️ View all ignored comments in this repo
  • The constraint 'TokenStoreType extends string' is too restrictive. It should likely be 'TokenStoreType extends string | object' to match the condition check in line 113 where TokenStoreType is checked against {}
  • Return type mismatch - the interface declares useUsers() returning ServerUser[] but the Team interface that this extends declares useUsers() returning TeamUser[]
  • There is a syntax error in the super constructor call due to the ellipsis operator used incorrectly. Objects aren't being merged correctly. This syntax usage can lead to runtime errors when trying to pass the merged object to 'super()'. Verify that the intended alterations to the object occur before or outside of the super() call if needed.
  • Throwing an error when no active span is found is too aggressive. The log function should gracefully fallback to console.log or another logging mechanism when there's no active span, since not all execution contexts will have an active span. This makes the code less resilient and could break functionality in non-traced environments.

📚 Relevant Docs

  • Function sets backendContext with a new configuration but doesn't pass 'defaultProjectKeys'. Since defaultProjectKeys is required in the type definition and cannot be updated (throws error if tried to set), this will cause a type error.
  • The schema is using array syntax for pick() which is incorrect for Yup schemas. The pick() method in Yup expects individual arguments, not an array. Should be changed to: emailConfigSchema.pick('type', 'host', 'port', 'username', 'sender_name', 'sender_email')

📚 Relevant Docs

  • Creating a refresh token with current timestamp as expiration means it expires immediately. Should set a future date for token expiration.
  • The 'tools' object is initialized as an empty object, even though 'tools' is presumably expected to contain tool definitions. This could cause the server capabilities to lack necessary tool configurations, thus potentially impacting functionalities that depend on certain tool setups.

📚 Relevant Docs

  • 'STACK_SECRET_SERVER_KEY' is potentially being included in every request header without checking its existence again here. Although it's checked during initialization, this could lead to security issues as it's exposed in all communications where the header is logged or captured.

📚 Relevant Docs

  • When adding 'use client' directive at the beginning, it doesn't check if file.text already contains the 'use client' directive. This could lead to duplicate 'use client' directives if the file already has one.

📚 Relevant Docs

⚠️ Only 5 files were analyzed due to processing limits.

Need help? Join our Discord for support!
https://discord.gg/NCpkJ4kF

@@ -0,0 +1,123 @@
import { sendEmailFromTemplate } from "@/lib/emails";
Copy link
Contributor

Choose a reason for hiding this comment

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

Typographical error: The directory/file name 'email-verifiation-required' appears to contain a spelling mistake. It should likely be 'email-verification-required'.

Copy link

patched-codes bot commented Jul 3, 2025

Documentation Changes Required

  1. docs/templates/sdk/types/project.mdx

    • Add documentation for the new emailVerificationRequired config property under the Project config section.
    • Include explanation of this new configuration option in the properties list.
    • Update the TypeScript type definition to include emailVerificationRequired: boolean;.
  2. docs/templates/sdk/objects/stack-app.mdx

    • Add documentation for the new redirectToEmailVerificationRequired method.
    • Include this new method in the table of contents section.
    • Create a new method entry with appropriate description, parameters, and return type, similar to other redirect methods.
  3. docs/templates/customization/custom-pages.mdx

    • Update the section explaining how to customize URLs to include the new emailVerificationRequired URL option.
    • Add the emailVerificationRequired property to the example urls object to show how it can be customized.

Please ensure these changes are reflected in the relevant documentation files to keep them in sync with the recent code changes.

user_id: options.userId,
});

if (!user.primary_email) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In throwEmailVerificationRequiredErrorIfNeeded, if the user lacks a primary_email, the function logs an error and returns. Consider throwing an error instead to avoid ambiguous client behavior.

const headerText = t("Verify your email to continue");
const instructionText = t("Enter the six-digit code sent to your email");

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The useEffect here automatically redirects if a user exists or if the nonce is missing. This may trigger an unintended redirect even if the email isn’t verified. Consider checking the user’s email verification status (e.g. user.primary_email_verified) before redirecting.

return result;
if (result.status === 'ok') {
if (result.data.accessToken && result.data.refreshToken) {
await this._signInToAccountWithTokens(result.data as any);
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid casting tokens with 'as any' when calling _signInToAccountWithTokens. Consider updating the types so that the returned data fits the expected structure and no unsafe cast is needed.

throw new StatusError(400, "Email verification is required, but no email verification callback URL was provided. If you enabled the email verification required setting, you need to update the Stack Auth client to use this feature.");
}

// TODO: check if callback url is valid
Copy link
Contributor

Choose a reason for hiding this comment

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

The added check for callbackUrl is good. Consider further validating that the URL is non‐empty and properly formatted (using a tagged template literal like urlString`` if constructing URLs) to avoid potential issues.

Suggested change
// TODO: check if callback url is valid
new URL(options.callbackUrl);

This comment was generated because it violated a code review rule: mrule_pmzJAgHDlFZgwIwD.

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