Merge pull request #45235 from deads2k/auth-03-remove-header

Automatic merge from submit-queue

remove bearer token from headers after we consume it

Updates the bearer token authenticator to remove the bearer token from the request headers after it is consumed.  Nothing else in the stack should try to use it and we don't want to accidentally leak it somewhere.

@liggitt @kubernetes/sig-auth-pr-reviews

staging/src/k8s.io/apiserver/pkg/authentication/request/bearertoken/bearertoken.go

@@ -53,6 +53,11 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool,
 	}
 
 	user, ok, err := a.auth.AuthenticateToken(token)
+	// if we authenticated successfully, go ahead and remove the bearer token so that no one
+	// is ever tempted to use it inside of the API server
+	if ok {
+		req.Header.Del("Authorization")
+	}
 
 	// If the token authenticator didn't error, provide a default error
 	if !ok && err == nil {

staging/src/k8s.io/apiserver/pkg/authentication/request/bearertoken/bearertoken_test.go

@@ -19,6 +19,7 @@ package bearertoken
 import (
 	"errors"
 	"net/http"
+	"reflect"
 	"testing"
 
 	"k8s.io/apiserver/pkg/authentication/authenticator"
@@ -103,3 +104,94 @@ func TestAuthenticateRequestBadValue(t *testing.T) {
 		}
 	}
 }
+
+func TestBearerToken(t *testing.T) {
+	tests := map[string]struct {
+		AuthorizationHeaders []string
+		TokenAuth            authenticator.Token
+
+		ExpectedUserName             string
+		ExpectedOK                   bool
+		ExpectedErr                  bool
+		ExpectedAuthorizationHeaders []string
+	}{
+		"no header": {
+			AuthorizationHeaders:         nil,
+			ExpectedUserName:             "",
+			ExpectedOK:                   false,
+			ExpectedErr:                  false,
+			ExpectedAuthorizationHeaders: nil,
+		},
+		"empty header": {
+			AuthorizationHeaders:         []string{""},
+			ExpectedUserName:             "",
+			ExpectedOK:                   false,
+			ExpectedErr:                  false,
+			ExpectedAuthorizationHeaders: []string{""},
+		},
+		"non-bearer header": {
+			AuthorizationHeaders:         []string{"Basic 123"},
+			ExpectedUserName:             "",
+			ExpectedOK:                   false,
+			ExpectedErr:                  false,
+			ExpectedAuthorizationHeaders: []string{"Basic 123"},
+		},
+		"empty bearer token": {
+			AuthorizationHeaders:         []string{"Bearer "},
+			ExpectedUserName:             "",
+			ExpectedOK:                   false,
+			ExpectedErr:                  false,
+			ExpectedAuthorizationHeaders: []string{"Bearer "},
+		},
+		"valid bearer token removing header": {
+			AuthorizationHeaders:         []string{"Bearer 123"},
+			TokenAuth:                    authenticator.TokenFunc(func(t string) (user.Info, bool, error) { return &user.DefaultInfo{Name: "myuser"}, true, nil }),
+			ExpectedUserName:             "myuser",
+			ExpectedOK:                   true,
+			ExpectedErr:                  false,
+			ExpectedAuthorizationHeaders: nil,
+		},
+		"invalid bearer token": {
+			AuthorizationHeaders:         []string{"Bearer 123"},
+			TokenAuth:                    authenticator.TokenFunc(func(t string) (user.Info, bool, error) { return nil, false, nil }),
+			ExpectedUserName:             "",
+			ExpectedOK:                   false,
+			ExpectedErr:                  true,
+			ExpectedAuthorizationHeaders: []string{"Bearer 123"},
+		},
+		"error bearer token": {
+			AuthorizationHeaders:         []string{"Bearer 123"},
+			TokenAuth:                    authenticator.TokenFunc(func(t string) (user.Info, bool, error) { return nil, false, errors.New("error") }),
+			ExpectedUserName:             "",
+			ExpectedOK:                   false,
+			ExpectedErr:                  true,
+			ExpectedAuthorizationHeaders: []string{"Bearer 123"},
+		},
+	}
+
+	for k, tc := range tests {
+		req, _ := http.NewRequest("GET", "/", nil)
+		for _, h := range tc.AuthorizationHeaders {
+			req.Header.Add("Authorization", h)
+		}
+
+		bearerAuth := New(tc.TokenAuth)
+		u, ok, err := bearerAuth.AuthenticateRequest(req)
+		if tc.ExpectedErr != (err != nil) {
+			t.Errorf("%s: Expected err=%v, got %v", k, tc.ExpectedErr, err)
+			continue
+		}
+		if ok != tc.ExpectedOK {
+			t.Errorf("%s: Expected ok=%v, got %v", k, tc.ExpectedOK, ok)
+			continue
+		}
+		if ok && u.GetName() != tc.ExpectedUserName {
+			t.Errorf("%s: Expected username=%v, got %v", k, tc.ExpectedUserName, u.GetName())
+			continue
+		}
+		if !reflect.DeepEqual(req.Header["Authorization"], tc.ExpectedAuthorizationHeaders) {
+			t.Errorf("%s: Expected headers=%#v, got %#v", k, tc.ExpectedAuthorizationHeaders, req.Header["Authorization"])
+			continue
+		}
+	}
+}