Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clientv3 returns ErrAuthOldRevision when changing other user password in jwt-auth-token #12147

Closed
LeoYang90 opened this issue Jul 17, 2020 · 3 comments
Labels

Comments

@LeoYang90
Copy link

LeoYang90 commented Jul 17, 2020

Convey("Given a fresh clientV3 with auth", t, func() {
    cli, err := clientv3.New(clientv3.Config{
			Endpoints:   conf.Endpoints,
			DialTimeout: conf.DialTimeout,
			Username:    "root",
			Password:    "123",
		})
    So(err, ShouldBeNil)
    defer cli.Close()

    cliUser1, err := clientv3.New(clientv3.Config{
			Endpoints:   conf.Endpoints,
			DialTimeout: conf.DialTimeout,
			Username:    "user1",
			Password:    "user1-123",
		})
    So(err, ShouldBeNil)
    defer cliUser1.Close()

    cliUser2, err := clientv3.New(clientv3.Config{
			Endpoints:   conf.Endpoints,
			DialTimeout: conf.DialTimeout,
			Username:    "user2",
			Password:    "user2-123",
		})
    So(err, ShouldBeNil)
    defer cliUser2.Close()

    Convey("user1 change password", func() {
	  _, err = cli.UserChangePassword(context.TODO(), "user1", "user1-345")
	  So(err, ShouldBeNil)

	  Convey("user1 cli should throw ErrAuthFailed, user2 cli should not be affected", func() {
		_, err = cliUser1.Put(context.TODO(),"foo", "bar")
		So(err, ShouldResemble, rpctypes.ErrAuthFailed)

		_, err = cliUser2.Put(context.TODO(),"foo1", "bar1")
		So(err, ShouldBeNil)
	  })
    })
})

I think cliUser2.Put should not be affected by changing password. But the test returns ErrAuthOldRevision.

Clientv3 can automatically retry when get a ErrInvalidAuthToken, but server return ErrAuthOldRevision:

image

So, should we change it to ErrInvalidAuthToken?

@xqzhang2015
Copy link

@xiang90 @mitake Do you have any idea about this question?

I'm faced with the same issue. When receiving ErrAuthOldRevision, which may be from updating other users or roles, the client needs to update the auth token.

I have some optional proposals:

  • make getToken() as public and developer could add an interceptor for handling ErrAuthOldRevision.
  • make unaryClientInterceptor() general, which could handle ErrInvalidAuthToken, ErrAuthOldRevision, and any other errors. Options:
    • passing error list
    • passing error handling function list
  • add a new optional interceptor for handling ErrAuthOldRevision.

Do you guys have any comments? Thanks ahead.

@LeoYang90 FYI

@mitake
Copy link
Contributor

mitake commented Aug 30, 2020

Thanks for reporting @LeoYang90 . Yes auth store has one shared revision for all auth information so the error code can be returned to a user which isn't changed by the operation. Similar issue happens to Authenticate() RPC call (will be fixed in #10408) and I'm planning to fix them. We have some ongoing client side changes (e.g. #12165) so it might need some time, sorry for keeping you waiting.
I think your 2nd approach should be used @xqzhang2015

make unaryClientInterceptor() general, which could handle ErrInvalidAuthToken, ErrAuthOldRevision, and any other errors. Options:
passing error list
passing error handling function list

@stale
Copy link

stale bot commented Nov 28, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 21 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 28, 2020
@stale stale bot closed this as completed Dec 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants