Implement profile_image upload and remove endpoints

TNL-1537

Co-Authored-By: Andy Armstrong <andya@edx.org>
Co-Authored-By: cahrens <christina@edx.org>

common/djangoapps/enrollment/views.py

@@ -18,7 +18,10 @@ from opaque_keys.edx.keys import CourseKey
 from embargo import api as embargo_api
 from cors_csrf.authentication import SessionAuthenticationCrossDomainCsrf
 from cors_csrf.decorators import ensure_csrf_cookie_cross_domain
-from util.authentication import SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser
+from openedx.core.lib.api.authentication import (
+    SessionAuthenticationAllowInactiveUser,
+    OAuth2AuthenticationAllowInactiveUser,
+)
 from util.disable_rate_limit import can_disable_rate_limit
 from enrollment import api
 from enrollment.errors import (

lms/djangoapps/commerce/views.py

@@ -15,7 +15,7 @@ from course_modes.models import CourseMode
 from courseware import courses
 from enrollment.api import add_enrollment
 from student.models import CourseEnrollment
-from util.authentication import SessionAuthenticationAllowInactiveUser
+from openedx.core.lib.api.authentication import SessionAuthenticationAllowInactiveUser
 
 
 log = logging.getLogger(__name__)

lms/djangoapps/mobile_api/utils.py

@@ -2,14 +2,16 @@
 Common utility methods and decorators for Mobile APIs.
 """
 
-
 import functools
 from rest_framework import permissions
-
-from util.authentication import SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser
 from opaque_keys.edx.keys import CourseKey
 from xmodule.modulestore.django import modulestore
+
 from courseware.courses import get_course_with_access
+from openedx.core.lib.api.authentication import (
+    SessionAuthenticationAllowInactiveUser,
+    OAuth2AuthenticationAllowInactiveUser,
+)
 from openedx.core.lib.api.permissions import IsUserInUrl
 
 

lms/envs/common.py

@@ -2259,7 +2259,13 @@ FIELD_OVERRIDE_PROVIDERS = ()
 
 # PROFILE IMAGE CONFIG
 # TODO: add these settings to aws.py as well
-PROFILE_IMAGE_BACKEND = 'django.core.files.storage.FileSystemStorage'
+# WARNING: Certain django storage backends do not support atomic
+# file overwrites (including the default, specified below) - instead
+# there are separate calls to delete and then write a new file in the
+# storage backend.  This introduces the risk of a race condition
+# occurring when a user uploads a new profile image to replace an
+# earlier one (the file will temporarily be deleted).
+PROFILE_IMAGE_BACKEND = 'storages.backends.overwrite.OverwriteStorage'
 # PROFILE_IMAGE_DOMAIN points to the domain from which we serve image
 # files from.  When this is '/', it refers to the same domain as the
 # app server.  If serving from a different domain, specify that here
@@ -2272,4 +2278,5 @@ PROFILE_IMAGE_DEFAULT_FILENAME = 'default_profile_image'  # TODO: determine fina
 # platform unaware of current image URLs, resulting in reverting all
 # users' profile images to the default placeholder image.
 PROFILE_IMAGE_SECRET_KEY = 'placeholder secret key'
-
+PROFILE_IMAGE_MAX_BYTES = 1024 * 1024
+PROFILE_IMAGE_MIN_BYTES = 100

lms/envs/test.py

@@ -483,3 +483,5 @@ PROFILE_IMAGE_DOMAIN = 'http://example-storage.com/'
 PROFILE_IMAGE_URL_PATH = 'profile_images/'
 PROFILE_IMAGE_DEFAULT_FILENAME = 'default'
 PROFILE_IMAGE_SECRET_KEY = 'secret'
+PROFILE_IMAGE_MAX_BYTES = 1024 * 1024
+PROFILE_IMAGE_MIN_BYTES = 100

lms/urls.py

@@ -91,6 +91,7 @@ urlpatterns = (
 if settings.FEATURES["ENABLE_USER_REST_API"]:
     urlpatterns += (
         url(r'^api/user/', include('openedx.core.djangoapps.user_api.urls')),
+        url(r'^api/profile_images/', include('openedx.core.djangoapps.profile_images.urls')),
     )
 
 if settings.FEATURES["ENABLE_COMBINED_LOGIN_REGISTRATION"]:
@@ -637,6 +638,9 @@ urlpatterns = patterns(*urlpatterns)
 
 if settings.DEBUG:
     urlpatterns += static(settings.STATIC_URL, document_root=settings.STATIC_ROOT)
+    urlpatterns += static(
+        settings.PROFILE_IMAGE_DOMAIN + settings.PROFILE_IMAGE_URL_PATH, document_root=settings.MEDIA_ROOT
+    )
 
     # in debug mode, allow any template to be rendered (most useful for UX reference templates)
     urlpatterns += url(r'^template/(?P<template>.+)$', 'debug.views.show_reference_template'),

openedx/core/djangoapps/profile_images/images.py

@@ -0,0 +1,132 @@
+"""
+Image file manipulation functions related to profile images.
+"""
+from cStringIO import StringIO
+
+from django.conf import settings
+from django.core.files.base import ContentFile
+from django.utils.translation import ugettext as _, ugettext_noop as _noop
+from PIL import Image
+
+from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_storage
+
+
+FILE_UPLOAD_TOO_LARGE = _noop('Maximum file size exceeded.')
+FILE_UPLOAD_TOO_SMALL = _noop('Minimum file size not met.')
+FILE_UPLOAD_BAD_TYPE = _noop('Unsupported file type.')
+FILE_UPLOAD_BAD_EXT = _noop('File extension does not match data.')
+FILE_UPLOAD_BAD_MIMETYPE = _noop('Content-Type header does not match data.')
+
+
+class ImageValidationError(Exception):
+    """
+    Exception to use when the system rejects a user-supplied source image.
+    """
+    @property
+    def user_message(self):
+        """
+        Translate the developer-facing exception message for API clients.
+        """
+        # pylint: disable=translation-of-non-string
+        return _(self.message)
+
+
+def validate_uploaded_image(uploaded_file):
+    """
+    Raises ImageValidationError if the server should refuse to use this
+    uploaded file as the source image for a user's profile image.  Otherwise,
+    returns nothing.
+    """
+    # validation code by @pmitros,
+    # adapted from https://github.com/pmitros/ProfileXBlock
+    # see also: http://en.wikipedia.org/wiki/Magic_number_%28programming%29
+    image_types = {
+        'jpeg': {
+            'extension': [".jpeg", ".jpg"],
+            'mimetypes': ['image/jpeg', 'image/pjpeg'],
+            'magic': ["ffd8"]
+        },
+        'png': {
+            'extension': [".png"],
+            'mimetypes': ['image/png'],
+            'magic': ["89504e470d0a1a0a"]
+        },
+        'gif': {
+            'extension': [".gif"],
+            'mimetypes': ['image/gif'],
+            'magic': ["474946383961", "474946383761"]
+        }
+    }
+
+    if uploaded_file.size > settings.PROFILE_IMAGE_MAX_BYTES:
+        raise ImageValidationError(FILE_UPLOAD_TOO_LARGE)
+    elif uploaded_file.size < settings.PROFILE_IMAGE_MIN_BYTES:
+        raise ImageValidationError(FILE_UPLOAD_TOO_SMALL)
+
+    # check the file extension looks acceptable
+    filename = unicode(uploaded_file.name).lower()
+    filetype = [ft for ft in image_types if any(filename.endswith(ext) for ext in image_types[ft]['extension'])]
+    if not filetype:
+        raise ImageValidationError(FILE_UPLOAD_BAD_TYPE)
+    filetype = filetype[0]
+
+    # check mimetype matches expected file type
+    if uploaded_file.content_type not in image_types[filetype]['mimetypes']:
+        raise ImageValidationError(FILE_UPLOAD_BAD_MIMETYPE)
+
+    # check magic number matches expected file type
+    headers = image_types[filetype]['magic']
+    if uploaded_file.read(len(headers[0]) / 2).encode('hex') not in headers:
+        raise ImageValidationError(FILE_UPLOAD_BAD_EXT)
+    # avoid unexpected errors from subsequent modules expecting the fp to be at 0
+    uploaded_file.seek(0)
+
+
+def _get_scaled_image_file(image_obj, size):
+    """
+    Given a PIL.Image object, get a resized copy using `size` (square) and
+    return a file-like object containing the data saved as a JPEG.
+
+    Note that the file object returned is a django ContentFile which holds
+    data in memory (not on disk).
+    """
+    if image_obj.mode != "RGB":
+        image_obj = image_obj.convert("RGB")
+    scaled = image_obj.resize((size, size), Image.ANTIALIAS)
+    string_io = StringIO()
+    scaled.save(string_io, format='JPEG')
+    image_file = ContentFile(string_io.getvalue())
+    return image_file
+
+
+def create_profile_images(image_file, profile_image_names):
+    """
+    Generates a set of image files based on image_file and
+    stores them according to the sizes and filenames specified
+    in `profile_image_names`.
+    """
+    image_obj = Image.open(image_file)
+
+    # first center-crop the image if needed (but no scaling yet).
+    width, height = image_obj.size
+    if width != height:
+        side = width if width < height else height
+        image_obj = image_obj.crop(((width - side) / 2, (height - side) / 2, (width + side) / 2, (height + side) / 2))
+
+    storage = get_profile_image_storage()
+    for size, name in profile_image_names.items():
+        scaled_image_file = _get_scaled_image_file(image_obj, size)
+        # Store the file.
+        try:
+            storage.save(name, scaled_image_file)
+        finally:
+            scaled_image_file.close()
+
+
+def remove_profile_images(profile_image_names):
+    """
+    Physically remove the image files specified in `profile_image_names`
+    """
+    storage = get_profile_image_storage()
+    for name in profile_image_names.values():
+        storage.delete(name)

openedx/core/djangoapps/profile_images/tests/helpers.py

@@ -0,0 +1,55 @@
+"""
+Helper methods for use in profile image tests.
+"""
+from contextlib import contextmanager
+import os
+from tempfile import NamedTemporaryFile
+
+from django.core.files.uploadedfile import UploadedFile
+from PIL import Image
+
+
+@contextmanager
+def make_image_file(dimensions=(320, 240), extension=".jpeg", force_size=None):
+    """
+    Yields a named temporary file created with the specified image type and
+    options.
+
+    Note the default dimensions are unequal (not a square) ensuring that center-square
+    cropping logic will be exercised during tests.
+
+    The temporary file will be closed and deleted automatically upon exiting
+    the `with` block.
+    """
+    image = Image.new('RGB', dimensions, "green")
+    image_file = NamedTemporaryFile(suffix=extension)
+    try:
+        image.save(image_file)
+        if force_size is not None:
+            image_file.seek(0, os.SEEK_END)
+            bytes_to_pad = force_size - image_file.tell()
+            # write in hunks of 256 bytes
+            hunk, byte_ = bytearray([0] * 256), bytearray([0])
+            num_hunks, remainder = divmod(bytes_to_pad, 256)
+            for _ in xrange(num_hunks):
+                image_file.write(hunk)
+            for _ in xrange(remainder):
+                image_file.write(byte_)
+            image_file.flush()
+        image_file.seek(0)
+        yield image_file
+    finally:
+        image_file.close()
+
+
+@contextmanager
+def make_uploaded_file(content_type, *a, **kw):
+    """
+    Wrap the result of make_image_file in a django UploadedFile.
+    """
+    with make_image_file(*a, **kw) as image_file:
+        yield UploadedFile(
+            image_file,
+            content_type=content_type,
+            size=os.path.getsize(image_file.name),
+        )

openedx/core/djangoapps/profile_images/tests/test_images.py

@@ -0,0 +1,182 @@
+"""
+Test cases for image processing functions in the profile image package.
+"""
+from contextlib import closing
+from itertools import product
+import os
+from tempfile import NamedTemporaryFile
+import unittest
+
+from django.conf import settings
+from django.core.files.uploadedfile import UploadedFile
+from django.test import TestCase
+from django.test.utils import override_settings
+import ddt
+import mock
+from PIL import Image
+
+from ..images import (
+    FILE_UPLOAD_TOO_LARGE,
+    FILE_UPLOAD_TOO_SMALL,
+    FILE_UPLOAD_BAD_TYPE,
+    FILE_UPLOAD_BAD_EXT,
+    FILE_UPLOAD_BAD_MIMETYPE,
+    create_profile_images,
+    ImageValidationError,
+    remove_profile_images,
+    validate_uploaded_image,
+)
+from .helpers import make_image_file, make_uploaded_file
+
+
+@ddt.ddt
+@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Profile Image API is only supported in LMS')
+class TestValidateUploadedImage(TestCase):
+    """
+    Test validate_uploaded_image
+    """
+    def check_validation_result(self, uploaded_file, expected_failure_message):
+        """
+        Internal DRY helper.
+        """
+        if expected_failure_message is not None:
+            with self.assertRaises(ImageValidationError) as ctx:
+                validate_uploaded_image(uploaded_file)
+            self.assertEqual(ctx.exception.message, expected_failure_message)
+        else:
+            validate_uploaded_image(uploaded_file)
+            self.assertEqual(uploaded_file.tell(), 0)
+
+    @ddt.data(
+        (99, FILE_UPLOAD_TOO_SMALL),
+        (100, ),
+        (1024, ),
+        (1025, FILE_UPLOAD_TOO_LARGE),
+    )
+    @ddt.unpack
+    @override_settings(PROFILE_IMAGE_MIN_BYTES=100, PROFILE_IMAGE_MAX_BYTES=1024)
+    def test_file_size(self, upload_size, expected_failure_message=None):
+        """
+        Ensure that files outside the accepted size range fail validation.
+        """
+        with make_uploaded_file(
+            dimensions=(1, 1), extension=".png", content_type="image/png", force_size=upload_size
+        ) as uploaded_file:
+            self.check_validation_result(uploaded_file, expected_failure_message)
+
+    @ddt.data(
+        (".gif", "image/gif"),
+        (".jpg", "image/jpeg"),
+        (".jpeg", "image/jpeg"),
+        (".png", "image/png"),
+        (".bmp", "image/bmp", FILE_UPLOAD_BAD_TYPE),
+        (".tif", "image/tiff", FILE_UPLOAD_BAD_TYPE),
+    )
+    @ddt.unpack
+    def test_extension(self, extension, content_type, expected_failure_message=None):
+        """
+        Ensure that files whose extension is not supported fail validation.
+        """
+        with make_uploaded_file(extension=extension, content_type=content_type) as uploaded_file:
+            self.check_validation_result(uploaded_file, expected_failure_message)
+
+    def test_extension_mismatch(self):
+        """
+        Ensure that validation fails when the file extension does not match the
+        file data.
+        """
+        # make a bmp, try to fool the function into thinking it's a jpeg
+        with make_image_file(extension=".bmp") as bmp_file:
+            with closing(NamedTemporaryFile(suffix=".jpeg")) as fake_jpeg_file:
+                fake_jpeg_file.write(bmp_file.read())
+                fake_jpeg_file.seek(0)
+                uploaded_file = UploadedFile(
+                    fake_jpeg_file,
+                    content_type="image/jpeg",
+                    size=os.path.getsize(fake_jpeg_file.name)
+                )
+                with self.assertRaises(ImageValidationError) as ctx:
+                    validate_uploaded_image(uploaded_file)
+                self.assertEqual(ctx.exception.message, FILE_UPLOAD_BAD_EXT)
+
+    def test_content_type(self):
+        """
+        Ensure that validation fails when the content_type header and file
+        extension do not match
+        """
+        with make_uploaded_file(extension=".jpeg", content_type="image/gif") as uploaded_file:
+            with self.assertRaises(ImageValidationError) as ctx:
+                validate_uploaded_image(uploaded_file)
+            self.assertEqual(ctx.exception.message, FILE_UPLOAD_BAD_MIMETYPE)
+
+
+@ddt.ddt
+@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Profile Image API is only supported in LMS')
+class TestGenerateProfileImages(TestCase):
+    """
+    Test create_profile_images
+    """
+    @ddt.data(
+        *product(
+            ["gif", "jpg", "png"],
+            [(1, 1), (10, 10), (100, 100), (1000, 1000), (1, 10), (10, 100), (100, 1000), (1000, 999)],
+        )
+    )
+    @ddt.unpack
+    def test_generation(self, image_type, dimensions):
+        """
+        Ensure that regardless of the input format or dimensions, the outcome
+        of calling the function is square jpeg files with explicitly-requested
+        dimensions being saved to the profile image storage backend.
+        """
+        extension = "." + image_type
+        content_type = "image/" + image_type
+        requested_sizes = {
+            10: "ten.jpg",
+            100: "hundred.jpg",
+            1000: "thousand.jpg",
+        }
+        mock_storage = mock.Mock()
+        with make_uploaded_file(dimensions=dimensions, extension=extension, content_type=content_type) as uploaded_file:
+            with mock.patch(
+                "openedx.core.djangoapps.profile_images.images.get_profile_image_storage",
+                return_value=mock_storage,
+            ):
+                create_profile_images(uploaded_file, requested_sizes)
+                names_and_files = [v[0] for v in mock_storage.save.call_args_list]
+                actual_sizes = {}
+                for name, file_ in names_and_files:
+                    # get the size of the image file and ensure it's square jpeg
+                    with closing(Image.open(file_)) as image_obj:
+                        width, height = image_obj.size
+                        self.assertEqual(width, height)
+                        self.assertEqual(image_obj.format, 'JPEG')
+                        actual_sizes[width] = name
+                self.assertEqual(requested_sizes, actual_sizes)
+                mock_storage.save.reset_mock()
+
+
+@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Profile Image API is only supported in LMS')
+class TestRemoveProfileImages(TestCase):
+    """
+    Test remove_profile_images
+    """
+    def test_remove(self):
+        """
+        Ensure that the outcome of calling the function is that the named images
+        are deleted from the profile image storage backend.
+        """
+        requested_sizes = {
+            10: "ten.jpg",
+            100: "hundred.jpg",
+            1000: "thousand.jpg",
+        }
+        mock_storage = mock.Mock()
+        with mock.patch(
+            "openedx.core.djangoapps.profile_images.images.get_profile_image_storage",
+            return_value=mock_storage,
+        ):
+            remove_profile_images(requested_sizes)
+            deleted_names = [v[0][0] for v in mock_storage.delete.call_args_list]
+            self.assertEqual(requested_sizes.values(), deleted_names)
+            mock_storage.save.reset_mock()

openedx/core/djangoapps/profile_images/tests/test_views.py

@@ -0,0 +1,304 @@
+"""
+Test cases for the HTTP endpoints of the profile image api.
+"""
+from contextlib import closing
+import unittest
+
+import ddt
+from django.conf import settings
+from django.core.urlresolvers import reverse
+import mock
+from mock import patch
+
+from PIL import Image
+from rest_framework.test import APITestCase, APIClient
+
+from student.tests.factories import UserFactory
+
+from ...user_api.accounts.image_helpers import (
+    set_has_profile_image,
+    get_profile_image_names,
+    get_profile_image_storage
+)
+from ..images import create_profile_images, ImageValidationError
+from .helpers import make_image_file
+
+TEST_PASSWORD = "test"
+
+
+class ProfileImageEndpointTestCase(APITestCase):
+    """
+    Base class / shared infrastructure for tests of profile_image "upload" and
+    "remove" endpoints.
+    """
+    # subclasses should override this with the name of the view under test, as
+    # per the urls.py configuration.
+    _view_name = None
+
+    def setUp(self):
+        super(ProfileImageEndpointTestCase, self).setUp()
+        self.user = UserFactory.create(password=TEST_PASSWORD)
+        # Ensure that parental controls don't apply to this user
+        self.user.profile.year_of_birth = 1980
+        self.user.profile.save()
+        self.url = reverse(self._view_name, kwargs={'username': self.user.username})
+        self.client.login(username=self.user.username, password=TEST_PASSWORD)
+        self.storage = get_profile_image_storage()
+        # this assertion is made here as a sanity check because all tests
+        # assume user.profile.has_profile_image is False by default
+        self.assertFalse(self.user.profile.has_profile_image)
+
+    def tearDown(self):
+        super(ProfileImageEndpointTestCase, self).tearDown()
+        for name in get_profile_image_names(self.user.username).values():
+            self.storage.delete(name)
+
+    def check_images(self, exist=True):
+        """
+        If exist is True, make sure the images physically exist in storage
+        with correct sizes and formats.
+
+        If exist is False, make sure none of the images exist.
+        """
+        for size, name in get_profile_image_names(self.user.username).items():
+            if exist:
+                self.assertTrue(self.storage.exists(name))
+                with closing(Image.open(self.storage.path(name))) as img:
+                    self.assertEqual(img.size, (size, size))
+                    self.assertEqual(img.format, 'JPEG')
+            else:
+                self.assertFalse(self.storage.exists(name))
+
+    def check_response(self, response, expected_code, expected_developer_message=None, expected_user_message=None):
+        """
+        Make sure the response has the expected code, and if that isn't 204,
+        optionally check the correctness of a developer-facing message.
+        """
+        self.assertEqual(expected_code, response.status_code)
+        if expected_code == 204:
+            self.assertIsNone(response.data)
+        else:
+            if expected_developer_message is not None:
+                self.assertEqual(response.data.get('developer_message'), expected_developer_message)
+            if expected_user_message is not None:
+                self.assertEqual(response.data.get('user_message'), expected_user_message)
+
+    def check_has_profile_image(self, has_profile_image=True):
+        """
+        Make sure the value of self.user.profile.has_profile_image is what we
+        expect.
+        """
+        # it's necessary to reload this model from the database since save()
+        # would have been called on another instance.
+        profile = self.user.profile.__class__.objects.get(user=self.user)
+        self.assertEqual(profile.has_profile_image, has_profile_image)
+
+
+@ddt.ddt
+@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Profile Image API is only supported in LMS')
+class ProfileImageUploadTestCase(ProfileImageEndpointTestCase):
+    """
+    Tests for the profile_image upload endpoint.
+    """
+    _view_name = "profile_image_upload"
+
+    def test_unsupported_methods(self):
+        """
+        Test that GET, PUT, PATCH, and DELETE are not supported.
+        """
+        self.assertEqual(405, self.client.get(self.url).status_code)
+        self.assertEqual(405, self.client.put(self.url).status_code)
+        self.assertEqual(405, self.client.patch(self.url).status_code)
+        self.assertEqual(405, self.client.delete(self.url).status_code)
+
+    def test_anonymous_access(self):
+        """
+        Test that an anonymous client (not logged in) cannot POST.
+        """
+        anonymous_client = APIClient()
+        response = anonymous_client.post(self.url)
+        self.assertEqual(401, response.status_code)
+
+    def test_upload_self(self):
+        """
+        Test that an authenticated user can POST to their own upload endpoint.
+        """
+        with make_image_file() as image_file:
+            response = self.client.post(self.url, {'file': image_file}, format='multipart')
+            self.check_response(response, 204)
+            self.check_images()
+            self.check_has_profile_image()
+
+    def test_upload_other(self):
+        """
+        Test that an authenticated user cannot POST to another user's upload endpoint.
+        """
+        different_user = UserFactory.create(password=TEST_PASSWORD)
+        different_client = APIClient()
+        different_client.login(username=different_user.username, password=TEST_PASSWORD)
+        with make_image_file() as image_file:
+            response = different_client.post(self.url, {'file': image_file}, format='multipart')
+            self.check_response(response, 404)
+            self.check_images(False)
+            self.check_has_profile_image(False)
+
+    def test_upload_staff(self):
+        """
+        Test that an authenticated staff cannot POST to another user's upload endpoint.
+        """
+        staff_user = UserFactory(is_staff=True, password=TEST_PASSWORD)
+        staff_client = APIClient()
+        staff_client.login(username=staff_user.username, password=TEST_PASSWORD)
+        with make_image_file() as image_file:
+            response = staff_client.post(self.url, {'file': image_file}, format='multipart')
+            self.check_response(response, 403)
+            self.check_images(False)
+            self.check_has_profile_image(False)
+
+    def test_upload_missing_file(self):
+        """
+        Test that omitting the file entirely from the POST results in HTTP 400.
+        """
+        response = self.client.post(self.url, {}, format='multipart')
+        self.check_response(
+            response, 400,
+            expected_developer_message=u"No file provided for profile image",
+            expected_user_message=u"No file provided for profile image",
+        )
+        self.check_images(False)
+        self.check_has_profile_image(False)
+
+    def test_upload_not_a_file(self):
+        """
+        Test that sending unexpected data that isn't a file results in HTTP
+        400.
+        """
+        response = self.client.post(self.url, {'file': 'not a file'}, format='multipart')
+        self.check_response(
+            response, 400,
+            expected_developer_message=u"No file provided for profile image",
+            expected_user_message=u"No file provided for profile image",
+        )
+        self.check_images(False)
+        self.check_has_profile_image(False)
+
+    def test_upload_validation(self):
+        """
+        Test that when upload validation fails, the proper HTTP response and
+        messages are returned.
+        """
+        with make_image_file() as image_file:
+            with mock.patch(
+                'openedx.core.djangoapps.profile_images.views.validate_uploaded_image',
+                side_effect=ImageValidationError(u"test error message")
+            ):
+                response = self.client.post(self.url, {'file': image_file}, format='multipart')
+                self.check_response(
+                    response, 400,
+                    expected_developer_message=u"test error message",
+                    expected_user_message=u"test error message",
+                )
+                self.check_images(False)
+                self.check_has_profile_image(False)
+
+    @patch('PIL.Image.open')
+    def test_upload_failure(self, image_open):
+        """
+        Test that when upload validation fails, the proper HTTP response and
+        messages are returned.
+        """
+        image_open.side_effect = [Exception(u"whoops"), None]
+        with make_image_file() as image_file:
+            response = self.client.post(self.url, {'file': image_file}, format='multipart')
+            self.check_response(
+                response, 400,
+                expected_developer_message=u"Upload failed for profile image: whoops",
+                expected_user_message=u"Upload failed for profile image",
+            )
+            self.check_images(False)
+            self.check_has_profile_image(False)
+
+
+@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Profile Image API is only supported in LMS')
+class ProfileImageRemoveTestCase(ProfileImageEndpointTestCase):
+    """
+    Tests for the profile_image remove endpoint.
+    """
+    _view_name = "profile_image_remove"
+
+    def setUp(self):
+        super(ProfileImageRemoveTestCase, self).setUp()
+        with make_image_file() as image_file:
+            create_profile_images(image_file, get_profile_image_names(self.user.username))
+            self.check_images()
+            set_has_profile_image(self.user.username, True)
+
+    def test_unsupported_methods(self):
+        """
+        Test that GET, PUT, PATCH, and DELETE are not supported.
+        """
+        self.assertEqual(405, self.client.get(self.url).status_code)
+        self.assertEqual(405, self.client.put(self.url).status_code)
+        self.assertEqual(405, self.client.patch(self.url).status_code)
+        self.assertEqual(405, self.client.delete(self.url).status_code)
+
+    def test_anonymous_access(self):
+        """
+        Test that an anonymous client (not logged in) cannot call GET or POST.
+        """
+        anonymous_client = APIClient()
+        for request in (anonymous_client.get, anonymous_client.post):
+            response = request(self.url)
+            self.assertEqual(401, response.status_code)
+
+    def test_remove_self(self):
+        """
+        Test that an authenticated user can POST to remove their own profile
+        images.
+        """
+        response = self.client.post(self.url)
+        self.check_response(response, 204)
+        self.check_images(False)
+        self.check_has_profile_image(False)
+
+    def test_remove_other(self):
+        """
+        Test that an authenticated user cannot POST to remove another user's
+        profile images.
+        """
+        different_user = UserFactory.create(password=TEST_PASSWORD)
+        different_client = APIClient()
+        different_client.login(username=different_user.username, password=TEST_PASSWORD)
+        response = different_client.post(self.url)
+        self.check_response(response, 404)
+        self.check_images(True)  # thumbnails should remain intact.
+        self.check_has_profile_image(True)
+
+    def test_remove_staff(self):
+        """
+        Test that an authenticated staff user can POST to remove another user's
+        profile images.
+        """
+        staff_user = UserFactory(is_staff=True, password=TEST_PASSWORD)
+        staff_client = APIClient()
+        staff_client.login(username=staff_user.username, password=TEST_PASSWORD)
+        response = self.client.post(self.url)
+        self.check_response(response, 204)
+        self.check_images(False)
+        self.check_has_profile_image(False)
+
+    @patch('student.models.UserProfile.save')
+    def test_remove_failure(self, user_profile_save):
+        """
+        Test that when upload validation fails, the proper HTTP response and
+        messages are returned.
+        """
+        user_profile_save.side_effect = [Exception(u"whoops"), None]
+        response = self.client.post(self.url)
+        self.check_response(
+            response, 400,
+            expected_developer_message=u"Delete failed for profile image: whoops",
+            expected_user_message=u"Delete failed for profile image",
+        )
+        self.check_images(True)  # thumbnails should remain intact.
+        self.check_has_profile_image(True)

openedx/core/djangoapps/profile_images/urls.py

@@ -0,0 +1,22 @@
+"""
+Defines the URL routes for this app.
+"""
+from .views import ProfileImageUploadView, ProfileImageRemoveView
+
+from django.conf.urls import patterns, url
+
+USERNAME_PATTERN = r'(?P<username>[\w.+-]+)'
+
+urlpatterns = patterns(
+    '',
+    url(
+        r'^v0/' + USERNAME_PATTERN + '/upload$',
+        ProfileImageUploadView.as_view(),
+        name="profile_image_upload"
+    ),
+    url(
+        r'^v0/' + USERNAME_PATTERN + '/remove$',
+        ProfileImageRemoveView.as_view(),
+        name="profile_image_remove"
+    ),
+)

openedx/core/djangoapps/profile_images/views.py

@@ -0,0 +1,152 @@
+"""
+This module implements the upload and remove endpoints of the profile image api.
+"""
+from contextlib import closing
+
+from django.utils.translation import ugettext as _
+from rest_framework import permissions, status
+from rest_framework.parsers import MultiPartParser, FormParser
+from rest_framework.response import Response
+from rest_framework.views import APIView
+
+from openedx.core.djangoapps.user_api.errors import UserNotFound
+from openedx.core.lib.api.authentication import (
+    OAuth2AuthenticationAllowInactiveUser,
+    SessionAuthenticationAllowInactiveUser,
+)
+from openedx.core.lib.api.permissions import IsUserInUrl, IsUserInUrlOrStaff
+from openedx.core.djangoapps.user_api.accounts.image_helpers import set_has_profile_image, get_profile_image_names
+from .images import validate_uploaded_image, create_profile_images, remove_profile_images, ImageValidationError
+
+
+class ProfileImageUploadView(APIView):
+    """
+    **Use Cases**
+
+        Uploads an image to be used for the user's profile.
+
+    **Example Requests**:
+
+        POST /api/profile_images/v0/{username}/upload
+
+    **Response for POST**
+
+        Users can only upload their own profile image. If the requesting user does not have username
+        "username", this method will return with a status of 403 for staff access but a 404 for ordinary
+        users to avoid leaking the existence of the account.
+
+        This method will also return a 404 if no user exists with username "username".
+
+        If the upload could not be performed then this method returns a 400 with specific errors
+        in the returned JSON.
+
+        If the update is successful, a 204 status is returned with no additional content.
+
+    """
+    parser_classes = (MultiPartParser, FormParser,)
+
+    authentication_classes = (OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser)
+    permission_classes = (permissions.IsAuthenticated, IsUserInUrl)
+
+    def post(self, request, username):
+        """
+        POST /api/profile_images/v0/{username}/upload
+        """
+        # validate request:
+        # verify that the user's
+        # ensure any file was sent
+        if 'file' not in request.FILES:
+            return Response(
+                {
+                    "developer_message": u"No file provided for profile image",
+                    "user_message": _(u"No file provided for profile image"),
+
+                },
+                status=status.HTTP_400_BAD_REQUEST
+            )
+
+        try:
+            # process the upload.
+            uploaded_file = request.FILES['file']
+
+            # no matter what happens, delete the temporary file when we're done
+            with closing(uploaded_file):
+
+                # image file validation.
+                try:
+                    validate_uploaded_image(uploaded_file)
+                except ImageValidationError as error:
+                    return Response(
+                        {"developer_message": error.message, "user_message": error.user_message},
+                        status=status.HTTP_400_BAD_REQUEST,
+                    )
+
+                # generate profile pic and thumbnails and store them
+                create_profile_images(uploaded_file, get_profile_image_names(username))
+
+                # update the user account to reflect that a profile image is available.
+                set_has_profile_image(username, True)
+        except Exception as error:
+            return Response(
+                {
+                    "developer_message": u"Upload failed for profile image: {error}".format(error=error),
+                    "user_message": _(u"Upload failed for profile image"),
+
+                },
+                status=status.HTTP_400_BAD_REQUEST
+            )
+
+        # send client response.
+        return Response(status=status.HTTP_204_NO_CONTENT)
+
+
+class ProfileImageRemoveView(APIView):
+    """
+    **Use Cases**
+
+        Removes all of the profile images associated with the user's account.
+
+    **Example Requests**:
+
+        POST /api/profile_images/v0/{username}/remove
+
+    **Response for POST**
+
+        Users are authorized to delete their own profile images, while staff can delete images for
+        any account. All other users will receive a 404 to avoid leaking the existence of the account.
+
+        This method will also return a 404 if no user exists with username "username".
+
+        If the delete could not be performed then this method returns a 400 with specific errors
+        in the returned JSON.
+
+        If the delete is successful, a 204 status is returned with no additional content.
+
+    """
+    authentication_classes = (OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser)
+    permission_classes = (permissions.IsAuthenticated, IsUserInUrlOrStaff)
+
+    def post(self, request, username):  # pylint: disable=unused-argument
+        """
+        POST /api/profile_images/v0/{username}/remove
+        """
+        try:
+            # update the user account to reflect that the images were removed.
+            set_has_profile_image(username, False)
+
+            # remove physical files from storage.
+            remove_profile_images(get_profile_image_names(username))
+        except UserNotFound:
+            return Response(status=status.HTTP_404_NOT_FOUND)
+        except Exception as error:
+            return Response(
+                {
+                    "developer_message": u"Delete failed for profile image: {error}".format(error=error),
+                    "user_message": _(u"Delete failed for profile image"),
+
+                },
+                status=status.HTTP_400_BAD_REQUEST
+            )
+
+        # send client response.
+        return Response(status=status.HTTP_204_NO_CONTENT)

openedx/core/djangoapps/user_api/accounts/helpers.py

@@ -1,57 +0,0 @@
-"""
-Helper functions for the accounts API.
-"""
-import hashlib
-
-from django.conf import settings
-from django.core.files.storage import FileSystemStorage, get_storage_class
-
-PROFILE_IMAGE_SIZES_MAP = {
-    'full': 500,
-    'large': 120,
-    'medium': 50,
-    'small': 30
-}
-_PROFILE_IMAGE_SIZES = PROFILE_IMAGE_SIZES_MAP.values()
-PROFILE_IMAGE_FORMAT = 'jpg'
-
-
-def get_profile_image_url_for_user(user, size):
-    """Return the URL to a user's profile image for a given size.
-    Note that based on the value of
-    django.conf.settings.PROFILE_IMAGE_DOMAIN, the URL may be relative,
-    and in that case the caller is responsible for constructing the full
-    URL.
-
-    If the user has not yet uploaded a profile image, return the URL to
-    the default edX user profile image.
-
-    Arguments:
-        user (django.auth.User): The user for whom we're generating a
-        profile image URL.
-
-    Returns:
-        string: The URL for the user's profile image.
-
-    Raises:
-        ValueError: The caller asked for an unsupported image size.
-    """
-    if size not in _PROFILE_IMAGE_SIZES:
-        raise ValueError('Unsupported profile image size: {size}'.format(size=size))
-
-    if user.profile.has_profile_image:
-        name = hashlib.md5(settings.PROFILE_IMAGE_SECRET_KEY + user.username).hexdigest()
-    else:
-        name = settings.PROFILE_IMAGE_DEFAULT_FILENAME
-
-    filename = '{name}_{size}.{format}'.format(name=name, size=size, format=PROFILE_IMAGE_FORMAT)
-
-    # Note that, for now, the backend will be FileSystemStorage.  When
-    # we eventually support s3 storage, we'll need to pass a parameter
-    # to the storage class indicating the s3 bucket which we're using
-    # for profile picture uploads.
-    storage_class = get_storage_class(settings.PROFILE_IMAGE_BACKEND)
-    if storage_class == FileSystemStorage:
-        kwargs = {'base_url': (settings.PROFILE_IMAGE_DOMAIN + settings.PROFILE_IMAGE_URL_PATH)}
-    storage = storage_class(**kwargs)
-    return storage.url(filename)

openedx/core/djangoapps/user_api/accounts/image_helpers.py

@@ -0,0 +1,115 @@
+"""
+Helper functions for the accounts API.
+"""
+import hashlib
+
+from django.conf import settings
+from django.core.exceptions import ObjectDoesNotExist
+from django.core.files.storage import get_storage_class
+
+from student.models import UserProfile
+from ..errors import UserNotFound
+
+
+PROFILE_IMAGE_SIZES_MAP = {
+    'full': 500,
+    'large': 120,
+    'medium': 50,
+    'small': 30
+}
+_PROFILE_IMAGE_SIZES = PROFILE_IMAGE_SIZES_MAP.values()
+
+
+def get_profile_image_storage():
+    """
+    Configures and returns a django Storage instance that can be used
+    to physically locate, read and write profile images.
+    """
+    storage_class = get_storage_class(settings.PROFILE_IMAGE_BACKEND)
+    return storage_class(base_url=(settings.PROFILE_IMAGE_DOMAIN + settings.PROFILE_IMAGE_URL_PATH))
+
+
+def _make_profile_image_name(username):
+    """
+    Returns the user-specific part of the image filename, based on a hash of
+    the username.
+    """
+    return hashlib.md5(settings.PROFILE_IMAGE_SECRET_KEY + username).hexdigest()
+
+
+def _get_profile_image_filename(name, size):
+    """
+    Returns the full filename for a profile image, given the name and size.
+    """
+    return '{name}_{size}.jpg'.format(name=name, size=size)
+
+
+def _get_profile_image_urls(name):
+    """
+    Returns a dict containing the urls for a complete set of profile images,
+    keyed by "friendly" name (e.g. "full", "large", "medium", "small").
+    """
+    storage = get_profile_image_storage()
+    return {
+        size_display_name: storage.url(_get_profile_image_filename(name, size))
+        for size_display_name, size in PROFILE_IMAGE_SIZES_MAP.items()
+    }
+
+
+def get_profile_image_names(username):
+    """
+    Returns a dict containing the filenames for a complete set of profile
+    images, keyed by pixel size.
+    """
+    name = _make_profile_image_name(username)
+    return {size: _get_profile_image_filename(name, size) for size in _PROFILE_IMAGE_SIZES}
+
+
+def get_profile_image_urls_for_user(user):
+    """
+    Return a dict {size:url} for each profile image for a given user.
+    Notes:
+      - this function does not determine whether the set of profile images
+    exists, only what the URLs will be if they do exist.  It is assumed that
+    callers will use `_get_default_profile_image_urls` instead to provide
+    a set of urls that point to placeholder images, when there are no user-
+    submitted images.
+      - based on the value of django.conf.settings.PROFILE_IMAGE_DOMAIN,
+    the URL may be relative, and in that case the caller is responsible for
+    constructing the full URL if needed.
+
+    Arguments:
+        user (django.contrib.auth.User): the user for whom we are getting urls.
+
+    Returns:
+        dictionary of {size_display_name: url} for each image.
+
+    """
+    if user.profile.has_profile_image:
+        return _get_profile_image_urls(_make_profile_image_name(user.username))
+    else:
+        return _get_default_profile_image_urls()
+
+
+def _get_default_profile_image_urls():
+    """
+    Returns a dict {size:url} for a complete set of default profile images,
+    used as a placeholder when there are no user-submitted images.
+
+    TODO The result of this function should be memoized, but not in tests.
+    """
+    return _get_profile_image_urls(settings.PROFILE_IMAGE_DEFAULT_FILENAME)
+
+
+def set_has_profile_image(username, has_profile_image=True):
+    """
+    System (not user-facing) API call used to store whether the user has
+    uploaded a profile image.  Used by profile_image API.
+    """
+    try:
+        profile = UserProfile.objects.get(user__username=username)
+    except ObjectDoesNotExist:
+        raise UserNotFound()
+
+    profile.has_profile_image = has_profile_image
+    profile.save()

openedx/core/djangoapps/user_api/accounts/serializers.py

@@ -4,7 +4,8 @@ from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH
 from openedx.core.djangoapps.user_api.serializers import ReadOnlyFieldsSerializerMixin
 
 from student.models import UserProfile, LanguageProficiency
-from .helpers import get_profile_image_url_for_user, PROFILE_IMAGE_SIZES_MAP
+from .image_helpers import get_profile_image_urls_for_user
+
 
 PROFILE_IMAGE_KEY_PREFIX = 'image_url'
 
@@ -100,10 +101,10 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
     def get_profile_image(self, user_profile):
         """ Returns metadata about a user's profile image. """
         data = {'has_image': user_profile.has_profile_image}
+        urls = get_profile_image_urls_for_user(user_profile.user)
         data.update({
-            '{image_key_prefix}_{size}'.format(image_key_prefix=PROFILE_IMAGE_KEY_PREFIX, size=size_display_name):
-            get_profile_image_url_for_user(user_profile.user, size_value)
-            for size_display_name, size_value in PROFILE_IMAGE_SIZES_MAP.items()
+            '{image_key_prefix}_{size}'.format(image_key_prefix=PROFILE_IMAGE_KEY_PREFIX, size=size_display_name): url
+            for size_display_name, url in urls.items()
         })
         return data
 

openedx/core/djangoapps/user_api/accounts/tests/test_api.py

@@ -192,12 +192,13 @@ class TestAccountApi(TestCase):
         self.assertEqual(0, len(pending_change))
 
 
-@patch('openedx.core.djangoapps.user_api.accounts.helpers._PROFILE_IMAGE_SIZES', [50, 10])
+@patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10])
 @patch.dict(
-    'openedx.core.djangoapps.user_api.accounts.helpers.PROFILE_IMAGE_SIZES_MAP', {'full': 50, 'small': 10}, clear=True
+    'openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', {'full': 50, 'small': 10}, clear=True
 )
 @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS')
 class AccountSettingsOnCreationTest(TestCase):
