refactor: replace deprecated rebind_noauth_module_to_user, get_real_user, runtime.hostname, runtime.course_id [BD-13]

[tecoholic]

Description

The rebind_noauth_module_to_user function is deprecated^[1] in the core edx-platform. This is now replaced with a "rebind_user" service. This PR brings this change to the LTI Consumer XBlock.

This should be considered for review and merged after openedx/openedx-platform#30320 is merged.

Edit
The PR is now updated with changes related to the deprecation of the hostname parameter^[2] from the runtime as well.

It also replaces usages of get_real_user with the UserService and course_id with runtime.scope_ids.usage_id.context_key

This should be considered for merging only after openedx/openedx-platform#30308, openedx/openedx-platform#30320, and openedx/openedx-platform#30715 are merged.

Testing instructions

The testing instructions of the parent PR can be used to test the LTI Consumer XBlock.

Additional information

[1] - openedx/openedx-platform#30320
[2] - openedx/openedx-platform#30308

[openedx-webhooks]

Thanks for the pull request, @tecoholic!

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

[codecov]

Codecov Report

Base: 97.78% // Head: 97.78% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (f647c5f) compared to base (224e65e).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #249   +/-   ##
=======================================
  Coverage   97.78%   97.78%           
=======================================
  Files          72       72           
  Lines        5870     5876    +6     
=======================================
+ Hits         5740     5746    +6     
  Misses        130      130           

Flag	Coverage Δ
unittests	97.78% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
lti_consumer/api.py	98.86% <ø> (ø)
lti_consumer/signals/signals.py	100.00% <ø> (ø)
lti_consumer/tests/unit/plugin/test_views.py	100.00% <ø> (ø)
...r/tests/unit/plugin/test_views_lti_deep_linking.py	100.00% <ø> (ø)
..._consumer/tests/unit/plugin/test_views_lti_nrps.py	91.05% <ø> (ø)
lti_consumer/tests/unit/test_api.py	100.00% <ø> (ø)
lti_consumer/tests/unit/test_models.py	100.00% <ø> (ø)
lti_consumer/lti_1p1/oauth.py	100.00% <100.00%> (ø)
lti_consumer/lti_xblock.py	95.65% <100.00%> (+<0.01%)	⬆️
lti_consumer/models.py	91.51% <100.00%> (ø)
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Agrendalath]

I've reused your script from openedx/openedx-platform#30320, but it was giving me the following error:

Traceback (most recent call last):
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/views/decorators/clickjacking.py", line 50, in wrapped_view
    resp = view_func(*args, **kwargs)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/courseware/module_render.py", line 895, in handle_xblock_callback_noauth
    return _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course=course)
  File "/edx/app/edxapp/edx-platform/lms/djangoapps/courseware/module_render.py", line 1108, in _invoke_xblock_handler
    resp = handler_instance.handle(handler, req, suffix)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/xblock/mixins.py", line 84, in handle
    return self.runtime.handle(self, handler_name, request, suffix)
  File "/edx/app/edxapp/edx-platform/common/lib/xmodule/xmodule/x_module.py", line 1367, in handle
    return super().handle(block, handler_name, request, suffix=suffix)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/xblock/runtime.py", line 1081, in handle
    results = handler(request, suffix)
  File "/edx/shared-src/xblock-lti-consumer/lti_consumer/lti_xblock.py", line 1325, in result_service_handler
    log_authorization_header(request, lti_provider_key, lti_provider_secret)
  File "/edx/shared-src/xblock-lti-consumer/lti_consumer/lti_1p1/oauth.py", line 153, in log_authorization_header
    params = client.get_oauth_params(request)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/oauthlib/oauth1/rfc5849/__init__.py", line 185, in get_oauth_params
    params.append(('oauth_body_hash', base64.b64encode(hashlib.sha1(request.body.decode()).digest()).decode('utf-8')))
TypeError: Unicode-objects must be encoded before hashing

To avoid this, I've replaced the following line in /edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/oauthlib/oauth1/rfc5849/__init__.py.

- params.append(('oauth_body_hash', base64.b64encode(hashlib.sha1(request.body.encode('utf-8')).digest()).decode('utf-8')))
+ params.append(('oauth_body_hash', base64.b64encode(hashlib.sha1(request.body.decode().encode('utf-8')).digest()).decode('utf-8')))

Should I do something different while sending this request?

Edit: also, please bump the version in lti_consumer/__init__.py and add an entry to the changelog.

[giovannicimolin]

@Agrendalath @tecoholic Can you double-check grade linking still work? https://github.com/openedx/xblock-lti-consumer/blob/master/lti_consumer/signals.py#L34

I'm worried that set_user_module_score on that function will fail.

[Agrendalath]

@giovannicimolin, I've followed the instructions from openedx/openedx-platform#30320 on the devstack with a modification mentioned in my previous comment. Should we do something else to check it?

[tecoholic]

@giovannicimolin @Agrendalath I might have jumped the gun on this one. I think I didn't do a proper install inside the container while testing this. Let me rework this & submit it again for review.

[tecoholic]

@giovannicimolin The test I created for testing the LTI 2.0 endpoints in openedx/openedx-platform#30320 doesn't seem to create LtiAgsScore objects and hence can't trigger the post_save signal you have noted. So, I couldn't test it.

Is there a way to test this? Should we setup a LTI 1.3 client for this?

[giovannicimolin]

@tecoholic The LTI AGS models are only used by LTI 1.3 with grades enabled.

To avoid the hassle of setting it up, you can:

