From ad72900dad23d8013ec5ddaf9fb575c4e62b1b61 Mon Sep 17 00:00:00 2001 From: tangcong Date: Thu, 9 Mar 2023 12:21:50 +0800 Subject: [PATCH] server/auth: fix auth panic bug when user changes password Signed-off-by: tangcong --- server/auth/store.go | 3 ++- server/auth/store_test.go | 26 ++++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/server/auth/store.go b/server/auth/store.go index 40262c76d62..b4bf0607314 100644 --- 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 index 0287c2e53d7..c8d5d3f9e82 100644 --- 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) {