Merge pull request #15512 from engow/automated-cherry-pick-of-#15432-…

…origin-release-3.5 [3.5] server/auth: fix auth panic bug when user changes password
etcd-io · Apr 7, 2023 · 5872b80 · 5872b80
2 parents 4501fd8 + f7ac9df
commit 5872b80
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 3 deletions.
diff --git a/server/auth/store.go b/server/auth/store.go
@@ -479,7 +479,8 @@ func (as *authStore) UserChangePassword(r *pb.AuthUserChangePasswordRequest) (*p
 	var password []byte
 	var err error
 
-	if !user.Options.NoPassword {
+	// Backward compatible with old versions of etcd, user options is nil
+	if user.Options == nil || !user.Options.NoPassword {
 		password, err = as.selectPassword(r.Password, r.HashedPassword)
 		if err != nil {
 			return nil, ErrNoPasswordUser

diff --git a/server/auth/store_test.go b/server/auth/store_test.go
@@ -125,13 +125,28 @@ func setupAuthStore(t *testing.T) (store *authStore, teardownfunc func(t *testin
 		t.Fatal(err)
 	}
 
+	// The UserAdd function cannot generate old etcd version user data (user's option is nil)
+	// add special users through the underlying interface
+	err = addUserWithNoOption(as)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	tearDown := func(_ *testing.T) {
 		b.Close()
 		as.Close()
 	}
 	return as, tearDown
 }
 
+func addUserWithNoOption(as *authStore) error {
+	_, err := as.UserAdd(&pb.AuthUserAddRequest{Name: "foo-no-user-options", Password: "bar"})
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
 func enableAuthAndCreateRoot(as *authStore) error {
 	_, err := as.UserAdd(&pb.AuthUserAddRequest{Name: "root", HashedPassword: encodePassword("root"), Options: &authpb.UserAddOptions{NoPassword: false}})
 	if err != nil {
@@ -203,8 +218,8 @@ func TestRecoverWithEmptyRangePermCache(t *testing.T) {
 		t.Fatalf("expected auth enabled got disabled")
 	}
 
-	if len(as.rangePermCache) != 2 {
-		t.Fatalf("rangePermCache should have permission information for 2 users (\"root\" and \"foo\"), but has %d information", len(as.rangePermCache))
+	if len(as.rangePermCache) != 3 {
+		t.Fatalf("rangePermCache should have permission information for 3 users (\"root\" and \"foo\",\"foo-no-user-options\"), but has %d information", len(as.rangePermCache))
 	}
 	if _, ok := as.rangePermCache["root"]; !ok {
 		t.Fatal("user \"root\" should be created by setupAuthStore() but doesn't exist in rangePermCache")
@@ -335,6 +350,12 @@ func TestUserChangePassword(t *testing.T) {
 	if err != ErrUserNotFound {
 		t.Fatalf("expected %v, got %v", ErrUserNotFound, err)
 	}
+
+	// change a user（user option is nil) password
+	_, err = as.UserChangePassword(&pb.AuthUserChangePasswordRequest{Name: "foo-no-user-options", HashedPassword: encodePassword("bar")})
+	if err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestRoleAdd(t *testing.T) {