BB-2332: Add preflight response validation

lti_consumer/lti_1p3/consumer.py

@@ -5,7 +5,6 @@ import json
 import time
 
 # Quality checks failing due to know pylint bug
-# pylint: disable=relative-import
 from six.moves.urllib.parse import urlencode
 
 from Crypto.PublicKey import RSA
@@ -16,7 +15,7 @@ from jwkest import jwk
 from .constants import LTI_1P3_ROLE_MAP, LTI_BASE_MESSAGE
 
 
-class LtiConsumer1p3(object):
+class LtiConsumer1p3:
     """
     LTI 1.3 Consumer Implementation
     """
@@ -199,10 +198,12 @@ class LtiConsumer1p3(object):
         This will add all required parameters from the LTI 1.3 spec and any additional ones set in
         the configuration and JTW encode the message using the provided key.
         """
+        # Validate preflight response
+        self._validate_preflight_response(preflight_response)
+
         # Start from base message
         lti_message = LTI_BASE_MESSAGE.copy()
 
-        # TODO: Validate preflight response
         # Add base parameters
         lti_message.update({
             # Issuer
@@ -273,3 +274,19 @@ class LtiConsumer1p3(object):
         public_keys = jwk.KEYS()
         public_keys.append(self.jwk)
         return json.loads(public_keys.dump_jwks())
+
+    def _validate_preflight_response(self, response):
+        """
+        Validates a preflight response to be used in a launch request
+
+        Raises ValueError in case of validation failure
+
+        :param response: the preflight response to be validated
+        """
+        try:
+            assert response.get("nonce")
+            assert response.get("state")
+            assert response.get("client_id") == self.client_id
+            assert response.get("redirect_uri") == self.launch_url
+        except AssertionError:
+            raise ValueError("Preflight reponse failed validation")

lti_consumer/tests/unit/test_lti_1p3_consumer.py

@@ -23,6 +23,8 @@ OIDC_URL = "http://test-platform/oidc"
 LAUNCH_URL = "http://test-platform/launch"
 CLIENT_ID = "1"
 DEPLOYMENT_ID = "1"
+NONCE = "1234"
+STATE = "ABCD"
 # Consider storing a fixed key
 RSA_KEY_ID = "1"
 RSA_KEY = RSA.generate(2048).export_key('PEM')
@@ -73,7 +75,12 @@ class TestLti1p3Consumer(TestCase):
         parameters, but allows overriding them.
         """
         if preflight_response is None:
-            preflight_response = {"nonce": "", "state": ""}
+            preflight_response = {
+                "client_id": CLIENT_ID,
+                "redirect_uri": LAUNCH_URL,
+                "nonce": NONCE,
+                "state": STATE
+            }
 
         return self.lti_consumer.generate_launch_request(
             preflight_response,
@@ -91,6 +98,22 @@ class TestLti1p3Consumer(TestCase):
 
         return JWS().verify_compact(token, keys=key_set)
 
+    @ddt.data(
+        ({"client_id": CLIENT_ID, "redirect_uri": LAUNCH_URL, "nonce": STATE, "state": STATE}, True),
+        ({"client_id": "2", "redirect_uri": LAUNCH_URL, "nonce": STATE, "state": STATE}, False),
+        ({"client_id": CLIENT_ID, "redirect_uri": LAUNCH_URL[::-1], "nonce": STATE, "state": STATE}, False),
+        ({"redirect_uri": LAUNCH_URL, "nonce": NONCE, "state": STATE}, False),
+        ({"client_id": CLIENT_ID, "nonce": NONCE, "state": STATE}, False),
+        ({"client_id": CLIENT_ID, "redirect_uri": LAUNCH_URL, "state": STATE}, False),
+        ({"client_id": CLIENT_ID, "redirect_uri": LAUNCH_URL, "nonce": NONCE}, False),
+    )
+    @ddt.unpack
+    def test_preflight_validation(self, preflight_response, success):
+        if success:
+            return self.lti_consumer._validate_preflight_response(preflight_response)  # pylint: disable=protected-access
+        with self.assertRaises(ValueError):
+            return self.lti_consumer._validate_preflight_response(preflight_response)  # pylint: disable=protected-access
+
     @ddt.data(
         (
             'student',
@@ -253,6 +276,8 @@ class TestLti1p3Consumer(TestCase):
         self._setup_lti_user()
         launch_request = self._get_lti_message(
             preflight_response={
+                "client_id": "1",
+                "redirect_uri": "http://test-platform/launch",
                 "nonce": "test",
                 "state": "state"
             },

lti_consumer/tests/unit/test_lti_consumer.py

@@ -926,6 +926,7 @@ class TestLtiConsumer1p3XBlock(TestCase):
 
         self.xblock_attributes = {
             'lti_version': 'lti_1p3',
+            'lti_1p3_client_id': '1',
             'lti_1p3_launch_url': 'http://tool.example/launch',
             'lti_1p3_oidc_url': 'http://tool.example/oidc',
             'lti_1p3_tool_public_key': ''
@@ -957,7 +958,8 @@ class TestLtiConsumer1p3XBlock(TestCase):
 
         # Craft request sent back by LTI tool
         request = make_request('', 'GET')
-        request.query_string = "state=state_test_123&nonce=nonce&login_hint=oidchint&lti_message_hint=ltihint"
+        request.query_string = "client_id=1&redirect_uri=http://tool.example/launch&state=state_test_123&nonce=nonce\
+&login_hint=oidchint&lti_message_hint=ltihint"
 
         response = self.xblock.lti_1p3_launch_callback(request)