Fix several issues when using the library in Laravel

[dbhynds]

This PR addresses numerous issues with the library:

• The documentation for the Cookie and Cache objects indicates that they are interfaces, when in fact they implementations. The converts them to interfaces and builds out separate implementations for them.
• Removes a number of ?> tags per PSR-2 2.2
• Create a class for launch-related constants and removes magic strings.
• Fixes several issues with LTI_Launch_message::validate_jwt_signature() to allow these failures to be properly handled per the LTI 1.3 certification process (specifically, no kid specified, and expired jwt token)
• Throws an exception when no Deployment ID is specified
• Exposes the cookie prefix so the cookie can be referenced outside of the context of this library without using magic strings

This PR is dependent on EricTendian's PR: #44

[dbhynds]

@MartinLenord do you have any feedback on these changes?

[MartinLenord]

Thanks for the contribution, i'm sorry that i've been so slow to look at it (with the cancellation of the Europe meet and LILI, the plans i had for updating this have been pushed back a little)

I've made a couple of small suggestions, but otherwise i'd be happy to merge in the changes

[MartinLenord]

It's a shame that parseKey is private in the firebase implementation,

It might be neater here to just parse the keyset then check for the key we are looking for is there, then we can avoid the whole for loop here.

try {
    $parsed_key_set = JWK::parseKeySet($_public_key_set);
    if (isset($parsed_key_set[$this->jwt['header']['kid']])) {
        return openssl_pkey_get_details($parsed_key_set[$this->jwt['header']['kid']);
    }
} catch(\Exception $e) {
    throw new LTI_Exception("Unable to parse public key", 1);
}

[dbhynds]

It's made public in the latest version thanks to EricTendian :)
https://github.com/firebase/php-jwt/blob/v5.2.0/src/JWK.php#L35

[MartinLenord]

This is super neat and something i've been meaning to do for an age but never brought myself to do it, so thanks for that

[MartinLenord]

Naming is always a pain, it would be clearer if we did <SERVICE>_<TYPE>_<NAME> e.g. NRPS_CLAIM_SERVICE and NRPS_SCOPE_READ_ONLY

[dbhynds]

Sounds good. I'll implement this as soon as I get a chance.

[dbhynds]

@MartinLenord I finally got some time to go back and implement the feedback. Let me know if you have any additional comments.

Also, can we get this repo added to packagist? It makes me a little nervous that our composer.json is just pointing to the master branch. As it stands, anyone could push breaking changes to master and we'd have no idea and no easy way to lock it down to a specific version.

[mumitr0ll]

@dbhynds - IMS will discuss the possible movement of a point-in-time copy of the master to a repo site. Martin L can't do that on his own - that must be a change from IMS Global.

However, you worries are real and we will look into it. If IMS doesn't want to (for some technical reason unknown to me) use pakagist or some other repo site, could we do Releases here in Github? Would your php composer suite be able to query and use named/numbered Releases?
-Dereck

[dbhynds]

@mumitr0ll Thanks for sharing that context. Github releases would be an acceptable fallback if the packagist option doesn't work out.

[ernst77]

Any update on the merge? I will be using this branch instead of the official one.

[tijsverwest]

Hi, any updates on this? Would be nice to have it merged, if possible. Thanks!

[dbhynds]

Thanks for those who have asked. Yesterday, I actually released a large refactor to this library (based on this fork) to Packagist: https://packagist.org/packages/packbackbooks/lti-1p3-tool. We're still doing final testing on it to ensure it's stable, but should be releasing 1.0 of the package early next week. There are some considerable changes as part of the upgrade (most classes and methods got renamed). However, if you want to start using the new package, do:

composer require packbackbooks/lti-1p3-tool

Thanks for the interest.