mailpipe: fix course -> error.course in InvalidAssignmentSubject handling.
[pygrader.git] / pygrader / mailpipe.py
index 845c2d0b31e4d55f984cf8658f0822658ff62ce4..be7b1a02c6a51eda8a8f016b7a033f446fa4e82e 100644 (file)
@@ -21,11 +21,13 @@ from __future__ import absolute_import
 from email import message_from_file as _message_from_file
 from email.header import decode_header as _decode_header
 from email.mime.text import MIMEText as _MIMEText
+from email.utils import parseaddr as _parseaddr
 import mailbox as _mailbox
 import re as _re
 import sys as _sys
 
 import pgp_mime as _pgp_mime
+import pgp_mime.key as _pgp_mime_key
 
 from . import LOG as _LOG
 from .email import construct_email as _construct_email
@@ -76,13 +78,14 @@ class AmbiguousAddress (_InvalidMessage):
 
 
 class WrongSignatureMessage (_InsecureMessage):
-    def __init__(self, pgp_key=None, signatures=None, decrypted=None,
-                 **kwargs):
+    def __init__(self, pgp_key=None, signatures=None, fingerprints=None,
+                 decrypted=None, **kwargs):
         if 'error' not in kwargs:
             kwargs['error'] = 'not signed by the expected key'
         super(WrongSignatureMessage, self).__init__(**kwargs)
         self.pgp_key = pgp_key
         self.signatures = signatures
+        self.fingerprints = fingerprints
         self.decrypted = decrypted
 
 
@@ -794,8 +797,47 @@ def _parse_message(course, message, trust_email_infrastructure=False):
         raise
     return (original, message, person, subject, target)
 
-def _get_message_person(course, message):
-    sender = message['Return-Path']  # RFC 822
+def _get_message_person(course, message, trust_admin_from=True):
+    """Get the `Person` that sent the message.
+
+    We use 'Return-Path' (envelope from) instead of the message's From
+    header, because it's more consistent and harder to fake.  However,
+    there may be times when you *want* to send a message in somebody
+    elses name.
+
+    For example, if a student submitted an assignment from an
+    unexpected address, you might add that address to their entry in
+    your course config, and then bounce the message back into
+    pygrader.  In this case, the From header will still be the
+    student, but the 'Return-Path' will be you.  With
+    `trust_admin_from` (on by default), messages who's 'Return-Path'
+    matches a professor or TA will have their 'From' line used to find
+    the final person responsible for the message.
+
+    >>> from pygrader.model.course import Course
+    >>> from pygrader.model.person import Person
+    >>> from pgp_mime import encodedMIMEText
+
+    >>> course = Course(people=[
+    ...     Person(
+    ...         name='Gandalf', emails=['g@grey.edu'], groups=['professors']),
+    ...     Person(name='Bilbo', emails=['bb@shire.org']),
+    ...     ])
+    >>> message = encodedMIMEText('testing')
+    >>> message['Return-Path'] = '<g@grey.edu>'
+    >>> message['From'] = 'Bill <bb@shire.org>'
+    >>> message['Message-ID'] = '<123.456@home.net>'
+
+    >>> person = _get_message_person(course=course, message=message)
+    >>> print(person)
+    <Person Bilbo>
+
+    >>> person = _get_message_person(
+    ...     course=course, message=message, trust_admin_from=False)
+    >>> print(person)
+    <Person Gandalf>
+    """
+    sender = message['return-path']  # RFC 822
     if sender is None:
         raise NoReturnPath(message)
     sender = sender[1:-1]  # strip wrapping '<' and '>'
@@ -804,7 +846,26 @@ def _get_message_person(course, message):
         raise UnregisteredAddress(message=message, address=sender)
     if len(people) > 1:
         raise AmbiguousAddress(message=message, address=sender, people=people)
