[BD-24] [TNL-7318]: LTI AGS Implementation - LineItem Service #92

giovannicimolin · 2020-07-21T18:19:44Z

This PR implements the LTI AGS LineItem Service.

What does these changes do?
They are implementing the LTI AGS LineItem implementation and putting on the groundwork required to launch other LTI Advantage services.
The changes here include a partial shift to the LTI configuration being stored in a model, authentication methods and all things needed to get the LineItem service working.

Where is the LTI configuration stored?
In the XBlocks, we're only currently using a model as a proxy to the settings stored in the blocks.

What are the main architecture changes?
LineItems are stored on Django models and use DRF to provide a CRUD API for LineItems.
XBlock configuration is still stored on xblock fields, but it's accessed through a python API.

What are the next steps for this?
Chunk this change into manageable and easy to review pull requests:

Proxying configuration through Django model (and a few small fixes). PR Merged: [BD-24] [TNL-7318] BB-2355: Move LTI configuration to models. #93
Adding authentication and token verification support. PR Merged: [BD-24] [TNL-7318]: BB-2355 - Add decoding and scope verification methods #95
Adding some Django support. PR Merged: [BD-24] [TNL-7318] BB-2355: Add LTI support and Django authentication extension. #105
Add LTI AGS endpoints. This one.

Testing instructions:

Checkout this branch on your master's devstack src folder.
Install it on lms and studio using pip install -e /edx/src/xblock-lti-consumer, apply migrations.
Edit studio.yml and enable the following feature flag:

FEATURES:
    LTI_1P3_ENABLED: true

Restart Studio (make studio-restart) and log in.
Create a new course, add lti_consumer to the advanced modules list and add it to a course.
Set LTI Version to LTI 1.3, and mark .
Go to https://lti-ri.imsglobal.org/keygen/index and copy the public key, paste it into the LTI 1.3 Tool Public Key field. Save the private key for later.
Generate a client token: go to www.jwt.io, select RS-256 and paste the private key in the private key file. Header and Body as follows and then copy the JWT.

# HEADER:
{
  "alg": "RS256",
  "typ": "JWT",
  "kid": "123"
}

# PAYLOAD:
{
  "sub": "1234567890",
  "name": "John Doe",
  "admin": true,
  "iat": 1516239022
}

