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

[MST-1473] Proctoring Service Launch Implementation #275

Closed
Closed
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
17 changes: 3 additions & 14 deletions lti_consumer/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,25 +144,14 @@ def get_lti_1p3_launch_info(config_id=None, block=None):
}


def get_lti_1p3_launch_start_url(config_id=None, block=None, deep_link_launch=False, dl_content_id=None, hint=""):
def get_lti_1p3_launch_start_url(config_id=None, block=None, lti_hint="", hint=""):
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 makes it easy for the proctoring preflight view to generate the launch URL without adding another "proctoring" parameter to get_lti_1p3_launch_start_url. I thought it would be better for the caller to determine the appropriate lti_hint, but I can also see an argument that callers shouldn't concern themselves with the details of lti_hint (although hint is a parameter already). I didn't like the idea of needing to add additional variables for this function to determine the appropriate lti_hint.

Copy link
Contributor

@zacharis278 zacharis278 Aug 22, 2022

Choose a reason for hiding this comment

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

would it be appropriate to change hint to login_hint? I think that makes the distinction between the parameters clearer.

edit: or possibly universally using message_hint instead of lti_hint if possible, the latter seems a bit vague to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I definitely support this. I found the distinction confusing myself and had to keep checking which was which. I'll just echo the names as they are in the specification: login_hint and lti_message_hint.

"""
Computes and retrieves the LTI URL that starts the OIDC flow.
"""
# Retrieve LTI consumer
lti_config = _get_lti_config(config_id, block)
lti_consumer = lti_config.get_lti_consumer()

# Change LTI hint depending on LTI launch type
lti_hint = ""
# Case 1: Performs Deep Linking configuration flow. Triggered by staff users to
# configure tool options and select content to be presented.
if deep_link_launch:
lti_hint = "deep_linking_launch"
# Case 2: Perform a LTI Launch for `ltiResourceLink` content types, since they
# need to use the launch mechanism from the callback view.
elif dl_content_id:
lti_hint = f"deep_linking_content_launch:{dl_content_id}"

