Skip to content

Commit

Permalink
SAK-40921 - Fix content Item / deep link flow (sakaiproject#6242)
Browse files Browse the repository at this point in the history 
* SAK-40921 - Fix content Item / deep link flow

* SAK-40921 - Change ltiLink -> ltiResourceLink more faffing around

* SAK-40921 - Issues identified working with Bb

* SAK-40921 - Fix updated data claim string

* SAK-40921 - Fix some of the debug logic in the logic

* SAK-40921 - Move the OIDC flow to its own servlet to avoid RequestFilter

* SAK-40921 - Remove unneeded dependencies

* SAK-40921 - Add alg for Martin / TurnitIn

* SAK-40921 - Switch to the new deployment_id / subj / lti1_1_user_id pattern

* SAK-40921 - Reviewer comments
  • Loading branch information
@csev
csev authored Nov 16, 2018
1 parent 2454540 commit 5cc139e
Show file tree
Hide file tree
Showing 21 changed files with 375 additions and 55 deletions.
29 changes: 15 additions & 14 deletions basiclti/basiclti-blis/src/java/org/sakaiproject/lti13/LTI13Servlet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
*/
package org.sakaiproject.lti13;

import com.fasterxml.jackson.core.JsonProcessingException;
import io.jsonwebtoken.Jws;

import java.io.IOException;
Expand Down Expand Up @@ -54,7 +53,6 @@
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Properties;
import java.util.Set;
import java.util.TreeMap;

Expand All @@ -64,11 +62,8 @@
import org.sakaiproject.authz.api.Member;
import org.sakaiproject.authz.api.Role;
import org.sakaiproject.basiclti.util.LegacyShaUtil;
import org.sakaiproject.basiclti.util.SakaiBLTIUtil;
import static org.sakaiproject.basiclti.util.SakaiBLTIUtil.getInt;
import static org.sakaiproject.basiclti.util.SakaiBLTIUtil.getLongKey;
import static org.sakaiproject.basiclti.util.SakaiBLTIUtil.getRB;
import static org.sakaiproject.basiclti.util.SakaiBLTIUtil.postError;
import org.sakaiproject.basiclti.util.SakaiBLTIUtil;
import static org.sakaiproject.basiclti.util.SakaiBLTIUtil.LTI13_PATH;
import static org.sakaiproject.basiclti.util.SakaiBLTIUtil.getOurServerUrl;
Expand All @@ -78,7 +73,6 @@
import org.sakaiproject.lti.api.LTIService;
import static org.sakaiproject.lti13.LineItemUtil.getLineItem;

import org.tsugi.http.HttpUtil;
import org.tsugi.basiclti.BasicLTIUtil;
import org.tsugi.jackson.JacksonUtil;
import org.tsugi.lti13.LTI13KeySetUtil;
Expand All @@ -101,9 +95,9 @@
import org.sakaiproject.tool.cover.SessionManager;
import org.sakaiproject.user.api.User;
import org.sakaiproject.user.cover.UserDirectoryService;
import org.apache.commons.lang.StringUtils;
import org.tsugi.ags2.objects.LineItem;
import org.tsugi.ags2.objects.Result;
import org.tsugi.basiclti.XMLMap;
import org.tsugi.lti13.objects.LaunchLIS;

/**
Expand Down Expand Up @@ -204,6 +198,7 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) t
return;
}

// TODO: Remove this after transition to new servlet is complete.
// /imsblis/lti13/oidc_auth?state=42&login_hint=/access/basiclti/site/92e..e8e67/content:6
if (parts.length == 4 && "oidc_auth".equals(parts[3]) ) {
handleOIDCAuthorization(request, response);
Expand Down Expand Up @@ -1061,7 +1056,7 @@ protected LineItem getLineItemFilter(HttpServletRequest request)
String lti_link_id = request.getParameter("lti_link_id");
if ( lti_link_id != null && lti_link_id.length() > 0 ) {
found = true;
retval.ltiLinkId = lti_link_id;
retval.resourceLinkId = lti_link_id;
}
String resource_id = request.getParameter("resource_id");
if ( resource_id != null && resource_id.length() > 0 ) {
Expand Down Expand Up @@ -1548,24 +1543,30 @@ private void handleLineItemsDelete(String signed_placement, String lineItem, Htt
private void handleOIDCAuthorization(HttpServletRequest request, HttpServletResponse response) throws IOException {

String state = (String) request.getParameter("state");
if ( state != null && state.trim().length() < 1 ) state = null;
state = StringUtils.trimToNull(state);

String login_hint = (String) request.getParameter("login_hint");
if ( login_hint != null && login_hint.trim().length() < 1 ) state = null;
if ( StringUtils.isEmpty(login_hint) ) state = null;

String nonce = (String) request.getParameter("nonce");
nonce = StringUtils.trimToNull(nonce);

if ( state == null || login_hint == null ) {
LTI13Util.return400(response, "Missing login_hint or state parameter");
if ( state == null || login_hint == null || nonce == null ) {
LTI13Util.return400(response, "Missing login_hint, nonce or state parameter");
log.error("Missing login_hint or state parameter");
return;
}

if ( login_hint.contains("&") || login_hint.contains("?") || ! login_hint.startsWith("/access/basiclti/site/") ) {
if ( ! login_hint.startsWith("/access/basiclti/site/") ) {
LTI13Util.return400(response, "Bad format for login_hint");
log.error("Bad format for login_hint");
return;
}

String redirect = login_hint + "?state=" + java.net.URLEncoder.encode(state);
String redirect = login_hint;
redirect += ( redirect.contains("?") ? "&" : "?");
redirect += "state=" + java.net.URLEncoder.encode(state);
redirect += "&nonce=" + java.net.URLEncoder.encode(nonce);
log.debug("redirect={}", redirect);

try {
Expand Down
6 changes: 3 additions & 3 deletions basiclti/basiclti-blis/src/java/org/sakaiproject/lti13/LineItemUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ public static List<LineItem> getLineItemsForTool(String signed_placement, Site s
LineItem item = getLineItem(signed_placement, gAssignment);

if ( filter != null ) {
if ( filter.ltiLinkId != null && ! filter.ltiLinkId.equals(item.ltiLinkId)) continue;
if ( filter.resourceLinkId != null && ! filter.resourceLinkId.equals(item.resourceLinkId)) continue;
if ( filter.resourceId != null && ! filter.resourceId.equals(item.resourceId)) continue;
if ( filter.tag != null && ! filter.tag.equals(item.tag)) continue;
}
Expand Down Expand Up @@ -477,7 +477,7 @@ public static List<LineItem> getPreCreatedLineItems(Site site, Long tool_id, Lin
Map<String, Object> content = (Map) i.next();
LineItem item = getLineItem(content);
if ( filter != null ) {
if ( filter.ltiLinkId != null && ! filter.ltiLinkId.equals(item.ltiLinkId)) continue;
if ( filter.resourceLinkId != null && ! filter.resourceLinkId.equals(item.resourceLinkId)) continue;
if ( filter.resourceId != null && ! filter.resourceId.equals(item.resourceId)) continue;
if ( filter.tag != null && ! filter.tag.equals(item.tag)) continue;
}
Expand All @@ -497,7 +497,7 @@ public static LineItem getLineItem(Map<String, Object> content) {
}
LineItem li = new LineItem();
li.label = (String) content.get(LTIService.LTI_TITLE);
li.ltiLinkId = resource_link_id;
li.resourceLinkId = resource_link_id;
if ( context_id != null && signed_placement != null ) {
li.id = getOurServerUrl() + LTI13_PATH + "lineitem/" + signed_placement;
}
Expand Down
50 changes: 36 additions & 14 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 @@ -106,6 +106,7 @@
import org.sakaiproject.service.gradebook.shared.CommentDefinition;

import net.oauth.OAuth;
import org.apache.commons.lang.StringUtils;

import org.apache.commons.math3.util.Precision;
import org.sakaiproject.exception.IdUnusedException;
Expand Down Expand Up @@ -854,7 +855,8 @@ public static String[] postLaunchHTML(String descriptor, String contextId, Strin

// This must return an HTML message as the [0] in the array
// If things are successful - the launch URL is in [1]
public static String[] postLaunchHTML(Map<String, Object> content, Map<String, Object> tool, String state, LTIService ltiService, ResourceLoader rb) {
public static String[] postLaunchHTML(Map<String, Object> content, Map<String, Object> tool,
String state, String nonce, LTIService ltiService, ResourceLoader rb) {
if (content == null) {
return postError("<p>" + getRB(rb, "error.content.missing", "Content item is missing or improperly configured.") + "</p>");
}
Expand Down Expand Up @@ -1002,6 +1004,7 @@ public static String[] postLaunchHTML(Map<String, Object> content, Map<String, O

setProperty(toolProps, "launch_url", launch_url);
setProperty(toolProps, "state", state); // So far LTI 1.3 only
setProperty(toolProps, "nonce", nonce); // So far LTI 1.3 only

setProperty(toolProps, LTIService.LTI_SECRET, secret);
setProperty(toolProps, "key", key);
Expand Down Expand Up @@ -1427,7 +1430,7 @@ public static DeepLinkResponse getDeepLinkFromToken(Map<String, Object> tool, St
* successful - the launch URL is in [1]
*/
public static String[] postContentItemSelectionRequest(Long toolKey, Map<String, Object> tool,
String state, ResourceLoader rb, String contentReturn, Properties dataProps) {
String state, String nonce, ResourceLoader rb, String contentReturn, Properties dataProps) {
if (tool == null) {
return postError("<p>" + getRB(rb, "error.tool.missing", "Tool is missing or improperly configured.") + "</p>");
}
Expand Down Expand Up @@ -1555,6 +1558,7 @@ public static String[] postContentItemSelectionRequest(Long toolKey, Map<String,
Properties toolProps = new Properties();
toolProps.put("launch_url", launch_url);
setProperty(toolProps, "state", state); // So far LTI 1.3 only
setProperty(toolProps, "nonce", nonce); // So far LTI 1.3 only
toolProps.put(LTIService.LTI_DEBUG, dodebug ? "1" : "0");

Map<String, Object> content = null;
Expand Down Expand Up @@ -1790,16 +1794,22 @@ public static String[] postLaunchJWT(Properties toolProps, Properties ltiProps,
lj.message_type = LaunchJWT.MESSAGE_TYPE_DEEP_LINK;
deepLink = true;
}
lj.launch_url = launch_url; // The actual launch URL
lj.target_link_uri = launch_url; // The actual launch URL
lj.launch_presentation.css_url = ltiProps.getProperty("launch_presentation_css_url");
lj.locale = ltiProps.getProperty("launch_presentation_locale");
lj.launch_presentation.return_url = ltiProps.getProperty("launch_presentation_return_url");
lj.issuer = getOurServerUrl();
lj.audience = client_id;
lj.deployment_id = org_guid;
lj.subject = ltiProps.getProperty("user_id");
String deployment_id = ServerConfigurationService.getString("lti13.deployment_id", "1");
lj.deployment_id = deployment_id;
String subject = getOurServerUrl();
if ( ! "1".equals(deployment_id) ) {
subject += "/deployment/" + deployment_id;
}
lj.subject = subject + "/user/" + ltiProps.getProperty("user_id");
lj.lti1_1_user_id = (String) ltiProps.getProperty("user_id");
lj.name = ltiProps.getProperty("lis_person_name_full");
lj.nonce = new Long(System.currentTimeMillis()) + "_42";
lj.nonce = toolProps.getProperty("nonce");
lj.email = ltiProps.getProperty("lis_person_contact_email_primary");
lj.issued = new Long(System.currentTimeMillis() / 1000L);
lj.expires = lj.issued + 3600L;
Expand Down Expand Up @@ -1899,7 +1909,7 @@ public static String[] postLaunchJWT(Properties toolProps, Properties ltiProps,
Extra fields for DeepLink
lti_message_type=ContentItemSelectionRequest
accept_copy_advice=false
accept_media_types=application/vnd.ims.lti.v1.ltilink
accept_media_types=application/vnd.ims.lti.v1.ltiResourceLink
accept_multiple=false
accept_presentation_document_targets=iframe,window
accept_unsigned=true
Expand All @@ -1909,7 +1919,7 @@ public static String[] postLaunchJWT(Properties toolProps, Properties ltiProps,
data={"remember":"always bring a towel"}
"deep_link_return_url": "https://platform.example/deep_links",
"accept_types": ["link", "file", "html", "ltiLink", "image"],
"accept_types": ["link", "file", "html", "ltiResourceLink", "image"],
"accept_media_types": "image/:::asterisk:::,text/html",
"accept_presentation_document_targets": ["iframe", "window", "embed"],
"accept_multiple": true,
Expand All @@ -1922,7 +1932,12 @@ public static String[] postLaunchJWT(Properties toolProps, Properties ltiProps,
if ( deepLink ) {
DeepLink ci = new DeepLink();
// accept_copy_advice is not in deep linking - files are to be copied - images maybe
ci.accept_media_types = ltiProps.getProperty("accept_media_types");
String accept_media_types = ltiProps.getProperty("accept_media_types");
if ( ContentItem.MEDIA_LTILINKITEM.equals(accept_media_types) ) {
ci.accept_types.add(DeepLink.ACCEPT_TYPE_LTILINK);
} else {
ci.accept_media_types = ltiProps.getProperty("accept_media_types");
}
ci.accept_multiple = "true".equals(ltiProps.getProperty("accept_multiple"));
String target = ltiProps.getProperty("accept_presentation_document_targets");
if ( target != null ) {
Expand All @@ -1931,10 +1946,11 @@ public static String[] postLaunchJWT(Properties toolProps, Properties ltiProps,
ci.accept_presentation_document_targets.add(piece);
}
}

// Accept_unsigned is not in DeepLinking - they are signed JWTs
ci.auto_create = "true".equals(ltiProps.getProperty("auto_create"));
// can_confirm is not there
ci.deep_link_return_url = ltiProps.getProperty("content_item_return_url");
ci.deep_link_return_url = ltiProps.getProperty(BasicLTIConstants.CONTENT_ITEM_RETURN_URL);
ci.data = ltiProps.getProperty("data");
lj.deep_link = ci;
}
Expand All @@ -1961,21 +1977,27 @@ public static String[] postLaunchJWT(Properties toolProps, Properties ltiProps,
}

String state = toolProps.getProperty("state");
if ( state != null && state.trim().length() < 1 ) state = null;
state = StringUtils.trimToNull(state);

String lti13_oidc_redirect = toNull((String) tool.get(LTIService.LTI13_OIDC_REDIRECT));
if ( lti13_oidc_redirect != null ) launch_url = lti13_oidc_redirect;

String html = "<form action=\"" + launch_url + "\" method=\"POST\">\n"
String form_id = java.util.UUID.randomUUID().toString();
String html = "<form action=\"" + launch_url + "\" id=\""+ form_id + "\" method=\"POST\">\n"
+ " <input type=\"hidden\" name=\"id_token\" value=\"" + BasicLTIUtil.htmlspecialchars(jws) + "\" />\n";

if ( state != null ) {
html += " <input type=\"hidden\" name=\"state\" value=\"" + BasicLTIUtil.htmlspecialchars(state) + "\" />\n";
}

html += " <input type=\"submit\" value=\"Go!\" />\n</form>\n";
if ( dodebug ) {
html += " <input type=\"submit\" value=\"Proceed with LTI 1.3 Launch\" />\n</form>\n";
}
html += " </form>\n";

if (dodebug) {
if ( ! dodebug ) {
html += "<script>\n document.getElementById(\"" + form_id + "\").submit();\n</script>\n";
} else {
html += "<p>\n--- Unencoded JWT:<br/>"
+ BasicLTIUtil.htmlspecialchars(ljs)
+ "</p>\n<p>\n--- State:<br/>"
Expand Down
34 changes: 24 additions & 10 deletions ...ti/basiclti-impl/src/java/org/sakaiproject/basiclti/impl/BasicLTISecurityServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,15 +262,22 @@ private void doSplash(HttpServletRequest req, HttpServletResponse res, String sp
login_hint required, Hint to the Authorization Server about the login
identifier the End-User might use to log in (if necessary).
*/
private void redirectOIDC(HttpServletRequest req, HttpServletResponse res, String oidc_endpoint, ResourceLoader rb)
private void redirectOIDC(HttpServletRequest req, HttpServletResponse res, Map<String, Object> tool, String oidc_endpoint, ResourceLoader rb)
{
// req.getRequestURL()=http://localhost:8080/access/basiclti/site/85fd092b-1755-4aa9-8abc-e6549527dce0/content:0
// req.getRequestURI()=/access/basiclti/site/85fd092b-1755-4aa9-8abc-e6549527dce0/content:0
String login_hint = req.getRequestURI().toString();
// TODO: Encrypt the login hint
String query_string = req.getQueryString();
if ( StringUtils.isNotEmpty(query_string)) {
login_hint = login_hint + "?" + query_string;
}
String launch_url = (String) tool.get(LTIService.LTI_LAUNCH);
String redirect = oidc_endpoint;
redirect += "?iss=" + java.net.URLEncoder.encode(SakaiBLTIUtil.getOurServerUrl());
redirect += "&login_hint=" + java.net.URLEncoder.encode(login_hint);
if ( StringUtils.isNotEmpty(launch_url)) {
redirect += "&target_link_uri=" + java.net.URLEncoder.encode(launch_url);
}
try {
res.sendRedirect(redirect);
} catch (IOException unlikely) {
Expand Down Expand Up @@ -346,12 +353,16 @@ else if ( refId.startsWith("tool:") && refId.length() > 5 )
tool.put(LTIService.LTI_SITE_ID, ref.getContext());
}
String state = req.getParameter("state");
String nonce = req.getParameter("nonce");

String oidc_endpoint = (String) tool.get(LTIService.LTI13_OIDC_ENDPOINT);
log.debug("State={} oidc_endpoint={}",state, oidc_endpoint);
if (StringUtils.isNotBlank(oidc_endpoint) && StringUtils.isEmpty(state)) {
redirectOIDC(req, res, oidc_endpoint, rb);
log.debug("State={} nonce={} oidc_endpoint={}",state, nonce, oidc_endpoint);

if (StringUtils.isNotBlank(oidc_endpoint) &&
( StringUtils.isEmpty(state) || StringUtils.isEmpty(state) ) ) {
redirectOIDC(req, res, tool, oidc_endpoint, rb);
}
retval = SakaiBLTIUtil.postContentItemSelectionRequest(toolKey, tool, state, rb, contentReturn, propData);
retval = SakaiBLTIUtil.postContentItemSelectionRequest(toolKey, tool, state, nonce, rb, contentReturn, propData);
}
}
else if ( refId.startsWith("content:") && refId.length() > 8 )
Expand Down Expand Up @@ -402,12 +413,15 @@ else if ( refId.startsWith("content:") && refId.length() > 8 )
return;
}
String state = req.getParameter("state");
String nonce = req.getParameter("nonce");

String oidc_endpoint = (String) tool.get(LTIService.LTI13_OIDC_ENDPOINT);
log.debug("State={} oidc_endpoint={}",state, oidc_endpoint);
if (StringUtils.isNotBlank(oidc_endpoint) && StringUtils.isEmpty(state)) {
redirectOIDC(req, res, oidc_endpoint, rb);
log.debug("State={} nonce={} oidc_endpoint={}",state, nonce, oidc_endpoint);
if (StringUtils.isNotBlank(oidc_endpoint) &&
(StringUtils.isEmpty(state) || StringUtils.isEmpty(nonce) ) ) {
redirectOIDC(req, res, tool, oidc_endpoint, rb);
}
retval = SakaiBLTIUtil.postLaunchHTML(content, tool, state, ltiService, rb);
retval = SakaiBLTIUtil.postLaunchHTML(content, tool, state, nonce, ltiService, rb);
}
else if (refId.startsWith("export:") && refId.length() > 7)
{
Expand Down
13 changes: 13 additions & 0 deletions basiclti/basiclti-oidc/maven.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0" encoding="UTF-8"?>

<project
xmlns:j="jelly:core"
xmlns:license="license"
xmlns:util="jelly:util"
xmlns:ant="jelly:ant"
xmlns:artifact="artifact"
xmlns:maven="jelly:maven"
default="sakai:build">

</project>

Loading

0 comments on commit 5cc139e

Please sign in to comment.