From 488ca3d920dddf6031b32c1ebc63d20011651920 Mon Sep 17 00:00:00 2001 From: penghai zhang Date: Wed, 25 Oct 2023 01:01:25 +0000 Subject: [PATCH 01/14] Merge branch 'chore/update-birt-dependencies' into 'develop' OEQ-1730 chore: update BIRT dependencies to v4.9.2 See merge request edalex-group/development/oeq/openequella!279 --- Source/Plugins/Birt/org.eclipse.birt.osgi/build.sbt | 2 +- Source/Plugins/Core/com.equella.reporting/build.sbt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Plugins/Birt/org.eclipse.birt.osgi/build.sbt b/Source/Plugins/Birt/org.eclipse.birt.osgi/build.sbt index be8ceff9c5..c345f5eb4d 100644 --- a/Source/Plugins/Birt/org.eclipse.birt.osgi/build.sbt +++ b/Source/Plugins/Birt/org.eclipse.birt.osgi/build.sbt @@ -4,7 +4,7 @@ lazy val BirtOsgi = config("birtosgi") lazy val CustomCompile = config("compile") extend BirtOsgi libraryDependencies := Seq( - "com.github.openequella" % "birt-osgi" % "4.9.1" artifacts Artifact("birt-osgi", + "com.github.openequella" % "birt-osgi" % "4.9.2" artifacts Artifact("birt-osgi", Artifact.DefaultType, "zip"), "com.github.equella.reporting" % "reporting-common" % "6.5", diff --git a/Source/Plugins/Core/com.equella.reporting/build.sbt b/Source/Plugins/Core/com.equella.reporting/build.sbt index 64c878a23a..1d7710fc85 100644 --- a/Source/Plugins/Core/com.equella.reporting/build.sbt +++ b/Source/Plugins/Core/com.equella.reporting/build.sbt @@ -4,7 +4,7 @@ lazy val Birt = config("birt") lazy val CustomCompile = config("compile") extend Birt libraryDependencies ++= Seq( - "com.github.openequella" % "birt-report-framework" % "4.9.1" artifacts Artifact( + "com.github.openequella" % "birt-report-framework" % "4.9.2" artifacts Artifact( "birt-report-framework", Artifact.DefaultType, "zip"), From bf4e6b3670096b20fea9e042c9d202d8496819a9 Mon Sep 17 00:00:00 2001 From: penghai zhang Date: Thu, 9 Nov 2023 01:17:07 +0000 Subject: [PATCH 02/14] Merge branch 'refactor/reporting-structure' into 'develop' OEQ-1737 refactor: consolidate two BIRT reporting projects into one See merge request edalex-group/development/oeq/openequella!285 --- .../org.eclipse.birt.osgi/birt-MANIFEST.MF | 135 ------------------ .../Birt/org.eclipse.birt.osgi/build.sbt | 42 ------ .../Birt/org.eclipse.birt.osgi/plugin-jpf.xml | 10 -- .../org.eclipse.birt.osgi/readme-birt.txt | 13 -- .../Core/com.equella.reporting/build.sbt | 75 ++++++++-- .../core/reporting/ReportingServiceImpl.java | 3 +- 6 files changed, 64 insertions(+), 214 deletions(-) delete mode 100644 Source/Plugins/Birt/org.eclipse.birt.osgi/birt-MANIFEST.MF delete mode 100644 Source/Plugins/Birt/org.eclipse.birt.osgi/build.sbt delete mode 100644 Source/Plugins/Birt/org.eclipse.birt.osgi/plugin-jpf.xml delete mode 100644 Source/Plugins/Birt/org.eclipse.birt.osgi/readme-birt.txt diff --git a/Source/Plugins/Birt/org.eclipse.birt.osgi/birt-MANIFEST.MF b/Source/Plugins/Birt/org.eclipse.birt.osgi/birt-MANIFEST.MF deleted file mode 100644 index fb11971514..0000000000 --- a/Source/Plugins/Birt/org.eclipse.birt.osgi/birt-MANIFEST.MF +++ /dev/null @@ -1,135 +0,0 @@ -Manifest-Version: 1.0 -Ant-Version: Apache Ant 1.8.2 -Created-By: 1.5.0_09-b03 (Sun Microsystems Inc.) -Bundle-ManifestVersion: 2 -Bundle-Name: BIRT Report Engine API Fragment -Bundle-SymbolicName: org.eclipse.birt.api -Bundle-Version: 1.0.0 -Bundle-Vendor: Eclipse.org -Fragment-Host: system.bundle; extension:=framework -Export-Package: org.eclipse.birt.report.item.crosstab.core,org.eclipse - .birt.report.item.crosstab.core.de,org.eclipse.birt.report.item.cross - tab.core.de.internal,org.eclipse.birt.report.item.crosstab.core.i18n, - org.eclipse.birt.report.item.crosstab.core.script,org.eclipse.birt.re - port.item.crosstab.core.util,org.eclipse.birt.report.engine.api,org.e - clipse.birt.report.engine.api.impl,org.eclipse.birt.report.engine.i18 - n,org.eclipse.birt.report.engine.ir,org.eclipse.birt.report.engine.ut - il,org.eclipse.birt.report.engine.emitter.config,org.eclipse.birt.rep - ort.data.adapter.api,org.eclipse.birt.report.data.adapter.api.script, - org.eclipse.birt.report.data.adapter.api.timeFunction,org.eclipse.bir - t.report.data.adapter.i18n,org.eclipse.birt.report.engine.api.script, - org.eclipse.birt.report.engine.api.script.element,org.eclipse.birt.re - port.engine.api.script.eventadapter,org.eclipse.birt.report.engine.ap - i.script.eventhandler,org.eclipse.birt.report.engine.api.script.insta - nce,javax.servlet,javax.servlet.http,javax.servlet.resources,org.ecli - pse.birt.data.aggregation.api,org.w3c.flute.parser,org.w3c.flute.pars - er.selectors,org.w3c.flute.util,org.eclipse.birt.report.engine.dataex - traction,org.mozilla.classfile,org.mozilla.javascript,org.mozilla.jav - ascript.debug,org.mozilla.javascript.jdk13,org.mozilla.javascript.jdk - 15,org.mozilla.javascript.optimizer,org.mozilla.javascript.regexp,org - .mozilla.javascript.resources,org.mozilla.javascript.serialize,org.mo - zilla.javascript.tools,org.mozilla.javascript.tools.debugger,org.mozi - lla.javascript.tools.debugger.treetable,org.mozilla.javascript.tools. - idswitch,org.mozilla.javascript.tools.jsc,org.mozilla.javascript.tool - s.resources,org.mozilla.javascript.tools.shell,org.mozilla.javascript - .xml,org.mozilla.javascript.xml.impl.xmlbeans,org.mozilla.javascript. - xmlimpl,com.ibm.icu;version="4.4.2",com.ibm.icu.impl;version="4.4.2", - com.ibm.icu.impl.data;version="4.4.2",com.ibm.icu.impl.data.icudt44b; - version="4.4.2",com.ibm.icu.impl.data.icudt44b.brkitr;version="4.4.2" - ,com.ibm.icu.impl.data.icudt44b.coll;version="4.4.2",com.ibm.icu.impl - .data.icudt44b.curr;version="4.4.2",com.ibm.icu.impl.data.icudt44b.la - ng;version="4.4.2",com.ibm.icu.impl.data.icudt44b.rbnf;version="4.4.2 - ",com.ibm.icu.impl.data.icudt44b.region;version="4.4.2",com.ibm.icu.i - mpl.data.icudt44b.translit;version="4.4.2",com.ibm.icu.impl.data.icud - t44b.zone;version="4.4.2",com.ibm.icu.impl.duration;version="4.4.2",c - om.ibm.icu.impl.duration.impl;version="4.4.2",com.ibm.icu.impl.durati - on.impl.data;version="4.4.2",com.ibm.icu.impl.locale;version="4.4.2", - com.ibm.icu.lang;version="4.4.2",com.ibm.icu.math;version="4.4.2",com - .ibm.icu.text;version="4.4.2",com.ibm.icu.util;version="4.4.2",org.ec - lipse.birt.report.model.adapter.oda,org.apache.commons.codec,org.apac - he.commons.codec.binary,org.apache.commons.codec.digest,org.apache.co - mmons.codec.language,org.apache.commons.codec.net,org.eclipse.datatoo - ls.connectivity.oda.design,org.w3c.css.sac,org.w3c.css.sac.helpers,or - g.eclipse.birt.chart.aggregate,org.eclipse.birt.chart.api,org.eclipse - .birt.chart.computation,org.eclipse.birt.chart.computation.withaxes,o - rg.eclipse.birt.chart.computation.withoutaxes,org.eclipse.birt.chart. - datafeed,org.eclipse.birt.chart.device,org.eclipse.birt.chart.engine. - i18n,org.eclipse.birt.chart.event,org.eclipse.birt.chart.exception,or - g.eclipse.birt.chart.factory,org.eclipse.birt.chart.integrate,org.ecl - ipse.birt.chart.internal.computations,org.eclipse.birt.chart.internal - .datafeed,org.eclipse.birt.chart.internal.factory,org.eclipse.birt.ch - art.internal.layout,org.eclipse.birt.chart.internal.log,org.eclipse.b - irt.chart.internal.model,org.eclipse.birt.chart.internal.prefs,org.ec - lipse.birt.chart.log,org.eclipse.birt.chart.model,org.eclipse.birt.ch - art.model.attribute,org.eclipse.birt.chart.model.attribute.impl,org.e - clipse.birt.chart.model.attribute.util,org.eclipse.birt.chart.model.c - omponent,org.eclipse.birt.chart.model.component.impl,org.eclipse.birt - .chart.model.component.util,org.eclipse.birt.chart.model.data,org.ecl - ipse.birt.chart.model.data.impl,org.eclipse.birt.chart.model.data.uti - l,org.eclipse.birt.chart.model.impl,org.eclipse.birt.chart.model.layo - ut,org.eclipse.birt.chart.model.layout.impl,org.eclipse.birt.chart.mo - del.layout.util,org.eclipse.birt.chart.model.type,org.eclipse.birt.ch - art.model.type.impl,org.eclipse.birt.chart.model.type.util,org.eclips - e.birt.chart.model.util,org.eclipse.birt.chart.plugin,org.eclipse.bir - t.chart.render,org.eclipse.birt.chart.script,org.eclipse.birt.chart.s - tyle,org.eclipse.birt.chart.util,schema.doc,org.eclipse.emf.ecore,org - .eclipse.emf.ecore.impl,org.eclipse.emf.ecore.plugin,org.eclipse.emf. - ecore.resource,org.eclipse.emf.ecore.resource.impl,org.eclipse.emf.ec - ore.util,org.eclipse.emf.ecore.xml.namespace,org.eclipse.emf.ecore.xm - l.namespace.impl,org.eclipse.emf.ecore.xml.namespace.util,org.eclipse - .emf.ecore.xml.type,org.eclipse.emf.ecore.xml.type.impl,org.eclipse.e - mf.ecore.xml.type.internal,org.eclipse.emf.ecore.xml.type.util,org.ec - lipse.birt.chart.reportitem.api,org.eclipse.birt.chart.reportitem.i18 - n,org.eclipse.birt.chart.examples.radar.i18n,org.eclipse.birt.chart.e - xamples.radar.model,org.eclipse.birt.chart.examples.radar.model.type, - org.eclipse.birt.chart.examples.radar.model.type.impl,org.eclipse.bir - t.chart.examples.radar.model.type.util,org.eclipse.birt.chart.example - s.radar.render,javax.servlet.jsp,javax.servlet.jsp.el,javax.servlet.j - sp.resources,javax.servlet.jsp.tagext,org.eclipse.birt.doc.legacy,org - .eclipse.birt.doc.romdoc,org.eclipse.birt.doc.schema,org.eclipse.birt - .doc.util,org.eclipse.birt.report.model.activity,org.eclipse.birt.rep - ort.model.api,org.eclipse.birt.report.model.api.activity,org.eclipse. - birt.report.model.api.command,org.eclipse.birt.report.model.api.core, - org.eclipse.birt.report.model.api.css,org.eclipse.birt.report.model.a - pi.elements,org.eclipse.birt.report.model.api.elements.structures,org - .eclipse.birt.report.model.api.elements.table,org.eclipse.birt.report - .model.api.extension,org.eclipse.birt.report.model.api.filterExtensio - n,org.eclipse.birt.report.model.api.filterExtension.interfaces,org.ec - lipse.birt.report.model.api.metadata,org.eclipse.birt.report.model.ap - i.oda,org.eclipse.birt.report.model.api.oda.interfaces,org.eclipse.bi - rt.report.model.api.olap,org.eclipse.birt.report.model.api.scripts,or - g.eclipse.birt.report.model.api.simpleapi,org.eclipse.birt.report.mod - el.api.util,org.eclipse.birt.report.model.api.validators,org.eclipse. - birt.report.model.command,org.eclipse.birt.report.model.core,org.ecli - pse.birt.report.model.core.namespace,org.eclipse.birt.report.model.cs - s,org.eclipse.birt.report.model.css.property,org.eclipse.birt.report. - model.elements,org.eclipse.birt.report.model.elements.interfaces,org. - eclipse.birt.report.model.elements.olap,org.eclipse.birt.report.model - .elements.strategy,org.eclipse.birt.report.model.extension,org.eclips - e.birt.report.model.extension.oda,org.eclipse.birt.report.model.i18n, - org.eclipse.birt.report.model.metadata,org.eclipse.birt.report.model. - metadata.validators,org.eclipse.birt.report.model.parser,org.eclipse. - birt.report.model.parser.treebuild,org.eclipse.birt.report.model.util - ,org.eclipse.birt.report.model.util.copy,org.eclipse.birt.report.mode - l.util.impl,org.eclipse.birt.report.model.util.xpathparser,org.eclips - e.birt.report.model.validators,org.eclipse.birt.report.model.writer,o - rg.eclipse.birt.core.archive,org.eclipse.birt.core.archive.cache,org. - eclipse.birt.core.archive.compound,org.eclipse.birt.core.archive.comp - ound.v3,org.eclipse.birt.core.data,org.eclipse.birt.core.exception,or - g.eclipse.birt.core.format,org.eclipse.birt.core.framework,org.eclips - e.birt.core.framework.jar,org.eclipse.birt.core.framework.osgi,org.ec - lipse.birt.core.i18n,org.eclipse.birt.core.script,org.eclipse.birt.co - re.script.functionservice,org.eclipse.birt.core.script.functionservic - e.impl,org.eclipse.birt.core.template,org.eclipse.birt.core.util,java - x.olap,javax.olap.cursor,javax.olap.query.querycoremodel,org.eclipse. - birt.data.engine.api,org.eclipse.birt.data.engine.api.aggregation,org - .eclipse.birt.data.engine.api.querydefn,org.eclipse.birt.data.engine. - api.script,org.eclipse.birt.data.engine.api.timefunction,org.eclipse. - birt.data.engine.core,org.eclipse.birt.data.engine.core.security,org. - eclipse.birt.data.engine.i18n,org.eclipse.birt.data.engine.olap.api,o - rg.eclipse.birt.data.engine.olap.api.query,org.eclipse.emf.common,org - .eclipse.emf.common.archive,org.eclipse.emf.common.command,org.eclips - e.emf.common.notify,org.eclipse.emf.common.notify.impl,org.eclipse.em - f.common.util,org.eclipse.emf.ecore.xmi,org.eclipse.emf.ecore.xmi.imp - l,org.eclipse.emf.ecore.xmi.util,com.tle.reporting,org.eclipse.datato - ols.connectivity.oda diff --git a/Source/Plugins/Birt/org.eclipse.birt.osgi/build.sbt b/Source/Plugins/Birt/org.eclipse.birt.osgi/build.sbt deleted file mode 100644 index c345f5eb4d..0000000000 --- a/Source/Plugins/Birt/org.eclipse.birt.osgi/build.sbt +++ /dev/null @@ -1,42 +0,0 @@ -import Path.flat - -lazy val BirtOsgi = config("birtosgi") -lazy val CustomCompile = config("compile") extend BirtOsgi - -libraryDependencies := Seq( - "com.github.openequella" % "birt-osgi" % "4.9.2" artifacts Artifact("birt-osgi", - Artifact.DefaultType, - "zip"), - "com.github.equella.reporting" % "reporting-common" % "6.5", - "com.github.equella.reporting" % "reporting-oda" % "6.5", - "com.github.equella.reporting" % "reporting-oda-connectors" % "6.5", - "org.apache.commons" % "com.springsource.org.apache.commons.httpclient" % "3.1.0", - "org.apache.commons" % "com.springsource.org.apache.commons.logging" % "1.1.1", - "org.apache.commons" % "com.springsource.org.apache.commons.codec" % "1.6.0", - xstreamDep, - "javax.xml.stream" % "com.springsource.javax.xml.stream" % "1.0.1" -).map(_ % BirtOsgi) - -resolvers += Resolver.url("my-test-repo", - url("https://repository.springsource.com/ivy/bundles/external/"))( - Patterns(false, "[organisation]/[module]/[revision]/[artifact]-[revision].[ext]")) - -ivyConfigurations := overrideConfigs(BirtOsgi, CustomCompile)(ivyConfigurations.value) - -(Compile / resourceGenerators) += Def.task { - val baseDir = (Compile / resourceManaged).value - val baseBirt = baseDir / "ReportEngine" - IO.delete(baseBirt) - val outPlugins = baseBirt / "lib" - val ur = update.value - val pluginJars = - Classpaths.managedJars(BirtOsgi, Set("jar"), ur).files.filter(_.getName.endsWith(".jar")) - val unzipped = IO.unzip(ur.select(artifact = artifactFilter(extension = "zip")).head, baseBirt) - val copied = IO.copy(pluginJars.pair(flat(outPlugins), errorIfNone = false)) - val birtManifest = baseDirectory.value / "birt-MANIFEST.MF" - val manifestPlugin = outPlugins / "org.eclipse.birt.api.jar" - IO.zip(Seq(birtManifest -> "META-INF/MANIFEST.MF"), - manifestPlugin, - Option((ThisBuild / buildTimestamp).value)) - unzipped.toSeq ++ copied :+ manifestPlugin -}.taskValue diff --git a/Source/Plugins/Birt/org.eclipse.birt.osgi/plugin-jpf.xml b/Source/Plugins/Birt/org.eclipse.birt.osgi/plugin-jpf.xml deleted file mode 100644 index 7b5b2abb3f..0000000000 --- a/Source/Plugins/Birt/org.eclipse.birt.osgi/plugin-jpf.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - - - - - - - - diff --git a/Source/Plugins/Birt/org.eclipse.birt.osgi/readme-birt.txt b/Source/Plugins/Birt/org.eclipse.birt.osgi/readme-birt.txt deleted file mode 100644 index 31b68e1d66..0000000000 --- a/Source/Plugins/Birt/org.eclipse.birt.osgi/readme-birt.txt +++ /dev/null @@ -1,13 +0,0 @@ -To be able to compile the com.tle.reporting.* plugins you must follow these instructions: - -1. In Eclipse, right click on the build.xml file of this plugin and select Run As -> Ant Build..., then choose the "make" target and click Run -2. Refresh this project in Eclipse. -3. Add the Target Platform: - a. Window -> Preferences -> Plug-in Development -> Target Platform - b. Click Add - c. Choose 'Current Target' and click Next - d. Click Add, choose Directory and click Next - e. Enter the path to {your codebase}\Source\Plugins\Birt\org.eclipse.birt.osgi\resources\birt\plugins and click Finish - f. Click Finish - - diff --git a/Source/Plugins/Core/com.equella.reporting/build.sbt b/Source/Plugins/Core/com.equella.reporting/build.sbt index 1d7710fc85..e814782225 100644 --- a/Source/Plugins/Core/com.equella.reporting/build.sbt +++ b/Source/Plugins/Core/com.equella.reporting/build.sbt @@ -8,27 +8,78 @@ libraryDependencies ++= Seq( "birt-report-framework", Artifact.DefaultType, "zip"), - "com.github.equella.reporting" % "reporting-common" % "6.5", - "com.github.equella.reporting" % "reporting-oda" % "6.5", - "com.github.equella.reporting" % "reporting-oda-connectors" % "6.5", - "rhino" % "js" % "1.7R2" + "com.github.openequella" % "birt-osgi" % "4.9.2" artifacts Artifact("birt-osgi", + Artifact.DefaultType, + "zip"), + "com.github.equella.reporting" % "reporting-common" % "6.5", + "com.github.equella.reporting" % "reporting-oda" % "6.5", + "com.github.equella.reporting" % "reporting-oda-connectors" % "6.5", + "org.apache.commons" % "com.springsource.org.apache.commons.httpclient" % "3.1.0", + "org.apache.commons" % "com.springsource.org.apache.commons.logging" % "1.1.1", + "org.apache.commons" % "com.springsource.org.apache.commons.codec" % "1.6.0", + xstreamDep, + "javax.xml.stream" % "com.springsource.javax.xml.stream" % "1.0.1" ).map(_ % Birt) +resolvers += Resolver.url("SpringSource Ivy Repository", + url("https://repository.springsource.com/ivy/bundles/external/"))( + Patterns(false, "[organisation]/[module]/[revision]/[artifact]-[revision].[ext]")) + ivyConfigurations := overrideConfigs(Birt, CustomCompile)(ivyConfigurations.value) // This setting should include all the managed JARs and unmanaged JARs extracted from zip file -// birt-report-framework. +// birt-report-framework and birt-osgi. jpfLibraryJars := { + val updateReport = update.value val unmanagedJarBase = baseDirectory.value / "lib" + val osgiBaseDir = (Compile / resourceManaged).value / "ReportEngine" + + // Copy managed Jars to the unmanaged Jar base. + def copyManagedJars: Set[File] = { + val managedJars: Seq[File] = + Classpaths + .managedJars(Birt, Set("jar"), updateReport) + .files + .filter(_.getName.endsWith(".jar")) + IO.copy(managedJars.pair(flat(unmanagedJarBase), errorIfNone = false)) + } + + // Unzip framework to the unmanaged Jar base. + def unzipFramework: Set[File] = { + val framework = + updateReport.select(artifact = artifactFilter(name = "birt-report-framework")).head + IO.unzip(framework, unmanagedJarBase) + } + + // Unzip osgi to the managed resources directory and classes directory, which will achieve + // the same result as `Compile / resourceGenerators`. + def unzipOSGI(): Unit = { + val osgi = updateReport.select(artifact = artifactFilter(name = "birt-osgi")).head + Array(osgiBaseDir, (Compile / classDirectory).value / "ReportEngine").foreach(IO.unzip(osgi, _)) + } + + // Copy OSGI Jars to unmanaged Jar base, excluding Jars that cause dependency conflicts. + def copyOSGIToUnmanagedJars: Set[File] = { + def exclusionRule(file: File) = + List( + "javax.xml_1.3.4.v201005080400.jar", + "js.jar", + "BirtSample.jar", + "sampledb.jar", + "guava-r09.jar" + ).contains(file.getName) + + val fileFilter = "*.jar" -- new SimpleFileFilter(exclusionRule) + val jars = osgiBaseDir / "platform" / "plugins" ** fileFilter - // Copy managed Jars to unmanaged Jar base. - val updateReport = update.value - val managedJars: Seq[File] = - Classpaths.managedJars(Birt, Set("jar"), updateReport).files.filter(_.getName.endsWith(".jar")) - IO.copy(managedJars.pair(flat(unmanagedJarBase), errorIfNone = false)) + IO.copy(jars.pair(flat(unmanagedJarBase), errorIfNone = false)) + } - IO.unzip(updateReport.select(artifact = artifactFilter(name = "birt-report-framework")).head, - unmanagedJarBase) + copyManagedJars + unzipFramework + unzipOSGI() + copyOSGIToUnmanagedJars + // Return all the Jars in the unmanaged Jar base. (unmanagedJarBase ** "*.jar").classpath } diff --git a/Source/Plugins/Core/com.equella.reporting/src/com/tle/core/reporting/ReportingServiceImpl.java b/Source/Plugins/Core/com.equella.reporting/src/com/tle/core/reporting/ReportingServiceImpl.java index c0c1844b95..852329f743 100644 --- a/Source/Plugins/Core/com.equella.reporting/src/com/tle/core/reporting/ReportingServiceImpl.java +++ b/Source/Plugins/Core/com.equella.reporting/src/com/tle/core/reporting/ReportingServiceImpl.java @@ -148,8 +148,7 @@ private synchronized IReportEngine getReportEngine() { if (configService.isDebuggingMode()) { engineConfig.setOSGiConfig(ImmutableMap.of("osgi.dev", "true")); } - URL resource = - pluginService.getClassLoader("org.eclipse.birt.osgi").getResource("ReportEngine/"); + URL resource = getClass().getClassLoader().getResource("ReportEngine/"); String birtHome = IoUtil.url2file(resource).getAbsolutePath(); engineConfig.setBIRTHome(birtHome); From ed6362ce5b6484568e30a2a9d5ff49cd9fcb2707 Mon Sep 17 00:00:00 2001 From: Penghai Date: Wed, 15 Nov 2023 10:30:46 +1100 Subject: [PATCH 03/14] build: bump the version to 2023.1.1 --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 28afe552f3..ba0aac8e89 100644 --- a/build.sbt +++ b/build.sbt @@ -124,7 +124,7 @@ name := "Equella" (ThisBuild / equellaMajor) := 2023 (ThisBuild / equellaMinor) := 1 -(ThisBuild / equellaPatch) := 0 +(ThisBuild / equellaPatch) := 1 (ThisBuild / equellaStream) := "Stable" (ThisBuild / equellaBuild) := buildConfig.value.getString("build.buildname") (ThisBuild / buildTimestamp) := Instant.now().getEpochSecond From 736f808fe0b16958b8e5bb196e4b283e57686d5e Mon Sep 17 00:00:00 2001 From: Penghai Date: Wed, 15 Nov 2023 16:23:36 +1100 Subject: [PATCH 04/14] chore: bump release stream from Alpha to RC --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index ca4a1ee9c0..a99eccedf9 100644 --- a/build.sbt +++ b/build.sbt @@ -129,7 +129,7 @@ name := "Equella" (ThisBuild / equellaMajor) := 2023 (ThisBuild / equellaMinor) := 2 (ThisBuild / equellaPatch) := 0 -(ThisBuild / equellaStream) := "Alpha" +(ThisBuild / equellaStream) := "RC" (ThisBuild / equellaBuild) := buildConfig.value.getString("build.buildname") (ThisBuild / buildTimestamp) := Instant.now().getEpochSecond From 3aa10b4ff009df0972b5c8533505bdba6a16dc94 Mon Sep 17 00:00:00 2001 From: penghai zhang Date: Sun, 1 Oct 2023 23:22:55 +0000 Subject: [PATCH 05/14] Merge branch 'fix/cannot-upgrade-to-20232' into 'develop' OEQ-1669 chore: make sure the ID of upgrade process UpdateApacheDaemon is consistent See merge request edalex-group/development/oeq/openequella!264 --- .../upgraders/apachedaemon/UpdateApacheDaemon.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Source/Tools/UpgradeInstallation/scalasrc/com/tle/upgrade/upgraders/apachedaemon/UpdateApacheDaemon.scala b/Source/Tools/UpgradeInstallation/scalasrc/com/tle/upgrade/upgraders/apachedaemon/UpdateApacheDaemon.scala index d96195482b..b057a83162 100644 --- a/Source/Tools/UpgradeInstallation/scalasrc/com/tle/upgrade/upgraders/apachedaemon/UpdateApacheDaemon.scala +++ b/Source/Tools/UpgradeInstallation/scalasrc/com/tle/upgrade/upgraders/apachedaemon/UpdateApacheDaemon.scala @@ -46,7 +46,13 @@ class UpdateApacheDaemon extends AbstractUpgrader { val MANAGER_SERVER_LINUX = "manager" val MANAGER_SERVER_WINDOWS = "manager.bat" - override def getId: String = s"UpdateApacheDaemon-${UpgradeMain.getCommit}" + // The ID of this upgrade was initially done by using `UpgradeMain.getCommit`, which results in + // different IDs in each release. This will break the upgrade process because this upgrade is + // mandatory so its ID MUST be consistent across all the releases; otherwise, an exception about + // missing this upgrade will be thrown during the upgrade. + // Considering this upgrade was implemented in 2023.1, it makes sense to use the short commit hash + // tagged by '2023.1' as the ID. + override def getId: String = s"UpdateApacheDaemon-g448a8ab" override def isBackwardsCompatible: Boolean = false From c01f13fe4d60fbbc53a4e5ba38657271d07b1a91 Mon Sep 17 00:00:00 2001 From: I Stevenson Date: Mon, 20 Nov 2023 07:13:05 +0000 Subject: [PATCH 06/14] build: pin CI for 2023.2 release --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 669c14abda..4f7ceca4a9 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,4 @@ include: project: edalex-group/development/oeq/openequella-ci file: build/main.yml - ref: main + ref: 6ef017a59f7eda0c6bf74cceba7020cf72fb37bf From 164d24998498c79c07b50dfe5e1f8e096c112ece Mon Sep 17 00:00:00 2001 From: Penghai Date: Wed, 22 Nov 2023 11:40:49 +1100 Subject: [PATCH 07/14] feat: make sure Oauth Client credentials are validated before revoking a token --- .../lang/i18n-resource-centre.properties | 1 + .../com/tle/web/oauth/OAuthWebConstants.java | 2 + .../web/oauth/servlet/OAuthTokenServlet.java | 51 +++++++++++++++++-- .../test/webservices/rest/OAuthTest.java | 34 +++++++++++-- 4 files changed, 80 insertions(+), 8 deletions(-) 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 51fbfc6be4..3aec13df66 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 @@ -2981,6 +2981,7 @@ oauth.error.noaccess=You do not have the required permissions to view this page oauth.error.parammandatory=Parameter ''{0}'' must be supplied oauth.error.tokennotfound=Unrecognised token value oauth.error.usernotfound=Impersonated user ''{0}'' does not exist +oauth.error.validationfailed=Client credential validation failed oauth.filter.error.mustbehttps=This request must be made over HTTPS oauth.logon.title=Client requesting access oauth.lti.error.client.disabled=Internal error\: LTI consumer is disabled. Please check LTI consumer configuration diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/OAuthWebConstants.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/OAuthWebConstants.java index 6aae5645fd..fcbd9cd254 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/OAuthWebConstants.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/OAuthWebConstants.java @@ -53,6 +53,8 @@ public final class OAuthWebConstants { public static final String OAUTH_TOKEN_URL = "oauth/access_token"; public static final String HEADER_AUTHORIZATION = "Authorization"; + + public static final String BASIC_AUTHORIZATION_PREFIX = "basic "; public static final String HEADER_X_AUTHORIZATION = "X-Authorization"; public static final String AUTHORIZATION_ACCESS_TOKEN = "access_token"; public static final String AUTHORIZATION_BEARER = "Bearer"; diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java index 94e6c46e6b..67d9350940 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java @@ -19,7 +19,9 @@ package com.tle.web.oauth.servlet; import com.dytech.edge.exceptions.WebException; +import com.rometools.rome.io.impl.Base64; import com.tle.common.Check; +import com.tle.core.encryption.EncryptionService; import com.tle.core.guice.Bind; import com.tle.core.oauth.OAuthConstants; import com.tle.web.oauth.OAuthException; @@ -32,6 +34,7 @@ import com.tle.web.oauth.service.OAuthWebService.AuthorisationDetails; import java.io.IOException; import java.time.Instant; +import java.util.Optional; import javax.inject.Inject; import javax.inject.Singleton; import javax.servlet.http.HttpServletRequest; @@ -50,6 +53,8 @@ public class OAuthTokenServlet extends AbstractOAuthServlet { @Inject private OAuthWebService oauthWebService; + @Inject private EncryptionService encryptionService; + @Override protected void doService(HttpServletRequest request, HttpServletResponse response) throws WebException { @@ -165,14 +170,50 @@ else if (isClientCredentials) { } } + private boolean validateCredentials(String auth) { + if (auth.toLowerCase().startsWith(OAuthWebConstants.BASIC_AUTHORIZATION_PREFIX)) { + // Remove the prefix `Basic `. + String decoded = Base64.decode(auth.substring(6)); + String[] credentials = decoded.split(":"); + if (credentials.length >= 2) { + String clientId = credentials[0]; + String clientSecret = credentials[1]; + + IOAuthClient client = oauthWebService.getByClientIdOnly(clientId); + + return Optional.ofNullable(client) + .map(IOAuthClient::getClientSecret) + .map(secret -> encryptionService.decrypt(secret)) + .map(secret -> secret.equals(clientSecret)) + .orElse(false); + } + } + + return false; + } + /** - * According to the spec for OAuth 2.0 - * Token Revocation >, the response code should be 200 regardless whether the token is valid - * or not. Hence, token validation is not needed. However, if a token is not present in the + * Revoke an Oauth token. Client credentials must be validated first. If the validation fails, + * return a 401 error. + * + *

According to the spec for OAuth + * 2.0 Token Revocation >, the response code should be 200 regardless whether the token is + * valid or not. Hence, token validation is not needed. However, if a token is not present in the * request payload, return a 400 error. */ private void revokeToken(HttpServletRequest request) { - String token = getParameter(request, TOKEN_PARAM, true); - oauthWebService.revokeToken(token); + Optional.ofNullable(request.getHeader(OAuthWebConstants.HEADER_AUTHORIZATION)) + .filter(this::validateCredentials) + .ifPresentOrElse( + (auth) -> { + String token = getParameter(request, TOKEN_PARAM, true); + oauthWebService.revokeToken(token); + }, + () -> { + throw new OAuthException( + HttpServletResponse.SC_UNAUTHORIZED, + OAuthConstants.ERROR_INVALID_REQUEST, + text("oauth.error.validationfailed")); + }); } } diff --git a/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java b/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java index 9c00d670fb..5551556e52 100644 --- a/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java +++ b/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java @@ -12,6 +12,7 @@ import com.tle.webtests.pageobject.oauth.OAuthSettingsPage; import com.tle.webtests.pageobject.oauth.OAuthTokenRedirect; import com.tle.webtests.test.users.TokenSecurity; +import com.unboundid.util.Base64; import java.io.IOException; import java.text.ParseException; import java.util.Collections; @@ -279,6 +280,10 @@ public void testOAuthTokenValidity() throws IOException { @Test(description = "Revoke valid tokens") public void testOAuthTokenRevocation() throws IOException { + String[] credentials = getClientCredentials(); + String clientId = credentials[0]; + String clientSecret = credentials[1]; + String token = requestToken(CLIENT_ID_VALIDITY); String currentUserAPIPath = context.getBaseUrl() + "api/content/currentuser"; @@ -287,7 +292,7 @@ public void testOAuthTokenRevocation() throws IOException { assertEquals(200, response.getStatusLine().getStatusCode()); // Now revoke the token. - response = revokeOauthToken(token); + response = revokeOauthToken(token, clientId, clientSecret); assertResponse(response, 200, "Token revocation should return 200"); // The token should not work now. @@ -297,10 +302,20 @@ public void testOAuthTokenRevocation() throws IOException { @Test(description = "Revoke invalid tokens") public void testOAuthInvalidTokenRevocation() throws IOException { - HttpResponse response = revokeOauthToken("bad token"); + String[] credentials = getClientCredentials(); + String clientId = credentials[0]; + String clientSecret = credentials[1]; + + HttpResponse response = revokeOauthToken("bad token", clientId, clientSecret); assertResponse(response, 200, "Revoking invalid token should return 200"); } + @Test(description = "Revoke token with bad credentials") + public void testOAuthTokenRevocationBadCred() throws IOException { + HttpResponse response = revokeOauthToken("token", "bad client ID", "bad client secret"); + assertResponse(response, 401, "Revoking token without correct credentials should return 401"); + } + private HttpResponse rawTokenExecute(HttpUriRequest request, String rawToken) throws IOException { final Header tokenHeader = new BasicHeader("X-Authorization", rawToken); request.setHeader(tokenHeader); @@ -311,12 +326,25 @@ private HttpResponse rawTokenExecute(String requestUrl, String token) throws IOE return rawTokenExecute(new HttpGet(requestUrl), "access_token=" + token); } - private HttpResponse revokeOauthToken(String token) throws IOException { + private HttpResponse revokeOauthToken(String token, String clientId, String clientSecret) + throws IOException { HttpPost post = new HttpPost(context.getBaseUrl() + TOKEN_REVOCATION); + post.addHeader( + "Authorization", "Basic " + Base64.encode((clientId + ":" + clientSecret).getBytes())); UrlEncodedFormEntity payload = new UrlEncodedFormEntity(Collections.singletonList(new BasicNameValuePair("token", token))); post.setEntity(payload); return execute(post, false); } + + private String[] getClientCredentials() { + logon(); + OAuthSettingsPage oAuthSettingsPage = new OAuthSettingsPage(context).load(); + OAuthClientEditorPage editorPage = oAuthSettingsPage.editClient(CLIENT_ID_VALIDITY); + String clientId = editorPage.getClientId(); + String clientSecret = editorPage.getSecret(); + + return new String[] {clientId, clientSecret}; + } } From 03248cae885e004ce15d47e26e35707da7b642f8 Mon Sep 17 00:00:00 2001 From: Penghai Date: Wed, 22 Nov 2023 13:40:17 +1100 Subject: [PATCH 08/14] chore: use java.util.Base64 to encode and decode --- .../src/com/tle/web/oauth/servlet/OAuthTokenServlet.java | 6 +++--- .../com/tle/webtests/test/webservices/rest/OAuthTest.java | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java index 67d9350940..49ea245403 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java @@ -19,7 +19,6 @@ package com.tle.web.oauth.servlet; import com.dytech.edge.exceptions.WebException; -import com.rometools.rome.io.impl.Base64; import com.tle.common.Check; import com.tle.core.encryption.EncryptionService; import com.tle.core.guice.Bind; @@ -34,6 +33,7 @@ import com.tle.web.oauth.service.OAuthWebService.AuthorisationDetails; import java.io.IOException; import java.time.Instant; +import java.util.Base64; import java.util.Optional; import javax.inject.Inject; import javax.inject.Singleton; @@ -173,8 +173,8 @@ else if (isClientCredentials) { private boolean validateCredentials(String auth) { if (auth.toLowerCase().startsWith(OAuthWebConstants.BASIC_AUTHORIZATION_PREFIX)) { // Remove the prefix `Basic `. - String decoded = Base64.decode(auth.substring(6)); - String[] credentials = decoded.split(":"); + byte[] decoded = Base64.getDecoder().decode(auth.substring(6)); + String[] credentials = new String(decoded).split(":"); if (credentials.length >= 2) { String clientId = credentials[0]; String clientSecret = credentials[1]; diff --git a/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java b/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java index 5551556e52..0d3181e55a 100644 --- a/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java +++ b/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java @@ -12,9 +12,9 @@ import com.tle.webtests.pageobject.oauth.OAuthSettingsPage; import com.tle.webtests.pageobject.oauth.OAuthTokenRedirect; import com.tle.webtests.test.users.TokenSecurity; -import com.unboundid.util.Base64; import java.io.IOException; import java.text.ParseException; +import java.util.Base64; import java.util.Collections; import java.util.List; import java.util.UUID; @@ -330,7 +330,8 @@ private HttpResponse revokeOauthToken(String token, String clientId, String clie throws IOException { HttpPost post = new HttpPost(context.getBaseUrl() + TOKEN_REVOCATION); post.addHeader( - "Authorization", "Basic " + Base64.encode((clientId + ":" + clientSecret).getBytes())); + "Authorization", + "Basic " + Base64.getEncoder().encodeToString((clientId + ":" + clientSecret).getBytes())); UrlEncodedFormEntity payload = new UrlEncodedFormEntity(Collections.singletonList(new BasicNameValuePair("token", token))); post.setEntity(payload); From fabe72d2e1ec3a0c3f820adbadce3e51da4fc4fb Mon Sep 17 00:00:00 2001 From: Penghai Date: Wed, 22 Nov 2023 13:52:59 +1100 Subject: [PATCH 09/14] test: slightly simplify OauthTest by reusing variables --- .../test/webservices/rest/OAuthTest.java | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java b/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java index 0d3181e55a..ded9dbe3d0 100644 --- a/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java +++ b/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java @@ -35,6 +35,7 @@ public class OAuthTest extends AbstractRestApiTest { private static final String CLIENT_ID_SERVER_FLOW = "testOAuthServerSideFlowClient"; private static final String CLIENT_ID = "testOAuthTokenLoginClient"; private static final String CLIENT_ID_VALIDITY = "testOAuthTokenValidityClient"; + private String clientSecretValidity; private static final String REDIRECT_URI = "default"; private static final String TOKEN_REVOCATION = "oauth/revoke"; @@ -259,7 +260,7 @@ public void testOAuthTokenValidity() throws IOException { OAuthSettingsPage oAuthSettingsPage = new OAuthSettingsPage(context).load(); OAuthClientEditorPage editorPage = oAuthSettingsPage.editClient(CLIENT_ID_VALIDITY); - String secret = editorPage.getSecret(); + clientSecretValidity = editorPage.getSecret(); editorPage.setValidity(validity); editorPage.save(); @@ -269,7 +270,7 @@ public void testOAuthTokenValidity() throws IOException { + "oauth/access_token?grant_type=client_credentials&client_id=" + CLIENT_ID_VALIDITY + "&redirect_uri=default&client_secret=" - + secret; + + clientSecretValidity; final HttpResponse response = execute(new HttpGet(tokenGetUrl), false); final JsonNode tokenNode = readJson(mapper, response); @@ -278,12 +279,10 @@ public void testOAuthTokenValidity() throws IOException { Assert.assertEquals(days, validity); } - @Test(description = "Revoke valid tokens") + @Test( + description = "Revoke valid tokens", + dependsOnMethods = {"testOAuthTokenValidity"}) public void testOAuthTokenRevocation() throws IOException { - String[] credentials = getClientCredentials(); - String clientId = credentials[0]; - String clientSecret = credentials[1]; - String token = requestToken(CLIENT_ID_VALIDITY); String currentUserAPIPath = context.getBaseUrl() + "api/content/currentuser"; @@ -292,7 +291,7 @@ public void testOAuthTokenRevocation() throws IOException { assertEquals(200, response.getStatusLine().getStatusCode()); // Now revoke the token. - response = revokeOauthToken(token, clientId, clientSecret); + response = revokeOauthToken(token, CLIENT_ID_VALIDITY, clientSecretValidity); assertResponse(response, 200, "Token revocation should return 200"); // The token should not work now. @@ -300,13 +299,9 @@ public void testOAuthTokenRevocation() throws IOException { assertEquals(403, response.getStatusLine().getStatusCode()); } - @Test(description = "Revoke invalid tokens") + @Test(description = "Revoke invalid tokens", dependsOnMethods = "testOAuthTokenValidity") public void testOAuthInvalidTokenRevocation() throws IOException { - String[] credentials = getClientCredentials(); - String clientId = credentials[0]; - String clientSecret = credentials[1]; - - HttpResponse response = revokeOauthToken("bad token", clientId, clientSecret); + HttpResponse response = revokeOauthToken("bad token", CLIENT_ID_VALIDITY, clientSecretValidity); assertResponse(response, 200, "Revoking invalid token should return 200"); } From 9efd1afa15c02392a93f6517220e58dddabf6d05 Mon Sep 17 00:00:00 2001 From: Penghai Date: Wed, 22 Nov 2023 14:31:40 +1100 Subject: [PATCH 10/14] refactor: update how to get Oauth client ID and secret from a decoded string A client ID can have colons so we need to use index of the last colon to split the decoded string. --- .../com/tle/web/oauth/servlet/OAuthTokenServlet.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java index 49ea245403..a907e577d5 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java @@ -173,11 +173,12 @@ else if (isClientCredentials) { private boolean validateCredentials(String auth) { if (auth.toLowerCase().startsWith(OAuthWebConstants.BASIC_AUTHORIZATION_PREFIX)) { // Remove the prefix `Basic `. - byte[] decoded = Base64.getDecoder().decode(auth.substring(6)); - String[] credentials = new String(decoded).split(":"); - if (credentials.length >= 2) { - String clientId = credentials[0]; - String clientSecret = credentials[1]; + String decoded = new String(Base64.getDecoder().decode(auth.substring(6))); + // Use index of the last colon because a Client ID may have colons. + int delimiterIndex = decoded.lastIndexOf(":"); + if (delimiterIndex > 0) { + String clientId = decoded.substring(0, delimiterIndex); + String clientSecret = decoded.substring(delimiterIndex + 1); IOAuthClient client = oauthWebService.getByClientIdOnly(clientId); From 38e02df56e575c0fe2695ec70a4c7d13e7df03ec Mon Sep 17 00:00:00 2001 From: Penghai Date: Fri, 8 Dec 2023 17:37:47 +1100 Subject: [PATCH 11/14] fix: make sure OAuth token can be only revoked by the client which generates the token --- .../web/oauth/service/OAuthWebService.java | 5 +++-- .../oauth/service/OAuthWebServiceImpl.java | 13 ++++++++--- .../web/oauth/servlet/OAuthTokenServlet.java | 16 ++++++-------- .../test/webservices/rest/OAuthTest.java | 22 ++++++++++++++++--- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/service/OAuthWebService.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/service/OAuthWebService.java index 344f295f42..c71673e125 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/service/OAuthWebService.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/service/OAuthWebService.java @@ -103,11 +103,12 @@ List> getOauthSignatureParams( IOAuthClient getByClientIdAndRedirectUrl(String clientId, String redirectUrl); /** - * Revoke a token from the current Institution. + * Revoke a token if the token was generated by the provided client. * * @param token Token to be revoked - may be invalid already + * @param clientId ID of the Client where the token was generated */ - void revokeToken(String token); + void revokeTokenForClient(String token, String clientId); class AuthorisationDetails { private String userId; diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/service/OAuthWebServiceImpl.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/service/OAuthWebServiceImpl.java index 3ac4ec4531..d966b5c158 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/service/OAuthWebServiceImpl.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/service/OAuthWebServiceImpl.java @@ -26,6 +26,7 @@ import com.tle.common.ExpiringValue; import com.tle.common.institution.CurrentInstitution; import com.tle.common.oauth.beans.OAuthClient; +import com.tle.common.oauth.beans.OAuthToken; import com.tle.common.usermanagement.user.UserState; import com.tle.common.usermanagement.user.valuebean.UserBean; import com.tle.core.encryption.EncryptionService; @@ -255,9 +256,15 @@ public IOAuthClient getByClientIdAndRedirectUrl(String clientId, String redirect } @Override - public void revokeToken(String token) { - LOGGER.info("OAUTH token revoked: " + token); - oauthService.deleteToken(token); + public void revokeTokenForClient(String token, String clientId) { + Optional.ofNullable(oauthService.getToken(token)) + .map(OAuthToken::getClient) + .filter(c -> c.getClientId().equals(clientId)) + .ifPresent( + (client) -> { + oauthService.deleteToken(token); + LOGGER.info("OAUTH token revoked: " + token); + }); } /** Throw an exception if any SINGLE_PARAMETERS occur repeatedly. */ diff --git a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java index a907e577d5..0ed2fae856 100644 --- a/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java +++ b/Source/Plugins/Core/com.equella.core/src/com/tle/web/oauth/servlet/OAuthTokenServlet.java @@ -170,7 +170,7 @@ else if (isClientCredentials) { } } - private boolean validateCredentials(String auth) { + private Optional validateCredentials(String auth) { if (auth.toLowerCase().startsWith(OAuthWebConstants.BASIC_AUTHORIZATION_PREFIX)) { // Remove the prefix `Basic `. String decoded = new String(Base64.getDecoder().decode(auth.substring(6))); @@ -183,14 +183,12 @@ private boolean validateCredentials(String auth) { IOAuthClient client = oauthWebService.getByClientIdOnly(clientId); return Optional.ofNullable(client) - .map(IOAuthClient::getClientSecret) - .map(secret -> encryptionService.decrypt(secret)) - .map(secret -> secret.equals(clientSecret)) - .orElse(false); + .filter(c -> encryptionService.decrypt(c.getClientSecret()).equals(clientSecret)) + .map(IOAuthClient::getClientId); } } - return false; + return Optional.empty(); } /** @@ -204,11 +202,11 @@ private boolean validateCredentials(String auth) { */ private void revokeToken(HttpServletRequest request) { Optional.ofNullable(request.getHeader(OAuthWebConstants.HEADER_AUTHORIZATION)) - .filter(this::validateCredentials) + .flatMap(this::validateCredentials) .ifPresentOrElse( - (auth) -> { + (client) -> { String token = getParameter(request, TOKEN_PARAM, true); - oauthWebService.revokeToken(token); + oauthWebService.revokeTokenForClient(token, client); }, () -> { throw new OAuthException( diff --git a/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java b/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java index ded9dbe3d0..85f280dcf8 100644 --- a/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java +++ b/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java @@ -279,9 +279,7 @@ public void testOAuthTokenValidity() throws IOException { Assert.assertEquals(days, validity); } - @Test( - description = "Revoke valid tokens", - dependsOnMethods = {"testOAuthTokenValidity"}) + @Test(description = "Revoke valid tokens", dependsOnMethods = "testOAuthTokenValidity") public void testOAuthTokenRevocation() throws IOException { String token = requestToken(CLIENT_ID_VALIDITY); String currentUserAPIPath = context.getBaseUrl() + "api/content/currentuser"; @@ -311,6 +309,24 @@ public void testOAuthTokenRevocationBadCred() throws IOException { assertResponse(response, 401, "Revoking token without correct credentials should return 401"); } + @Test( + description = "Credentials are good, but the token was not generate from this client.", + dependsOnMethods = "testOAuthTokenValidity") + public void testTokenRevocationWrongClient() throws IOException { + String token = requestToken(CLIENT_ID); + HttpResponse response = + revokeOauthToken(requestToken(CLIENT_ID), CLIENT_ID_VALIDITY, clientSecretValidity); + assertResponse( + response, + 200, + "Good credentials should return 200 although the token was not generated from the client"); + + // The token should still work. + String currentUserAPIPath = context.getBaseUrl() + "api/content/currentuser"; + response = rawTokenExecute(currentUserAPIPath, token); + assertEquals(200, response.getStatusLine().getStatusCode()); + } + private HttpResponse rawTokenExecute(HttpUriRequest request, String rawToken) throws IOException { final Header tokenHeader = new BasicHeader("X-Authorization", rawToken); request.setHeader(tokenHeader); From 5fad1488ffb5b11f92be8e1d581ae24635a56ed7 Mon Sep 17 00:00:00 2001 From: Penghai Date: Fri, 8 Dec 2023 19:50:36 +1100 Subject: [PATCH 12/14] test: revoke the token generated already --- .../java/com/tle/webtests/test/webservices/rest/OAuthTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java b/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java index 85f280dcf8..3f6bd566f3 100644 --- a/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java +++ b/autotest/OldTests/src/test/java/com/tle/webtests/test/webservices/rest/OAuthTest.java @@ -314,8 +314,7 @@ public void testOAuthTokenRevocationBadCred() throws IOException { dependsOnMethods = "testOAuthTokenValidity") public void testTokenRevocationWrongClient() throws IOException { String token = requestToken(CLIENT_ID); - HttpResponse response = - revokeOauthToken(requestToken(CLIENT_ID), CLIENT_ID_VALIDITY, clientSecretValidity); + HttpResponse response = revokeOauthToken(token, CLIENT_ID_VALIDITY, clientSecretValidity); assertResponse( response, 200, From e84c6f0dfaabe8198909251196e2da902bc87847 Mon Sep 17 00:00:00 2001 From: Penghai Date: Thu, 14 Dec 2023 09:48:32 +1100 Subject: [PATCH 13/14] build: update GitLab CI ref to main --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4f7ceca4a9..669c14abda 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,4 @@ include: project: edalex-group/development/oeq/openequella-ci file: build/main.yml - ref: 6ef017a59f7eda0c6bf74cceba7020cf72fb37bf + ref: main From ca0c0ce61812d984784b48432b0eff450ce86d8c Mon Sep 17 00:00:00 2001 From: Penghai Date: Thu, 14 Dec 2023 09:51:57 +1100 Subject: [PATCH 14/14] build: reset the version stream to Alpha --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 524726dca6..7549aaa73c 100644 --- a/build.sbt +++ b/build.sbt @@ -129,7 +129,7 @@ name := "Equella" (ThisBuild / equellaMajor) := 2024 (ThisBuild / equellaMinor) := 1 (ThisBuild / equellaPatch) := 0 -(ThisBuild / equellaStream) := "Stable" +(ThisBuild / equellaStream) := "Alpha" (ThisBuild / equellaBuild) := buildConfig.value.getString("build.buildname") (ThisBuild / buildTimestamp) := Instant.now().getEpochSecond