You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by ra...@apache.org on 2017/06/19 14:23:18 UTC

[incubator-openwhisk] branch master updated: Update OPTIONS Respones for Web Actions (Review) (#2327)

This is an automated email from the ASF dual-hosted git repository.

rabbah 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 5632460  Update OPTIONS Respones for Web Actions (Review) (#2327)
5632460 is described below

commit 5632460c86137a66bf0c9a9532526552a1b4e5b1
Author: James Dubee <jw...@us.ibm.com>
AuthorDate: Mon Jun 19 10:23:14 2017 -0400

    Update OPTIONS Respones for Web Actions (Review) (#2327)
    
    Web actions may now optionally elect to respond to OPTIONS verb.
---
 .../scala/whisk/core/controller/RestAPIs.scala     | 18 +++++-----
 .../scala/whisk/core/controller/WebActions.scala   | 16 ++++++++-
 docs/annotations.md                                | 15 ++++----
 docs/webactions.md                                 | 42 ++++++++++++++++++++++
 .../whisk/core/cli/test/WskWebActionsTests.scala   | 38 ++++++++++----------
 .../core/controller/test/WebActionsApiTests.scala  | 38 ++++++++++++++++----
 6 files changed, 124 insertions(+), 43 deletions(-)

diff --git a/core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala b/core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala
index 56f4c9f..225f981 100644
--- a/core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala
@@ -168,20 +168,22 @@ protected[controller] class RestAPIVersion(apipath: String, apiversion: String)(
                                     rules.routes(user) ~
                                     activations.routes(user) ~
                                     packages.routes(user)
-                            } ~ webexp.routes(user)
-                } ~ {
-                    webexp.routes()
+                            }
                 } ~ {
                     swaggerRoutes
-                } ~ options {
-                    complete(OK)
                 }
             } ~ {
                 // web actions are distinct to separate the cors header
                 // and allow the actions themselves to respond to options
-                authenticate(basicauth) {
-                    user => web.routes(user)
-                } ~ web.routes()
+                authenticate(basicauth) { user =>
+                    web.routes(user) ~ webexp.routes(user)
+                } ~ {
+                    web.routes() ~ webexp.routes()
+                } ~ options {
+                    sendCorsHeaders {
+                        complete(OK)
+                    }
+                }
             }
         }
     }
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 f10cfec..96f9d50 100644
--- a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
@@ -40,6 +40,7 @@ import spray.json.DefaultJsonProtocol._
 import spray.routing.Directives
 import spray.routing.RequestContext
 import spray.routing.Route
+import spray.http.HttpMethods.{ OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH }
 import whisk.common.TransactionId
 import whisk.core.controller.actions.BlockingInvokeTimeout
 import whisk.core.controller.actions.PostActionActivation
@@ -317,6 +318,9 @@ trait WhiskWebActionsApi
     private lazy val validNameSegment = pathPrefix(EntityName.REGEX.r)
     private lazy val packagePrefix = pathPrefix("default".r | EntityName.REGEX.r)
 
