From 8a3205f13b298f9c831bd2c6e822d09fe3f799ae Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 3 Jul 2013 10:04:22 -0400 Subject: [PATCH 1/3] Add an API to interact with users and preferences The new API uses Django REST Framework. For now, it is designed specifically to support the use cases required by the forum digest notifier (not yet built), with a goal of making it more generally useful over time. --- lms/djangoapps/user_api/__init__.py | 0 .../user_api/migrations/0001_initial.py | 78 ++++ .../user_api/migrations/__init__.py | 0 lms/djangoapps/user_api/models.py | 12 + lms/djangoapps/user_api/serializers.py | 26 ++ lms/djangoapps/user_api/tests.py | 361 ++++++++++++++++++ lms/djangoapps/user_api/views.py | 33 ++ lms/envs/common.py | 4 + lms/urls.py | 10 + requirements/edx/base.txt | 2 + 10 files changed, 526 insertions(+) create mode 100644 lms/djangoapps/user_api/__init__.py create mode 100644 lms/djangoapps/user_api/migrations/0001_initial.py create mode 100644 lms/djangoapps/user_api/migrations/__init__.py create mode 100644 lms/djangoapps/user_api/models.py create mode 100644 lms/djangoapps/user_api/serializers.py create mode 100644 lms/djangoapps/user_api/tests.py create mode 100644 lms/djangoapps/user_api/views.py diff --git a/lms/djangoapps/user_api/__init__.py b/lms/djangoapps/user_api/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/lms/djangoapps/user_api/migrations/0001_initial.py b/lms/djangoapps/user_api/migrations/0001_initial.py new file mode 100644 index 000000000000..754960e36db8 --- /dev/null +++ b/lms/djangoapps/user_api/migrations/0001_initial.py @@ -0,0 +1,78 @@ +# -*- coding: utf-8 -*- +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + + +class Migration(SchemaMigration): + + def forwards(self, orm): + # Adding model 'UserPreference' + db.create_table('user_api_userpreference', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('user', self.gf('django.db.models.fields.related.ForeignKey')(related_name='+', to=orm['auth.User'])), + ('key', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)), + ('value', self.gf('django.db.models.fields.TextField')()), + )) + db.send_create_signal('user_api', ['UserPreference']) + + # Adding unique constraint on 'UserPreference', fields ['user', 'key'] + db.create_unique('user_api_userpreference', ['user_id', 'key']) + + + def backwards(self, orm): + # Removing unique constraint on 'UserPreference', fields ['user', 'key'] + db.delete_unique('user_api_userpreference', ['user_id', 'key']) + + # Deleting model 'UserPreference' + db.delete_table('user_api_userpreference') + + + models = { + 'auth.group': { + 'Meta': {'object_name': 'Group'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}), + 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}) + }, + 'auth.permission': { + 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'}, + 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'auth.user': { + 'Meta': {'object_name': 'User'}, + 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}), + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}), + 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}), + 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}), + 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}), + 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}), + 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'}) + }, + 'contenttypes.contenttype': { + 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"}, + 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}), + 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'user_api.userpreference': { + 'Meta': {'unique_together': "(('user', 'key'),)", 'object_name': 'UserPreference'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'key': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'+'", 'to': "orm['auth.User']"}), + 'value': ('django.db.models.fields.TextField', [], {}) + } + } + + complete_apps = ['user_api'] \ No newline at end of file diff --git a/lms/djangoapps/user_api/migrations/__init__.py b/lms/djangoapps/user_api/migrations/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/lms/djangoapps/user_api/models.py b/lms/djangoapps/user_api/models.py new file mode 100644 index 000000000000..3450c03aa313 --- /dev/null +++ b/lms/djangoapps/user_api/models.py @@ -0,0 +1,12 @@ +from django.contrib.auth.models import User +from django.db import models + + +class UserPreference(models.Model): + """A user's preference, stored as generic text to be processed by client""" + user = models.ForeignKey(User, db_index=True, related_name="+") + key = models.CharField(max_length=255, db_index=True) + value = models.TextField() + + class Meta: + unique_together = ("user", "key") diff --git a/lms/djangoapps/user_api/serializers.py b/lms/djangoapps/user_api/serializers.py new file mode 100644 index 000000000000..42c42cf891b3 --- /dev/null +++ b/lms/djangoapps/user_api/serializers.py @@ -0,0 +1,26 @@ +from django.contrib.auth.models import User +from rest_framework import serializers +from student.models import UserProfile +from user_api.models import UserPreference + + +class UserSerializer(serializers.HyperlinkedModelSerializer): + name = serializers.SerializerMethodField("get_name") + + def get_name(self, user): + profile = UserProfile.objects.get(user=user) + return profile.name + + class Meta: + model = User + # This list is the minimal set required by the notification service + fields = ("id", "email", "name") + read_only_fields = ("id", "email") + + +class UserPreferenceSerializer(serializers.HyperlinkedModelSerializer): + user = UserSerializer() + + class Meta: + model = UserPreference + depth = 1 diff --git a/lms/djangoapps/user_api/tests.py b/lms/djangoapps/user_api/tests.py new file mode 100644 index 000000000000..1266eddda06a --- /dev/null +++ b/lms/djangoapps/user_api/tests.py @@ -0,0 +1,361 @@ +from django.contrib.auth.models import User +from django.db import IntegrityError +from django.test import TestCase +from django.test.utils import override_settings +from factory import DjangoModelFactory +import json +import re +from unittest import SkipTest + +from student.tests.factories import UserFactory +from user_api.models import UserPreference + + +TEST_API_KEY = "test_api_key" +USER_LIST_URI = "/user_api/v1/users/" +USER_PREFERENCE_LIST_URI = "/user_api/v1/user_prefs/" + + +class UserPreferenceFactory(DjangoModelFactory): + FACTORY_FOR = UserPreference + + user = None + key = None + value = "default test value" + + +class UserPreferenceModelTest(TestCase): + def test_duplicate_user_key(self): + user = UserFactory.create() + UserPreferenceFactory.create(user=user, key="testkey", value="first") + self.assertRaises( + IntegrityError, + UserPreferenceFactory.create, + user=user, + key="testkey", + value="second" + ) + + def test_arbitrary_values(self): + user = UserFactory.create() + UserPreferenceFactory.create(user=user, key="testkey0", value="") + UserPreferenceFactory.create(user=user, key="testkey1", value="This is some English text!") + UserPreferenceFactory.create(user=user, key="testkey2", value="{'some': 'json'}") + + +class UserApiTestCase(TestCase): + def setUp(self): + super(UserApiTestCase, self).setUp() + self.users = [ + UserFactory.create( + email="test{0}@test.org".format(i), + profile__name="Test {0}".format(i) + ) + for i in range(5) + ] + self.prefs = [ + UserPreferenceFactory.create(user=self.users[0], key="key0"), + UserPreferenceFactory.create(user=self.users[0], key="key1"), + UserPreferenceFactory.create(user=self.users[1], key="key0") + ] + + def get_with_auth(self, *args, **kwargs): + """Issue a get request to the given URI with the API key header""" + return self.client.get(*args, HTTP_X_EDX_API_KEY=TEST_API_KEY, **kwargs) + + def get_json(self, *args, **kwargs): + """Make a request with the given args and return the parsed JSON repsonse""" + resp = self.get_with_auth(*args, **kwargs) + self.assertHttpOK(resp) + self.assertTrue(resp["Content-Type"].startswith("application/json")) + return json.loads(resp.content) + + def get_uri_for_user(self, target_user): + """Given a user object, get the URI for the corresponding resource""" + users = self.get_json(USER_LIST_URI)["results"] + for user in users: + if user["id"] == target_user.id: + return user["url"] + self.fail() + + def get_uri_for_pref(self, target_pref): + """Given a user preference object, get the URI for the corresponding resource""" + prefs = self.get_json(USER_PREFERENCE_LIST_URI)["results"] + for pref in prefs: + if (pref["user"]["id"] == target_pref.user.id and pref["key"] == target_pref.key): + return pref["url"] + self.fail() + + def assertAllowedMethods(self, uri, expected_methods): + """Assert that the allowed methods for the given URI match the expected list""" + resp = self.client.options(uri) + self.assertHttpOK(resp) + allow_header = resp.get("Allow") + self.assertIsNotNone(allow_header) + allowed_methods = re.split('[^A-Z]+', allow_header) + self.assertItemsEqual(allowed_methods, expected_methods) + + def assertSelfReferential(self, obj): + """Assert that accessing the "url" entry in the given object returns the same object""" + copy = self.get_json(obj["url"]) + self.assertEqual(obj, copy) + + def assertUserIsValid(self, user): + """Assert that the given user result is valid""" + self.assertItemsEqual(user.keys(), ["email", "id", "name", "url"]) + self.assertSelfReferential(user) + + def assertPrefIsValid(self, pref): + self.assertItemsEqual(pref.keys(), ["user", "key", "value", "url"]) + self.assertSelfReferential(pref) + self.assertUserIsValid(pref["user"]) + + def assertHttpOK(self, response): + """Assert that the given response has the status code 200""" + self.assertEqual(response.status_code, 200) + + def assertHttpForbidden(self, response): + """Assert that the given response has the status code 403""" + self.assertEqual(response.status_code, 403) + + def assertHttpMethodNotAllowed(self, response): + """Assert that the given response has the status code 405""" + self.assertEqual(response.status_code, 405) + + +class UserViewSetTest(UserApiTestCase): + LIST_URI = USER_LIST_URI + + def setUp(self): + super(UserViewSetTest, self).setUp() + self.DETAIL_URI = self.get_uri_for_user(self.users[0]) + + # List view tests + + def test_options_list(self): + self.assertAllowedMethods(self.LIST_URI, ["OPTIONS", "GET", "HEAD"]) + + def test_post_list_not_allowed(self): + self.assertHttpMethodNotAllowed(self.client.post(self.LIST_URI)) + + def test_put_list_not_allowed(self): + self.assertHttpMethodNotAllowed(self.client.put(self.LIST_URI)) + + def test_patch_list_not_allowed(self): + raise SkipTest("Django 1.4's test client does not support patch") + + def test_delete_list_not_allowed(self): + self.assertHttpMethodNotAllowed(self.client.delete(self.LIST_URI)) + + @override_settings(EDX_API_KEY=TEST_API_KEY) + def test_list_unauthorized(self): + self.assertHttpForbidden(self.client.get(self.LIST_URI)) + + @override_settings(EDX_API_KEY=TEST_API_KEY) + def test_list_auth(self): + self.assertHttpOK(self.get_with_auth(self.LIST_URI)) + + def test_get_list_empty(self): + User.objects.all().delete() + result = self.get_json(self.LIST_URI) + self.assertEqual(result["count"], 0) + self.assertIsNone(result["next"]) + self.assertIsNone(result["previous"]) + self.assertEqual(result["results"], []) + + def test_get_list_nonempty(self): + result = self.get_json(self.LIST_URI) + self.assertEqual(result["count"], 5) + self.assertIsNone(result["next"]) + self.assertIsNone(result["previous"]) + users = result["results"] + self.assertEqual(len(users), 5) + for user in users: + self.assertUserIsValid(user) + + def test_get_list_pagination(self): + first_page = self.get_json(self.LIST_URI, data={"page_size": 3}) + self.assertEqual(first_page["count"], 5) + first_page_next_uri = first_page["next"] + self.assertIsNone(first_page["previous"]) + first_page_users = first_page["results"] + self.assertEqual(len(first_page_users), 3) + + second_page = self.get_json(first_page_next_uri) + self.assertEqual(second_page["count"], 5) + self.assertIsNone(second_page["next"]) + second_page_prev_uri = second_page["previous"] + second_page_users = second_page["results"] + self.assertEqual(len(second_page_users), 2) + + self.assertEqual(self.get_json(second_page_prev_uri), first_page) + + for user in first_page_users + second_page_users: + self.assertUserIsValid(user) + all_user_uris = [user["url"] for user in first_page_users + second_page_users] + self.assertEqual(len(set(all_user_uris)), 5) + + # Detail view tests + + def test_options_detail(self): + self.assertAllowedMethods(self.DETAIL_URI, ["OPTIONS", "GET", "HEAD"]) + + def test_post_detail_not_allowed(self): + self.assertHttpMethodNotAllowed(self.client.post(self.DETAIL_URI)) + + def test_put_detail_not_allowed(self): + self.assertHttpMethodNotAllowed(self.client.put(self.DETAIL_URI)) + + def test_patch_detail_not_allowed(self): + raise SkipTest("Django 1.4's test client does not support patch") + + def test_delete_detail_not_allowed(self): + self.assertHttpMethodNotAllowed(self.client.delete(self.DETAIL_URI)) + + @override_settings(EDX_API_KEY=TEST_API_KEY) + def test_get_detail_unauthorized(self): + self.assertHttpForbidden(self.client.get(self.DETAIL_URI)) + + @override_settings(EDX_API_KEY=TEST_API_KEY) + def test_detail_auth(self): + self.assertHttpOK(self.get_with_auth(self.DETAIL_URI)) + + def test_get_detail(self): + user = self.users[1] + uri = self.get_uri_for_user(user) + self.assertEqual( + self.get_json(uri), + { + "email": user.email, + "id": user.id, + "name": user.profile.name, + "url": uri, + } + ) + + +class UserPreferenceViewSetTest(UserApiTestCase): + LIST_URI = USER_PREFERENCE_LIST_URI + + def setUp(self): + super(UserPreferenceViewSetTest, self).setUp() + self.DETAIL_URI = self.get_uri_for_pref(self.prefs[0]) + + # List view tests + + def test_options_list(self): + self.assertAllowedMethods(self.LIST_URI, ["OPTIONS", "GET", "HEAD"]) + + def test_put_list_not_allowed(self): + self.assertHttpMethodNotAllowed(self.client.put(self.LIST_URI)) + + def test_patch_list_not_allowed(self): + raise SkipTest("Django 1.4's test client does not support patch") + + def test_delete_list_not_allowed(self): + self.assertHttpMethodNotAllowed(self.client.delete(self.LIST_URI)) + + @override_settings(EDX_API_KEY=TEST_API_KEY) + def test_list_unauthorized(self): + self.assertHttpForbidden(self.client.get(self.LIST_URI)) + + @override_settings(EDX_API_KEY=TEST_API_KEY) + def test_list_auth(self): + self.assertHttpOK(self.get_with_auth(self.LIST_URI)) + + def test_get_list_empty(self): + UserPreference.objects.all().delete() + result = self.get_json(self.LIST_URI) + self.assertEqual(result["count"], 0) + self.assertIsNone(result["next"]) + self.assertIsNone(result["previous"]) + self.assertEqual(result["results"], []) + + def test_get_list_nonempty(self): + result = self.get_json(self.LIST_URI) + self.assertEqual(result["count"], 3) + self.assertIsNone(result["next"]) + self.assertIsNone(result["previous"]) + prefs = result["results"] + self.assertEqual(len(prefs), 3) + for pref in prefs: + self.assertPrefIsValid(pref) + + def test_get_list_filter_key_empty(self): + result = self.get_json(self.LIST_URI, data={"key": "non-existent"}) + self.assertEqual(result["count"], 0) + self.assertEqual(result["results"], []) + + def test_get_list_filter_key_nonempty(self): + result = self.get_json(self.LIST_URI, data={"key": "key0"}) + self.assertEqual(result["count"], 2) + prefs = result["results"] + self.assertEqual(len(prefs), 2) + for pref in prefs: + self.assertPrefIsValid(pref) + self.assertEqual(pref["key"], "key0") + + def test_get_list_pagination(self): + first_page = self.get_json(self.LIST_URI, data={"page_size": 2}) + self.assertEqual(first_page["count"], 3) + first_page_next_uri = first_page["next"] + self.assertIsNone(first_page["previous"]) + first_page_prefs = first_page["results"] + self.assertEqual(len(first_page_prefs), 2) + + second_page = self.get_json(first_page_next_uri) + self.assertEqual(second_page["count"], 3) + self.assertIsNone(second_page["next"]) + second_page_prev_uri = second_page["previous"] + second_page_prefs = second_page["results"] + self.assertEqual(len(second_page_prefs), 1) + + self.assertEqual(self.get_json(second_page_prev_uri), first_page) + + for pref in first_page_prefs + second_page_prefs: + self.assertPrefIsValid(pref) + all_pref_uris = [pref["url"] for pref in first_page_prefs + second_page_prefs] + self.assertEqual(len(set(all_pref_uris)), 3) + + # Detail view tests + + def test_options_detail(self): + self.assertAllowedMethods(self.DETAIL_URI, ["OPTIONS", "GET", "HEAD"]) + + def test_post_detail_not_allowed(self): + self.assertHttpMethodNotAllowed(self.client.post(self.DETAIL_URI)) + + def test_put_detail_not_allowed(self): + self.assertHttpMethodNotAllowed(self.client.put(self.DETAIL_URI)) + + def test_patch_detail_not_allowed(self): + raise SkipTest("Django 1.4's test client does not support patch") + + def test_delete_detail_not_allowed(self): + self.assertHttpMethodNotAllowed(self.client.delete(self.DETAIL_URI)) + + @override_settings(EDX_API_KEY=TEST_API_KEY) + def test_detail_unauthorized(self): + self.assertHttpForbidden(self.client.get(self.DETAIL_URI)) + + @override_settings(EDX_API_KEY=TEST_API_KEY) + def test_detail_auth(self): + self.assertHttpOK(self.get_with_auth(self.DETAIL_URI)) + + def test_get_detail(self): + pref = self.prefs[1] + uri = self.get_uri_for_pref(pref) + self.assertEqual( + self.get_json(uri), + { + "user": { + "email": pref.user.email, + "id": pref.user.id, + "name": pref.user.profile.name, + "url": self.get_uri_for_user(pref.user), + }, + "key": pref.key, + "value": pref.value, + "url": uri, + } + ) diff --git a/lms/djangoapps/user_api/views.py b/lms/djangoapps/user_api/views.py new file mode 100644 index 000000000000..f7c7c65463bd --- /dev/null +++ b/lms/djangoapps/user_api/views.py @@ -0,0 +1,33 @@ +from django.conf import settings +from django.contrib.auth.models import User +from rest_framework import filters +from rest_framework import mixins +from rest_framework import permissions +from rest_framework import viewsets +from user_api.models import UserPreference +from user_api.serializers import UserSerializer, UserPreferenceSerializer + + +class ApiKeyHeaderPermission(permissions.BasePermission): + def has_permission(self, request, view): + """Check for the X-Edx-Api-Key HTTP header, if required""" + api_key = getattr(settings, "EDX_API_KEY", None) + return api_key is None or request.META.get("HTTP_X_EDX_API_KEY") == api_key + + +class UserViewSet(viewsets.ReadOnlyModelViewSet): + permission_classes = (ApiKeyHeaderPermission,) + queryset = User.objects.all() + serializer_class = UserSerializer + paginate_by = 10 + paginate_by_param = "page_size" + + +class UserPreferenceViewSet(viewsets.ReadOnlyModelViewSet): + permission_classes = (ApiKeyHeaderPermission,) + queryset = UserPreference.objects.all() + filter_backends = (filters.DjangoFilterBackend,) + filter_fields = ("key",) + serializer_class = UserPreferenceSerializer + paginate_by = 10 + paginate_by_param = "page_size" diff --git a/lms/envs/common.py b/lms/envs/common.py index 8b2a1f28cfe3..ddbbf0d9ad6c 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -743,6 +743,10 @@ 'django_comment_client', 'django_comment_common', 'notes', + + # User API + 'rest_framework', + 'user_api', ) ######################### MARKETING SITE ############################### diff --git a/lms/urls.py b/lms/urls.py index 085a35b9f480..14fb0a140bc5 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -2,6 +2,8 @@ from django.conf.urls import patterns, include, url from django.contrib import admin from django.conf.urls.static import static +from rest_framework import routers +from user_api import views as user_api_views # Not used, the work is done in the imported module. from . import one_time_startup # pylint: disable=W0611 @@ -61,6 +63,14 @@ url(r'^heartbeat$', include('heartbeat.urls')), ) +user_api_router = routers.DefaultRouter() +user_api_router.register(r'users', user_api_views.UserViewSet) +user_api_router.register(r'user_prefs', user_api_views.UserPreferenceViewSet) + +urlpatterns += ( + url(r'^user_api/v1/', include(user_api_router.urls)), +) + # University profiles only make sense in the default edX context if not settings.MITX_FEATURES["USE_CUSTOM_THEME"]: urlpatterns += ( diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 910b6f3defc3..4c4ab5a29f4f 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -7,6 +7,7 @@ celery==3.0.19 distribute>=0.6.28 django-celery==3.0.17 django-countries==1.5 +django-filter==0.6.0 django-followit==0.0.3 django-keyedcache==1.4-6 django-kombu==0.9.4 @@ -20,6 +21,7 @@ django-ses==0.4.1 django-storages==1.1.5 django-threaded-multihost==1.4-1 django-method-override==0.1.0 +djangorestframework==2.3.5 django==1.4.5 feedparser==5.1.3 fs==0.4.0 From 734c2a60f67fcdab6b850bae76306c6167407e1f Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 5 Jul 2013 14:40:25 -0400 Subject: [PATCH 2/3] Add endpoints to set user notification preference Two endpoints are intended to be used by AJAX calls from the web UI with an authenticated user. When enabled, the value of the preference is an encryption of the username, to be used as a token for one-click unsubscribe links. The third endpoint is the target of unsubscribe links, which displays a page with an appropriate message to the user. --- CHANGELOG.rst | 3 + lms/djangoapps/notification_prefs/__init__.py | 1 + lms/djangoapps/notification_prefs/tests.py | 185 ++++++++++++++++++ lms/djangoapps/notification_prefs/views.py | 119 +++++++++++ lms/envs/common.py | 3 + lms/static/sass/application.scss.mako | 1 + lms/static/sass/shared/_unsubscribe.scss | 23 +++ lms/templates/unsubscribe.html | 17 ++ lms/urls.py | 5 +- requirements/edx/base.txt | 1 + 10 files changed, 357 insertions(+), 1 deletion(-) create mode 100644 lms/djangoapps/notification_prefs/__init__.py create mode 100644 lms/djangoapps/notification_prefs/tests.py create mode 100644 lms/djangoapps/notification_prefs/views.py create mode 100644 lms/static/sass/shared/_unsubscribe.scss create mode 100644 lms/templates/unsubscribe.html diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4d117a9c73ce..eca79f1e9da0 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,9 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +LMS: Added endpoints for AJAX requests to enable/disable notifications +(which are not yet implemented) and a one-click unsubscribe page. + Common: Added *experimental* support for jsinput type. Common: Added setting to specify Celery Broker vhost diff --git a/lms/djangoapps/notification_prefs/__init__.py b/lms/djangoapps/notification_prefs/__init__.py new file mode 100644 index 000000000000..daed38e2b050 --- /dev/null +++ b/lms/djangoapps/notification_prefs/__init__.py @@ -0,0 +1 @@ +NOTIFICATION_PREF_KEY = "notification_pref" diff --git a/lms/djangoapps/notification_prefs/tests.py b/lms/djangoapps/notification_prefs/tests.py new file mode 100644 index 000000000000..4bdb858d88a4 --- /dev/null +++ b/lms/djangoapps/notification_prefs/tests.py @@ -0,0 +1,185 @@ +from django.contrib.auth.models import AnonymousUser +from django.http import Http404 +from django.test import TestCase +from django.test.client import Client, RequestFactory +from django.test.utils import override_settings +from mitxmako.middleware import MakoMiddleware +from student.tests.factories import UserFactory +from user_api.models import UserPreference +from notification_prefs import NOTIFICATION_PREF_KEY +from notification_prefs.views import ajax_enable, ajax_disable, unsubscribe + + +@override_settings(SECRET_KEY="test secret key") +class NotificationPrefViewTest(TestCase): + @classmethod + def setUpClass(cls): + # Make sure global state is set up appropriately + Client().get("/") + + def setUp(self): + self.user = UserFactory.create(username="testuser") + # Username with length equal to AES block length to test padding + self.aes_block_length_user = UserFactory.create(username="sixteencharsuser") + # Tokens are intentionally hard-coded instead of computed to help us + # avoid breaking existing links. + self.tokens = { + # Encrypted value: "testuser" + "\x08" * 8 + self.user: "DyYxCj3oVl9vVgq_VHlfqw==", + # Encrypted value: "sixteencharsuser" + "\x10" * 16 + self.aes_block_length_user: "_E9YK4jYDL1MBMFWd_Dt4tRGw8HDEmlcLVFawgY9wI8=", + } + self.request_factory = RequestFactory() + + def create_prefs(self): + for (user, token) in self.tokens.items(): + UserPreference.objects.create(user=user, key=NOTIFICATION_PREF_KEY, value=token) + + def assertPrefValid(self, user): + self.assertEqual( + UserPreference.objects.get(user=user, key=NOTIFICATION_PREF_KEY).value, + self.tokens[user] + ) + + def assertNotPrefExists(self, user): + self.assertFalse( + UserPreference.objects.filter(user=user, key=NOTIFICATION_PREF_KEY).exists() + ) + + # AJAX enable view + + def test_ajax_enable_get(self): + request = self.request_factory.get("dummy") + request.user = self.user + response = ajax_enable(request) + self.assertEqual(response.status_code, 405) + self.assertNotPrefExists(self.user) + + def test_ajax_enable_anon_user(self): + request = self.request_factory.post("dummy") + request.user = AnonymousUser() + response = ajax_enable(request) + self.assertEqual(response.status_code, 403) + self.assertNotPrefExists(self.user) + + def test_ajax_enable_success(self): + def test_user(user): + request = self.request_factory.post("dummy") + request.user = user + response = ajax_enable(request) + self.assertEqual(response.status_code, 204) + self.assertPrefValid(user) + + test_user(self.user) + test_user(self.aes_block_length_user) + + def test_ajax_enable_already_enabled(self): + self.create_prefs() + request = self.request_factory.post("dummy") + request.user = self.user + response = ajax_enable(request) + self.assertEqual(response.status_code, 204) + self.assertPrefValid(self.user) + + def test_ajax_enable_distinct_values(self): + request = self.request_factory.post("dummy") + request.user = self.user + ajax_enable(request) + other_user = UserFactory.create() + request.user = other_user + ajax_enable(request) + self.assertNotEqual( + UserPreference.objects.get(user=self.user, key=NOTIFICATION_PREF_KEY).value, + UserPreference.objects.get(user=other_user, key=NOTIFICATION_PREF_KEY).value + ) + + # AJAX disable view + + def test_ajax_disable_get(self): + self.create_prefs() + request = self.request_factory.get("dummy") + request.user = self.user + response = ajax_disable(request) + self.assertEqual(response.status_code, 405) + self.assertPrefValid(self.user) + + def test_ajax_disable_anon_user(self): + self.create_prefs() + request = self.request_factory.post("dummy") + request.user = AnonymousUser() + response = ajax_disable(request) + self.assertEqual(response.status_code, 403) + self.assertPrefValid(self.user) + + def test_ajax_disable_success(self): + self.create_prefs() + request = self.request_factory.post("dummy") + request.user = self.user + response = ajax_disable(request) + self.assertEqual(response.status_code, 204) + self.assertNotPrefExists(self.user) + + def test_ajax_disable_already_disabled(self): + request = self.request_factory.post("dummy") + request.user = self.user + response = ajax_disable(request) + self.assertEqual(response.status_code, 204) + self.assertNotPrefExists(self.user) + + # Unsubscribe view + + def test_unsubscribe_post(self): + request = self.request_factory.post("dummy") + response = unsubscribe(request, "dummy") + self.assertEqual(response.status_code, 405) + + def test_unsubscribe_invalid_token(self): + def test_invalid_token(token): + request = self.request_factory.get("dummy") + self.assertRaises(Http404, unsubscribe, request, token) + + # Invalid base64 encoding + test_invalid_token("Non-ASCII\xff") + test_invalid_token("ZOMG INVALID BASE64 CHARS!!!") + test_invalid_token(self.tokens[self.user][:-1]) + + # Token of wrong length + test_invalid_token(self.tokens[self.user][:-4]) + + # Invalid padding (ends in 0 byte) + # Encrypted value: "testuser" + "\x00" * 8 + test_invalid_token("yhrNEjt48uMRZc3U3uR4vA==") + + # Invalid padding (ends in byte > 16) + # Encrypted value: "testusertestuser" + test_invalid_token("LqItcaGOQXK0mglIElnMng==") + + # Invalid padding (entire string is padding) + # Encrypted value: "\x10" * 16 + test_invalid_token("1EbDwcMSaVwtUVrCBj3Ajw==") + + # Nonexistent user + # Encrypted value: "nonexistentuser\x01" + test_invalid_token("KnJTFMYitSOem5Sw2LuYBg==") + + def test_unsubscribe_success(self): + self.create_prefs() + + def test_user(user): + request = self.request_factory.get("dummy") + request.user = AnonymousUser() + response = unsubscribe(request, self.tokens[user]) + self.assertEqual(response.status_code, 200) + self.assertNotPrefExists(user) + + test_user(self.user) + test_user(self.aes_block_length_user) + + def test_unsubscribe_twice(self): + self.create_prefs() + request = self.request_factory.get("dummy") + request.user = AnonymousUser() + unsubscribe(request, self.tokens[self.user]) + response = unsubscribe(request, self.tokens[self.user]) + self.assertEqual(response.status_code, 200) + self.assertNotPrefExists(self.user) diff --git a/lms/djangoapps/notification_prefs/views.py b/lms/djangoapps/notification_prefs/views.py new file mode 100644 index 000000000000..834993c37bdf --- /dev/null +++ b/lms/djangoapps/notification_prefs/views.py @@ -0,0 +1,119 @@ +from base64 import urlsafe_b64encode, urlsafe_b64decode +from Crypto.Cipher import AES +from django.conf import settings +from django.contrib.auth.models import User +from django.http import Http404, HttpResponse, HttpResponseForbidden, HttpResponseNotAllowed +from hashlib import sha256 +from mitxmako.shortcuts import render_to_response +from notification_prefs import NOTIFICATION_PREF_KEY +from user_api.models import UserPreference + + +class UsernameCodec(object): + AES_BLOCK_LEN = 16 + + def __init__(self): + hash_ = sha256() + hash_.update(settings.SECRET_KEY) + self.cipher = AES.new(hash_.digest()) + + def _add_padding(self, str): + """Return str with PKCS#7 padding added""" + padding_len = self.AES_BLOCK_LEN - (len(str) % self.AES_BLOCK_LEN) + return str + (padding_len * chr(padding_len)) + + def _remove_padding(self, str): + """Return str with PKCS#7 padding trimmed""" + num_pad_bytes = ord(str[-1]) + if num_pad_bytes < 1 or num_pad_bytes > self.AES_BLOCK_LEN or num_pad_bytes >= len(str): + return None + return str[:-num_pad_bytes] + + def encode(self, username): + return urlsafe_b64encode(self.cipher.encrypt(self._add_padding(username))) + + def decode(self, encoded): + return self._remove_padding(self.cipher.decrypt(urlsafe_b64decode(encoded))) + + +def _validate_ajax(request): + """ + Ensure that `request` is valid + + If the request is invalid, an appropriate response is returned. Otherwise, + None is returned. + """ + if request.method != "POST": + return HttpResponseNotAllowed(["POST"]) + + if not request.user.is_authenticated(): + return HttpResponseForbidden() + + +def ajax_enable(request): + """ + A view that enables notifications for the authenticated user + + This view should be invoked by an AJAX POST call. It returns status 204 + (no content) or an error. If notifications were already enabled for this + user, this has no effect. Otherwise, a preference is created with the + unsubscribe token (an ecnryption of the username) as the value.unsernam + """ + validation_response = _validate_ajax(request) + if validation_response is not None: + return validation_response + + UserPreference.objects.get_or_create( + user=request.user, + key=NOTIFICATION_PREF_KEY, + defaults={ + "value": UsernameCodec().encode(request.user.username) + } + ) + + return HttpResponse(status=204) + + +def ajax_disable(request): + """ + A view that disables notifications for the authenticated user + + This view should be invoked by an AJAX POST call. It returns status 204 + (no content) or an error. + """ + validation_response = _validate_ajax(request) + if validation_response is not None: + return validation_response + + UserPreference.objects.filter( + user=request.user, + key=NOTIFICATION_PREF_KEY + ).delete() + + return HttpResponse(status=204) + + +def unsubscribe(request, token): + """ + A view that disables notifications for a user who may not be authenticated + + This view is meant to be the target of an unsubscribe link. The request + must be a GET, and the `token` parameter must decrypt to a valid username. + + A 405 will be returned if the request method is not GET. A 404 will be + returned if the token parameter is missing or if the given token does not + decrypt to a valid username. On success, the response will contain a page + indicating success. + """ + if request.method != "GET": + return HttpResponseNotAllowed(["GET"]) + + try: + username = UsernameCodec().decode(token.encode()) + user = User.objects.get(username=username) + except Exception as e: + raise Http404(e.message) + + UserPreference.objects.filter(user=user, key=NOTIFICATION_PREF_KEY).delete() + + return render_to_response("unsubscribe.html", {}) diff --git a/lms/envs/common.py b/lms/envs/common.py index ddbbf0d9ad6c..c71ff9f8cb8c 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -747,6 +747,9 @@ # User API 'rest_framework', 'user_api', + + # Notification preferences setting + 'notification_prefs', ) ######################### MARKETING SITE ############################### diff --git a/lms/static/sass/application.scss.mako b/lms/static/sass/application.scss.mako index 823587bf0487..d30c0c16a198 100644 --- a/lms/static/sass/application.scss.mako +++ b/lms/static/sass/application.scss.mako @@ -30,6 +30,7 @@ @import 'shared/course_filter'; @import 'shared/modal'; @import 'shared/activation_messages'; +@import 'shared/unsubscribe'; @import 'multicourse/home'; @import 'multicourse/dashboard'; diff --git a/lms/static/sass/shared/_unsubscribe.scss b/lms/static/sass/shared/_unsubscribe.scss new file mode 100644 index 000000000000..4d870704a2bb --- /dev/null +++ b/lms/static/sass/shared/_unsubscribe.scss @@ -0,0 +1,23 @@ +.container.unsubscribe { + padding: 60px 0px 120px; + + h1 { + margin-bottom: 20px; + padding: 10px; + @extend .success-message-colors; + } + + h1 + hr { + margin-bottom: 30px; + } + + .message { + background: rgb(252,252,252); + border: 1px solid rgb(200,200,200); + box-shadow: 0 3px 20px 0 rgba(0,0,0, 0.2); + border-radius: 4px; + margin: 0 auto; + padding: 40px; + width: flex-grid(6); + } +} diff --git a/lms/templates/unsubscribe.html b/lms/templates/unsubscribe.html new file mode 100644 index 000000000000..6f8c042d76cf --- /dev/null +++ b/lms/templates/unsubscribe.html @@ -0,0 +1,17 @@ +<%! from django.core.urlresolvers import reverse %> +<%inherit file="main.html" /> + +<%namespace name='static' file='static_content.html'/> + +
+ +
+

