Skip to content

Commit

Permalink
SAK-48270 LTI ignore incoming institutional roles on context launch (s…
Browse files Browse the repository at this point in the history 
…akaiproject#11161)

* SAK-48270 - Ignore incoming institutional roles on context launch

* Just get rid of those insitutional roles completely

* Fix unit test and remove "Ignore"

* Update basiclti/basiclti-common/src/test/org/sakaiproject/basiclti/util/SakaiBLTIUtilTest.java

Co-authored-by: Earle Nietzel <[email protected]>

Co-authored-by: Earle Nietzel <[email protected]>
  • Loading branch information
@csev @ern
csev and ern authored Jan 10, 2023
1 parent 0f64b29 commit 054a17b
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 154 deletions.
28 changes: 2 additions & 26 deletions basiclti/basiclti-common/src/java/org/sakaiproject/basiclti/util/SakaiBLTIUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,18 +210,9 @@ public class SakaiBLTIUtil {
// A blank *is* part of the Sakai role and *is not* part of the LTI role
"Teaching Assistant:TeachingAssistant,http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor#TeachingAssistant;" +
// !site.template.lti roles - The simplest mapping :)
"Faculty:Faculty,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Faculty;" +
"Member:Member,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Member;" +
"Learner:Learner,http://purl.imsglobal.org/vocab/lis/v2/membership#Learner;" +
"Mentor:Mentor,http://purl.imsglobal.org/vocab/lis/v2/membership#Mentor;" +
"Staff:Staff,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Staff;" +
"Alumni:Alumni,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Alumni;" +
"ProspectiveStudent:ProspectiveStudent,http://purl.imsglobal.org/vocab/lis/v2/institution/person#ProspectiveStudent;" +
"ContentDeveloper:ContentDeveloper,http://purl.imsglobal.org/vocab/lis/v2/membership#ContentDeveloper;" +
"Guest:Guest,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Guest;" +
"Other:Other,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Other;" +
"Administrator:Administrator,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator;" +
"Observer:Observer,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Observer;"
"ContentDeveloper:ContentDeveloper,http://purl.imsglobal.org/vocab/lis/v2/membership#ContentDeveloper;"
;

public static final String LTI_INBOUND_ROLE_MAP = "lti.inbound.role.map";
Expand All @@ -235,22 +226,7 @@ public class SakaiBLTIUtil {

"http://purl.imsglobal.org/vocab/lis/v2/membership#Manager=Manager,Guest,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/membership#Member=Member,Guest,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/membership#Officer=Officer,Guest,Student,access;" +

"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator=Instructor,maintain;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Faculty=Faculty,Instructor,maintain;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Guest=Guest,Learner,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#None=None,Learner,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Other=Other,Learner,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Staff=Staff,Learner,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Student=Learner,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Alumni=Alumni,Guest,Learner,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Instructor=Instructor,maintain;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Learner=Learner,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Member=Member,Learner,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Mentor=Mentor,Learner,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#Observer=Observer,Guest,Learner,Student,access;" +
"http://purl.imsglobal.org/vocab/lis/v2/institution/person#ProspectiveStudent=ProspectiveStudent,Guest,Learner,Student,access;"
"http://purl.imsglobal.org/vocab/lis/v2/membership#Officer=Officer,Guest,Student,access;"
;

public static final String LTI_LEGACY_ROLE_MAP = "lti.legacy.role.map";
Expand Down
33 changes: 17 additions & 16 deletions basiclti/basiclti-common/src/test/org/sakaiproject/basiclti/util/SakaiBLTIUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,15 +461,13 @@ public void testDefaultRoleMap() {
Map<String, String> roleMap = SakaiBLTIUtil.convertOutboundRoleMapPropToMap(SakaiBLTIUtil.LTI_OUTBOUND_ROLE_MAP_DEFAULT);

assertTrue(roleMap instanceof Map);
assertEquals(18, roleMap.size());
assertEquals(9, roleMap.size());
assertTrue(roleMap.get("Yada") == null);
assertEquals(roleMap.get("access"), "Learner,http://purl.imsglobal.org/vocab/lis/v2/membership#Learner");
assertEquals(roleMap.get("maintain"), "Instructor,http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor");
assertEquals(roleMap.get("Student"), "Learner,http://purl.imsglobal.org/vocab/lis/v2/membership#Learner");
assertEquals(roleMap.get("Learner"), "Learner,http://purl.imsglobal.org/vocab/lis/v2/membership#Learner");
assertEquals(roleMap.get("Instructor"), "Instructor,http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor");
assertEquals(roleMap.get("Guest"), "Guest,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Guest");
assertEquals(roleMap.get("Observer"), "Observer,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Observer");
// Blanks are really important below
assertEquals(roleMap.get("Teaching Assistant"), "TeachingAssistant,http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor#TeachingAssistant");
}
Expand All @@ -483,7 +481,7 @@ public void testInboundRoleMap() {

Map<String, List<String>> roleMap = SakaiBLTIUtil.convertInboundRoleMapPropToMap(SakaiBLTIUtil.LTI_INBOUND_ROLE_MAP_DEFAULT);
assertTrue(roleMap instanceof Map);
assertEquals(roleMap.size(), 23);
assertEquals(roleMap.size(), 9);
assertTrue(roleMap.get("Yada") == null);

List<String> roleList = roleMap.get("http://purl.imsglobal.org/vocab/lis/v2/membership#Learner");
Expand Down Expand Up @@ -532,6 +530,7 @@ public static String mapOutboundRole(String sakaiRole, String toolOutboundMapStr
return SakaiBLTIUtil.mapOutboundRole(sakaiRole, toolRoleMap, propRoleMap, defaultRoleMap, propLegacyMap, defaultLegacyMap);
}

// https://www.imsglobal.org/spec/lti/v1p3/#role-vocabularies
@Test
public void testOutbound() {
String toolProp = "ToolI:Instructor;ToolM:Instructor,Learner;ToolA:"+BasicLTIConstants.MEMBERSHIP_ROLE_INSTITUTION_ADMIN+";";
Expand All @@ -555,16 +554,10 @@ public void testOutbound() {
assertTrue(imsRole.contains("Instructor"));
assertTrue(imsRole.contains("Administrator"));

imsRole = mapOutboundRole("Guest", toolProp);
assertEquals("Guest,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Guest", imsRole);

// Extra from properties
imsRole = mapOutboundRole("Dude", toolProp);
assertEquals("Dude,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Abides", imsRole);

imsRole = mapOutboundRole("Staff", toolProp);
assertEquals("Staff,Dude,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Staff", imsRole);

// Tool maps to legacy Instructor - upconverted
imsRole = mapOutboundRole("ToolI", toolProp);
assertEquals("http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor", imsRole);
Expand Down Expand Up @@ -645,11 +638,12 @@ public void testInbound() {
assertEquals(s, sakaiRole);
}

// Institution but not context roles from https://www.imsglobal.org/spec/lti/v1p3/#role-vocabularies
// We don't do person/Student as it maps to Learner
for (String s : "Faculty,Guest,None,Other,Staff,Alumni,Observer,ProspectiveStudent".split(",") ) {
sakaiRole = mapInboundRole("http://purl.imsglobal.org/vocab/lis/v2/institution/person#" + s, ltiRoles, null);
assertEquals(s, sakaiRole);
// We ignore institution roles from https://www.imsglobal.org/spec/lti/v1p3/#role-vocabularies
for (String s : "Faculty,Guest,None,Other,Staff,Person,Student,Alumni,Observer,ProspectiveStudent".split(",") ) {
String ltiRole = "http://purl.imsglobal.org/vocab/lis/v2/institution/person#" + s;
sakaiRole = mapInboundRole(ltiRole, ltiRoles, null);
if ( sakaiRole != null ) log.warn("LTI Role [{}] should map to Ignore or null instead of [{}]", ltiRole, sakaiRole);
assertTrue(sakaiRole==null);
}
}

Expand All @@ -658,7 +652,14 @@ public void testInception() {
String imsRole;
String sakaiRole;

for (String roundTrip : "Instructor,Learner,Guest,Teaching Assistant,Mentor,Alumni,ProspectiveStudent".split(",") ) {
// Institutional roles - Just say no
for (String roundTrip : "Faculty,Member,Alumni,ProspectiveStudent,Guest,Other".split(",") ) {
imsRole = mapOutboundRole(roundTrip, null);
assertTrue(imsRole==null);
}

// Institutional roles do not round trip - Faculty, Member, Staff, Alumni, ProspectiveStudent, Guest, Other
for (String roundTrip : "Instructor,Learner,Teaching Assistant,Mentor".split(",") ) {
imsRole = mapOutboundRole(roundTrip, null);
sakaiRole = mapInboundRole(imsRole, ltiRoles, null);
assertEquals(roundTrip, sakaiRole);
Expand Down
31 changes: 3 additions & 28 deletions basiclti/docs/LTIROLES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,9 @@ But here is a copy (may be out of date) so that we can talk about them.
Instructor:Instructor,http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor;
Student:Learner,http://purl.imsglobal.org/vocab/lis/v2/membership#Learner;
Teaching Assistant:TeachingAssistant,http://purl.imsglobal.org/vocab/lis/v2/membership#Instructor#TeachingAssistant;
Faculty:Faculty,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Faculty;
Member:Member,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Member;
Learner:Learner,http://purl.imsglobal.org/vocab/lis/v2/membership#Learner;
Mentor:Mentor,http://purl.imsglobal.org/vocab/lis/v2/membership#Mentor;
Staff:Staff,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Staff;
Alumni:Alumni,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Alumni;
ProspectiveStudent:ProspectiveStudent,http://purl.imsglobal.org/vocab/lis/v2/institution/person#ProspectiveStudent;
ContentDeveloper:ContentDeveloper,http://purl.imsglobal.org/vocab/lis/v2/membership#ContentDeveloper;
Guest:Guest,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Guest;
Other:Other,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Other;
Administrator:Administrator,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator;
Observer:Observer,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Observer;

Newlines added to the `admin` entry for readibility - they are all one line, comma separated.

Expand All @@ -103,7 +94,7 @@ and map it back and forth :)
You can add Sakai-side default mappings or override any or all of the mappings above through
(you guessed it) a Sakai property:

lti.outbound.role.map=Sakaiger:Mascot,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Mascot;Friend...
lti.outbound.role.map=Sakaiger:Mascot,http://purl.imsglobal.org/vocab/lis/v2/membership#Mascot;Friend...

These mappings in `sakai.properties` are one *very long* line with semicolon separator.

Expand All @@ -130,7 +121,7 @@ These examples are using the LTI 1.1 "short form" for the LTI roles.
But you can also specify the full length roles as well, and you can specify more than one LTI
role be the result of the role mapping. (blanks and linebreaks added for readablility):

Instructor:Instructor,http://purl.imsglobal.org/vocab/lis/v2/institution/person#Faculty;
Instructor:Instructor,http://purl.imsglobal.org/vocab/lis/v2/membership#Faculty;
Teaching Assistant:Learner,http://purl.imsglobal.org/vocab/lis/v2/membership#ContentDeveloper

Inbound LTI Role Mapping
Expand All @@ -157,22 +148,6 @@ specfic (site.template).
http://purl.imsglobal.org/vocab/lis/v2/membership#Member=Learner,Student,access
http://purl.imsglobal.org/vocab/lis/v2/membership#Officer=Learner,Student,access

http://purl.imsglobal.org/vocab/lis/v2/institution/person#Administrator=Instructor,maintain
http://purl.imsglobal.org/vocab/lis/v2/institution/person#Faculty=Instructor,maintain
http://purl.imsglobal.org/vocab/lis/v2/institution/person#Guest=Guest,Learner,Student,access
http://purl.imsglobal.org/vocab/lis/v2/institution/person#None=Guest,Learner,Student,access
http://purl.imsglobal.org/vocab/lis/v2/institution/person#Other=Other,Learner,Student,access
http://purl.imsglobal.org/vocab/lis/v2/institution/person#Staff=Staff,Learner,Student,access
http://purl.imsglobal.org/vocab/lis/v2/institution/person#Student=Learner,Student,access

http://purl.imsglobal.org/vocab/lis/v2/institution/person#Alumni=Alumni,Guest,Learner,Student,access
http://purl.imsglobal.org/vocab/lis/v2/institution/person#Instructor=Instructor,maintain
http://purl.imsglobal.org/vocab/lis/v2/institution/person#Learner=Learner,Student,access
http://purl.imsglobal.org/vocab/lis/v2/institution/person#Member=Member,Learner,Student,access
http://purl.imsglobal.org/vocab/lis/v2/institution/person#Mentor=Mentor,Learner,Student,access
http://purl.imsglobal.org/vocab/lis/v2/institution/person#Observer=Observer,Guest,Learner,Student,access
http://purl.imsglobal.org/vocab/lis/v2/institution/person#ProspectiveStudent=ProspectiveStudent,Guest,Learner,Student,access

Note that `TeachingAssistant` is a little weird because it is a sub-type of `Instructor` and the
use of a blank within Sakai and no blank in LTI.

Expand All @@ -182,7 +157,7 @@ The actual default list is stored in
You can add new default mappings or override any or all of the mappings above through
Sakai property:

lti.inbound.role.map=http://purl.imsglobal.org/vocab/lis/v2/institution/person#Mascot=Learner,Student,access;http://...
lti.inbound.role.map=http://purl.imsglobal.org/vocab/lis/v2/membership#Mascot=Learner,Student,access;http://...

These mappings in `sakai.properties` are one *very long* line with semicolon separator.

Expand Down
Loading

0 comments on commit 054a17b

Please sign in to comment.