Merge pull request #41775 from CaoShuFeng/username_password_401_403

Automatic merge from submit-queue

Ensure invalid username/password returns 401 error, not 403

If a user attempts to use basic auth, and the username/password combination
is rejected, the authenticator should return an error. This distinguishes
requests that did not provide username/passwrod (and are unauthenticated
without error) from ones that attempted to, and failed.

Related to:
https://github.com/kubernetes/kubernetes/pull/39408

pkg/kubeapiserver/authenticator/config.go

@@ -209,7 +209,8 @@ func (config AuthenticatorConfig) New() (authenticator.Request, *spec.SecurityDe
 	authenticator = group.NewAuthenticatedGroupAdder(authenticator)
 
 	if config.Anonymous {
-		// If the authenticator chain returns an error, return an error (don't consider a bad bearer token anonymous).
+		// If the authenticator chain returns an error, return an error (don't consider a bad bearer token
+		// or invalid username/password combination anonymous).
 		authenticator = union.NewFailOnError(authenticator, anonymous.NewAuthenticator())
 	}
 

staging/src/k8s.io/apiserver/plugin/pkg/authenticator/request/basicauth/basicauth.go

@@ -17,6 +17,7 @@ limitations under the License.
 package basicauth
 
 import (
+	"errors"
 	"net/http"
 
 	"k8s.io/apiserver/pkg/authentication/authenticator"
@@ -33,11 +34,21 @@ func New(auth authenticator.Password) *Authenticator {
 	return &Authenticator{auth}
 }
 
+var errInvalidAuth = errors.New("invalid username/password combination")
+
 // AuthenticateRequest authenticates the request using the "Authorization: Basic" header in the request
 func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool, error) {
 	username, password, found := req.BasicAuth()
 	if !found {
 		return nil, false, nil
 	}
-	return a.auth.AuthenticatePassword(username, password)
+
+	user, ok, err := a.auth.AuthenticatePassword(username, password)
+
+	// If the password authenticator didn't error, provide a default error
+	if !ok && err == nil {
+		err = errInvalidAuth
+	}
+
+	return user, ok, err
 }

staging/src/k8s.io/apiserver/plugin/pkg/authenticator/request/basicauth/basicauth_test.go

@@ -60,11 +60,13 @@ func TestBasicAuth(t *testing.T) {
 			ExpectedCalled:   true,
 			ExpectedUsername: "user_with_empty_password",
 			ExpectedPassword: "",
+			ExpectedErr:      true,
 		},
 		"valid basic header": {
 			ExpectedCalled:   true,
 			ExpectedUsername: "myuser",
 			ExpectedPassword: "mypassword:withcolon",
+			ExpectedErr:      true,
 		},
 		"password auth returned user": {
 			Password:         testPassword{User: &user.DefaultInfo{Name: "returneduser"}, OK: true},