Skip to content

Commit

Permalink
clientv3: do not refresh token when using TLS CommonName based authen…
Browse files Browse the repository at this point in the history
…tication

When users use the TLS CommonName based authentication, the
authTokenBundle is always nil. But it's possible for the clients
to get `rpctypes.ErrAuthOldRevision` response when the clients
concurrently modify auth data (e.g, addUser, deleteUser etc.).
In this case, there is no need to refresh the token; instead the
clients just need to retry the operations (e.g. Put, Delete etc).

Signed-off-by: Benjamin Wang <[email protected]>
  • Loading branch information
ahrtr committed Nov 16, 2022
1 parent 5f387e6 commit 69aace2
Showing 1 changed file with 20 additions and 11 deletions.
31 changes: 20 additions & 11 deletions client/v3/retry_interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,7 @@ func (c *Client) unaryClientInterceptor(optFuncs ...retryOption) grpc.UnaryClien
continue
}
if c.shouldRefreshToken(lastErr, callOpts) {
// clear auth token before refreshing it.
// call c.Auth.Authenticate with an invalid token will always fail the auth check on the server-side,
// if the server has not apply the patch of pr #12165 (https://github.com/etcd-io/etcd/pull/12165)
// and a rpctypes.ErrInvalidAuthToken will recursively call c.getToken until system run out of resource.
c.authTokenBundle.UpdateAuthToken("")

gterr := c.getToken(ctx)
gterr := c.refreshToken(ctx)
if gterr != nil {
c.GetLogger().Warn(
"retrying of unary invoker failed to fetch new auth token",
Expand Down Expand Up @@ -161,6 +155,24 @@ func (c *Client) shouldRefreshToken(err error, callOpts *options) bool {
(rpctypes.Error(err) == rpctypes.ErrInvalidAuthToken || rpctypes.Error(err) == rpctypes.ErrAuthOldRevision)
}

func (c *Client) refreshToken(ctx context.Context) error {
if c.authTokenBundle == nil {
// c.authTokenBundle will be initialized only when
// c.Username != "" && c.Password != "".
//
// When users use the TLS CommonName based authentication, the
// authTokenBundle is always nil. But it's possible for the clients
// to get `rpctypes.ErrAuthOldRevision` response when the clients
// concurrently modify auth data (e.g, addUser, deleteUser etc.).
// In this case, there is no need to refresh the token; instead the
// clients just need to retry the operations (e.g. Put, Delete etc).
return nil
}
// clear auth token before refreshing it.
c.authTokenBundle.UpdateAuthToken("")
return c.getToken(ctx)
}

// type serverStreamingRetryingStream is the implementation of grpc.ClientStream that acts as a
// proxy to the underlying call. If any of the RecvMsg() calls fail, it will try to reestablish
// a new ClientStream according to the retry policy.
Expand Down Expand Up @@ -259,10 +271,7 @@ func (s *serverStreamingRetryingStream) receiveMsgAndIndicateRetry(m interface{}
return true, err
}
if s.client.shouldRefreshToken(err, s.callOpts) {
// clear auth token to avoid failure when call getToken
s.client.authTokenBundle.UpdateAuthToken("")

gterr := s.client.getToken(s.ctx)
gterr := s.client.refreshToken(s.ctx)
if gterr != nil {
s.client.lg.Warn("retry failed to fetch new auth token", zap.Error(gterr))
return false, err // return the original error for simplicity
Expand Down

0 comments on commit 69aace2

Please sign in to comment.