You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/06/08 09:20:20 UTC

[GitHub] [apisix-ingress-controller] tokers commented on a change in pull request #528: chore: add authentication for ApisixRoute

tokers commented on a change in pull request #528:
URL: https://github.com/apache/apisix-ingress-controller/pull/528#discussion_r647265332



##########
File path: pkg/kube/apisix/apis/config/v2alpha1/types.go
##########
@@ -194,6 +195,16 @@ type ApisixRouteHTTPPlugin struct {
 // any plugins.
 type ApisixRouteHTTPPluginConfig map[string]interface{}
 
+type ApisixRouteAuthentication struct {
+	Enable  bool                             `json:"enable" yaml:"enable"`
+	Type    string                           `json:"type" yaml:"type"`
+	KeyAuth ApisixRouteAuthenticationKeyAuth `json:"keyauth,omitempty" yaml:"keyauth,omitempty"`
+}
+
+type ApisixRouteAuthenticationKeyAuth struct {

Review comment:
       Ditto.

##########
File path: pkg/kube/apisix/apis/config/v2alpha1/types.go
##########
@@ -194,6 +195,16 @@ type ApisixRouteHTTPPlugin struct {
 // any plugins.
 type ApisixRouteHTTPPluginConfig map[string]interface{}
 
+type ApisixRouteAuthentication struct {

Review comment:
       Should add some comments to explain the usage of this type.

##########
File path: test/e2e/features/consumer.go
##########
@@ -57,5 +54,146 @@ spec:
 			"username": "foo",
 			"password": "bar",
 		})
+
+		backendSvc, backendPorts := s.DefaultHTTPBackend()
+		ar := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2alpha1
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+ http:
+ - name: rule1
+   match:
+     hosts:
+     - httpbin.org
+     paths:
+       - /ip
+     exprs:
+     - subject:
+         scope: Header
+         name: X-Foo
+       op: Equal
+       value: bar
+   backend:
+     serviceName: %s
+     servicePort: %d
+   authentication:
+     enable: true
+     type: basicAuth
+`, backendSvc, backendPorts[0])
+		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(ar), "creating ApisixRoute with basicAuth")
+		assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixRoutesCreated(1), "Checking number of routes")
+		assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixUpstreamsCreated(1), "Checking number of upstreams")
+
+		_ = s.NewAPISIXClient().GET("/ip").
+			WithHeader("Host", "httpbin.org").
+			WithHeader("X-Foo", "bar").
+			WithHeader("Authorization", "Basic Zm9vOmJhcg==").
+			Expect().
+			Status(http.StatusOK)
+
+		msg := s.NewAPISIXClient().GET("/ip").
+			WithHeader("Host", "httpbin.org").
+			WithHeader("X-Foo", "bar").
+			Expect().
+			Status(http.StatusUnauthorized).
+			Body().
+			Raw()
+		assert.Contains(ginkgo.GinkgoT(), msg, "Missing authorization in request")
+
+		msg = s.NewAPISIXClient().GET("/ip").
+			WithHeader("Host", "httpbin.org").
+			WithHeader("X-Foo", "baz").
+			WithHeader("Authorization", "Basic Zm9vOmJhcg==").
+			Expect().
+			Status(http.StatusNotFound).
+			Body().
+			Raw()
+		assert.Contains(ginkgo.GinkgoT(), msg, "404 Route Not Found")
+	})
+
+	ginkgo.It("ApisixRoute with keyAuth consumer", func() {
+		ac := `
+apiVersion: apisix.apache.org/v2alpha1
+kind: ApisixConsumer
+metadata:
+  name: keyvalue
+spec:
+  authParameter:
+    keyAuth:
+      value:
+        key: foo
+`
+		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(ac), "creating keyAuth ApisixConsumer")
+
+		// Wait until the ApisixConsumer create event was delivered.
+		time.Sleep(6 * time.Second)
+
+		grs, err := s.ListApisixConsumers()
+		assert.Nil(ginkgo.GinkgoT(), err, "listing consumer")
+		assert.Len(ginkgo.GinkgoT(), grs, 1)
+		assert.Len(ginkgo.GinkgoT(), grs[0].Plugins, 1)
+		basicAuth, _ := grs[0].Plugins["key-auth"]
+		assert.Equal(ginkgo.GinkgoT(), basicAuth, map[string]interface{}{
+			"key": "foo",
+		})
+
+		backendSvc, backendPorts := s.DefaultHTTPBackend()
+		ar := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2alpha1
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+ http:
+ - name: rule1
+   match:
+     hosts:
+     - httpbin.org
+     paths:
+       - /ip
+     exprs:
+     - subject:
+         scope: Header
+         name: X-Foo
+       op: Equal
+       value: bar
+   backend:
+     serviceName: %s
+     servicePort: %d
+   authentication:
+     enable: true
+     type: keyAuth
+`, backendSvc, backendPorts[0])
+		assert.Nil(ginkgo.GinkgoT(), s.CreateResourceFromString(ar), "creating ApisixRoute with keyAuth")
+		assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixRoutesCreated(1), "Checking number of routes")
+		assert.Nil(ginkgo.GinkgoT(), s.EnsureNumApisixUpstreamsCreated(1), "Checking number of upstreams")
+
+		_ = s.NewAPISIXClient().GET("/ip").
+			WithHeader("Host", "httpbin.org").
+			WithHeader("X-Foo", "bar").
+			WithHeader("apikey", "foo").
+			Expect().
+			Status(http.StatusOK)
+
+		msg := s.NewAPISIXClient().GET("/ip").
+			WithHeader("Host", "httpbin.org").
+			WithHeader("X-Foo", "bar").
+			Expect().
+			Status(http.StatusUnauthorized).
+			Body().
+			Raw()
+		assert.Contains(ginkgo.GinkgoT(), msg, "Missing API key found in request")
+
+		msg = s.NewAPISIXClient().GET("/ip").
+			WithHeader("Host", "httpbin.org").
+			WithHeader("X-Foo", "baz").
+			WithHeader("apikey", "baz").
+			Expect().
+			Status(http.StatusNotFound).
+			Body().
+			Raw()
+		assert.Contains(ginkgo.GinkgoT(), msg, "404 Route Not Found")

Review comment:
       Should also add test cases for:
   
   1. Use secret to fetch basic auth and key auth parameters.
   2. Test with authentication `false`




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