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

[CAL-4159] Email address with ' (apostrophe) not accepted #15471

Open
andy-vdg opened this issue Jun 18, 2024 · 10 comments · May be fixed by #15535
Open

[CAL-4159] Email address with ' (apostrophe) not accepted #15471

andy-vdg opened this issue Jun 18, 2024 · 10 comments · May be fixed by #15535
Labels
booking-page area: booking page, public booking page, booker 🐛 bug Something isn't working Medium priority Created by Linear-GitHub Sync
Milestone

Comments

@andy-vdg
Copy link

andy-vdg commented Jun 18, 2024

Issue Summary

I've had someone try and use my scheduling page to book in but they couldn't because an email address with an ' (apostrophe) is not accepted as a valid email address.

Steps to Reproduce

  1. Go to a scheduling page
  2. Pick a date and time
  3. Add an email address i.e. john.o'[email protected]
  4. Address is not accepted

Actual Results

  • Email address not accepted

Expected Results

  • It should accept this email address as it is valid according to RFC5321/5322

Technical details

Here's a regex that will correctly match all email addresses (according to ChatGPT)

^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*$

Evidence

image

CAL-4159

@dosubot dosubot bot added booking-page area: booking page, public booking page, booker 🐛 bug Something isn't working labels Jun 18, 2024
Copy link

dosubot bot commented Jun 18, 2024

Hey there, @andy-vdg! I'm a bot here to help you with any issues, questions, or contributions. Let's work together to solve this bug!

To resolve the issue of email addresses with an apostrophe not being accepted, you need to update the email validation regex to include apostrophes. Here's how you can do it:

  1. Update the email validation regex in the backend:

    Modify the z.string().email() validation to use a custom regex pattern that includes the apostrophe character. Here is an example of how you can update the code:

    import { withValidation } from "next-validations";
    import { z } from "zod";
    
    import { baseApiParams } from "./baseApiParams";
    
    // Custom email regex pattern to include apostrophe
    const emailRegex = /^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*$/;
    
    export const schemaQueryAttendeeEmail = baseApiParams.extend({
      attendeeEmail: z.string().regex(emailRegex, { message: "Invalid email address" }),
    });
    
    export const schemaQuerySingleOrMultipleAttendeeEmails = z.object({
      attendeeEmail: z.union([z.string().regex(emailRegex, { message: "Invalid email address" }), z.array(z.string().regex(emailRegex, { message: "Invalid email address" }))]).optional(),
    });
    
    export const withValidQueryAttendeeEmail = withValidation({
      schema: schemaQueryAttendeeEmail,
      type: "Zod",
      mode: "query",
    });
  2. Update other parts of the codebase for consistency:

    Ensure that other parts of the codebase that perform email validation are also updated. This includes functions like checkInputEmailIsValid, createNewUsersConnectToOrgIfExists, and various email verification functions. You can find these in files such as inviteMemberUtils.ts, verifyEmail.ts, queryUserEmail.ts, and queryAttendeeEmail.ts [1][2][3][4].

  3. Update frontend components:

    The SecondaryEmailModal component in apps/web/components/settings/SecondaryEmailModal.tsx also performs email validation. Update the validation to use the new regex pattern:

    const formMethods = useForm<FormValues>({
      resolver: zodResolver(
        z.object({
          email: z.string().regex(emailRegex), // Replace with your new regex
        })
      ),
    });
  4. Update automated tests:

    Ensure that the automated tests for email validation include cases with apostrophes. Update the test cases in inviteMemberUtils.test.ts:

    describe("checkInputEmailIsValid", () => {
        it("should throw a TRPCError with code BAD_REQUEST if the email is invalid", () => {
            const invalidEmail = "invalid-email";
            expect(() => checkInputEmailIsValid(invalidEmail)).toThrow(TRPCError);
            expect(() => checkInputEmailIsValid(invalidEmail)).toThrowError(
                "Invite failed because invalid-email is not a valid email address"
            );
        });
    
        it("should not throw an error if the email is valid", () => {
            const validEmail = "[email protected]";
            const validEmailWithApostrophe = "john.o'[email protected]";
            expect(() => checkInputEmailIsValid(validEmail)).not.toThrow();
            expect(() => checkInputEmailIsValid(validEmailWithApostrophe)).not.toThrow();
        });
    });