# Prepare and return OIDC flow start url
return lti_consumer.prepare_preflight_url(
hint=hint,
Expand All @@ -189,15 +178,15 @@ def get_lti_1p3_content_url(config_id=None, block=None, hint=""):

# If there's no content items, return normal LTI launch URL
if not content_items.count():
return get_lti_1p3_launch_start_url(config_id, block, hint=hint)
return get_lti_1p3_launch_start_url(config_id, block, lti_hint="deep_linking", hint=hint)

# If there's a single `ltiResourceLink` content, return the launch
# url for that specif deep link
if content_items.count() == 1 and content_items.get().content_type == LtiDlContentItem.LTI_RESOURCE_LINK:
return get_lti_1p3_launch_start_url(
config_id,
block,
dl_content_id=content_items.get().id,
lti_hint=f"deep_linking_content_launch:{content_items.get().id}",
hint=hint,
)

Expand Down
3 changes: 3 additions & 0 deletions lti_consumer/lti_1p3/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,6 @@ class LTI_1P3_CONTEXT_TYPE(Enum): # pylint: disable=invalid-name
course_offering = 'http://purl.imsglobal.org/vocab/lis/v2/course#CourseOffering'
course_section = 'http://purl.imsglobal.org/vocab/lis/v2/course#CourseSection'
course_template = 'http://purl.imsglobal.org/vocab/lis/v2/course#CourseTemplate'


LTI_PROCTORING_DATA_KEYS = ['attempt_number', 'resource_link', 'session_data', 'start_assessment_url']
227 changes: 227 additions & 0 deletions lti_consumer/lti_1p3/consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
"""
from urllib.parse import urlencode

from lti_consumer.utils import check_token_claim
from . import constants, exceptions
from .constants import (
LTI_1P3_ROLE_MAP,
LTI_BASE_MESSAGE,
LTI_1P3_ACCESS_TOKEN_REQUIRED_CLAIMS,
LTI_1P3_ACCESS_TOKEN_SCOPES,
LTI_1P3_CONTEXT_TYPE,
LTI_PROCTORING_DATA_KEYS,
)
from .key_handlers import ToolKeyHandler, PlatformKeyHandler
from .ags import LtiAgs
Expand Down Expand Up @@ -659,3 +661,228 @@ def enable_nrps(self, context_memberships_url):

# Include LTI NRPS claim inside the LTI Launch message
self.set_extra_claim(self.nrps.get_lti_nrps_launch_claim())


class LtiProctoringConsumer(LtiConsumer1p3):
"""
This class is an LTI Proctoring Services LTI consumer implementation.

It builds on top of the LtiConsumer1p3r and adds support for the LTI Proctoring Services specification. The
specification can be found here: http://www.imsglobal.org/spec/proctoring/v1p0.

This consumer currently only supports the "Assessment Proctoring Messages" and the proctoring assessmen flow.
It does not currently support the Assessment Control Service.

The LtiProctoringConsumer requires necessary context to work properly, including data like attempt_number,
resource_link, etc. This information is provided to the consumer through the set_proctoring_data method, which
is called from the consuming context to pass in necessary data.
"""
def __init__(
self,
iss,
lti_oidc_url,
lti_launch_url,
client_id,
deployment_id,
rsa_key,
rsa_key_id,
tool_key=None,
tool_keyset_url=None,
):
"""
Initialize the LtiProctoringConsumer by delegating to LtiConsumer1p3's __init__ method.
"""
super().__init__(
iss,
lti_oidc_url,
lti_launch_url,
client_id,
deployment_id,
rsa_key,
rsa_key_id,
tool_key,
tool_keyset_url
)
self.proctoring_data = {}

def set_proctoring_data(self, **kwargs):
"""
Set the self.proctoring_data dictionary with the provided kwargs, so long as a given key is in
LTI_PROCTORING_DATA_KEYS.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

If the kwargs are an incomplete set, what happens? Should this be more strongly typed with actual args?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I did it this way originally because I thought we'd need partial data in some places, but that's no longer the case. I'll change it back to something like set_proctoring_data(attempt_number, resource_link, ...).

for key, value in kwargs.items():
if key in LTI_PROCTORING_DATA_KEYS:
self.proctoring_data[key] = value

def _get_base_claims(self):
"""
Return claims common to all LTI Proctoring Services LTI launch messages, to be used when creating LTI launch
messages.
"""
proctoring_claims = {
"https://purl.imsglobal.org/spec/lti-ap/claim/attempt_number": self.proctoring_data.get("attempt_number"),
"https://purl.imsglobal.org/spec/lti-ap/claim/session_data": self.proctoring_data.get("session_data"),
}

return proctoring_claims

def get_start_proctoring_claims(self):
"""
Return claims specific to LTI Proctoring Services LtiStartProctoring LTI launch message,
to be injected into the LTI launch message.
"""
proctoring_claims = self._get_base_claims()
proctoring_claims.update({
"https://purl.imsglobal.org/spec/lti/claim/message_type": "LtiStartProctoring",
"https://purl.imsglobal.org/spec/lti-ap/claim/start_assessment_url":
self.proctoring_data.get("start_assessment_url"),
})

return proctoring_claims

def get_end_assessment_claims(self):
"""
Return claims specific to LTI Proctoring Services LtiEndAssessment LTI launch message,
to be injected into the LTI launch message.
"""
proctoring_claims = self._get_base_claims()
proctoring_claims.update({
"https://purl.imsglobal.org/spec/lti/claim/message_type": "LtiEndAssessment",
})

return proctoring_claims

def generate_launch_request(
self,
preflight_response,
resource_link
):
"""
Build and return LTI launch message for proctoring.

This method overrides LtiConsumer1p3's method to include proctoring specific launch claims. It leverages
the set_extra_claim method to include these additional claims in the LTI launch message.
"""
lti_message_hint = preflight_response.get("lti_message_hint")
proctoring_claims = None
if lti_message_hint == "LtiStartProctoring":
proctoring_claims = self.get_start_proctoring_claims()
elif lti_message_hint == "LtiEndAssessment":
proctoring_claims = self.get_end_assessment_claims()
else:
raise ValueError('lti_message_hint must be one of [LtiStartProctoring, LtiStartAssessment].')

self.set_extra_claim(proctoring_claims)

return super().generate_launch_request(preflight_response, resource_link)

def check_and_decode_token(self, token):
"""
Once the Proctoring Tool is satisfied that the user has completed the necessary proctoring set up and that the
assessment will be proctored securely, it redirects the user to the Assessment Platform, directing the browser
to make a POST request containing a JWT and the session_data. This is the "Start Assessment" message.

This method validates the JWT signature and decodes the JWT. It also validates the claims in the JWT according
to the Proctoring Services specification.

It either returns a dictionary containing information required by the start_assessment, or it raises an
exception if any claims is missing or invalid.
"""
# Decode token and check expiration.
proctoring_response = self.tool_jwt.validate_and_decode(token)

# TODO: We MUST perform other forms of validation here.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reread the specification, and now I cannot find where I read that we MUST do this validation. I swore I read it, but I cannot find it again. Based on that, I think the majority of this validation is not necessary. I may have misread a paragraph like the following as a requirement that we validate that the tool did its job. I don't think we need to do that. That would mean that the attempt_number and resource_link would not longer be needed by the start_assessment view.

4.3.1.6 Attempt number claim
The https://purl.imsglobal.org/spec/lti-ap/claim/attempt_number claim specifies the candidate's attempt number, as set by the Assessment Platform. The Proctoring Tool MUST copy this value to the Start Assessment message unchanged.

# TODO: We MUST validate the verified_user claim if it is provided, although it is optional to provide it.
# An Assessment Platform MAY reject the Start Assessment message if a required identity claim is missing
# (indicating that it has not been verified by the Proctoring Tool). Which identity claims a
# Proctoring Tool will verify is subject to agreement outside of the scope of this specification but, at a
# minimum, it is recommended that Proctoring Tools performing identity verification are able to verify the
# given_name, family_name and name claims.
# See 3.3 Transferring the Candidate Back to the Assessment Platform.

# -------------------------
# Check Required LTI Claims
# -------------------------

# Check that the response message_type claim is "LtiStartAssessment".
claim_key = "https://purl.imsglobal.org/spec/lti/claim/message_type"
check_token_claim(
proctoring_response,
claim_key,
"LtiStartAssessment",
f"Token's {claim_key} claim should be LtiStartAssessment."
)

# # Check that the response version claim is "1.3.0".
claim_key = "https://purl.imsglobal.org/spec/lti/claim/version"
check_token_claim(
proctoring_response,
claim_key,
"1.3.0",
f"Token's {claim_key} claim should be 1.3.0."
)

# Check that the response session_data claim is the correct anti-CSRF token.
claim_key = "https://purl.imsglobal.org/spec/lti-ap/claim/session_data"
check_token_claim(
proctoring_response,
claim_key,
self.proctoring_data.get("session_data"),
f"Token's {claim_key} claim is not correct."
)

# TODO: Right now, the library doesn't support additional claims within the resource_link claim.
# Once it does, we should check the entire claim instead of just the id.
claim_key = "https://purl.imsglobal.org/spec/lti/claim/resource_link"
check_token_claim(
proctoring_response,
claim_key,
{"id": self.proctoring_data.get("resource_link")},
f"Token's {claim_key} claim is not correct."
)

claim_key = "https://purl.imsglobal.org/spec/lti-ap/claim/attempt_number"
check_token_claim(
proctoring_response,
claim_key,
self.proctoring_data.get("attempt_number"),
f"Token's {claim_key} claim is not correct."
)

# -------------------------
# Check Optional LTI Claims
# -------------------------

verified_user = proctoring_response.get("https://purl.imsglobal.org/spec/lti-ap/claim/verified_user", {})
# See 4.3.2.1 Verified user claim.
# The iss and sub attributes SHOULD NOT be included as they are opaque to the Proctoring Tool and cannot be
# independently verified.
iss = verified_user.get('iss')
if iss is not None:
raise exceptions.InvalidClaimValue('Token verified_user claim should not contain the iss claim.')
sub = verified_user.get('sub')
if sub is not None:
raise exceptions.InvalidClaimValue('Token verified_user claim should not contain the sub claim.')

# See 4.3.2.1 Verified user claim.
# If the picture attribute is provided it MUST point to a picture taken by the Proctoring Tool.
# It MUST NOT be the same picture provided by the Assessment Platform in the Start Proctoring message.
picture = verified_user.get('picture')
if picture and picture == self.lti_claim_user_data.get('picture'):
raise exceptions.InvalidClaimValue(
'If the verified_claim is provided and contains the picture claim,'
' the picture claim should not be the same picture provided by the Assessment Platform to the Tool.'
)
# TODO: We can leverage these verified user claims. For example, we could use
# these claims for Name Affirmation.
Copy link
Contributor

Choose a reason for hiding this comment

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

it might make sense to pull out the claim check in to a separate function which would be easier to test by itself

it could return the verified user we could just get that out again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you referring to lines 856 - 877?

Copy link
Contributor

Choose a reason for hiding this comment

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

yep!


end_assessment_return = proctoring_response.get(
"https://purl.imsglobal.org/spec/lti-ap/claim/end_assessment_return"
)

response = {
'end_assessment_return': end_assessment_return,
'verified_user': verified_user,
}

return response
6 changes: 5 additions & 1 deletion lti_consumer/lti_1p3/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ class NoSuitableKeys(Lti1p3Exception):
message = "JWKS could not be loaded from the URL."


class BadJwtSignature(Lti1p3Exception):
message = "The JWT signature is invalid."


class UnknownClientId(Lti1p3Exception):
pass

Expand Down Expand Up @@ -67,7 +71,7 @@ class LtiAdvantageServiceNotSetUp(Lti1p3Exception):


class LtiNrpsServiceNotSetUp(Lti1p3Exception):
message = "LTI Names and Role Provisioning Services is not set up."
message = "The LTI Names and Role Provisioning Service is not set up."


class LtiDeepLinkingContentTypeNotSupported(Lti1p3Exception):
Expand Down
4 changes: 3 additions & 1 deletion lti_consumer/lti_1p3/key_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import json

from Cryptodome.PublicKey import RSA
from jwkest import BadSyntax, WrongNumberOfParts, jwk
from jwkest import BadSignature, BadSyntax, WrongNumberOfParts, jwk
from jwkest.jwk import RSAKey, load_jwks_from_url
from jwkest.jws import JWS, NoSuitableSigningKeys
from jwkest.jwt import JWT
Expand Down Expand Up @@ -124,6 +124,8 @@ def validate_and_decode(self, token):
raise exceptions.NoSuitableKeys() from err
except (BadSyntax, WrongNumberOfParts) as err:
raise exceptions.MalformedJwtToken() from err
except BadSignature as err:
raise exceptions.BadJwtSignature() from err


class PlatformKeyHandler:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 3.2.14 on 2022-08-11 18:45

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('lti_consumer', '0015_add_additional_1p3_fields'),
]

operations = [
migrations.AddField(
model_name='lticonfiguration',
name='lti_1p3_proctoring_enabled',
field=models.BooleanField(default=False, help_text='Enable LTI Proctoring Services', verbose_name='Enable LTI Proctoring Services'),
),
]
Loading