From 5824b5c8c6304252afc1902d1a65df13e7adf1ef Mon Sep 17 00:00:00 2001 From: "ning.yougang" Date: Fri, 28 Sep 2018 17:44:29 +0800 Subject: [PATCH 1/5] Add protect feature to avoid update or delete actions by mistake - As more and more production system uses openwhisk, Users will need some features to protect their service safe from mistake. If we have this feature, user can protect their actions from updating or deletion by mistake. - Apply unix style permission management - Make the action's downloadable for the shared user --- .../openwhisk/core/entity/WhiskAction.scala | 53 ++++++ .../apache/openwhisk/http/ErrorResponse.scala | 9 +- .../openwhisk/core/controller/Actions.scala | 113 +++++++----- .../core/entitlement/Entitlement.scala | 126 ++++++++++++++ docs/actions.md | 58 +++++++ .../controller/test/ActionsApiTests.scala | 163 ++++++++++++++++++ 6 files changed, 473 insertions(+), 49 deletions(-) diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala index a51e5527889..2bea93cbdd3 100644 --- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala +++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala @@ -76,6 +76,16 @@ case class WhiskActionPut(exec: Option[Exec] = None, case _ => this } getOrElse this } + + protected[core] def getPermissions(): Option[String] = { + annotations match { + case Some(value) => + value + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + case None => None + } + } } abstract class WhiskActionLike(override val name: EntityName) extends WhiskEntity(name, "action") { @@ -352,6 +362,49 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[ val execFieldName = "exec" val requireWhiskAuthHeader = "x-require-whisk-auth" + // annotation permission key name + val permissionsFieldName = "permissions" + + val defaultPermissions = "rwxr-x" + + // notes on users, just have 2 type users, + // 1. the action's owner + // 2. the user (not the owner) who used the shared action directly(e.g. get, invoke), we call it "the shared user" + // + // Notes on permission control + // 1. the owner has read(or download) permission on any situation, but for the shared user, + // in spite of has read permission on any situation, but can set it undownloadable or downloadable + // 2. the shared user can't update/delete the action on any situation. + // 3. the owner's permission can affect the shared user's permission, e.g + // if the owner is not given execute permission, the shared user can't have execute permission as well. + // + // Notes on permission values, include below permission value + // 1. permission code:rwxr-x: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(yes), this is default + // 2. permission code:rwxr--: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(no) + // 3. permission code:r-xr-x: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(yes) + // 4. permission code:r-xr--: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(no) + // 5. permission code:r--r--: owner:read(yes)/write(no)/execute(no)|the shared action's user:download(yes)/write(no)/execute(no) + // 6. permission code:rw-r--: owner:read(yes)/write(yes)/execute(no)|the shared action's user:download(yes)/write(no)/execute(no) + // 7. permission code:rwx--x: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(no)/write(no)/execute(yes) + // 8. permission code:rwx---: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(no)/write(no)/execute(no) + // 9. permission code:r-x--x: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(no)/write(no)/execute(yes) + // 10. permission code:r-x---: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(no)/write(no)/execute(no) + // 11. permission code:r-----: owner:read(yes)/write(no)/execute(no)|the shared action's user:download(no)/write(no)/execute(no) + // 12. permission code:rw----: owner:read(yes)/write(yes)/execute(no)|the shared action's user:download(no)/write(no)/execute(no) + val permissionList = List( + defaultPermissions, + "rwxr--", + "r-xr-x", + "r-xr--", + "r--r--", + "rw-r--", + "rwx--x", + "rwx---", + "r-x--x", + "r-x---", + "r-----", + "rw----") + override val collectionName = "actions" override val cacheEnabled = true diff --git a/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala b/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala index 695ec176084..707433675b1 100644 --- a/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala +++ b/common/scala/src/main/scala/org/apache/openwhisk/http/ErrorResponse.scala @@ -20,7 +20,6 @@ package org.apache.openwhisk.http import scala.concurrent.duration.Duration import scala.concurrent.duration.FiniteDuration import scala.util.Try - import akka.http.scaladsl.model.StatusCode import akka.http.scaladsl.model.StatusCodes.Forbidden import akka.http.scaladsl.model.StatusCodes.NotFound @@ -28,15 +27,9 @@ import akka.http.scaladsl.model.MediaType import akka.http.scaladsl.server.Directives import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport.sprayJsonMarshaller import akka.http.scaladsl.server.StandardRoute - import spray.json._ - import org.apache.openwhisk.common.TransactionId -import org.apache.openwhisk.core.entity.SizeError -import org.apache.openwhisk.core.entity.ByteSize -import org.apache.openwhisk.core.entity.Exec -import org.apache.openwhisk.core.entity.ExecMetaDataBase -import org.apache.openwhisk.core.entity.ActivationId +import org.apache.openwhisk.core.entity.{ActivationId, ByteSize, Exec, ExecMetaDataBase, SizeError} object Messages { diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala index a6e4554b071..320c32edebe 100644 --- a/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala +++ b/core/controller/src/main/scala/org/apache/openwhisk/core/controller/Actions.scala @@ -218,9 +218,22 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with onComplete(checkAdditionalPrivileges) { case Success(_) => - putEntity(WhiskAction, entityStore, entityName.toDocId, overwrite, update(user, request) _, () => { - make(user, entityName, request) - }) + val operation = if (overwrite) "update" else "create" + onComplete( + entitlementProvider + .checkActionPermissions( + operation, + user, + entityStore, + entityName, + WhiskAction.get, + content.getPermissions())) { + case Success(_) => + putEntity(WhiskAction, entityStore, entityName.toDocId, overwrite, update(user, request) _, () => { + make(user, entityName, request) + }) + case Failure(f) => super.handleEntitlementFailure(f) + } case Failure(f) => super.handleEntitlementFailure(f) } @@ -241,37 +254,43 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with */ override def activate(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters])( implicit transid: TransactionId) = { - parameter( - 'blocking ? false, - 'result ? false, - 'timeout.as[FiniteDuration] ? controllerActivationConfig.maxWaitForBlockingActivation) { - (blocking, result, waitOverride) => - entity(as[Option[JsObject]]) { payload => - getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some { - act: WhiskActionMetaData => - // resolve the action --- special case for sequences that may contain components with '_' as default package - val action = act.resolve(user.namespace) - onComplete(entitleReferencedEntitiesMetaData(user, Privilege.ACTIVATE, Some(action.exec))) { - case Success(_) => - val actionWithMergedParams = env.map(action.inherit(_)) getOrElse action - - // incoming parameters may not override final parameters (i.e., parameters with already defined values) - // on an action once its parameters are resolved across package and binding - val allowInvoke = payload - .map(_.fields.keySet.forall(key => !actionWithMergedParams.immutableParameters.contains(key))) - .getOrElse(true) + onComplete( + entitlementProvider + .checkActionPermissions("invoke", user, entityStore, entityName, WhiskAction.get)) { + case Success(_) => + parameter( + 'blocking ? false, + 'result ? false, + 'timeout.as[FiniteDuration] ? controllerActivationConfig.maxWaitForBlockingActivation) { + (blocking, result, waitOverride) => + entity(as[Option[JsObject]]) { payload => + getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some { + act: WhiskActionMetaData => + // resolve the action --- special case for sequences that may contain components with '_' as default package + val action = act.resolve(user.namespace) + onComplete(entitleReferencedEntitiesMetaData(user, Privilege.ACTIVATE, Some(action.exec))) { + case Success(_) => + val actionWithMergedParams = env.map(action.inherit(_)) getOrElse action + + // incoming parameters may not override final parameters (i.e., parameters with already defined values) + // on an action once its parameters are resolved across package and binding + val allowInvoke = payload + .map(_.fields.keySet.forall(key => !actionWithMergedParams.immutableParameters.contains(key))) + .getOrElse(true) + + if (allowInvoke) { + doInvoke(user, actionWithMergedParams, payload, blocking, waitOverride, result) + } else { + terminate(BadRequest, Messages.parametersNotAllowed) + } - if (allowInvoke) { - doInvoke(user, actionWithMergedParams, payload, blocking, waitOverride, result) - } else { - terminate(BadRequest, Messages.parametersNotAllowed) + case Failure(f) => + super.handleEntitlementFailure(f) } - - case Failure(f) => - super.handleEntitlementFailure(f) - } - }) + }) + } } + case Failure(f) => super.handleEntitlementFailure(f) } } @@ -333,11 +352,17 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with * - 500 Internal Server Error */ override def remove(user: Identity, entityName: FullyQualifiedEntityName)(implicit transid: TransactionId) = { - deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({})) + onComplete( + entitlementProvider + .checkActionPermissions("remove", user, entityStore, entityName, WhiskAction.get)) { + case Success(_) => + deleteEntity(WhiskAction, entityStore, entityName.toDocId, (a: WhiskAction) => Future.successful({})) + case Failure(f) => super.handleEntitlementFailure(f) + } } /** 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)( + private def fetchEntity(user: Identity, entityName: FullyQualifiedEntityName, env: Option[Parameters], code: Boolean)( implicit transid: TransactionId) = { val resolvedPkg: Future[Either[String, FullyQualifiedEntityName]] = if (entityName.path.defaultPackage) { Future.successful(Right(entityName)) @@ -357,13 +382,19 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with case Left(f) => terminate(Forbidden, f) case Right(_) => if (code) { - getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some { - action: WhiskAction => - val mergedAction = env map { - action inherit _ - } getOrElse action - complete(OK, mergedAction) - }) + onComplete( + entitlementProvider + .checkActionPermissions("download", user, entityStore, entityName, WhiskAction.get)) { + case Success(_) => + getEntity(WhiskAction.resolveActionAndMergeParameters(entityStore, entityName), Some { + action: WhiskAction => + val mergedAction = env map { + action inherit _ + } getOrElse action + complete(OK, mergedAction) + }) + case Failure(f) => super.handleEntitlementFailure(f) + } } else { getEntity(WhiskActionMetaData.resolveActionAndMergeParameters(entityStore, entityName), Some { action: WhiskActionMetaData => @@ -396,7 +427,7 @@ trait WhiskActionsApi extends WhiskCollectionAPI with PostActionActivation with if (executeOnly && user.namespace.name != entityName.namespace) { terminate(Forbidden, forbiddenGetAction(entityName.path.asString)) } else { - fetchEntity(entityName, env, code) + fetchEntity(user, entityName, env, code) } } } diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala index 8092ad8f0bd..1fa33d1dfff 100644 --- a/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala +++ b/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala @@ -36,9 +36,12 @@ import org.apache.openwhisk.core.loadBalancer.{LoadBalancer, ShardingContainerPo import org.apache.openwhisk.http.ErrorResponse import org.apache.openwhisk.http.Messages import org.apache.openwhisk.core.connector.MessagingProvider +import org.apache.openwhisk.core.database.ArtifactStore import org.apache.openwhisk.spi.SpiLoader import org.apache.openwhisk.spi.Spi +import spray.json.DefaultJsonProtocol._ + object types { type Entitlements = TrieMap[(Subject, String), Set[Privilege]] } @@ -244,6 +247,129 @@ protected[core] abstract class EntitlementProvider( .getOrElse(Future.successful(())) } + /** + * Checks if action operation(get/write/execute) whether feasible + * + * @param operation the action operation, e.g. get/write/execute + * @param user the user who get/write/execute the action + * @param entityStore store to write the action to + * @param entityName entityName + * @param permissions the passed permission code + * @return a promise that completes with success iff action operation is feasible + */ + protected[core] def checkActionPermissions( + operation: String, + user: Identity, + entityStore: ArtifactStore[WhiskEntity], + entityName: FullyQualifiedEntityName, + get: (ArtifactStore[WhiskEntity], DocId, DocRevision, Boolean) => Future[WhiskAction], + permissions: Option[String] = None)(implicit transid: TransactionId): Future[Unit] = { + if (operation == "create") { + permissions + .map { value => + if (WhiskAction.permissionList.contains(value)) { + Future.successful(()) + } else { + val errorInfo = + s"give error permission code: ${value}, available permission is in ${WhiskAction.permissionList}" + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } + } + .getOrElse(Future.successful(())) + } else if (operation == "update") { + get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + val errorInfo = s"have no permission to ${operation} this action" + permissions match { + case Some(value) => + if (!WhiskAction.permissionList.contains(value)) { + val errorInfo = + s"give error permission code: ${value}, available permission is in ${WhiskAction.permissionList}" + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + val passedUpdatePermission = value.charAt(1) + if (passedUpdatePermission == 'w') { // make it to modifiable + Future.successful(()) + } else { + val currentUpdatePermission = currentPermissions.charAt(1) + if (currentUpdatePermission == '-') { + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } + } + } + case None => + val currentUpdatePermission = currentPermissions.charAt(1) + if (currentUpdatePermission == '-') { + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } + } + } + } else if (operation == "remove") { + get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + val currentUpdatePermission = currentPermissions.charAt(1) + if (currentUpdatePermission == '-') { + val errorInfo = s"have no permission to ${operation} this action" + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } + } + } else if (operation == "invoke") { + get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + // the user who is owner by default + var currentExecutePermission = currentPermissions.charAt(2) + var errorInfo = s"have no permission to ${operation} this action" + if (user.namespace.name.asString != entityName.path.root.asString) { // the user who invoke the shared action + currentExecutePermission = currentPermissions.charAt(5) + errorInfo = s"have no permission to ${operation} this shared action" + } + if (currentExecutePermission == '-') { //have no permission + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } + } + } else { // download the code + get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + val errorInfo = s"have no permission to download this shared action" + val currentDownloadPermission = currentPermissions.charAt(3) + if (user.namespace.name.asString != entityName.path.root.asString) { // the shared user who download the action + if (currentDownloadPermission == '-') { + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } + } else { + // the owner has download permission on any situation + Future.successful(()) + } + } + } + } + /** * Checks if a subject has the right to access a specific resource. The entitlement may be implicit, * that is, inferred based on namespaces that a subject belongs to and the namespace of the diff --git a/docs/actions.md b/docs/actions.md index fa59b8e1969..f4973ecb3ab 100644 --- a/docs/actions.md +++ b/docs/actions.md @@ -658,6 +658,64 @@ You can clean up by deleting actions that you do not want to use. actions /guest/mySequence private sequence ``` +## action permission management + +* Notes on users, just have 2 type users, + - the action's owner + - the user (not the owner) who used the shared action directly(e.g. get, invoke), we call it "the shared user" + +* Notes on permission control + - the owner has read(or download) permission on any situation, but for the shared user, + in spite of has read permission on any situation, but can set it undownloadable or downloadable + - the shared user can't update/delete the action forever. + - the owner's permission can affect other user's permission, e.g + if the owner is not given execute permission, the shared user can't have execute permission as well. + +* Notes on permission values, include below permission value + - permission code:rwxr-x: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:read(yes)/write(no)/execute(yes), this is default + - permission code:rwxr--: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:read(yes)/write(no)/execute(no) + - permission code:r-xr-x: owner:read(yes)/write(no)/execute(yes)|the shared action's user:read(yes)/write(no)/execute(yes) + - permission code:r-xr--: owner:read(yes)/write(no)/execute(yes)|the shared action's user:read(yes)/write(no)/execute(no) + - permission code:r--r--: owner:read(yes)/write(no)/execute(no)|the shared action's user:read(yes)/write(no)/execute(no) + - permission code:rw-r--: owner:read(yes)/write(yes)/execute(no)|the shared action's user:read(yes)/write(no)/execute(no) + - permission code:rwx--x: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(no)/write(no)/execute(yes) + - permission code:rwx---: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(no)/write(no)/execute(no) + - permission code:r-x--x: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(no)/write(no)/execute(yes) + - permission code:r-x---: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(no)/write(no)/execute(no) + - permission code:r-----: owner:read(yes)/write(no)/execute(no)|the shared action's user:download(no)/write(no)/execute(no) + - permission code:rw----: owner:read(yes)/write(yes)/execute(no)|the shared action's user:download(no)/write(no)/execute(no) + +When create action without permissions annotation, permission control keeps the same as before, +e.g. the owner has all permissions(create/update(or delete)/invoke), the user(not owner) doesn't have update/delete permission on the shared action. + +When create(or update) action with permissions annotation, must specify the permissions annotation to a correct scope, e.g. +``` +wsk action create ${action} ${code_path} --annotation permissions ${permission code in correct scope} +``` + +Make action unmodifiable(can't updated and can't deleted), e.g. +``` +wsk action update ${action} --annotation permissions r-xr-x +``` +Then, it will be failed when update this action or delete action. + +Make action modifiable again, e.g. +``` +wsk action update ${action} --annotation permissions rwxr-x +``` +Then, it will be successful when update this action or delete action. + +Make action unexecutable, e.g. +``` +wsk action update ${action} --annotation permissions rw-r-- +``` +Then, it will be failed when invoke this action. + +Make action executable again, e.g. +``` +wsk action update ${action} --annotation permissions rwxr-x +``` +Then, it will be successful again when invoke this action. ## Accessing action metadata within the action body diff --git a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/ActionsApiTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/ActionsApiTests.scala index cf38dd8b322..cbfb9dbf83a 100644 --- a/tests/src/test/scala/org/apache/openwhisk/core/controller/test/ActionsApiTests.scala +++ b/tests/src/test/scala/org/apache/openwhisk/core/controller/test/ActionsApiTests.scala @@ -435,6 +435,169 @@ class ActionsApiTests extends ControllerTestCommon with WhiskActionsApi { } } + it should "create action not allowed when permission code is not legal" in { + implicit val tid = transid() + + // Below permission code(rwxrwx) is not legal, because the user(not owner) can't update/delete shared action + val action = + WhiskAction( + namespace, + aname(), + jsDefault(""), + annotations = Parameters(WhiskAction.permissionsFieldName, JsString("rwxrwx"))) + val content = JsObject( + "exec" -> JsObject("code" -> "".toJson, "kind" -> action.exec.kind.toJson), + "annotations" -> action.annotations.toJson) + + // create failed due to the permission code(rwxrwx) is not legal + Put(s"$collectionPath/${action.name}", content) ~> Route.seal(routes(creds)) ~> check { + status should be(Forbidden) + } + } + + it should "create action allowed when permission code is legal" in { + implicit val tid = transid() + + val action = + WhiskAction( + namespace, + aname(), + jsDefault(""), + annotations = Parameters(WhiskAction.permissionsFieldName, JsString("rwxr-x"))) + val content = JsObject( + "exec" -> JsObject("code" -> "".toJson, "kind" -> action.exec.kind.toJson), + "annotations" -> action.annotations.toJson) + + // create successfully because the permission code(rwxr-x) is legal + Put(s"$collectionPath/${action.name}", content) ~> Route.seal(routes(creds)) ~> check { + status should be(OK) + } + } + + it should "the owner can not update action when permission code's write bit is unwritable" in { + implicit val tid = transid() + val action = + WhiskAction( + namespace, + aname(), + jsDefault(""), + annotations = Parameters(WhiskAction.permissionsFieldName, JsString("r-xr-x"))) + val content = JsObject( + "exec" -> JsObject("code" -> "".toJson, "kind" -> action.exec.kind.toJson), + "annotations" -> action.annotations.toJson) + + // create successfully + Put(s"$collectionPath/${action.name}", content) ~> Route.seal(routes(creds)) ~> check { + status should be(OK) + } + + // update not allowed because the owner's write bit is unwritable + Put(s"$collectionPath/${action.name}?overwrite=true", content) ~> Route.seal(routes(creds)) ~> check { + status should be(Forbidden) + } + + // update the owner's write bit to writable + val writableAnnotation = + JsObject("annotations" -> Parameters(WhiskAction.permissionsFieldName, JsString("rwxr-x")).toJson) + Put(s"$collectionPath/${action.name}?overwrite=true", writableAnnotation) ~> Route.seal(routes(creds)) ~> check { + status should be(OK) + } + + // update allowed after change the owner's write bit to writable + Put(s"$collectionPath/${action.name}?overwrite=true", content) ~> Route.seal(routes(creds)) ~> check { + status should be(OK) + } + } + + it should "the owner can not delete action when permission code's write bit is unwritable" in { + implicit val tid = transid() + val action = + WhiskAction( + namespace, + aname(), + jsDefault(""), + annotations = Parameters(WhiskAction.permissionsFieldName, JsString("r-xr-x"))) + val content = JsObject( + "exec" -> JsObject("code" -> "".toJson, "kind" -> action.exec.kind.toJson), + "annotations" -> action.annotations.toJson) + + // create successfully + Put(s"$collectionPath/${action.name}", content) ~> Route.seal(routes(creds)) ~> check { + status should be(OK) + } + + // delete not allowed because the owner's write bit is unwritable + Delete(s"$collectionPath/${action.name}") ~> Route.seal(routes(creds)) ~> check { + status should be(Forbidden) + } + + // update the owner's write bit to writable + val writableAnnotation = + JsObject("annotations" -> Parameters(WhiskAction.permissionsFieldName, JsString("rwxr-x")).toJson) + Put(s"$collectionPath/${action.name}?overwrite=true", writableAnnotation) ~> Route.seal(routes(creds)) ~> check { + status should be(OK) + } + + // delete allowed after change the owner's write bit to writable + Delete(s"$collectionPath/${action.name}") ~> Route.seal(routes(creds)) ~> check { + status should be(OK) + } + } + + it should "the owner can not invoke the action when permission code's execute bit is unexecutable" in { + implicit val tid = transid() + val action = WhiskAction( + namespace, + aname(), + jsDefault("??"), + annotations = Parameters(WhiskAction.permissionsFieldName, JsString("rwxr-x"))) + val activation = WhiskActivation( + action.namespace, + action.name, + creds.subject, + activationIdFactory.make(), + start = Instant.now, + end = Instant.now, + response = ActivationResponse.success(Some(JsObject("test" -> "yes".toJson)))) + put(entityStore, action) + + try { + // do not store the activation in the db, instead register it as the response to generate on active ack + loadBalancer.whiskActivationStub = Some((500.milliseconds, Right(activation))) + + // invoke successfully because the owner's execute bit is executable + Post(s"$collectionPath/${action.name}?blocking=true") ~> Route.seal(routes(creds)) ~> check { + status shouldBe OK + } + + // update the owner's execute bit to unexecutable + val unexecutableAnnotation = + JsObject("annotations" -> Parameters(WhiskAction.permissionsFieldName, JsString("rw-r--")).toJson) + Put(s"$collectionPath/${action.name}?overwrite=true", unexecutableAnnotation) ~> Route.seal(routes(creds)) ~> check { + status should be(OK) + } + + // invoke failed after change the owner's execute bit to unexecutable + Post(s"$collectionPath/${action.name}?blocking=true") ~> Route.seal(routes(creds)) ~> check { + status shouldBe Forbidden + } + + // update the owner's execute bit to executable again + val executableAnnotation = + JsObject("annotations" -> Parameters(WhiskAction.permissionsFieldName, JsString("rwxr--")).toJson) + Put(s"$collectionPath/${action.name}?overwrite=true", executableAnnotation) ~> Route.seal(routes(creds)) ~> check { + status should be(OK) + } + + // invoke successfully after change the owner's execute bit to executable again + Post(s"$collectionPath/${action.name}?blocking=true") ~> Route.seal(routes(creds)) ~> check { + status should be(OK) + } + } finally { + loadBalancer.whiskActivationStub = None + } + } + it should "report NotFound for delete non existent action" in { implicit val tid = transid() Delete(s"$collectionPath/xyz") ~> Route.seal(routes(creds)) ~> check { From b9a8ca8febf9c5fb9c6f4606b269819a3c54b4aa Mon Sep 17 00:00:00 2001 From: "ning.yougang" Date: Tue, 8 Sep 2020 13:36:10 +0800 Subject: [PATCH 2/5] Fix test cases error --- .../core/entitlement/Entitlement.scala | 178 ++++++++++-------- 1 file changed, 103 insertions(+), 75 deletions(-) diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala index 1fa33d1dfff..358b1155c15 100644 --- a/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala +++ b/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala @@ -277,96 +277,124 @@ protected[core] abstract class EntitlementProvider( } .getOrElse(Future.successful(())) } else if (operation == "update") { - get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction => - val currentPermissions = whiskAction.annotations - .get(WhiskAction.permissionsFieldName) - .map(value => value.convertTo[String]) - .getOrElse(WhiskAction.defaultPermissions) - - val errorInfo = s"have no permission to ${operation} this action" - permissions match { - case Some(value) => - if (!WhiskAction.permissionList.contains(value)) { - val errorInfo = - s"give error permission code: ${value}, available permission is in ${WhiskAction.permissionList}" - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) - } else { - val passedUpdatePermission = value.charAt(1) - if (passedUpdatePermission == 'w') { // make it to modifiable - Future.successful(()) + get(entityStore, entityName.toDocId, DocRevision.empty, true) + .flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + val errorInfo = s"have no permission to ${operation} this action" + permissions match { + case Some(value) => + if (!WhiskAction.permissionList.contains(value)) { + val errorInfo = + s"give error permission code: ${value}, available permission is in ${WhiskAction.permissionList}" + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) } else { - val currentUpdatePermission = currentPermissions.charAt(1) - if (currentUpdatePermission == '-') { - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) - } else { + val passedUpdatePermission = value.charAt(1) + if (passedUpdatePermission == 'w') { // make it to modifiable Future.successful(()) + } else { + val currentUpdatePermission = currentPermissions.charAt(1) + if (currentUpdatePermission == '-') { + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } } } - } - case None => - val currentUpdatePermission = currentPermissions.charAt(1) - if (currentUpdatePermission == '-') { - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) - } else { - Future.successful(()) - } + case None => + val currentUpdatePermission = currentPermissions.charAt(1) + if (currentUpdatePermission == '-') { + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } + } + } + .recoverWith { + case t: RejectRequest => + Future.failed(t) + case _ => + Future.successful(()) } - } } else if (operation == "remove") { - get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction => - val currentPermissions = whiskAction.annotations - .get(WhiskAction.permissionsFieldName) - .map(value => value.convertTo[String]) - .getOrElse(WhiskAction.defaultPermissions) - - val currentUpdatePermission = currentPermissions.charAt(1) - if (currentUpdatePermission == '-') { - val errorInfo = s"have no permission to ${operation} this action" - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) - } else { - Future.successful(()) + get(entityStore, entityName.toDocId, DocRevision.empty, true) + .flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + val currentUpdatePermission = currentPermissions.charAt(1) + if (currentUpdatePermission == '-') { + val errorInfo = s"have no permission to ${operation} this action" + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } + } + .recoverWith { + case t: RejectRequest => + Future.failed(t) + case _ => + Future.successful(()) } - } } else if (operation == "invoke") { - get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction => - val currentPermissions = whiskAction.annotations - .get(WhiskAction.permissionsFieldName) - .map(value => value.convertTo[String]) - .getOrElse(WhiskAction.defaultPermissions) - - // the user who is owner by default - var currentExecutePermission = currentPermissions.charAt(2) - var errorInfo = s"have no permission to ${operation} this action" - if (user.namespace.name.asString != entityName.path.root.asString) { // the user who invoke the shared action - currentExecutePermission = currentPermissions.charAt(5) - errorInfo = s"have no permission to ${operation} this shared action" + get(entityStore, entityName.toDocId, DocRevision.empty, true) + .flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + // the user who is owner by default + var currentExecutePermission = currentPermissions.charAt(2) + var errorInfo = s"have no permission to ${operation} this action" + if (user.namespace.name.asString != entityName.path.root.asString) { // the user who invoke the shared action + currentExecutePermission = currentPermissions.charAt(5) + errorInfo = s"have no permission to ${operation} this shared action" + } + if (currentExecutePermission == '-') { //have no permission + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } } - if (currentExecutePermission == '-') { //have no permission - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) - } else { - Future.successful(()) + .recoverWith { + case t: RejectRequest => + Future.failed(t) + case _ => + Future.successful(()) } - } } else { // download the code - get(entityStore, entityName.toDocId, DocRevision.empty, true).flatMap { whiskAction => - val currentPermissions = whiskAction.annotations - .get(WhiskAction.permissionsFieldName) - .map(value => value.convertTo[String]) - .getOrElse(WhiskAction.defaultPermissions) - - val errorInfo = s"have no permission to download this shared action" - val currentDownloadPermission = currentPermissions.charAt(3) - if (user.namespace.name.asString != entityName.path.root.asString) { // the shared user who download the action - if (currentDownloadPermission == '-') { - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + get(entityStore, entityName.toDocId, DocRevision.empty, true) + .flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + val errorInfo = s"have no permission to download this shared action" + val currentDownloadPermission = currentPermissions.charAt(3) + if (user.namespace.name.asString != entityName.path.root.asString) { // the shared user who download the action + if (currentDownloadPermission == '-') { + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } } else { + // the owner has download permission on any situation Future.successful(()) } - } else { - // the owner has download permission on any situation - Future.successful(()) } - } + .recoverWith { + case t: RejectRequest => + Future.failed(t) + case _ => + Future.successful(()) + } } } From c00f5f6d0a4156505395a8c1e15c2e3c38c529b2 Mon Sep 17 00:00:00 2001 From: "ning.yougang" Date: Wed, 14 Oct 2020 13:37:06 +0800 Subject: [PATCH 3/5] Fix review points --- .../openwhisk/core/entity/WhiskAction.scala | 26 ++----------------- 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala index 2bea93cbdd3..921dff0d0ff 100644 --- a/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala +++ b/common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala @@ -367,30 +367,8 @@ object WhiskAction extends DocumentFactory[WhiskAction] with WhiskEntityQueries[ val defaultPermissions = "rwxr-x" - // notes on users, just have 2 type users, - // 1. the action's owner - // 2. the user (not the owner) who used the shared action directly(e.g. get, invoke), we call it "the shared user" - // - // Notes on permission control - // 1. the owner has read(or download) permission on any situation, but for the shared user, - // in spite of has read permission on any situation, but can set it undownloadable or downloadable - // 2. the shared user can't update/delete the action on any situation. - // 3. the owner's permission can affect the shared user's permission, e.g - // if the owner is not given execute permission, the shared user can't have execute permission as well. - // - // Notes on permission values, include below permission value - // 1. permission code:rwxr-x: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(yes), this is default - // 2. permission code:rwxr--: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(no) - // 3. permission code:r-xr-x: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(yes) - // 4. permission code:r-xr--: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(yes)/write(no)/execute(no) - // 5. permission code:r--r--: owner:read(yes)/write(no)/execute(no)|the shared action's user:download(yes)/write(no)/execute(no) - // 6. permission code:rw-r--: owner:read(yes)/write(yes)/execute(no)|the shared action's user:download(yes)/write(no)/execute(no) - // 7. permission code:rwx--x: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(no)/write(no)/execute(yes) - // 8. permission code:rwx---: owner:read(yes)/write(yes)/execute(yes)|the shared action's user:download(no)/write(no)/execute(no) - // 9. permission code:r-x--x: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(no)/write(no)/execute(yes) - // 10. permission code:r-x---: owner:read(yes)/write(no)/execute(yes)|the shared action's user:download(no)/write(no)/execute(no) - // 11. permission code:r-----: owner:read(yes)/write(no)/execute(no)|the shared action's user:download(no)/write(no)/execute(no) - // 12. permission code:rw----: owner:read(yes)/write(yes)/execute(no)|the shared action's user:download(no)/write(no)/execute(no) + // For more information about permission, please refer to the online manual at + // https://github.com/apache/openwhisk/blob/master/docs/actions.md#action-permission-management val permissionList = List( defaultPermissions, "rwxr--", From 51ca755e779dd06e5cf05d9814f2469890bba3f9 Mon Sep 17 00:00:00 2001 From: "ning.yougang" Date: Fri, 28 May 2021 14:02:09 +0800 Subject: [PATCH 4/5] Optimize the download code for shared user readable --- .../org/apache/openwhisk/core/entitlement/Entitlement.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala index 358b1155c15..817ef295b32 100644 --- a/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala +++ b/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala @@ -376,10 +376,10 @@ protected[core] abstract class EntitlementProvider( .map(value => value.convertTo[String]) .getOrElse(WhiskAction.defaultPermissions) - val errorInfo = s"have no permission to download this shared action" - val currentDownloadPermission = currentPermissions.charAt(3) if (user.namespace.name.asString != entityName.path.root.asString) { // the shared user who download the action - if (currentDownloadPermission == '-') { + val errorInfo = s"have no permission to download this shared action" + val downloadPermissionOfSharedUser = currentPermissions.charAt(3) + if (downloadPermissionOfSharedUser == '-') { Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) } else { Future.successful(()) From 92b2a38ecae2a7dece59341fe7ea867d95a1a97f Mon Sep 17 00:00:00 2001 From: "ning.yougang" Date: Fri, 28 May 2021 14:20:03 +0800 Subject: [PATCH 5/5] Change from if/else to match/case statement --- .../core/entitlement/Entitlement.scala | 236 +++++++++--------- 1 file changed, 119 insertions(+), 117 deletions(-) diff --git a/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala b/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala index 817ef295b32..a9881467c48 100644 --- a/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala +++ b/core/controller/src/main/scala/org/apache/openwhisk/core/entitlement/Entitlement.scala @@ -264,137 +264,139 @@ protected[core] abstract class EntitlementProvider( entityName: FullyQualifiedEntityName, get: (ArtifactStore[WhiskEntity], DocId, DocRevision, Boolean) => Future[WhiskAction], permissions: Option[String] = None)(implicit transid: TransactionId): Future[Unit] = { - if (operation == "create") { - permissions - .map { value => - if (WhiskAction.permissionList.contains(value)) { - Future.successful(()) - } else { - val errorInfo = - s"give error permission code: ${value}, available permission is in ${WhiskAction.permissionList}" - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + operation match { + case "create" => + permissions + .map { value => + if (WhiskAction.permissionList.contains(value)) { + Future.successful(()) + } else { + val errorInfo = + s"give error permission code: ${value}, available permission is in ${WhiskAction.permissionList}" + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } } - } - .getOrElse(Future.successful(())) - } else if (operation == "update") { - get(entityStore, entityName.toDocId, DocRevision.empty, true) - .flatMap { whiskAction => - val currentPermissions = whiskAction.annotations - .get(WhiskAction.permissionsFieldName) - .map(value => value.convertTo[String]) - .getOrElse(WhiskAction.defaultPermissions) - - val errorInfo = s"have no permission to ${operation} this action" - permissions match { - case Some(value) => - if (!WhiskAction.permissionList.contains(value)) { - val errorInfo = - s"give error permission code: ${value}, available permission is in ${WhiskAction.permissionList}" - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) - } else { - val passedUpdatePermission = value.charAt(1) - if (passedUpdatePermission == 'w') { // make it to modifiable - Future.successful(()) + .getOrElse(Future.successful(())) + case "update" => + get(entityStore, entityName.toDocId, DocRevision.empty, true) + .flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + val errorInfo = s"have no permission to ${operation} this action" + permissions match { + case Some(value) => + if (!WhiskAction.permissionList.contains(value)) { + val errorInfo = + s"give error permission code: ${value}, available permission is in ${WhiskAction.permissionList}" + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) } else { - val currentUpdatePermission = currentPermissions.charAt(1) - if (currentUpdatePermission == '-') { - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) - } else { + val passedUpdatePermission = value.charAt(1) + if (passedUpdatePermission == 'w') { // make it to modifiable Future.successful(()) + } else { + val currentUpdatePermission = currentPermissions.charAt(1) + if (currentUpdatePermission == '-') { + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } } } - } - case None => - val currentUpdatePermission = currentPermissions.charAt(1) - if (currentUpdatePermission == '-') { - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) - } else { - Future.successful(()) - } + case None => + val currentUpdatePermission = currentPermissions.charAt(1) + if (currentUpdatePermission == '-') { + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } + } } - } - .recoverWith { - case t: RejectRequest => - Future.failed(t) - case _ => - Future.successful(()) - } - } else if (operation == "remove") { - get(entityStore, entityName.toDocId, DocRevision.empty, true) - .flatMap { whiskAction => - val currentPermissions = whiskAction.annotations - .get(WhiskAction.permissionsFieldName) - .map(value => value.convertTo[String]) - .getOrElse(WhiskAction.defaultPermissions) - - val currentUpdatePermission = currentPermissions.charAt(1) - if (currentUpdatePermission == '-') { - val errorInfo = s"have no permission to ${operation} this action" - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) - } else { - Future.successful(()) + .recoverWith { + case t: RejectRequest => + Future.failed(t) + case _ => + Future.successful(()) } - } - .recoverWith { - case t: RejectRequest => - Future.failed(t) - case _ => - Future.successful(()) - } - } else if (operation == "invoke") { - get(entityStore, entityName.toDocId, DocRevision.empty, true) - .flatMap { whiskAction => - val currentPermissions = whiskAction.annotations - .get(WhiskAction.permissionsFieldName) - .map(value => value.convertTo[String]) - .getOrElse(WhiskAction.defaultPermissions) - - // the user who is owner by default - var currentExecutePermission = currentPermissions.charAt(2) - var errorInfo = s"have no permission to ${operation} this action" - if (user.namespace.name.asString != entityName.path.root.asString) { // the user who invoke the shared action - currentExecutePermission = currentPermissions.charAt(5) - errorInfo = s"have no permission to ${operation} this shared action" + case "remove" => + get(entityStore, entityName.toDocId, DocRevision.empty, true) + .flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + val currentUpdatePermission = currentPermissions.charAt(1) + if (currentUpdatePermission == '-') { + val errorInfo = s"have no permission to ${operation} this action" + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } } - if (currentExecutePermission == '-') { //have no permission - Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) - } else { - Future.successful(()) + .recoverWith { + case t: RejectRequest => + Future.failed(t) + case _ => + Future.successful(()) } - } - .recoverWith { - case t: RejectRequest => - Future.failed(t) - case _ => - Future.successful(()) - } - } else { // download the code - get(entityStore, entityName.toDocId, DocRevision.empty, true) - .flatMap { whiskAction => - val currentPermissions = whiskAction.annotations - .get(WhiskAction.permissionsFieldName) - .map(value => value.convertTo[String]) - .getOrElse(WhiskAction.defaultPermissions) - - if (user.namespace.name.asString != entityName.path.root.asString) { // the shared user who download the action - val errorInfo = s"have no permission to download this shared action" - val downloadPermissionOfSharedUser = currentPermissions.charAt(3) - if (downloadPermissionOfSharedUser == '-') { + case "invoke" => + get(entityStore, entityName.toDocId, DocRevision.empty, true) + .flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + // the user who is owner by default + var currentExecutePermission = currentPermissions.charAt(2) + var errorInfo = s"have no permission to ${operation} this action" + if (user.namespace.name.asString != entityName.path.root.asString) { // the user who invoke the shared action + currentExecutePermission = currentPermissions.charAt(5) + errorInfo = s"have no permission to ${operation} this shared action" + } + if (currentExecutePermission == '-') { //have no permission Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) } else { Future.successful(()) } - } else { - // the owner has download permission on any situation - Future.successful(()) } - } - .recoverWith { - case t: RejectRequest => - Future.failed(t) - case _ => - Future.successful(()) - } + .recoverWith { + case t: RejectRequest => + Future.failed(t) + case _ => + Future.successful(()) + } + case _ => + // download the code + get(entityStore, entityName.toDocId, DocRevision.empty, true) + .flatMap { whiskAction => + val currentPermissions = whiskAction.annotations + .get(WhiskAction.permissionsFieldName) + .map(value => value.convertTo[String]) + .getOrElse(WhiskAction.defaultPermissions) + + if (user.namespace.name.asString != entityName.path.root.asString) { // the shared user who download the action + val errorInfo = s"have no permission to download this shared action" + val downloadPermissionOfSharedUser = currentPermissions.charAt(3) + if (downloadPermissionOfSharedUser == '-') { + Future.failed(RejectRequest(Forbidden, Some(ErrorResponse(errorInfo, transid)))) + } else { + Future.successful(()) + } + } else { + // the owner has download permission on any situation + Future.successful(()) + } + } + .recoverWith { + case t: RejectRequest => + Future.failed(t) + case _ => + Future.successful(()) + } } }