Merge pull request #16929 from krousey/basic_auth_cleanup

Auto commit by PR queue bot

pkg/client/unversioned/request_test.go

@@ -18,7 +18,6 @@ package unversioned
 
 import (
 	"bytes"
-	"encoding/base64"
 	"errors"
 	"io"
 	"io/ioutil"
@@ -1126,36 +1125,14 @@ func TestBody(t *testing.T) {
 	}
 }
 
-func authFromReq(r *http.Request) (*Config, bool) {
-	auth, ok := r.Header["Authorization"]
-	if !ok {
-		return nil, false
-	}
-
-	encoded := strings.Split(auth[0], " ")
-	if len(encoded) != 2 || encoded[0] != "Basic" {
-		return nil, false
-	}
-
-	decoded, err := base64.StdEncoding.DecodeString(encoded[1])
-	if err != nil {
-		return nil, false
-	}
-	parts := strings.Split(string(decoded), ":")
-	if len(parts) != 2 {
-		return nil, false
-	}
-	return &Config{Username: parts[0], Password: parts[1]}, true
-}
-
 // checkAuth sets errors if the auth found in r doesn't match the expectation.
 // TODO: Move to util, test in more places.
-func checkAuth(t *testing.T, expect *Config, r *http.Request) {
+func checkAuth(t *testing.T, expectedUser, expectedPass string, r *http.Request) {
-	foundAuth, found := authFromReq(r)
+	user, pass, found := r.BasicAuth()
 	if !found {
 		t.Errorf("no auth found")
-	} else if e, a := expect, foundAuth; !api.Semantic.DeepDerivative(e, a) {
+	} else if user != expectedUser || pass != expectedPass {
-		t.Fatalf("Wrong basic auth: wanted %#v, got %#v", e, a)
+		t.Fatalf("Wrong basic auth: wanted %s:%s, got %s:%s", expectedUser, expectedPass, user, pass)
 	}
 }
 
@@ -1169,9 +1146,8 @@ func TestWatch(t *testing.T) {
 		{watch.Deleted, &api.Pod{ObjectMeta: api.ObjectMeta{Name: "last"}}},
 	}
 
-	auth := &Config{Username: "user", Password: "pass"}
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		checkAuth(t, auth, r)
+		checkAuth(t, "user", "pass", r)
 		flusher, ok := w.(http.Flusher)
 		if !ok {
 			panic("need flusher!")
@@ -1225,11 +1201,10 @@ func TestWatch(t *testing.T) {
 }
 
 func TestStream(t *testing.T) {
-	auth := &Config{Username: "user", Password: "pass"}
 	expectedBody := "expected body"
 
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		checkAuth(t, auth, r)
+		checkAuth(t, "user", "pass", r)
 		flusher, ok := w.(http.Flusher)
 		if !ok {
 			panic("need flusher!")

pkg/client/unversioned/transport_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package unversioned
 
 import (
-	"encoding/base64"
 	"net/http"
 	"testing"
 
@@ -60,7 +59,7 @@ func TestBasicAuthRoundTripper(t *testing.T) {
 	if rt.Request == req {
 		t.Fatalf("round tripper should have copied request object: %#v", rt.Request)
 	}
-	if rt.Request.Header.Get("Authorization") != "Basic "+base64.StdEncoding.EncodeToString([]byte("user:pass")) {
+	if user, pass, found := rt.Request.BasicAuth(); !found || user != "user" || pass != "pass" {
 		t.Errorf("unexpected authorization header: %#v", rt.Request)
 	}
 }

plugin/pkg/auth/authenticator/request/basicauth/basicauth.go

@@ -17,10 +17,7 @@ limitations under the License.
 package basicauth
 
 import (
-	"encoding/base64"
-	"errors"
 	"net/http"
-	"strings"
 
 	"k8s.io/kubernetes/pkg/auth/authenticator"
 	"k8s.io/kubernetes/pkg/auth/user"
@@ -38,26 +35,9 @@ func New(auth authenticator.Password) *Authenticator {
 
 // AuthenticateRequest authenticates the request using the "Authorization: Basic" header in the request
 func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool, error) {
-	auth := strings.TrimSpace(req.Header.Get("Authorization"))
+	username, password, found := req.BasicAuth()
-	if auth == "" {
+	if !found {
 		return nil, false, nil
 	}
-	parts := strings.Split(auth, " ")
-	if len(parts) < 2 || strings.ToLower(parts[0]) != "basic" {
-		return nil, false, nil
-	}
-
-	payload, err := base64.StdEncoding.DecodeString(parts[1])
-	if err != nil {
-		return nil, false, err
-	}
-
-	pair := strings.SplitN(string(payload), ":", 2)
-	if len(pair) != 2 {
-		return nil, false, errors.New("malformed basic auth header")
-	}
-
-	username := pair[0]
-	password := pair[1]
 	return a.auth.AuthenticatePassword(username, password)
 }

plugin/pkg/auth/authenticator/request/basicauth/basicauth_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package basicauth
 
 import (
-	"encoding/base64"
 	"errors"
 	"net/http"
 	"testing"
@@ -56,40 +55,18 @@ func TestBasicAuth(t *testing.T) {
 		ExpectedOK   bool
 		ExpectedErr  bool
 	}{
-		"no header": {
+		"no auth": {},
-			Header: "",
-		},
-		"non-basic header": {
-			Header: "Bearer foo",
-		},
-		"empty value basic header": {
-			Header: "Basic",
-		},
-		"whitespace value basic header": {
-			Header: "Basic  ",
-		},
-		"non base-64 basic header": {
-			Header:      "Basic !@#$",
-			ExpectedErr: true,
-		},
-		"malformed basic header": {
-			Header:      "Basic " + base64.StdEncoding.EncodeToString([]byte("user_without_password")),
-			ExpectedErr: true,
-		},
 		"empty password basic header": {
-			Header:           "Basic " + base64.StdEncoding.EncodeToString([]byte("user_with_empty_password:")),
 			ExpectedCalled:   true,
 			ExpectedUsername: "user_with_empty_password",
 			ExpectedPassword: "",
 		},
 		"valid basic header": {
-			Header:           "Basic " + base64.StdEncoding.EncodeToString([]byte("myuser:mypassword:withcolon")),
 			ExpectedCalled:   true,
 			ExpectedUsername: "myuser",
 			ExpectedPassword: "mypassword:withcolon",
 		},
 		"password auth returned user": {
-			Header:           "Basic " + base64.StdEncoding.EncodeToString([]byte("myuser:mypw")),
 			Password:         testPassword{User: &user.DefaultInfo{Name: "returneduser"}, OK: true},
 			ExpectedCalled:   true,
 			ExpectedUsername: "myuser",
@@ -98,7 +75,6 @@ func TestBasicAuth(t *testing.T) {
 			ExpectedOK:       true,
 		},
 		"password auth returned error": {
-			Header:           "Basic " + base64.StdEncoding.EncodeToString([]byte("myuser:mypw")),
 			Password:         testPassword{Err: errors.New("auth error")},
 			ExpectedCalled:   true,
 			ExpectedUsername: "myuser",
@@ -112,35 +88,35 @@ func TestBasicAuth(t *testing.T) {
 		auth := authenticator.Request(New(&password))
 
 		req, _ := http.NewRequest("GET", "/", nil)
-		if testCase.Header != "" {
+		if testCase.ExpectedUsername != "" || testCase.ExpectedPassword != "" {
-			req.Header.Set("Authorization", testCase.Header)
+			req.SetBasicAuth(testCase.ExpectedUsername, testCase.ExpectedPassword)
 		}
 
 		user, ok, err := auth.AuthenticateRequest(req)
 
 		if testCase.ExpectedCalled != password.Called {
-			t.Fatalf("%s: Expected called=%v, got %v", k, testCase.ExpectedCalled, password.Called)
+			t.Errorf("%s: Expected called=%v, got %v", k, testCase.ExpectedCalled, password.Called)
 			continue
 		}
 		if testCase.ExpectedUsername != password.Username {
-			t.Fatalf("%s: Expected called with username=%v, got %v", k, testCase.ExpectedUsername, password.Username)
+			t.Errorf("%s: Expected called with username=%v, got %v", k, testCase.ExpectedUsername, password.Username)
 			continue
 		}
 		if testCase.ExpectedPassword != password.Password {
-			t.Fatalf("%s: Expected called with password=%v, got %v", k, testCase.ExpectedPassword, password.Password)
+			t.Errorf("%s: Expected called with password=%v, got %v", k, testCase.ExpectedPassword, password.Password)
 			continue
 		}
 
 		if testCase.ExpectedErr != (err != nil) {
-			t.Fatalf("%s: Expected err=%v, got err=%v", k, testCase.ExpectedErr, err)
+			t.Errorf("%s: Expected err=%v, got err=%v", k, testCase.ExpectedErr, err)
 			continue
 		}
 		if testCase.ExpectedOK != ok {
-			t.Fatalf("%s: Expected ok=%v, got ok=%v", k, testCase.ExpectedOK, ok)
+			t.Errorf("%s: Expected ok=%v, got ok=%v", k, testCase.ExpectedOK, ok)
 			continue
 		}
 		if testCase.ExpectedUser != "" && testCase.ExpectedUser != user.GetName() {
-			t.Fatalf("%s: Expected user.GetName()=%v, got %v", k, testCase.ExpectedUser, user.GetName())
+			t.Errorf("%s: Expected user.GetName()=%v, got %v", k, testCase.ExpectedUser, user.GetName())
 			continue
 		}
 	}

plugin/pkg/auth/authenticator/request/keystone/keystone_test.go

@@ -1,147 +0,0 @@
-/*
-Copyright 2014 The Kubernetes Authors All rights reserved.
-
-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 keystone
-
-import (
-	"encoding/base64"
-	"net/http"
-	"testing"
-
-	"k8s.io/kubernetes/pkg/auth/user"
-	"k8s.io/kubernetes/plugin/pkg/auth/authenticator/request/basicauth"
-)
-
-type testKeystoneAuthenticator struct {
-	User user.Info
-	OK   bool
-	Err  error
-}
-
-func (osClient *testKeystoneAuthenticator) AuthenticatePassword(username string, password string) (user.Info, bool, error) {
-
-	userPasswordMap := map[string]string{
-		"user1": "password1",
-		"user2": "password2",
-		"user3": "password3",
-		"user4": "password4",
-		"user5": "password5",
-		"user6": "password6",
-		"user7": "password7",
-		"user8": "password8",
-		"user9": "password9",
-	}
-
-	if userPasswordMap[username] == password {
-		return &user.DefaultInfo{Name: username}, true, nil
-	}
-	return nil, false, nil
-}
-
-func TestKeystoneAuth(t *testing.T) {
-
-	testCases := map[string]struct {
-		Header                string
-		keystoneAuthenticator testKeystoneAuthenticator
-
-		ExpectedCalled   bool
-		ExpectedUsername string
-		ExpectedPassword string
-
-		ExpectedUser string
-		ExpectedOK   bool
-		ExpectedErr  bool
-	}{
-		"no header": {
-			Header: "",
-		},
-		"non-basic header": {
-			Header: "Bearer foo",
-		},
-		"empty value basic header": {
-			Header: "Basic",
-		},
-		"whitespace value basic header": {
-			Header: "Basic  ",
-		},
-		"non base-64 basic header": {
-			Header:      "Basic !@#$",
-			ExpectedErr: true,
-		},
-		"malformed basic header": {
-			Header:      "Basic " + base64.StdEncoding.EncodeToString([]byte("user_without_password")),
-			ExpectedErr: true,
-		},
-		"empty password basic header": {
-			Header:     "Basic " + base64.StdEncoding.EncodeToString([]byte("user1:")),
-			ExpectedOK: false,
-		},
-		"valid basic header": {
-			Header:      "Basic " + base64.StdEncoding.EncodeToString([]byte("user1:password1:withcolon")),
-			ExpectedOK:  false,
-			ExpectedErr: false,
-		},
-		"password auth returned user": {
-			Header:           "Basic " + base64.StdEncoding.EncodeToString([]byte("user1:password1")),
-			ExpectedCalled:   true,
-			ExpectedUsername: "user1",
-			ExpectedPassword: "password1",
-			ExpectedOK:       true,
-		},
-		"password auth returned error": {
-			Header:           "Basic " + base64.StdEncoding.EncodeToString([]byte("user1:password2")),
-			ExpectedCalled:   true,
-			ExpectedUsername: "user1",
-			ExpectedPassword: "password1",
-			ExpectedErr:      false,
-			ExpectedOK:       false,
-		},
-	}
-
-	for k, testCase := range testCases {
-
-		ksAuth := testCase.keystoneAuthenticator
-
-		auth := basicauth.New(&ksAuth)
-
-		req, _ := http.NewRequest("GET", "/", nil)
-		if testCase.Header != "" {
-			req.Header.Set("Authorization", testCase.Header)
-		}
-
-		user, ok, err := auth.AuthenticateRequest(req)
-
-		if testCase.ExpectedErr && err == nil {
-			t.Errorf("%s: Expected error, got none", k)
-			continue
-		}
-		if !testCase.ExpectedErr && err != nil {
-			t.Errorf("%s: Did not expect error, got err:%v", k, err)
-			continue
-		}
-		if testCase.ExpectedOK != ok {
-			t.Errorf("%s: Expected ok=%v, got %v", k, testCase.ExpectedOK, ok)
-			continue
-		}
-
-		if testCase.ExpectedOK {
-			if testCase.ExpectedUsername != user.GetName() {
-				t.Errorf("%s: Expected user.name=%v, got %v", k, testCase.ExpectedUsername, user.GetName())
-				continue
-			}
-		}
-	}
-}