From 12c2be576a664891f1eedc77eb3f379eec748e1f Mon Sep 17 00:00:00 2001 From: Brian Reilly Date: Tue, 22 Feb 2022 23:17:15 +0000 Subject: [PATCH 01/19] Update cromwell version from 77 to 78 --- project/Version.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Version.scala b/project/Version.scala index 1d03339842f..fb665bce138 100644 --- a/project/Version.scala +++ b/project/Version.scala @@ -5,7 +5,7 @@ import sbt._ object Version { // Upcoming release, or current if we're on a master / hotfix branch - val cromwellVersion = "77" + val cromwellVersion = "78" /** * Returns true if this project should be considered a snapshot. From 24b27a2ff443ad76f760f5293461a3561098f17a Mon Sep 17 00:00:00 2001 From: Miguel Covarrubias Date: Wed, 23 Feb 2022 19:27:30 -0500 Subject: [PATCH 02/19] Cromwell Swagger UI Restructuring [CROM-6862] (#6682) --- .../cromwell/webservice/SwaggerService.scala | 4 - .../webservice/SwaggerUiHttpService.scala | 126 ++++++++---------- .../webservice/SwaggerServiceSpec.scala | 13 -- .../webservice/SwaggerUiHttpServiceSpec.scala | 113 +++------------- 4 files changed, 74 insertions(+), 182 deletions(-) diff --git a/engine/src/main/scala/cromwell/webservice/SwaggerService.scala b/engine/src/main/scala/cromwell/webservice/SwaggerService.scala index 0285b3d83f7..182da3ac36c 100644 --- a/engine/src/main/scala/cromwell/webservice/SwaggerService.scala +++ b/engine/src/main/scala/cromwell/webservice/SwaggerService.scala @@ -1,9 +1,5 @@ package cromwell.webservice -import cromwell.webservice.routes.CromwellApiService - trait SwaggerService extends SwaggerUiResourceHttpService { override def swaggerServiceName = "cromwell" - - override def swaggerUiVersion = CromwellApiService.swaggerUiVersion } diff --git a/engine/src/main/scala/cromwell/webservice/SwaggerUiHttpService.scala b/engine/src/main/scala/cromwell/webservice/SwaggerUiHttpService.scala index f366808e7be..d63ab3bcb51 100644 --- a/engine/src/main/scala/cromwell/webservice/SwaggerUiHttpService.scala +++ b/engine/src/main/scala/cromwell/webservice/SwaggerUiHttpService.scala @@ -1,51 +1,38 @@ package cromwell.webservice import akka.http.scaladsl.model.{HttpResponse, StatusCodes} +import akka.http.scaladsl.server import akka.http.scaladsl.server.Directives._ import akka.http.scaladsl.server.Route import akka.stream.scaladsl.Flow import akka.util.ByteString +import cromwell.webservice.routes.CromwellApiService /** - * Serves up the swagger UI from org.webjars/swagger-ui. + * Serves up the swagger UI from org.webjars/swagger-ui, lightly editing the index.html. */ trait SwaggerUiHttpService { - /** - * @return The version of the org.webjars/swagger-ui artifact. For example "2.1.1". - */ - def swaggerUiVersion: String - - /** - * Informs the swagger UI of the base of the application url, as hosted on the server. - * If your entire app is served under "http://myserver/myapp", then the base URL is "/myapp". - * If the app is served at the root of the application, leave this value as the empty string. - * - * @return The base URL used by the application, or the empty string if there is no base URL. For example "/myapp". - */ - def swaggerUiBaseUrl: String = "" - - /** - * @return The path to the swagger UI html documents. For example "swagger" - */ - def swaggerUiPath: String = "swagger" - /** - * The path to the actual swagger documentation in either yaml or json, to be rendered by the swagger UI html. - * - * @return The path to the api documentation to render in the swagger UI. - * For example "api-docs" or "swagger/common.yaml". - */ - def swaggerUiDocsPath: String = "api-docs" - - /** - * @return When true, if someone requests / (or /baseUrl if setup), redirect to the swagger UI. - */ - def swaggerUiFromRoot: Boolean = true - - private def routeFromRoot: Route = get { - pathEndOrSingleSlash { - // Redirect / to the swagger UI - redirect(s"$swaggerUiBaseUrl/$swaggerUiPath", StatusCodes.TemporaryRedirect) + private lazy val resourceDirectory = s"META-INF/resources/webjars/swagger-ui/${CromwellApiService.swaggerUiVersion}" + + private val serveIndex: server.Route = { + val swaggerOptions = + """ + | validatorUrl: null, + | apisSorter: "alpha", + | operationsSorter: "alpha" + """.stripMargin + + mapResponseEntity { entityFromJar => + entityFromJar.transformDataBytes(Flow.fromFunction[ByteString, ByteString] { original: ByteString => + ByteString( + original.utf8String + .replace("""url: "https://petstore.swagger.io/v2/swagger.json"""", "url: 'cromwell.yaml'") + .replace("""layout: "StandaloneLayout"""", s"""layout: "StandaloneLayout", $swaggerOptions""") + ) + }) + } { + getFromResource(s"$resourceDirectory/index.html") } } @@ -55,21 +42,32 @@ trait SwaggerUiHttpService { * @return Route serving the swagger UI. */ final def swaggerUiRoute: Route = { - val route = get { - pathPrefix(separateOnSlashes(swaggerUiPath)) { - // when the user hits the doc url, redirect to the index.html with api docs specified on the url - pathEndOrSingleSlash { - redirect( - s"$swaggerUiBaseUrl/$swaggerUiPath/index.html?url=$swaggerUiBaseUrl/$swaggerUiDocsPath", - StatusCodes.TemporaryRedirect) - } ~ getFromResourceDirectory(s"META-INF/resources/webjars/swagger-ui/$swaggerUiVersion") + pathEndOrSingleSlash { + get { + serveIndex + } + } ~ + // We have to be explicit about the paths here since we're matching at the root URL and we don't + // want to catch all paths lest we circumvent Spray's not-found and method-not-allowed error + // messages. + (pathPrefixTest("swagger-ui") | pathPrefixTest("oauth2") | pathSuffixTest("js") + | pathSuffixTest("css") | pathPrefixTest("favicon")) { + get { + getFromResourceDirectory(resourceDirectory) + } + } ~ + // Redirect legacy `/swagger` or `/swagger/index.html?url=/swagger/cromwell.yaml#fragment` requests to the root + // URL. The latter form is (somewhat magically) well-behaved in throwing away the `url` query parameter that was + // the subject of the CVE linked below while preserving any fragment identifiers to scroll to the right spot in + // the Swagger UI. + // https://github.com/swagger-api/swagger-ui/security/advisories/GHSA-qrmm-w75w-3wpx + (path("swagger" / "index.html") | path ("swagger")) { + get { + redirect("/", StatusCodes.MovedPermanently) + } } - } - if (swaggerUiFromRoot) route ~ routeFromRoot else route } - } - /** * An extension of HttpService to serve up a resource containing the swagger api as yaml or json. The resource * directory and path on the classpath must match the path for route. The resource can be any file type supported by the @@ -84,7 +82,7 @@ trait SwaggerResourceHttpService { /** * @return The directory for the resource under the classpath, and in the url */ - def swaggerDirectory: String = "swagger" + private val swaggerDirectory: String = "swagger" /** * @return Name of the service, used to map the documentation resource at "/uiPath/serviceName.resourceType". @@ -96,29 +94,17 @@ trait SwaggerResourceHttpService { */ def swaggerResourceType: String = "yaml" - /** - * Swagger UI sends HTTP OPTIONS before ALL requests, and expects a status 200 / OK. When true (the default) the - * swaggerResourceRoute will return 200 / OK for requests for OPTIONS. - * - * See also: - * - https://github.com/swagger-api/swagger-ui/issues/1209 - * - https://github.com/swagger-api/swagger-ui/issues/161 - * - https://groups.google.com/forum/#!topic/swagger-swaggersocket/S6_I6FBjdZ8 - * - * @return True if status code 200 should be returned for HTTP OPTIONS requests for the swagger resource. - */ - def swaggerAllOptionsOk: Boolean = true - /** * @return The path to the swagger docs. */ - protected def swaggerDocsPath = s"$swaggerDirectory/$swaggerServiceName.$swaggerResourceType" + private lazy val swaggerDocsPath = s"$swaggerDirectory/$swaggerServiceName.$swaggerResourceType" /** * @return A route that returns the swagger resource. */ final def swaggerResourceRoute: Route = { - val swaggerDocsDirective = path(separateOnSlashes(swaggerDocsPath)) + // Serve Cromwell API docs from either `/swagger/cromwell.yaml` or just `cromwell.yaml`. + val swaggerDocsDirective = path(separateOnSlashes(swaggerDocsPath)) | path(s"$swaggerServiceName.$swaggerResourceType") def injectBasePath(basePath: Option[String])(response: HttpResponse): HttpResponse = { basePath match { @@ -138,12 +124,10 @@ trait SwaggerResourceHttpService { } } - if (swaggerAllOptionsOk) { - route ~ options { - // Also return status 200 / OK for all OPTIONS requests. - complete(StatusCodes.OK) - } - } else route + route ~ options { + // Also return status 200 / OK for all OPTIONS requests. + complete(StatusCodes.OK) + } } } @@ -151,8 +135,6 @@ trait SwaggerResourceHttpService { * Extends the SwaggerUiHttpService and SwaggerResourceHttpService to serve up both. */ trait SwaggerUiResourceHttpService extends SwaggerUiHttpService with SwaggerResourceHttpService { - override def swaggerUiDocsPath = swaggerDocsPath - /** * @return A route that redirects to the swagger UI and returns the swagger resource. */ diff --git a/engine/src/test/scala/cromwell/webservice/SwaggerServiceSpec.scala b/engine/src/test/scala/cromwell/webservice/SwaggerServiceSpec.scala index f4150094184..f68189ea66e 100644 --- a/engine/src/test/scala/cromwell/webservice/SwaggerServiceSpec.scala +++ b/engine/src/test/scala/cromwell/webservice/SwaggerServiceSpec.scala @@ -69,19 +69,6 @@ class SwaggerServiceSpec extends AnyFlatSpec with CromwellTimeoutSpec with Swagg } } - it should "return the index.html" in { - Get("/swagger/index.html?url=/swagger/cromwell.yaml") ~> - swaggerUiResourceRoute ~> - check { - assertResult(StatusCodes.OK) { - status - } - assertResult("") { - responseAs[String].take(50) - } - assertResult(ContentTypes.`text/html(UTF-8)`)(contentType) - } - } - it should "return status OK when getting OPTIONS on paths" in { val pathExamples = Table("path", "/", "/swagger", "/swagger/cromwell.yaml", "/swagger/index.html", "/api", "/api/workflows/", "/api/workflows/v1", "/workflows/v1/outputs", "/workflows/v1/status", diff --git a/CromIAM/src/test/scala/cromiam/webservice/SwaggerUiHttpServiceSpec.scala b/CromIAM/src/test/scala/cromiam/webservice/SwaggerUiHttpServiceSpec.scala index e95b2fda630..1d0103a09fd 100644 --- a/CromIAM/src/test/scala/cromiam/webservice/SwaggerUiHttpServiceSpec.scala +++ b/CromIAM/src/test/scala/cromiam/webservice/SwaggerUiHttpServiceSpec.scala @@ -6,7 +6,6 @@ import akka.http.scaladsl.server.Route import akka.http.scaladsl.testkit.ScalatestRouteTest import common.assertion.CromwellTimeoutSpec import cromiam.server.config.SwaggerOauthConfig -import cromiam.webservice.SwaggerUiHttpServiceSpec._ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers import org.scalatest.prop.TableDrivenPropertyChecks @@ -14,8 +13,6 @@ import org.scalatest.prop.TableDrivenPropertyChecks trait SwaggerUiHttpServiceSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers with ScalatestRouteTest with SwaggerUiHttpService { def actorRefFactory = system - - override def swaggerUiVersion = TestSwaggerUiVersion } trait SwaggerResourceHttpServiceSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers with ScalatestRouteTest with @@ -46,47 +43,18 @@ class BasicSwaggerUiHttpServiceSpec extends SwaggerUiHttpServiceSpec { // Replace same magic string used in SwaggerUiResourceHttpService.rewriteSwaggerIndex data.replace("window.ui = ui", "replaced-client-id") - it should "redirect / to /swagger" in { - Get() ~> swaggerUiRoute ~> check { - status should be(StatusCodes.TemporaryRedirect) - header("Location") should be(Option(Location(Uri("/swagger")))) - contentType should be(ContentTypes.`text/html(UTF-8)`) - } - } - - it should "not return options for /" in { - Options() ~> Route.seal(swaggerUiRoute) ~> check { - status should be(StatusCodes.MethodNotAllowed) - contentType should be(ContentTypes.`text/plain(UTF-8)`) - } - } - - it should "redirect /swagger to the index.html" in { + it should "redirect /swagger to /" in { Get("/swagger") ~> swaggerUiRoute ~> check { - status should be(StatusCodes.TemporaryRedirect) - header("Location") should be(Option(Location(Uri("/swagger/index.html?url=/api-docs")))) - contentType should be(ContentTypes.`text/html(UTF-8)`) + status should be(StatusCodes.MovedPermanently) + header("Location") should be(Option(Location(Uri("/")))) } } - it should "return index.html from the swagger-ui jar" in { - Get("/swagger/index.html") ~> swaggerUiRoute ~> check { - status should be(StatusCodes.OK) - responseAs[String] should include("replaced-client-id") - contentType should be(ContentTypes.`text/html(UTF-8)`) - } - } -} - -class NoRedirectRootSwaggerUiHttpServiceSpec extends SwaggerUiHttpServiceSpec { - override def swaggerUiFromRoot = false - - behavior of "SwaggerUiHttpService" - - it should "not redirect / to /swagger" in { - Get() ~> Route.seal(swaggerUiRoute) ~> check { - status should be(StatusCodes.NotFound) - contentType should be(ContentTypes.`text/plain(UTF-8)`) + it should "redirect /swagger/index.html?url=/swagger/cromiam.yaml to /" in { + Get("/swagger/index.html?url=/swagger/cromiam.yaml") ~> swaggerUiRoute ~> check { + status should be(StatusCodes.MovedPermanently) + header("Location") should be(Option(Location(Uri("/")))) + responseAs[String] shouldEqual """This and all future requests should be directed to this URI.""" } } @@ -97,20 +65,23 @@ class NoRedirectRootSwaggerUiHttpServiceSpec extends SwaggerUiHttpServiceSpec { } } - it should "redirect /swagger to the index.html" in { - Get("/swagger") ~> swaggerUiRoute ~> check { - status should be(StatusCodes.TemporaryRedirect) - header("Location") should be(Option(Location(Uri("/swagger/index.html?url=/api-docs")))) - contentType should be(ContentTypes.`text/html(UTF-8)`) + it should "return index.html from the swagger-ui jar for /" in { + Get("/") ~> swaggerUiRoute ~> check { + status should be(StatusCodes.OK) + responseAs[String].take(15) should be("