+    # pylint: disable=missing-docstring
 
     USERNAME = u'frank-underwood'
     PASSWORD = u'ṕáśśẃőŕd'

openedx/core/djangoapps/user_api/accounts/tests/test_helpers.py

@@ -1,69 +0,0 @@
-"""
-Tests for helpers.py
-"""
-from ddt import ddt, data
-import hashlib
-from mock import patch
-from unittest import skipUnless
-
-from django.conf import settings
-from django.test import TestCase
-
-from student.tests.factories import UserFactory
-
-from ...models import UserProfile
-from ..helpers import get_profile_image_url_for_user
-
-
-@ddt
-@patch('openedx.core.djangoapps.user_api.accounts.helpers._PROFILE_IMAGE_SIZES', [50, 10])
-@patch.dict(
-    'openedx.core.djangoapps.user_api.accounts.helpers.PROFILE_IMAGE_SIZES_MAP', {'full': 50, 'small': 10}, clear=True
-)
-@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
-class ProfileImageUrlTestCase(TestCase):
-    """
-    Tests for `get_profile_image_url_for_user`.
-    """
-    def setUp(self):
-        super(ProfileImageUrlTestCase, self).setUp()
-        self.user = UserFactory()
-
-        # Ensure that parental controls don't apply to this user
-        self.user.profile.year_of_birth = 1980
-        self.user.profile.save()
-
-    def verify_url(self, user, pixels, filename):
-        """
-        Helper method to verify that we're correctly generating profile
-        image URLs.
-        """
-        self.assertEqual(
-            get_profile_image_url_for_user(user, pixels),
-            'http://example-storage.com/profile_images/{filename}_{pixels}.jpg'.format(filename=filename, pixels=pixels)
-        )
-
-    @data(10, 50)
-    def test_profile_image_urls(self, pixels):
-        """
-        Verify we get the URL to the default image if the user does not
-        have a profile image.
-        """
-        # By default new users will have no profile image.
-        self.verify_url(self.user, pixels, 'default')
-        # A user can add an image, then remove it.  We should get the
-        # default image URL in that case.
-        self.user.profile.has_profile_image = True
-        self.user.profile.save()
-        self.verify_url(self.user, pixels, hashlib.md5('secret' + self.user.username).hexdigest())
-        self.user.profile.has_profile_image = False
-        self.user.profile.save()
-        self.verify_url(self.user, pixels, 'default')
-
-    @data(1, 5000)
-    def test_unsupported_sizes(self, image_size):
-        """
-        Verify that we cannot ask for image sizes which are unsupported.
-        """
-        with self.assertRaises(ValueError):
-            get_profile_image_url_for_user(self.user, image_size)

