Enable Customization of Auth Request Redirect (#1993)

Adds the 'nginx.ingress.kubernetes.io/auth-request-redirect'
annotation, which allows the customization of the
'X-Auth-Request-Redirect' Header. Fixes: #1979

internal/ingress/annotations/authreq/main.go

@@ -36,6 +36,7 @@ type Config struct {
 	SigninURL       string   `json:"signinUrl"`
 	Method          string   `json:"method"`
 	ResponseHeaders []string `json:"responseHeaders,omitEmpty"`
+	RequestRedirect string   `json:"requestRedirect"`
 }
 
 // Equal tests for equality between two Config types
@@ -58,10 +59,6 @@ func (e1 *Config) Equal(e2 *Config) bool {
 	if e1.Method != e2.Method {
 		return false
 	}
-	if e1.Method != e2.Method {
-		return false
-	}
-
 	for _, ep1 := range e1.ResponseHeaders {
 		found := false
 		for _, ep2 := range e2.ResponseHeaders {
@@ -74,6 +71,9 @@ func (e1 *Config) Equal(e2 *Config) bool {
 			return false
 		}
 	}
+	if e1.RequestRedirect != e2.RequestRedirect {
+		return false
+	}
 
 	return true
 }
@@ -112,41 +112,40 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation {
 // ParseAnnotations parses the annotations contained in the ingress
 // rule used to use an Config URL as source for authentication
 func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) {
-	str, err := parser.GetStringAnnotation("auth-url", ing)
+	// Required Parameters
+	urlString, err := parser.GetStringAnnotation("auth-url", ing)
 	if err != nil {
 		return nil, err
 	}
-
-	if str == "" {
+	if urlString == "" {
 		return nil, ing_errors.NewLocationDenied("an empty string is not a valid URL")
 	}
 
-	signin, _ := parser.GetStringAnnotation("auth-signin", ing)
-
-	ur, err := url.Parse(str)
+	authUrl, err := url.Parse(urlString)
 	if err != nil {
 		return nil, err
 	}
-	if ur.Scheme == "" {
+	if authUrl.Scheme == "" {
 		return nil, ing_errors.NewLocationDenied("url scheme is empty")
 	}
-	if ur.Host == "" {
+	if authUrl.Host == "" {
 		return nil, ing_errors.NewLocationDenied("url host is empty")
 	}
-
-	if strings.Contains(ur.Host, "..") {
+	if strings.Contains(authUrl.Host, "..") {
 		return nil, ing_errors.NewLocationDenied("invalid url host")
 	}
 
-	m, _ := parser.GetStringAnnotation("auth-method", ing)
-	if len(m) != 0 && !validMethod(m) {
+	authMethod, _ := parser.GetStringAnnotation("auth-method", ing)
+	if len(authMethod) != 0 && !validMethod(authMethod) {
 		return nil, ing_errors.NewLocationDenied("invalid HTTP method")
 	}
 
-	h := []string{}
+	// Optional Parameters
+	signIn, _ := parser.GetStringAnnotation("auth-signin", ing)
+
+	responseHeaders := []string{}
 	hstr, _ := parser.GetStringAnnotation("auth-response-headers", ing)
 	if len(hstr) != 0 {
-
 		harr := strings.Split(hstr, ",")
 		for _, header := range harr {
 			header = strings.TrimSpace(header)
@@ -154,16 +153,19 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) {
 				if !validHeader(header) {
 					return nil, ing_errors.NewLocationDenied("invalid headers list")
 				}
-				h = append(h, header)
+				responseHeaders = append(responseHeaders, header)
 			}
 		}
 	}
 
+	requestRedirect, _ := parser.GetStringAnnotation("auth-request-redirect", ing)
+
 	return &Config{
-		URL:             str,
-		Host:            ur.Hostname(),
-		SigninURL:       signin,
-		Method:          m,
-		ResponseHeaders: h,
+		URL:             urlString,
+		Host:            authUrl.Hostname(),
+		SigninURL:       signIn,
+		Method:          authMethod,
+		ResponseHeaders: responseHeaders,
+		RequestRedirect: requestRedirect,
 	}, nil
 }

internal/ingress/annotations/authreq/main_test.go

@@ -72,25 +72,28 @@ func TestAnnotations(t *testing.T) {
 	ing.SetAnnotations(data)
 
 	tests := []struct {
-		title     string
-		url       string
-		signinURL string
-		method    string
-		expErr    bool
+		title           string
+		url             string
+		signinURL       string
+		method          string
+		requestRedirect string
+		expErr          bool
 	}{
-		{"empty", "", "", "", true},
-		{"no scheme", "bar", "bar", "", true},
-		{"invalid host", "http://", "http://", "", true},
-		{"invalid host (multiple dots)", "http://foo..bar.com", "http://foo..bar.com", "", true},
-		{"valid URL", "http://bar.foo.com/external-auth", "http://bar.foo.com/external-auth", "", false},
-		{"valid URL - send body", "http://foo.com/external-auth", "http://foo.com/external-auth", "POST", false},
-		{"valid URL - send body", "http://foo.com/external-auth", "http://foo.com/external-auth", "GET", false},
+		{"empty", "", "", "", "", true},
+		{"no scheme", "bar", "bar", "", "", true},
+		{"invalid host", "http://", "http://", "", "", true},
+		{"invalid host (multiple dots)", "http://foo..bar.com", "http://foo..bar.com", "", "", true},
+		{"valid URL", "http://bar.foo.com/external-auth", "http://bar.foo.com/external-auth", "", "", false},
+		{"valid URL - send body", "http://foo.com/external-auth", "http://foo.com/external-auth", "POST", "", false},
+		{"valid URL - send body", "http://foo.com/external-auth", "http://foo.com/external-auth", "GET", "", false},
+		{"valid URL - request redirect", "http://foo.com/external-auth", "http://foo.com/external-auth", "GET", "http://foo.com/redirect-me", false},
 	}
 
 	for _, test := range tests {
 		data[parser.GetAnnotationWithPrefix("auth-url")] = test.url
 		data[parser.GetAnnotationWithPrefix("auth-signin")] = test.signinURL
 		data[parser.GetAnnotationWithPrefix("auth-method")] = fmt.Sprintf("%v", test.method)
+		data[parser.GetAnnotationWithPrefix("auth-request-redirect")] = test.requestRedirect
 
 		i, err := NewParser(&resolver.Mock{}).Parse(ing)
 		if test.expErr {
@@ -112,6 +115,9 @@ func TestAnnotations(t *testing.T) {
 		if u.Method != test.method {
 			t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.method, u.Method)
 		}
+		if u.RequestRedirect != test.requestRedirect {
+			t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.requestRedirect, u.RequestRedirect)
+		}
 	}
 }
 

internal/ingress/controller/template/template.go

@@ -214,7 +214,6 @@ func buildLocation(input interface{}) string {
 	return path
 }
 
-// TODO: Needs Unit Tests
 func buildAuthLocation(input interface{}) string {
 	location, ok := input.(*ingress.Location)
 	if !ok {
@@ -227,7 +226,7 @@ func buildAuthLocation(input interface{}) string {
 	}
 
 	str := base64.URLEncoding.EncodeToString([]byte(location.Path))
-	// avoid locations containing the = char
+	// removes "=" after encoding
 	str = strings.Replace(str, "=", "", -1)
 	return fmt.Sprintf("/_external-auth-%v", str)
 }

internal/ingress/controller/template/template_test.go

@@ -26,6 +26,8 @@ import (
 	"strings"
 	"testing"
 
+	"encoding/base64"
+	"fmt"
 	"k8s.io/ingress-nginx/internal/file"
 	"k8s.io/ingress-nginx/internal/ingress"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/authreq"
@@ -172,6 +174,26 @@ func TestBuildProxyPass(t *testing.T) {
 	}
 }
 
+func TestBuildAuthLocation(t *testing.T) {
+	authURL := "foo.com/auth"
+
+	loc := &ingress.Location{
+		ExternalAuth: authreq.Config{
+			URL: authURL,
+		},
+		Path: "/cat",
+	}
+
+	str := buildAuthLocation(loc)
+
+	encodedAuthURL := strings.Replace(base64.URLEncoding.EncodeToString([]byte(loc.Path)), "=", "", -1)
+	expected := fmt.Sprintf("/_external-auth-%v", encodedAuthURL)
+
+	if str != expected {
+		t.Errorf("Expected \n'%v'\nbut returned \n'%v'", expected, str)
+	}
+}
+
 func TestBuildAuthResponseHeaders(t *testing.T) {
 	loc := &ingress.Location{
 		ExternalAuth: authreq.Config{ResponseHeaders: []string{"h1", "H-With-Caps-And-Dashes"}},