Summary

Change e-mail sending to use <Name> via Review Board when e-mail spoofing disabled.

Review Request #9216 — Created Sept. 24, 2017 and submitted Feb. 11, 2019, 11:19 a.m.

Information

Owner

bleblan2

Repository

Djblets

Branch

release-0.10.x

Bugs

4578

Depends On

Blocks

9209

Commit

2179201...

Reviewers

Groups

djblets, students

People

Description

When e-mail messages are created with enabled_smart_spoofing disabled,
the 'From' header should match a DMARC failure but instead the original
from_address is used.

This changes makes e-mail messages copy the same functionality as DMARC
not allowing spoofing when enable_smart_spoofing is disabled. This
changes the mail tests to expect the change in functionality when
enable_smart_spoofing is False.

This change is from the comments on the review for adding an option for
disabling e-mail smart spoofing (https://reviews.reviewboard.org/r/9209).

Testing Done

The full testing suite for djblets was run.

Issues

Description	From	Last Updated
Please add the bug number in the bugs field.	brennie	Oct. 1, 2017, 2:04 p.m.
Can you run all of djblets tests?	brennie	Oct. 3, 2017, 12:38 p.m.
This needs unit tests for the new conditionals. Right now, I'm not 100% sure that this doesn't regress functionality.	chipx86	Oct. 18, 2017, 6:28 p.m.
Format as: if (not enable_smart_spoofing or parsed_from_email != parsed_sender_email and not is_email_allowed_by_dmarc(parsed_from_email)):	brennie	Oct. 3, 2017, 12:30 p.m.
Use %-formatting: '"doc via example.com" <%s>' % settings.DEFAULT_FROM_EMAIL	brennie	Oct. 3, 2017, 12:22 p.m.
Same here re: formatting.	brennie	Oct. 3, 2017, 12:22 p.m.
The comments for all this logic no longer match the conditional, making it harder to understand the impact of the …	chipx86	Oct. 18, 2017, 6:20 p.m.

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues
```
Can you run all of djblets tests?
```

djblets/mail/message.py (Diff revision 1)

The issue has been resolved. Show all issues

Format as:

if (not enable_smart_spoofing or
    parsed_from_email != parsed_sender_email and            
    not is_email_allowed_by_dmarc(parsed_from_email)):

djblets/mail/tests.py (Diff revision 1)

The issue has been resolved. Show all issues

Use %-formatting:

'"doc via example.com" <%s>' % settings.DEFAULT_FROM_EMAIL

djblets/mail/tests.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same here re: formatting.
```

The issue has been resolved. Show all issues

Please add the bug number in the bugs field.

Change Summary:

Fix formatting issues brought up in review comments.

Testing Done:

~		The testing suite for djblets mail was run.
	~	The full testing suite for djblets was run.

Commit:

7185029c7484540f459491e6c77146e19b8b449f

cf262ca146f17d7f493eae5f84ef86ba8e672faa

Diff:

Revision 2 (+13 -6)

Show changes

	djblets/mail/message.py
	djblets/mail/tests.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

The issue has been resolved. Show all issues

This needs unit tests for the new conditionals. Right now, I'm not 100% sure that this doesn't regress functionality.

bleblan2 Oct. 18, 2017, 6:31 p.m.

I've added a test case for having enable_smart_spoofing=False. As for regressing functionality, that all depends on what the expected behaviour is for when enable_smart_spoofing is set to False.

djblets/mail/message.py (Diff revision 2)

The issue has been resolved. Show all issues

The comments for all this logic no longer match the conditional, making it harder to understand the impact of the change in behavior.

bleblan2 Oct. 18, 2017, 6:20 p.m.

Moving the check for enable_smart_spoofing was done to match the expected functionality I understood from your comment https://reviews.reviewboard.org/r/9209/#comment39603. Without this, passing in enable_smart_spoofing as False doesn't send the email as a <Name> via <service> but instead skips over the code at line 187 where build_email_address_via_service is called to avoid spoofing and sends with the original sender and from_email that were passed in.