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

Review Request #9216 - Created Sept. 24, 2017 and updated

Brian LeBlanc
Djblets
release-0.10.x
4578
9209
2179201...
djblets, students

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).

The full testing suite for djblets was run.

  • 0
  • 0
  • 7
  • 0
  • 7
Description From Last Updated
Barret Rennie
  1. 
      
  2. Can you run all of djblets tests?

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

    Format as:

    if (not enable_smart_spoofing or
        parsed_from_email != parsed_sender_email and            
        not is_email_allowed_by_dmarc(parsed_from_email)):
    
  4. djblets/mail/tests.py (Diff revision 1)
     
     

    Use %-formatting:

    '"doc via example.com" <%s>' % settings.DEFAULT_FROM_EMAIL
    
  5. djblets/mail/tests.py (Diff revision 1)
     
     

    Same here re: formatting.

  6. 
      
Barret Rennie
  1. 
      
  2. Please add the bug number in the bugs field.

  3. 
      
Brian LeBlanc
Barret Rennie
  1. Ship It!
  2. 
      
Christian Hammond
  1. 
      
  2. This needs unit tests for the new conditionals. Right now, I'm not 100% sure that this doesn't regress functionality.

    1. 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.

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

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

    1. 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.

  4. 
      
Brian LeBlanc
Brian LeBlanc
Review request changed

Change Summary:

Add test case for enable_smart_spoofing=False.

Commit:

-779e35857216724e8a37361aaf34fb622d1932b3
+2179201c0db5dfbd7d4ed0a6741893b1f812ffd6

Diff:

Revision 4 (+59 -16)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...