diff --git a/docs/examples/auth/oauth-external-auth/README.md b/docs/examples/auth/oauth-external-auth/README.md index c151be340..9199a6dcf 100644 --- a/docs/examples/auth/oauth-external-auth/README.md +++ b/docs/examples/auth/oauth-external-auth/README.md @@ -25,7 +25,7 @@ metadata: name: application annotations: nginx.ingress.kubernetes.io/auth-url: "https://$host/oauth2/auth" - nginx.ingress.kubernetes.io/auth-signin: "https://$host/oauth2/start?rd=$request_uri" + nginx.ingress.kubernetes.io/auth-signin: "https://$host/oauth2/start?rd=$escaped_request_uri" ... ``` diff --git a/docs/examples/auth/oauth-external-auth/dashboard-ingress.yaml b/docs/examples/auth/oauth-external-auth/dashboard-ingress.yaml index 743f6b49e..17a222939 100644 --- a/docs/examples/auth/oauth-external-auth/dashboard-ingress.yaml +++ b/docs/examples/auth/oauth-external-auth/dashboard-ingress.yaml @@ -3,7 +3,7 @@ kind: Ingress metadata: annotations: nginx.ingress.kubernetes.io/auth-url: "https://$host/oauth2/auth" - nginx.ingress.kubernetes.io/auth-signin: "https://$host/oauth2/start?rd=$request_uri" + nginx.ingress.kubernetes.io/auth-signin: "https://$host/oauth2/start?rd=$escaped_request_uri" name: external-auth-oauth2 namespace: kube-system spec: diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index adfd0d1a6..df301a0f7 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -815,14 +815,14 @@ func buildAuthSignURL(input interface{}) string { u, _ := url.Parse(s) q := u.Query() if len(q) == 0 { - return fmt.Sprintf("%v?rd=$pass_access_scheme://$http_host$request_uri", s) + return fmt.Sprintf("%v?rd=$pass_access_scheme://$http_host$escaped_request_uri", s) } if q.Get("rd") != "" { return s } - return fmt.Sprintf("%v&rd=$pass_access_scheme://$http_host$request_uri", s) + return fmt.Sprintf("%v&rd=$pass_access_scheme://$http_host$escaped_request_uri", s) } var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index f42514c9a..256b5ae0f 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -697,8 +697,8 @@ func TestBuildAuthSignURL(t *testing.T) { cases := map[string]struct { Input, Output string }{ - "default url": {"http://google.com", "http://google.com?rd=$pass_access_scheme://$http_host$request_uri"}, - "with random field": {"http://google.com?cat=0", "http://google.com?cat=0&rd=$pass_access_scheme://$http_host$request_uri"}, + "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"}, } for k, tc := range cases { diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index c57272d72..35516c4bc 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -977,6 +977,7 @@ stream { {{ end }} {{ if $location.ExternalAuth.SigninURL }} + set_escape_uri $escaped_request_uri $request_uri; error_page 401 = {{ buildAuthSignURL $location.ExternalAuth.SigninURL }}; {{ end }} diff --git a/test/e2e/annotations/auth.go b/test/e2e/annotations/auth.go index 31cc1f504..1816f2614 100644 --- a/test/e2e/annotations/auth.go +++ b/test/e2e/annotations/auth.go @@ -19,6 +19,7 @@ package annotations import ( "fmt" "net/http" + "net/url" "os/exec" "time" @@ -27,7 +28,9 @@ import ( "github.com/parnurzeal/gorequest" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -265,6 +268,77 @@ var _ = framework.IngressNginxDescribe("Annotations - Auth", func() { Expect(len(errs)).Should(BeNumerically("==", 0)) Expect(resp.StatusCode).Should(Equal(http.StatusInternalServerError)) }) + + Context("when external authentication is configured", func() { + host := "auth" + + BeforeEach(func() { + err := f.NewHttpbinDeployment() + Expect(err).NotTo(HaveOccurred()) + + var httpbinIP string + err = wait.PollImmediate(time.Second, time.Minute, func() (bool, error) { + e, err := f.KubeClientSet.CoreV1().Endpoints(f.IngressController.Namespace).Get("httpbin", metav1.GetOptions{}) + if errors.IsNotFound(err) { + return false, nil + } + if err != nil { + return false, err + } + if len(e.Subsets) < 1 || len(e.Subsets[0].Addresses) < 1 { + return false, nil + } + httpbinIP = e.Subsets[0].Addresses[0].IP + return true, nil + }) + Expect(err).NotTo(HaveOccurred()) + + bi, err := f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &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", + })) + Expect(err).NotTo(HaveOccurred()) + Expect(bi).NotTo(BeNil()) + + err = f.WaitForNginxServer(host, func(server string) bool { + return Expect(server).ShouldNot(ContainSubstring("return 503")) + }) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should return status code 200 when signed in", func() { + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Retry(10, 1*time.Second, http.StatusNotFound). + Set("Host", host). + SetBasicAuth("user", "password"). + End() + + for _, err := range errs { + Expect(err).NotTo(HaveOccurred()) + } + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + }) + + It("should redirect to signin url when not signed in", func() { + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Retry(10, 1*time.Second, http.StatusNotFound). + Set("Host", host). + RedirectPolicy(func(req gorequest.Request, via []gorequest.Request) error { + return http.ErrUseLastResponse + }). + Param("a", "b"). + Param("c", "d"). + End() + + for _, err := range errs { + Expect(err).NotTo(HaveOccurred()) + } + Expect(resp.StatusCode).Should(Equal(http.StatusFound)) + Expect(resp.Header.Get("Location")).Should(Equal(fmt.Sprintf("http://%s/auth/start?rd=http://%s%s", host, host, url.QueryEscape("/?a=b&c=d")))) + }) + }) }) // TODO: test Digest Auth diff --git a/test/e2e/framework/echo.go b/test/e2e/framework/deployment.go similarity index 76% rename from test/e2e/framework/echo.go rename to test/e2e/framework/deployment.go index bda12f7e5..225e2af57 100644 --- a/test/e2e/framework/echo.go +++ b/test/e2e/framework/deployment.go @@ -37,35 +37,45 @@ func (f *Framework) NewEchoDeployment() error { // NewEchoDeploymentWithReplicas creates a new deployment of the echoserver image in a particular namespace. Number of // replicas is configurable func (f *Framework) NewEchoDeploymentWithReplicas(replicas int32) error { + return f.NewDeployment("http-svc", "gcr.io/google_containers/echoserver:1.10", 8080, replicas) +} + +// NewHttpbinDeployment creates a new single replica deployment of the httpbin image in a particular namespace. +func (f *Framework) NewHttpbinDeployment() error { + return f.NewDeployment("httpbin", "kennethreitz/httpbin", 80, 1) +} + +// NewDeployment creates a new deployment in a particular namespace. +func (f *Framework) NewDeployment(name, image string, port int32, replicas int32) error { deployment := &extensions.Deployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "http-svc", + Name: name, Namespace: f.IngressController.Namespace, }, Spec: extensions.DeploymentSpec{ Replicas: NewInt32(replicas), Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - "app": "http-svc", + "app": name, }, }, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "app": "http-svc", + "app": name, }, }, Spec: corev1.PodSpec{ TerminationGracePeriodSeconds: NewInt64(0), Containers: []corev1.Container{ { - Name: "http-svc", - Image: "gcr.io/google_containers/echoserver:1.10", + Name: name, + Image: image, Env: []corev1.EnvVar{}, Ports: []corev1.ContainerPort{ { Name: "http", - ContainerPort: 8080, + ContainerPort: port, }, }, }, @@ -81,7 +91,7 @@ func (f *Framework) NewEchoDeploymentWithReplicas(replicas int32) error { } if d == nil { - return fmt.Errorf("unexpected error creating deployement for echoserver") + return fmt.Errorf("unexpected error creating deployement %s", name) } err = WaitForPodsReady(f.KubeClientSet, 5*time.Minute, int(replicas), f.IngressController.Namespace, metav1.ListOptions{ @@ -93,7 +103,7 @@ func (f *Framework) NewEchoDeploymentWithReplicas(replicas int32) error { service := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: "http-svc", + Name: name, Namespace: f.IngressController.Namespace, }, Spec: corev1.ServiceSpec{ @@ -101,12 +111,12 @@ func (f *Framework) NewEchoDeploymentWithReplicas(replicas int32) error { { Name: "http", Port: 80, - TargetPort: intstr.FromInt(8080), + TargetPort: intstr.FromInt(int(port)), Protocol: "TCP", }, }, Selector: map[string]string{ - "app": "http-svc", + "app": name, }, }, } @@ -117,7 +127,7 @@ func (f *Framework) NewEchoDeploymentWithReplicas(replicas int32) error { } if s == nil { - return fmt.Errorf("unexpected error creating service for echoserver deployment") + return fmt.Errorf("unexpected error creating service %s", name) } return nil