openedx/core/djangoapps/user_api/accounts/tests/test_image_helpers.py

@@ -0,0 +1,60 @@
+"""
+Tests for helpers.py
+"""
+import hashlib
+from mock import patch
+from unittest import skipUnless
+
+from django.conf import settings
+from django.test import TestCase
+
+from ..image_helpers import get_profile_image_urls_for_user
+from student.tests.factories import UserFactory
+
+TEST_SIZES = {'full': 50, 'small': 10}
+
+
+@patch.dict('openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', TEST_SIZES, clear=True)
+@skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
+class ProfileImageUrlTestCase(TestCase):
+    """
+    Tests for profile image URL generation helpers.
+    """
+    def setUp(self):
+        super(ProfileImageUrlTestCase, self).setUp()
+        self.user = UserFactory()
+        # Ensure that parental controls don't apply to this user
+        self.user.profile.year_of_birth = 1980
+        self.user.profile.has_profile_image = False
+        self.user.profile.save()
+
+    def verify_url(self, actual_url, expected_name, expected_pixels):
+        """
+        Verify correct url structure.
+        """
+        self.assertEqual(
+            actual_url,
+            'http://example-storage.com/profile_images/{0}_{1}.jpg'.format(expected_name, expected_pixels),
+        )
+
+    def verify_urls(self, expected_name, actual_urls):
+        """
+        Verify correct url dictionary structure.
+        """
+        self.assertEqual(set(TEST_SIZES.keys()), set(actual_urls.keys()))
+        for size_display_name, url in actual_urls.items():
+            self.verify_url(url, expected_name, TEST_SIZES[size_display_name])
+
+    def test_get_profile_image_urls(self):
+        """
+        Tests `get_profile_image_urls_for_user`
+        """
+        self.user.profile.has_profile_image = True
+        self.user.profile.save()
+        expected_name = hashlib.md5('secret' + self.user.username).hexdigest()
+        actual_urls = get_profile_image_urls_for_user(self.user)
+        self.verify_urls(expected_name, actual_urls)
+
+        self.user.profile.has_profile_image = False
+        self.user.profile.save()
+        self.verify_urls('default', get_profile_image_urls_for_user(self.user))