-    return people[0]
+    person = people[0]
+    if trust_admin_from and person.is_admin():
+        mid = message['message-id']
+        from_headers = message.get_all('from')
+        if len(from_headers) == 0:
+            _LOG.debug("no 'From' headers in {}".format(mid))
+        elif len(from_headers) > 1:
+            _LOG.debug("multiple 'From' headers in {}".format(mid))
+        else:
+            name,address = _parseaddr(from_headers[0])
+            people = list(course.find_people(email=address))
+            if len(people) == 0:
+                _LOG.debug("'From' address {} is unregistered".format(address))
+            if len(people) > 1:
+                _LOG.debug("'From' address {} is ambiguous".format(address))
+            _LOG.debug('message from {} treated as being from {}'.format(
+                    person, people[0]))
+            person = people[0]
+    _LOG.debug('message from {}'.format(person))
+    return person
 
 def _get_message_subject(message):
     """
@@ -939,14 +1000,29 @@ def _get_verified_message(message, pgp_key):
     for signature in signatures:
         _LOG.debug(signature.dumps())
     match = None
-    matches = [s for s in signatures if s.fingerprint.endswith(pgp_key)]
+    fingerprints = dict((s.fingerprint, s) for s in signatures)
+    for s in signatures:
+        for key in _pgp_mime_key.lookup_keys([s.fingerprint]):
+            if key.subkeys[0].fingerprint != s.fingerprint:
+                # the signature was made with a subkey.  Add the primary.
+                fingerprints[key.subkeys[0].fingerprint] = s
+    if pgp_key.startswith('0x'):
+        key_tail = pgp_key[len('0x'):]
+    else:
+        key_tail = pgp_key
+    matches = [fingerprints[f] for f in fingerprints.keys()
+               if f.endswith(key_tail)]
     if len(matches) == 0:
         raise WrongSignatureMessage(
             message=message, pgp_key=pgp_key, signatures=signatures,
-            decrypted=decrypted)
+            fingerprints=fingerprints, decrypted=decrypted)
     signature = matches[0]
     if not verified:
-        if signature.get_summary() != 0:
+        problems = [k for k,v in signature.summary.items() if v]
+        for good in ['green', 'valid']:
+            if good in problems:
+                problems.remove(good)
+        if problems:
             raise UnverifiedSignatureMessage(
                 message=message, signature=signature, decrypted=decrypted)
         # otherwise, we may have an untrusted key.  We'll count that
@@ -1019,7 +1095,7 @@ def _get_error_response(error):
             # prefer a submittable example assignment
             assignments = [
                 a for a in error.course.assignments if a.submittable]
-            assignments += course.assignments  # but fall back to any one
+            assignments += error.course.assignments  # but fall back to any one
             hint = (
                 'Remember to use the full name for the assignment in the\n'
                 'subject.  For example:\n'
@@ -1041,9 +1117,25 @@ def _get_error_response(error):
     elif isinstance(error, _UnsignedMessage):
         subject = 'unsigned message {}'.format(error.message['Message-ID'])
         text = (
-            'We received an email message from you without a valid\n'
-            'PGP signature.'
+            'We received an email message from you without a PGP\n'
+            'signature.'
             )
+    elif isinstance(error, WrongSignatureMessage):
+        lines = [
+            'We received an email message from you without a valid',
+            'PGP signature.  We were expecting a signature by',
+            '{}, but got signatures by:'.format(error.person.pgp_key),
+            ]
+        lines.extend(['  {}'.format(s.fingerprint) for s in error.signatures])
+        text = '\n'.join(lines)
+    elif isinstance(error, UnverifiedSignatureMessage):
+        text = (
+            'We received an email message from you with an unverified\n'
+            'signature:\n\n'
+            '{}\n\n'
+            'If this is the key you intended to use, contact your\n'
+            'professor or TA.'
+            ).format(error.signature.dumps(prefix='  '))
     elif isinstance(error, _PermissionViolationMessage):
         text = (
             'We received an email from you with the following subject:\n'
@@ -1053,7 +1145,11 @@ def _get_error_response(error):
             '  * {}').format(
             error.subject, '\n  * '.join(error.allowed_groups))
     elif isinstance(error, _InvalidMessage):
-        text = subject
+        text = (
+            'We received an email from you with the following subject:\n'
+            '  {!r}\n'
+            'but the message was invalid:\n'
+            '  {}').format(error.subject, error)
     else:
         raise NotImplementedError((type(error), error))
     if target is None: