Giveth / giveth-dapps-v2

This project is the aggregation of GIVeconomy and Giveth.io DApps in a single repo
https://staging.giveth.io
GNU General Public License v3.0
61 stars 34 forks source link

Create flow for verifying email ownership on profile creation and edit #4476

Open divine-comedian opened 1 month ago

divine-comedian commented 1 month ago

@WhyldWanderer was checking our subscriber list in Ortto and found many users have fake emails in their profiles which makes it difficult to communicate with them about their project's status or opportunities/campaigns they should be aware of.

This issue is for designing a flow that requires users to validate their email address while either creating their Giveth profile for the first time or changing their email address when editing their profile info.

Requirements

for current users

Flow

  1. User goes to My Profile
  2. Clicks on update Profile
  3. Enters email address
  4. Clicks Verify Email button
  5. BE generates verification code and sends it in email
  6. User receives Verification code to email entered
  7. User copies verification code from email and enters it into input field
  8. BE checks if code is valid
  9. If code is valid then user receives confirmation and the email is considered verified
  10. user clicks save profile, receives toast notif that info has been saved

https://github.com/user-attachments/assets/949e833b-1262-457e-a523-800f15a3a96f

Figma

https://www.figma.com/design/vMiyVd1Ly5LjgSyZrChVV8/Giveth-users?node-id=3068-66717&t=TTng1YwgiqprVvdc-1


@jainkrati - Do you know of a service or package that handles these type of email verification methods or is it something where the back-end services are typically built from scratch?

Tosinolawale commented 1 month ago

https://www.figma.com/design/vMiyVd1Ly5LjgSyZrChVV8/Giveth-users?node-id=3069-66987&t=7DZN7YOa8X4HDscB-1 @divine-comedian Created a flow here. Check it out

Tosinolawale commented 1 month ago

https://www.figma.com/design/vMiyVd1Ly5LjgSyZrChVV8/Giveth-users?node-id=3068-66717&t=TTng1YwgiqprVvdc-1

Attached with Video showing prototype Email Verification Giveth.webm @divine-comedian

jainkrati commented 1 month ago

Looping in @ae2079 to do the research on the packages available, under review from @RamRamez

RamRamez commented 1 month ago

@jainkrati We already have this service in impact-graph and notification-center service for project verification form and I think we can use those services for this task. Also we can use same Ortto template, so I believe this issue is straightforward.

ae2079 commented 1 month ago

Looping in @ae2079 to do the research on the packages available, under review from @RamRamez

I've checked the existing code and done some research, and I think we can handle email verification using our Ortto client without adding new packages. We can generate random codes, save them in Redis with an expiration time, and create a /verify-email endpoint. I'll also update the front-end to support this flow. This approach keeps things simple and efficient with our current setup.

ae2079 commented 1 month ago

@divine-comedian As I should spend all of my capacity on the Qacc project, I can not do this issue in the next 6 weeks, so I unassigned this issue from myself

RamRamez commented 1 month ago

@Reshzera Could you please take this issue? We already have email verification in impact-graph and notification-center and you can use them. Please ping me and we can have a short call.

cc @jainkrati

Reshzera commented 1 month ago

@RamRamez Sure

RamRamez commented 1 month ago

In email verification step in the form verification design, we didn't had a code to confirm, we used JWT secret that is sent with email address, and when user opens the link in the email, the email address will be automatically confirmed. I believe it's better to have a consistent design in the app for email confirmation.

@Tosinolawale is it possible to change the design, so removing code input and instead telling user to check their email. because otherwise we need a new Ortto template, need some changes on notification-center for new Ortto template, and we need to implement verification with code in impact-graph, it will add at least 2 days of devs work.

@divine-comedian WDYT?

image
Tosinolawale commented 1 month ago

In email verification step in the form verification design, we didn't had a code to confirm, we used JWT secret that is sent with email address, and when user opens the link in the email, the email address will be automatically confirmed. I believe it's better to have a consistent design in the app for email confirmation.

@Tosinolawale is it possible to change the design, so removing code input and instead telling user to check their email. because otherwise we need a new Ortto template, need some changes on notification-center for new Ortto template, and we need to implement verification with code in impact-graph, it will add at least 2 days of devs work.

@divine-comedian WDYT?

image

Email Verification was the Initial design but @divine-comedian brought up issues of email verifications from Multisig account. Either works fine for me, So just let me know what the final decision is in terms of feasibility

divine-comedian commented 1 month ago

@RamRamez - I would like to get rid of the current email verification step inside of project verification form and instead have this verify step using a code, but that's a later issue.

As Tosin mentioned we have had issues with multisig users not being able to verify their project because of needing to load an external link outside of the giveth safe app. In addition, the UX of sending another link in an email is sort of confusing, the user clicks the link from their email and now has another browser window open with their verification form open - which one do they proceed from??

I believe the code is the best way to go forward, I acknowledge the extra work it adds. Is there an existing package or code template that we can use for sending and verifying codes with email addresses? This is a quite common pattern we see on most web apps with login.

This issue I would say is lower on the priority list so if there is more pressing work then let's code this feature when it seems reasonable.

RamRamez commented 1 month ago

@divine-comedian I got it. So we need to change the current email verification process in impact-graph.

@Reshzera Could you please investigate how we can implement this new feature in impact-graph? Is there a good library/package out there to reduce development work?

Reshzera commented 1 month ago

@RamRamez Sure I'll take a look

divine-comedian commented 1 month ago

@Reshzera here is the code snippet you can use to trigger the activity in ortto

"activities": [
        {
            "activity_id": "act:cm:email-verification-code",
            "attributes": {
                "int:cm:code": 1723565874,
                "str:cm:email": "example string value",
                "str:cm:userid": "example string value"
            },
            "fields": {
                "str::email": "contact@email.com"
                "str:cm:user-id": "example string value"
            },
        }
    ],
}`;

email has been drafted and should trigger properly when activity is triggered.

RamRamez commented 3 weeks ago

@Reshzera told me he has removed email verification from FE in project verification form, now, we don't these fields from project_verification_form table on BE nor on FE: emailConfirmed, emailConfirmationToken, emailConfirmationTokenExpiredAt, emailConfirmationSent, emailConfirmationSentAt, emailConfirmedAt, email

So, if we remove those fields from DB, those changes are not backward compatible and we should be careful and should handle FE and BE releases at same time.

Or, we can only remove it from UI, remove that step from BE, and keep DB as is, it's safe and fast to do.

@CarlosQ96 @mohammadranjbarz WDYT?

mohammadranjbarz commented 3 weeks ago

@Reshzera told me he has removed email verification from FE in project verification form, now, we don't these fields from project_verification_form table on BE nor on FE: emailConfirmed, emailConfirmationToken, emailConfirmationTokenExpiredAt, emailConfirmationSent, emailConfirmationSentAt, emailConfirmedAt, email

So, if we remove those fields from DB, those changes are not backward compatible and we should be careful and should handle FE and BE releases at same time.

Or, we can only remove it from UI, remove that step from BE, and keep DB as is, it's safe and fast to do.

@CarlosQ96 @mohammadranjbarz WDYT?

I do agree

Reshzera commented 3 weeks ago

Ok, thanks for the help guys, so just to confirm, I'll let this columns on the DB and just change the validation on the verification form?

RamRamez commented 3 weeks ago

@Reshzera Yes you can start. I think you can't only remove it from UI, because you need to disable email verification step from BE too, so, it's still not backward compatible. You can either only do necessary changes and keep DB intact for now and remove those fields from DB later, or you can do them all at once. It's up to you.

mohammadranjbarz commented 3 weeks ago

@Reshzera says he afraid to mess up something while implementing the backend side of this issue, so he needs someone who knows the backend and DB entities continue his work, as I'm busy with givEconomy things, @CarlosQ96 can you continue his work (this is Rafael WIP branch https://github.com/Giveth/impact-graph/tree/feat/issue-4476)?

maryjaf commented 2 weeks ago

I see some open pull request on this issue, Is this ready for testing ? @Reshzera