Skip to content

Commit

Permalink
Merge pull request #39 from Harvard-University-iCommons/feature/djang…
Browse files Browse the repository at this point in the history
…o1_10_monkeypatching_fix

Feature/django1 10 monkeypatching fix
  • Loading branch information
bermudezjd authored Mar 16, 2017
2 parents 5efd857 + be632fe commit 1d7bd62
Show file tree
Hide file tree
Showing 10 changed files with 186 additions and 24 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ dist
*.egg-info
*.egg
.idea
.tox/
.eggs/
34 changes: 26 additions & 8 deletions django_auth_lti/patch_reverse.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
Monkey-patch django.core.urlresolvers.reverse to add resource_link_id to all URLs
Monkey-patch django's reverse function to add resource_link_id to all URLs.
"""
from urlparse import urlparse, urlunparse, parse_qs
from urllib import urlencode
Expand All @@ -8,43 +8,61 @@

from .thread_local import get_current_request


django_reverse = None


def reverse(*args, **kwargs):
"""
Call django's reverse function and append the current resource_link_id as a query parameter
Call django's reverse function and append the current resource_link_id as a
query parameter
:param kwargs['exclude_resource_link_id']: Do not add the resource link id as a query parameter
:param kwargs['exclude_resource_link_id']: Do not add the resource link id
as a query parameter
:returns Django named url
"""
request = get_current_request()

# Check for custom exclude_resource_link_id kwarg and remove it before passing kwargs to django reverse
# Check for custom exclude_resource_link_id kwarg and remove it before
# passing kwargs to django reverse
exclude_resource_link_id = kwargs.pop('exclude_resource_link_id', False)

url = django_reverse(*args, **kwargs)
if not exclude_resource_link_id:
# Append resource_link_id query param if exclude_resource_link_id kwarg was not passed or is False
# Append resource_link_id query param if exclude_resource_link_id kwarg
# was not passed or is False
parsed = urlparse(url)
query = parse_qs(parsed.query)
if 'resource_link_id' not in query.keys():
query['resource_link_id'] = request.LTI.get('resource_link_id')
url = urlunparse(
(parsed.scheme, parsed.netloc, parsed.path, parsed.params, urlencode(query), parsed.fragment)
(parsed.scheme, parsed.netloc, parsed.path, parsed.params,
urlencode(query), parsed.fragment)
)
return url


def patch_reverse():
"""
Monkey-patches the django.core.urlresolvers.reverse function. Will not patch twice.
Monkey-patches the reverse function. Will not patch twice.
"""
global django_reverse
if urlresolvers.reverse is not reverse:
django_reverse = urlresolvers.reverse
urlresolvers.reverse = reverse

# Django 1.10 moves url helper functions like `reverse` into a new urls
# module, so we need to patch it as well. In addition, the
# django.shortcuts module now includes `reverse` directly, and the
# module appears to be loaded before middleware so we need to
# retroactively patch that `reverse` reference as well.
try:
import django
from django import urls

urls.reverse = reverse
django.shortcuts.reverse = reverse
except ImportError:
pass


patch_reverse()
4 changes: 2 additions & 2 deletions django_auth_lti/tests/test_lti_auth_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
from mock import patch
from django.test import RequestFactory
from django.contrib.auth import models
from django_auth_lti.middleware import LTIAuthMiddleware
from django_auth_lti.middleware_patched import MultiLTILaunchAuthMiddleware


@patch('django_auth_lti.middleware.logger')
class TestLTIAuthMiddleware(unittest.TestCase):
longMessage = True

def setUp(self):
self.mw = LTIAuthMiddleware()
self.mw = MultiLTILaunchAuthMiddleware()

def build_lti_launch_request(self, post_data):
"""
Expand Down
24 changes: 12 additions & 12 deletions run_tests.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
#!/usr/bin/env python
import os
import sys

import django
from django.conf import settings


def runtests():
settings.configure(
# App-specific setttings here
)
# settings must be configured for this import to work
from django.test.runner import DiscoverRunner
DiscoverRunner(interactive=False, failfast=False).run_tests(['django_auth_lti'])

if __name__ == '__main__':
runtests()
from django.test.utils import get_runner

if __name__ == "__main__":
os.environ['DJANGO_SETTINGS_MODULE'] = 'tests.test_settings'
django.setup()
TestRunner = get_runner(settings)
test_runner = TestRunner()
failures = test_runner.run_tests(["tests"])
sys.exit(bool(failures))
4 changes: 2 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

