Fix forwarding of auth-response-headers to gRPC backends (kubernetes#…

…7331)

* add e2e test for auth-response-headers annotation

* add e2e test for grpc with auth-response-headers

* fix forwarding of auth header to GRPC backends

* add test case for proxySetHeader(nil)

internal/ingress/controller/template/template.go

@@ -563,7 +563,7 @@ func shouldApplyGlobalAuth(input interface{}, globalExternalAuthURL string) bool
 	return false
 }
 
-func buildAuthResponseHeaders(headers []string) []string {
+func buildAuthResponseHeaders(proxySetHeader string, headers []string) []string {
 	res := []string{}
 
 	if len(headers) == 0 {
@@ -574,7 +574,7 @@ func buildAuthResponseHeaders(headers []string) []string {
 		hvar := strings.ToLower(h)
 		hvar = strings.NewReplacer("-", "_").Replace(hvar)
 		res = append(res, fmt.Sprintf("auth_request_set $authHeader%v $upstream_http_%v;", i, hvar))
-		res = append(res, fmt.Sprintf("proxy_set_header '%v' $authHeader%v;", h, i))
+		res = append(res, fmt.Sprintf("%s '%v' $authHeader%v;", proxySetHeader, h, i))
 	}
 	return res
 }
@@ -668,7 +668,7 @@ func buildProxyPass(host string, b interface{}, loc interface{}) string {
 		var xForwardedPrefix string
 
 		if len(location.XForwardedPrefix) > 0 {
-			xForwardedPrefix = fmt.Sprintf("proxy_set_header X-Forwarded-Prefix \"%s\";\n", location.XForwardedPrefix)
+			xForwardedPrefix = fmt.Sprintf("%s X-Forwarded-Prefix \"%s\";\n", proxySetHeader(location), location.XForwardedPrefix)
 		}
 
 		return fmt.Sprintf(`

internal/ingress/controller/template/template_test.go

@@ -506,7 +506,7 @@ func TestBuildAuthResponseHeaders(t *testing.T) {
 		"proxy_set_header 'H-With-Caps-And-Dashes' $authHeader1;",
 	}
 
-	headers := buildAuthResponseHeaders(externalAuthResponseHeaders)
+	headers := buildAuthResponseHeaders(proxySetHeader(nil), externalAuthResponseHeaders)
 
 	if !reflect.DeepEqual(expected, headers) {
 		t.Errorf("Expected \n'%v'\nbut returned \n'%v'", expected, headers)
@@ -1182,23 +1182,40 @@ func TestBuildCustomErrorLocationsPerServer(t *testing.T) {
 }
 
 func TestProxySetHeader(t *testing.T) {
-	invalidType := &ingress.Ingress{}
-	expected := "proxy_set_header"
-	actual := proxySetHeader(invalidType)
-
-	if expected != actual {
-		t.Errorf("Expected '%v' but returned '%v'", expected, actual)
-	}
-
-	grpcBackend := &ingress.Location{
-		BackendProtocol: "GRPC",
+	tests := []struct {
+		name     string
+		loc      interface{}
+		expected string
+	}{
+		{
+			name:     "nil",
+			loc:      nil,
+			expected: "proxy_set_header",
+		},
+		{
+			name:     "invalid type",
+			loc:      &ingress.Ingress{},
+			expected: "proxy_set_header",
+		},
+		{
+			name:     "http backend",
+			loc:      &ingress.Location{},
+			expected: "proxy_set_header",
+		},
+		{
+			name: "gRPC backend",
+			loc: &ingress.Location{
+				BackendProtocol: "GRPC",
+			},
+			expected: "grpc_set_header",
+		},
 	}
-
-	expected = "grpc_set_header"
-	actual = proxySetHeader(grpcBackend)
-
-	if expected != actual {
-		t.Errorf("Expected '%v' but returned '%v'", expected, actual)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := proxySetHeader(tt.loc); got != tt.expected {
+				t.Errorf("proxySetHeader() = %v, expected %v", got, tt.expected)
+			}
+		})
 	}
 }
 

rootfs/etc/nginx/template/nginx.tmpl

@@ -1178,7 +1178,7 @@ stream {
             auth_request        {{ $authPath }};
             auth_request_set    $auth_cookie $upstream_http_set_cookie;
             add_header          Set-Cookie $auth_cookie;
-            {{- range $line := buildAuthResponseHeaders $externalAuth.ResponseHeaders }}
+            {{- range $line := buildAuthResponseHeaders $proxySetHeader $externalAuth.ResponseHeaders }}
             {{ $line }}
             {{- end }}
             {{ end }}
@@ -1196,7 +1196,7 @@ stream {
             auth_digest {{ $location.BasicDigestAuth.Realm | quote }};
             auth_digest_user_file {{ $location.BasicDigestAuth.File }};
             {{ end }}
-            proxy_set_header Authorization "";
+            {{ $proxySetHeader }} Authorization "";
             {{ end }}
             {{ end }}
 

test/e2e/annotations/auth.go

@@ -453,6 +453,29 @@ http {
 				Expect().
 				Status(http.StatusOK)
 		})
+
+		ginkgo.It("should overwrite Foo header with auth response", func() {
+			var (
+				rewriteHeader = "Foo"
+				rewriteVal    = "bar"
+			)
+			annotations["nginx.ingress.kubernetes.io/auth-response-headers"] = rewriteHeader
+			f.UpdateIngress(ing)
+
+			f.WaitForNginxServer(host, func(server string) bool {
+				return strings.Contains(server, fmt.Sprintf("proxy_set_header '%s' $authHeader0;", rewriteHeader))
+			})
+
+			f.HTTPTestClient().
+				GET("/").
+				WithHeader("Host", host).
+				WithHeader(rewriteHeader, rewriteVal).
+				WithBasicAuth("user", "password").
+				Expect().
+				Status(http.StatusOK).
+				Body().
+				NotContainsFold(fmt.Sprintf("%s=%s", rewriteHeader, rewriteVal))
+		})
 	})
 
 	ginkgo.Context("when external authentication is configured with a custom redirect param", func() {

test/e2e/annotations/grpc.go

@@ -27,6 +27,7 @@ import (
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
+	"google.golang.org/grpc/metadata"
 	core "k8s.io/api/core/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -121,6 +122,79 @@ var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() {
 		assert.Equal(ginkgo.GinkgoT(), metadata["content-type"].Values[0], "application/grpc")
 	})
 
+	ginkgo.It("authorization metadata should be overwritten by external auth response headers", func() {
+		f.NewGRPCBinDeployment()
+		f.NewHttpbinDeployment()
+
+		host := "echo"
+
+		svc := &core.Service{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "grpcbin-test",
+				Namespace: f.Namespace,
+			},
+			Spec: corev1.ServiceSpec{
+				ExternalName: fmt.Sprintf("grpcbin.%v.svc.cluster.local", f.Namespace),
+				Type:         corev1.ServiceTypeExternalName,
+				Ports: []corev1.ServicePort{
+					{
+						Name:       host,
+						Port:       9000,
+						TargetPort: intstr.FromInt(9000),
+						Protocol:   "TCP",
+					},
+				},
+			},
+		}
+		f.EnsureService(svc)
+
+		err := framework.WaitForEndpoints(f.KubeClientSet, framework.DefaultTimeout, framework.HTTPBinService, f.Namespace, 1)
+		assert.Nil(ginkgo.GinkgoT(), err)
+
+		e, err := f.KubeClientSet.CoreV1().Endpoints(f.Namespace).Get(context.TODO(), framework.HTTPBinService, metav1.GetOptions{})
+		assert.Nil(ginkgo.GinkgoT(), err)
+
+		assert.GreaterOrEqual(ginkgo.GinkgoT(), len(e.Subsets), 1, "expected at least one endpoint")
+		assert.GreaterOrEqual(ginkgo.GinkgoT(), len(e.Subsets[0].Addresses), 1, "expected at least one address ready in the endpoint")
+
+		httpbinIP := e.Subsets[0].Addresses[0].IP
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/auth-url":              fmt.Sprintf("http://%s/response-headers?authorization=foo", httpbinIP),
+			"nginx.ingress.kubernetes.io/auth-response-headers": "Authorization",
+			"nginx.ingress.kubernetes.io/backend-protocol":      "GRPC",
+		}
+
+		ing := framework.NewSingleIngressWithTLS(host, "/", host, []string{host}, f.Namespace, "grpcbin-test", 9000, annotations)
+
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return strings.Contains(server, "grpc_pass grpc://upstream_balancer;")
+			})
+
+		conn, _ := grpc.Dial(f.GetNginxIP()+":443",
+			grpc.WithTransportCredentials(
+				credentials.NewTLS(&tls.Config{
+					ServerName:         "echo",
+					InsecureSkipVerify: true,
+				}),
+			),
+		)
+		defer conn.Close()
+
+		client := pb.NewGRPCBinClient(conn)
+		ctx := metadata.AppendToOutgoingContext(context.Background(),
+			"authorization", "bar")
+
+		res, err := client.HeadersUnary(ctx, &pb.EmptyMessage{})
+		assert.Nil(ginkgo.GinkgoT(), err)
+
+		metadata := res.GetMetadata()
+		assert.Equal(ginkgo.GinkgoT(), "foo", metadata["authorization"].Values[0])
+	})
+
 	ginkgo.It("should return OK for service with backend protocol GRPCS", func() {
 		f.NewGRPCBinDeployment()