From aa130d4cf6bc3e240722b7ab1e5300e982d4b08c Mon Sep 17 00:00:00 2001 From: C Beach Date: Mon, 13 Jan 2020 15:54:32 -0600 Subject: [PATCH 1/5] Chore: Begin efforts for 2019.1.3 hotfix --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 098461b0b0..4ecc915fa8 100644 --- a/build.sbt +++ b/build.sbt @@ -113,7 +113,7 @@ name := "Equella" equellaMajor in ThisBuild := 2019 equellaMinor in ThisBuild := 1 -equellaPatch in ThisBuild := 2 +equellaPatch in ThisBuild := 3 equellaStream in ThisBuild := "Stable" equellaBuild in ThisBuild := buildConfig.value.getString("build.buildname") From 0f12d109510bc988d3bcc7ec2ab7f1fa8260fa8b Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Tue, 26 Nov 2019 22:40:54 -0600 Subject: [PATCH 2/5] #1295 - Add detailed log around deleting attachments (round 2) (#1358) * #1295 - enhance file audit trail --- .../src/com/dytech/common/io/FileUtils.java | 9 ++++- .../services/impl/FileSystemServiceImpl.java | 39 +++++++++++++------ 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/Platform/Plugins/com.tle.platform.common/src/com/dytech/common/io/FileUtils.java b/Platform/Plugins/com.tle.platform.common/src/com/dytech/common/io/FileUtils.java index 2eb4871696..9cd8089b0e 100644 --- a/Platform/Plugins/com.tle.platform.common/src/com/dytech/common/io/FileUtils.java +++ b/Platform/Plugins/com.tle.platform.common/src/com/dytech/common/io/FileUtils.java @@ -82,11 +82,17 @@ public static boolean delete(Path f, FileCallback callback) { public static boolean delete(Path f, @Nullable FileCallback callback, boolean failOnError) throws IOException { + if (LOGGER.isTraceEnabled()) { + // Exposes the code flows that delete files + Exception tracer = new Exception("Debug stack trace - about to delete - " + f.toString()); + LOGGER.trace(tracer.getMessage(), tracer); + } if (!Files.exists(f, LinkOption.NOFOLLOW_LINKS)) { + LOGGER.debug("File does not exist. Could not delete [" + f.toString() + "]"); return true; } if (Files.isDirectory(f, LinkOption.NOFOLLOW_LINKS)) { - LOGGER.debug("File is a folder, deleting contents"); + LOGGER.debug("File [" + f.toString() + "] is a folder, deleting contents"); try { final DeleteVisitor visitor = new DeleteVisitor(callback); Files.walkFileTree(f, EnumSet.noneOf(FileVisitOption.class), Integer.MAX_VALUE, visitor); @@ -325,6 +331,7 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx } try { Files.delete(dir); + LOGGER.warn("Deleting folder " + dir.toString()); } catch (Exception e) { success = false; LOGGER.warn("Folder deletion failed. Could not delete " + dir.toString()); diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/core/services/impl/FileSystemServiceImpl.java b/Source/Plugins/Core/com.equella.core/src/com/tle/core/services/impl/FileSystemServiceImpl.java index 81547e0487..0fd8dcc99c 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/core/services/impl/FileSystemServiceImpl.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/core/services/impl/FileSystemServiceImpl.java @@ -283,7 +283,7 @@ public FileInfo write( } catch (AccessDeniedException ex) { String serviceUser = System.getProperty("user.name"); throw new IOException( - "Couldn't create directories on file store. Are you sure that EQUELLA running as '" + "Couldn't create directories on file store. Are you sure that openEQUELLA running as '" + serviceUser + "' user has the correct permissions?", ex); @@ -426,29 +426,44 @@ public void commitFiles(TemporaryFileHandle staging, String folder, FileHandle d File from = getFile(staging, folder); File to = getFile(destination); - LOGGER.info("commitFiles: from " + from.getAbsolutePath()); - LOGGER.info("commitFiles: to " + to.getAbsolutePath()); + final String fromStr = from.getAbsolutePath(); + final String toStr = to.getAbsolutePath(); + LOGGER.info("commitFiles: from [" + fromStr + "] to [" + toStr + "]"); File trash = null; if (FileSystemHelper.exists(to)) { - LOGGER.debug("Destination exists renaming"); + LOGGER.debug("Destination [" + to + "] exists. Moving to a staging 'trash' folder"); trash = getFile(new TrashFile(staging)); - LOGGER.debug("Moving current to trash"); + LOGGER.debug("Moving current [" + to + "] to trash [" + trash + "]"); + if (!FileSystemHelper.rename(to, trash, false)) { FileSystemHelper.rename(trash, to, true); - throw new FileSystemException("Couldn't move to Trash."); + throw new FileSystemException("Couldn't move [" + to + "] to trash [" + trash + "]."); } } - LOGGER.debug("About to rename staging to real item"); + LOGGER.debug("About to rename staging [" + from + "] to real item [" + to + "]."); if (!FileSystemHelper.exists(from)) { - LOGGER.debug("no files to commit - making blank dir"); + LOGGER.debug("no files to commit - making blank dir [" + to + "]"); FileSystemHelper.mkdir(to); } else { - LOGGER.debug("Renaming staging to real item"); + LOGGER.debug("Renaming staging [" + from + "] to real item [" + to + "]"); if (!FileSystemHelper.rename(from, to, false)) { + LOGGER.debug( + "The rename of staging [" + + from + + "] to real item [" + + to + + "] didn't work. Trying rename of trash to real item."); if (trash != null) { - FileSystemHelper.rename(trash, to, true); + if (!FileSystemHelper.rename(trash, to, true)) { + LOGGER.debug( + "The rename of trash [" + + trash + + "] to real item [" + + to + + "] didn't work. Silently proceeding."); + } } throw new FileSystemException( "Failed to commit to staging: " @@ -459,10 +474,10 @@ public void commitFiles(TemporaryFileHandle staging, String folder, FileHandle d } if (trash != null) { - LOGGER.debug("Deleting trash"); + LOGGER.debug("Deleting trash [" + trash + "]"); FileUtils.delete(trash.toPath(), null); } - LOGGER.debug("Done"); + LOGGER.debug("Done committing files from [" + staging + "] to [" + to + "]"); } @Override From 447624f65633b0ac500b4124aee4dbb497976010 Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Thu, 28 Nov 2019 07:59:16 -0600 Subject: [PATCH 3/5] Added config to disable RemoveStagingAreas (#1359) #1344 Added config to disable RemoveStagingAreas --- .../staging/service/impl/StagingServiceImpl.java | 5 ++++- .../tle/core/scheduler/impl/SchedulerModule.java | 1 + .../standard/task/RemoveStagingAreas.java | 15 ++++++++++++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/core/filesystem/staging/service/impl/StagingServiceImpl.java b/Source/Plugins/Core/com.equella.core/src/com/tle/core/filesystem/staging/service/impl/StagingServiceImpl.java index 1461cab219..f54a0f5d62 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/core/filesystem/staging/service/impl/StagingServiceImpl.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/core/filesystem/staging/service/impl/StagingServiceImpl.java @@ -67,10 +67,12 @@ public void removeStagingArea(StagingFile staging, boolean removeFiles) { if (s == null) { LOGGER.error("Staging area does not exist"); } else { + LOGGER.debug("Deleting Staging entry in DB [" + s.getStagingID() + "]"); stagingDao.delete(s); } if (removeFiles) { + LOGGER.debug("Removing Staging area in the filestore [" + s.getStagingID() + "]"); fileSystemService.removeFile(staging); } } @@ -100,9 +102,10 @@ public void matched(Path file, String relFilepath) { String uuid = file.getFileName().toString(); if (!stagingExists(uuid)) { try { + LOGGER.debug("Deleting staging area [" + uuid + "]"); FileUtils.delete(file, null, true); } catch (IOException ex) { - LOGGER.warn("Could not delete staging area: " + uuid, ex); + LOGGER.warn("Could not delete staging area [" + uuid + "]", ex); } } } diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/core/scheduler/impl/SchedulerModule.java b/Source/Plugins/Core/com.equella.core/src/com/tle/core/scheduler/impl/SchedulerModule.java index 33e559c2ee..65df7946c1 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/core/scheduler/impl/SchedulerModule.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/core/scheduler/impl/SchedulerModule.java @@ -26,5 +26,6 @@ public class SchedulerModule extends OptionalConfigModule { protected void configure() { bindInt("com.tle.core.tasks.RemoveDeletedItems.daysBeforeRemoval"); bindInt("com.tle.core.tasks.RemoveOldAuditLogs.daysBeforeRemoval"); + bindBoolean("com.tle.core.tasks.RemoveStagingAreas.enable", true); } } diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/core/scheduler/standard/task/RemoveStagingAreas.java b/Source/Plugins/Core/com.equella.core/src/com/tle/core/scheduler/standard/task/RemoveStagingAreas.java index 452c08e353..14a58bd8bd 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/core/scheduler/standard/task/RemoveStagingAreas.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/core/scheduler/standard/task/RemoveStagingAreas.java @@ -22,16 +22,29 @@ import com.tle.core.guice.Bind; import com.tle.core.scheduler.ScheduledTask; import javax.inject.Inject; +import javax.inject.Named; import javax.inject.Singleton; +import org.apache.log4j.Logger; /** @author Nicholas Read */ @Bind @Singleton public class RemoveStagingAreas implements ScheduledTask { + private static final Logger LOGGER = Logger.getLogger(RemoveStagingAreas.class); + @Inject private StagingService stagingService; + @com.google.inject.Inject(optional = true) + @Named("com.tle.core.tasks.RemoveStagingAreas.enable") + // Can be overrode by the optional-config.properties + private boolean enableTask = true; + @Override public void execute() { - stagingService.removeUnusedStagingAreas(); + if (enableTask) { + stagingService.removeUnusedStagingAreas(); + } else { + LOGGER.debug("RemoveStagingAreas is disabled. Not running task."); + } } } From 76ff0f6e9e0ec787bd7660b6b7c4ce201695a564 Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Thu, 28 Nov 2019 07:57:42 -0600 Subject: [PATCH 4/5] #1360 - added tomcat config for internalProxies (#1361) --- .../src/com/tle/tomcat/guice/TomcatModule.java | 1 + .../tle/tomcat/service/impl/TomcatServiceImpl.java | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/tomcat/guice/TomcatModule.java b/Source/Plugins/Core/com.equella.core/src/com/tle/tomcat/guice/TomcatModule.java index 3831d14237..80a9685c6a 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/tomcat/guice/TomcatModule.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/tomcat/guice/TomcatModule.java @@ -39,6 +39,7 @@ protected void configure() { bindBoolean("tomcat.useBio"); bindBoolean("sessions.neverPersist"); bindBoolean("userService.useXForwardedFor"); + bindProp("tomcat.internalProxies"); } } } diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/tomcat/service/impl/TomcatServiceImpl.java b/Source/Plugins/Core/com.equella.core/src/com/tle/tomcat/service/impl/TomcatServiceImpl.java index 6bf1da3f23..c352d1ad0b 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/tomcat/service/impl/TomcatServiceImpl.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/tomcat/service/impl/TomcatServiceImpl.java @@ -99,6 +99,10 @@ public class TomcatServiceImpl implements TomcatService, StartupBean, TomcatRest @Named("tomcat.useBio") private boolean useBio = false; + @Inject(optional = true) + @Named("tomcat.internalProxies") + private String internalProxies; + @Inject private DataSourceService dataSourceService; @Inject private ZookeeperService zookeeperService; @@ -124,8 +128,17 @@ public void startup() { context.setDocBase(new File(".").getAbsolutePath()); context.setUseHttpOnly(false); if (useXForwardedFor) { + LOGGER.debug("Enabling the Tomcat RemoteIpValve."); RemoteIpValve protoValve = new RemoteIpValve(); protoValve.setProtocolHeader("X-Forwarded-Proto"); + + if (!Check.isEmpty(internalProxies)) { + LOGGER.debug( + "Setting the Tomcat RemoteIpValve InternalProxies to: [" + internalProxies + "]"); + protoValve.setInternalProxies(internalProxies); + } else { + LOGGER.debug("Not enabling the Tomcat RemoteIpValve InternalProxies - config is empty"); + } context.getPipeline().addValve(protoValve); } From 976366c9794c25351f3b7bf5bbc743d495b560dc Mon Sep 17 00:00:00 2001 From: Diego del Blanco Date: Wed, 22 Jan 2020 15:24:20 -0500 Subject: [PATCH 5/5] #1424 Convert the bb lti flow to a generic lti flow oEQ now supports a generic LTI that be used for Blackboard integration. --- .../lti/consumers/entity/LtiConsumer.java | 4 + .../Core/com.equella.core/plugin-jpf.xml | 28 +-- .../lang/i18n-resource-centre.properties | 4 + .../resources/view/editconsumer.ftl | 17 +- .../BlackboardLtiWrapperExtension.java | 69 ------ .../BrightspaceLtiWrapperExtension.java | 11 +- .../CanvasLtiWrapperExtension.java | 7 +- ...GenericLtiContentItemPlacementReturn.java} | 19 +- .../GenericLtiIntegration.java} | 119 ++-------- .../GenericLtiSessionData.java} | 10 +- .../GenericLtiSignon.java} | 8 +- .../generic/GenericLtiWrapperExtension.java | 97 ++++++++ .../guice/GenericLtiIntegrationModule.java} | 16 +- .../section/LtiConsumerEditorSection.java | 30 +++ .../LisLtiWrapperExtension.java | 11 +- .../web/lti/usermanagement/LtiWrapper.java | 19 +- .../usermanagement/LtiWrapperExtension.java | 7 +- .../GenericLtiWrapperExtensionTest.java | 223 ++++++++++++++++++ 18 files changed, 466 insertions(+), 233 deletions(-) delete mode 100644 Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardLtiWrapperExtension.java rename Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/{blackboard/BlackboardContentItemPlacementReturn.java => generic/GenericLtiContentItemPlacementReturn.java} (93%) rename Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/{blackboard/BlackboardLtiIntegration.java => generic/GenericLtiIntegration.java} (75%) rename Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/{blackboard/BlackboardLtiSessionData.java => generic/GenericLtiSessionData.java} (81%) rename Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/{blackboard/BlackboardSignon.java => generic/GenericLtiSignon.java} (88%) create mode 100644 Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiWrapperExtension.java rename Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/{blackboard/guice/BlackboardLtiIntegrationModule.java => generic/guice/GenericLtiIntegrationModule.java} (67%) create mode 100644 Source/Plugins/Core/com.equella.core/test/java/com/tle/com/tle/integration/lti/generic/GenericLtiWrapperExtensionTest.java diff --git a/Source/Plugins/Core/com.equella.base/src/com/tle/common/lti/consumers/entity/LtiConsumer.java b/Source/Plugins/Core/com.equella.base/src/com/tle/common/lti/consumers/entity/LtiConsumer.java index ed9f6f0031..a918100709 100644 --- a/Source/Plugins/Core/com.equella.base/src/com/tle/common/lti/consumers/entity/LtiConsumer.java +++ b/Source/Plugins/Core/com.equella.base/src/com/tle/common/lti/consumers/entity/LtiConsumer.java @@ -35,6 +35,10 @@ @Entity @AccessType("field") public class LtiConsumer extends BaseEntity { + public static final String ATT_CUSTOM_USER_ID = "ATT_CUSTOM_USER_ID"; + public static final String ATT_CUSTOM_USERNAME = "ATT_CUSTOM_USERNAME"; + public static final String ATT_CUSTOM_ENABLE_ID_PREFIX = "ATT_CUSTOM_ENABLE_ID_PREFIX"; + @Index(name = "consumerKey") @Column(length = 255, nullable = false) private String consumerKey; diff --git a/Source/Plugins/Core/com.equella.core/plugin-jpf.xml b/Source/Plugins/Core/com.equella.core/plugin-jpf.xml index 9196d2f1cf..9bc4798abe 100644 --- a/Source/Plugins/Core/com.equella.core/plugin-jpf.xml +++ b/Source/Plugins/Core/com.equella.core/plugin-jpf.xml @@ -122,7 +122,7 @@ - + @@ -2173,9 +2173,9 @@ - - - + + + @@ -2185,23 +2185,23 @@ - - - + + + - - - + + + - - - - + + + + diff --git a/Source/Plugins/Core/com.equella.core/resources/lang/i18n-resource-centre.properties b/Source/Plugins/Core/com.equella.core/resources/lang/i18n-resource-centre.properties index f49482dc72..745e5bda19 100644 --- a/Source/Plugins/Core/com.equella.core/resources/lang/i18n-resource-centre.properties +++ b/Source/Plugins/Core/com.equella.core/resources/lang/i18n-resource-centre.properties @@ -1314,6 +1314,10 @@ editor.container.option.default=Default editor.container.option.embed=Embed editor.container.option.newwindow=New window editor.customparams=Custom parameters +editor.custom.lti.params.help=The following custom LTI parameters are only in effect for the /ltisignon.do endpoint +editor.custom.lti.params.user.id=Custom user ID +editor.custom.lti.params.username=Custom username attribute +editor.custom.lti.params.prefix.id=Prefix the user ID with a value unique to this consumer editor.dropdown.option.choosetype=Choose a connector type... editor.email=Share launcher's email with tool editor.error.accessdenied=You do not have the required permission to access this page\: {0} diff --git a/Source/Plugins/Core/com.equella.core/resources/view/editconsumer.ftl b/Source/Plugins/Core/com.equella.core/resources/view/editconsumer.ftl index a914d1c5ad..8c9f0d1a3e 100644 --- a/Source/Plugins/Core/com.equella.core/resources/view/editconsumer.ftl +++ b/Source/Plugins/Core/com.equella.core/resources/view/editconsumer.ftl @@ -6,7 +6,6 @@ <#include "/com.tle.web.sections.standard@/dropdown.ftl" /> <#include "/com.tle.web.sections.standard@/textfield.ftl"/> <#include "/com.tle.web.sections.standard@/autocomplete.ftl"/> - <@css "editconsumer.css" /> <@setting label=b.key("editor.key") mandatory=true error=m.errors["key"]> @@ -21,6 +20,18 @@ <@setting label=b.key("editor.postfix") help=b.key("editor.username.help")> <@textfield section=s.postfixField maxlength=50 /> +
+<@setting label='' help=b.key('editor.custom.lti.params.help') /> +<@setting label=b.key("editor.custom.lti.params.user.id")> + <@textfield section=s.customUserIdParameterField maxlength=128 /> + +<@setting label=b.key("editor.custom.lti.params.username")> + <@textfield section=s.customUsernameParameterField maxlength=128 /> + +<@setting label=''> +
<@render s.customEnableIdPrefixField />
+ +
<@a.div id="allowed"> <@setting label=b.key("editor.allowed.label") help=b.key("editor.allowed.help")> ${m.prettyExpression} @@ -40,8 +51,8 @@ <@a.div id="customrole"> <@setting label=b.key("editor.role.custom.label") error=m.errors["nocustomrole"] help=b.key("editor.role.custom.help")> - <@render section=s.customRolesTable /> - <@autocomplete section=s.customRoleField class="custom-role" placeholder=b.key("editor.role.custom.placeholder") /> + <@render section=s.customRolesTable /> + <@autocomplete section=s.customRoleField class="custom-role" placeholder=b.key("editor.role.custom.placeholder") /> <@render section=s.customRoleDialog.opener class="add">${b.key("editor.table.roles.add")} diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardLtiWrapperExtension.java b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardLtiWrapperExtension.java deleted file mode 100644 index f6926945ef..0000000000 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardLtiWrapperExtension.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Licensed to The Apereo Foundation under one or more contributor license - * agreements. See the NOTICE file distributed with this work for additional - * information regarding copyright ownership. - * - * The Apereo Foundation licenses this file to you under the Apache License, - * Version 2.0, (the "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at: - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.tle.integration.lti.blackboard; - -import com.tle.core.guice.Bind; -import com.tle.web.lti.usermanagement.LtiWrapperExtension; -import javax.inject.Singleton; -import javax.servlet.http.HttpServletRequest; - -/** - * Checks for Canvas custom_bb_user_login_id param to match an existing user. - * - *

NOTE 1/21/19 (CB): This was ported over from the Canvas logic. We may not Need this - * eventually. - */ -@SuppressWarnings("nls") -@Bind -@Singleton -public class BlackboardLtiWrapperExtension implements LtiWrapperExtension { - // TODO - 'lis_sourcedid' looks to provide this detail. Anything we could gain by allowing it to - // be custom? - @Override - public String getUserId(HttpServletRequest request) { - return request.getParameter("custom_bb_user_id"); - } - - // TODO - 'lis_sourcedid' looks to provide this detail. Anything we could gain by allowing it to - // be custom? - @Override - public String getUsername(HttpServletRequest request) { - return request.getParameter("custom_bb_user_login_id"); - } - - @Override - public String getFirstName(HttpServletRequest request) { - return null; - } - - @Override - public String getLastName(HttpServletRequest request) { - return null; - } - - @Override - public String getEmail(HttpServletRequest request) { - return null; - } - - @Override - public boolean isPrefixUserId() { - return true; - } -} diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/brightspace/BrightspaceLtiWrapperExtension.java b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/brightspace/BrightspaceLtiWrapperExtension.java index 5fade7384c..23fbffde83 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/brightspace/BrightspaceLtiWrapperExtension.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/brightspace/BrightspaceLtiWrapperExtension.java @@ -22,6 +22,7 @@ import com.google.common.base.Strings; import com.google.inject.Inject; import com.tle.common.Utils; +import com.tle.common.lti.consumers.entity.LtiConsumer; import com.tle.core.guice.Bind; import com.tle.web.lti.usermanagement.LtiWrapperExtension; import javax.inject.Named; @@ -46,13 +47,13 @@ public class BrightspaceLtiWrapperExtension implements LtiWrapperExtension { private String userIdParameter; @Override - public String getUserId(HttpServletRequest request) { + public String getUserId(HttpServletRequest request, LtiConsumer consumer) { final String rawId = request.getParameter(userIdParameter); if (!Strings.isNullOrEmpty(rawId)) { return truncatedUniqueName(rawId); } // Fall back to username param (which may mean username and user ID are the same, which is fine) - final String rawUsername = getUsername(request); + final String rawUsername = getUsername(request, consumer); if (!Strings.isNullOrEmpty(rawUsername)) { return truncatedUniqueName(rawUsername); } @@ -60,7 +61,7 @@ public String getUserId(HttpServletRequest request) { } /** - * EQUELLA only allows 40 chars for user IDs. We hope that this is vaguely unique. + * openEQUELLA only allows 40 chars for user IDs. We hope that this is vaguely unique. * * @param rawName * @return @@ -84,7 +85,7 @@ private String truncatedUniqueName(String rawName) { } @Override - public String getUsername(HttpServletRequest request) { + public String getUsername(HttpServletRequest request, LtiConsumer consumer) { return request.getParameter(usernameParameter); } @@ -107,7 +108,7 @@ public String getEmail(HttpServletRequest request) { } @Override - public boolean isPrefixUserId() { + public boolean isPrefixUserId(LtiConsumer consumer) { return false; } } diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/canvasextension/CanvasLtiWrapperExtension.java b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/canvasextension/CanvasLtiWrapperExtension.java index 0d5b342daf..33ec6e8a9c 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/canvasextension/CanvasLtiWrapperExtension.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/canvasextension/CanvasLtiWrapperExtension.java @@ -18,6 +18,7 @@ package com.tle.integration.lti.canvasextension; +import com.tle.common.lti.consumers.entity.LtiConsumer; import com.tle.core.guice.Bind; import com.tle.web.lti.usermanagement.LtiWrapperExtension; import javax.inject.Singleton; @@ -33,12 +34,12 @@ @Singleton public class CanvasLtiWrapperExtension implements LtiWrapperExtension { @Override - public String getUserId(HttpServletRequest request) { + public String getUserId(HttpServletRequest request, LtiConsumer consumer) { return request.getParameter("custom_canvas_user_id"); } @Override - public String getUsername(HttpServletRequest request) { + public String getUsername(HttpServletRequest request, LtiConsumer consumer) { return request.getParameter("custom_canvas_user_login_id"); } @@ -58,7 +59,7 @@ public String getEmail(HttpServletRequest request) { } @Override - public boolean isPrefixUserId() { + public boolean isPrefixUserId(LtiConsumer consumer) { return true; } } diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardContentItemPlacementReturn.java b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiContentItemPlacementReturn.java similarity index 93% rename from Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardContentItemPlacementReturn.java rename to Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiContentItemPlacementReturn.java index 09e79d72d4..e5340b7728 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardContentItemPlacementReturn.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiContentItemPlacementReturn.java @@ -16,7 +16,7 @@ * limitations under the License. */ -package com.tle.integration.lti.blackboard; +package com.tle.integration.lti.generic; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.core.JsonProcessingException; @@ -48,6 +48,7 @@ import com.tle.web.sections.js.generic.SimpleElementId; import com.tle.web.sections.render.HiddenInput; import com.tle.web.sections.render.HtmlRenderer; +import com.tle.web.sections.render.TextUtils; import com.tle.web.selection.SelectedResource; import com.tle.web.selection.SelectionService; import com.tle.web.selection.SelectionSession; @@ -60,15 +61,14 @@ import javax.inject.Inject; import org.apache.log4j.Logger; -// TODO: defer - we should be able to make this a generic LTI return @Bind -public class BlackboardContentItemPlacementReturn extends AbstractPrototypeSection +public class GenericLtiContentItemPlacementReturn extends AbstractPrototypeSection implements HtmlRenderer { - private static final Logger LOGGER = Logger.getLogger(BlackboardContentItemPlacementReturn.class); + private static final Logger LOGGER = Logger.getLogger(GenericLtiContentItemPlacementReturn.class); @Inject private SelectionService selectionService; @Inject private IntegrationService integrationService; - @Inject private BlackboardLtiIntegration blackboardLtiIntegration; + @Inject private GenericLtiIntegration genericLtiIntegration; @Inject private ItemResolver itemResolver; @Inject private OAuthWebService oauthWebService; @Inject private LtiConsumerService consumerService; @@ -86,10 +86,10 @@ public SectionResult renderHtml(RenderEventContext context) throws Exception { final IItem item = getItemForResource(resource); final LmsLink link = - blackboardLtiIntegration + genericLtiIntegration .getLinkForResource( context, - blackboardLtiIntegration.createViewableItem(item, resource), + genericLtiIntegration.createViewableItem(item, resource), resource, false, session.isAttachmentUuidUrls()) @@ -145,10 +145,7 @@ private String buildSelectionJson(List links) { graph.id = link.getUrl(); graph.url = link.getUrl(); graph.title = link.getName(); - graph.text = link.getName(); - // FIXME: Is there a custom BB property for the description? Base LTI spec doesn't specify - // one - // graph.description = TextUtils.INSTANCE.ensureWrap(link.getDescription(),250, 250, true); + graph.text = TextUtils.INSTANCE.ensureWrap(link.getDescription(), 250, 250, true); graph.mediaType = "application/vnd.ims.lti.v1.ltilink"; graph.windowTarget = "_blank"; final ContentItemSelection.ContentItemGraph.ContentItemPlacementAdvice placementAdvice = diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardLtiIntegration.java b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiIntegration.java similarity index 75% rename from Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardLtiIntegration.java rename to Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiIntegration.java index 23644e25ba..81e012beb8 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardLtiIntegration.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiIntegration.java @@ -16,7 +16,7 @@ * limitations under the License. */ -package com.tle.integration.lti.blackboard; +package com.tle.integration.lti.generic; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.PrettyPrinter; @@ -38,7 +38,6 @@ import com.tle.common.connectors.entity.Connector; import com.tle.common.usermanagement.user.CurrentUser; import com.tle.common.usermanagement.user.UserState; -import com.tle.core.connectors.blackboard.service.BlackboardConnectorService; import com.tle.core.connectors.service.ConnectorRepositoryService; import com.tle.core.connectors.service.ConnectorService; import com.tle.core.guice.Bind; @@ -75,32 +74,16 @@ @Singleton @NonNullByDefault @SuppressWarnings("nls") -public class BlackboardLtiIntegration extends AbstractIntegrationService { - private static final Logger LOGGER = Logger.getLogger(BlackboardLtiIntegration.class); - - // TODO - I think all this can be removed. - // Omitted logic for now... - // - // private static final String CUSTOM_CANVAS_COURSE_ID = "custom_bb_course_id"; - // private static final String CUSTOM_CANVAS_API_DOMAIN = "custom_bb_api_domain"; - // private static final String LIS_COURSE_OFFERING_SOURCEDID = "lis_course_offering_sourcedid"; - private static final String CONTENT_ITEM_SELECTION_REQUEST = "ContentItemSelectionRequest"; +public class GenericLtiIntegration extends AbstractIntegrationService { + private static final Logger LOGGER = Logger.getLogger(GenericLtiIntegration.class); - // TODO - I think all this can be removed. - // Omitted logic for now... - // - // These two are only supplied to us if configured in the Canvas LTI tool setup. E.g.: - // Custom Fields: - // course_id=$Canvas.course.sisSourceId - // course_code=$com.instructure.contextLabel - // private static final String CUSTOM_COURSE_ID = "custom_course_id"; - // private static final String CUSTOM_COURSE_CODE = "custom_course_code"; + private static final String CONTENT_ITEM_SELECTION_REQUEST = "ContentItemSelectionRequest"; static { - PluginResourceHandler.init(BlackboardLtiIntegration.class); + PluginResourceHandler.init(GenericLtiIntegration.class); } - @PlugKey("integration.receipt.addedtoblackboard") + @PlugKey("integration.receipt.addedtolti") private static String KEY_RECEIPT_ADDED; @PlugKey("canvas.error.requireoneconnector") @@ -116,77 +99,42 @@ public class BlackboardLtiIntegration extends AbstractIntegrationService courseStructureCache; private final ObjectMapper objectMapper = new ObjectMapper(); - // TODO - I think all this can be removed. - // private static final Multimap CONTENT_TYPES_TO_MIME = HashMultimap.create(6, - // 6); - // - // static - // { - // // oembed,lti_launch_url,url,image_url,iframe - // // CONTENT_TYPES_TO_MIME.put("oembed", ""); - // CONTENT_TYPES_TO_MIME.put("image_url", "image/jpeg"); - // CONTENT_TYPES_TO_MIME.put("image_url", "image/gif"); - // CONTENT_TYPES_TO_MIME.put("image_url", "image/png"); - // } - // Omitted logic for now... - // - // @PostConstruct - // public void setupCache() - // { - // courseStructureCache = cacheService.getCache("BlackboardSignonCourseStructure", 100, 2, - // TimeUnit.MINUTES); - // } - @Override protected String getIntegrationType() { - return "blackboardLti"; + return "lti"; } - public boolean isItemOnly(BlackboardLtiSessionData data) { + public boolean isItemOnly(GenericLtiSessionData data) { return false; } @Override - public BlackboardLtiSessionData createDataForViewing(SectionInfo info) { - return new BlackboardLtiSessionData(); + public GenericLtiSessionData createDataForViewing(SectionInfo info) { + return new GenericLtiSessionData(); } @Override public void setupSingleSignOn(SectionInfo info, SingleSignonForm form) { - final BlackboardLtiSessionData data = new BlackboardLtiSessionData(info.getRequest()); + final GenericLtiSessionData data = new GenericLtiSessionData(info.getRequest()); String courseId = null; final UserState userState = CurrentUser.getUserState(); - // TODO: I think this can be removed as well - // final List courseCodes = new ArrayList(); String courseCode = form.getCourseCode(); if (userState instanceof LtiUserState) { final LtiUserState ltiUserState = (LtiUserState) userState; final LtiData ltiData = ltiUserState.getData(); if (ltiData != null) { - // TODO: does BB pass anything about this? Want a second opinion, but I don't think so. - data.setApiDomain(null); // ltiData.getCustom(CUSTOM_CANVAS_API_DOMAIN)); courseId = form.getCourseId(); if (Strings.isNullOrEmpty(courseId)) { - // courseId = ltiData.getCustom(CUSTOM_CANVAS_COURSE_ID); courseId = ltiData.getContextId(); } data.setCourseId(courseId); data.setContextTitle(ltiData.getContextTitle()); - // - // courseCodes.add(ltiData.getCustom(CUSTOM_COURSE_CODE)); - // courseCodes.add(ltiData.getCustom(CUSTOM_COURSE_ID)); - // courseCodes.add(ltiData.getCustom(LIS_COURSE_OFFERING_SOURCEDID)); - // courseCodes.add(ltiData.getContextLabel()); - if (Strings.isNullOrEmpty(courseCode)) { courseCode = ltiData.getContextLabel(); } @@ -209,7 +157,7 @@ public void setupSingleSignOn(SectionInfo info, SingleSignonForm form) { actionInfo = new IntegrationActionInfo(); } - LOGGER.debug("BlackboardLtiIntegration.setupSingleSignOn: about to forward - data=" + data); + LOGGER.debug("GenericLtiIntegration.setupSingleSignOn: about to forward - data=" + data); integrationService.standardForward( info, convertToForward(actionInfo, form), data, actionInfo, form); } @@ -231,7 +179,7 @@ private String convertToForward(IntegrationActionInfo action, SingleSignonForm m @Override public SelectionSession setupSelectionSession( SectionInfo info, - BlackboardLtiSessionData data, + GenericLtiSessionData data, SelectionSession session, SingleSignonForm form) { final boolean structured = "structured".equals(data.getAction()); @@ -253,7 +201,7 @@ public SelectionSession setupSelectionSession( @Nullable private String initStructure( - BlackboardLtiSessionData data, SelectionSession session, SingleSignonForm form) { + GenericLtiSessionData data, SelectionSession session, SingleSignonForm form) { final String courseId = data.getCourseId(); String structure = form.getStructure(); if (structure == null) { @@ -263,7 +211,7 @@ private String initStructure( } structure = courseStructureCache.get(courseId).orNull(); } - // if no structure, get from Canvas + // if no structure, get from LMS if (structure == null) { final ObjectNode root = objectMapper.createObjectNode(); root.put("id", courseId); @@ -300,7 +248,7 @@ private String initStructure( return structure; } - private Connector findConnector(BlackboardLtiSessionData data) { + private Connector findConnector(GenericLtiSessionData data) { Connector connector = null; final String connectorUuid = data.getConnectorUuid(); if (connectorUuid != null) { @@ -327,14 +275,14 @@ private Connector findConnector(BlackboardLtiSessionData data) { } @Override - public boolean select(SectionInfo info, BlackboardLtiSessionData data, SelectionSession session) { + public boolean select(SectionInfo info, GenericLtiSessionData data, SelectionSession session) { try { // TODO: a find a better way to determine if in structured session or not if (!session.getLayout().equals(RootSelectionSection.Layout.COURSE)) { String lti_message_type = data.getLtiMessageType(); if (lti_message_type != null && lti_message_type.equalsIgnoreCase(CONTENT_ITEM_SELECTION_REQUEST)) { - info.forward(info.createForward("/blackboardlticipreturn.do")); + info.forward(info.createForward("/lticipreturn.do")); } else { final SelectedResource resource = getFirstSelectedResource(session); final IItem item = getItemForResource(resource); @@ -365,25 +313,6 @@ public boolean select(SectionInfo info, BlackboardLtiSessionData data, Selection final StringBuilder retUrl = new StringBuilder(launchPresentationReturnUrl); retUrl.append(launchPresentationReturnUrl.contains("?") ? "&" : "?"); - // final Set extContentReturnTypes = data.getExtContentReturnTypes(); - // - // if( extContentReturnTypes.contains("image_url") - // && CONTENT_TYPES_TO_MIME.get("image_url").contains(mimeType) ) - // { - // retUrl.append("return_type=image_url"); - // - // retUrl.append("&url="); - // retUrl.append(URLEncoder.encode(link.getUrl(), "UTF-8")); - // - // retUrl.append("&alt="); - // retUrl.append(URLEncoder.encode(link.getName(), "UTF-8")); - // } - // "oembed" not supported yet - // "iframe" - // "url" - - // else - // { retUrl.append("return_type=lti_launch_url"); retUrl.append("&url="); @@ -404,9 +333,7 @@ public boolean select(SectionInfo info, BlackboardLtiSessionData data, Selection // maintain the selections so they survive the forwarding return true; - } else // TODO - I don't think there is a way to select multiple resources with the current Bb - // UI. - { + } else { final Connector connector = findConnector(data); final String courseId = session.getStructure().getId(); @@ -437,24 +364,24 @@ public boolean select(SectionInfo info, BlackboardLtiSessionData data, Selection } @Override - public String getClose(BlackboardLtiSessionData data) { + public String getClose(GenericLtiSessionData data) { return data.getLaunchPresentationReturnUrl(); } @Nullable @Override - public String getCourseInfoCode(BlackboardLtiSessionData data) { + public String getCourseInfoCode(GenericLtiSessionData data) { return data.getCourseInfoCode(); } @Nullable @Override - public NameValue getLocation(BlackboardLtiSessionData data) { + public NameValue getLocation(GenericLtiSessionData data) { return null; } @Override - protected boolean canSelect(BlackboardLtiSessionData data) { + protected boolean canSelect(GenericLtiSessionData data) { // can be select_link, embed_content etc final String sd = data.getSelectionDirective(); return sd != null || "ContentItemSelectionRequest".equals(data.getLtiMessageType()); diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardLtiSessionData.java b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiSessionData.java similarity index 81% rename from Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardLtiSessionData.java rename to Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiSessionData.java index 279a0c16d3..d3ce281523 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardLtiSessionData.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiSessionData.java @@ -16,24 +16,24 @@ * limitations under the License. */ -package com.tle.integration.lti.blackboard; +package com.tle.integration.lti.generic; import com.tle.integration.lti.LtiSessionData; import javax.servlet.http.HttpServletRequest; -public class BlackboardLtiSessionData extends LtiSessionData { +public class GenericLtiSessionData extends LtiSessionData { private static final long serialVersionUID = 1L; - public BlackboardLtiSessionData() { + public GenericLtiSessionData() { super(); } - public BlackboardLtiSessionData(HttpServletRequest request) { + public GenericLtiSessionData(HttpServletRequest request) { super(request); } @Override public String getIntegrationType() { - return "blackboardlti"; + return "lti"; } } diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardSignon.java b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiSignon.java similarity index 88% rename from Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardSignon.java rename to Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiSignon.java index fbffa5d64f..0f1706b340 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/BlackboardSignon.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiSignon.java @@ -16,7 +16,7 @@ * limitations under the License. */ -package com.tle.integration.lti.blackboard; +package com.tle.integration.lti.generic; import com.tle.annotation.NonNullByDefault; import com.tle.core.guice.Bind; @@ -34,14 +34,14 @@ @SuppressWarnings("nls") @NonNullByDefault @Bind -public class BlackboardSignon extends AbstractPrototypeSection +public class GenericLtiSignon extends AbstractPrototypeSection implements AfterParametersListener { - @Inject private BlackboardLtiIntegration blackboardLtiIntegration; + @Inject private GenericLtiIntegration genericLtiIntegration; @Override public void afterParameters(SectionInfo info, ParametersEvent event) { final SingleSignonForm model = getModel(info); - blackboardLtiIntegration.setupSingleSignOn(info, model); + genericLtiIntegration.setupSingleSignOn(info, model); } @Override diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiWrapperExtension.java b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiWrapperExtension.java new file mode 100644 index 0000000000..4c18890489 --- /dev/null +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/GenericLtiWrapperExtension.java @@ -0,0 +1,97 @@ +/* + * Licensed to The Apereo Foundation under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * The Apereo Foundation licenses this file to you under the Apache License, + * Version 2.0, (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.tle.integration.lti.generic; + +import com.tle.common.externaltools.constants.ExternalToolConstants; +import com.tle.common.lti.consumers.entity.LtiConsumer; +import com.tle.core.guice.Bind; +import com.tle.web.lti.usermanagement.LtiWrapperExtension; +import javax.inject.Singleton; +import javax.servlet.http.HttpServletRequest; +import org.apache.commons.lang.StringUtils; + +@Bind +@Singleton +public class GenericLtiWrapperExtension implements LtiWrapperExtension { + + @Override + public String getUserId(HttpServletRequest request, LtiConsumer consumer) { + return getGenericLtiParam( + request, consumer, LtiConsumer.ATT_CUSTOM_USER_ID, ExternalToolConstants.USER_ID); + } + + @Override + public String getUsername(HttpServletRequest request, LtiConsumer consumer) { + return getGenericLtiParam( + request, + consumer, + LtiConsumer.ATT_CUSTOM_USERNAME, + ExternalToolConstants.LIS_PERSON_SOURCEDID); + } + + /** This is the fallback to standard LIS params */ + @Override + public String getFirstName(HttpServletRequest request) { + return request.getParameter(ExternalToolConstants.LIS_PERSON_NAME_GIVEN); + } + + /** This is the fallback to standard LIS params */ + @Override + public String getLastName(HttpServletRequest request) { + return request.getParameter(ExternalToolConstants.LIS_PERSON_NAME_FAMILY); + } + + /** This is the fallback to standard LIS params */ + @Override + public String getEmail(HttpServletRequest request) { + return request.getParameter(ExternalToolConstants.LIS_PERSON_CONTACT_EMAIL_PRIMARY); + } + + /** + * Default is true (prefix the user id with a hash of the consumer). + * + *

This can be overridden by configuring the custom LTI setting of 'Prefix ID' + * + *

Note: This is different then the username prefix that an LTI Consumer can be configured to + * append. + */ + @Override + public boolean isPrefixUserId(LtiConsumer consumer) { + if (consumer == null) { + return true; + } + + return consumer.getAttribute(LtiConsumer.ATT_CUSTOM_ENABLE_ID_PREFIX, true); + } + + private String getGenericLtiParam( + HttpServletRequest request, LtiConsumer consumer, String customParam, String defaultParam) { + + if ((request == null) || (consumer == null)) { + return null; + } + + String paramVal = request.getParameter(consumer.getAttribute(customParam)); + if (StringUtils.isEmpty(paramVal)) { + paramVal = request.getParameter(defaultParam); + } + + return paramVal; + } +} diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/guice/BlackboardLtiIntegrationModule.java b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/guice/GenericLtiIntegrationModule.java similarity index 67% rename from Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/guice/BlackboardLtiIntegrationModule.java rename to Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/guice/GenericLtiIntegrationModule.java index 559b8c6a92..5ec65cbb67 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/blackboard/guice/BlackboardLtiIntegrationModule.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/integration/lti/generic/guice/GenericLtiIntegrationModule.java @@ -16,23 +16,23 @@ * limitations under the License. */ -package com.tle.integration.lti.blackboard.guice; +package com.tle.integration.lti.generic.guice; import com.google.inject.name.Names; -import com.tle.integration.lti.blackboard.BlackboardContentItemPlacementReturn; -import com.tle.integration.lti.blackboard.BlackboardSignon; +import com.tle.integration.lti.generic.GenericLtiContentItemPlacementReturn; +import com.tle.integration.lti.generic.GenericLtiSignon; import com.tle.web.sections.equella.guice.SectionsModule; @SuppressWarnings("nls") -public class BlackboardLtiIntegrationModule extends SectionsModule { +public class GenericLtiIntegrationModule extends SectionsModule { @Override protected void configure() { bind(Object.class) - .annotatedWith(Names.named("/blackboardltisignon")) - .toProvider(node(BlackboardSignon.class)); + .annotatedWith(Names.named("/ltisignon")) + .toProvider(node(GenericLtiSignon.class)); bind(Object.class) - .annotatedWith(Names.named("/blackboardlticipreturn")) - .toProvider(node(BlackboardContentItemPlacementReturn.class)); + .annotatedWith(Names.named("/lticipreturn")) + .toProvider(node(GenericLtiContentItemPlacementReturn.class)); } } diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/consumers/section/LtiConsumerEditorSection.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/consumers/section/LtiConsumerEditorSection.java index 2b76c2c7be..a046c98510 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/consumers/section/LtiConsumerEditorSection.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/consumers/section/LtiConsumerEditorSection.java @@ -63,6 +63,7 @@ import com.tle.web.sections.render.SectionRenderable; import com.tle.web.sections.render.TextLabel; import com.tle.web.sections.result.util.KeyLabel; +import com.tle.web.sections.standard.Checkbox; import com.tle.web.sections.standard.ComponentFactory; import com.tle.web.sections.standard.Link; import com.tle.web.sections.standard.SingleSelectionList; @@ -132,6 +133,13 @@ private static enum RoleTypes { @Component private TextField consumerSecretField; @Component private TextField prefixField; @Component private TextField postfixField; + @Component private TextField customUserIdParameterField; + @Component private TextField customUsernameParameterField; + @Component private Checkbox customEnableIdPrefixField; + + @PlugKey("editor.custom.lti.params.prefix.id") + private static Label LABEL_CUSTOM_ENABLE_ID_PREFIX_FIELD; + @Component private SingleSelectionList unknownUserList; @Component private SelectionsTable instructorRolesTable; @@ -262,6 +270,8 @@ public void registered(String id, SectionTree tree) { tree, this, events.getEventHandler("removeCustom"), "customrole"); customRoleField.setAutoCompleteCallback(ajax.getAjaxFunction("customRolesAutocomplete")); + + customEnableIdPrefixField.setLabel(LABEL_CUSTOM_ENABLE_ID_PREFIX_FIELD); } @Override @@ -413,6 +423,10 @@ protected void loadFromSession( bean.getConsumerSecret() == null ? UUID.randomUUID().toString() : bean.getConsumerSecret()); prefixField.setValue(info, bean.getPrefix()); postfixField.setValue(info, bean.getPostfix()); + customUserIdParameterField.setValue(info, bean.getAttribute(LtiConsumer.ATT_CUSTOM_USER_ID)); + customUsernameParameterField.setValue(info, bean.getAttribute(LtiConsumer.ATT_CUSTOM_USERNAME)); + customEnableIdPrefixField.setChecked( + info, bean.getAttribute(LtiConsumer.ATT_CUSTOM_ENABLE_ID_PREFIX, true)); allowedSelector.setExpression(info, bean.getAllowedExpression()); LtiConsumerEditorModel model = getModel(info); if (!Check.isEmpty(bean.getInstructorRoles())) { @@ -486,6 +500,10 @@ protected void saveToSession( boolean validate) { LtiConsumerEditingBean bean = session.getBean(); bean.setConsumerKey(consumerKeyField.getValue(info)); + bean.setAttribute(LtiConsumer.ATT_CUSTOM_USER_ID, customUserIdParameterField.getValue(info)); + bean.setAttribute(LtiConsumer.ATT_CUSTOM_USERNAME, customUsernameParameterField.getValue(info)); + bean.setAttribute( + LtiConsumer.ATT_CUSTOM_ENABLE_ID_PREFIX, customEnableIdPrefixField.isChecked(info)); bean.setConsumerSecret(consumerSecretField.getValue(info)); bean.setPrefix(prefixField.getValue(info)); bean.setPostfix(postfixField.getValue(info)); @@ -679,6 +697,18 @@ public TextField getPostfixField() { return postfixField; } + public TextField getCustomUserIdParameterField() { + return customUserIdParameterField; + } + + public TextField getCustomUsernameParameterField() { + return customUsernameParameterField; + } + + public Checkbox getCustomEnableIdPrefixField() { + return customEnableIdPrefixField; + } + public ExpressionSelectorDialog getAllowedSelector() { return allowedSelector; } diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LisLtiWrapperExtension.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LisLtiWrapperExtension.java index ea9334c11e..518fab327e 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LisLtiWrapperExtension.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LisLtiWrapperExtension.java @@ -19,21 +19,26 @@ package com.tle.web.lti.usermanagement; import com.tle.common.externaltools.constants.ExternalToolConstants; +import com.tle.common.lti.consumers.entity.LtiConsumer; import com.tle.core.guice.Bind; import javax.inject.Singleton; import javax.servlet.http.HttpServletRequest; /** @author Aaron */ + +// FIXME: With the new GenericLtiWrapperExtension, this code shouldn't be called anymore + @Bind @Singleton public class LisLtiWrapperExtension implements LtiWrapperExtension { + @Override - public String getUserId(HttpServletRequest request) { + public String getUserId(HttpServletRequest request, LtiConsumer consumer) { return null; } @Override - public String getUsername(HttpServletRequest request) { + public String getUsername(HttpServletRequest request, LtiConsumer consumer) { return request.getParameter(ExternalToolConstants.LIS_PERSON_SOURCEDID); } @@ -53,7 +58,7 @@ public String getEmail(HttpServletRequest request) { } @Override - public boolean isPrefixUserId() { + public boolean isPrefixUserId(LtiConsumer consumer) { return true; } } diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LtiWrapper.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LtiWrapper.java index 64cb30c295..4c9f3ab0a7 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LtiWrapper.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LtiWrapper.java @@ -106,7 +106,7 @@ public ModifiableUserState authenticateRequest(HttpServletRequest request) { ModifiableUserState userState = null; final String username = getUsername(request, null, consumer); - final String userId = getUserId(request); + final String userId = getUserId(request, consumer); if (username != null) { userState = getChain().authenticateUserFromUsername(username, null); @@ -137,7 +137,7 @@ public ModifiableUserState authenticateRequest(HttpServletRequest request) { ltiUserState.setData(generateLtiData(request)); ltiUserState.getUsersRoles().addAll(getDynamicRoles(request, consumer)); if (ltiUserState.getUserBean() == null) { - setupUser(request, ltiUserState); + setupUser(request, ltiUserState, consumer); } if (userCreated && !Check.isEmpty(createGroups)) { @@ -169,7 +169,7 @@ private String getUsername( @Nullable LtiConsumer consumer) { String username = fallbackUsername; for (LtiWrapperExtension extension : extensions.getBeanList()) { - final String extensionUsername = extension.getUsername(request); + final String extensionUsername = extension.getUsername(request, consumer); if (!Strings.isNullOrEmpty(extensionUsername)) { username = extensionUsername; break; @@ -186,13 +186,13 @@ private String getUsername( return username; } - private String getUserId(HttpServletRequest request) { + private String getUserId(HttpServletRequest request, @Nullable LtiConsumer consumer) { String userId = null; boolean prefix = true; for (LtiWrapperExtension extension : extensions.getBeanList()) { - userId = extension.getUserId(request); + userId = extension.getUserId(request, consumer); if (userId != null) { - prefix = extension.isPrefixUserId(); + prefix = extension.isPrefixUserId(consumer); break; } } @@ -333,12 +333,13 @@ private Collection getDynamicRoles(HttpServletRequest request, LtiConsum return extraRoles; } - private void setupUser(HttpServletRequest request, ModifiableUserState state) { - final String userId = getUserId(request); + private void setupUser( + HttpServletRequest request, ModifiableUserState state, LtiConsumer consumer) { + final String userId = getUserId(request, consumer); state.setLoggedInUser( new DefaultUserBean( userId, - getUsername(request, "lti:" + userId, null), + getUsername(request, "lti:" + userId, consumer), getFirstName(request), getLastName(request), getEmail(request))); diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LtiWrapperExtension.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LtiWrapperExtension.java index 5d852fa204..7966c48b62 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LtiWrapperExtension.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/web/lti/usermanagement/LtiWrapperExtension.java @@ -20,16 +20,17 @@ import com.tle.annotation.NonNullByDefault; import com.tle.annotation.Nullable; +import com.tle.common.lti.consumers.entity.LtiConsumer; import javax.servlet.http.HttpServletRequest; /** @author Aaron */ @NonNullByDefault public interface LtiWrapperExtension { @Nullable - String getUserId(HttpServletRequest request); + String getUserId(HttpServletRequest request, LtiConsumer consumer); @Nullable - String getUsername(HttpServletRequest request); + String getUsername(HttpServletRequest request, LtiConsumer consumer); @Nullable String getFirstName(HttpServletRequest request); @@ -40,5 +41,5 @@ public interface LtiWrapperExtension { @Nullable String getEmail(HttpServletRequest request); - boolean isPrefixUserId(); + boolean isPrefixUserId(LtiConsumer consumer); } diff --git a/Source/Plugins/Core/com.equella.core/test/java/com/tle/com/tle/integration/lti/generic/GenericLtiWrapperExtensionTest.java b/Source/Plugins/Core/com.equella.core/test/java/com/tle/com/tle/integration/lti/generic/GenericLtiWrapperExtensionTest.java new file mode 100644 index 0000000000..508a0c4dd4 --- /dev/null +++ b/Source/Plugins/Core/com.equella.core/test/java/com/tle/com/tle/integration/lti/generic/GenericLtiWrapperExtensionTest.java @@ -0,0 +1,223 @@ +package com.tle.com.tle.integration.lti.generic; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertNull; +import static junit.framework.Assert.assertTrue; + +import com.tle.common.externaltools.constants.ExternalToolConstants; +import com.tle.common.lti.consumers.entity.LtiConsumer; +import com.tle.integration.lti.generic.GenericLtiWrapperExtension; +import org.apache.struts.mock.MockHttpServletRequest; +import org.junit.Test; + +@SuppressWarnings("nls") +public class GenericLtiWrapperExtensionTest { + @Test + public void testGetUserIdRequestNull() { + LtiConsumer consumer = setupLtiConsumerCustomBoth(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUserId(null, consumer); + assertNull(s); + } + + @Test + public void testGetUserIdConsumerNull() { + MockHttpServletRequest req = setupMockRequestAllValues(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUserId(req, null); + assertNull(s); + } + + @Test + public void testGetUserIdCustom() { + MockHttpServletRequest req = setupMockRequestAllValues(); + + LtiConsumer consumer = setupLtiConsumerCustomId(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUserId(req, consumer); + assertEquals("custom-user-id-val", s); + } + + @Test + public void testGetUserIdCustomNotInRequest() { + MockHttpServletRequest req = setupMockRequestOnlyDefault(); + + LtiConsumer consumer = setupLtiConsumerCustomId(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUserId(req, consumer); + assertEquals("default-user-id-val", s); + } + + @Test + public void testGetUserIdCustomEmpty() { + MockHttpServletRequest req = setupMockRequestAllValues(); + + LtiConsumer consumer = setupLtiConsumerCustomUsername(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUserId(req, consumer); + assertEquals("default-user-id-val", s); + } + + @Test + public void testGetUserIdDefaultNotInRequest() { + MockHttpServletRequest req = setupMockRequestNoValues(); + + LtiConsumer consumer = setupLtiConsumerCustomUsername(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUserId(req, consumer); + assertNull(s); + } + + @Test + public void testGetUsernameRequestNull() { + LtiConsumer consumer = setupLtiConsumerCustomBoth(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUsername(null, consumer); + assertNull(s); + } + + @Test + public void testGetUsernameConsumerNull() { + MockHttpServletRequest req = setupMockRequestAllValues(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUsername(req, null); + assertNull(s); + } + + @Test + public void testGetUsernameCustom() { + MockHttpServletRequest req = setupMockRequestAllValues(); + + LtiConsumer consumer = setupLtiConsumerCustomUsername(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUsername(req, consumer); + assertEquals("custom-username-val", s); + } + + @Test + public void testGetUsernameCustomNotInRequest() { + MockHttpServletRequest req = setupMockRequestOnlyDefault(); + + LtiConsumer consumer = setupLtiConsumerCustomUsername(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUsername(req, consumer); + assertEquals("default-username-val", s); + } + + @Test + public void testGetUsernameCustomEmpty() { + MockHttpServletRequest req = setupMockRequestAllValues(); + + LtiConsumer consumer = setupLtiConsumerCustomId(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUsername(req, consumer); + assertEquals("default-username-val", s); + } + + @Test + public void testGetUsernameDefaultNotInRequest() { + MockHttpServletRequest req = setupMockRequestNoValues(); + + LtiConsumer consumer = setupLtiConsumerCustomId(); + + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + final String s = genericLti.getUsername(req, consumer); + assertNull(s); + } + + @Test + public void testIsPrefixUserIdFromNullConsumer() { + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + + final boolean b = genericLti.isPrefixUserId(null); + assertTrue(b); + } + + @Test + public void testIsPrefixUserIdTrue() { + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + + LtiConsumer consumer = setupLtiConsumerCustomBoth(); + + final boolean b = genericLti.isPrefixUserId(consumer); + assertTrue(b); + } + + @Test + public void testIsPrefixUserIdDefault() { + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + + LtiConsumer consumer = setupLtiConsumerCustomBoth(); + + final boolean b = genericLti.isPrefixUserId(consumer); + assertTrue(b); + } + + @Test + public void testIsPrefixUserIdFalse() { + GenericLtiWrapperExtension genericLti = new GenericLtiWrapperExtension(); + + LtiConsumer consumer = setupLtiConsumerCustomId(); + + final boolean b = genericLti.isPrefixUserId(consumer); + assertFalse(b); + } + + private MockHttpServletRequest setupMockRequestAllValues() { + MockHttpServletRequest req = new MockHttpServletRequest(); + req.addParameter("mycustomid", "custom-user-id-val"); + req.addParameter("mycustomname", "custom-username-val"); + req.addParameter(ExternalToolConstants.LIS_PERSON_SOURCEDID, "default-username-val"); + req.addParameter(ExternalToolConstants.USER_ID, "default-user-id-val"); + + return req; + } + + private MockHttpServletRequest setupMockRequestOnlyDefault() { + MockHttpServletRequest req = new MockHttpServletRequest(); + req.addParameter(ExternalToolConstants.LIS_PERSON_SOURCEDID, "default-username-val"); + req.addParameter(ExternalToolConstants.USER_ID, "default-user-id-val"); + + return req; + } + + private MockHttpServletRequest setupMockRequestNoValues() { + return new MockHttpServletRequest(); + } + + private LtiConsumer setupLtiConsumerCustomBoth() { + LtiConsumer consumer = new LtiConsumer(); + consumer.setAttribute(LtiConsumer.ATT_CUSTOM_USER_ID, "mycustomid"); + consumer.setAttribute(LtiConsumer.ATT_CUSTOM_USERNAME, "mycustomname"); + consumer.setAttribute(LtiConsumer.ATT_CUSTOM_ENABLE_ID_PREFIX, true); + return consumer; + } + + private LtiConsumer setupLtiConsumerCustomId() { + LtiConsumer consumer = new LtiConsumer(); + consumer.setAttribute(LtiConsumer.ATT_CUSTOM_USER_ID, "mycustomid"); + consumer.setAttribute(LtiConsumer.ATT_CUSTOM_USERNAME, ""); + consumer.setAttribute(LtiConsumer.ATT_CUSTOM_ENABLE_ID_PREFIX, false); + return consumer; + } + + private LtiConsumer setupLtiConsumerCustomUsername() { + LtiConsumer consumer = new LtiConsumer(); + consumer.setAttribute(LtiConsumer.ATT_CUSTOM_USER_ID, ""); + consumer.setAttribute(LtiConsumer.ATT_CUSTOM_USERNAME, "mycustomname"); + consumer.setAttribute(LtiConsumer.ATT_CUSTOM_ENABLE_ID_PREFIX, false); + return consumer; + } +}