[BD-24] [TNL-7318] BB-2355: Add LTI support and Django authentication extension. #105
Conversation
openedx-webhooks
added
blended
|
Thanks for the pull request, @giovannicimolin! I've created BLENDED-590 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. |
lti_consumer/lti_1p3/extensions/rest_framework/authentication.py
Outdated
Show resolved
Hide resolved
| raise exceptions.AuthenticationFailed(msg) | ||
|
|
||
| # Passing parameters back to the view through the request. | ||
| # Not exactly optimal. |
There was a problem hiding this comment.
I'd like to know more about this comment....
There was a problem hiding this comment.
I'll improve the comment.
There was a problem hiding this comment.
@nedbat Is it better now?
lti_consumer/lti_1p3/tests/extensions/rest_framework/test_authentication.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
@giovannicimolin I've read through the code and left a question on the passing the LTI configuration back to the view.
I also tested it locally and it's looking good.
lti_consumer/lti_1p3/extensions/rest_framework/authentication.py
Outdated
Show resolved
Hide resolved
lti_consumer/lti_1p3/tests/extensions/rest_framework/test_authentication.py
Outdated
Show resolved
Hide resolved
| Since the base implementation of this library uses JWT tokens, we expect | ||
| a RSA256 signed token that contains the allowed scopes. | ||
| """ | ||
| keyword = 'Bearer' |
There was a problem hiding this comment.
| keyword = 'Bearer' | |
| KEYWORD = 'Bearer' |
There was a problem hiding this comment.
I've based this class on https://github.com/encode/django-rest-framework/blob/master/rest_framework/authentication.py#L148 which uses a lowercase variable. I don't see any issue changing it to uppercase though.
Do you want me to change it?
| lti_config_id = request.parser_context['kwargs'].get('lti_config_id') | ||
|
|
||
| # Check if auth token is present on request and is correctly formatted. | ||
| if not auth or auth[0].lower() != self.keyword.lower(): |
There was a problem hiding this comment.
| if not auth or auth[0].lower() != self.keyword.lower(): | |
| if not auth or auth[0].lower() != self.KEYWORD.lower(): |
…entication.py Co-authored-by: Ned Batchelder <[email protected]>
This PR adds an authentication class that's used by LTI extensions to check the JWT token and a Django router for LTI related viewsets.
Also includes a small fix (includes
issin LTI payload message).Testing instructions::
Notes:
Split from #92 since it ended up growing too much.
Reviewers: