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

Adding support for email identity #3061

Merged
merged 33 commits into from
Sep 18, 2024
Merged

Conversation

silva-fj
Copy link
Contributor

@silva-fj silva-fj commented Sep 11, 2024

This PR adds support for email identities.

Important notes:
We have some limitations due to our current rust tool chain version and the compatibility with sgx environment, so we can't use the available libraries to send emails (via SMTP). After exploring some options available I found SendGrid to be the most convenient option (IMO). It provides an api we can use to send emails via http, the services are reliable and the pricing is reasonable. So, I have implemented a sengrid mailer. Having said that, we could easily migrate to other options if needed in the future.

We may as well need some design for the email template, for now I've added an example template that looks like this:

Screenshot 2024-09-11 at 4 42 33 PM

The flow is the following:

  • User makes an RPC request to identity_requestEmailVerification with the prime identity and the email address.
  • The user receives an email with the verification code.
  • The user makes a request to link the identity sending the email and verification code as validation data.

We need to create/setup an account in SendGrid and setup the email address that is going to be used to send the verification templates, then the following env variables need to be defined:

  • SENDGRID_API_KEY
  • SENDGRID_FROM_EMAIL

This PR (#3063) needs to be merged first

Important

Please note that similar to the OAuth2 verification, we need to implement "native" requests for this to work with multi-worker setup, as verification codes are stored in memory and not synced between workers.

@silva-fj silva-fj requested a review from a team September 11, 2024 15:19
Copy link

linear bot commented Sep 11, 2024

@silva-fj silva-fj force-pushed the p-685-add-support-for-email-identity branch from 99fcc5c to 616816c Compare September 11, 2024 16:19
fn send(&mut self, mail: Mail) -> Result<(), String> {
let sendgrid_mail = SendGridMail::new(self.from.clone(), mail);
self.client
.post(String::default(), &sendgrid_mail)
Copy link
Member

Choose a reason for hiding this comment

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

This means sendgrid account owner knows what emails were linked and when ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an interesting point. The SendGrid account owner could know what emails requested the verification code in the last few days, but this is true for any service. We would have to setup and manage our own SMTP server if we want to prevent this, but even if we do that it would be very difficult to convince users that this is really the case. There has to be some level of trust for this solution. Another solution could be to add the option to verify with ZK proofs, so users can decide what to use. The tradeoff is that the UX is really bad

Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

I haven't tested it but the logic looks good!

Do you know if we still have SendGrid account? @jonalvarezz
I remember we had it for MCP

@silva-fj silva-fj enabled auto-merge (squash) September 18, 2024 07:34
@silva-fj silva-fj merged commit 867777f into dev Sep 18, 2024
28 checks passed
@silva-fj silva-fj deleted the p-685-add-support-for-email-identity branch September 18, 2024 08:09
@BillyWooo BillyWooo added the C1-noteworthy Non-breaking change but is worth noticing for client label Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C1-noteworthy Non-breaking change but is worth noticing for client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants