auth: fix NoPassWord check when add user

[YoyinZyc]

fix #11414
There is one more concern about the usage of NoPassword. If the user is created like etcdctl user add user:, should it be inferred as no password?
@jingyih PHAL

[jingyih]

Could we add an e2e test to mimic the work flow reported in the original issue? I am surprised that the bug was not caught by test.

[jingyih]

The fix looks good to me.

[codecov-io]

Codecov Report

Merging #11418 into master will decrease coverage by 0.48%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #11418      +/-   ##
==========================================
- Coverage   64.28%   63.79%   -0.49%     
==========================================
  Files         403      403              
  Lines       38070    38071       +1     
==========================================
- Hits        24472    24289     -183     
- Misses      11965    12140     +175     
- Partials     1633     1642       +9

Impacted Files	Coverage Δ
auth/store.go	45.66% <100%> (-9.64%)	⬇️
client/keys.go	68.34% <0%> (-23.12%)	⬇️
client/members.go	65.32% <0%> (-20.17%)	⬇️
pkg/transport/timeout_conn.go	60% <0%> (-20%)	⬇️
proxy/grpcproxy/register.go	69.44% <0%> (-11.12%)	⬇️
auth/simple_token.go	77.23% <0%> (-9.76%)	⬇️
clientv3/leasing/util.go	91.66% <0%> (-6.67%)	⬇️
proxy/grpcproxy/watcher.go	89.79% <0%> (-4.09%)	⬇️
clientv3/concurrency/election.go	78.12% <0%> (-3.91%)	⬇️
... and 20 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f81002...5127cfb. Read the comment docs.

[YoyinZyc]

Could we add an e2e test to mimic the work flow reported in the original issue? I am surprised that the bug was not caught by test.

SGTM. I will add more tests for NoPassword

[jingyih]

Can we explore the option of modifying the existing testV3CurlAuth e2e test to include both w/ and w/o user option cases?

[jingyih]

cc @mitake

[mitake]

LGTM, thanks @YoyinZyc ! If @jingyih is ok with TestV3CurlAuthWithoutOption, I think it's ok to merge (I have no strong opinions about it).

It is not limited to this bug but I feel we should have a testing framework which can detect issues related to protobuf update (it might be useful for validating upgrading and downgrading an etcd cluster with different versions).

[jingyih]

It is not limited to this bug but I feel we should have a testing framework which can detect issues related to protobuf update (it might be useful for validating upgrading and downgrading an etcd cluster with different versions).

Today there are some upgrade e2e tests. I believe in #11362 we are adding some downgrade e2e tests. But in these tests we do not exercise all possible APIs. So I agree, having a framework that supports version skew sounds good to me.

[jingyih]

Thanks for adding a new test case. I think this is fine. Let's also backport to 3.4.