Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't allow anonymous users to upload files by poking the upload endpoint directly #182

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions dev.xml
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,13 @@ configure_settings = {
},
],
'ROOT_URLCONF': 'tests.urls',
'MIDDLEWARE': [
'django.contrib.sessions.middleware.SessionMiddleware',
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
],
}

settings.configure(**configure_settings)
Expand Down Expand Up @@ -289,6 +296,13 @@ configure_settings = {
},
],
'ROOT_URLCONF': 'tests.urls',
'MIDDLEWARE': [
'django.contrib.sessions.middleware.SessionMiddleware',
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
],
}

settings.configure(**configure_settings)
Expand Down
13 changes: 13 additions & 0 deletions docs-src/customization.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ You may place any of the variables outlined in this page in your `settings.py`,
* [`MARKDOWNX_MARKDOWN_EXTENSION_CONFIGS`](#markdownx_markdown_extension_configs)
* [`MARKDOWNX_URLS_PATH`](#markdownx_urls_path)
* [`MARKDOWNX_UPLOAD_URLS_PATH`](#markdownx_upload_urls_path)
* [`MARKDOWNX_UPLOAD_ALLOW_ANONYMOUS`](#markdownx_upload_allow_anonymous)
* [`MARKDOWNX_MEDIA_PATH`](#markdownx_media_path)
* [`MARKDOWNX_UPLOAD_MAX_SIZE`](#markdownx_upload_max_size)
* [`MARKDOWNX_UPLOAD_CONTENT_TYPES`](#markdownx_upload_content_types)
Expand Down Expand Up @@ -152,6 +153,18 @@ Relative URL to which the Markdown text is sent to be encoded as HTML.
MARKDOWNX_URLS_PATH = '/markdownx/markdownify/'
```


### `MARKDOWNX_UPLOAD_ALLOW_ANONYMOUS`

Default: `False`

Set to `True` if you wish to allow image uploads from anonymous (unauthenticated) users. If you are using MarkdownX in your admin exclusively, or otherwise only to users who are authenticated, you almost certainly do not want to change this from the default of `False`.

```python
MARKDOWNX_UPLOAD_ALLOW_ANONYMOUS = False
```


### `MARKDOWNX_UPLOAD_URLS_PATH`

Default: `'/markdownx/upload/'`
Expand Down
5 changes: 5 additions & 0 deletions docs-src/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ Django MarkdownX may be installed directly using Python Package Index (PyPi):
python3 -m pip install django-markdownx
```

`request.user` must be available via some middleware if you wish to restrict
uploads to logged-in users via the `MARKDOWNX_UPLOAD_ALLOW_ANONYMOUS` setting.
Most likely, you are using `django.contrib.auth.middleware.AuthenticationMiddleware`
already, which sets this attribute.

## From the source

Should you wish to download and install it using the source code, you can do as follows:
Expand Down
2 changes: 2 additions & 0 deletions markdownx/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ def _mdx(var, default):

# Image
# --------------------
MARKDOWNX_UPLOAD_ALLOW_ANONYMOUS = _mdx('UPLOAD_ALLOW_ANONYMOUS', False)

MARKDOWNX_UPLOAD_MAX_SIZE = _mdx('UPLOAD_MAX_SIZE', FIFTY_MEGABYTES)

MARKDOWNX_UPLOAD_CONTENT_TYPES = _mdx('UPLOAD_CONTENT_TYPES', VALID_CONTENT_TYPES)
Expand Down
39 changes: 36 additions & 3 deletions markdownx/tests/tests.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,51 @@
import os
import re
from contextlib import contextmanager
from unittest import mock

from django.test import TestCase
from django.urls import reverse


class SimpleTest(TestCase):

def test_upload(self):
@contextmanager
def _get_image_fp(self):
full_path = os.path.join(
os.path.dirname(__file__),
'static',
'django-markdownx-preview.png',
)
with open(full_path, 'rb') as fp:
yield fp

def test_upload_anonymous_fails(self):
url = reverse('markdownx_upload')
with open('markdownx/tests/static/django-markdownx-preview.png', 'rb') as fp:

# Test that image upload fails for an anonymous user when
# MARKDOWNX_UPLOAD_ALLOW_ANONYMOUS is the default False.
with self._get_image_fp() as fp:
response = self.client.post(url, {'image': fp}, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
json = response.json()
self.assertEqual(response.status_code, 403)

def test_upload_anonymous_succeeds_with_setting(self):
"""
Ensures that uploads succeed when MARKDOWNX_UPLOAD_ALLOW_ANONYMOUS
is True. This implicitly tests the authenticated case as well.
"""
url = reverse('markdownx_upload')

# A patch is required here because the view sets the
# MARKDOWNX_UPLOAD_ALLOW_ANONYMOUS at first import, reading from
# django.conf.settings once, which means Django's standard
# override_settings helper does not work. There's probably a case for
# re-working the app-local settings.
with mock.patch('markdownx.settings.MARKDOWNX_UPLOAD_ALLOW_ANONYMOUS', True):
with self._get_image_fp() as fp:
response = self.client.post(url, {'image': fp}, HTTP_X_REQUESTED_WITH='XMLHttpRequest')

self.assertEqual(response.status_code, 200)
json = response.json()
self.assertIn('image_code', json)

match = re.findall(r'(markdownx/[\w\-]+\.png)', json['image_code'])
Expand Down
19 changes: 17 additions & 2 deletions markdownx/views.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
from django.core.exceptions import PermissionDenied
from django.http import HttpResponse
from django.http import JsonResponse
from django.utils.module_loading import import_string
from django.views.generic.edit import BaseFormView
from django.views.generic.edit import View

from . import settings
from .forms import ImageForm
from .settings import MARKDOWNX_MARKDOWNIFY_FUNCTION

markdownify_func = import_string(MARKDOWNX_MARKDOWNIFY_FUNCTION)
markdownify_func = import_string(settings.MARKDOWNX_MARKDOWNIFY_FUNCTION)


class MarkdownifyView(View):
Expand Down Expand Up @@ -39,6 +40,20 @@ class ImageUploadView(BaseFormView):
form_class = ImageForm
success_url = '/'

def dispatch(self, request, *args, **kwargs):
"""
Raises PermissionDenied if the current user is not authenticated and
MARKDOWNX_UPLOAD_ALLOW_ANONYMOUS is not set.

:param request: Django request
:type request: django.http.request.HttpRequest
:rtype: django.http.JsonResponse, django.http.HttpResponse
"""
if not settings.MARKDOWNX_UPLOAD_ALLOW_ANONYMOUS and not request.user.is_authenticated:
raise PermissionDenied

return super().dispatch(request, *args, **kwargs)

def form_invalid(self, form):
"""
Handling of invalid form events.
Expand Down