Retrieve an LTI Authentication Key: copy the OAuth Token URL and make the following request (don't forget to replace the JWT from the previous step). This will yield your access token to use the endpoints.

curl --request POST \
  --url 'http://localhost:18000/api/lti_consumer/v1/token/block-v1:OC+LTI+2020+type@lti_consumer+block@adfaf874726a4060808f04cb2bfaa6e2' \
  --header 'content-type: application/x-www-form-urlencoded' \
  --data grant_type=client_credentials \
  --data client_assertion_type=urn:ietf:params:oauth:client-assertion-type:jwt-bearer \
  --data client_assertion=PUT_YOUR_JWT_HERE \
  --data scope=https://purl.imsglobal.org/spec/lti-ags/scope/lineitem

Go into Django admin, and copy the LTI Configuration ID (http://localhost:18000/admin/lti_consumer/lticonfiguration/).
Make a request to the LTI AGS List endpoint:

curl --request GET \
  --url http://localhost:18000/api/lti_consumer/v1/lti/LTI_CONFIG_ID/lti-ags \
  --header 'authorization: Bearer ACCESS_TOKEN_HERE' 

# Should return []

Create a new LineItem using POST:

curl --request POST \
  --url http://localhost:18000/api/lti_consumer/v1/lti/LTI_CONFIG_ID/lti-ags/ \
  --header 'authorization: Bearer ACCESS_TOKEN_HERE' \
  --header 'content-type: application/vnd.ims.lis.v2.lineitem+json' \
  --data '{"scoreMaximum": 100, "label": "Score", "tag": "score", "resourceId": "some-random-string"}
'

Run step 9 again, and verify that the LineItem is retrieved.

(Optionally)
If you have a LTI 1.3 tool set up locally:
13. Perform a launch, and check that the LTI message includes the AGS claim:

# In the LTI message
  "https://purl.imsglobal.org/spec/lti-ags/claim/endpoint": {
    "scope": [
      "https://purl.imsglobal.org/spec/lti-ags/scope/lineitem",
      "https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly",
      "https://purl.imsglobal.org/spec/lti-ags/scope/score"
    ],
    "lineitems": "http://localhost:18000/api/lti_consumer/v1/lti/3/lti-ags"
  }

Certification suite:
To run the IMS certification suite properly, you'll need to override LMS_BASE_URL to match the ngrok url (so that the LTI AGS Claim contains the right URL).

Reviewers:

openedx-webhooks · 2020-07-21T18:19:48Z

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

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

giovannicimolin · 2020-09-04T15:39:36Z

@nedbat Since the first two PRs supporting this one were merged, I'll pick this up next week, rebase, add tests and make sure it's fully compliant to the LTI spec (at least for the LineItem endpoint).

In the meantime, can you take a look at the architectural decisions I've taken here? I'm mostly concerned about these things:

Authentication class implementation (here): I feel like there's a better way to pass the LTI consumer back to the view without needing to re-instance it (and without storing it in a model).
Authorization is a custom permission class that makes use if the LTI consumer passed back through the request in the Authentication phase (here)
Pagination isn't implemented yet, and will need a custom class that complies with the LTI spec (I will split up a separate task to handle this)

References:
https://www.imsglobal.org/spec/lti-ags/v2p0#normative-references
https://www.imsglobal.org/spec/lti-ags/v2p0/#example-application-vnd-ims-lis-v2-lineitem-json-representation
https://www.imsglobal.org/spec/lti/v1p3/

coveralls · 2020-09-11T19:53:15Z

Coverage decreased (-0.1%) to 98.792% when pulling 309b323 on open-craft:giovanni/bb-2355-add-lineitem-endpoint into 11152be on edx:master.

giovannicimolin · 2020-09-11T22:54:24Z

@nedbat At this point the main implementation is finished. I'm just writing tests (a lot of them) and cleaning it up.

giovannicimolin · 2020-09-29T17:40:20Z

@viadanna Ready for review.

Signed-off-by: Giovanni Cimolin da Silva <[email protected]>

lti_consumer/lti_1p3/extensions/rest_framework/permissions.py

lti_consumer/utils.py

shimulch

LGTM 👍

I tested this as per testing instruction
I read through the code
~~- [ ] I checked for accessibility issues~~
Includes documentation
~~- [ ] I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.~~

nedbat

Very nice. A few comments, I think only one point that could be a real problem.

lti_consumer/plugin/views.py

lti_consumer/lti_1p3/extensions/rest_framework/renderers.py

lti_consumer/lti_1p3/tests/test_ags.py

nedbat · 2020-10-01T20:23:45Z

lti_consumer/lti_1p3/constants.py

+    # LTI-AGS Scopes
+    'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem.readonly',
+    'https://purl.imsglobal.org/spec/lti-ags/scope/lineitem',
+]


We don't have to add https://purl.imsglobal.org/spec/lti-ags/scope/result.readonly and https://purl.imsglobal.org/spec/lti-ags/scope/score here?

@nedbat The results and score services are being implemented in separate tasks (https://openedx.atlassian.net/browse/TNL-7331 and https://openedx.atlassian.net/browse/TNL-7332) to keep the diff size smaller.

There's already a WIP PR out, based on this branch: #108. Which will be rebased when this lands.

nedbat · 2020-10-01T20:34:10Z

lti_consumer/lti_1p3/extensions/rest_framework/serializers.py

+
+    Recreated here since we cannot import directly from
+    from the platform like so:
+    `from openedx.core.lib.api.serializers import UsageKeyField`


I wonder if this means this code should go into the opaque-keys library? (Not now, but eventually.)

@nedbat Yes, this serializer should be moved from the core to opaque-keys.
If you prefer, I can move this to compat.py and mock the serializer in tests.
What do you say?

I am fine with merging this as is, and bringing up this need with the owners of opaque-keys to address.

@nedbat Thanks for that! Can you ping the owners on this use case? I'm not sure if there's other places duplicating this code (or avoiding UsageKey serialization on plugins at all).

lti_consumer/lti_1p3/extensions/rest_framework/serializers.py

nedbat · 2020-10-01T20:40:55Z

lti_consumer/lti_1p3/extensions/rest_framework/serializers.py

+        "endDateTime": "2018-04-06T22:05:03Z"
+    }
+
+    Note: The platform MUST NOT modify the 'resourceId', 'resourceLinkId' and 'tag' values.


I don't understand what this means. Why would a serializer ever modify anything?

@nedbat That was a straight copy from the IMS LTI-AGS spec (here). I've moved this to the model since it makes more sense to have it there.

lti_consumer/tests/unit/plugin/test_views_lti_ags.py

giovannicimolin · 2020-10-01T22:27:46Z

@shimulch @nedbat I've addressed your review comments.

nedbat · 2020-10-05T19:25:46Z

I've successfully run the IMS Validator on this branch.

nedbat · 2020-10-05T21:08:49Z

openedx-webhooks added blended PR is managed through 2U's blended developmnt program needs triage labels Jul 21, 2020

giovannicimolin changed the title ~~[BD-3] [TNL-7318]: LTI AGS PoC Implementation - Do not merge. WIP~~ [BD-24] [TNL-7318]: LTI AGS PoC Implementation - Do not merge. WIP Jul 21, 2020

giovannicimolin mentioned this pull request Jul 23, 2020

[BD-24] [TNL-7318] BB-2355: Move LTI configuration to models. #93

Merged

3 tasks

giovannicimolin mentioned this pull request Jul 30, 2020

[BD-24] [TNL-7318]: BB-2355 - Add decoding and scope verification methods #95

Merged

3 tasks

giovannicimolin force-pushed the giovanni/bb-2355-add-lineitem-endpoint branch from 8074666 to 3a8aec4 Compare September 8, 2020 14:12

giovannicimolin changed the title ~~[BD-24] [TNL-7318]: LTI AGS PoC Implementation - Do not merge. WIP~~ [BD-24] [TNL-7318]: LTI AGS Implementation WIP Sep 11, 2020

giovannicimolin force-pushed the giovanni/bb-2355-add-lineitem-endpoint branch from 8d5360c to bcd36aa Compare September 15, 2020 18:56

giovannicimolin mentioned this pull request Sep 15, 2020

[BD-24] [TNL-7318] BB-2355: Add LTI support and Django authentication extension. #105

Merged

3 tasks

giovannicimolin force-pushed the giovanni/bb-2355-add-lineitem-endpoint branch 2 times, most recently from acaca93 to 11fd22d Compare September 23, 2020 18:28

giovannicimolin changed the title ~~[BD-24] [TNL-7318]: LTI AGS Implementation WIP~~ [BD-24] [TNL-7318]: LTI AGS Implementation - LineItem Service Sep 29, 2020

giovannicimolin added 5 commits September 29, 2020 18:44

Add initial version of LineItem Implementation

f3422fc

Signed-off-by: Giovanni Cimolin da Silva <[email protected]>

Adding tests

94e33ca

Add LTI tests and fix quality isses

d1203e8

Quality nit

82f7084

Add missing requirement, upgrade

ef0cc83

giovannicimolin force-pushed the giovanni/bb-2355-add-lineitem-endpoint branch from 4bc81ae to ef0cc83 Compare September 29, 2020 21:45

giovannicimolin added 2 commits September 29, 2020 18:48

Squash migrations

48e50d2

Fix merge conflicts

9467b86

pcockwell mentioned this pull request Sep 30, 2020

[BD-24] Implement LTI AGS Score Publish Service and Results #108

Merged

2 tasks

shimulch reviewed Oct 1, 2020

View reviewed changes

lti_consumer/lti_1p3/extensions/rest_framework/permissions.py Outdated Show resolved Hide resolved

shimulch reviewed Oct 1, 2020

View reviewed changes

lti_consumer/utils.py Outdated Show resolved Hide resolved

shimulch approved these changes Oct 1, 2020

View reviewed changes

nedbat suggested changes Oct 1, 2020

View reviewed changes

Addressing review comments

309b323

nedbat approved these changes Oct 5, 2020

View reviewed changes

nedbat merged commit 33bb7c7 into openedx:master Oct 5, 2020

openedx-webhooks added merged and removed needs triage labels Oct 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BD-24] [TNL-7318]: LTI AGS Implementation - LineItem Service #92

