search

celtic-project / LTI-PHP

PHP class library for building LTI integrations
GNU Lesser General Public License v3.0
47 stars 37 forks source link

Roles mangled by Membership service due to hardcoded LTI version #52

Closed chrispittman closed 1 year ago

chrispittman commented 1 year ago

In two places in Service/Membership/getMembers(), Tool::parseRoles() is called, with a hardcoded LTI version of Util::LTI_VERSION2. This leads to a bug in LTI 1.3 tools, which can receive membership roles for a user such as this pair (for a user who is both a teacher and a TA in the same course):

  [ "http://purl.imsglobal.org/vocab/lis/v2/membership/Instructor#TeachingAssistant" , 
    "http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor" ]

parseRoles() using LTI_VERSION2 incorrectly collapses these down into one role: 

  http://purl.imsglobal.org/vocab/lis/v2/membership#TeachingAssistant

...which isn't even one of the roles that the LTI 1.3 spec allows, as far as I can tell.

Changing the hardcoded version in these calls to LTI_VERSION1P3 causes this pair of roles to be preserved correctly.

(Offhand, it looks like the Membership class has access to the platform's LTI version (through $this->source->getPlatform()->ltiVersion)...maybe it could use that instead of hardcoding LTI_VERSION2?)

spvickers commented 1 year ago

Thanks for reporting this, I will take a closer look. On first glance the first call to parseRoles in Service/Membership looks correct as it relates to a JSON-LD response, but the second may be wrong.

spvickers commented 1 year ago

Thanks again for reporting this,. You are quite correct, the LTI version passed to the parseRoles method should be the version to be used for the roles being returned. I have committed the change to fix this.