openedx/core/djangoapps/user_api/accounts/tests/test_views.py

@@ -97,9 +97,9 @@ class UserAPITestCase(APITestCase):
 
 @ddt.ddt
 @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS')
-@patch('openedx.core.djangoapps.user_api.accounts.helpers._PROFILE_IMAGE_SIZES', [50, 10])
+@patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10])
 @patch.dict(
-    'openedx.core.djangoapps.user_api.accounts.helpers.PROFILE_IMAGE_SIZES_MAP', {'full': 50, 'small': 10}, clear=True
+    'openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', {'full': 50, 'small': 10}, clear=True
 )
 class TestAccountAPI(UserAPITestCase):
     """
@@ -200,7 +200,7 @@ class TestAccountAPI(UserAPITestCase):
         """
         client = self.login_client(api_client, user)
         response = client.get(reverse("accounts_api", kwargs={'username': "does_not_exist"}))
-        self.assertEqual(404, response.status_code)
+        self.assertEqual(403 if user == "staff_user" else 404, response.status_code)
 
     # Note: using getattr so that the patching works even if there is no configuration.
     # This is needed when testing CMS as the patching is still executed even though the
@@ -279,7 +279,6 @@ class TestAccountAPI(UserAPITestCase):
             for empty_field in ("year_of_birth", "level_of_education", "mailing_address", "bio"):
                 self.assertIsNone(data[empty_field])
             self.assertIsNone(data["country"])
