Skip to content

Commit

Permalink
Fix CSRF exempts in the LTI Tool v1.3
Browse files Browse the repository at this point in the history
In Django's class-based views, function decorators must be used with
`method_decorator`.
https://docs.djangoproject.com/en/3.2/topics/class-based-views/intro/#decorating-the-class

Move the `csrf_exempt` for the class `LtiLaunchView` from urls.py to
the view class definition so that it is more consistent with the other
LTI Tool views. The same view also requires
the `xframe_options_exempt` decorator.

Class `LtiSessionMixin` does not need `csrf_exempt`.
Most of its child classes are able to normally check CSRF tokens
because the HTML forms are first rendered in A+.

Class `LtiExerciseView` needs both `csrf_exempt` and
`xframe_options_exempt`.

* `csrf_exempt` is needed since it is used in class `ExerciseView`
  and the exercise forms loaded from assessment services do not include
  any CSRF tokens.
* `xframe_options_exempt` is needed since LTI Tools may be embedded
  in iframes in the LTI Platform.

Fixes #1099
  • Loading branch information
markkuriekkinen committed Apr 14, 2023
1 parent 1a4b747 commit 7e4bd40
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
3 changes: 1 addition & 2 deletions lti_tool/urls.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from django.conf.urls import url
from django.views.decorators.csrf import csrf_exempt

from . import views
from exercise.views import SubmissionPollView
Expand Down Expand Up @@ -37,7 +36,7 @@
views.LtiSelectExerciseView.as_view(),
name="lti-select-exercise"),
url(LTI_PREFIX + r'launch/$',
csrf_exempt(views.LtiLaunchView.as_view()),
views.LtiLaunchView.as_view(),
name="lti-launch"),
url(LTI_SUBMISSION_URL_PREFIX + r'$',
views.LtiSubmissionView.as_view(),
Expand Down
21 changes: 16 additions & 5 deletions lti_tool/views.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import logging

from django.http import HttpResponse
from django.views.decorators.clickjacking import xframe_options_exempt
from django.views.decorators.csrf import csrf_exempt
from django.shortcuts import redirect, get_object_or_404
from django.contrib.auth.models import User
from django.contrib.auth import login
from django.urls import reverse
from django.utils.decorators import method_decorator
from pylti1p3.contrib.django import DjangoOIDCLogin, DjangoMessageLaunch
from pylti1p3.deep_link_resource import DeepLinkResource
from pylti1p3.lineitem import LineItem
Expand All @@ -21,9 +23,10 @@
from authorization.permissions import ACCESS
from .utils import get_tool_conf, get_launch_data_storage, get_launch_url


logger = logging.getLogger('aplus.lti_tool')

# CSRF exempts due to difficulty in proper implementation. To re-review later.

@csrf_exempt
@xframe_options_exempt
def lti_login(request):
Expand All @@ -35,12 +38,12 @@ def lti_login(request):
return oidc_login.enable_check_cookies().redirect(target_link_uri)


@method_decorator(csrf_exempt, name='dispatch')
@method_decorator(xframe_options_exempt, name='dispatch')
class LtiLaunchView(BaseRedirectView):

access_mode = ACCESS.ANONYMOUS

# this has been CSRF exempted in the urls file
@xframe_options_exempt
def post(self, request, *args, **kwargs):
tool_conf = get_tool_conf()
launch_data_storage = get_launch_data_storage()
Expand Down Expand Up @@ -109,8 +112,7 @@ def post(self, request, *args, **kwargs):

class LtiSessionMixin(BaseMixin):

@csrf_exempt
@xframe_options_exempt
@method_decorator(xframe_options_exempt)
def dispatch(self, *args, **kwargs):
return super().dispatch(*args, **kwargs)

Expand Down Expand Up @@ -170,6 +172,15 @@ class LtiExerciseView(LtiSessionMixin, ExerciseView):
submission_url_name = "lti-submission"
exercise_url_name = "lti-exercise"

# The dispatch method needs to be overridden here so that we can apply
# different method decorators specifically in this class.
# csrf_exempt is needed since it is used in ExerciseView and
# the exercise forms do not include any csrf token.
@method_decorator(csrf_exempt)
@method_decorator(xframe_options_exempt)
def dispatch(self, request, *args, **kwargs):
return super().dispatch(request, *args, **kwargs)

def post(self, request, *args, **kwargs):
return super().post(request, *args, **kwargs, redirect_view='lti-submission')

Expand Down

0 comments on commit 7e4bd40

Please sign in to comment.