LTI 1.3 platform implementation

[PasiSa]

Description

What?

This pull requests implements basic LTI 1.3 platform functionality according to the Core specification and Security Framework specification. It also includes parts of the Assignment and Grade Services specification, allowing the LTI tool to submit points to A+ platform.

Changes have been distributed in the following sections of in the code:

• External_services contains most of the core functionality needed for A+ authentication and resource link launch. This involves a new table in database for LTI 1.3 Service.
• Exercise models have been updated to support LTI 1.3 exercises. This involves a new table in database for LTI 1.3 Exercise.
• The lineitems and scores API endpoints needed by the Assignments and Grade services specification are implemented using the Django Rest Framework extending the courses API. It works otherwise similarly than existing routes in the API, except that it applies OAuth2 Bearer tokens for access control. Therefore this seemed a logical place for this functionality, even though it is LTI 1.3 related. In principle the API endpoints could also be used outside LTI, if valid bearer tokens are obtained somehow.
• Authorization contains additions related to bearer token management. Also there is a new database table for storing active bearer tokens. Expired bearer tokens are cleaned up from the database once an hour. This was later changed to use signed JWT tokens, that do not need separate table in database.

Notes:

• The scores API requires that the URL does not end with slash. This deviates from the earlier Django default functionality that automatically appended a slash at the end of the URL, and therefore the API routing needed to be changed to support URLs that either end or not with a slash. This also caused some of the existing tests to break, so they needed to be modified (this is a separate commit). However, it seems that existing URLs and functionalities continue to work as before from user's perspective.
• The DB model verbose texts were deliberately left non-translated. Some of the terms are such that trying to translate them to Finnish would only confuse the admin user in admin view.
• We reuse the private key from APLUS_AUTH settings for the OAuth2 JWT exchanges. I didn't see benefit in specifying a separate keyset for this use.

Small changes are needed also in RST tools and gitmanager to parse the LTI 1.3 related exercise parameters.

This is tested against Moodle 4 as LTI 1.3 tool, and Koodisäilö's recent LTI 1.3 modifications.

Why?

[ANSWER HERE]

How?

[ANSWER HERE]

Fixes #

Testing

Remember to add or update unit tests for new features and changes.

• How to test your changes in A-plus
• How to test accessibility

What type of test did you run?

• Accessibility test using the WAVE extension.
• Django unit tests.
• Selenium tests.
• Other test. (Add a description below)
• Manual testing.

[ADD A DESCRIPTION ABOUT WHAT YOU TESTED MANUALLY]

Did you test the changes in

• Chrome
• Firefox
• This pull request cannot be tested in the browser.

Think of what is affected by these changes and could become broken

Translation

• Did you modify or add new strings in the user interface? (Read about how to create translation)

Programming style

• Did you follow our style guides?
• Did you use Python type hinting in all functions that you added or edited? (type hints for function parameters and return values)

Have you updated the README or other relevant documentation?

• documents inside the doc directory.
• README.md.
• Aplus Manual.
• Other documentation (mention below which documentation).

Is it Done?

• Reviewer has finished the code review
• After the review, the developer has made changes accordingly
• Customer/Teacher has accepted the implementation of the feature

Clean up your git commit history before submitting the pull request!

[ihalaij1]

I did not find any real issues in the code just from looking at it. But I did add some minor comments.

[PasiSa]

Thanks @ihalaij1 , fixed the issues you pointed out.

[markkuriekkinen]

I have reviewed most of the code thus far. I have a few fairly small comments.

[markkuriekkinen]

Additional comments from me. I have now reviewed all the code.

[lainets]

Great changes! There are still a couple things left

[PasiSa]

@lainets @Mankro : now the bearer token to jwt tokens change is pushed, if you want to take a look. I made it a separate commit for clarity, but if everything looks ok, I can squash everything, and then it is ready for merging on my behalf.

[lainets]

I otherwise approve but note that a JWT token cannot be revoked, so we do not want the expiry time to be long. It seems the OIDC server defaults it to 3600 seconds (60 minutes) which is probably fine but I'd like it to be explicitly set just in case (I assume the LTI 1.3 spec allows changing it). What do you think? Should we set it explicitly or let the library decide?

[PasiSa]

I don't have strong opinions about the expiry time, but the LTI relationships are quite static and long-term (you configure it once, and then the service is accessible probably for at least several months), so I don't see one-hour expiry time as big issue. The expiry time would be a new settings variable, then?

[lainets]

The concern with the expiry time is not the service itself but any bad actors that may get their hands on the token, e.g. by hacking said service. While we can remove the service from our platform, they would still have a valid token that can be used until the expiry time. Technically, we could check if the token's owner is still in the lti service database when checking the permissions but I'm not sure if that is worth it.

Yes, it would be a new settings variable.

[markkuriekkinen]

Looks good overall. I still have a few small comments.

[PasiSa]

The concern with the expiry time is not the service itself but any bad actors that may get their hands on the token, e.g. by hacking said service. While we can remove the service from our platform, they would still have a valid token that can be used until the expiry time. Technically, we could check if the token's owner is still in the lti service database when checking the permissions but I'm not sure if that is worth it.

Yes, it would be a new settings variable.

I could not not figure out how to adjust the expiry time in this case with this API. It is possible to override the "exp" claim, but the OAuth2 also has the "expires_in" parameter in message header that still is 3600. Note that all examples in OAuth2 specification also use 3600, so apparently they don't consider that such dangerous value.

[lainets]

Based on https://github.com/oauthlib/oauthlib/blob/d4b6699f8ccb608152b764919e0bd3d38a7b171f/oauthlib/openid/connect/core/endpoints/pre_configured.py#LL35C12-L35C12, the expiry time is set with token_expires_in in the OIDCServer constructor. But I supposed it isn't such a big issue; you can just leave it be.

[PasiSa]

Yes, that was it. Thanks! Now updated according to latest comments. Squash now, maybe?

[PasiSa]

By the way, I don't know why this linter message comes up here but not in master:

selenium_test/test/test_initializer.py
  Line: 3[9](https://github.com/PasiSa/a-plus/actions/runs/5277166108/jobs/9544839929#step:5:10)
    pylint: unexpected-keyword-arg / Unexpected keyword argument 'chrome_options' in constructor call (col 21)

I haven't touched selenium tests in any way.

[lainets]

Seems like chrome_options had been deprecated and now was finally removed. It needs to be changed to options. I'll push a fix directly to master (if I can, we will see)

[markkuriekkinen]

Wonderful, I will merge this now!