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

csantanapr 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 4dfe548  Update require-whisk-auth behavior to secure web action 
(#3388)
4dfe548 is described below

commit 4dfe54812706d55af3619fcc7180bb7bcb708254
Author: Mark Deuser <mdeu...@us.ibm.com>
AuthorDate: Wed Mar 7 18:35:30 2018 -0500

    Update require-whisk-auth behavior to secure web action (#3388)
---
 .../main/scala/whisk/core/entity/WhiskAction.scala |  3 ++
 .../scala/whisk/core/controller/WebActions.scala   | 40 +++++++++++++-
 docs/annotations.md                                |  2 +-
 docs/webactions.md                                 | 16 +++++-
 .../core/controller/test/WebActionsApiTests.scala  | 62 +++++++++++++++++++---
 5 files changed, 111 insertions(+), 12 deletions(-)

diff --git a/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala 
b/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala
index 07b9efe..97f98ae 100644
--- a/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala
+++ b/common/scala/src/main/scala/whisk/core/entity/WhiskAction.scala
@@ -313,6 +313,9 @@ object WhiskAction extends DocumentFactory[WhiskAction] 
with WhiskEntityQueries[
 
   override val cacheEnabled = true
 
+  val requireWhiskAuthAnnotation = "require-whisk-auth"
+  val requireWhiskAuthHeader = "x-require-whisk-auth"
+
   // overriden to store attached code
   override def put[A >: WhiskAction](db: ArtifactStore[A], doc: WhiskAction)(
     implicit transid: TransactionId,
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 9987a21..b0cd6ba 100644
--- a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
@@ -483,7 +483,14 @@ trait WhiskWebActionsApi extends Directives with 
ValidateRequestSize with PostAc
               provide(fullyQualifiedActionName(actionName)) { fullActionName =>
                 onComplete(verifyWebAction(fullActionName, 
onBehalfOf.isDefined)) {
                   case Success((actionOwnerIdentity, action)) =>
-                    if 
(!action.annotations.getAs[Boolean]("web-custom-options").exists(identity)) {
+                    var requiredAuthOk =
+                      requiredWhiskAuthSuccessful(action.annotations, 
context.headers).getOrElse(true)
+                    if (!requiredAuthOk) {
+                      logging.debug(
+                        this,
+                        "web action with require-whisk-auth was invoked 
without a matching x-require-whisk-auth header value")
+                      terminate(Unauthorized)
+                    } else if 
(!action.annotations.getAs[Boolean]("web-custom-options").exists(identity)) {
                       respondWithHeaders(defaultCorsResponse(context.headers)) 
{
                         if (context.method == OPTIONS) {
                           complete(OK, HttpEntity.Empty)
@@ -719,7 +726,8 @@ trait WhiskWebActionsApi extends Directives with 
ValidateRequestSize with PostAc
   private def confirmExportedAction(actionLookup: Future[WhiskActionMetaData], 
authenticated: Boolean)(
     implicit transid: TransactionId): Future[WhiskActionMetaData] = {
     actionLookup flatMap { action =>
-      val requiresAuthenticatedUser = 
action.annotations.getAs[Boolean]("require-whisk-auth").exists(identity)
+      val requiresAuthenticatedUser =
+        
action.annotations.getAs[Boolean](WhiskAction.requireWhiskAuthAnnotation).exists(identity)
       val isExported = 
action.annotations.getAs[Boolean]("web-export").exists(identity)
 
       if ((isExported && requiresAuthenticatedUser && authenticated) ||
@@ -751,4 +759,32 @@ trait WhiskWebActionsApi extends Directives with 
ValidateRequestSize with PostAc
 
     projection.getOrElse(List.empty)
   }
+
+  /**
+   * Check if "require-whisk-auth" authentication is needed, and if so, 
authenticate the request
+   * NOTE: Only number or string JSON "require-whisk-auth" annotation values 
are supported
+   *
+   * @param annotations - web action annotations
+   * @param reqHeaders - web action invocation request headers
+   * @return Option[Boolean]
+   *         None if annotations does not include require-whisk-auth (i.e. 
auth test not needed)
+   *         Some(true) if annotations includes require-whisk-auth and it's 
value matches the request header `X-Require-Whisk-Auth` value
+   *         Some(false) if annotations includes require-whisk-auth and the 
request does not include the header `X-Require-Whisk-Auth`
+   *         Some(false) if annotations includes require-whisk-auth and it's 
value deos not match the request header `X-Require-Whisk-Auth` value
+   */
+  private def requiredWhiskAuthSuccessful(annotations: Parameters, reqHeaders: 
Seq[HttpHeader]): Option[Boolean] = {
+    annotations
+      .get(WhiskAction.requireWhiskAuthAnnotation)
+      .flatMap {
+        case JsString(authStr) => Some(authStr)
+        case JsNumber(authNum) => Some(authNum.toString)
+        case _                 => None
+      }
+      .map { reqWhiskAuthAnnotationStr =>
+        reqHeaders
+          .find(_.is(WhiskAction.requireWhiskAuthHeader))
+          .map(_.value == reqWhiskAuthAnnotationStr)
+          .getOrElse(false) // false => when no x-require-whisk-auth header is 
present
+      }
+  }
 }
diff --git a/docs/annotations.md b/docs/annotations.md
index d92bf33..ab31bca 100644
--- a/docs/annotations.md
+++ b/docs/annotations.md
@@ -48,7 +48,7 @@ and must be present and explicitly set to `true` to have an 
affect. The annotati
 * `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.
 * `raw-http`: When set, the HTTP request query and body parameters are passed 
to the action as reserved properties.
 * `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.
-* `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).
+* `require-whisk-auth`: This annotation protects the web action so that it is 
only invoked by requests that provide appropriate authentication credentials. 
When set to a boolean value, it controls whether or not the request's Basic 
Authentication value (i.e. Whisk auth key) will be authenticated - a value of 
`true` will authenticate the credentials, a value of `false` will invoke the 
action without any authentication. When set to a number or a string, this value 
must match the request's  [...]
 
 # Annotations specific to activations
 
diff --git a/docs/webactions.md b/docs/webactions.md
index 2aaa608..6bd1f31 100644
--- a/docs/webactions.md
+++ b/docs/webactions.md
@@ -34,7 +34,7 @@ https://${APIHOST}/api/v1/web/guest/demo/hello
 ```
 
 Using the `--web` flag with a value of `true` or `yes` allows an action to be 
accessible via REST interface without the
-need for credentials. A web action can be invoked using a URL that is 
structured as follows:
+need for credentials. To configure a web action with credentials refer to 
[Securing web actions](webactions.md#securing-web-actions). A web action can be 
invoked using a URL that is structured as follows:
 `https://{APIHOST}/api/v1/web/{QUALIFIED ACTION NAME}.{EXT}`. The fully 
qualified name of an action consists of three
 parts: the namespace, the package name, and the action name.
 
@@ -280,6 +280,20 @@ $ wsk action create /guest/demo/hello hello.js \
 
 The result of these changes is that the `name` is bound to `Jane` and may not 
be overridden by query or body parameters because of the final annotation. This 
secures the action against query or body parameters that try to change this 
value whether by accident or intentionally. 
 
+## Securing web actions
+
+By default, a web action can be invoked by anyone having the web action's 
invocation URL. Use the `require-whisk-auth` [web action 
annotation](annotations.md#annotations-specific-to-web-actions) to secure the 
web action. When the `require-whisk-auth` annotation is set to `true`, the 
action will authenticate the invocation request's Basic Authorization 
credentials against the action owner's whisk auth key.  When set to a number or 
a case-sensitive string, the action's invocation request m [...]
+
+```bash
+$ wsk action update /guest/demo/hello hello.js --web true -a 
require-whisk-auth my-secret
+```
+
+```bash
+$ curl https://${APIHOST}/api/v1/web/guest/demo/hello.json?name=Jane -X GET -H 
"X-Require-Whisk-Auth: my-secret"
+```
+
+It's important to note that the owner of the web action owns all of the web 
action's activations records and will incur the cost of running the action in 
the system regardless of how the action was invoked.
+
 ## Disabling web actions
 
 To disable a web action from being invoked via web API 
(`https://APIHOST/api/v1/web/`), pass a value of `false` or `no` to the `--web` 
flag while updating an action with the CLI.
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 044ab53..36555b9 100644
--- a/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
@@ -133,6 +133,8 @@ trait WebActionsApiBaseTests extends ControllerTestCommon 
with BeforeAndAfterEac
   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 requireAuthenticationAsBoolean = true // toggle value set in 
require-whisk-auth annotation (true or  requireAuthenticationKey)
+  var requireAuthenticationKey = "example-web-action-api-key"
   var customOptions = true // toogle web-custom-options annotation on action
   var invocationCount = 0
   var invocationsAllowed = 0
@@ -148,6 +150,7 @@ trait WebActionsApiBaseTests extends ControllerTestCommon 
with BeforeAndAfterEac
     failThrottleForSubject = None
     actionResult = None
     requireAuthentication = false
+    requireAuthenticationAsBoolean = true
     customOptions = true
     assert(invocationsAllowed == invocationCount, "allowed invoke count did 
not match actual")
   }
@@ -197,7 +200,9 @@ trait WebActionsApiBaseTests extends ControllerTestCommon 
with BeforeAndAfterEac
               annotations ++
                 Parameters("web-export", JsBoolean(true)) ++ {
                 if (requireAuthentication) {
-                  Parameters("require-whisk-auth", JsBoolean(true))
+                  Parameters(
+                    "require-whisk-auth",
+                    (if (requireAuthenticationAsBoolean) JsBoolean(true) else 
JsString(requireAuthenticationKey)))
                 } else Parameters()
               } ++ {
                 if (customOptions) {
@@ -209,7 +214,9 @@ trait WebActionsApiBaseTests extends ControllerTestCommon 
with BeforeAndAfterEac
                 Parameters("web-export", JsBoolean(true)) ++
                 Parameters("raw-http", JsBoolean(true)) ++ {
                 if (requireAuthentication) {
-                  Parameters("require-whisk-auth", JsBoolean(true))
+                  Parameters(
+                    "require-whisk-auth",
+                    (if (requireAuthenticationAsBoolean) JsBoolean(true) else 
JsString(requireAuthenticationKey)))
                 } else Parameters()
               } ++ {
                 if (customOptions) {
@@ -379,14 +386,17 @@ trait WebActionsApiBaseTests extends ControllerTestCommon 
with BeforeAndAfterEac
 
       Seq(s"$systemId/proxy/export_auth").foreach { path =>
         allowedMethods.foreach { m =>
-          if (creds.isDefined)
-            invocationsAllowed += 1
           requireAuthentication = true
+          Seq(true, false).foreach { useReqWhiskAuthBool =>
+            requireAuthenticationAsBoolean = useReqWhiskAuthBool
+          }
 
-          m(s"$testRoutePath/${path}.json") ~> Route.seal(routes(creds)) ~> 
check {
-            creds match {
-              case None => status should be(Unauthorized)
-              case Some(user) =>
+          if (requireAuthenticationAsBoolean) {
+            if (creds.isDefined) {
+              val user = creds.get
+              invocationsAllowed += 1
+              m(s"$testRoutePath/${path}.json") ~> Route
+                .seal(routes(creds)) ~> check {
                 status should be(OK)
                 val response = responseAs[JsObject]
                 response shouldBe JsObject(
@@ -394,6 +404,42 @@ trait WebActionsApiBaseTests extends ControllerTestCommon 
with BeforeAndAfterEac
                   "action" -> "export_auth".toJson,
                   "content" -> metaPayload(m.method.name.toLowerCase, 
JsObject(), creds, pkgName = "proxy"))
                 
response.fields("content").asJsObject.fields(webApiDirectives.namespace) 
shouldBe user.namespace.toJson
+              }
+            } else {
+              m(s"$testRoutePath/${path}.json") ~> Route.seal(routes(creds)) 
~> check {
+                status should be(Unauthorized)
+              }
+            }
+          } else if (creds.isDefined) {
+            val user = creds.get
+            invocationsAllowed += 1
+
+            // web action require-whisk-auth is set and the header 
X-Require-Whisk-Auth value does not matches
+            m(s"$testRoutePath/${path}.json") ~> 
addHeader("X-Require-Whisk-Auth", requireAuthenticationKey) ~> Route
+              .seal(routes(creds)) ~> check {
+              status should be(OK)
+              val response = responseAs[JsObject]
+              response shouldBe JsObject(
+                "pkg" -> s"$systemId/proxy".toJson,
+                "action" -> "export_auth".toJson,
+                "content" -> metaPayload(
+                  m.method.name.toLowerCase,
+                  JsObject(),
+                  creds,
+                  pkgName = "proxy",
+                  headers = List(RawHeader("X-Require-Whisk-Auth", 
requireAuthenticationKey))))
+              
response.fields("content").asJsObject.fields(webApiDirectives.namespace) 
shouldBe user.namespace.toJson
+            }
+
+            // web action require-whisk-auth is set, but the header 
X-Require-Whisk-Auth value does not match
+            m(s"$testRoutePath/${path}.json") ~> 
addHeader("X-Require-Whisk-Auth", requireAuthenticationKey + "-bad") ~> Route
+              .seal(routes(creds)) ~> check {
+              status should be(Unauthorized)
+            }
+          } else {
+            // web action require-whisk-auth is set, but the header 
X-Require-Whisk-Auth value is not set
+            m(s"$testRoutePath/${path}.json") ~> Route.seal(routes(creds)) ~> 
check {
+              status should be(Unauthorized)
             }
           }
         }

-- 
To stop receiving notification emails like this one, please contact
csantan...@apache.org.

Reply via email to