Implement standard LTI tables#1194
Conversation
cj-young
changed the title
| $postParams = $request->request->all(); | ||
| $getParams = $request->query->all(); | ||
| $allParams = array_merge($postParams, $getParams); | ||
| $iss = $allParams['iss']; |
There was a problem hiding this comment.
Is there a reason for this setup? For instance, are the same parameters handled via POST sometimes and GET other times? I like that this will catch both, but don't know if I'd reverse the order.
According to https://www.php.net/manual/en/function.array-merge.php, "If the input arrays have the same string keys, then the later value for that key will overwrite the previous one." I'd like to think that in case of conflict, if someone edits the GET parameters, it can't overwrite what the system puts in the POST.
There was a problem hiding this comment.
In the LTI spec, the data in the initial launch can be sent by either POST or GET. This method was used previously, it has just been moved here. I can't imagine the order mattering, if the LMS follows spec it shouldn't have information in both (or at the very least conflicting information in each). I will edit it to make it more explicitly use one or the other, but in practice it probably won't matter.
| class SodiumEncryptionService implements EncryptionServiceInterface | ||
| { | ||
|
|
||
| private string $encodedKey = 'niLb/WbAODNi7E4ccHHa/pPU3Bd9h6z1NXmjA981D4o='; |
There was a problem hiding this comment.
RED ALERT!!!! Big no-no. This absolutely should be in an environment variable loaded from there.
There was a problem hiding this comment.
The key was already hard-coded in User.php. This is addressed in PR #1078. Should we fix it in this PR or just merge the other one? At least for now, it shouldn't be a huge issue unless someone gets access to our database.
3 participants
Resolves #1193.
This implements the recommended registration, keyset, key, and deployment LTI tables. The keysets, keys, and deployments are currently practically unused but were added to match standard implementations.
The
JWK_BASE_URLvariable has been completely removed. UDOIT works from any instance without requiring any environment variable changes.A command has been created to initialize institutions/registrations and is documented in the READMEs.
The only non-standard parts of the registration table is the API client ID, secret, and OAuth login endpoint. Typical LTI authentication uses the
grant_typefield of valueclient_credentialsto bypass the user consent screen. However, this form of authentication doesn't allow a sufficient level of authorization required by UDOIT (like updating course content).The registration table and the institution table currently have a one-to-one relationship. Going forward, the institution table should be used primarily for UDOIT-specific data, and the registration table for LMS-interfacing data.