+    private val allowOrigin = `Access-Control-Allow-Origin`(AllOrigins)
+    private val allowMethods = `Access-Control-Allow-Methods`(OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH)
+
     /** Extracts the HTTP method, headers, query params and unmatched (remaining) path. */
     private val requestMethodParamsAndPath = {
         extract { ctx =>
@@ -407,7 +411,17 @@ trait WhiskWebActionsApi
                             provide(fullyQualifiedActionName(actionName)) { fullActionName =>
                                 onComplete(verifyWebAction(fullActionName, onBehalfOf.isDefined)) {
                                     case Success((actionOwnerIdentity, action)) =>
-                                        extractEntityAndProcessRequest(actionOwnerIdentity, action, extension, onBehalfOf, context, e)
+                                        context.method match {
+                                            // if options method and action opted into default response, send standard response
+                                            case OPTIONS if !action.annotations.asBool("web-custom-options").exists(identity) =>
+                                                respondWithHeaders(allowOrigin, allowMethods) {
+                                                    complete(OK, HttpEntity.Empty)
+                                                }
+
+                                            // otherwise not an options method, or action will respond to options verb
+                                            case _ =>
+                                                extractEntityAndProcessRequest(actionOwnerIdentity, action, extension, onBehalfOf, context, e)
+                                        }
 
                                     case Failure(t: RejectRequest) =>
                                         terminate(t.code, t.message)
diff --git a/docs/annotations.md b/docs/annotations.md
index 42142b5..588d1f2 100644
--- a/docs/annotations.md
+++ b/docs/annotations.md
@@ -41,10 +41,11 @@ The annotations are _not_ checked. So while it is conceivable to use the annotat
 
 # Annotations specific to web actions
 
-We recently extended the core API with new features. To enable packages and actions to participate in these features, we introduced the following new annotations that are semantically meaningful. These annotations must be explicitly set to `true` to have affect. Changing the value from `true` to `false` will exclude the attached asset from the new API. The annotations have no meaning otherwise in the system. The annotations are:
-
-1. `final`: Applies only to an action. It makes all of the action parameters that are already defined immutable. A parameter of an action carrying the annotation may not be overridden by invoke-time parameters once the parameter has a value defined through its enclosing package or the action definition.
-2. `web-export`: Applies only to an action. If present, it makes its corresponding action accessible to REST calls _without_ authentication. We call these [_web actions_](webactions.md) because they allow one to use OpenWhisk actions from a browser for example. It is important to note that the _owner_ of the web action incurs the cost of running them in the system (i.e., the _owner_ of the action also owns the activations record).
-3. `require-whisk-auth`: Applies onto to an action. If an action carries the `web-export` annotation, and this annotation is also `true`, the route is only accessible to an authenticated subject. It is important to note that the _owner_ of the web action incurs the cost of running them in the system (i.e., the _owner_ of the action also owns the activations record).
-4. `raw-http`: Applies only to an action in the presence of a `web-export` annotation. If present, the HTTP request query and body parameters are passed to the action as reserved properties.
-
+Web actions are enabled with explicit annotations which decorate individual actions. The annotations only apply to the [web actions](webactions.md) API,
+and must be present and explicitly set to `true` to have an affect. The annotations have no meaning otherwise in the system. The annotations are:
+
+1. `web-export`: Makes its corresponding action accessible to REST calls _without_ authentication. We call these [_web actions_](webactions.md) because they allow one to use OpenWhisk actions from a browser for example. It is important to note that the _owner_ of the web action incurs the cost of running them in the system (i.e., the _owner_ of the action also owns the activations record). The rest of the annotations described below have no effect on the action unless this annotation is  [...]
+2. `final`: Makes all of the action parameters that are already defined immutable. A parameter of an action carrying the annotation may not be overridden by invoke-time parameters once the parameter has a value defined through its enclosing package or the action definition.
+3. `raw-http`: When set, the HTTP request query and body parameters are passed to the action as reserved properties.
+4. `web-custom-options`: When set, this annotation enables a web action to respond to OPTIONS requests with customized headers, otherwise a [default CORS response](webactions.md#options-requests) applies.
+5. `require-whisk-auth`: This annotation protects the web action so that it is only accessible to an authenticated subject. It is important to note that the _owner_ of the web action will still incur the cost of running them in the system (i.e., the _owner_ of the action also owns the activations record).
diff --git a/docs/webactions.md b/docs/webactions.md
index 7162904..61bc087 100644
--- a/docs/webactions.md
+++ b/docs/webactions.md
@@ -360,6 +360,48 @@ $ curl -k -H "content-type: application" -X POST -d "Decoded body" https://${API
   "body": "Decoded body"
 }
 ```
+## Options Requests
+
+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:
+
+```
+Access-Control-Allow-Origin: *
+Access-Control-Allow-Methods: OPTIONS, GET, DELETE, POST, PUT, HEAD, PATCH
+```
+
+Alternatively, OPTIONS requests can be handled manually by a web action. To enable this option add a
+`web-custom-options` annotation with a value of `true` to a web action. When this feature is enabled, CORS headers will
+not automatically be added to the request response. Instead, it is the developer's responsibility to append their
+desired headers programmatically. Below is an example of creating custom responses to OPTIONS requests.
+
+```
+function main(params) {
+  if (params.__ow_method == "options") {
+    return {
+      headers: {
+        'Access-Control-Allow-Methods': 'OPTIONS, GET',
+        'Access-Control-Allow-Origin': 'example.com'
+      },
+      statusCode: 200
+    }
+  }
+}
+```
+
+Save the above function to `custom-options.js` and execute the following commands:
+
+```
+$ wsk action create custom-option custom-options.js --web true -a web-custom-options true
+$ curl https://${APIHOST}/api/v1/web/guest/default/custom-options.http -kvX OPTIONS
+< HTTP/1.1 200 OK
+< Server: nginx/1.11.13
+< Content-Length: 0
+< Connection: keep-alive
+< Access-Control-Allow-Methods: OPTIONS, GET
+< Access-Control-Allow-Origin: example.com
+```
 
 ## Error Handling
 
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 547d8eb..31c776a 100644
--- a/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala
+++ b/tests/src/test/scala/whisk/core/cli/test/WskWebActionsTests.scala
@@ -148,7 +148,7 @@ trait WskWebActionsTests
 
             assetHelper.withCleaner(wsk.action, name) {
                 (action, _) =>
-                    action.create(name, file, annotations = Map("web-export" -> true.toJson))
+                    action.create(name, file, web = Some("true"))
             }
 
             val host = getServiceURL()
@@ -187,7 +187,7 @@ trait WskWebActionsTests
 
             assetHelper.withCleaner(wsk.action, name) {
                 (action, _) =>
-                    action.create(name, file, annotations = Map("web-export" -> true.toJson, "require-whisk-auth" -> true.toJson))
+                    action.create(name, file, web = Some("true"), annotations = Map("require-whisk-auth" -> true.toJson))
             }
 
             val host = getServiceURL()
@@ -210,25 +210,23 @@ trait WskWebActionsTests
             authorizedResponse.body.asString shouldBe namespace
     }
 
-    if (testRoutePath == "/api/v1/web") {
-        it should "ensure that CORS header is preserved" in withAssetCleaner(wskprops) {
-            (wp, assetHelper) =>
-                val name = "webaction"
-                val file = Some(TestUtils.getTestActionFilename("corsHeaderMod.js"))
+    it should "ensure that CORS header is preserved for custom options" in withAssetCleaner(wskprops) {
+        (wp, assetHelper) =>
+            val name = "webaction"
+            val file = Some(TestUtils.getTestActionFilename("corsHeaderMod.js"))
 
-                assetHelper.withCleaner(wsk.action, name) {
-                    (action, _) =>
-                        action.create(name, file, annotations = Map("web-export" -> true.toJson))
-                }
+            assetHelper.withCleaner(wsk.action, name) {
+                (action, _) =>
+                    action.create(name, file, web = Some("true"), annotations = Map("web-custom-options" -> true.toJson))
+            }
 
-                val host = getServiceURL()
-                val url = host + s"$testRoutePath/$namespace/default/webaction.http"
+            val host = getServiceURL()
+            val url = host + s"$testRoutePath/$namespace/default/webaction.http"
 
-                val response = RestAssured.given().config(sslconfig).options(url)
-                response.statusCode shouldBe 200
-                response.header("Access-Control-Allow-Origin") shouldBe "Origin set from Web Action"
-                response.header("Access-Control-Allow-Headers") shouldBe "Headers set from Web Action"
-        }
+            val response = RestAssured.given().config(sslconfig).options(url)
+            response.statusCode shouldBe 200
+            response.header("Access-Control-Allow-Origin") shouldBe "Origin set from Web Action"
+            response.header("Access-Control-Allow-Headers") shouldBe "Headers set from Web Action"
     }
 
     it should "invoke web action to ensure the returned body argument is correct" in withAssetCleaner(wskprops) {
@@ -239,7 +237,7 @@ trait WskWebActionsTests
 
             assetHelper.withCleaner(wsk.action, name) {
                 (action, _) =>
-                    action.create(name, file, annotations = Map("web-export" -> true.toJson))
+                    action.create(name, file, web = Some("true"))
             }
 
             val host = getServiceURL()
@@ -265,7 +263,7 @@ trait WskWebActionsTests
 
             assetHelper.withCleaner(wsk.action, name) {
                 (action, _) =>
-                    action.create(name, file, annotations = Map("web-export" -> true.toJson))
+                    action.create(name, file, web = Some("true"))
             }
 
             val host = getServiceURL()
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 b85fc03..c2063dd 100644
--- a/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
@@ -153,11 +153,12 @@ trait WebActionsApiTests extends ControllerTestCommon with BeforeAndAfterEach wi
 
     behavior of "Web actions API"
 
-    var failActionLookup = false // toggle to cause action lookup to fail
-    var failActivation = 0 // toggle to cause action to fail
-    var failThrottleForSubject: Option[Subject] = None // toggle to cause throttle to fail for subject
+    var failActionLookup = false                        // toggle to cause action lookup to fail
+    var failActivation = 0                              // toggle to cause action to fail
+    var failThrottleForSubject: Option[Subject] = None  // toggle to cause throttle to fail for subject
     var actionResult: Option[JsObject] = None
-    var requireAuthentication = false // toggle require-whisk-auth annotation on action
+    var requireAuthentication = false                   // toggle require-whisk-auth annotation on action
+    var customOptions = true                            // toogle web-custom-options annotation on action
     var invocationCount = 0
     var invocationsAllowed = 0
 
@@ -172,6 +173,7 @@ trait WebActionsApiTests extends ControllerTestCommon with BeforeAndAfterEach wi
         failThrottleForSubject = None
         actionResult = None
         requireAuthentication = false
+        customOptions = true
         assert(invocationsAllowed == invocationCount, "allowed invoke count did not match actual")
     }
 
@@ -215,6 +217,10 @@ trait WebActionsApiTests extends ControllerTestCommon with BeforeAndAfterEach wi
                                 if (requireAuthentication) {
                                     Parameters("require-whisk-auth", JsBoolean(true))
                                 } else Parameters()
+                            } ++ {
+                                if (customOptions) {
+                                    Parameters("web-custom-options", JsBoolean(true))
+                                } else Parameters()
                             }
                     } else if (actionName.name.asString.startsWith("raw_export_")) {
                         annotations ++
@@ -223,6 +229,10 @@ trait WebActionsApiTests extends ControllerTestCommon with BeforeAndAfterEach wi
                                 if (requireAuthentication) {
                                     Parameters("require-whisk-auth", JsBoolean(true))
                                 } else Parameters()
+                            } ++ {
+                                if (customOptions) {
+                                    Parameters("web-custom-options", JsBoolean(true))
+                                } else Parameters()
                             }
                     } else annotations
                 })
@@ -1093,7 +1103,7 @@ trait WebActionsApiTests extends ControllerTestCommon with BeforeAndAfterEach wi
                 }
         }
 
-        it should s"invoke action with options verb (auth? ${creds.isDefined})" in {
+        it should s"invoke action with options verb with custom options (auth? ${creds.isDefined})" in {
             implicit val tid = transid()
 
             Seq(s"$systemId/proxy/export_c.http").
@@ -1102,10 +1112,24 @@ trait WebActionsApiTests extends ControllerTestCommon with BeforeAndAfterEach wi
                     actionResult = Some(
                         JsObject(
                             "headers" -> JsObject(
-                                "allow" -> "options, head, get, post, put".toJson)))
+                                "Access-Control-Allow-Methods" -> "OPTIONS, GET, PATCH".toJson)))
 
                     Options(s"$testRoutePath/$path") ~> sealRoute(routes(creds)) ~> check {
-                        header("allow").get.toString shouldBe "allow: options, head, get, post, put"
+                        header("Access-Control-Allow-Origin") shouldBe None
+                        header("Access-Control-Allow-Methods").get.toString shouldBe "Access-Control-Allow-Methods: OPTIONS, GET, PATCH"
+                    }
+                }
+        }
+
+        it should s"invoke action with options verb without custom options (auth? ${creds.isDefined})" in {
+            implicit val tid = transid()
+            customOptions = false
+
+            Seq(s"$systemId/proxy/export_c.http").
+                foreach { path =>
+                    Options(s"$testRoutePath/$path") ~> sealRoute(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"
                     }
                 }
         }

-- 
To stop receiving notification emails like this one, please contact
['"commits@openwhisk.apache.org" <co...@openwhisk.apache.org>'].