UserAdd issue / question

[mrtvfuencxozd]

Hi,
Just starting to use etcd, I'm facing an issue when adding user.
I end up unable to log-in.

I think I found the issue, but I'd like to check that I'm not wrong somewhere.

here is the context:
I'm running etcd v3.4.3 (docker gcr.io/etcd-development/etcd:v3.4.3)
For some reason still not clear to me, I haven't managed to get the client from v3.4 using go modules so clientv3 I'm using if from v3.3.

In this version, there is no UserAddWithOptions, only UserAdd
Because of this, etcd ends up adding a user without options.
Looking at the code involved, I think something is wrong, and if I understand it correctly, this would explain my problem.

Code is

func (as *authStore) UserAdd(r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse, error) {
	if len(r.Name) == 0 {
		return nil, ErrUserEmpty
	}

	var hashed []byte
	var err error
	if r.Options != nil && !r.Options.NoPassword {
		hashed, err = bcrypt.GenerateFromPassword([]byte(r.Password), as.bcryptCost)
		if err != nil {
			if as.lg != nil {
				as.lg.Warn(
					"failed to bcrypt hash password",
					zap.String("user-name", r.Name),
					zap.Error(err),
				)
			} else {
				plog.Errorf("failed to hash password: %s", err)
			}
			return nil, err
		}
	}

	tx := as.be.BatchTx()
	tx.Lock()
	defer tx.Unlock()

	user := getUser(as.lg, tx, r.Name)
	if user != nil {
		return nil, ErrUserAlreadyExist
	}

	options := r.Options
	if options == nil {
		options = &authpb.UserAddOptions{
			NoPassword: false,
		}
	}
	newUser := &authpb.User{
		Name:     []byte(r.Name),
		Password: hashed,
		Options:  options,
	}
	fmt.Printf("putting used %#v\n", *newUser)
	putUser(as.lg, tx, newUser)

	as.commitRevision(tx)

	if as.lg != nil {
		as.lg.Info("added a user", zap.String("user-name", r.Name))
	} else {
		plog.Noticef("added a new user: %s", r.Name)
	}
	return &pb.AuthUserAddResponse{}, nil
}

The top part, when there is no options (old default) does not go through bcrypt and as such don't save the hashed password.
However the bottom part considers that no option means NoPassword: false

As such, request to add user using the old UsedAdd call leads to an existing user with an emty hash.

I think that either the top if should be:

    if r.Options == nil || !r.Options.NoPassword {

or the options should be earlier unmarshaled with default value.

Does this make sense or am I wrong somewhere ?

Thanks,
Pascal

[mrtvfuencxozd]

By the way,
with the change proposed above, my basic tests are working now. I wanted to check that this was the way to go.
If this is, I can create a PR.

[jingyih]

Thanks for the digging! You are correct. This is a bug. There is already a PR (which does exactly what you proposed) to fix this:) We will get it merged in master branch and then backport to 3.4.

#11418

[mrtvfuencxozd]

Right,
I missed it while searching.

Thanks.

[jingyih]

Fixed by #11418.