You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2018/11/07 06:39:15 UTC

[GitHub] rabbah closed pull request #3880: Modify that web action in the bound package can be accessed.

rabbah closed pull request #3880: Modify that web action in the bound package can be accessed.
URL: https://github.com/apache/incubator-openwhisk/pull/3880
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

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 eb6c5c2ef8..45e1251549 100644
--- a/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
+++ b/core/controller/src/main/scala/whisk/core/controller/WebActions.scala
@@ -50,6 +50,7 @@ import RestApiCommons.{jsonPrettyResponsePrinter => jsonPrettyPrinter}
 import whisk.common.TransactionId
 import whisk.core.controller.actions.PostActionActivation
 import whisk.core.database._
+import whisk.core.entitlement.{Collection, Privilege, Resource}
 import whisk.core.entity._
 import whisk.core.entity.types._
 import whisk.core.loadBalancer.LoadBalancerException
@@ -373,6 +374,9 @@ trait WhiskWebActionsApi
   /** Configured authentication provider. */
   protected val authenticationProvider = SpiLoader.get[AuthenticationDirectiveProvider]
 
+  /** The collection type for this trait. */
+  protected val collection = Collection(Collection.ACTIONS)
+
   /** The prefix for web invokes e.g., /web. */
   private lazy val webRoutePrefix = {
     pathPrefix(webInvokePathSegments.map(_segmentStringToPathMatcher(_)).reduceLeft(_ / _))
@@ -448,23 +452,6 @@ trait WhiskWebActionsApi
     }
   }
 
-  /**
-   * Gets package from datastore.
-   * This method is factored out to allow mock testing.
-   */
-  protected def getPackage(pkgName: FullyQualifiedEntityName)(implicit transid: TransactionId): Future[WhiskPackage] = {
-    WhiskPackage.get(entityStore, pkgName.toDocId)
-  }
-
-  /**
-   * Gets action from datastore.
-   * This method is factored out to allow mock testing.
-   */
-  protected def getAction(actionName: FullyQualifiedEntityName)(
-    implicit transid: TransactionId): Future[WhiskActionMetaData] = {
-    WhiskActionMetaData.get(entityStore, actionName.toDocId)
-  }
-
   /**
    * Gets identity from datastore.
    * This method is factored out to allow mock testing.
@@ -545,27 +532,15 @@ trait WhiskWebActionsApi
    */
   private def verifyWebAction(actionName: FullyQualifiedEntityName, authenticated: Boolean)(
     implicit transid: TransactionId) = {
-    for {
-      // lookup the identity for the action namespace
-      actionOwnerIdentity <- identityLookup(actionName.path.root) flatMap { i =>
-        entitlementProvider.checkThrottles(i) map (_ => i)
-      }
 
-      // lookup the action - since actions are stored relative to package name
-      // the lookup will fail if the package name for the action refers to a binding instead
-      // also merge package and action parameters at the same time
-      // precedence order for parameters:
-      // package.params -> action.params -> query.params -> request.entity (body) -> augment arguments (namespace, path)
-      action <- confirmExportedAction(actionLookup(actionName), authenticated) flatMap { a =>
-        if (a.namespace.defaultPackage) {
-          Future.successful(a)
-        } else {
-          pkgLookup(a.namespace.toFullyQualifiedEntityName) map { pkg =>
-            (a.inherit(pkg.parameters))
-          }
+    // lookup the identity for the action namespace
+    identityLookup(actionName.path.root) flatMap { actionOwnerIdentity =>
+      confirmExportedAction(actionLookup(actionName), authenticated) flatMap { a =>
+        checkEntitlement(actionOwnerIdentity, a) map { _ =>
+          (actionOwnerIdentity, a)
         }
       }
-    } yield (actionOwnerIdentity, action)
+    }
   }
 
   private def extractEntityAndProcessRequest(actionOwnerIdentity: Identity,
@@ -696,25 +671,6 @@ trait WhiskWebActionsApi
     }
   }
 
-  /**
-   * Gets package from datastore and confirms it is not a binding.
-   */
-  private def pkgLookup(pkg: FullyQualifiedEntityName)(implicit transid: TransactionId): Future[WhiskPackage] = {
-    getPackage(pkg).filter {
-      _.binding.isEmpty
-    } recoverWith {
-      case _: ArtifactStoreException | DeserializationException(_, _, _) =>
-        // if the package lookup fails or the package doesn't conform to expected invariants,
-        // fail the request with BadRequest so as not to leak information about the existence
-        // of packages that are otherwise private
-        logging.debug(this, s"package which does not exist")
-        Future.failed(RejectRequest(NotFound))
-      case _: NoSuchElementException =>
-        logging.debug(this, s"'$pkg' is a binding")
-        Future.failed(RejectRequest(NotFound))
-    }
-  }
-
   /**
    * Gets the action if it exists and fail future with RejectRequest if it does not.
    *
@@ -722,7 +678,7 @@ trait WhiskWebActionsApi
    */
   private def actionLookup(actionName: FullyQualifiedEntityName)(
     implicit transid: TransactionId): Future[WhiskActionMetaData] = {
-    getAction(actionName) recoverWith {
+    WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, actionName) recoverWith {
       case _: ArtifactStoreException | DeserializationException(_, _, _) =>
         Future.failed(RejectRequest(NotFound))
     }
@@ -765,6 +721,17 @@ trait WhiskWebActionsApi
     }
   }
 