By making these updates, you will ensure that email addresses with apostrophes are accepted across the entire application.

To continue talking to Dosu, mention @dosu.

@RajdeepDs
Copy link
Contributor

@PeerRich
Do we need to allow apostrophes in email validation for this page only, or should this change be applied across the entire application? While allowing apostrophes would improve compatibility with some email addresses.

@ciaranha ciaranha added the Medium priority Created by Linear-GitHub Sync label Jun 21, 2024
@ciaranha
Copy link
Member

@PeerRich Do we need to allow apostrophes in email validation for this page only, or should this change be applied across the entire application? While allowing apostrophes would improve compatibility with some email addresses.

You should be able to book someone and sign up for an account with an email with an apostrophe.

@PeerRich
Copy link
Member

ive never seen apostrophes in an email. didnt know that was a valid schema. i dont mind allowing it

@PeerRich
Copy link
Member

we should be allowing it everywhere

@ciaranha
Copy link
Member

ciaranha commented Jun 21, 2024

ive never seen apostrophes in an email. didnt know that was a valid schema. i dont mind allowing it

Me either, and I've seen a lot of Irish email addresses 😅 Gmail don't allow it. Marking as Low priority.

CleanShot 2024-06-21 at 11 59 59@2x

@ciaranha ciaranha added Low priority Created by Linear-GitHub Sync and removed Medium priority Created by Linear-GitHub Sync labels Jun 21, 2024
@RajdeepDs
Copy link
Contributor

Thanks for clarifying! I'm updating the email validations using regex, so an user with an apostrophe in their email, can able to sign up or book appointments.

@andy-vdg
Copy link
Author

@ciaranha I hadn't come across it either before - but as luck would have it - this week I had two separate people from two different continents (UK and NZ) try and book in with a ' in their email address.
Turns out even though Gmail doesn't allow it - Microsoft does.
Thanks @RajdeepDs much appreciated.

@ciaranha
Copy link
Member

ciaranha commented Jun 21, 2024

@ciaranha I hadn't come across it either before - but as luck would have it - this week I had two separate people from two different continents (UK and NZ) try and book in with a ' in their email address. Turns out even though Gmail doesn't allow it - Microsoft does. Thanks @RajdeepDs much appreciated.

Ah great to know, thanks for sharing @andy-vdg 🙏 Bumping back to medium priority 😅 Looks like we'll have a fix for this out pretty quickly!

@ciaranha ciaranha added Medium priority Created by Linear-GitHub Sync and removed Low priority Created by Linear-GitHub Sync labels Jun 21, 2024
@RajdeepDs RajdeepDs linked a pull request Jun 22, 2024 that will close this issue
3 tasks
@dosubot dosubot bot added this to the v4.4 milestone Jul 8, 2024
@keithwillcode keithwillcode modified the milestones: v4.4, v4.5 Aug 19, 2024
@keithwillcode keithwillcode changed the title Email address with ' (apostrophe) not accepted [CAL-4159] Email address with ' (apostrophe) not accepted Aug 19, 2024
@ArnavNath550
Copy link
Contributor

Hi Guys!
I have just tried attempting this one, just take a look guys and see if this looks good ✨
attempt /#16348

Thank You,
AR. Nath

@keithwillcode keithwillcode modified the milestones: v4.5, v4.6 Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
booking-page area: booking page, public booking page, booker 🐛 bug Something isn't working Medium priority Created by Linear-GitHub Sync
Projects
None yet
6 participants