SAK-42242 - Improve error checking / messages (#7193)

sakaiproject · Jul 31, 2019 · 2a483a0 · 2a483a0
1 parent 918112b
commit 2a483a0
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 1 deletion.
diff --git a/basiclti/basiclti-docs/resources/docs/Advantage_TestPlan.xls b/basiclti/basiclti-docs/resources/docs/Advantage_TestPlan.xls
diff --git a/...ti/basiclti-impl/src/java/org/sakaiproject/basiclti/impl/BasicLTISecurityServiceImpl.java b/...ti/basiclti-impl/src/java/org/sakaiproject/basiclti/impl/BasicLTISecurityServiceImpl.java
@@ -300,7 +300,7 @@ private void redirectOIDC(HttpServletRequest req, HttpServletResponse res,
 	*/
 	private void handleLTI112(HttpServletRequest req, HttpServletResponse res, Map<String, Object> tool)
 	{
-		String default_launch_type = ServerConfigurationService.getString(SakaiBLTIUtil.BASICLTI_LTI11_LAUNCH_TYPE, 
+		String default_launch_type = ServerConfigurationService.getString(SakaiBLTIUtil.BASICLTI_LTI11_LAUNCH_TYPE,
 				SakaiBLTIUtil.BASICLTI_LTI11_LAUNCH_TYPE_DEFAULT);
 		Long lti11_launch_type = SakaiBLTIUtil.getLongKey(tool.get(LTIService.LTI11_LAUNCH_TYPE));
 
@@ -329,6 +329,23 @@ private void handleLTI112(HttpServletRequest req, HttpServletResponse res, Map<S
 		}
 	}
 
+	/**
+	 * Do some sanity checking on the aunch data to make sure we have enough to accomplish the launch
+	 */
+	private boolean sanityCheck(HttpServletRequest req, HttpServletResponse res,
+		Map<String, Object> content, Map<String, Object> tool, ResourceLoader rb)
+	{
+
+		String oidc_endpoint = (String) tool.get(LTIService.LTI13_OIDC_ENDPOINT);
+		if (SakaiBLTIUtil.isLTI13(tool, content) && StringUtils.isBlank(oidc_endpoint) ) {
+			String errorMessage = "<p>" + SakaiBLTIUtil.getRB(rb, "error.no.oidc_endpoint", "Missing oidc_endpoint value for LTI 1.3 launch") + "</p>";
+			sendHTMLPage(res, errorMessage);
+			return false;
+		}
+
+		return true;
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */
@@ -388,6 +405,9 @@ public void handleAccess(HttpServletRequest req, HttpServletResponse res, Refere
 					String oidc_endpoint = (String) tool.get(LTIService.LTI13_OIDC_ENDPOINT);
 					log.debug("State={} nonce={} oidc_endpoint={}",state, nonce, oidc_endpoint);
 
+					// Sanity check for missing config data
+					if ( ! sanityCheck(req, res, null, tool, rb) ) return;
+
 					if (SakaiBLTIUtil.isLTI13(tool, null) && StringUtils.isNotBlank(oidc_endpoint) &&
 							( StringUtils.isEmpty(state) || StringUtils.isEmpty(state) ) ) {
 						redirectOIDC(req, res, null, tool, oidc_endpoint, rb);
@@ -454,6 +474,10 @@ else if ( refId.startsWith("content:") && refId.length() > 8 )
 					if ( tool != null ) {
 						String oidc_endpoint = (String) tool.get(LTIService.LTI13_OIDC_ENDPOINT);
 						log.debug("State={} nonce={} oidc_endpoint={}",state, nonce, oidc_endpoint);
+
+						// Sanity check for missing config data
+						if ( ! sanityCheck(req, res, content, tool, rb) ) return;
+
 						if (SakaiBLTIUtil.isLTI13(tool, content) && StringUtils.isNotBlank(oidc_endpoint) &&
 								(StringUtils.isEmpty(state) || StringUtils.isEmpty(nonce) ) ) {
 							redirectOIDC(req, res, content, tool, oidc_endpoint, rb);

diff --git a/basiclti/tsugi-util/src/java/org/tsugi/lti13/LTI13Util.java b/basiclti/tsugi-util/src/java/org/tsugi/lti13/LTI13Util.java
@@ -163,6 +163,7 @@ public static String breakKeyIntoLines(String rawkey) {
 	}
 
 	public static Key string2PrivateKey(String keyString) {
+		if ( keyString == null ) return null;
 		try {
 			KeyFactory kf = KeyFactory.getInstance("RSA");
 
@@ -177,6 +178,7 @@ public static Key string2PrivateKey(String keyString) {
 	}
 
 	public static Key string2PublicKey(String keyString) {
+		if ( keyString == null ) return null;
 		try {
 			KeyFactory kf = KeyFactory.getInstance("RSA");
 

diff --git a/basiclti/tsugi-util/src/test/org/tsugi/lti13/LTI13UtilTest.java b/basiclti/tsugi-util/src/test/org/tsugi/lti13/LTI13UtilTest.java
@@ -50,6 +50,25 @@ public void testRSAFromString() throws
 		assertEquals(serialized, newSer);
 	}
 
+	@Test
+	public void testNullAndBlank() {
+		Key x = LTI13Util.string2PublicKey(null);
+		x = LTI13Util.string2PublicKey("");
+		assertNull(x);
+		x = LTI13Util.string2PublicKey(" ");
+		assertNull(x);
+		x = LTI13Util.string2PublicKey("\n");
+		assertNull(x);
+
+		x = LTI13Util.string2PrivateKey(null);
+		x = LTI13Util.string2PrivateKey("");
+		assertNull(x);
+		x = LTI13Util.string2PrivateKey(" ");
+		assertNull(x);
+		x = LTI13Util.string2PrivateKey("\n");
+		assertNull(x);
+	}
+
 	@Test
 	public void testSHA256() {
 		String hash = LTI13Util.sha256("Yada");