Unsubscribe Successful!

+
+ +

+ You will no longer receive notification emails from edX. + Click here to return to your dashboard. +

+
+
diff --git a/lms/urls.py b/lms/urls.py index 14fb0a140bc5..81b91ccea931 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -339,7 +339,10 @@ url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/news$', 'courseware.views.news', name="news"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/discussion/', - include('django_comment_client.urls')) + include('django_comment_client.urls')), + url(r'^notification_prefs/enable/', 'notification_prefs.views.ajax_enable'), + url(r'^notification_prefs/disable/', 'notification_prefs.views.ajax_disable'), + url(r'^notification_prefs/unsubscribe/(?P[a-zA-Z0-9-_=]+)/', 'notification_prefs.views.unsubscribe'), ) urlpatterns += ( # This MUST be the last view in the courseware--it's a catch-all for custom tabs. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 4c4ab5a29f4f..dd9d8b5a3ea7 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -37,6 +37,7 @@ path.py==3.0.1 Pillow==1.7.8 pip>=1.3 polib==1.0.3 +pycrypto>=2.6 pygments==1.5 pygraphviz==1.1 pymongo==2.4.1 From 1281027424c8b556d75dcc3db3d1a4ed4945b0f8 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 17 Jul 2013 14:28:18 -0400 Subject: [PATCH 3/3] Add acceptance test for one-click unsubscribe page --- common/djangoapps/terrain/steps.py | 7 ++++++ .../features/unsubscribe.feature | 22 ++++++++++++++++ .../features/unsubscribe.py | 25 +++++++++++++++++++ lms/envs/acceptance.py | 4 +++ 4 files changed, 58 insertions(+) create mode 100644 lms/djangoapps/notification_prefs/features/unsubscribe.feature create mode 100644 lms/djangoapps/notification_prefs/features/unsubscribe.py diff --git a/common/djangoapps/terrain/steps.py b/common/djangoapps/terrain/steps.py index bd0b448e117c..cfa8de15b7d6 100644 --- a/common/djangoapps/terrain/steps.py +++ b/common/djangoapps/terrain/steps.py @@ -129,6 +129,13 @@ def should_have_link_with_id_and_text(step, link_id, text): assert_equals(link.text, text) +@step(r'should see a link to "([^"]*)" with the text "([^"]*)"$') +def should_have_link_with_path_and_text(step, path, text): + link = world.browser.find_link_by_text(text) + assert len(link) > 0 + assert_equals(link.first["href"], django_url(path)) + + @step(r'should( not)? see "(.*)" (?:somewhere|anywhere) (?:in|on) (?:the|this) page') def should_see_in_the_page(step, doesnt_appear, text): if doesnt_appear: diff --git a/lms/djangoapps/notification_prefs/features/unsubscribe.feature b/lms/djangoapps/notification_prefs/features/unsubscribe.feature new file mode 100644 index 000000000000..9649fc1cc5b9 --- /dev/null +++ b/lms/djangoapps/notification_prefs/features/unsubscribe.feature @@ -0,0 +1,22 @@ +Feature: One-click unsubscribe + As a user with notifications enabled + I want to be able to unsubscribe from notifications + + Scenario: Unsubscribe when not logged in + Given I am an edX user + And I am not logged in + And I have notifications enabled + When I access my unsubscribe url + Then my notifications should be disabled + And I should see "Unsubscribe Successful!" somewhere on the page + And I should see "Click here to return to your dashboard" somewhere on the page + And I should see a link to "/dashboard" with the text "here" + + Scenario: Unsubscribe when logged in + Given I am a logged in user + And I have notifications enabled + When I access my unsubscribe url + Then my notifications should be disabled + And I should see "Unsubscribe Successful!" somewhere on the page + And I should see "Click here to return to your dashboard" somewhere on the page + And I should see a link to "/dashboard" with the text "here" diff --git a/lms/djangoapps/notification_prefs/features/unsubscribe.py b/lms/djangoapps/notification_prefs/features/unsubscribe.py new file mode 100644 index 000000000000..e4d2147e4da2 --- /dev/null +++ b/lms/djangoapps/notification_prefs/features/unsubscribe.py @@ -0,0 +1,25 @@ +from django.contrib.auth.models import User +from lettuce import step, world +from notification_prefs import NOTIFICATION_PREF_KEY +from user_api.models import UserPreference + + +USERNAME = "robot" +UNSUB_TOKEN = "av9E-14sAP1bVBRCPbrTHQ==" + + +@step(u"I have notifications enabled") +def enable_notifications(step): + user = User.objects.get(username=USERNAME) + UserPreference.objects.create(user=user, key=NOTIFICATION_PREF_KEY, value=UNSUB_TOKEN) + + +@step(u"I access my unsubscribe url") +def access_unsubscribe_url(step): + world.visit("/notification_prefs/unsubscribe/{0}/".format(UNSUB_TOKEN)) + + +@step(u"my notifications should be disabled") +def notifications_should_be_disabled(step): + user = User.objects.get(username=USERNAME) + assert not UserPreference.objects.filter(user=user, key=NOTIFICATION_PREF_KEY).exists() diff --git a/lms/envs/acceptance.py b/lms/envs/acceptance.py index 087c1ca85ca9..5f8fd99c16ee 100644 --- a/lms/envs/acceptance.py +++ b/lms/envs/acceptance.py @@ -79,6 +79,10 @@ def seed(): # acceptance tests. This makes them faster and more reliable MITX_FEATURES['STUB_VIDEO_FOR_TESTING'] = True +# Forums are disabled in test.py to speed up unit tests, but we do not have +# per-test control for acceptance tests +MITX_FEATURES['ENABLE_DISCUSSION_SERVICE'] = True + # Include the lettuce app for acceptance testing, including the 'harvest' django-admin command INSTALLED_APPS += ('lettuce.django',) LETTUCE_APPS = ('courseware',)