[BD-24] [TNL-7318]: LTI AGS Implementation - LineItem Service #92

giovannicimolin commented Jul 21, 2020 •

edited

Loading

openedx-webhooks commented Jul 21, 2020 •

edited

Loading

giovannicimolin commented Sep 4, 2020 •

edited

Loading

coveralls commented Sep 11, 2020 •

edited

Loading

giovannicimolin commented Sep 11, 2020 •

edited

Loading

giovannicimolin commented Sep 29, 2020

shimulch left a comment

nedbat left a comment

nedbat Oct 1, 2020

giovannicimolin Oct 1, 2020

nedbat Oct 1, 2020

giovannicimolin Oct 1, 2020

nedbat Oct 2, 2020

giovannicimolin Oct 2, 2020

nedbat Oct 1, 2020

giovannicimolin Oct 1, 2020

giovannicimolin commented Oct 1, 2020

nedbat commented Oct 5, 2020

nedbat commented Oct 5, 2020

[BD-24] [TNL-7318]: LTI AGS Implementation - LineItem Service #92

[BD-24] [TNL-7318]: LTI AGS Implementation - LineItem Service #92

Conversation

giovannicimolin commented Jul 21, 2020 • edited Loading

openedx-webhooks commented Jul 21, 2020 • edited Loading

giovannicimolin commented Sep 4, 2020 • edited Loading

coveralls commented Sep 11, 2020 • edited Loading

giovannicimolin commented Sep 11, 2020 • edited Loading

giovannicimolin commented Sep 29, 2020

shimulch left a comment

Choose a reason for hiding this comment

nedbat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giovannicimolin commented Oct 1, 2020

nedbat commented Oct 5, 2020

nedbat commented Oct 5, 2020

giovannicimolin commented Jul 21, 2020 •

edited

Loading

openedx-webhooks commented Jul 21, 2020 •

edited

Loading

giovannicimolin commented Sep 4, 2020 •

edited

Loading

coveralls commented Sep 11, 2020 •

edited

Loading

giovannicimolin commented Sep 11, 2020 •

edited

Loading