-            # TODO: what should the format of this be?
             self.assertEqual("m", data["gender"])
             self.assertEqual("Learn a lot", data["goals"])
             self.assertEqual(self.user.email, data["email"])
@@ -323,7 +322,7 @@ class TestAccountAPI(UserAPITestCase):
         is_staff access).
         """
         client = self.login_client(api_client, user)
-        self.send_patch(client, {}, expected_status=404)
+        self.send_patch(client, {}, expected_status=403 if user == "staff_user" else 404)
 
     @ddt.data(
         ("client", "user"),

openedx/core/djangoapps/user_api/accounts/views.py

@@ -8,9 +8,12 @@ from django.db import transaction
 from rest_framework.views import APIView
 from rest_framework.response import Response
 from rest_framework import status
-from util.authentication import SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser
 from rest_framework import permissions
 
+from openedx.core.lib.api.authentication import (
+    SessionAuthenticationAllowInactiveUser,
+    OAuth2AuthenticationAllowInactiveUser,
+)
 from ..errors import UserNotFound, UserNotAuthorized, AccountUpdateError, AccountValidationError
 from openedx.core.lib.api.parsers import MergePatchParser
 from .api import get_account_settings, update_account_settings
@@ -123,8 +126,10 @@ class AccountView(APIView):
 
         **Response Values for PATCH**
 
-            Users can modify only their own account information. If the user
-            attempts to modify another user's account, a 404 error is returned.
+            Users can only modify their own account information. If the requesting
+            user does not have username "username", this method will return with
+            a status of 403 for staff access but a 404 for ordinary users
+            to avoid leaking the existence of the account.
 
             If no user exists with the specified username, a 404 error is
             returned.
@@ -158,7 +163,7 @@ class AccountView(APIView):
                 if key.startswith(PROFILE_IMAGE_KEY_PREFIX):
                     account_settings['profile_image'][key] = request.build_absolute_uri(value)
         except UserNotFound:
-            return Response(status=status.HTTP_404_NOT_FOUND)
+            return Response(status=status.HTTP_403_FORBIDDEN if request.user.is_staff else status.HTTP_404_NOT_FOUND)
 
         return Response(account_settings)
 
@@ -173,7 +178,9 @@ class AccountView(APIView):
         try:
             with transaction.commit_on_success():
                 update_account_settings(request.user, request.DATA, username=username)
-        except (UserNotFound, UserNotAuthorized):
+        except UserNotAuthorized:
+            return Response(status=status.HTTP_403_FORBIDDEN if request.user.is_staff else status.HTTP_404_NOT_FOUND)
+        except UserNotFound:
             return Response(status=status.HTTP_404_NOT_FOUND)
         except AccountValidationError as err:
             return Response({"field_errors": err.field_errors}, status=status.HTTP_400_BAD_REQUEST)

openedx/core/djangoapps/user_api/preferences/api.py

@@ -256,7 +256,6 @@ def update_email_opt_in(user, org, opt_in):
         log.warn(u"Could not update organization wide preference due to IntegrityError: {}".format(err.message))
 
 
-
 def _track_update_email_opt_in(user_id, organization, opt_in):
     """Track an email opt-in preference change.
 