setup(
name='django-auth-lti',
version='1.2.6',
version='1.2.7',
packages=['django_auth_lti'],
include_package_data=True,
license='TBD License', # example license
Expand All @@ -33,7 +33,7 @@
"Django>=1.6",
"ims-lti-py==0.6",
"django-braces==1.3.1",
"oauth2==1.9.0.post1", # to catch errors uncaught by ims-lti-py
"oauth2==1.9.0.post1", # to catch errors uncaught by ims-lti-py
],
tests_require=[
'mock',
Expand Down
Empty file added tests/__init__.py
Empty file.
67 changes: 67 additions & 0 deletions tests/test_lti_auth_middleware.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import unittest
import mock
from mock import patch
from django.test import RequestFactory
from django.contrib.auth import models
from django_auth_lti.middleware import LTIAuthMiddleware


@patch('django_auth_lti.middleware.logger')
class TestLTIAuthMiddleware(unittest.TestCase):
longMessage = True

def setUp(self):
self.mw = LTIAuthMiddleware()

def build_lti_launch_request(self, post_data):
"""
Utility method that builds a fake lti launch request with custom data.
"""
# Add message type to post data
post_data.update(lti_message_type='basic-lti-launch-request')
# Add resource_link_id to post data
post_data.update(resource_link_id='d202fb112a14f27107149ed874bf630aa8e029a5')

request = RequestFactory().post('/fake/lti/launch', post_data)
request.user = mock.Mock(name='User', spec=models.User)
request.session = {}
return request

@patch('django_auth_lti.middleware.auth')
def test_roles_merged_with_custom_roles(self, mock_auth, mock_logger):
"""
Assert that 'roles' list in session contains merged set of roles when custom role key is
defined and values have been passed in.
"""
request = self.build_lti_launch_request({
'roles': 'RoleOne,RoleTwo',
'test_custom_role_key': 'My,Custom,Roles',
})
with patch('django_auth_lti.middleware.settings', LTI_CUSTOM_ROLE_KEY='test_custom_role_key'):
self.mw.process_request(request)
self.assertEqual(request.LTI.get('roles'), ['RoleOne', 'RoleTwo', 'My', 'Custom', 'Roles'])

@patch('django_auth_lti.middleware.auth')
def test_roles_merge_with_empty_custom_roles(self, mock_auth, mock_logger):
"""
Assert that 'roles' list in session contains original set when custom role key is defined with empty data.
"""
request = self.build_lti_launch_request({
'roles': 'RoleOne,RoleTwo',
'test_custom_role_key': '',
})
with patch('django_auth_lti.middleware.settings', LTI_CUSTOM_ROLE_KEY='test_custom_role_key'):
self.mw.process_request(request)
self.assertEqual(request.LTI.get('roles'), ['RoleOne', 'RoleTwo'])

@patch('django_auth_lti.middleware.auth')
def test_roles_not_merged_with_no_role_key(self, mock_auth, mock_logger):
"""
Assert that 'roles' list in session contains original set when no custom role key is defined.
"""
request = self.build_lti_launch_request({
'roles': 'RoleOne,RoleTwo',
'test_custom_role_key': 'My,Custom,Roles',
})
self.mw.process_request(request)
self.assertEqual(request.LTI.get('roles'), ['RoleOne', 'RoleTwo'])
29 changes: 29 additions & 0 deletions tests/test_settings.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import os

# Build paths inside the project like this: os.path.join(BASE_DIR, ...)
BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))

SECRET_KEY = 'fake-key'

INSTALLED_APPS = [
'django.contrib.auth',
'django.contrib.contenttypes',
'django.contrib.sessions',
'django.contrib.messages',
'django.contrib.staticfiles',
'django_auth_lti',
]

TEMPLATES = [
{
'BACKEND': 'django.template.backends.django.DjangoTemplates',
'APP_DIRS': True,
},
]

DATABASES = {
'default': {
'ENGINE': 'django.db.backends.sqlite3',
'NAME': os.path.join(BASE_DIR, 'db.sqlite3'),
}
}
36 changes: 36 additions & 0 deletions tests/test_verification.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
from unittest import TestCase
from mock import MagicMock
from django_auth_lti.verification import is_allowed
from django.core.exceptions import ImproperlyConfigured, PermissionDenied

class TestVerification(TestCase):

def test_is_allowed_success(self):
request = MagicMock(LTI={"roles": ["admin"]})
allowed_roles = ["admin", "student"]
user_is_allowed = is_allowed(request, allowed_roles, False)
self.assertTrue(user_is_allowed)

def test_is_allowed_success_one_role(self):
request = MagicMock(LTI={"roles": ["admin"]})
allowed_roles = "admin"
user_is_allowed = is_allowed(request, allowed_roles, False)
self.assertTrue(user_is_allowed)

def test_is_allowed_failure(self):
request = MagicMock(LTI={"roles":[]})
allowed_roles = ["admin", "student"]
user_is_allowed = is_allowed(request, allowed_roles, False)
self.assertFalse(user_is_allowed)

def test_is_allowed_failure_one_role(self):
request = MagicMock(LTI={"roles":[]})
allowed_roles = "admin"
user_is_allowed = is_allowed(request, allowed_roles, False)
self.assertFalse(user_is_allowed)

def test_is_allowed_exception(self):
request = MagicMock(LTI={"roles":["TF"]})
allowed_roles = ["admin", "student"]
self.assertRaises(PermissionDenied, is_allowed,
request, allowed_roles, True)
10 changes: 10 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[tox]
envlist =
py27-django{18,19,110}
[testenv]
deps =
py27: mock
django18: Django >= 1.8, < 1.9
django19: Django >= 1.9, < 1.10
django110: Django >= 1.10, < 1.11
commands = python run_tests.py

0 comments on commit 1d7bd62

Please sign in to comment.