From 0a4ee958c7f203644975359aca18bb4d021dd8c4 Mon Sep 17 00:00:00 2001 From: Kris Date: Fri, 6 Nov 2015 10:19:01 -0800 Subject: [PATCH] Use http's basic auth instead of manual encoding --- pkg/client/unversioned/request_test.go | 37 +---- pkg/client/unversioned/transport_test.go | 3 +- .../request/basicauth/basicauth.go | 24 +-- .../request/basicauth/basicauth_test.go | 42 ++--- .../request/keystone/keystone_test.go | 147 ------------------ 5 files changed, 18 insertions(+), 235 deletions(-) delete mode 100644 plugin/pkg/auth/authenticator/request/keystone/keystone_test.go diff --git a/pkg/client/unversioned/request_test.go b/pkg/client/unversioned/request_test.go index 1400756cc10..877f6f437cf 100644 --- a/pkg/client/unversioned/request_test.go +++ b/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) { - foundAuth, found := authFromReq(r) +func checkAuth(t *testing.T, expectedUser, expectedPass string, r *http.Request) { + user, pass, found := r.BasicAuth() if !found { t.Errorf("no auth found") - } else if e, a := expect, foundAuth; !api.Semantic.DeepDerivative(e, a) { - t.Fatalf("Wrong basic auth: wanted %#v, got %#v", e, a) + } else if user != expectedUser || pass != expectedPass { + 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!") diff --git a/pkg/client/unversioned/transport_test.go b/pkg/client/unversioned/transport_test.go index 8ba609110f4..ed03af3e467 100644 --- a/pkg/client/unversioned/transport_test.go +++ b/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) } } diff --git a/plugin/pkg/auth/authenticator/request/basicauth/basicauth.go b/plugin/pkg/auth/authenticator/request/basicauth/basicauth.go index b9ef3b4cf6f..62d78a7254e 100644 --- a/plugin/pkg/auth/authenticator/request/basicauth/basicauth.go +++ b/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")) - if auth == "" { + username, password, found := req.BasicAuth() + 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) } diff --git a/plugin/pkg/auth/authenticator/request/basicauth/basicauth_test.go b/plugin/pkg/auth/authenticator/request/basicauth/basicauth_test.go index 951ed974296..1030f75fe6a 100644 --- a/plugin/pkg/auth/authenticator/request/basicauth/basicauth_test.go +++ b/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": { - 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, - }, + "no auth": {}, "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 != "" { - req.Header.Set("Authorization", testCase.Header) + if testCase.ExpectedUsername != "" || testCase.ExpectedPassword != "" { + 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 } } diff --git a/plugin/pkg/auth/authenticator/request/keystone/keystone_test.go b/plugin/pkg/auth/authenticator/request/keystone/keystone_test.go deleted file mode 100644 index 6437b40dc4e..00000000000 --- a/plugin/pkg/auth/authenticator/request/keystone/keystone_test.go +++ /dev/null @@ -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 - } - } - } -}