+  /**
+   * Checks if an action is executable.
+   */
+  private def checkEntitlement(identity: Identity, action: WhiskActionMetaData)(
+    implicit transid: TransactionId): Future[Unit] = {
+
+    val fqn = action.fullyQualifiedName(false)
+    val resource = Resource(fqn.path, collection, Some(fqn.name.asString))
+    entitlementProvider.check(identity, Privilege.ACTIVATE, resource)
+  }
+
   /**
    * Determines the result projection path, if any.
    *
diff --git a/docs/webactions.md b/docs/webactions.md
index d5c71c8a47..ba62efde4f 100644
--- a/docs/webactions.md
+++ b/docs/webactions.md
@@ -164,7 +164,7 @@ Web actions bring some additional features that include:
 1. `Content extensions`: the request must specify its desired content type as one of `.json`, `.html`, `.http`, `.svg` or `.text`. This is done by adding an extension to the action name in the URI, so that an action `/guest/demo/hello` is referenced as `/guest/demo/hello.http` for example to receive an HTTP response back. For convenience, the `.http` extension is assumed when no extension is detected.
 2. `Projecting fields from the result`: When used with content extensions other than `.http`, the path that follows the action name is used to project out one or more levels of the response. For example, 
 `/guest/demo/hello.html/body`. This allows an action which returns a dictionary `{body: "..." }` to project the `body` property and directly return its string value instead. The projected path follows an absolute path model (as in XPath).
-3. `Query and body parameters as input`: the action receives query parameters as well as parameters in the request body. The precedence order for merging parameters is: package parameters, action parameters, query parameter, body parameters with each of these overriding any previous values in case of overlap . As an example `/guest/demo/hello.http?name=Jane` will pass the argument `{name: "Jane"}` to the action.
+3. `Query and body parameters as input`: the action receives query parameters as well as parameters in the request body. The precedence order for merging parameters is: package parameters, binding parameters, action parameters, query parameter, body parameters with each of these overriding any previous values in case of overlap . As an example `/guest/demo/hello.http?name=Jane` will pass the argument `{name: "Jane"}` to the action.
 4. `Form data`: in addition to the standard `application/json`, web actions may receive URL encoded from data `application/x-www-form-urlencoded data` as input.
 5. `Activation via multiple HTTP verbs`: a web action may be invoked via any of these HTTP methods: `GET`, `POST`, `PUT`, `PATCH`, and `DELETE`, as well as `HEAD` and `OPTIONS`.
 6. `Non JSON body and raw HTTP entity handling`: A web action may accept an HTTP request body other than a JSON object, and may elect to always receive such values as opaque values (plain text when not binary, or base64 encoded string otherwise).
@@ -485,6 +485,17 @@ $ curl https://${APIHOST}/api/v1/web/guest/default/custom-options.http -kvX OPTI
 < Access-Control-Allow-Origin: example.com
 ```
 
+## Web Actions in Shared Packages
+
+A web action in a shared (i.e., public) package is accessible as a web action either directly via the package's fully
+qualified name, or via a package binding. It is important to note that a web action in a public package will be
+accessible for all bindings of the package even if the binding is private. This is because the web action annotation
+is carried on the action and cannot be overridden. If you do not wish to expose a web action through your package
+bindings, then you should clone-and-own the package instead.
+
+Action parameters are inherited from its package, and the binding if there is one. You can make package parameters
+[immutable](./annotations.md#protected-parameters) by defining their values through a package binding.
+
 ## Error Handling
 
 When an OpenWhisk action fails, there are two different failure modes. The first is known as an _application error_ and is analogous to a caught exception: the action returns a JSON object containing a top level `error` property. The second is a _developer error_ which occurs when the action fails catastrophically and does not produce a response (this is similar to an uncaught exception). For web actions, the controller handles application errors as follows:
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 e4bb43bef7..508b687b19 100644
--- a/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
+++ b/tests/src/test/scala/whisk/core/controller/test/WebActionsApiTests.scala
@@ -23,10 +23,8 @@ 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.{BeforeAndAfterEach, FlatSpec, Matchers}
 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
@@ -46,11 +44,7 @@ import spray.json._
 import spray.json.DefaultJsonProtocol._
 import whisk.common.TransactionId
 import whisk.core.WhiskConfig
-import whisk.core.controller.Context
-import whisk.core.controller.RejectRequest
-import whisk.core.controller.WhiskWebActionsApi
-import whisk.core.controller.WebApiDirectives
-import whisk.core.database.NoDocumentException
+import whisk.core.controller._
 import whisk.core.entitlement.EntitlementProvider
 import whisk.core.entitlement.Privilege
 import whisk.core.entitlement.Resource
@@ -60,6 +54,8 @@ import whisk.core.loadBalancer.LoadBalancer
 import whisk.http.ErrorResponse
 import whisk.http.Messages
 
+import scala.collection.immutable.Set
+
 /**
  * Tests web actions API.
  *
@@ -126,21 +122,37 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
   val systemKey = BasicAuthenticationAuthKey(uuid, Secret())
   val systemIdentity =
     Future.successful(Identity(systemId, Namespace(EntityName(systemId.asString), uuid), systemKey, Privilege.ALL))
+  val namespace = EntityPath(systemId.asString)
+  val proxyNamespace = namespace.addPath(EntityName("proxy"))
   override lazy val entitlementProvider = new TestingEntitlementProvider(whiskConfig, loadBalancer)
   protected val testRoutePath = webInvokePathSegments.mkString("/", "/", "")
+  def aname() = MakeName.next("web_action_tests")
 
   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 failCheckEntitlement = false // toggle to cause entitlement to fail
   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 testParametersInInvokeAction = true // toggle to test parameter in invokeAction
   var requireAuthenticationKey = "example-web-action-api-key"
-  var customOptions = true // toogle web-custom-options annotation on action
   var invocationCount = 0
   var invocationsAllowed = 0
+  lazy val testFixturesToGc = {
+    implicit val tid = transid()
+    Seq(
+      stubPackage,
+      stubAction(namespace, EntityName("export_c")),
+      stubAction(proxyNamespace, EntityName("export_c")),
+      stubAction(proxyNamespace, EntityName("raw_export_c"))).map { f =>
+      put(entityStore, f, garbageCollect = false)
+    }
+  }
+
+  override def beforeAll() = {
+    testFixturesToGc.foreach(f => ())
+  }
 
   override def beforeEach() = {
     invocationCount = 0
@@ -148,14 +160,19 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
   }
 
   override def afterEach() = {
-    failActionLookup = false
     failActivation = 0
     failThrottleForSubject = None
+    failCheckEntitlement = false
     actionResult = None
-    requireAuthentication = false
     requireAuthenticationAsBoolean = true
-    customOptions = true
+    testParametersInInvokeAction = true
     assert(invocationsAllowed == invocationCount, "allowed invoke count did not match actual")
+    cleanup()
+  }
+
+  override def afterAll() = {
+    implicit val tid = transid()
+    testFixturesToGc.foreach(delete(entityStore, _))
   }
 
   val allowedMethodsWithEntity = {
@@ -175,75 +192,60 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
     "D6fzsAAAAASUVORK5CYII="
 
   // there is only one package that is predefined 'proxy'
-  val packages = Seq(
-    WhiskPackage(
-      EntityPath(systemId.asString),
-      EntityName("proxy"),
-      parameters = Parameters("x", JsString("X")) ++ Parameters("z", JsString("z"))))
-
-  override protected def getPackage(pkgName: FullyQualifiedEntityName)(implicit transid: TransactionId) = {
-    Future {
-      packages.find(_.fullyQualifiedName(false) == pkgName).get
-    } recoverWith {
-      case _: NoSuchElementException => Future.failed(NoDocumentException("does not exist"))
-    }
-  }
+  val stubPackage = WhiskPackage(
+    EntityPath(systemId.asString),
+    EntityName("proxy"),
+    parameters = Parameters("x", JsString("X")) ++ Parameters("z", JsString("z")))
+
+  val packages = Seq(stubPackage)
 
   val defaultActionParameters = {
     Parameters("y", JsString("Y")) ++ Parameters("z", JsString("Z")) ++ Parameters("empty", JsNull)
   }
 
   // action names that start with 'export_' will automatically have an web-export annotation added by the test harness
-  override protected def getAction(actionName: FullyQualifiedEntityName)(implicit transid: TransactionId) = {
-    if (!failActionLookup) {
-      def theAction = {
-        val annotations = Parameters(WhiskActionMetaData.finalParamsAnnotationName, JsBoolean(true))
-
-        WhiskActionMetaData(
-          actionName.path,
-          actionName.name,
-          js6MetaData(binary = false),
-          defaultActionParameters,
-          annotations = {
-            if (actionName.name.asString.startsWith("export_")) {
-              annotations ++
-                Parameters("web-export", JsBoolean(true)) ++ {
-                if (requireAuthentication) {
-                  Parameters(
-                    "require-whisk-auth",
-                    (if (requireAuthenticationAsBoolean) JsBoolean(true) else JsString(requireAuthenticationKey)))
-                } else Parameters()
-              } ++ {
-                if (customOptions) {
-                  Parameters("web-custom-options", JsBoolean(true))
-                } else Parameters()
-              }
-            } else if (actionName.name.asString.startsWith("raw_export_")) {
-              annotations ++
-                Parameters("web-export", JsBoolean(true)) ++
-                Parameters("raw-http", JsBoolean(true)) ++ {
-                if (requireAuthentication) {
-                  Parameters(
-                    "require-whisk-auth",
-                    (if (requireAuthenticationAsBoolean) JsBoolean(true) else JsString(requireAuthenticationKey)))
-                } else Parameters()
-              } ++ {
-                if (customOptions) {
-                  Parameters("web-custom-options", JsBoolean(true))
-                } else Parameters()
-              }
-            } else annotations
-          })
-      }
-
-      if (actionName.path.defaultPackage) {
-        Future.successful(theAction)
-      } else {
-        getPackage(actionName.path.toFullyQualifiedEntityName) map (_ => theAction)
-      }
-    } else {
-      Future.failed(NoDocumentException("doesn't exist"))
-    }
+  protected def stubAction(namespace: EntityPath,
+                           name: EntityName,
+                           customOptions: Boolean = true,
+                           requireAuthentication: Boolean = false,
+                           requireAuthenticationAsBoolean: Boolean = true) = {
+
+    val annotations = Parameters(WhiskActionMetaData.finalParamsAnnotationName, JsBoolean(true))
+    WhiskAction(
+      namespace,
+      name,
+      jsDefault("??"),
+      defaultActionParameters,
+      annotations = {
+        if (name.asString.startsWith("export_")) {
+          annotations ++
+            Parameters("web-export", JsBoolean(true)) ++ {
+            if (requireAuthentication) {
+              Parameters(
+                "require-whisk-auth",
+                (if (requireAuthenticationAsBoolean) JsBoolean(true) else JsString(requireAuthenticationKey)))
+            } else Parameters()
+          } ++ {
+            if (customOptions) {
+              Parameters("web-custom-options", JsBoolean(true))
+            } else Parameters()
+          }
+        } else if (name.asString.startsWith("raw_export_")) {
+          annotations ++
+            Parameters("web-export", JsBoolean(true)) ++
+            Parameters("raw-http", JsBoolean(true)) ++ {
+            if (requireAuthentication) {
+              Parameters(
+                "require-whisk-auth",
+                (if (requireAuthenticationAsBoolean) JsBoolean(true) else JsString(requireAuthenticationKey)))
+            } else Parameters()
+          } ++ {
+            if (customOptions) {
+              Parameters("web-custom-options", JsBoolean(true))
+            } else Parameters()
+          }
+        } else annotations
+      })
   }
 
   // there is only one identity defined for the fully qualified name of the web action: 'systemId'
@@ -294,15 +296,15 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
         })
 
       // check that action parameters were merged with package
-      // all actions have default parameters (see actionLookup stub)
-      if (!action.namespace.defaultPackage) {
-        getPackage(action.namespace.toFullyQualifiedEntityName) foreach { pkg =>
-          action.parameters shouldBe (pkg.parameters ++ defaultActionParameters)
+      // all actions have default parameters (see stubAction)
+      if (testParametersInInvokeAction) {
+        if (!action.namespace.defaultPackage) {
+          action.parameters shouldBe (stubPackage.parameters ++ defaultActionParameters)
+        } else {
+          action.parameters shouldBe defaultActionParameters
         }
-      } else {
-        action.parameters shouldBe defaultActionParameters
+        action.parameters.get("z") shouldBe defaultActionParameters.get("z")
       }
-      action.parameters.get("z") shouldBe defaultActionParameters.get("z")
 
       Future.successful(Right(activation))
     } else if (failActivation == 1) {
@@ -363,6 +365,8 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
     it should s"reject requests when Identity, package or action lookup fail or missing annotation (auth? ${creds.isDefined})" in {
       implicit val tid = transid()
 
+      put(entityStore, stubAction(namespace, EntityName("c")))
+
       // the first of these fails in the identity lookup,
       // the second in the package lookup (does not exist),
       // the third fails the annotation check (no web-export annotation because name doesn't start with export_c)
@@ -370,8 +374,6 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
       Seq("guest/proxy/c", s"$systemId/doesnotexist/c", s"$systemId/default/c", s"$systemId/proxy/export_fail")
         .foreach { path =>
           allowedMethods.foreach { m =>
-            failActionLookup = path.endsWith("fail")
-
             m(s"$testRoutePath/${path}.json") ~> Route.seal(routes(creds)) ~> check {
               status should be(NotFound)
             }
@@ -393,70 +395,77 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
     it should s"reject requests when authentication is required but none given (auth? ${creds.isDefined})" in {
       implicit val tid = transid()
 
-      Seq(s"$systemId/proxy/export_auth").foreach { path =>
-        allowedMethods.foreach { m =>
-          requireAuthentication = true
-          Seq(true, false).foreach { useReqWhiskAuthBool =>
-            requireAuthenticationAsBoolean = useReqWhiskAuthBool
-          }
+      allowedMethods.foreach { m =>
+        Seq(true, false).foreach { useReqWhiskAuthBool =>
+          requireAuthenticationAsBoolean = useReqWhiskAuthBool
+        }
 
-          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(
-                  "pkg" -> s"$systemId/proxy".toJson,
-                  "action" -> "export_auth".toJson,
-                  "content" -> metaPayload(m.method.name.toLowerCase, JsObject.empty, creds, pkgName = "proxy"))
-                response
-                  .fields("content")
-                  .asJsObject
-                  .fields(webApiDirectives.namespace) shouldBe user.namespace.name.toJson
-              }
-            } else {
-              m(s"$testRoutePath/${path}.json") ~> Route.seal(routes(creds)) ~> check {
-                status should be(Unauthorized)
-              }
-            }
-          } else if (creds.isDefined) {
+        val entityName = MakeName.next("export")
+        val action = stubAction(
+          proxyNamespace,
+          entityName,
+          requireAuthentication = true,
+          requireAuthenticationAsBoolean = requireAuthenticationAsBoolean)
+        val path = action.fullyQualifiedName(false)
+
+        put(entityStore, action)
+
+        if (requireAuthenticationAsBoolean) {
+          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
+            m(s"$testRoutePath/${path}.json") ~> 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.empty,
-                  creds,
-                  pkgName = "proxy",
-                  headers = List(RawHeader("X-Require-Whisk-Auth", requireAuthenticationKey))))
+                "action" -> entityName.asString.toJson,
+                "content" -> metaPayload(m.method.name.toLowerCase, JsObject.empty, creds, pkgName = "proxy"))
               response
                 .fields("content")
                 .asJsObject
                 .fields(webApiDirectives.namespace) shouldBe user.namespace.name.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)
             }
           }
+        } 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" -> entityName.asString.toJson,
+              "content" -> metaPayload(
+                m.method.name.toLowerCase,
+                JsObject.empty,
+                creds,
+                pkgName = "proxy",
+                headers = List(RawHeader("X-Require-Whisk-Auth", requireAuthenticationKey))))
+            response
+              .fields("content")
+              .asJsObject
+              .fields(webApiDirectives.namespace) shouldBe user.namespace.name.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)
+          }
         }
       }
     }
@@ -612,6 +621,208 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
       }
     }
 
+    it should s"invoke action in a binding of private package (auth? ${creds.isDefined})" in {
+      implicit val tid = transid()
+
+      val provider = WhiskPackage(EntityPath(systemId.asString), aname(), None, stubPackage.parameters)
+      val reference = WhiskPackage(EntityPath(systemId.asString), aname(), provider.bind)
+      val action = stubAction(provider.fullPath, EntityName("export_c"))
+
+      put(entityStore, provider)
+      put(entityStore, reference)
+      put(entityStore, action)
+
+      Seq(s"$systemId/${reference.name}/export_c.json").foreach { path =>
+        allowedMethods.foreach { m =>
+          invocationsAllowed += 1
+          m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
+            status should be(OK)
+          }
+        }
+      }
+    }
+
+    it should s"invoke action in a binding of public package (auth? ${creds.isDefined})" in {
+      implicit val tid = transid()
+
+      val provider = WhiskPackage(EntityPath("guest"), aname(), None, stubPackage.parameters, publish = true)
+      val reference = WhiskPackage(EntityPath(systemId.asString), aname(), provider.bind)
+      val action = stubAction(provider.fullPath, EntityName("export_c"))
+
+      put(entityStore, provider)
+      put(entityStore, reference)
+      put(entityStore, action)
+
+      Seq(s"$systemId/${reference.name}/export_c.json").foreach { path =>
+        allowedMethods.foreach { m =>
+          invocationsAllowed += 1
+          m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
+            status should be(OK)
+          }
+        }
+      }
+    }
+
+    it should s"invoke action relative to a binding where the action doesn't exist (auth? ${creds.isDefined})" in {
+      implicit val tid = transid()
+
+      val provider = WhiskPackage(EntityPath("guest"), aname(), None, stubPackage.parameters, publish = true)
+      val reference = WhiskPackage(EntityPath(systemId.asString), aname(), provider.bind)
+
+      put(entityStore, provider)
+      put(entityStore, reference)
+      // action is not created
+
+      Seq(s"$systemId/${reference.name}/export_c.json").foreach { path =>
+        allowedMethods.foreach { m =>
+          m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
+            status should be(NotFound)
+          }
+        }
+      }
+    }
+
+    it should s"invoke action in non-existing binding (auth? ${creds.isDefined})" in {
+      implicit val tid = transid()
+
+      val provider = WhiskPackage(EntityPath("guest"), aname(), None, stubPackage.parameters, publish = true)
+      val action = stubAction(provider.fullPath, EntityName("export_c"))
+      val reference = WhiskPackage(EntityPath(systemId.asString), aname(), provider.bind)
+
+      put(entityStore, provider)
+      put(entityStore, action)
+      // reference is not created
+
+      Seq(s"$systemId/${reference.name}/export_c.json").foreach { path =>
+        allowedMethods.foreach { m =>
+          m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
+            status should be(NotFound)
+          }
+        }
+      }
+    }
+
+    it should s"not inherit annotations of package binding (auth? ${creds.isDefined})" in {
+      implicit val tid = transid()
+
+      val provider = WhiskPackage(EntityPath("guest"), aname(), None, stubPackage.parameters, publish = true)
+      val reference = WhiskPackage(
+        EntityPath(systemId.asString),
+        aname(),
+        provider.bind,
+        annotations = Parameters("web-export", JsBoolean(false)))
+      val action = stubAction(provider.fullPath, EntityName("export_c"))
+
+      put(entityStore, provider)
+      put(entityStore, reference)
+      put(entityStore, action)
+
+      Seq(s"$systemId/${reference.name}/export_c.json").foreach { path =>
+        allowedMethods.foreach { m =>
+          invocationsAllowed += 1
+          m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
+            status should be(OK)
+          }
+        }
+      }
+    }
+
+    it should s"reject request that tries to override final parameters of action in package binding (auth? ${creds.isDefined})" in {
+      implicit val tid = transid()
+
+      val provider = WhiskPackage(EntityPath("guest"), aname(), None, publish = true)
+      val reference = WhiskPackage(EntityPath(systemId.asString), aname(), provider.bind, stubPackage.parameters)
+      val action = stubAction(provider.fullPath, EntityName("export_c"))
+
+      put(entityStore, provider)
+      put(entityStore, reference)
+      put(entityStore, action)
+
+      val contentX = JsObject("x" -> "overriden".toJson)
+      val contentZ = JsObject("z" -> "overriden".toJson)
+
+      allowedMethodsWithEntity.foreach { m =>
+        invocationsAllowed += 1
+
+        m(s"$testRoutePath/$systemId/${reference.name}/export_c.json?x=overriden") ~> Route.seal(routes(creds)) ~> check {
+          status should be(BadRequest)
+          responseAs[ErrorResponse].error shouldBe Messages.parametersNotAllowed
+        }
+
+        m(s"$testRoutePath/$systemId/${reference.name}/export_c.json?y=overriden") ~> Route.seal(routes(creds)) ~> check {
+          status should be(BadRequest)
+          responseAs[ErrorResponse].error shouldBe Messages.parametersNotAllowed
+        }
+
+        m(s"$testRoutePath/$systemId/${reference.name}/export_c.json", contentX) ~> Route.seal(routes(creds)) ~> check {
+          status should be(BadRequest)
+          responseAs[ErrorResponse].error shouldBe Messages.parametersNotAllowed
+        }
+
+        m(s"$testRoutePath/$systemId/${reference.name}/export_c.json?y=overriden", contentZ) ~> Route.seal(
+          routes(creds)) ~> check {
+          status should be(BadRequest)
+          responseAs[ErrorResponse].error shouldBe Messages.parametersNotAllowed
+        }
+
+        m(s"$testRoutePath/$systemId/${reference.name}/export_c.json?empty=overriden") ~> Route.seal(routes(creds)) ~> check {
+          status should be(OK)
+          val response = responseAs[JsObject]
+          response shouldBe JsObject(
+            "pkg" -> s"guest/${provider.name}".toJson,
+            "action" -> "export_c".toJson,
+            "content" -> metaPayload(
+              m.method.name.toLowerCase,
+              Map("empty" -> "overriden").toJson.asJsObject,
+              creds,
+              pkgName = "proxy"))
+        }
+      }
+    }
+
+    it should s"match precedence order for merging parameters (auth? ${creds.isDefined})" in {
+      implicit val tid = transid()
+
+      testParametersInInvokeAction = false
+
+      val provider = WhiskPackage(
+        EntityPath("guest"),
+        aname(),
+        None,
+        Parameters("a", JsString("A")) ++ Parameters("b", JsString("b")),
+        publish = true)
+      val reference = WhiskPackage(
+        EntityPath(systemId.asString),
+        aname(),
+        provider.bind,
+        Parameters("a", JsString("a")) ++ Parameters("c", JsString("c")))
+
+      // stub action has defaultActionParameters
+      val action = stubAction(provider.fullPath, EntityName("export_c"))
+
+      put(entityStore, provider)
+      put(entityStore, reference)
+      put(entityStore, action)
+
+      Seq(s"$systemId/${reference.name}/export_c.json").foreach { path =>
+        allowedMethods.foreach { m =>
+          invocationsAllowed += 1
+          m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
+            status should be(OK)
+            val response = responseAs[JsObject]
+
+            response shouldBe JsObject(
+              "pkg" -> s"guest/${provider.name}".toJson,
+              "action" -> "export_c".toJson,
+              "content" -> metaPayload(
+                m.method.name.toLowerCase,
+                Map("a" -> "a", "b" -> "b", "c" -> "c").toJson.asJsObject,
+                creds))
+          }
+        }
+      }
+    }
+
     it should s"project a field from the result object (auth? ${creds.isDefined})" in {
       implicit val tid = transid()
 
@@ -1312,17 +1523,15 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
         s"$systemId/proxy/export_c.xyzz/",
         s"$systemId/proxy/export_c.xyzz/content").foreach { path =>
         allowedMethods.foreach { m =>
-          actionResult = Some(JsObject(webApiDirectives.statusCode -> Created.intValue.toJson))
-
           m(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
+
             if (webApiDirectives.enforceExtension) {
               status should be(NotAcceptable)
               confirmErrorWithTid(
                 responseAs[JsObject],
                 Some(Messages.contentTypeExtensionNotSupported(WhiskWebActionsApi.allowedExtensions)))
             } else {
-              invocationsAllowed += 1
-              status should be(Created)
+              status should be(NotFound)
             }
           }
         }
@@ -1489,25 +1698,27 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
 
     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", s"$systemId/proxy/export_c.json").foreach { path =>
-        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, Origin, X-Requested-With, Content-Type, Accept, User-Agent"
+      put(entityStore, stubAction(proxyNamespace, EntityName("export_without_custom_options"), false))
+
+      Seq(s"$systemId/proxy/export_without_custom_options.http", s"$systemId/proxy/export_without_custom_options.json")
+        .foreach { path =>
+          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, Origin, X-Requested-With, Content-Type, Accept, User-Agent"
+                  }
                 }
               }
-            }
+          }
         }
-      }
     }
 
     it should s"invoke action with head verb (auth? ${creds.isDefined})" in {
@@ -1646,6 +1857,19 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
       }
     }
 
+    it should s"reject invocation of web action which has no entitlement (auth? ${creds.isDefined})" in {
+      implicit val tid = transid()
+
+      Seq(s"$systemId/proxy/export_c.http").foreach { path =>
+        actionResult = Some(JsObject("body" -> "Plain text".toJson))
+        failCheckEntitlement = true
+
+        Get(s"$testRoutePath/$path") ~> Route.seal(routes(creds)) ~> check {
+          status should be(Forbidden)
+        }
+      }
+    }
+
     it should s"not invoke an action more than once when determining entity type (auth? ${creds.isDefined})" in {
       implicit val tid = transid()
 
@@ -1745,14 +1969,22 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
   class TestingEntitlementProvider(config: WhiskConfig, loadBalancer: LoadBalancer)
       extends EntitlementProvider(config, loadBalancer, ControllerInstanceId("0")) {
 
-    protected[core] override def checkThrottles(user: Identity)(implicit transid: TransactionId): Future[Unit] = {
+    // The check method checks both throttle and entitlement.
+    protected[core] override def check(user: Identity, right: Privilege, resource: Resource)(
+      implicit transid: TransactionId): Future[Unit] = {
       val subject = user.subject
-      logging.debug(this, s"test throttle is checking user '$subject' has not exceeded activation quota")
 
-      failThrottleForSubject match {
-        case Some(subject) if subject == user.subject =>
-          Future.failed(RejectRequest(TooManyRequests, Messages.tooManyRequests(2, 1)))
-        case _ => Future.successful({})
+      // first, check entitlement
+      if (failCheckEntitlement) {
+        Future.failed(RejectRequest(Forbidden))
+      } else {
+        // then, check throttle
+        logging.debug(this, s"test throttle is checking user '$subject' has not exceeded activation quota")
+        failThrottleForSubject match {
+          case Some(subject) if subject == user.subject =>
+            Future.failed(RejectRequest(TooManyRequests, Messages.tooManyRequests(2, 1)))
+          case _ => Future.successful({})
+        }
       }
     }
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services