server/auth: fix auth panic bug when user changes password

Signed-off-by: tangcong <tangcong506@foxmail.com>
etcd-io · Mar 12, 2023 · ad72900 · ad72900
1 parent 5b34975
commit ad72900
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 3 deletions.
diff --git a/server/auth/store.go b/server/auth/store.go
@@ -493,7 +493,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
@@ -114,12 +114,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
+	addUserWithNoOption(as)
+
 	tearDown := func(_ *testing.T) {
 		as.Close()
 	}
 	return as, tearDown
 }
 
+func addUserWithNoOption(as *authStore) {
+	tx := as.be.BatchTx()
+	tx.Lock()
+	defer tx.Unlock()
+	tx.UnsafePutUser(&authpb.User{
+		Name:     []byte("foo-no-user-options"),
+		Password: []byte("bar"),
+	})
+	as.commitRevision(tx)
+	as.refreshRangePermCache(tx)
+}
+
 func enableAuthAndCreateRoot(as *authStore) error {
 	_, err := as.UserAdd(&pb.AuthUserAddRequest{Name: "root", HashedPassword: encodePassword("root"), Options: &authpb.UserAddOptions{NoPassword: false}})
 	if err != nil {
@@ -191,8 +207,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")
@@ -323,6 +339,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) {