1. Add an LTI 1.3 block and set the block to scored and add weight.
2. Go to Django admin and create a new LtiAgsLineItem and fill in all fields.
3. Still in the Django admin, add a new LtiAgsScore and fill in all fields.

This should trigger the signal, you can add a debugger/breakpoint to analyze the parameters and match the conditions. 😄

[tecoholic]

@giovannicimolin I was able to test the the signal is triggered and processed without issue using the following steps:

1. Add a LTI 1.3 Block with Scored = True, Weight default to 1.0 and LTI and I set Tool launch URL to https://saltire.lti.app/tool so that the student view would load something instead of just showing the "set the url" error.
2. Set the subsection as a graded section (Mid-term Exam)
3. Open the LTI block in LMS after saving and publishing the course
4. An LtiAgsLineItem was automatically added when the student view was loaded. Verified by checking the admin (http://localhost:18000/admin/lti_consumer/ltiagslineitem/)
5. Added an external id in the Admin (http://localhost:18000/admin/external_user_ids/externalid/) for the user I was testing, as the signal fetches user using get_user_from_external_user_id and the signal was failing without one. I am assuming, normally this gets created when an actual LTI 1.3 tool interacts with the LTI Consumer.
6. Added a LtiAgsScore in the admin and used the UUID of the external user id, watched the signal using a debugger.

Everything worked as expected and the score was verified in the gradebook as well.

Does the above testing resolve our concerns?

cc: @Agrendalath

[giovannicimolin]

@tecoholic That's perfect, thank you for testing it!

No more concerns from my side! 👍

[Agrendalath]

@tecoholic, please rebase this on the master branch (we need to bump the version to 4.2.0).

👍

• I tested this: verified that the XBlock is working correctly with the new rebind_user service.
• 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]

Just repeating the note from the description - we shouldn't merge this before openedx/openedx-platform#30320.

[tecoholic]

@Agrendalath A new commit (05bda98) to related to the hostname deprecation is also now added as a part of the PR. Kindly review additional changes when you have the time. Thanks in advance.

[Agrendalath]

@giovannicimolin, @ashultz0, I'm not familiar with the change introduced in this commit, so I just wanted to mention that this scenario (i.e. creating a new LtiAgsScore via Django admin) raises the following exception:

2022-11-22 15:20:56,058 ERROR 5002 [django.request] [user None] [ip None] log.py:224 - Internal Server Error: /admin/lti_consumer/ltiagsscore/add/
Traceback (most recent call last):
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/lib/python3.8/contextlib.py", line 75, in inner
    return func(*args, **kwds)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/contrib/admin/options.py", line 616, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/utils/decorators.py", line 130, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/contrib/admin/sites.py", line 232, in inner
    return view(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/contrib/admin/options.py", line 1657, in add_view
    return self.changeform_view(request, None, form_url, extra_context)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/utils/decorators.py", line 43, in _wrapper
    return bound_method(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/utils/decorators.py", line 130, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/contrib/admin/options.py", line 1540, in changeform_view
    return self._changeform_view(request, object_id, form_url, extra_context)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/contrib/admin/options.py", line 1586, in _changeform_view
    self.save_model(request, new_object, form, not add)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/contrib/admin/options.py", line 1099, in save_model
    obj.save()
  File "/edx/shared-src/xblock-lti-consumer/lti_consumer/models.py", line 704, in save
    super().save(*args, **kwargs)
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/base.py", line 739, in save
    self.save_base(using=using, force_insert=force_insert,
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/base.py", line 787, in save_base
    post_save.send(
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/dispatch/dispatcher.py", line 180, in send
    return [
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/dispatch/dispatcher.py", line 181, in <listcomp>
    (receiver, receiver(signal=self, sender=sender, **named))
  File "/edx/shared-src/xblock-lti-consumer/lti_consumer/signals/signals.py", line 64, in publish_grade_on_score_update
    raise exc
  File "/edx/shared-src/xblock-lti-consumer/lti_consumer/signals/signals.py", line 53, in publish_grade_on_score_update
    block.set_user_module_score(user, score, block.max_score(), instance.comment)
  File "/edx/shared-src/xblock-lti-consumer/lti_consumer/lti_xblock.py", line 1423, in set_user_module_score
    self.runtime.rebind_noauth_module_to_user(self, user)
  File "/edx/app/edxapp/edx-platform/xmodule/services.py", line 192, in rebind_noauth_module_to_user
    raise RebindUserServiceError(err_msg)
xmodule.services.RebindUserServiceError: rebind_noauth_module_to_user can only be called from a module bound to an anonymous user

The current PR doesn't change this behavior, but please let me know if you'd like to review it before we merge.

[Agrendalath]

👍

• I tested this: verified that the XBlock is working correctly (with this exception) after replacing deprecated runtime attributes
• 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]

Nit: are we still using this and xblock.location (above) anywhere?

[ashultz0]

@Agrendalath thanks for pointing that out. We'll have to figure out how to let that work without so much magic.

[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.

[ashultz0]

this is going into the platform today along with a mostly unrelated bugfix as 7.0.1 openedx/openedx-platform#31369

[ashultz0]

this line turns out to have a bug - it should be
real_user = self.xblock.runtime.service(self.xblock, 'user').get_user_by_anonymous_id(anon_id)

[ashultz0]

@Agrendalath @tecoholic 7.0.1 failed on edx production in the outcome service and had to be rolled back, I think I have figured out why and noted above

we're actually already changing that to allow external instead of anonymous IDs in 7.0.2 which will I believe fix the problem without having to revert all the rest of the refactors

[Agrendalath]

@ashultz0, good catch. Thank you for fixing this!