Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add proctorio redirect url #227

Merged
merged 1 commit into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions edx_exams/apps/core/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
from django.contrib import admin
from django.contrib.auth.admin import UserAdmin
from django.utils.translation import gettext_lazy as _

from edx_exams.apps.lti.utils import get_lti_root
from lti_consumer.utils import get_lti_api_base

from .models import (
AssessmentControlResult,
Expand Down Expand Up @@ -61,7 +60,10 @@ class ExamAttemptAdmin(admin.ModelAdmin):

def change_view(self, request, object_id, form_url='', extra_context=None):
extra_context = extra_context or {}
extra_context['LTI_ROOT'] = get_lti_root()
# technically this should be be using get_lti_view_base() but the
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the customization on this django view can eventually be removed. It was only needed when we didn't have a UI to test with. I didn't want to add all that to this PR tho

# setting behind that isn't available in edx-exams and we know they
# are the same in this service.
extra_context['LTI_ROOT'] = get_lti_api_base()
return super().change_view(
request, object_id, form_url, extra_context=extra_context,
)
Expand Down
6 changes: 3 additions & 3 deletions edx_exams/apps/lti/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from lti_consumer.data import Lti1p3LaunchData, Lti1p3ProctoringLaunchData
from lti_consumer.lti_1p3.extensions.rest_framework.authentication import Lti1p3ApiAuthentication
from lti_consumer.models import LtiConfiguration, LtiProctoringConsumer
from lti_consumer.utils import get_lti_api_base

from edx_exams.apps.api.test_utils import ExamsAPITestCase, UserFactory
from edx_exams.apps.core.models import AssessmentControlResult, CourseStaffRole
Expand All @@ -24,7 +25,6 @@
ExamFactory,
ProctoringProviderFactory
)
from edx_exams.apps.lti.utils import get_lti_root

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -404,7 +404,7 @@ def test_start_proctoring_launch_data(self, mock_get_lti_launch_url):
self.client.get(self.url, **headers)

expected_proctoring_start_assessment_url = urljoin(
get_lti_root(),
get_lti_api_base(),
reverse('lti_consumer:lti_consumer.start_proctoring_assessment_endpoint')
)
expected_proctoring_launch_data = Lti1p3ProctoringLaunchData(
Expand All @@ -425,7 +425,7 @@ def test_start_proctoring_launch_data(self, mock_get_lti_launch_url):
context_id=self.course_id,
context_label=self.content_id,
custom_parameters={
'custom_url': 'https://test.learning:2000/exam',
'custom_url': 'test.exams:18740/browser_lock/',
},
)

Expand Down
8 changes: 0 additions & 8 deletions edx_exams/apps/lti/utils.py

This file was deleted.

17 changes: 9 additions & 8 deletions edx_exams/apps/lti/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import logging
from decimal import Decimal
from urllib.parse import urljoin
from urllib.parse import urljoin, urlparse

from django.conf import settings
from django.contrib.auth import login
Expand All @@ -20,6 +20,7 @@
from lti_consumer.lti_1p3.extensions.rest_framework.authentication import Lti1p3ApiAuthentication
from lti_consumer.lti_1p3.extensions.rest_framework.permissions import LtiProctoringAcsPermissions
from lti_consumer.models import LtiConfiguration
from lti_consumer.utils import get_lti_api_base
from rest_framework import status
from rest_framework.decorators import api_view, authentication_classes, permission_classes
from rest_framework.permissions import IsAuthenticated
Expand All @@ -36,7 +37,6 @@
from edx_exams.apps.core.exceptions import ExamIllegalStatusTransition
from edx_exams.apps.core.models import AssessmentControlResult, User
from edx_exams.apps.core.statuses import ExamAttemptStatus
from edx_exams.apps.lti.utils import get_lti_root

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -235,12 +235,12 @@ def start_proctoring(request, attempt_id):
lti_config = LtiConfiguration.objects.get(id=lti_config_id)

proctoring_start_assessment_url = urljoin(
get_lti_root(),
get_lti_api_base(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this helper function already existed in the lti library. Removed a 'duplicate' util from this service

reverse('lti_consumer:lti_consumer.start_proctoring_assessment_endpoint')
)

assessment_control_url = urljoin(
get_lti_root(),
get_lti_api_base(),
reverse('lti:acs', kwargs={'lti_config_id': lti_config_id}),
)

Expand All @@ -261,12 +261,13 @@ def start_proctoring(request, attempt_id):
proctoring_launch_data=proctoring_launch_data,
context_id=exam.course_id,
context_label=exam.content_id,
custom_parameters={
# The protocol is intentionally stripped from this url, the proctoring tool
# is appending https:// to anything we pass here. This is unavoidable at the moment.
'custom_url': urlparse(get_lti_api_base()).netloc + reverse('browser_lock'),
},
)

# temporary addition for testing with Proctorio in stage
if settings.LTI_CUSTOM_URL_CLAIM: # pragma: no cover
launch_data.custom_parameters['custom_url'] = settings.LTI_CUSTOM_URL_CLAIM

return redirect(get_lti_1p3_launch_start_url(launch_data))


Expand Down
4 changes: 4 additions & 0 deletions edx_exams/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from django.conf import settings
from django.contrib import admin
from django.urls import include, path
from django.views.generic import RedirectView
from edx_api_doc_tools import make_api_info, make_docs_urls

from edx_exams.apps.api import urls as api_urls
Expand All @@ -37,6 +38,9 @@
path('health/', core_views.Health.as_view(), name='health'),
path('lti/', include(lti_urls)),
path('lti/', include('lti_consumer.plugin.urls')),
# url for the exam inside the Proctorio browser extension the browser will fall through
# to this redirect if the browser extension is not installed
path('browser_lock/', RedirectView.as_view(url='https://getproctorio.com'), name='browser_lock'),
]

if settings.DEBUG and os.environ.get('ENABLE_DJANGO_TOOLBAR', False): # pragma: no cover
Expand Down
Loading