feat: only add acs scope when proctoring (#373)

* feat: only add acs scope when proctoring

* feat: helper function to get token scopes

* feat: improved scope helper functions

* test: tests for new acs filtering

* test: improved test coverage for proctoring scopes

* chore: version bump

* docs: changelog update

* fix: removed duplicate test

Author: ilee2u

CHANGELOG.rst

@@ -17,6 +17,11 @@ Unreleased
 ~~~~~~~~~~
 * Improved logging for Proctoring LTI 1.3 launch failure.
 
+9.5.1 - 2023-05-19
+------------------
+* Added gate to ensure the ACS scope is only added when using the LtiProctoringConsumer
+* Moved scope validation to a helper function
+
 9.5.0 - 2023-05-08
 ------------------
 * Return HTML message telling user that exam is ready to start on start assessment response to LTI proctoring launch.

lti_consumer/__init__.py

@@ -4,4 +4,4 @@
 from .apps import LTIConsumerApp
 from .lti_xblock import LtiConsumerXBlock
 
-__version__ = '9.5.0'
+__version__ = '9.5.1'

lti_consumer/lti_1p3/constants.py

@@ -65,11 +65,9 @@
 
     # LTI-NRPS Scopes
     'https://purl.imsglobal.org/spec/lti-nrps/scope/contextmembership.readonly',
-
-    # ACS Scope
-    'https://purl.imsglobal.org/spec/lti-ap/scope/control.all',
 ]
 
+LTI_1P3_ACS_SCOPE = 'https://purl.imsglobal.org/spec/lti-ap/scope/control.all'
 
 LTI_DEEP_LINKING_ACCEPTED_TYPES = [
     'ltiResourceLink',

lti_consumer/lti_1p3/consumer.py

@@ -16,6 +16,7 @@
     LTI_BASE_MESSAGE,
     LTI_1P3_ACCESS_TOKEN_REQUIRED_CLAIMS,
     LTI_1P3_ACCESS_TOKEN_SCOPES,
+    LTI_1P3_ACS_SCOPE,
     LTI_1P3_CONTEXT_TYPE,
     LTI_PROCTORING_DATA_KEYS,
 )
@@ -31,6 +32,7 @@ class LtiConsumer1p3:
     """
     LTI 1.3 Consumer Implementation
     """
+
     def __init__(
             self,
             iss,
@@ -407,6 +409,19 @@ def get_public_keyset(self):
         """
         return self.key_handler.get_public_jwk()
 
+    def _check_if_scope_is_valid(self, scope):
+        """
+        Helper function to return the list of valid scopes present in the
+        request to the access token endpoint.
+        """
+        # Currently there are no scopes, because there is no use for
+        # these access tokens until a tool needs to access the LMS.
+        # LTI Advantage extensions make use of this.
+        # TODO: Make this function should only return the NRPS and AGS scopes if those features are enabled.
+        if scope in LTI_1P3_ACCESS_TOKEN_SCOPES:
+            return True
+        return False
+
     def access_token(self, token_request_data):
         """
         Validate request and return JWT access token.
@@ -454,13 +469,14 @@ def access_token(self, token_request_data):
         # Check scopes and only return valid and supported ones
         valid_scopes = []
         requested_scopes = token_request_data['scope'].split(' ')
-
         for scope in requested_scopes:
-            # Currently there are no scopes, because there is no use for
-            # these access tokens until a tool needs to access the LMS.
-            # LTI Advantage extensions make use of this.
-            if scope in LTI_1P3_ACCESS_TOKEN_SCOPES:
+            if self._check_if_scope_is_valid(scope):
                 valid_scopes.append(scope)
+            else:
+                log.warning(
+                    f'Scope: {scope} found in the request is not a valid '
+                    f'LTI 1.3 access token or ACS scope'
+                )
 
         # Scopes are space separated as described in
         # https://tools.ietf.org/html/rfc6749
@@ -544,6 +560,7 @@ class LtiAdvantageConsumer(LtiConsumer1p3):
       Note: this is a partial implementation with read-only LineItems.
       Reference spec: https://www.imsglobal.org/spec/lti-ags/v2p0
     """
+
     def __init__(self, *args, **kwargs):
         """
         Override parent class and set up required LTI Advantage variables.
@@ -756,6 +773,7 @@ class LtiProctoringConsumer(LtiConsumer1p3):
     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,
@@ -877,6 +895,15 @@ def generate_launch_request(
 
         return super().generate_launch_request(preflight_response)
 
+    def _check_if_scope_is_valid(self, scope):
+        """
+        Override of Lti1p3Consumer's _check_if_scope_is_valid
+        that accounts for the ACS scope for proctoring.
+        """
+        if scope in (LTI_1P3_ACCESS_TOKEN_SCOPES, LTI_1P3_ACS_SCOPE):
+            return True
+        return False
+
     def check_and_decode_token(self, token):
         """
         Decodes a Tool JWT token and validates OAuth and LTI Proctoring Services specificatin related claims. Returns a

lti_consumer/lti_1p3/tests/test_consumer.py

@@ -39,12 +39,26 @@
 RSA_KEY = RSA.generate(2048).export_key('PEM')
 
 
+def _generate_token_request_data(token, scope):
+    """
+    Helper function to generate requests to the access_token endpoint
+    """
+    return {
+        # We don't actually care about these 2 first values
+        "grant_type": "client_credentials",
+        "client_assertion_type": "urn:ietf:params:oauth:client-assertion-type:jwt-bearer",
+        "client_assertion": token,
+        "scope": scope,
+    }
+
+
 # Test classes
 @ddt.ddt
 class TestLti1p3Consumer(TestCase):
     """
     Unit tests for LtiConsumer1p3
     """
+
     def setUp(self):
         super().setUp()
 
@@ -542,18 +556,31 @@ def test_access_token_invalid_jwt(self):
         """
         Check if access token with invalid request data raises.
         """
-        request_data = {
-            "grant_type": "client_credentials",
-            "client_assertion_type": "urn:ietf:params:oauth:client-assertion-type:jwt-bearer",
-            # This should be a valid JWT
-            "client_assertion": "invalid-jwt",
-            # Scope can be empty
-            "scope": "",
-        }
+        request_data = _generate_token_request_data("invalid_jwt", "")
 
         with self.assertRaises(exceptions.MalformedJwtToken):
             self.lti_consumer.access_token(request_data)
 
+    def test_access_token_no_acs(self):
+        """
+        Check that ACS does not work for the access token in the
+        default LTI 1.3 consumer
+        """
+        # Generate a dummy, but valid JWT
+        token = self.lti_consumer.key_handler.encode_and_sign(
+            {
+                "test": "test"
+            },
+            expiration=1000
+        )
+
+        request_data = _generate_token_request_data(token, "https://purl.imsglobal.org/spec/lti-ap/scope/control.all")
+
+        response = self.lti_consumer.access_token(request_data)
+
+        # Check no ACS scope present in returned token
+        self.assertEqual(response.get('scope'), '')
+
     def test_access_token(self):
         """
         Check if a valid access token is returned.
@@ -570,15 +597,7 @@ def test_access_token(self):
             expiration=1000
         )
 
-        request_data = {
-            # We don't actually care about these 2 first values
-            "grant_type": "client_credentials",
-            "client_assertion_type": "urn:ietf:params:oauth:client-assertion-type:jwt-bearer",
-            # This should be a valid JWT
-            "client_assertion": token,
-            # Scope can be empty
-            "scope": "",
-        }
+        request_data = _generate_token_request_data(token, "")
 
         response = self.lti_consumer.access_token(request_data)
 
@@ -656,6 +675,7 @@ class TestLtiAdvantageConsumer(TestCase):
     """
     Unit tests for LtiAdvantageConsumer
     """
+
     def setUp(self):
         super().setUp()
 
@@ -899,6 +919,7 @@ class TestLtiProctoringConsumer(TestCase):
     """
     Unit tests for LtiProctoringConsumer
     """
+
     def setUp(self):
         super().setUp()
 
@@ -1166,6 +1187,46 @@ def test_invalid_check_and_decode_token(self, claim_key):
         with self.assertRaises(MissingRequiredClaim):
             self.lti_consumer.check_and_decode_token(encoded_token)
 
+    def test_access_token_no_valid_scopes(self):
+        """
+        Ensure that the no scopes are returned in the access token if the request scopes are invalid
+        """
+        # Generate a dummy, but valid JWT
+        token = self.lti_consumer.key_handler.encode_and_sign(
+            {
+                "test": "test"
+            },
+            expiration=1000
+        )
+
+        # This should be a valid JWT w/ the ACS scope
+        request_data = _generate_token_request_data(token, "invalid_scope")
+
+        response = self.lti_consumer.access_token(request_data)
+
+        # Check that the response has the ACS scope
+        self.assertEqual(response.get('scope'), "")
+
+    def test_access_token(self):
+        """
+        Ensure that the ACS scope is added based on the request to the access token endpoint
+        """
+        # Generate a dummy, but valid JWT
+        token = self.lti_consumer.key_handler.encode_and_sign(
+            {
+                "test": "test"
+            },
+            expiration=1000
+        )
+
+        # This should be a valid JWT w/ the ACS scope
+        request_data = _generate_token_request_data(token, "https://purl.imsglobal.org/spec/lti-ap/scope/control.all")
+
+        response = self.lti_consumer.access_token(request_data)
+
+        # Check that the response has the ACS scope
+        self.assertEqual(response.get('scope'), "https://purl.imsglobal.org/spec/lti-ap/scope/control.all")
+
     def test_valid_check_and_decode_token(self):
         """
         Ensures that a valid LtiStartAssessment JWT is validated successfully.