From 9b9250b5c332d5695f99a7af948b6549a7c2e7af Mon Sep 17 00:00:00 2001 From: Nicholas Serra Date: Tue, 4 Apr 2017 13:45:49 -0400 Subject: [PATCH 01/10] Clear up user roles in tests --- tos/tests/test_views.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tos/tests/test_views.py b/tos/tests/test_views.py index bb307cb..c829d62 100644 --- a/tos/tests/test_views.py +++ b/tos/tests/test_views.py @@ -9,7 +9,10 @@ from tos.models import TermsOfService, UserAgreement, has_user_agreed_latest_tos class TestViews(TestCase): def setUp(self): + # User that has agreed to TOS self.user1 = get_runtime_user_model().objects.create_user('user1', 'user1@example.com', 'user1pass') + + # User that has not yet agreed to TOS self.user2 = get_runtime_user_model().objects.create_user('user2', 'user2@example.com', 'user2pass') self.tos1 = TermsOfService.objects.create( From e07fb710a8d8be92d5d97d786c7b55358a123286 Mon Sep 17 00:00:00 2001 From: Nicholas Serra Date: Tue, 4 Apr 2017 13:46:03 -0400 Subject: [PATCH 02/10] Generic index view for testing --- tos/tests/templates/index.html | 0 tos/tests/test_urls.py | 3 +++ 2 files changed, 3 insertions(+) create mode 100644 tos/tests/templates/index.html diff --git a/tos/tests/templates/index.html b/tos/tests/templates/index.html new file mode 100644 index 0000000..e69de29 diff --git a/tos/tests/test_urls.py b/tos/tests/test_urls.py index a931113..6c91a25 100644 --- a/tos/tests/test_urls.py +++ b/tos/tests/test_urls.py @@ -1,9 +1,12 @@ from django.conf.urls import include, url +from django.views.generic import TemplateView from tos.compat import patterns from tos import views urlpatterns = patterns('', + url(r'^$', TemplateView.as_view(template_name='index.html'), name='index'), + url(r'^login/$', views.login, {}, 'login'), url(r'^tos/', include('tos.urls')), ) From 74853bee8af95ef3d98f19efc9a85576154a01d2 Mon Sep 17 00:00:00 2001 From: Nicholas Serra Date: Tue, 4 Apr 2017 13:46:18 -0400 Subject: [PATCH 03/10] Cache setting for testing middleware --- runtests.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/runtests.py b/runtests.py index 6101052..be8937e 100755 --- a/runtests.py +++ b/runtests.py @@ -47,7 +47,14 @@ if not settings.configured: ], ROOT_URLCONF='tos.tests.test_urls', LOGIN_URL='/login/', - SITE_ID='1' + SITE_ID='1', + CACHES = { + 'tos': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'unique-snowflake', + } + }, + TOS_CACHE_NAME = 'tos' ) From fe5f5b47900796b64aa115d4d4b62b2d94deb9d4 Mon Sep 17 00:00:00 2001 From: Nicholas Serra Date: Tue, 4 Apr 2017 13:46:43 -0400 Subject: [PATCH 04/10] Set session vars in middleware --- tos/middleware.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tos/middleware.py b/tos/middleware.py index 29cab0a..e26ed8e 100644 --- a/tos/middleware.py +++ b/tos/middleware.py @@ -38,7 +38,8 @@ class UserAgreementMiddleware(deprecation.MiddlewareMixin if DJANGO_VERSION >= ( # for the user object. # NOTE: We use the user ID because it's not user-settable and it won't # ever change (usernames and email addresses can change) - user_id = request.session.get(session_key) + user_id = int(request.session['_auth_user_id']) + user_auth_backend = request.session['_auth_user_backend'] # Get the cache prefix key_version = cache.get('django:tos:key_version') @@ -62,6 +63,11 @@ class UserAgreementMiddleware(deprecation.MiddlewareMixin if DJANGO_VERSION >= ( cache.set('django:tos:agreed:{}'.format(user_id), user_agreed, version=key_version) if not user_agreed: + # Confirm view uses these session keys. Non-middleware flow sets them in login view, + # so we need to set them here. + request.session['tos_user'] = user_id + request.session['tos_backend'] = user_auth_backend + response = HttpResponseRedirect(tos_check_url) add_never_cache_headers(response) return response From b04d40010a1381875d373387c7d27d5274252ae6 Mon Sep 17 00:00:00 2001 From: Nicholas Serra Date: Tue, 4 Apr 2017 13:47:12 -0400 Subject: [PATCH 05/10] Some tests around middleware option --- tos/tests/test_middleware.py | 61 ++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 tos/tests/test_middleware.py diff --git a/tos/tests/test_middleware.py b/tos/tests/test_middleware.py new file mode 100644 index 0000000..8bfb9bc --- /dev/null +++ b/tos/tests/test_middleware.py @@ -0,0 +1,61 @@ +from django.conf import settings +from django.core.urlresolvers import reverse +from django.test import TestCase, override_settings + +from tos.compat import get_runtime_user_model +from tos.models import TermsOfService, UserAgreement + + +@override_settings( + MIDDLEWARE_CLASSES=settings.MIDDLEWARE_CLASSES + [ + 'tos.middleware.UserAgreementMiddleware', + ] +) +class TestMiddleware(TestCase): + + def setUp(self): + # User that has agreed to TOS + self.user1 = get_runtime_user_model().objects.create_user('user1', 'user1@example.com', 'user1pass') + + # User that has not yet agreed to TOS + self.user2 = get_runtime_user_model().objects.create_user('user2', 'user2@example.com', 'user2pass') + + self.tos1 = TermsOfService.objects.create( + content="first edition of the terms of service", + active=True + ) + self.tos2 = TermsOfService.objects.create( + content="second edition of the terms of service", + active=False + ) + self.login_url = getattr(settings, 'LOGIN_URL', '/login/') + + UserAgreement.objects.create( + terms_of_service=self.tos1, + user=self.user1 + ) + + def test_middleware_redirects(self): + """ + User that hasn't accepted TOS should be redirected to confirm. Also make sure + confirm works. + """ + self.client.login(username='user2', password='user2pass') + response = self.client.get(reverse('index')) + self.assertRedirects(response, reverse('tos_check_tos')) + + # Make sure confirm works after middleware redirect. + response = self.client.post(reverse('tos_check_tos'), {'accept': 'accept'}) + + # Confirm redirects. + self.assertEqual(response.status_code, 302) + + def test_middleware_doesnt_redirect(self): + """User that has accepted TOS should get 200.""" + self.client.login(username='user1', password='user1pass') + response = self.client.get(reverse('index')) + self.assertEqual(response.status_code, 200) + + def test_anonymous_user_200(self): + response = self.client.get(reverse('index')) + self.assertEqual(response.status_code, 200) From 9560e27f91b4edcdf61319ddbda4858199dbcdc4 Mon Sep 17 00:00:00 2001 From: Nicholas Serra Date: Tue, 4 Apr 2017 13:49:19 -0400 Subject: [PATCH 06/10] We don't need cache location --- runtests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/runtests.py b/runtests.py index be8937e..996e233 100755 --- a/runtests.py +++ b/runtests.py @@ -51,7 +51,6 @@ if not settings.configured: CACHES = { 'tos': { 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', - 'LOCATION': 'unique-snowflake', } }, TOS_CACHE_NAME = 'tos' From 241db08385bd9bf028c8b44a9ffa386f0a8bd9e6 Mon Sep 17 00:00:00 2001 From: Nicholas Serra Date: Tue, 4 Apr 2017 14:00:19 -0400 Subject: [PATCH 07/10] Add default dummy cache --- runtests.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/runtests.py b/runtests.py index 996e233..3e56770 100755 --- a/runtests.py +++ b/runtests.py @@ -49,6 +49,9 @@ if not settings.configured: LOGIN_URL='/login/', SITE_ID='1', CACHES = { + 'default': { + 'BACKEND': 'django.core.cache.backends.dummy.DummyCache', + }, 'tos': { 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', } From c7fa58ffe363eb396add4de52d7fc832cbeab76a Mon Sep 17 00:00:00 2001 From: Nicholas Serra Date: Tue, 4 Apr 2017 14:21:50 -0400 Subject: [PATCH 08/10] Better import for override_settings --- tos/tests/test_middleware.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tos/tests/test_middleware.py b/tos/tests/test_middleware.py index 8bfb9bc..3724556 100644 --- a/tos/tests/test_middleware.py +++ b/tos/tests/test_middleware.py @@ -1,6 +1,7 @@ from django.conf import settings from django.core.urlresolvers import reverse -from django.test import TestCase, override_settings +from django.test import TestCase +from django.test.utils import override_settings from tos.compat import get_runtime_user_model from tos.models import TermsOfService, UserAgreement From b826c969ad07c318fb9057202663631b9d29e068 Mon Sep 17 00:00:00 2001 From: Nicholas Serra Date: Tue, 4 Apr 2017 14:22:09 -0400 Subject: [PATCH 09/10] get_cache compat method --- tos/compat.py | 9 +++++++++ tos/middleware.py | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/tos/compat.py b/tos/compat.py index fccea30..bfb690e 100644 --- a/tos/compat.py +++ b/tos/compat.py @@ -41,6 +41,15 @@ def get_library(): return Library +def get_cache(cache_name): + if django.VERSION >= (1, 7): + from django.core.cache import caches + return caches[cache_name] + else: + from django.core.cache import get_cache + return get_cache(cache_name) + + if django.VERSION < (1, 5): from django.templatetags.future import url else: diff --git a/tos/middleware.py b/tos/middleware.py index e26ed8e..84935a8 100644 --- a/tos/middleware.py +++ b/tos/middleware.py @@ -1,15 +1,15 @@ from django import VERSION as DJANGO_VERSION from django.conf import settings from django.contrib.auth import SESSION_KEY as session_key -from django.core.cache import caches from django.core.urlresolvers import reverse from django.http import HttpResponseRedirect from django.utils import deprecation from django.utils.cache import add_never_cache_headers +from .compat import get_cache from .models import UserAgreement -cache = caches[getattr(settings, 'TOS_CACHE_NAME', 'default')] +cache = get_cache(getattr(settings, 'TOS_CACHE_NAME', 'default')) tos_check_url = reverse('tos_check_tos') From cfc66156902ec3987ca46feb9e5235bdcb76518e Mon Sep 17 00:00:00 2001 From: Nicholas Serra Date: Tue, 4 Apr 2017 14:51:23 -0400 Subject: [PATCH 10/10] 2.6 format compat --- tos/middleware.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tos/middleware.py b/tos/middleware.py index 84935a8..39abb89 100644 --- a/tos/middleware.py +++ b/tos/middleware.py @@ -46,11 +46,11 @@ class UserAgreementMiddleware(deprecation.MiddlewareMixin if DJANGO_VERSION >= ( # Skip if the user is allowed to skip - for instance, if the user is an # admin or a staff member - if cache.get('django:tos:skip_tos_check:{}'.format(str(user_id)), False, version=key_version): + if cache.get('django:tos:skip_tos_check:{0}'.format(str(user_id)), False, version=key_version): return None # Ping the cache for the user agreement - user_agreed = cache.get('django:tos:agreed:{}'.format(str(user_id)), None, version=key_version) + user_agreed = cache.get('django:tos:agreed:{0}'.format(str(user_id)), None, version=key_version) # If the cache is missing this user if user_agreed is None: @@ -60,7 +60,7 @@ class UserAgreementMiddleware(deprecation.MiddlewareMixin if DJANGO_VERSION >= ( terms_of_service__active=True).exists() # Set the value in the cache - cache.set('django:tos:agreed:{}'.format(user_id), user_agreed, version=key_version) + cache.set('django:tos:agreed:{0}'.format(user_id), user_agreed, version=key_version) if not user_agreed: # Confirm view uses these session keys. Non-middleware flow sets them in login view,