This is an automated email from the ASF dual-hosted git repository. dubeejw pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git
The following commit(s) were added to refs/heads/master by this push: new 7c4bb84 Echo Access-Control-Request-Headers as Access-Control-Allow-Headers instead of hardcoding value for header. (#2771) 7c4bb84 is described below commit 7c4bb84b4580093668ab435fd6ba079ed115160d Author: rodric rabbah <rod...@gmail.com> AuthorDate: Wed Sep 20 14:40:19 2017 -0400 Echo Access-Control-Request-Headers as Access-Control-Allow-Headers instead of hardcoding value for header. (#2771) --- .../scala/whisk/core/controller/WebActions.scala | 27 +++++--- docs/webactions.md | 4 +- .../whisk/core/cli/test/WskWebActionsTests.scala | 25 +++++-- .../core/controller/test/WebActionsApiTests.scala | 80 +++++++++++----------- 4 files changed, 83 insertions(+), 53 deletions(-) diff --git a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala index 2e2ba84..0b42d40 100644 --- a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala +++ b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala @@ -76,6 +76,7 @@ protected[controller] sealed class WebApiDirectives(prefix: String = "__ow_") { val body: String = fields("body") lazy val reservedProperties: Set[String] = Set(method, headers, path, namespace, query, body) + protected final def fields(f: String) = s"$prefix$f" } @@ -85,7 +86,6 @@ private case class Context(propertyMap: WebApiDirectives, path: String, query: Query, body: Option[JsValue] = None) { - val queryAsMap = query.toMap // returns true iff the attached query and body parameters contain a property @@ -171,9 +171,9 @@ protected[core] object WhiskWebActionsApi extends Directives { /** * Supported extensions, their default projection and transcoder to complete a request. * - * @param extension the supported media types for action response - * @param defaultProject the default media extensions for action projection - * @param transcoder the HTTP decoder and terminator for the extension + * @param extension the supported media types for action response + * @param defaultProjection the default media extensions for action projection + * @param transcoder the HTTP decoder and terminator for the extension */ protected case class MediaExtension(extension: String, defaultProjection: Option[List[String]], @@ -366,10 +366,18 @@ trait WhiskWebActionsApi extends Directives with ValidateRequestSize with PostAc private lazy val validNameSegment = pathPrefix(EntityName.REGEX.r) private lazy val packagePrefix = pathPrefix("default".r | EntityName.REGEX.r) - private val defaultCorsResponse = List( - `Access-Control-Allow-Origin`.*, - `Access-Control-Allow-Methods`(OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH), - `Access-Control-Allow-Headers`(`Authorization`.name, `Content-Type`.name)) + private val defaultCorsBaseResponse = + List(`Access-Control-Allow-Origin`.*, `Access-Control-Allow-Methods`(OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH)) + + private val defaultCorsWithAllowHeader = { + defaultCorsBaseResponse :+ `Access-Control-Allow-Headers`(`Authorization`.name, `Content-Type`.name) + } + + private def defaultCorsResponse(headers: Seq[HttpHeader]): List[HttpHeader] = { + headers.find(_.name == `Access-Control-Request-Headers`.name).map { h => + defaultCorsBaseResponse :+ `Access-Control-Allow-Headers`(h.value) + } getOrElse defaultCorsWithAllowHeader + } private def contentTypeFromEntity(entity: HttpEntity) = entity.contentType match { case ct if ct == ContentTypes.NoContentType => None @@ -388,6 +396,7 @@ trait WhiskWebActionsApi extends Directives with ValidateRequestSize with PostAc } def routes(user: Identity)(implicit transid: TransactionId): Route = routes(Some(user)) + def routes()(implicit transid: TransactionId): Route = routes(None) private val maxWaitForWebActionResult = Some(WhiskActionsApi.maxWaitForBlockingActivation) @@ -469,7 +478,7 @@ trait WhiskWebActionsApi extends Directives with ValidateRequestSize with PostAc onComplete(verifyWebAction(fullActionName, onBehalfOf.isDefined)) { case Success((actionOwnerIdentity, action)) => if (!action.annotations.asBool("web-custom-options").exists(identity)) { - respondWithHeaders(defaultCorsResponse) { + respondWithHeaders(defaultCorsResponse(context.headers)) { if (context.method == OPTIONS) { complete(OK, HttpEntity.Empty) } else { diff --git a/docs/webactions.md b/docs/webactions.md index b389a8c..5f69137 100644 --- a/docs/webactions.md +++ b/docs/webactions.md @@ -393,11 +393,13 @@ $ curl -k -H "content-type: application" -X POST -d "Decoded body" https://${API By default, an OPTIONS request made to a web action will result in CORS headers being automatically added to the response headers. These headers allow all origins and the options, get, delete, post, put, head, and patch HTTP verbs. -The headers are shown below: +In addition, the header `Access-Control-Request-Headers` is echoed back as the header `Access-Control-Allow-Headers` +if it is present in the HTTP request. Otherwise, a default value is generated as shown below. ``` Access-Control-Allow-Origin: * Access-Control-Allow-Methods: OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH +Access-Control-Allow-Headers: Authorization, Content-Type ``` Alternatively, OPTIONS requests can be handled manually by a web action. To enable this option add a diff --git a/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala b/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala index 5ff0c98..45c77ef 100644 --- a/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala +++ b/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala @@ -188,11 +188,28 @@ class WskWebActionsTests extends TestHelpers with WskTestHelpers with RestUtil w action.create(name, file, web = Some("true")) } - val responses = Seq( - RestAssured.given().config(sslconfig).options(s"$url.http"), - RestAssured.given().config(sslconfig).get(s"$url.json")) + Seq( + RestAssured + .given() + .config(sslconfig) + .header("Access-Control-Request-Headers", "x-custom-header") + .options(s"$url.http"), + RestAssured + .given() + .config(sslconfig) + .header("Access-Control-Request-Headers", "x-custom-header") + .get(s"$url.json")).foreach { response => + response.statusCode shouldBe 200 + response.header("Access-Control-Allow-Origin") shouldBe "*" + response.header("Access-Control-Allow-Methods") shouldBe "OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH" + response.header("Access-Control-Allow-Headers") shouldBe "x-custom-header" + response.header("Location") shouldBe null + response.header("Set-Cookie") shouldBe null + } - responses.foreach { response => + Seq( + RestAssured.given().config(sslconfig).options(s"$url.http"), + RestAssured.given().config(sslconfig).get(s"$url.json")).foreach { response => response.statusCode shouldBe 200 response.header("Access-Control-Allow-Origin") shouldBe "*" response.header("Access-Control-Allow-Methods") shouldBe "OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH" diff --git a/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala b/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala index d84e12c..f9efeb3 100644 --- a/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala +++ b/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala @@ -22,15 +22,12 @@ import java.util.Base64 import scala.concurrent.Future import scala.concurrent.duration.FiniteDuration - import org.junit.runner.RunWith import org.scalatest.BeforeAndAfterEach import org.scalatest.junit.JUnitRunner import org.scalatest.Matchers import org.scalatest.FlatSpec - import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport._ - import akka.http.scaladsl.model.FormData import akka.http.scaladsl.model.HttpEntity import akka.http.scaladsl.model.MediaTypes @@ -41,14 +38,11 @@ import akka.http.scaladsl.model.HttpResponse import akka.http.scaladsl.model.Uri.Query import akka.http.scaladsl.server.Route import akka.http.scaladsl.model.HttpMethods -import akka.http.scaladsl.model.headers.`Content-Type` +import akka.http.scaladsl.model.headers.{`Access-Control-Request-Headers`, `Content-Type`, RawHeader} import akka.http.scaladsl.model.ContentTypes -import akka.http.scaladsl.model.headers.RawHeader import akka.http.scaladsl.model.ContentType - import spray.json._ import spray.json.DefaultJsonProtocol._ - import whisk.common.TransactionId import whisk.core.WhiskConfig import whisk.core.controller.Context @@ -80,26 +74,6 @@ import whisk.http.Messages */ @RunWith(classOf[JUnitRunner]) -class WebActionsApiPropertiesTests extends FlatSpec with Matchers with WebActionsApiTests { - override lazy val webInvokePathSegments = Seq("web") - override lazy val webApiDirectives = new WebApiDirectives() - - "properties" should "match verion" in { - webApiDirectives.method shouldBe "__ow_method" - webApiDirectives.headers shouldBe "__ow_headers" - webApiDirectives.path shouldBe "__ow_path" - webApiDirectives.namespace shouldBe "__ow_user" - webApiDirectives.query shouldBe "__ow_query" - webApiDirectives.body shouldBe "__ow_body" - webApiDirectives.statusCode shouldBe "statusCode" - webApiDirectives.enforceExtension shouldBe false - webApiDirectives.reservedProperties shouldBe { - Set("__ow_method", "__ow_headers", "__ow_path", "__ow_user", "__ow_query", "__ow_body") - } - } -} - -@RunWith(classOf[JUnitRunner]) class WebActionsApiCommonTests extends FlatSpec with Matchers { "extension splitter" should "split action name and extension" in { Seq(".http", ".json", ".text", ".html", ".svg").foreach { ext => @@ -127,7 +101,27 @@ class WebActionsApiCommonTests extends FlatSpec with Matchers { } } -trait WebActionsApiTests extends ControllerTestCommon with BeforeAndAfterEach with WhiskWebActionsApi { +@RunWith(classOf[JUnitRunner]) +class WebActionsApiTests extends FlatSpec with Matchers with WebActionsApiBaseTests { + override lazy val webInvokePathSegments = Seq("web") + override lazy val webApiDirectives = new WebApiDirectives() + + "properties" should "match verion" in { + webApiDirectives.method shouldBe "__ow_method" + webApiDirectives.headers shouldBe "__ow_headers" + webApiDirectives.path shouldBe "__ow_path" + webApiDirectives.namespace shouldBe "__ow_user" + webApiDirectives.query shouldBe "__ow_query" + webApiDirectives.body shouldBe "__ow_body" + webApiDirectives.statusCode shouldBe "statusCode" + webApiDirectives.enforceExtension shouldBe false + webApiDirectives.reservedProperties shouldBe { + Set("__ow_method", "__ow_headers", "__ow_path", "__ow_user", "__ow_query", "__ow_body") + } + } +} + +trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEach with WhiskWebActionsApi { val systemId = Subject() val systemKey = AuthKey() val systemIdentity = Future.successful(Identity(systemId, EntityName(systemId.asString), systemKey, Privilege.ALL)) @@ -1447,9 +1441,12 @@ trait WebActionsApiTests extends ControllerTestCommon with BeforeAndAfterEach wi actionResult = Some(JsObject("headers" -> JsObject("Access-Control-Allow-Methods" -> "OPTIONS, GET, PATCH".toJson))) - Options(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check { - header("Access-Control-Allow-Origin") shouldBe None + // the added headers should be ignored + Options(s"$testRoutePath/$path") ~> addHeader(`Access-Control-Request-Headers`("x-custom-header")) ~> Route + .seal(routes(creds)) ~> check { + header("Access-Control-Allow-Origin") shouldBe empty header("Access-Control-Allow-Methods").get.toString shouldBe "Access-Control-Allow-Methods: OPTIONS, GET, PATCH" + header("Access-Control-Request-Headers") shouldBe empty } } } @@ -1474,17 +1471,22 @@ trait WebActionsApiTests extends ControllerTestCommon with BeforeAndAfterEach wi customOptions = false Seq(s"$systemId/proxy/export_c.http", s"$systemId/proxy/export_c.json").foreach { path => - allowedMethods.foreach { m => - invocationsAllowed += 1 - m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check { - header("Access-Control-Allow-Origin").get.toString shouldBe "Access-Control-Allow-Origin: *" - header("Access-Control-Allow-Methods").get.toString shouldBe "Access-Control-Allow-Methods: OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH" - header("Access-Control-Allow-Headers").get.toString shouldBe "Access-Control-Allow-Headers: Authorization, Content-Type" - } + Seq(`Access-Control-Request-Headers`("x-custom-header"), RawHeader("x-custom-header", "value")).foreach { + testHeader => + allowedMethods.foreach { m => + if (m != Options) invocationsAllowed += 1 // options verb does not invoke an action + m(s"$testRoutePath/$path") ~> addHeader(testHeader) ~> Route.seal(routes(creds)) ~> check { + header("Access-Control-Allow-Origin").get.toString shouldBe "Access-Control-Allow-Origin: *" + header("Access-Control-Allow-Methods").get.toString shouldBe "Access-Control-Allow-Methods: OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH" + if (testHeader.name == `Access-Control-Request-Headers`.name) { + header("Access-Control-Allow-Headers").get.toString shouldBe "Access-Control-Allow-Headers: x-custom-header" + } else { + header("Access-Control-Allow-Headers").get.toString shouldBe "Access-Control-Allow-Headers: Authorization, Content-Type" + } + } + } } } - - invocationsAllowed -= 2 // Options request does not cause invocation of an action } it should s"invoke action with head verb (auth? ${creds.isDefined})" in { -- To stop receiving notification emails like this one, please contact ['"commits@openwhisk.apache.org" <commits@openwhisk.apache.org>'].