[GitHub] [openwhisk] ddragosd commented on a change in pull request #4882: Allow OPTIONS response on web actions before checking for authentication.

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

ddragosd commented on a change in pull request #4882: Allow OPTIONS response on web actions before checking for authentication.
URL: https://github.com/apache/openwhisk/pull/4882#discussion_r408572359
 
 

 ##########
 File path: tests/src/test/scala/org/apache/openwhisk/core/controller/test/WebActionsApiTests.scala
 ##########
 @@ -393,53 +391,97 @@ trait WebActionsApiBaseTests extends ControllerTestCommon with BeforeAndAfterEac
         }
     }
 
-    it should s"reject requests when authentication is required but none given (auth? ${creds.isDefined})" in {
+    it should s"reject requests when whisk authentication is required but none given (auth? ${creds.isDefined})" in {
       implicit val tid = transid()
 
+      val entityName = MakeName.next("export")
+      val action =
+        stubAction(
+          proxyNamespace,
+          entityName,
+          customOptions = false,
+          requireAuthentication = true,
+          requireAuthenticationAsBoolean = true)
+      val path = action.fullyQualifiedName(false)
+      put(entityStore, action)
+
       allowedMethods.foreach { m =>
-        Seq(true, false).foreach { useReqWhiskAuthBool =>
-          requireAuthenticationAsBoolean = useReqWhiskAuthBool
+        m(s"$testRoutePath/${path}.json") ~> Route.seal(routes(creds)) ~> check {
+          if (m === Options) {
+            status should be(OK) // options response is always present regardless of auth
+            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"
+            header("Access-Control-Request-Headers") shouldBe empty
+          } else if (creds.isEmpty) {
+            status should be(Unauthorized) // if user is not authenticated, reject all requests
+          } else {
+            invocationsAllowed += 1
+            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"))
+            response
+              .fields("content")
+              .asJsObject
+              .fields(webApiDirectives.namespace) shouldBe creds.get.namespace.name.toJson
+          }
         }
+      }
+    }
 
-        val entityName = MakeName.next("export")
-        val action = stubAction(
+    it should s"reject requests when x-authentication is required but none given (auth? ${creds.isDefined})" in {
+      implicit val tid = transid()
+
+      val entityName = MakeName.next("export")
+      val action =
+        stubAction(
           proxyNamespace,
           entityName,
+          customOptions = false,
           requireAuthentication = true,
-          requireAuthenticationAsBoolean = requireAuthenticationAsBoolean)
-        val path = action.fullyQualifiedName(false)
+          requireAuthenticationAsBoolean = false)
+      val path = action.fullyQualifiedName(false)
+      put(entityStore, action)
 
-        put(entityStore, action)
+      allowedMethods.foreach { m =>
+        // web action require-whisk-auth is set, but the header X-Require-Whisk-Auth value does not match
+        m(s"$testRoutePath/${path}.json") ~> addHeader(
+          WhiskAction.requireWhiskAuthHeader,
+          requireAuthenticationKey + "-bad") ~> Route
+          .seal(routes(creds)) ~> check {
+          if (m == Options) {
+            status should be(OK) // options should always respond
+            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"
+            header("Access-Control-Request-Headers") shouldBe empty
+          } else {
+            status should be(Unauthorized)
+          }
+        }
 
-        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" -> 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 is not set
+        m(s"$testRoutePath/${path}.json") ~> Route.seal(routes(creds)) ~> check {
+          if (m == Options) {
+            status should be(OK) // options should always respond
+            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"
+            header("Access-Control-Request-Headers") shouldBe empty
 
 Review comment:
   👍 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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