Thanks for the contribution! I like the idea.

I will need to test this a bit before merging though ...

The one thing that bothers me currently is how the attachments are stored. I'm not sure if a simple folder in MEDIA_ROOT is the best choice. This approach has at least some privacy-issues...

It might be a good idea to have a configurable storage-engine for the attachments. With that it would be possible to store attachments outside of MEDIA_ROOT, so they are not publicly available to everyone who knows the url. The default could still be MEDIA_ROOT, but we should provide a mechanism, so that it can be changed on a per-project basis.

If stored in MEDIA_ROOT we should at least do something with the filename/path. Maybe store in a folder named with the md5 hash of file contents or so...
Otherwise, if two people upload a file with the same name, the later one will be renamed, which leaks the information to the user, that another file with the original name already exists and he can then access it, because he knows the url.

Example:
User 1 uploads "test.txt". It will be available at MEDIA_ROOT/attachments/test.txt.
Then User 2 uploads "test.txt" (different content). It will be renamed and be available at MEDIA_ROOT/attachments/test_sXcftg.txt.
User 2 now knows (by looking at the URL of his uploaded attachment in sent-messages), that a different file with URL MEDIA_ROOT/attachments/test.txt must exist and he can then open it by url, even though it was never idented for him.

Hi,

You are totally right !

(note that the attachment code will still use the configured DEFAULT_FILE_STORAGE regardless of the upload_to argument).

What we could do at best IMO:

1. Poor's man securing of the storage directory: MEDIA_ROOT/attachments/<some hash with sender/recipients ids and SECRET_KEY>/test.txt. This could be optional with a DJANGO_MESSAGES_SECURE_UPLOAD_DIR = False

2. Add a note in the documentation about security

3. Allow the user to pass a specific storage backend to the attachment.file FileField, eg:

def get_storage_backend():
    backend = getattr(settings, 'DJANGO_MESSAGES_STORAGE_BACKEND', None)
    if backend is None:
        return None
    # build backend from string here, not sure if we should handle backend args/kwargs ?
    return backend

@python_2_unicode_compatible
class Attachment(models.Model):
    """
    A message attachment. You can configure where attachments are stored with
    the ``DJANGO_MESSAGES_UPLOAD_TO`` setting, note that this setting is
    relative to your ``MEDIA_ROOT`` setting.
    """
    file = models.FileField(
        _('File'),
        upload_to=getattr(settings, 'DJANGO_MESSAGES_UPLOAD_TO', 'attachments'),
        storage=get_storage_backend(),
    )
    message = models.ForeignKey(
        Message,
        on_delete=models.CASCADE,
        verbose_name=_('Message'),
        related_name='attachments'
    )

    class Meta:
        verbose_name = _("Attachment")

What do you think ?

I opt for the third solution: allow a custom storage backend.

Done !

Looks good, thank you! I still would like to test the changes in a little example project on my dev machine. So please give me some time before it's getting merged.

Of course, no hurry :)