[BD-13][BB-6145] Move the rebind_noauth_module_to_user from ModuleSystem argument to service

[tecoholic]

Description

This removes the rebind_noauth_module_to_user argument from the ModuleSystem constructor and moves it to a separate service called "rebinder" in the class RebindModuleService. This is used in the LTI module to bind calls received by its noauth endpoint to bind the module the real_user.

The original function is defined as a local function and calls its parent function recursively to perform rebinding. Due to the heavy dependency of local variables, this new service is defined in the same module to minimize imports.

Supporting information

• OpenCraft ticket - https://tasks.opencraft.com/browse/BB-6145

Testing instructions

The changes primarily impact the LTI 2.0 JSON endpoint handlers in common/lib/xmodule/xmodule/lti_2_util.py (lines 173 and 256). We will test PUT handler to verify that the module rebinding is working as expected and the anonymous ID resolves into the correct user_id when updating the scores from external LTI tools.

1. Checkout to the master branch of edx-platform Setup a test course in the dev environment of choice.
2. Go to Advanced Settings and add "lti" to the Advanced Modules List. (Note: this is not the current lti_consumer module, this is the older lti module)
3. Scroll down to the LTI Passports and add "saltire:consumer_key:secret" to it. We will use the Saltire testing tool to get the various values for testing.
4. Save and go to the course editing page
5. Add a new Unit to the course
6. Select Advanced block type and select the LTI block to add an LTI XBlock
7. Click Edit on the LTI Block and set the following values and save the Block
   • LTI ID: saltire
   • LTI URL: https://saltire.lti.app/tool
   • Open in New Page: False
   • Scored: True
8. In the course overview, select the Settings button for the subsection with the unit and mark it as Graded (eg., Homework)
9. Publish the course and open the course as a student in the LMS
10. Expand the Message Parameters section in the Saltire Iframe and copy the user_id value. eg., e449db316ca1652a94487273c2e9f042
11. In a new tab open the URL http://<your-domain>/courses/<your-course-id>/lti_rest_endpoints/. eg., http://localhost:18000/courses/course-v1:UNIV+CS101+2022_T2/lti_rest_endpoints/
12. Note down the lti_2_0_result_service_json_endpoint URL. We will use this URL and the username later.
13. Now open the course as a staff/admin and navigate to Instructor -> Student Admin -> View Gradebook. Ideally the student user should be listed with no scores for the homework.

The next steps involve running a Python script to make OAuth requests. You can achieve the same with any API client which supports OAuth 1.0.

1. Setup a virtual environment and pip install requests-oauthlib
2. Create a Python file (test.py) with the following contents and fill in the REST_URL and USER_ID values we noted previously

# test.py
# Test script to make a OAuth signed PUT request to update the homework score via LTI 2.0 endpoint
import json

from requests_oauthlib import OAuth1Session
from oauthlib import oauth1

REST_URL = "<url-copied-from-the-rest-endpoints>"
USER_ID = "<user-id-copied-from-Saltire-ifram>"

url = f"{REST_URL}{USER_ID}"
lms = OAuth1Session('client_key', client_secret='secret', signature_method=oauth1.SIGNATURE_HMAC_SHA1, force_include_body=True)

score = {
    "@context" : "http://purl.imsglobal.org/ctx/lis/v2/Result",
    "@type" : "Result",
    "resultScore" : 0.9,
    "comment" : "This is exceptional work."
}

res = lms.put(url, data=json.dumps(score), headers={'Content-Type': 'application/vnd.ims.lis.v2.result+json'})

print(res.status_code)

3. Now when you run python test.py, it should print 200 and exit indicating the PUT request with a resultScore of successful. You can verify the same in the LMS logs and even notice the anonymous id getting translated into LMS's internal numerical ID in celery task logs.
4. Refresh the gradebook to verify that the homework now has a score of 90 for the student
5. Change the values (range 0.0-1.0) for resultScore to verify the scores are updated.

Testing the PR doesn't break module rebinding

1. Checkout the edx-platform to the PR branch: open-craft:tecoholic/BB-6145-deprecate-rebind-noauth-module-to-user
2. Restart the LMS service to ensure PR changes are loaded
3. Change the resultScore value and rerun the test script.
4. Verify the changes are reflected in the "Gradebook" and the requests are returning 200 without any errors. Additionally, see that the celery tasks are getting the correct user_id values in the logs.

Deadline

"None"

[openedx-webhooks]

Thanks for the pull request, @tecoholic! I've created BLENDED-1217 to keep track of it in Jira. More details are on the BD-13 project page.

When this pull request is ready, tag your edX technical lead.

[tecoholic]

@Agrendalath Thank you for the detailed review. I totally missed checking the LTI Consumer for the effects and also should have added the deprecation warning after you pointed it out in the hostname ticket.

I have incorporated most of the changes you pointed out. A couple of points that I would like to mention:

1. The XBlock runtime's "rebind_user" service isn't fully featured as the one initialized inside the module_render. While I was able to pass all the positional arguments of get_module_system_for_user in the initializer, I couldn't get a number of kwargs. So they will keep using the default values in the XBlock runtime. I have left a comment about this where the service is initialized.
2. I haven't used your suggestion of passing the XBlock as an initial argument to the service, because the LmsModuleSystem, initalizes the services as dict and thus doesn't have the reference to the XBlock during the service initialization. XBlock runtime could benefit from this, but as I mentioned in the previous points, the service is still very dependent on the module_system that I think, we should refactor this service when LTI Module is completely removed.

Kindly take review this again when you get some time. Thanks again for the detailed review catching a number of crucial issues.

[tecoholic]

@Agrendalath I have updated the PR with fixes for the quality check failures. Kindly approve the workflows.

[Agrendalath]

@ormsbee, this only needs one small change I've mentioned above to be completed. Would you like to take a look at this (and potentially openedx/xblock-lti-consumer#249) before we move forward?

[tecoholic]

@Agrendalath @ormsbee The change mentioned in the above comment is now included in the PR. Good for a review.

[Agrendalath]

👍

• I tested this: checked that the built-in LTI XBlock and LTI Consumer XBlock can rebind the user correctly
• I read through the code
• I checked for accessibility issues: n/a
• Includes documentation: n/a
• I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository: n/a

[Agrendalath]

@tecoholic, for the future - please don't squash commits until the review is finished. It's hard to follow exact changes, especially when we're rebasing the branch on the upstream one.

[tecoholic]

@Agrendalath Noted. I will keep in mind not to squash until the review is over. 👍

[openedx-webhooks]

@tecoholic 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.