Skip to content

Commit f11f6b2

Browse files
tysonnorristysonnorris
committed
manually merge vesh's pr apache#4935 to manually merge to runtime-prs-v84
1 parent 1450c86 commit f11f6b2

7 files changed

Lines changed: 188 additions & 20 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ kamon {
9999
}
100100

101101
whisk {
102+
shared-packages-execute-only = false
102103
metrics {
103104
# Enable/disable Prometheus support. If enabled then metrics would be exposed at `/metrics` endpoint
104105
# 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‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ object ConfigKeys {
265265
val featureFlags = "whisk.feature-flags"
266266

267267
val whiskConfig = "whisk.config"
268+
val sharedPackageExecuteOnly = s"whisk.shared-packages-execute-only"
268269
val swaggerUi = "whisk.swagger-ui"
269270

270271
val disableStoreResult = s"$activation.disable-store-result"

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,15 @@ object Messages {
229229

230230
/** Indicates that the container for the action could not be started. */
231231
val resourceProvisionError = "Failed to provision resources to run the action."
232+
233+
def forbiddenGetActionBinding(entityDocId: String) =
234+
s"GET not permitted for '$entityDocId'. Resource does not exist or is an action in a shared package binding."
235+
def forbiddenGetAction(entityPath: String) =
236+
s"GET not permitted for '$entityPath' since it's an action in a shared package"
237+
def forbiddenGetPackageBinding(packageName: String) =
238+
s"GET not permitted since $packageName is a binding of a shared package"
239+
def forbiddenGetPackage(packageName: String) =
240+
s"GET not permitted for '$packageName' since it's a shared package"
232241
}
233242

234243
/** Replaces rejections with Json object containing cause and transaction id. */

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

Lines changed: 62 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ import org.apache.openwhisk.http.Messages._
4343
import org.apache.openwhisk.core.entitlement.Resource
4444
import org.apache.openwhisk.core.entitlement.Collection
4545
import org.apache.openwhisk.core.loadBalancer.LoadBalancerException
46+
import pureconfig._
47+
import org.apache.openwhisk.core.ConfigKeys
4648

4749
/**
4850
* 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
102104
/** Database service to get activations. */
103105
protected val activationStore: ActivationStore
104106

107+
/** Config flag for Execute Only for Actions in Shared Packages */
108+
protected def executeOnly =
109+
loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
110+
105111
/** Entity normalizer to JSON object. */
106112
import RestApiCommons.emptyEntityToJsObject
107113

@@ -330,6 +336,56 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
330336
deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({}))
331337
}
332338

