manually merge vesh's pr apache#4935 to manually merge to runtime-prs-v84

Author: tysonnorris

common/scala/src/main/resources/application.conf

@@ -99,6 +99,7 @@ kamon {
 }
 
 whisk {
+    shared-packages-execute-only = false
     metrics {
         # Enable/disable Prometheus support. If enabled then metrics would be exposed at `/metrics` endpoint
         # If Prometheus is enabled then please review `kamon.metric.tick-interval` (set to 1 sec by default above).

common/scala/src/main/scala/org/apache/openwhisk/core/WhiskConfig.scala

@@ -265,6 +265,7 @@ object ConfigKeys {
   val featureFlags = "whisk.feature-flags"
 
   val whiskConfig = "whisk.config"
+  val sharedPackageExecuteOnly = s"whisk.shared-packages-execute-only"
   val swaggerUi = "whisk.swagger-ui"
 
   val disableStoreResult = s"$activation.disable-store-result"

common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala

@@ -229,6 +229,15 @@ object Messages {
 
   /** Indicates that the container for the action could not be started. */
   val resourceProvisionError = "Failed to provision resources to run the action."
+
+  def forbiddenGetActionBinding(entityDocId: String) =
+    s"GET not permitted for '$entityDocId'. Resource does not exist or is an action in a shared package binding."
+  def forbiddenGetAction(entityPath: String) =
+    s"GET not permitted for '$entityPath' since it's an action in a shared package"
+  def forbiddenGetPackageBinding(packageName: String) =
+    s"GET not permitted since $packageName is a binding of a shared package"
+  def forbiddenGetPackage(packageName: String) =
+    s"GET not permitted for '$packageName' since it's a shared package"
 }
 
 /** Replaces rejections with Json object containing cause and transaction id. */

core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala

@@ -43,6 +43,8 @@ import org.apache.openwhisk.http.Messages._
 import org.apache.openwhisk.core.entitlement.Resource
 import org.apache.openwhisk.core.entitlement.Collection
 import org.apache.openwhisk.core.loadBalancer.LoadBalancerException
+import pureconfig._
+import org.apache.openwhisk.core.ConfigKeys
 
 /**
  * A singleton object which defines the properties that must be present in a configuration
@@ -102,6 +104,10 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
   /** Database service to get activations. */
   protected val activationStore: ActivationStore
 
+  /** Config flag for Execute Only for Actions in Shared Packages */
+  protected def executeOnly =
+    loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
+
   /** Entity normalizer to JSON object. */
   import RestApiCommons.emptyEntityToJsObject
 
@@ -330,6 +336,56 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
     deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({}))
   }
 
+  /**GET --save which retrieves all source code for an entity **/
+  private def getEntityWithSourceCode(entityName: FullyQualifiedEntityName, env: Option[Parameters])(
+    implicit transid: TransactionId) = {
+    getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some { action: WhiskAction =>
+      val mergedAction = env map {
+        action inherit _
+      } getOrElse action
+      complete(OK, mergedAction)
+    })
+  }
+
+  /**Standard GET which just retrieves metadata**/
+  private def getEntityMetaData(entityName: FullyQualifiedEntityName, env: Option[Parameters])(
+    implicit transid: TransactionId) = {
+    getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
+      action: WhiskActionMetaData =>
+        val mergedAction = env map {
+          action inherit _
+        } getOrElse action
+        complete(OK, mergedAction)
+    })
+  }
+
+  /** Checks for package binding case. we don't want to allow get for a package binding in shared package */
+  private def fetchEntity(entityName: FullyQualifiedEntityName, env: Option[Parameters], code: Boolean)(
+    implicit transid: TransactionId) = {
+    if (entityName.path.defaultPackage) {
+      if (code) {
+        getEntityWithSourceCode(entityName, env)
+      } else {
+        getEntityMetaData(entityName, env)
+      }
+    } else {
+      getEntity(
+        WhiskPackage.resolveBinding(entityStore, entityName.path.toDocId, mergeParameters = true),
+        Some { pkg: WhiskPackage =>
+          val originalPackageLocation = pkg.fullyQualifiedName(withVersion = false).namespace
+          if (executeOnly && originalPackageLocation != entityName.namespace) {
+            terminate(Forbidden, forbiddenGetActionBinding(entityName.toDocId.asString))
+          } else {
+            if (code) {
+              getEntityWithSourceCode(entityName, env)
+            } else {
+              getEntityMetaData(entityName, env)
+            }
+          }
+        })
+    }
+  }
+
   /**
    * Gets action. The action name is prefixed with the namespace to create the primary index key.
    *
@@ -341,22 +397,12 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
   override def fetch(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters])(
     implicit transid: TransactionId) = {
     parameter('code ? true) { code =>
-      code match {
-        case true =>
-          getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some { action: WhiskAction =>
-            val mergedAction = env map {
-              action inherit _
-            } getOrElse action
-            complete(OK, mergedAction)
-          })
-        case false =>
-          getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
-            action: WhiskActionMetaData =>
-              val mergedAction = env map {
-                action inherit _
-              } getOrElse action
-              complete(OK, mergedAction)
-          })
+      //check if execute only is enabled, and if there is a discrepancy between the current user's namespace
+      //and that of the entity we are trying to fetch
+      if (executeOnly && user.namespace.name != entityName.namespace) {
+        terminate(Forbidden, forbiddenGetAction(entityName.path.asString))
+      } else {
+        fetchEntity(entityName, env, code)
       }
     }
   }

core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala

@@ -33,6 +33,9 @@ import org.apache.openwhisk.core.entity._
 import org.apache.openwhisk.core.entity.types.EntityStore
 import org.apache.openwhisk.http.ErrorResponse.terminate
 import org.apache.openwhisk.http.Messages
+import org.apache.openwhisk.http.Messages._
+import pureconfig._
+import org.apache.openwhisk.core.ConfigKeys
 
 trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
   services: WhiskServices =>
@@ -42,6 +45,10 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
   /** Database service to CRUD packages. */
   protected val entityStore: EntityStore
 
+  /** Config flag for Execute Only for Shared Packages */
+  protected def executeOnly =
+    loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
+
   /** Notification service for cache invalidation. */
   protected implicit val cacheChangeNotification: Some[CacheChangeNotification]
 
@@ -155,9 +162,18 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
    * - 404 Not Found
    * - 500 Internal Server Error
    */
+  //get method that also checks for execute only to deny access to shared package, but package binding is handled
+  //within the mergePackageWithBinding() method
   override def fetch(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters])(
     implicit transid: TransactionId) = {
-    getEntity(WhiskPackage.get(entityStore, entityName.toDocId), Some { mergePackageWithBinding() _ })
+    if (executeOnly && user.namespace.name != entityName.namespace) {
+      val value = entityName.toString
+      terminate(Forbidden, forbiddenGetPackage(entityName.asString))
+    } else {
+      getEntity(WhiskPackage.get(entityStore, entityName.toDocId), Some {
+        mergePackageWithBinding() _
+      })
+    }
   }
 
   /**
@@ -303,9 +319,17 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
           logging.error(this, s"unexpected package binding refers to itself: $docid")
           terminate(UnprocessableEntity, Messages.packageBindingCircularReference(b.fullyQualifiedName.toString))
         } else {
-          getEntity(WhiskPackage.get(entityStore, docid), Some {
-            mergePackageWithBinding(Some { wp }) _
-          })
+
+          /** Here's where I check package execute only case with package binding. */
+          if (executeOnly && wp.namespace.asString != b.namespace.asString) {
+            terminate(Forbidden, forbiddenGetPackageBinding(wp.name.asString))
+          } else {
+            getEntity(WhiskPackage.get(entityStore, docid), Some {
+              mergePackageWithBinding(Some {
+                wp
+              }) _
+            })
+          }
         }
     } getOrElse {
       val pkg = ref map { _ inherit wp.parameters } getOrElse wp

tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackageActionsApiTests.scala

@@ -636,4 +636,37 @@ class PackageActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
       responseAs[ErrorResponse].error shouldBe Messages.corruptedEntity
     }
   }
+
+  var testExecuteOnly = false
+  override def executeOnly = testExecuteOnly
+
+  it should ("allow access to get of action in binding of shared package when config option is disabled") in {
+    testExecuteOnly = false
+    implicit val tid = transid()
+    val auser = WhiskAuthHelpers.newIdentity()
+    val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
+    val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
+    val action = WhiskAction(provider.fullPath, aname(), jsDefault("??"), Parameters("a", "A"))
+    put(entityStore, provider)
+    put(entityStore, binding)
+    put(entityStore, action)
+    Get(s"$collectionPath/${binding.name}/${action.name}") ~> Route.seal(routes(auser)) ~> check {
+      status should be(OK)
+    }
+  }
+
+  it should ("deny access to get of action in binding of shared package when config option is enabled") in {
+    testExecuteOnly = true
+    implicit val tid = transid()
+    val auser = WhiskAuthHelpers.newIdentity()
+    val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
+    val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
+    val action = WhiskAction(provider.fullPath, aname(), jsDefault("??"), Parameters("a", "A"))
+    put(entityStore, provider)
+    put(entityStore, binding)
+    put(entityStore, action)
+    Get(s"$collectionPath/${binding.name}/${action.name}") ~> Route.seal(routes(auser)) ~> check {
+      status should be(Forbidden)
+    }
+  }
 }

tests/src/test/scala/org/apache/openwhisk/core/controller/test/PackagesApiTests.scala

@@ -884,4 +884,58 @@ class PackagesApiTests extends ControllerTestCommon with WhiskPackagesApi {
       responseAs[ErrorResponse].error shouldBe Messages.corruptedEntity
     }
   }
+
+  var testExecuteOnly = false
+  override def executeOnly = testExecuteOnly
+
+  it should ("allow access to get of shared package binding when config option is disabled") in {
+    testExecuteOnly = false
+    implicit val tid = transid()
+    val auser = WhiskAuthHelpers.newIdentity()
+    val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
+    val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
+    put(entityStore, provider)
+    put(entityStore, binding)
+    Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
+      status should be(OK)
+    }
+  }
+
+  it should ("allow access to get of shared package when config option is disabled") in {
+    testExecuteOnly = false
+    implicit val tid = transid()
+    val auser = WhiskAuthHelpers.newIdentity()
+    val provider = WhiskPackage(namespace, aname(), None, publish = true)
+    put(entityStore, provider)
+
+    Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
+      status should be(OK)
+    }
+  }
+
+  it should ("deny access to get of shared package binding when config option is enabled") in {
+    testExecuteOnly = true
+    implicit val tid = transid()
+    val auser = WhiskAuthHelpers.newIdentity()
+    val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
+    val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
+    put(entityStore, provider)
+    put(entityStore, binding)
+    Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
+      status should be(Forbidden)
+    }
+
+  }
+
+  it should ("deny access to get of shared package when config option is enabled") in {
+    testExecuteOnly = true
+    implicit val tid = transid()
+    val auser = WhiskAuthHelpers.newIdentity()
+    val provider = WhiskPackage(namespace, aname(), None, publish = true)
+    put(entityStore, provider)
+
+    Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
+      status should be(Forbidden)
+    }
+  }
 }