Improve the external authorization concept from opt-in to secure-by-default

docs/user-guide/nginx-configuration/annotations.md

@@ -27,6 +27,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
 |[nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream](#client-certificate-authentication)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/auth-url](#external-authentication)|string|
 |[nginx.ingress.kubernetes.io/auth-snippet](#external-authentication)|string|
+|[nginx.ingress.kubernetes.io/enable-global-auth](#external-authentication)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/backend-protocol](#backend-protocol)|string|HTTP,HTTPS,GRPC,GRPCS,AJP|
 |[nginx.ingress.kubernetes.io/canary](#canary)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/canary-by-header](#canary)|string|
@@ -389,6 +390,14 @@ nginx.ingress.kubernetes.io/auth-snippet: |
 !!! example
     Please check the [external-auth](../../examples/auth/external-auth/README.md) example.
 
+#### Global External Authentication
+
+By default the controller redirects all requests to an existing service that provides authentication if `global-auth-url` is set in the NGINX ConfigMap. If you want to disable this behavior for that ingress, you can use ssl-redirect: "false" in the NGINX ConfigMap.
+`nginx.ingress.kubernetes.io/enable-global-auth`:
+   indicates if GlobalExternalAuth configuration should be applied or not to this Ingress rule. Default values is set to `"true"`.
+
+!!! note For more information please see [global-auth-url](./configmap.md#global-auth-url).
+
 ### Rate limiting
 
 These annotations define a limit on the connections that can be opened by a single client IP address.

docs/user-guide/nginx-configuration/configmap.md

@@ -152,6 +152,12 @@ The following table shows a configuration option's name, type, and the default v
 |[limit-req-status-code](#limit-req-status-code)|int|503|
 |[limit-conn-status-code](#limit-conn-status-code)|int|503|
 |[no-tls-redirect-locations](#no-tls-redirect-locations)|string|"/.well-known/acme-challenge"|
+|[global-auth-url](#global-auth-url)|string|""|
+|[global-auth-method](#global-auth-method)|string|""|
+|[global-auth-signin](#global-auth-signin)|string|""|
+|[global-auth-response-headers](#global-auth-response-headers)|string|""|
+|[global-auth-request-redirect](#global-auth-request-redirect)|string|""|
+|[global-auth-snippet](#global-auth-snippet)|string|""|
 |[no-auth-locations](#no-auth-locations)|string|"/.well-known/acme-challenge"|
 |[block-cidrs](#block-cidrs)|[]string|""|
 |[block-user-agents](#block-user-agents)|[]string|""|
@@ -864,6 +870,45 @@ Sets the [status code to return in response to rejected connections](http://ngin
 A comma-separated list of locations on which http requests will never get redirected to their https counterpart.
 _**default:**_ "/.well-known/acme-challenge"
 
+## global-auth-url
+
+A url to an existing service that provides authentication for all the locations.
+Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-url`.
+Locations that should not get authenticated can be listed using `no-auth-locations` See [no-auth-locations](#no-auth-locations). In addition, each service can be excluded from authentication via annotation `enable-global-auth` set to "false".
+_**default:**_ ""
+
+_References:_ [https://github.com/kubernetes/ingress-nginx/blob/master/docs/user-guide/nginx-configuration/annotations.md#external-authentication](https://github.com/kubernetes/ingress-nginx/blob/master/docs/user-guide/nginx-configuration/annotations.md#external-authentication)
+
+## global-auth-method
+
+A HTTP method to use for an existing service that provides authentication for all the locations.
+Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-method`.
+_**default:**_ ""
+
+## global-auth-signin
+
+Sets the location of the error page for an existing service that provides authentication for all the locations.
+Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-signin`.
+_**default:**_ ""
+
+## global-auth-response-headers
+
+Sets the headers to pass to backend once authentication request completes. Applied to all the locations.
+Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-response-headers`.
+_**default:**_ ""
+
+## global-auth-request-redirect
+
+Sets the X-Auth-Request-Redirect header value. Applied to all the locations.
+Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-request-redirect`.
+_**default:**_ ""
+
+## global-auth-snippet
+
+Sets a custom snippet to use with external authentication. Applied to all the locations.
+Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-request-redirect`.
+_**default:**_ ""
+
 ## no-auth-locations
 
 A comma-separated list of locations that should not get authenticated.

internal/ingress/annotations/annotations.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/ingress-nginx/internal/ingress/annotations/alias"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/auth"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/authreq"
+	"k8s.io/ingress-nginx/internal/ingress/annotations/authreqglobal"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/authtls"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/backendprotocol"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/clientbodybuffersize"
@@ -83,6 +84,7 @@ type Ingress struct {
 	//TODO: Change this back into an error when https://github.com/imdario/mergo/issues/100 is resolved
 	Denied             *string
 	ExternalAuth       authreq.Config
+	EnableGlobalAuth   bool
 	HTTP2PushPreload   bool
 	Proxy              proxy.Config
 	RateLimit          ratelimit.Config
@@ -127,6 +129,7 @@ func NewAnnotationExtractor(cfg resolver.Resolver) Extractor {
 			"CustomHTTPErrors":     customhttperrors.NewParser(cfg),
 			"DefaultBackend":       defaultbackend.NewParser(cfg),
 			"ExternalAuth":         authreq.NewParser(cfg),
+			"EnableGlobalAuth":     authreqglobal.NewParser(cfg),
 			"HTTP2PushPreload":     http2pushpreload.NewParser(cfg),
 			"Proxy":                proxy.NewParser(cfg),
 			"RateLimit":            ratelimit.NewParser(cfg),

internal/ingress/annotations/authreq/main.go

@@ -17,6 +17,7 @@ limitations under the License.
 package authreq
 
 import (
+	"fmt"
 	"net/url"
 	"regexp"
 	"strings"
@@ -84,7 +85,8 @@ var (
 	headerRegexp = regexp.MustCompile(`^[a-zA-Z\d\-_]+$`)
 )
 
-func validMethod(method string) bool {
+// ValidMethod checks is the provided string a valid HTTP method
+func ValidMethod(method string) bool {
 	if len(method) == 0 {
 		return false
 	}
@@ -97,7 +99,8 @@ func validMethod(method string) bool {
 	return false
 }
 
-func validHeader(header string) bool {
+// ValidHeader checks is the provided string satisfies the header's name regex
+func ValidHeader(header string) bool {
 	return headerRegexp.Match([]byte(header))
 }
 
@@ -119,22 +122,13 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) {
 		return nil, err
 	}
 
-	authURL, err := url.Parse(urlString)
-	if err != nil {
-		return nil, err
-	}
-	if authURL.Scheme == "" {
-		return nil, ing_errors.NewLocationDenied("url scheme is empty")
-	}
-	if authURL.Host == "" {
-		return nil, ing_errors.NewLocationDenied("url host is empty")
-	}
-	if strings.Contains(authURL.Host, "..") {
-		return nil, ing_errors.NewLocationDenied("invalid url host")
+	authURL, message := ParseStringToURL(urlString)
+	if authURL == nil {
+		return nil, ing_errors.NewLocationDenied(message)
 	}
 
 	authMethod, _ := parser.GetStringAnnotation("auth-method", ing)
-	if len(authMethod) != 0 && !validMethod(authMethod) {
+	if len(authMethod) != 0 && !ValidMethod(authMethod) {
 		return nil, ing_errors.NewLocationDenied("invalid HTTP method")
 	}
 
@@ -156,7 +150,7 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) {
 		for _, header := range harr {
 			header = strings.TrimSpace(header)
 			if len(header) > 0 {
-				if !validHeader(header) {
+				if !ValidHeader(header) {
 					return nil, ing_errors.NewLocationDenied("invalid headers list")
 				}
 				responseHeaders = append(responseHeaders, header)
@@ -176,3 +170,22 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) {
 		AuthSnippet:     authSnippet,
 	}, nil
 }
+
+// ParseStringToURL parses the provided string into URL and returns error
+// message in case of failure
+func ParseStringToURL(input string) (*url.URL, string) {
+
+	parsedURL, err := url.Parse(input)
+	if err != nil {
+		return nil, fmt.Sprintf("%v is not a valid URL: %v", input, err)
+	}
+	if parsedURL.Scheme == "" {
+		return nil, "url scheme is empty."
+	} else if parsedURL.Host == "" {
+		return nil, "url host is empty."
+	} else if strings.Contains(parsedURL.Host, "..") {
+		return nil, "invalid url host."
+	}
+	return parsedURL, ""
+
+}

internal/ingress/annotations/authreq/main_test.go

@@ -18,6 +18,7 @@ package authreq
 
 import (
 	"fmt"
+	"net/url"
 	"reflect"
 	"testing"
 
@@ -178,3 +179,38 @@ func TestHeaderAnnotations(t *testing.T) {
 		}
 	}
 }
+
+func TestParseStringToURL(t *testing.T) {
+	validURL := "http://bar.foo.com/external-auth"
+	validParsedURL, _ := url.Parse(validURL)
+
+	tests := []struct {
+		title   string
+		url     string
+		message string
+		parsed  *url.URL
+		expErr  bool
+	}{
+		{"empty", "", "url scheme is empty.", nil, true},
+		{"no scheme", "bar", "url scheme is empty.", nil, true},
+		{"invalid host", "http://", "url host is empty.", nil, true},
+		{"invalid host (multiple dots)", "http://foo..bar.com", "invalid url host.", nil, true},
+		{"valid URL", validURL, "", validParsedURL, false},
+	}
+
+	for _, test := range tests {
+
+		i, err := ParseStringToURL(test.url)
+		if test.expErr {
+			if err != test.message {
+				t.Errorf("%v: expected error \"%v\" but \"%v\" was returned", test.title, test.message, err)
+			}
+			continue
+		}
+
+		if i.String() != test.parsed.String() {
+			t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.parsed, i)
+		}
+	}
+
+}

internal/ingress/annotations/authreqglobal/main.go

@@ -0,0 +1,45 @@
+/*
+Copyright 2015 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package authreqglobal
+
+import (
+	extensions "k8s.io/api/extensions/v1beta1"
+
+	"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
+	"k8s.io/ingress-nginx/internal/ingress/resolver"
+)
+
+type authReqGlobal struct {
+	r resolver.Resolver
+}
+
+// NewParser creates a new authentication request annotation parser
+func NewParser(r resolver.Resolver) parser.IngressAnnotation {
+	return authReqGlobal{r}
+}
+
+// ParseAnnotations parses the annotations contained in the ingress
+// rule used to enable or disable global external authentication
+func (a authReqGlobal) Parse(ing *extensions.Ingress) (interface{}, error) {
+
+	enableGlobalAuth, err := parser.GetBoolAnnotation("enable-global-auth", ing)
+	if err != nil {
+		enableGlobalAuth = true
+	}
+
+	return enableGlobalAuth, nil
+}

internal/ingress/annotations/authreqglobal/main_test.go

@@ -0,0 +1,82 @@
+/*
+Copyright 2015 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package authreqglobal
+
+import (
+	"testing"
+
+	api "k8s.io/api/core/v1"
+	extensions "k8s.io/api/extensions/v1beta1"
+	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
+	"k8s.io/ingress-nginx/internal/ingress/resolver"
+
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+func buildIngress() *extensions.Ingress {
+	defaultBackend := extensions.IngressBackend{
+		ServiceName: "default-backend",
+		ServicePort: intstr.FromInt(80),
+	}
+
+	return &extensions.Ingress{
+		ObjectMeta: meta_v1.ObjectMeta{
+			Name:      "foo",
+			Namespace: api.NamespaceDefault,
+		},
+		Spec: extensions.IngressSpec{
+			Backend: &extensions.IngressBackend{
+				ServiceName: "default-backend",
+				ServicePort: intstr.FromInt(80),
+			},
+			Rules: []extensions.IngressRule{
+				{
+					Host: "foo.bar.com",
+					IngressRuleValue: extensions.IngressRuleValue{
+						HTTP: &extensions.HTTPIngressRuleValue{
+							Paths: []extensions.HTTPIngressPath{
+								{
+									Path:    "/foo",
+									Backend: defaultBackend,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+func TestAnnotation(t *testing.T) {
+	ing := buildIngress()
+
+	data := map[string]string{}
+	data[parser.GetAnnotationWithPrefix("auth-url")] = "http://foo.com/external-auth"
+	data[parser.GetAnnotationWithPrefix("enable-global-auth")] = "false"
+	ing.SetAnnotations(data)
+
+	i, _ := NewParser(&resolver.Mock{}).Parse(ing)
+	u, ok := i.(bool)
+	if !ok {
+		t.Errorf("expected a Config type")
+	}
+	if u {
+		t.Errorf("Expected false but returned true")
+	}
+}