339+
/**GET --save which retrieves all source code for an entity **/
340+
private def getEntityWithSourceCode(entityName: FullyQualifiedEntityName, env: Option[Parameters])(
341+
implicit transid: TransactionId) = {
342+
getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some { action: WhiskAction =>
343+
val mergedAction = env map {
344+
action inherit _
345+
} getOrElse action
346+
complete(OK, mergedAction)
347+
})
348+
}
349+
350+
/**Standard GET which just retrieves metadata**/
351+
private def getEntityMetaData(entityName: FullyQualifiedEntityName, env: Option[Parameters])(
352+
implicit transid: TransactionId) = {
353+
getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
354+
action: WhiskActionMetaData =>
355+
val mergedAction = env map {
356+
action inherit _
357+
} getOrElse action
358+
complete(OK, mergedAction)
359+
})
360+
}
361+
362+
/** Checks for package binding case. we don't want to allow get for a package binding in shared package */
363+
private def fetchEntity(entityName: FullyQualifiedEntityName, env: Option[Parameters], code: Boolean)(
364+
implicit transid: TransactionId) = {
365+
if (entityName.path.defaultPackage) {
366+
if (code) {
367+
getEntityWithSourceCode(entityName, env)
368+
} else {
369+
getEntityMetaData(entityName, env)
370+
}
371+
} else {
372+
getEntity(
373+
WhiskPackage.resolveBinding(entityStore, entityName.path.toDocId, mergeParameters = true),
374+
Some { pkg: WhiskPackage =>
375+
val originalPackageLocation = pkg.fullyQualifiedName(withVersion = false).namespace
376+
if (executeOnly && originalPackageLocation != entityName.namespace) {
377+
terminate(Forbidden, forbiddenGetActionBinding(entityName.toDocId.asString))
378+
} else {
379+
if (code) {
380+
getEntityWithSourceCode(entityName, env)
381+
} else {
382+
getEntityMetaData(entityName, env)
383+
}
384+
}
385+
})
386+
}
387+
}
388+
333389
/**
334390
* Gets action. The action name is prefixed with the namespace to create the primary index key.
335391
*
@@ -341,22 +397,12 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with
341397
override def fetch(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters])(
342398
implicit transid: TransactionId) = {
343399
parameter('code ? true) { code =>
344-
code match {
345-
case true =>
346-
getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some { action: WhiskAction =>
347-
val mergedAction = env map {
348-
action inherit _
349-
} getOrElse action
350-
complete(OK, mergedAction)
351-
})
352-
case false =>
353-
getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some {
354-
action: WhiskActionMetaData =>
355-
val mergedAction = env map {
356-
action inherit _
357-
} getOrElse action
358-
complete(OK, mergedAction)
359-
})
400+
//check if execute only is enabled, and if there is a discrepancy between the current user's namespace
401+
//and that of the entity we are trying to fetch
402+
if (executeOnly && user.namespace.name != entityName.namespace) {
403+
terminate(Forbidden, forbiddenGetAction(entityName.path.asString))
404+
} else {
405+
fetchEntity(entityName, env, code)
360406
}
361407
}
362408
}

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

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ import org.apache.openwhisk.core.entity._
3333
import org.apache.openwhisk.core.entity.types.EntityStore
3434
import org.apache.openwhisk.http.ErrorResponse.terminate
3535
import org.apache.openwhisk.http.Messages
36+
import org.apache.openwhisk.http.Messages._
37+
import pureconfig._
38+
import org.apache.openwhisk.core.ConfigKeys
3639

3740
trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
3841
services: WhiskServices =>
@@ -42,6 +45,10 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
4245
/** Database service to CRUD packages. */
4346
protected val entityStore: EntityStore
4447

48+
/** Config flag for Execute Only for Shared Packages */
49+
protected def executeOnly =
50+
loadConfigOrThrow[Boolean](ConfigKeys.sharedPackageExecuteOnly)
51+
4552
/** Notification service for cache invalidation. */
4653
protected implicit val cacheChangeNotification: Some[CacheChangeNotification]
4754

@@ -155,9 +162,18 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
155162
* - 404 Not Found
156163
* - 500 Internal Server Error
157164
*/
165+
//get method that also checks for execute only to deny access to shared package, but package binding is handled
166+
//within the mergePackageWithBinding() method
158167
override def fetch(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters])(
159168
implicit transid: TransactionId) = {
160-
getEntity(WhiskPackage.get(entityStore, entityName.toDocId), Some { mergePackageWithBinding() _ })
169+
if (executeOnly && user.namespace.name != entityName.namespace) {
170+
val value = entityName.toString
171+
terminate(Forbidden, forbiddenGetPackage(entityName.asString))
172+
} else {
173+
getEntity(WhiskPackage.get(entityStore, entityName.toDocId), Some {
174+
mergePackageWithBinding() _
175+
})
176+
}
161177
}
162178

