SAK-48101 - Plus: TA comes across with access perms and is listed as a student in the GB (sakaiproject#11064)

* SAK-48101 - Plus: TA comes across with access perms and is listed as a student in the GB

Also incorporates:

SAK-48087 Add the ability to choose site template and LTI role mapping for each tenant on incoming launches
SAK-48117 Plus: Add a short description of plus tool

Co-authored-by: Adrian Fish <[email protected]>

Author: csev

basiclti/docs/LTIROLES.md

@@ -54,24 +54,6 @@ and the roles that Sakai provides via its Names and Roles Provisioning Service.
 When we reference "inbound" roles, we are referring to roles that are provided to
 Sakai when it is acting as an LTI provider.
 
-Per-Tool Outbound Role Mapping
-------------------------------
-
-There are special situations where a particular tool needs some very particular role
-mapping or your Sakai sites have additional roles beyond the out-of-the-box roles.
-When you are installing an LTI tool into Sakai, you have an option for mapping particular
-Sakai role to a particular LTI role using a set of mapping strings that specify a Sakai
-role and the corresponding LTI role:
-
-Teaching Assistant:Instructor;Librarian:Learner
-
-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;
-        Teaching Assistant:Learner,http://purl.imsglobal.org/vocab/lis/v2/membership#ContentDeveloper
-
 Outbound LTI Role Mapping
 -------------------------
 
@@ -134,6 +116,23 @@ cases or new roles arise in LTI and see common use, we can add them to the defau
 over time.   The overrides allow for a quick response when it might take a little while before
 we can fix Sakai and you get an upgraded version.
 
+Per-Tool Outbound Role Mapping
+------------------------------
+
+There are special situations where a particular tool needs some very particular role
+mapping or your Sakai sites have additional roles beyond the out-of-the-box roles.
+When you are installing an LTI tool into Sakai, you have an option for mapping a particular
+Sakai role to a particular LTI role using a set of mapping strings, like this:
+
+Teaching Assistant:Instructor;Librarian:Learner
+
+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;
+        Teaching Assistant:Learner,http://purl.imsglobal.org/vocab/lis/v2/membership#ContentDeveloper
+
 Inbound LTI Role Mapping
 ------------------------
 

basiclti/tsugi-util/src/java/org/tsugi/lti13/objects/LaunchJWT.java

@@ -91,7 +91,7 @@ public class LaunchJWT extends BaseJWT {
 
 	// This is in LaunchJWTs
 	@JsonProperty("nonce")
-    public String nonce;
+	public String nonce;
 
 	// Constructor
 	public LaunchJWT() {
@@ -133,4 +133,16 @@ public boolean isInstructor() {
 		return roles.contains(ROLE_INSTRUCTOR);
 	}
 
+	@JsonIgnore
+	public String getLTI11Roles() {
+		if ( roles == null ) return null;
+
+		StringBuilder roleStr = new StringBuilder();
+		for (String role : roles) {
+			if ( roleStr.length() > 0 ) roleStr.append(',');
+			roleStr.append(role);
+		}
+		return roleStr.toString();
+	}
+
 }

config/configuration/bundles/src/bundle/org/sakaiproject/config/bundle/default.sakai.properties

@@ -5835,7 +5835,7 @@ rubrics.integration.token-secret=12345678900909091234
 # Specify the realm id to be used to copy when making a new site if the plus.new.site.template
 # does not have a realm in response to the first Plus launch to a site or tool.
 # plus.new.site.realm
-# DEFAULT: !site.template
+# DEFAULT: !site.template.lti
 
 # Specify the default site type to be created when making a new site in response to the
 # first Plus launch to a site or tool.  This is a fallback value if Sakai Plus cannot

plus/README.md

@@ -52,8 +52,8 @@ running at least Sakai 23.
 
     https://trunk-mysql.nightly.sakaiproject.org
 
-Of course this server is reset every 24 hours so anything that you set up will vanish - and if
-you messJ anything up - your mistakes will convienently disappear.  If you want access to a server
+Of course nightly servers are reset every 24 hours so anything that you set up will vanish - and if
+you mess anything up - your mistakes will convienently disappear.  If you want access to a server
 for testing that will not be reset each evening, send an email to `plus at sakailms.org`.
 
 Enabling SakaiPlus in sakai.properties
@@ -62,9 +62,6 @@ Enabling SakaiPlus in sakai.properties
     # Needed for launches to work inside iframes
     sakai.cookieSameSite=none
 
-    lti.role.mapping.Instructor=maintain
-    lti.role.mapping.Student=access
-
     # Not enabled by default
     plus.provider.enabled=true
     plus.tools.allowed=sakai.resources:sakai.site
@@ -118,3 +115,92 @@ for different kinds of launches.
 It might be necessary to install SakaiPlus more than once so that it shows up in all the right placements
 in a particular LMS.
 
+SakaiPlus Tenants
+-----------------
+
+A SakaiPlus server can support many "tenants".  Each Learning System that you are plugging SakaiPlus into
+should have its own tenant.  In SakaiPlus, all data within a tenant is isolated (each tenant is a 'silo').
+This way you can have a multi-tenant SakaiPlus server to serve many different learning systems.  However
+it is also a quite typical use case to have one Enterprise LMS - say Canvas and one SakaiPlus server
+for the same school and to have a single Tenant entry in SakaiPlus for the Canvas system.
+
+You can create a "draft" tenant with a Title and Issuer and optionally a Registration Lock.  Once you have created
+a draft tenant, you can view the tenant to either start the LTI Dynamic Registration process or provide
+tool configuration to your calling learning system.
+
+You can view the documentation for LTI Dynamic Registration at:
+
+* [Dynamic Registration](https://www.imsglobal.org/spec/lti-dr/v1p0)
+
+Each tenant in SakaiPlus has a set of data:
+
+*Issuer*
+
+Issuer is different for each LMS, but it is usually a URL like "https://plus.sakalms.org" - with no trailing slash.
+Sometimes this will be the domain where the LMS is hosted. For some cloud-hosted providers, they use one
+issuer across all customers. This field is required.
+
+*Client ID*
+
+These are provided by the launching Learning system as part of tool registration.  If the Learning system
+supports LTI Dynamic Registration it will automatically populate this field.
+
+*Deployment ID*
+
+`Deployment ID` can be tricky.  For some systems it is the same for an entire system and is provided
+as part of Dynamic Registration.  For other systems a new `Deployent ID` is generated by each course.
+You can set the `Deployment ID` to `*` if you can accept any `Deployment ID` for a particular
+`Client ID`.  See the per-LMS installation instructions above for details.
+
+*Allowed Tools*
+
+This field is a colon-separated list of Sakai tool ids like "sakai.resources". There is a special
+"sakai.site" tool id which controls the availability of the "entire site" launch". A simple default
+for this is "sakai.site" or "sakai.site:sakai.resources: ...". This field is required.
+
+*New Window Tools*
+
+This field is a colon-separated list of Sakai tool ids which will be forced to always open in a
+ new window. The "sakai.site" is always launched in a new window. This is typically left blank unless
+it is known that a particular tool just does not work well in an iframe.  Or perhaps you are setting
+up a single tool server and want it to always be in a new window.
+
+*Trust Email*
+
+If the Learning system that is calling SakaiPlus for this tenant sends email, you *should* trust the
+email address to avoid creating multiple user records for each user in each site. Of you mark this tenant
+as 'trust email', and the calling system provides the email address of the user, multiple launches from
+multiple contexts will all be linked to the same user within this Tenant in SakaiPlus.
+
+*Site Template*
+
+This specifies an existing site like `!plussite` which will be copied to make new site when SakaiPlus
+receives an incoming site. This template site determines the defaoly tools that are added to the new
+SakaiPlus site.  The default is '!plussite' unless it is changed using the `plus.new.site.template` Sakai property.
+
+
+*Realm Template*
+
+This specifies an existing realm like `!site.template.lti` which will be copied to set the roles and
+permissions used when creating a new site when SakaiPlus receives an incoming site. The default is
+`!site.template.lti` unless it is changed using the `plus.new.site.realm` Sakai property.
+
+*Inbound Role Map*
+
+This field allows for overriding the default mapping from incoming LTI roles to Sakai roles.  See
+this documentation for detail on how role mapping works and the format for role mapping entries.
+
+* [Sakai to LTI Role Mapping](../basiclti/docs/LTIROLES.md)
+
+*Registration Lock*
+
+You set this field to "unlock" LTI Dynamic Registration for this tenant. It should only be set while
+performing dynamic registration and should be cleared after dynamic registration is complete.
+If the launching system does not support dynamic registration you will set these manually.
+
+The `LMS Keyset Url`, `LMS Authorization Url`, `LMS Token Url`, and `LMS Token Audience` fields are
+set up as part of tool registration with the calling learning system. If the system supports
+LTI Dynamic Registration these values should be set automatically.  The `LMS Token Audience`
+is left blank for most systems except for Desire2Learn.
+
+

plus/TODO.md

@@ -54,7 +54,7 @@ public interface PlusService {
 	public static final String PLUS_NEW_SITE_TEMPLATE_DEFAULT = "!plussite";
 	public static final String PLUS_NEW_SITE_TEMPLATE_BACKUP = "!worksite";
 	public static final String PLUS_NEW_SITE_REALM = "plus.new.site.realm";
-	public static final String PLUS_NEW_SITE_REALM_DEFAULT = "!site.template";
+	public static final String PLUS_NEW_SITE_REALM_DEFAULT = "!site.template.lti";
 	public static final String PLUS_NEW_SITE_TYPE = "plus.new.site.type";
 	public static final String PLUS_NEW_SITE_TYPE_DEFAULT = "project";
 

plus/api/src/main/java/org/sakaiproject/plus/api/PlusService.java

@@ -26,6 +26,9 @@
 import javax.persistence.JoinColumn;
 import javax.persistence.ManyToOne;
 import javax.persistence.Table;
+import javax.persistence.Lob;
+
+import org.apache.commons.lang3.StringUtils;
 
 import org.sakaiproject.springframework.data.PersistableEntity;
 
@@ -41,9 +44,6 @@
 @Data
 public class Membership extends BaseLTI implements PersistableEntity<Long> {
 
-	public static final Integer ROLE_LEARNER = 0;
-	public static final Integer ROLE_INSTRUCTOR = 1000;
-
 	@Id @GeneratedValue
 	@Column(name = "MEMBERSHIP_ID")
 	private Long id;
@@ -60,9 +60,16 @@ public class Membership extends BaseLTI implements PersistableEntity<Long> {
 	@ToString.Exclude
 	private Context context;
 
-	@Column(name = "ROLE", nullable = true)
-	private Integer role;
+	@Lob
+	@Column(name = "LTI_ROLES", nullable = true)
+	private String ltiRoles;
+
+	@Lob
+	@Column(name = "LTI_ROLES_OVERRIDE", nullable = true)
+	private String ltiRolesOverride;
 
-	@Column(name = "ROLE_OVERRIDE", nullable = true)
-	private Integer roleOverride;
+	public boolean isInstructor() {
+        if ( ltiRoles == null ) return false;
+        return StringUtils.containsIgnoreCase(ltiRoles, "instructor") || StringUtils.containsIgnoreCase(ltiRolesOverride, "instructor");
+    }
 }

plus/api/src/main/java/org/sakaiproject/plus/api/model/Membership.java

@@ -91,6 +91,16 @@ public class Tenant extends BaseLTI implements PersistableEntity<String> {
 	@Column(name = "VERBOSE")
 	private Boolean verbose = Boolean.FALSE;
 
+	@Column(name = "SITE_TEMPLATE", length = LENGTH_SAKAI_ID, nullable = true)
+	private String siteTemplate;
+
+	@Column(name = "REALM_TEMPLATE", length = LENGTH_SAKAI_ID, nullable = true)
+	private String realmTemplate;
+
+	@Lob
+	@Column(name = "INBOUND_ROLE_MAP", nullable = true)
+	private String inboundRoleMap;
+
 	@Column(name = "OIDC_AUTH", length = LENGTH_URI, nullable = true)
 	private String oidcAuth;
 

plus/api/src/main/java/org/sakaiproject/plus/api/model/Tenant.java

@@ -300,11 +300,8 @@ public Launch updateAll(LaunchJWT launchJWT, Tenant tenant)
 			membership = new Membership();
 			membership.setSubject(subject);
 			membership.setContext(context);
-			if (launchJWT.isInstructor() ) {
-				membership.setRole(Membership.ROLE_INSTRUCTOR);
-			} else {
-				membership.setRole(Membership.ROLE_LEARNER);
-			}
+			String ltiRoles = launchJWT.getLTI11Roles();
+			if ( StringUtils.isNotBlank(ltiRoles) ) membership.setLtiRoles(ltiRoles);
 			membership = membershipRepository.upsert(membership);
 		}
 
@@ -399,14 +396,8 @@ public Map<String,String> getPayloadFromLaunchJWT(Tenant tenant, LaunchJWT launc
 		payload.put(BasicLTIConstants.LIS_PERSON_NAME_FAMILY, launchJWT.family_name);
 		// payload.put(BasicLTIConstants.LIS_PERSON_NAME_MIDDLE, launchJWT.middle_name);
 
-		if ( launchJWT.roles != null ) {
-			StringBuilder roles = new StringBuilder();
-			for (String role : launchJWT.roles) {
-				if ( roles.length() > 0 ) roles.append(',');
-				roles.append(role);
-			}
-			if ( roles.length() > 0 ) payload.put(BasicLTIConstants.ROLES, roles.toString());
-		}
+		String ltiRoles = launchJWT.getLTI11Roles();
+		if ( StringUtils.isNotBlank(ltiRoles) ) payload.put(BasicLTIConstants.ROLES, ltiRoles);
 
 		// TODO: Ask for this in custom...
 		// payload.put(BasicLTIConstants.USER_IMAGE, );
@@ -703,11 +694,8 @@ public void syncSiteMemberships(String contextGuid, Site site) throws LTIExcepti
 				Membership membership = new Membership();
 				membership.setSubject(subject);
 				membership.setContext(context);
-				if (launchJWT.isInstructor() ) {
-					membership.setRole(Membership.ROLE_INSTRUCTOR);
-				} else {
-					membership.setRole(Membership.ROLE_LEARNER);
-				}
+				String ltiRoles = launchJWT.getLTI11Roles();
+				if ( StringUtils.isNotBlank(ltiRoles) ) membership.setLtiRoles(ltiRoles);
 				membership = membershipRepository.upsert(membership);
 
 				Map<String, String> payload = getPayloadFromLaunchJWT(tenant, launchJWT);

plus/impl/src/main/java/org/sakaiproject/plus/impl/PlusServiceImpl.java

@@ -66,15 +66,16 @@ public Membership upsert(Membership entity) {
 		if ( newEntity == null ) return save(entity);
 
 		boolean unchanged =
-			Objects.equals(entity.getRole(), newEntity.getRole())
-			&& Objects.equals(entity.getRoleOverride(), newEntity.getRoleOverride())
+			Objects.equals(entity.getLtiRoles(), newEntity.getLtiRoles())
+			&& Objects.equals(entity.getLtiRolesOverride(), newEntity.getLtiRolesOverride())
 		;
 
 		if ( unchanged ) return newEntity;
 
 		// Do the UPDATE variant of UPSERT
-		newEntity.setRole(entity.getRole());
-		newEntity.setRoleOverride(entity.getRoleOverride());
+		newEntity.setLtiRoles(entity.getLtiRoles());
+		newEntity.setLtiRolesOverride(entity.getLtiRolesOverride());
+		newEntity.setLtiRoles(entity.getLtiRoles());
 		return save(newEntity);
 	}