DeanHere / rietveld

Automatically exported from code.google.com/p/rietveld
Apache License 2.0
0 stars 0 forks source link

From field in emails should only use a domain that rietveld has permission to send from #466

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
This problem was found in the Chromium code review instance of rietveld.

What steps will reproduce the problem?
1. Create a code review issue with a non-chromium.org or non-google.com email 
(confirmed to happen for @adobe.com and @intel.com)
2. Set a colleague in the cc or reviewer fields 
(non-chromium.org/non-google.com)
3. Update the issue by commenting, etc
4. Note that the colleague will not receive any emails because they are tossed 
by the receiving mail server because it looks like the from header is forged.

Note that this is broken in the other way as well, where if that colleague 
comments on the issue, the original poster will not get the email notification, 
either.

There is more information in this chromium issue: 
https://code.google.com/p/chromium/issues/detail?id=307345

What is the expected output? What do you see instead?

The proper solution would be for rietveld to always send emails as something 
like chromium-reviews@chromium.org, and set the Reply-To header to the email of 
the user that made the update on the issue. (It would also work if there was a 
whitelist of domains that rietveld could set the from header directly for)

At what URL are you accessing Rietveld?  (e.g. codereview.appspot.com)
Please note if you are using the Google Apps Labs version (e.g.
codereview.<yourdomain>).

codereview.chromium.org

Original issue reported on code.google.com by bjone...@adobe.com on 24 Oct 2013 at 3:43

GoogleCodeExporter commented 9 years ago
https://developers.google.com/appengine/docs/python/mail/sendingmail
(...)
2. The address of the user for the current request signed in with a Google 
Account. You can determine the current user's email address with the Users API. 
The user's account must be a Gmail account, or be on a domain managed by Google 
Apps.
3. Any valid email receiving address for the app (such as 
xxx@APP-ID.appspotmail.com).
4. Any valid email receiving address of a domain account, such as 
support@example.com. Domain accounts are accounts outside of the Google domain 
with email addresses that do not end in @gmail.com or @APP-ID.appspotmail.com.
(...)

Rietveld uses reply@<appid>.appspotmail.com to receive the emails. As such in 
Chromium, we added reply@chromiumcodereview-hr.appspotmail.com as a member to 
chromium-reviews@chromium.org.

In practice, Rietveld uses the person who is sending the message as the From, 
which is described above as being "valid" if it is the currently set user. The 
code is at:
https://code.google.com/p/rietveld/source/browse/codereview/views.py#3674

The problem is that @chromium.org fall into #2 while non Google Apps users like 
@intel.com and @adobe.com falls into #4. I assume the email headers generated 
by AppEngine's email system for #4 look much more "forged" that the ones that 
falls into #2.

One option is to revert to using 
from=django_settings.RIETVELD_INCOMING_MAIL_ADDRESS only when the user is using 
a GA- account, that it, a Google Account that is not member of a Google Apps 
domain or on the GMail domain.

Original comment by maruel@chromium.org on 24 Oct 2013 at 12:30