163179
/**
@@ -303,9 +319,17 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with ReferencedEntities {
303319
logging.error(this, s"unexpected package binding refers to itself: $docid")
304320
terminate(UnprocessableEntity, Messages.packageBindingCircularReference(b.fullyQualifiedName.toString))
305321
} else {
306-
getEntity(WhiskPackage.get(entityStore, docid), Some {
307-
mergePackageWithBinding(Some { wp }) _
308-
})
322+
323+
/** Here's where I check package execute only case with package binding. */
324+
if (executeOnly && wp.namespace.asString != b.namespace.asString) {
325+
terminate(Forbidden, forbiddenGetPackageBinding(wp.name.asString))
326+
} else {
327+
getEntity(WhiskPackage.get(entityStore, docid), Some {
328+
mergePackageWithBinding(Some {
329+
wp
330+
}) _
331+
})
332+
}
309333
}
310334
} getOrElse {
311335
val pkg = ref map { _ inherit wp.parameters } getOrElse wp

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,4 +636,37 @@ class PackageActionsApiTests extends ControllerTestCommon with WhiskActionsApi {
636636
responseAs[ErrorResponse].error shouldBe Messages.corruptedEntity
637637
}
638638
}
639+
640+
var testExecuteOnly = false
641+
override def executeOnly = testExecuteOnly
642+
643+
it should ("allow access to get of action in binding of shared package when config option is disabled") in {
644+
testExecuteOnly = false
645+
implicit val tid = transid()
646+
val auser = WhiskAuthHelpers.newIdentity()
647+
val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
648+
val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
649+
val action = WhiskAction(provider.fullPath, aname(), jsDefault("??"), Parameters("a", "A"))
650+
put(entityStore, provider)
651+
put(entityStore, binding)
652+
put(entityStore, action)
653+
Get(s"$collectionPath/${binding.name}/${action.name}") ~> Route.seal(routes(auser)) ~> check {
654+
status should be(OK)
655+
}
656+
}
657+
658+
it should ("deny access to get of action in binding of shared package when config option is enabled") in {
659+
testExecuteOnly = true
660+
implicit val tid = transid()
661+
val auser = WhiskAuthHelpers.newIdentity()
662+
val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
663+
val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
664+
val action = WhiskAction(provider.fullPath, aname(), jsDefault("??"), Parameters("a", "A"))
665+
put(entityStore, provider)
666+
put(entityStore, binding)
667+
put(entityStore, action)
668+
Get(s"$collectionPath/${binding.name}/${action.name}") ~> Route.seal(routes(auser)) ~> check {
669+
status should be(Forbidden)
670+
}
671+
}
639672
}

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -884,4 +884,58 @@ class PackagesApiTests extends ControllerTestCommon with WhiskPackagesApi {
884884
responseAs[ErrorResponse].error shouldBe Messages.corruptedEntity
885885
}
886886
}
887+
888+
var testExecuteOnly = false
889+
override def executeOnly = testExecuteOnly
890+
891+
it should ("allow access to get of shared package binding when config option is disabled") in {
892+
testExecuteOnly = false
893+
implicit val tid = transid()
894+
val auser = WhiskAuthHelpers.newIdentity()
895+
val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
896+
val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
897+
put(entityStore, provider)
898+
put(entityStore, binding)
899+
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
900+
status should be(OK)
901+
}
902+
}
903+
904+
it should ("allow access to get of shared package when config option is disabled") in {
905+
testExecuteOnly = false
906+
implicit val tid = transid()
907+
val auser = WhiskAuthHelpers.newIdentity()
908+
val provider = WhiskPackage(namespace, aname(), None, publish = true)
909+
put(entityStore, provider)
910+
911+
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
912+
status should be(OK)
913+
}
914+
}
915+
916+
it should ("deny access to get of shared package binding when config option is enabled") in {
917+
testExecuteOnly = true
918+
implicit val tid = transid()
919+
val auser = WhiskAuthHelpers.newIdentity()
920+
val provider = WhiskPackage(namespace, aname(), None, Parameters("p", "P"), publish = true)
921+
val binding = WhiskPackage(EntityPath(auser.subject.asString), aname(), provider.bind, Parameters("b", "B"))
922+
put(entityStore, provider)
923+
put(entityStore, binding)
924+
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
925+
status should be(Forbidden)
926+
}
927+
928+
}
929+
930+
it should ("deny access to get of shared package when config option is enabled") in {
931+
testExecuteOnly = true
932+
implicit val tid = transid()
933+
val auser = WhiskAuthHelpers.newIdentity()
934+
val provider = WhiskPackage(namespace, aname(), None, publish = true)
935+
put(entityStore, provider)
936+
937+
Get(s"/$namespace/${collection.path}/${provider.name}") ~> Route.seal(routes(auser)) ~> check {
938+
status should be(Forbidden)
939+
}
940+
}
887941
}

0 commit comments

Comments
 (0)