Merge pull request #6294 from ianbuss/auth-error-redirect-param

Allow customisation of redirect URL parameter in external auth redirects
This commit is contained in:
Kubernetes Prow Robot 2020-11-23 01:27:37 -08:00 committed by GitHub
commit e3a3ea8826
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 233 additions and 86 deletions

View file

@ -440,6 +440,8 @@ Additionally it is possible to set:
`<Method>` to specify the HTTP method to use.
* `nginx.ingress.kubernetes.io/auth-signin`:
`<SignIn_URL>` to specify the location of the error page.
* `nginx.ingress.kubernetes.io/auth-signin-redirect-param`:
`<SignIn_URL>` to specify the URL parameter in the error page which should contain the original URL for a failed signin request.
* `nginx.ingress.kubernetes.io/auth-response-headers`:
`<Response_Header_1, ..., Response_Header_n>` to specify headers to pass to backend once authentication request completes.
* `nginx.ingress.kubernetes.io/auth-proxy-set-headers`:

View file

@ -177,6 +177,7 @@ The following table shows a configuration option's name, type, and the default v
|[global-auth-url](#global-auth-url)|string|""|
|[global-auth-method](#global-auth-method)|string|""|
|[global-auth-signin](#global-auth-signin)|string|""|
|[global-auth-signin-redirect-param](#global-auth-signin-redirect-param)|string|"rd"|
|[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|""|
@ -1053,6 +1054,12 @@ Sets the location of the error page for an existing service that provides authen
Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-signin`.
_**default:**_ ""
## global-auth-signin-redirect-param
Sets the query parameter in the error page signin URL which contains the original URL of the request that failed authentication.
Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-signin-redirect-param`.
_**default:**_ "rd"
## global-auth-response-headers
Sets the headers to pass to backend once authentication request completes. Applied to all the locations.

View file

@ -35,15 +35,16 @@ import (
type Config struct {
URL string `json:"url"`
// Host contains the hostname defined in the URL
Host string `json:"host"`
SigninURL string `json:"signinUrl"`
Method string `json:"method"`
ResponseHeaders []string `json:"responseHeaders,omitempty"`
RequestRedirect string `json:"requestRedirect"`
AuthSnippet string `json:"authSnippet"`
AuthCacheKey string `json:"authCacheKey"`
AuthCacheDuration []string `json:"authCacheDuration"`
ProxySetHeaders map[string]string `json:"proxySetHeaders,omitempty"`
Host string `json:"host"`
SigninURL string `json:"signinUrl"`
SigninURLRedirectParam string `json:"signinUrlRedirectParam,omitempty"`
Method string `json:"method"`
ResponseHeaders []string `json:"responseHeaders,omitempty"`
RequestRedirect string `json:"requestRedirect"`
AuthSnippet string `json:"authSnippet"`
AuthCacheKey string `json:"authCacheKey"`
AuthCacheDuration []string `json:"authCacheDuration"`
ProxySetHeaders map[string]string `json:"proxySetHeaders,omitempty"`
}
// DefaultCacheDuration is the fallback value if no cache duration is provided
@ -66,6 +67,9 @@ func (e1 *Config) Equal(e2 *Config) bool {
if e1.SigninURL != e2.SigninURL {
return false
}
if e1.SigninURLRedirectParam != e2.SigninURLRedirectParam {
return false
}
if e1.Method != e2.Method {
return false
}
@ -174,6 +178,11 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
klog.V(3).InfoS("auth-signin annotation is undefined and will not be set")
}
signInRedirectParam, err := parser.GetStringAnnotation("auth-signin-redirect-param", ing)
if err != nil {
klog.V(3).Infof("auth-signin-redirect-param annotation is undefined and will not be set")
}
authSnippet, err := parser.GetStringAnnotation("auth-snippet", ing)
if err != nil {
klog.V(3).InfoS("auth-snippet annotation is undefined and will not be set")
@ -230,16 +239,17 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
requestRedirect, _ := parser.GetStringAnnotation("auth-request-redirect", ing)
return &Config{
URL: urlString,
Host: authURL.Hostname(),
SigninURL: signIn,
Method: authMethod,
ResponseHeaders: responseHeaders,
RequestRedirect: requestRedirect,
AuthSnippet: authSnippet,
AuthCacheKey: authCacheKey,
AuthCacheDuration: authCacheDuration,
ProxySetHeaders: proxySetHeaders,
URL: urlString,
Host: authURL.Hostname(),
SigninURL: signIn,
SigninURLRedirectParam: signInRedirectParam,
Method: authMethod,
ResponseHeaders: responseHeaders,
RequestRedirect: requestRedirect,
AuthSnippet: authSnippet,
AuthCacheKey: authCacheKey,
AuthCacheDuration: authCacheDuration,
ProxySetHeaders: proxySetHeaders,
}, nil
}

View file

@ -72,30 +72,33 @@ func TestAnnotations(t *testing.T) {
ing.SetAnnotations(data)
tests := []struct {
title string
url string
signinURL string
method string
requestRedirect string
authSnippet string
authCacheKey string
expErr bool
title string
url string
signinURL string
signinURLRedirectParam string
method string
requestRedirect string
authSnippet string
authCacheKey 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},
{"valid URL - request redirect", "http://foo.com/external-auth", "http://foo.com/external-auth", "GET", "http://foo.com/redirect-me", "", "", false},
{"auth snippet", "http://foo.com/external-auth", "http://foo.com/external-auth", "", "", "proxy_set_header My-Custom-Header 42;", "", false},
{"auth cache ", "http://foo.com/external-auth", "http://foo.com/external-auth", "", "", "", "$foo$bar", 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},
{"auth snippet", "http://foo.com/external-auth", "http://foo.com/external-auth", "", "", "", "proxy_set_header My-Custom-Header 42;", "", false},
{"auth cache ", "http://foo.com/external-auth", "http://foo.com/external-auth", "", "", "", "", "$foo$bar", false},
{"redirect param", "http://bar.foo.com/external-auth", "http://bar.foo.com/external-auth", "origUrl", "", "", "", "", false},
}
for _, test := range tests {
data[parser.GetAnnotationWithPrefix("auth-url")] = test.url
data[parser.GetAnnotationWithPrefix("auth-signin")] = test.signinURL
data[parser.GetAnnotationWithPrefix("auth-signin-redirect-param")] = test.signinURLRedirectParam
data[parser.GetAnnotationWithPrefix("auth-method")] = fmt.Sprintf("%v", test.method)
data[parser.GetAnnotationWithPrefix("auth-request-redirect")] = test.requestRedirect
data[parser.GetAnnotationWithPrefix("auth-snippet")] = test.authSnippet
@ -122,6 +125,9 @@ func TestAnnotations(t *testing.T) {
if u.SigninURL != test.signinURL {
t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.signinURL, u.SigninURL)
}
if u.SigninURLRedirectParam != test.signinURLRedirectParam {
t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.signinURLRedirectParam, u.SigninURLRedirectParam)
}
if u.Method != test.method {
t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.method, u.Method)
}

View file

@ -708,7 +708,7 @@ func NewDefault() Configuration {
defNginxStatusIpv4Whitelist = append(defNginxStatusIpv4Whitelist, "127.0.0.1")
defNginxStatusIpv6Whitelist = append(defNginxStatusIpv6Whitelist, "::1")
defProxyDeadlineDuration := time.Duration(5) * time.Second
defGlobalExternalAuth := GlobalExternalAuth{"", "", "", "", append(defResponseHeaders, ""), "", "", "", []string{}, map[string]string{}}
defGlobalExternalAuth := GlobalExternalAuth{"", "", "", "", "", append(defResponseHeaders, ""), "", "", "", []string{}, map[string]string{}}
cfg := Configuration{
AllowBackendServerHeader: false,
@ -893,13 +893,14 @@ type ListenPorts struct {
type GlobalExternalAuth struct {
URL string `json:"url"`
// Host contains the hostname defined in the URL
Host string `json:"host"`
SigninURL string `json:"signinUrl"`
Method string `json:"method"`
ResponseHeaders []string `json:"responseHeaders,omitempty"`
RequestRedirect string `json:"requestRedirect"`
AuthSnippet string `json:"authSnippet"`
AuthCacheKey string `json:"authCacheKey"`
AuthCacheDuration []string `json:"authCacheDuration"`
ProxySetHeaders map[string]string `json:"proxySetHeaders,omitempty"`
Host string `json:"host"`
SigninURL string `json:"signinUrl"`
SigninURLRedirectParam string `json:"signinUrlRedirectParam"`
Method string `json:"method"`
ResponseHeaders []string `json:"responseHeaders,omitempty"`
RequestRedirect string `json:"requestRedirect"`
AuthSnippet string `json:"authSnippet"`
AuthCacheKey string `json:"authCacheKey"`
AuthCacheDuration []string `json:"authCacheDuration"`
ProxySetHeaders map[string]string `json:"proxySetHeaders,omitempty"`
}

View file

@ -37,31 +37,32 @@ import (
)
const (
customHTTPErrors = "custom-http-errors"
skipAccessLogUrls = "skip-access-log-urls"
whitelistSourceRange = "whitelist-source-range"
proxyRealIPCIDR = "proxy-real-ip-cidr"
bindAddress = "bind-address"
httpRedirectCode = "http-redirect-code"
blockCIDRs = "block-cidrs"
blockUserAgents = "block-user-agents"
blockReferers = "block-referers"
proxyStreamResponses = "proxy-stream-responses"
hideHeaders = "hide-headers"
nginxStatusIpv4Whitelist = "nginx-status-ipv4-whitelist"
nginxStatusIpv6Whitelist = "nginx-status-ipv6-whitelist"
proxyHeaderTimeout = "proxy-protocol-header-timeout"
workerProcesses = "worker-processes"
globalAuthURL = "global-auth-url"
globalAuthMethod = "global-auth-method"
globalAuthSignin = "global-auth-signin"
globalAuthResponseHeaders = "global-auth-response-headers"
globalAuthRequestRedirect = "global-auth-request-redirect"
globalAuthSnippet = "global-auth-snippet"
globalAuthCacheKey = "global-auth-cache-key"
globalAuthCacheDuration = "global-auth-cache-duration"
luaSharedDictsKey = "lua-shared-dicts"
plugins = "plugins"
customHTTPErrors = "custom-http-errors"
skipAccessLogUrls = "skip-access-log-urls"
whitelistSourceRange = "whitelist-source-range"
proxyRealIPCIDR = "proxy-real-ip-cidr"
bindAddress = "bind-address"
httpRedirectCode = "http-redirect-code"
blockCIDRs = "block-cidrs"
blockUserAgents = "block-user-agents"
blockReferers = "block-referers"
proxyStreamResponses = "proxy-stream-responses"
hideHeaders = "hide-headers"
nginxStatusIpv4Whitelist = "nginx-status-ipv4-whitelist"
nginxStatusIpv6Whitelist = "nginx-status-ipv6-whitelist"
proxyHeaderTimeout = "proxy-protocol-header-timeout"
workerProcesses = "worker-processes"
globalAuthURL = "global-auth-url"
globalAuthMethod = "global-auth-method"
globalAuthSignin = "global-auth-signin"
globalAuthSigninRedirectParam = "global-auth-signin-redirect-param"
globalAuthResponseHeaders = "global-auth-response-headers"
globalAuthRequestRedirect = "global-auth-request-redirect"
globalAuthSnippet = "global-auth-snippet"
globalAuthCacheKey = "global-auth-cache-key"
globalAuthCacheDuration = "global-auth-cache-duration"
luaSharedDictsKey = "lua-shared-dicts"
plugins = "plugins"
)
var (
@ -75,6 +76,7 @@ var (
"certificate_servers": 5,
"ocsp_response_cache": 5, // keep this same as certificate_servers
}
defaultGlobalAuthRedirectParam = "rd"
)
const (
@ -254,6 +256,19 @@ func ReadConfig(src map[string]string) config.Configuration {
}
}
// Verify that the configured global external authorization error page redirection URL parameter is set and valid. if not, set the default value
if val, ok := conf[globalAuthSigninRedirectParam]; ok {
delete(conf, globalAuthSigninRedirectParam)
redirectParam := strings.TrimSpace(val)
dummySigninURL, _ := parser.StringToURL(fmt.Sprintf("%s?%s=dummy", to.GlobalExternalAuth.SigninURL, redirectParam))
if dummySigninURL == nil {
klog.Warningf("Global auth redirect parameter denied - %v.", "global-auth-signin-redirect-param setting is invalid and will not be set")
} else {
to.GlobalExternalAuth.SigninURLRedirectParam = redirectParam
}
}
// Verify that the configured global external authorization response headers are valid. if not, set the default value
if val, ok := conf[globalAuthResponseHeaders]; ok {
delete(conf, globalAuthResponseHeaders)

View file

@ -229,6 +229,28 @@ func TestGlobalExternalAuthSigninParsing(t *testing.T) {
}
}
func TestGlobalExternalAuthSigninRedirectParamParsing(t *testing.T) {
testCases := map[string]struct {
param string
signin string
expect string
}{
"no param": {"", "http://bar.foo.com/auth-error-page", ""},
"valid param": {"orig", "http://bar.foo.com/auth-error-page", "orig"},
"no signin url": {"orig", "", ""},
}
for n, tc := range testCases {
cfg := ReadConfig(map[string]string{
"global-auth-signin": tc.signin,
"global-auth-signin-redirect-param": tc.param,
})
if cfg.GlobalExternalAuth.SigninURLRedirectParam != tc.expect {
t.Errorf("Testing %v. Expected \"%v\" but \"%v\" was returned", n, tc.expect, cfg.GlobalExternalAuth.SigninURLRedirectParam)
}
}
}
func TestGlobalExternalAuthResponseHeadersParsing(t *testing.T) {
testCases := map[string]struct {
headers string

View file

@ -50,9 +50,10 @@ import (
)
const (
slash = "/"
nonIdempotent = "non_idempotent"
defBufferSize = 65535
slash = "/"
nonIdempotent = "non_idempotent"
defBufferSize = 65535
defAuthSigninRedirectParam = "rd"
)
// TemplateWriter is the interface to render a template
@ -910,18 +911,21 @@ func buildForwardedFor(input interface{}) string {
return fmt.Sprintf("$http_%v", ffh)
}
func buildAuthSignURL(authSignURL string) string {
func buildAuthSignURL(authSignURL, authRedirectParam string) string {
u, _ := url.Parse(authSignURL)
q := u.Query()
if authRedirectParam == "" {
authRedirectParam = defaultGlobalAuthRedirectParam
}
if len(q) == 0 {
return fmt.Sprintf("%v?rd=$pass_access_scheme://$http_host$escaped_request_uri", authSignURL)
return fmt.Sprintf("%v?%v=$pass_access_scheme://$http_host$escaped_request_uri", authSignURL, authRedirectParam)
}
if q.Get("rd") != "" {
if q.Get(authRedirectParam) != "" {
return authSignURL
}
return fmt.Sprintf("%v&rd=$pass_access_scheme://$http_host$escaped_request_uri", authSignURL)
return fmt.Sprintf("%v&%v=$pass_access_scheme://$http_host$escaped_request_uri", authSignURL, authRedirectParam)
}
func buildAuthSignURLLocation(location, authSignURL string) string {

View file

@ -766,16 +766,19 @@ func TestFilterRateLimits(t *testing.T) {
func TestBuildAuthSignURL(t *testing.T) {
cases := map[string]struct {
Input, Output string
Input, RedirectParam, Output string
}{
"default url": {"http://google.com", "http://google.com?rd=$pass_access_scheme://$http_host$escaped_request_uri"},
"with random field": {"http://google.com?cat=0", "http://google.com?cat=0&rd=$pass_access_scheme://$http_host$escaped_request_uri"},
"with rd field": {"http://google.com?cat&rd=$request", "http://google.com?cat&rd=$request"},
"default url and redirect": {"http://google.com", "rd", "http://google.com?rd=$pass_access_scheme://$http_host$escaped_request_uri"},
"default url and custom redirect": {"http://google.com", "orig", "http://google.com?orig=$pass_access_scheme://$http_host$escaped_request_uri"},
"with random field": {"http://google.com?cat=0", "rd", "http://google.com?cat=0&rd=$pass_access_scheme://$http_host$escaped_request_uri"},
"with random field and custom redirect": {"http://google.com?cat=0", "orig", "http://google.com?cat=0&orig=$pass_access_scheme://$http_host$escaped_request_uri"},
"with rd field": {"http://google.com?cat&rd=$request", "rd", "http://google.com?cat&rd=$request"},
"with orig field": {"http://google.com?cat&orig=$request", "orig", "http://google.com?cat&orig=$request"},
}
for k, tc := range cases {
res := buildAuthSignURL(tc.Input)
res := buildAuthSignURL(tc.Input, tc.RedirectParam)
if res != tc.Output {
t.Errorf("%s: called buildAuthSignURL('%s'); expected '%v' but returned '%v'", k, tc.Input, tc.Output, res)
t.Errorf("%s: called buildAuthSignURL('%s','%s'); expected '%v' but returned '%v'", k, tc.Input, tc.RedirectParam, tc.Output, res)
}
}
}

View file

@ -1050,7 +1050,7 @@ stream {
add_header Set-Cookie $auth_cookie;
return 302 {{ buildAuthSignURL $externalAuth.SigninURL }};
return 302 {{ buildAuthSignURL $externalAuth.SigninURL $externalAuth.SigninURLRedirectParam }};
}
{{ end }}
{{ end }}

View file

@ -455,6 +455,83 @@ http {
})
})
ginkgo.Context("when external authentication is configured with a custom redirect param", func() {
host := "auth"
var annotations map[string]string
var ing *networking.Ingress
ginkgo.BeforeEach(func() {
f.NewHttpbinDeployment()
var httpbinIP string
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)
httpbinIP = e.Subsets[0].Addresses[0].IP
annotations = map[string]string{
"nginx.ingress.kubernetes.io/auth-url": fmt.Sprintf("http://%s/basic-auth/user/password", httpbinIP),
"nginx.ingress.kubernetes.io/auth-signin": "http://$host/auth/start",
"nginx.ingress.kubernetes.io/auth-signin-redirect-param": "orig",
}
ing = framework.NewSingleIngress(host, "/", 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 status code 200 when signed in", func() {
f.HTTPTestClient().
GET("/").
WithHeader("Host", host).
WithBasicAuth("user", "password").
Expect().
Status(http.StatusOK)
})
ginkgo.It("should redirect to signin url when not signed in", func() {
f.HTTPTestClient().
GET("/").
WithHeader("Host", host).
WithQuery("a", "b").
WithQuery("c", "d").
Expect().
Status(http.StatusFound).
Header("Location").Equal(fmt.Sprintf("http://%s/auth/start?orig=http://%s%s", host, host, url.QueryEscape("/?a=b&c=d")))
})
ginkgo.It("keeps processing new ingresses even if one of the existing ingresses is misconfigured", func() {
annotations["nginx.ingress.kubernetes.io/auth-type"] = "basic"
annotations["nginx.ingress.kubernetes.io/auth-secret"] = "something"
annotations["nginx.ingress.kubernetes.io/auth-realm"] = "test auth"
f.UpdateIngress(ing)
anotherHost := "different"
anotherAnnotations := map[string]string{}
anotherIng := framework.NewSingleIngress(anotherHost, "/", anotherHost, f.Namespace, framework.EchoService, 80, anotherAnnotations)
f.EnsureIngress(anotherIng)
f.WaitForNginxServer(anotherHost,
func(server string) bool {
return strings.Contains(server, "server_name "+anotherHost)
})
f.HTTPTestClient().
GET("/").
WithHeader("Host", anotherHost).
Expect().
Status(http.StatusOK)
})
})
ginkgo.Context("when external authentication with caching is configured", func() {
thisHost := "auth"
thatHost := "different"