Skip to content
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

Email verification #8240

Closed
FelixMalfait opened this issue Oct 31, 2024 · 6 comments · Fixed by #9288
Closed

Email verification #8240

FelixMalfait opened this issue Oct 31, 2024 · 6 comments · Fixed by #9288
Assignees
Labels
for experienced contributor scope: back+front Issues requiring full-stack knowledge

Comments

@FelixMalfait
Copy link
Member

FelixMalfait commented Oct 31, 2024

In the future, we're going to open a generous free plan on the cloud version, so we need to limit the number of spam accounts created.

Let's introduce email verification for people that don't use Microsoft/Google login.

Pre-requisite: introduce a server-level environment variable IS_EMAIL_VERIFICATION_REQUIRED (defaults to false), as some people self-hosting might not want to go through that (means they have to setup an email server!).

Signup Process

  • Set isEmailVerified to false initially (already the case)
  • Generate a unique email verification token.
  • Store the token in appToken
  • Send Email containing the verification link
  • Create a page to handle verification requests, which sets isEmailVerified to true and then automatically login the user

Note: users joining an existing workspace through an invite shouldn't have to validate their email again

Signin Process

During signin, check the isEmailVerified field.
If isEmailVerified is false, prevent login and prompt the user to verify their email.
Optionally, offer to resend the verification email.

Note: we already have the emailVerified column on user but it wasn't used - we might want to drop it or rename it to isEmailVerified to be consistent with other columns in the codebase
Todo during deploy: script/sql query to set emailVerified=true for every user that ever had a valid subscription (credit card = strong verification, we can consider the email is most likely to be valid)

Email content

We already have templates for Twenty emails (e.g. user invites)

image

https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=44465-119474&node-type=frame&t=L4Zw8NIeonoYkIdd-11

@Yashgupta9330
Copy link

I want to work on this issue can you please assign it to me

@FelixMalfait
Copy link
Member Author

Sure thanks @Yashgupta9330!

@samyakpiya
Copy link
Contributor

Hey @FelixMalfait,

This seems like a feature that'd be fun to work on! Could I be assigned?

@FelixMalfait
Copy link
Member Author

You're everywhere @samyakpiya 🐐 - thanks!

@samyakpiya
Copy link
Contributor

samyakpiya commented Dec 31, 2024

Hey @FelixMalfait,

I understand we’re planning to allow account creation first and then send a verification email to restrict login until the email is verified.

While working on the issue, I was thinking about instead sending a verification code that includes a magic link right after the user enters their email, before they set up their password. This could help reduce spam accounts, save resources, and enhance security.

It seems we already have the template for it.
https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=4162-56741
Image

Optioanally, we could create an Input OTP frontend UI component for it.

Let me know your thoughts!

@FelixMalfait
Copy link
Member Author

@samyakpiya we wanted to implement no-password + magic-link only initially, but then we asked for user feedback and it seems magic link isn't as popular these days, most people want a password.
I understand your proposal is to keep a password-login but defer password creation to post signup, which would work.
I think the "save resource" argument would have made a lot of sense if we had workspace schema creation directly upon signup but that's already differed, I don't think not asking for the password would make things structurally different.
So I would say from a UX perspective I agree your proposal is probably a bit better, but it's not probably not worth the added eng complexity of introducing a variation to the flow. Let's go for the simplest/most elegant eng implementation

@Bonapara Bonapara moved this from 🆕 New to 📋 Backlog in 🎯 Roadmap & Sprints Jan 10, 2025
pacyL2K19 pushed a commit to pacyL2K19/twenty that referenced this issue Jan 16, 2025
Closes twentyhq#8240

This PR introduces email verification for non-Microsoft/Google Emails:

https://github.com/user-attachments/assets/740e9714-5413-4fd8-b02e-ace728ea47ef

The email verification link is sent as part of the
`SignInUpStep.EmailVerification`. The email verification token
validation is handled on a separate page (`AppPath.VerifyEmail`). A
verification email resend can be triggered from both pages.

![image](https://github.com/user-attachments/assets/d52237dc-fcc6-4754-a40f-b7d6294eebad)

![image](https://github.com/user-attachments/assets/263a4b6b-db49-406b-9e43-6c0f90488bb8)

![image](https://github.com/user-attachments/assets/0343ae51-32ef-48b8-8167-a96deb7db99e)

![Screenshot 2025-01-05 at 11 56
56 PM](https://github.com/user-attachments/assets/475840d1-7d47-4792-b8c6-5c9ef5e02229)

![image](https://github.com/user-attachments/assets/a41b3b36-a36f-4a8e-b1f9-beeec7fe23e4)

![image](https://github.com/user-attachments/assets/e2fad9e2-f4b1-485e-8f4a-32163c2718e7)

expired, user does not exist, etc.):

![image](https://github.com/user-attachments/assets/92f4b65e-2971-4f26-a9fa-7aafadd2b305)

![image](https://github.com/user-attachments/assets/86d0f188-cded-49a6-bde9-9630fd18d71e)

- [x] Introduce server-level environment variable
IS_EMAIL_VERIFICATION_REQUIRED (defaults to false)
- [x] Ensure users joining an existing workspace through an invite are
not required to validate their email
- [x] Generate an email verification token
- [x] Store the token in appToken
- [x] Send email containing the verification link
  - [x] Create new email template for email verification
- [x] Create a frontend page to handle verification requests

- [x] After verifying user credentials, check if user's email is
verified and prompt to to verify
- [x] Show an option to resend the verification email

- [x] Rename the `emailVerified` colum on `user` to to `isEmailVerified`
for consistency

- [x] Run a script/sql query to set `isEmailVerified` to `true` for all
users with a Google/Microsoft email and all users that show an
indication of a valid subscription (e.g. linked credit card)
- I have created a draft migration file below that shows one possible
approach to implementing this change:

```typescript
import { MigrationInterface, QueryRunner } from 'typeorm';

export class UpdateEmailVerifiedForActiveUsers1733318043628
  implements MigrationInterface
{
  name = 'UpdateEmailVerifiedForActiveUsers1733318043628';

  public async up(queryRunner: QueryRunner): Promise<void> {
    await queryRunner.query(`
      CREATE TABLE core."user_email_verified_backup" AS
      SELECT id, email, "isEmailVerified"
      FROM core."user"
      WHERE "deletedAt" IS NULL;
    `);

    await queryRunner.query(`
      -- Update isEmailVerified for users who have been part of workspaces with active subscriptions
      UPDATE core."user" u
      SET "isEmailVerified" = true
      WHERE EXISTS (
        -- Check if user has been part of a workspace through userWorkspace table
        SELECT 1
        FROM core."userWorkspace" uw
        JOIN core."workspace" w ON uw."workspaceId" = w.id
        WHERE uw."userId" = u.id
        -- Check for valid subscription indicators
        AND (
          w."activationStatus" = 'ACTIVE'
          -- Add any other subscription-related conditions here
        )
      )
      AND u."deletedAt" IS NULL;
  `);
  }

  public async down(queryRunner: QueryRunner): Promise<void> {
    await queryRunner.query(`
      UPDATE core."user" u
      SET "isEmailVerified" = b."isEmailVerified"
      FROM core."user_email_verified_backup" b
      WHERE u.id = b.id;
    `);

    await queryRunner.query(`DROP TABLE core."user_email_verified_backup";`);
  }
}

```

---------

Co-authored-by: Antoine Moreaux <[email protected]>
Co-authored-by: Félix Malfait <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for experienced contributor scope: back+front Issues requiring full-stack knowledge
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants