From b9d0878ed29379ad8954ca2f21362d39eb8cc401 Mon Sep 17 00:00:00 2001 From: Cole Mickens Date: Sun, 29 Jan 2017 13:52:50 -0800 Subject: [PATCH] add signin url req support --- controllers/nginx/Makefile | 9 ++-- .../rootfs/etc/nginx/template/nginx.tmpl | 8 ++++ core/pkg/ingress/annotations/authreq/main.go | 42 +++++++------------ .../ingress/annotations/authreq/main_test.go | 29 +++++++------ core/pkg/ingress/annotations/parser/main.go | 35 ++++++++++++++++ 5 files changed, 79 insertions(+), 44 deletions(-) diff --git a/controllers/nginx/Makefile b/controllers/nginx/Makefile index 65eb4af98..ebcce0c44 100644 --- a/controllers/nginx/Makefile +++ b/controllers/nginx/Makefile @@ -6,6 +6,7 @@ BUILDTAGS= RELEASE?=0.9.0-beta.1 PREFIX?=gcr.io/google_containers/nginx-ingress-controller GOOS?=linux +DOCKER?=gcloud docker REPO_INFO=$(shell git config --get remote.origin.url) @@ -20,11 +21,11 @@ build: clean -ldflags "-s -w -X ${PKG}/pkg/version.RELEASE=${RELEASE} -X ${PKG}/pkg/version.COMMIT=${COMMIT} -X ${PKG}/pkg/version.REPO=${REPO_INFO}" \ -o rootfs/nginx-ingress-controller ${PKG}/pkg/cmd/controller -container: build - docker build -t $(PREFIX):$(RELEASE) rootfs +container: + $(DOCKER) build -t $(PREFIX):$(RELEASE) rootfs -push: container - gcloud docker push $(PREFIX):$(RELEASE) +push: + $(DOCKER) push $(PREFIX):$(RELEASE) fmt: @echo "+ $@" diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index 2c4d3cc2e..72526dfd9 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl @@ -235,6 +235,8 @@ http { {{ end }} {{ if not (empty $location.ExternalAuth.Method) }} proxy_method {{ $location.ExternalAuth.Method }}; + proxy_set_header X-Original-URI $request_uri; + proxy_set_header X-Scheme $scheme; {{ end }} proxy_set_header Host $host; proxy_pass_request_headers on; @@ -259,6 +261,10 @@ http { # this location requires authentication auth_request {{ $authPath }}; {{ end }} + + {{ if not (empty $location.ExternalAuth.SigninURL) }} + error_page 401 = {{ $location.ExternalAuth.SigninURL }}; + {{ end }} {{ if (and (not (empty $server.SSLCertificate)) $location.Redirect.SSLRedirect) }} # enforce ssl on server side @@ -301,6 +307,8 @@ http { proxy_set_header X-Forwarded-Host $host; proxy_set_header X-Forwarded-Port $pass_port; proxy_set_header X-Forwarded-Proto $pass_access_scheme; + proxy_set_header X-Original-URI $request_uri; + proxy_set_header X-Scheme $scheme; # mitigate HTTPoxy Vulnerability # https://www.nginx.com/blog/mitigating-the-httpoxy-vulnerability-with-nginx/ diff --git a/core/pkg/ingress/annotations/authreq/main.go b/core/pkg/ingress/annotations/authreq/main.go index 560a73868..a784e3871 100644 --- a/core/pkg/ingress/annotations/authreq/main.go +++ b/core/pkg/ingress/annotations/authreq/main.go @@ -17,9 +17,6 @@ limitations under the License. package authreq import ( - "net/url" - "strings" - "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/ingress/core/pkg/ingress/annotations/parser" @@ -28,16 +25,18 @@ import ( const ( // external URL that provides the authentication - authURL = "ingress.kubernetes.io/auth-url" - authMethod = "ingress.kubernetes.io/auth-method" - authBody = "ingress.kubernetes.io/auth-send-body" + authURL = "ingress.kubernetes.io/auth-url" + authSigninURL = "ingress.kubernetes.io/auth-signin" + authMethod = "ingress.kubernetes.io/auth-method" + authBody = "ingress.kubernetes.io/auth-send-body" ) // External returns external authentication configuration for an Ingress rule type External struct { - URL string `json:"url"` - Method string `json:"method"` - SendBody bool `json:"sendBody"` + URL string `json:"url"` + SigninURL string `json:"signinUrl"` + Method string `json:"method"` + SendBody bool `json:"sendBody"` } var ( @@ -68,29 +67,15 @@ func NewParser() parser.IngressAnnotation { // ParseAnnotations parses the annotations contained in the ingress // rule used to use an external URL as source for authentication func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) { - str, err := parser.GetStringAnnotation(authURL, ing) + auth, err := parser.GetURLAnnotation(authURL, ing) if err != nil { return nil, err } - if str == "" { - return nil, ing_errors.NewLocationDenied("an empty string is not a valid URL") - } - - ur, err := url.Parse(str) + signin, err := parser.GetURLAnnotation(authSigninURL, ing) if err != nil { return nil, err } - if ur.Scheme == "" { - return nil, ing_errors.NewLocationDenied("url scheme is empty") - } - if ur.Host == "" { - return nil, ing_errors.NewLocationDenied("url host is empty") - } - - if strings.Contains(ur.Host, "..") { - return nil, ing_errors.NewLocationDenied("invalid url host") - } m, _ := parser.GetStringAnnotation(authMethod, ing) if len(m) != 0 && !validMethod(m) { @@ -100,8 +85,9 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) { sb, _ := parser.GetBoolAnnotation(authBody, ing) return &External{ - URL: str, - Method: m, - SendBody: sb, + URL: auth.String(), + SigninURL: signin.String(), + Method: m, + SendBody: sb, }, nil } diff --git a/core/pkg/ingress/annotations/authreq/main_test.go b/core/pkg/ingress/annotations/authreq/main_test.go index 696d8bdc0..75cd6d2b7 100644 --- a/core/pkg/ingress/annotations/authreq/main_test.go +++ b/core/pkg/ingress/annotations/authreq/main_test.go @@ -67,23 +67,25 @@ func TestAnnotations(t *testing.T) { ing.SetAnnotations(data) tests := []struct { - title string - url string - method string - sendBody bool - expErr bool + title string + url string + signinURL string + method string + sendBody bool + expErr bool }{ - {"empty", "", "", false, true}, - {"no scheme", "bar", "", false, true}, - {"invalid host", "http://", "", false, true}, - {"invalid host (multiple dots)", "http://foo..bar.com", "", false, true}, - {"valid URL", "http://bar.foo.com/external-auth", "", false, false}, - {"valid URL - send body", "http://foo.com/external-auth", "POST", true, false}, - {"valid URL - send body", "http://foo.com/external-auth", "GET", true, false}, + {"empty", "", "", "", false, true}, + {"no scheme", "bar", "bar", "", false, true}, + {"invalid host", "http://", "http://", "", false, true}, + {"invalid host (multiple dots)", "http://foo..bar.com", "http://foo..bar.com", "", false, true}, + {"valid URL", "http://bar.foo.com/external-auth", "http://bar.foo.com/external-auth", "", false, false}, + {"valid URL - send body", "http://foo.com/external-auth", "http://foo.com/external-auth", "POST", true, false}, + {"valid URL - send body", "http://foo.com/external-auth", "http://foo.com/external-auth", "GET", true, false}, } for _, test := range tests { data[authURL] = test.url + data[authSigninURL] = test.signinURL data[authBody] = fmt.Sprintf("%v", test.sendBody) data[authMethod] = fmt.Sprintf("%v", test.method) @@ -101,6 +103,9 @@ func TestAnnotations(t *testing.T) { if u.URL != test.url { t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.url, u.URL) } + if u.SigninURL != test.signinURL { + t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.signinURL, u.SigninURL) + } if u.Method != test.method { t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.method, u.Method) } diff --git a/core/pkg/ingress/annotations/parser/main.go b/core/pkg/ingress/annotations/parser/main.go index bff6f2210..cbb902a62 100644 --- a/core/pkg/ingress/annotations/parser/main.go +++ b/core/pkg/ingress/annotations/parser/main.go @@ -17,7 +17,9 @@ limitations under the License. package parser import ( + "net/url" "strconv" + "strings" "k8s.io/kubernetes/pkg/apis/extensions" @@ -51,6 +53,30 @@ func (a ingAnnotations) parseString(name string) (string, error) { return "", errors.ErrMissingAnnotations } +func (a ingAnnotations) parseURL(name string) (*url.URL, error) { + val, ok := a[name] + if ok { + ur, err := url.Parse(val) + if err != nil { + return nil, err + } + if ur.Scheme == "" { + return nil, errors.NewLocationDenied("url scheme is empty") + } + if ur.Host == "" { + return nil, errors.NewLocationDenied("url host is empty") + } + if val == "" { + return nil, errors.NewLocationDenied("an empty string is not a valid URL") + } + if strings.Contains(ur.Host, "..") { + return nil, errors.NewLocationDenied("invalid url host") + } + return ur, nil + } + return nil, errors.ErrMissingAnnotations +} + func (a ingAnnotations) parseInt(name string) (int, error) { val, ok := a[name] if ok { @@ -100,3 +126,12 @@ func GetIntAnnotation(name string, ing *extensions.Ingress) (int, error) { } return ingAnnotations(ing.GetAnnotations()).parseInt(name) } + +// GetUrlAnnotation extracts a URL from an Ingress annotation +func GetURLAnnotation(name string, ing *extensions.Ingress) (*url.URL, error) { + err := checkAnnotation(name, ing) + if err != nil { + return nil, err + } + return ingAnnotations(ing.GetAnnotations()).parseURL(name) +}