fix: deny locations with invalid auth-url annotation (#8256)

* fix: deny locations with invalid auth-url annotation

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>

* Delete duplicate test

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>

internal/ingress/annotations/annotations_test.go

@@ -294,48 +294,3 @@ func TestCustomHTTPErrors(t *testing.T) {
 		}
 	}
 }
-
-/*
-func TestMergeLocationAnnotations(t *testing.T) {
-	// initial parameters
-	keys := []string{"BasicDigestAuth", "CorsConfig", "ExternalAuth", "RateLimit", "Redirect", "Rewrite", "Whitelist", "Proxy", "UsePortInRedirects"}
-
-	loc := ingress.Location{}
-	annotations := &Ingress{
-		BasicDigestAuth:    &auth.Config{},
-		CorsConfig:         &cors.Config{},
-		ExternalAuth:       &authreq.Config{},
-		RateLimit:          &ratelimit.Config{},
-		Redirect:           &redirect.Config{},
-		Rewrite:            &rewrite.Config{},
-		Whitelist:          &ipwhitelist.SourceRange{},
-		Proxy:              &proxy.Config{},
-		UsePortInRedirects: true,
-	}
-
-	// create test table
-	type fooMergeLocationAnnotationsStruct struct {
-		fName string
-		er    interface{}
-	}
-	fooTests := []fooMergeLocationAnnotationsStruct{}
-	for name, value := range keys {
-		fva := fooMergeLocationAnnotationsStruct{name, value}
-		fooTests = append(fooTests, fva)
-	}
-
-	// execute test
-	MergeWithLocation(&loc, annotations)
-
-	// check result
-	for _, foo := range fooTests {
-		fv := reflect.ValueOf(loc).FieldByName(foo.fName).Interface()
-		if !reflect.DeepEqual(fv, foo.er) {
-			t.Errorf("Returned %v but expected %v for the field %s", fv, foo.er, foo.fName)
-		}
-	}
-	if _, ok := annotations[DeniedKeyName]; ok {
-		t.Errorf("%s should be removed after mergeLocationAnnotations", DeniedKeyName)
-	}
-}
-*/

internal/ingress/annotations/auth/main_test.go

@@ -17,13 +17,12 @@ limitations under the License.
 package auth
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"testing"
 	"time"
 
-	"errors"
-
 	api "k8s.io/api/core/v1"
 	networking "k8s.io/api/networking/v1"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"

internal/ingress/annotations/authreq/main.go

@@ -164,7 +164,7 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
 
 	authURL, err := parser.StringToURL(urlString)
 	if err != nil {
-		return nil, ing_errors.InvalidContent{Name: err.Error()}
+		return nil, ing_errors.LocationDenied{Reason: fmt.Errorf("could not parse auth-url annotation: %v", err)}
 	}
 
 	authMethod, _ := parser.GetStringAnnotation("auth-method", ing)

test/e2e/annotations/auth.go

@@ -692,6 +692,39 @@ http {
 				Header("Location").Equal(fmt.Sprintf("http://%s/auth/start?rd=http://%s%s", thisHost, thisHost, url.QueryEscape("/?a=b&c=d")))
 		})
 	})
+
+	ginkgo.Context("with invalid auth-url should deny whole location", func() {
+		host := "auth"
+		var annotations map[string]string
+		var ing *networking.Ingress
+
+		ginkgo.BeforeEach(func() {
+			annotations = map[string]string{
+				"nginx.ingress.kubernetes.io/auth-url": "https://invalid..auth.url",
+			}
+
+			ing = framework.NewSingleIngress(host, "/denied-auth", host, f.Namespace, framework.EchoService, 80, annotations)
+			f.EnsureIngress(ing)
+
+			f.WaitForNginxServer(host, func(server string) bool {
+				return strings.Contains(server, "server_name auth")
+			})
+		})
+
+		ginkgo.It("should return 503 (location was denied)", func() {
+			f.HTTPTestClient().
+				GET("/denied-auth").
+				WithHeader("Host", host).
+				Expect().
+				Status(http.StatusServiceUnavailable)
+		})
+
+		ginkgo.It("should add error to the config", func() {
+			f.WaitForNginxServer(host, func(server string) bool {
+				return strings.Contains(server, "could not parse auth-url annotation: invalid url host")
+			})
+		})
+	})
 })
 
 // TODO: test Digest Auth