openedx/core/djangoapps/user_api/preferences/tests/test_views.py

@@ -160,7 +160,7 @@ class TestPreferencesAPI(UserAPITestCase):
                 "dict_pref": {"int_key": 10},
                 "string_pref": "value",
             },
-            expected_status=404,
+            expected_status=403 if user == "staff_user" else 404,
         )
 
     def test_update_preferences(self):
@@ -288,7 +288,7 @@ class TestPreferencesAPI(UserAPITestCase):
                 "new_pref": "new_value",
                 "extra_pref": None,
             },
-            expected_status=404
+            expected_status=403 if user == "staff_user" else 404
         )
 
 
@@ -503,7 +503,7 @@ class TestPreferencesDetailAPI(UserAPITestCase):
         self._set_url("new_key")
         client = self.login_client(api_client, user)
         new_value = "new value"
-        self.send_put(client, new_value, expected_status=404)
+        self.send_put(client, new_value, expected_status=403 if user == "staff_user" else 404)
 
     @ddt.data(
         (u"new value",),
@@ -531,7 +531,7 @@ class TestPreferencesDetailAPI(UserAPITestCase):
         """
         client = self.login_client(api_client, user)
         new_value = "new value"
-        self.send_put(client, new_value, expected_status=404)
+        self.send_put(client, new_value, expected_status=403 if user == "staff_user" else 404)
 
     @ddt.data(
         (None,),
@@ -578,4 +578,4 @@ class TestPreferencesDetailAPI(UserAPITestCase):
         Test that a client (logged in) cannot delete a preference for another user.
         """
         client = self.login_client(api_client, user)
-        self.send_delete(client, expected_status=404)
+        self.send_delete(client, expected_status=403 if user == "staff_user" else 404)

openedx/core/djangoapps/user_api/preferences/views.py

@@ -7,13 +7,17 @@ https://openedx.atlassian.net/wiki/display/TNL/User+API
 from rest_framework.views import APIView
 from rest_framework.response import Response
 from rest_framework import status
-from util.authentication import SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser
 from rest_framework import permissions
 
 from django.db import transaction
 from django.utils.translation import ugettext as _
 
+from openedx.core.lib.api.authentication import (
+    SessionAuthenticationAllowInactiveUser,
+    OAuth2AuthenticationAllowInactiveUser,
+)
 from openedx.core.lib.api.parsers import MergePatchParser
+from openedx.core.lib.api.permissions import IsUserInUrlOrStaff
 from ..errors import UserNotFound, UserNotAuthorized, PreferenceValidationError, PreferenceUpdateError
 from .api import (
     get_user_preference, get_user_preferences, set_user_preference, update_user_preferences, delete_user_preference
@@ -45,7 +49,8 @@ class PreferencesView(APIView):
         **Response for PATCH**
 
             Users can only modify their own preferences. If the requesting user does not have username
-            "username", this method will return with a status of 404.
+            "username", this method will return with a status of 403 for staff access but a 404 for ordinary
+            users to avoid leaking the existence of the account.
 
             This method will also return a 404 if no user exists with username "username".
 
@@ -61,7 +66,7 @@ class PreferencesView(APIView):
 
     """
     authentication_classes = (OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser)
-    permission_classes = (permissions.IsAuthenticated,)
+    permission_classes = (permissions.IsAuthenticated, IsUserInUrlOrStaff)
     parser_classes = (MergePatchParser,)
 
     def get(self, request, username):
@@ -70,7 +75,9 @@ class PreferencesView(APIView):
         """
         try:
             user_preferences = get_user_preferences(request.user, username=username)
-        except (UserNotFound, UserNotAuthorized):
+        except UserNotAuthorized:
+            return Response(status=status.HTTP_403_FORBIDDEN)
+        except UserNotFound:
             return Response(status=status.HTTP_404_NOT_FOUND)
 
         return Response(user_preferences)
@@ -91,7 +98,9 @@ class PreferencesView(APIView):
         try:
             with transaction.commit_on_success():
                 update_user_preferences(request.user, request.DATA, username=username)
-        except (UserNotFound, UserNotAuthorized):
+        except UserNotAuthorized:
+            return Response(status=status.HTTP_403_FORBIDDEN)
+        except UserNotFound:
             return Response(status=status.HTTP_404_NOT_FOUND)
         except PreferenceValidationError as error:
             return Response(
@@ -136,17 +145,25 @@ class PreferencesDetailView(APIView):
 
             A successful put returns a 204 and no content.
 
-            If the specified username or preference does not exist, this method returns a 404.
+            Users can only update their own preferences. If the requesting user does not have username
+            "username", this method will return with a status of 403 for staff access but a 404 for ordinary
+            users to avoid leaking the existence of the account.
+
+            If the specified preference does not exist, this method returns a 404.
 
         **Response for DELETE**
 
             A successful delete returns a 204 and no content.
 
-            If the specified username or preference does not exist, this method returns a 404.
+            Users can only delete their own preferences. If the requesting user does not have username
+            "username", this method will return with a status of 403 for staff access but a 404 for ordinary
+            users to avoid leaking the existence of the account.
+
+            If the specified preference does not exist, this method returns a 404.
 
     """
     authentication_classes = (OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser)
-    permission_classes = (permissions.IsAuthenticated,)
+    permission_classes = (permissions.IsAuthenticated, IsUserInUrlOrStaff)
 
     def get(self, request, username, preference_key):
         """
@@ -157,7 +174,9 @@ class PreferencesDetailView(APIView):
             # There was no preference with that key, raise a 404.
             if value is None:
                 return Response(status=status.HTTP_404_NOT_FOUND)
-        except (UserNotFound, UserNotAuthorized):
+        except UserNotAuthorized:
+            return Response(status=status.HTTP_403_FORBIDDEN)
+        except UserNotFound:
             return Response(status=status.HTTP_404_NOT_FOUND)
         return Response(value)
 
@@ -167,7 +186,9 @@ class PreferencesDetailView(APIView):
         """
         try:
             set_user_preference(request.user, preference_key, request.DATA, username=username)
-        except (UserNotFound, UserNotAuthorized):
+        except UserNotAuthorized:
+            return Response(status=status.HTTP_403_FORBIDDEN)
+        except UserNotFound:
             return Response(status=status.HTTP_404_NOT_FOUND)
         except PreferenceValidationError as error:
             return Response(
@@ -193,7 +214,9 @@ class PreferencesDetailView(APIView):
         """
         try:
             preference_existed = delete_user_preference(request.user, preference_key, username=username)
-        except (UserNotFound, UserNotAuthorized):
+        except UserNotAuthorized:
+            return Response(status=status.HTTP_403_FORBIDDEN)
+        except UserNotFound:
             return Response(status=status.HTTP_404_NOT_FOUND)
         except PreferenceUpdateError as error:
             return Response(

openedx/core/djangoapps/user_api/tests/test_views.py

@@ -1693,6 +1693,7 @@ class TestGoogleRegistrationView(
     """Tests the User API registration endpoint with Google authentication."""
     pass
 
+
 @ddt.ddt
 class UpdateEmailOptInTestCase(ApiTestCase, ModuleStoreTestCase):
     """Tests the UpdateEmailOptInPreference view. """

openedx/core/djangoapps/user_api/views.py

@@ -1,9 +1,6 @@
 """HTTP end-points for the User API. """
 import copy
-from openedx.core.lib.api.permissions import ApiKeyHeaderPermission
 from opaque_keys import InvalidKeyError
-import third_party_auth
-
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.http import HttpResponse
@@ -21,12 +18,14 @@ from rest_framework import viewsets
 from rest_framework.views import APIView
 from rest_framework.exceptions import ParseError
 from django_countries import countries
-from django_comment_common.models import Role
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
-from edxmako.shortcuts import marketing_link
 
+from openedx.core.lib.api.permissions import ApiKeyHeaderPermission
+import third_party_auth
+from django_comment_common.models import Role
+from edxmako.shortcuts import marketing_link
 from student.views import create_account_with_params, set_marketing_cookie
-from util.authentication import SessionAuthenticationAllowInactiveUser
+from openedx.core.lib.api.authentication import SessionAuthenticationAllowInactiveUser
 from util.json_request import JsonResponse
 from .preferences.api import update_email_opt_in
 from .helpers import FormDescription, shim_student_view, require_post_params

openedx/core/lib/api/permissions.py

@@ -52,9 +52,26 @@ class IsUserInUrl(permissions.BasePermission):
     Permission that checks to see if the request user matches the user in the URL.
     """
     def has_permission(self, request, view):
-        # Return a 404 instead of a 403 (Unauthorized). If one user is looking up
-        # other users, do not let them deduce the existence of an account.
+        """
+        Returns true if the current request is by the user themselves.
+
+        Note: a 404 is returned for non-staff instead of a 403. This is to prevent
+        users from being able to detect the existence of accounts.
+        """
         url_username = request.parser_context.get('kwargs', {}).get('username', '')
         if request.user.username.lower() != url_username.lower():
+            if request.user.is_staff:
+                return False  # staff gets 403
             raise Http404()
         return True
+
+
+class IsUserInUrlOrStaff(IsUserInUrl):
+    """
+    Permission that checks to see if the request user matches the user in the URL or has is_staff access.
+    """
+    def has_permission(self, request, view):
+        if request.user.is_staff:
+            return True
+
+        return super(IsUserInUrlOrStaff, self).has_permission(request, view)