[GitHub] upgle commented on a change in pull request #3880: Modify that web action in the bound package can be accessed.

[GitBox <gi...@apache.org>]

upgle commented on a change in pull request #3880: Modify that web action in the bound package can be accessed.
URL: https://github.com/apache/incubator-openwhisk/pull/3880#discussion_r208104271
 
 

 ##########
 File path: core/controller/src/main/scala/whisk/core/controller/WebActions.scala
 ##########
 @@ -552,8 +552,11 @@ trait WhiskWebActionsApi extends Directives with ValidateRequestSize with PostAc
         if (a.namespace.defaultPackage) {
           Future.successful(a)
         } else {
-          pkgLookup(a.namespace.toFullyQualifiedEntityName) map { pkg =>
-            (a.inherit(pkg.parameters))
+          // if action is not in the default package, then check entitlement
+          checkEntitlement(actionOwnerIdentity, a) flatMap { _ =>
+            pkgLookup(a.namespace.toFullyQualifiedEntityName) map { pkg =>
+              (a.inherit(pkg.parameters))
+            }
 
 Review comment:
   @rabbah 
   yes, I agree that it looks complicated. so I've changed some codes. (removed `pkgLookup` method which is not necessary anymore)
   
   1. action in a default package no need to resolve, just check entitlement and if it isexported.
   2. action in a bound package(new) is handled by `resolveActionAndMergeParameters()`, it would resolve action and merge parameters. 
   2. check whether action is in a proper package -> It check whether an action is exported by `confirmExportedAction()` method
   3. and all types of action is checked for entitlement and throttle by `checkEntitlement()` method
   
   please check changed code here
   
   ```scala
   private def verifyWebAction(actionName: FullyQualifiedEntityName, authenticated: Boolean)(
       implicit transid: TransactionId) = {
   
       // 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)}
         }
       }
     }
   ```
   
   https://github.com/apache/incubator-openwhisk/pull/3880/files#diff-a356e6b010b8aeb5ea7b6f7fec395038R535

----------------------------------------------------------------
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