etcd-io / jetcd

etcd java client
Apache License 2.0
1.08k stars 314 forks source link

JWT token is not refreshed if etcd responds with "auth: revision in header is old" #1344

Closed Rmarian closed 3 months ago

Rmarian commented 3 months ago

Versions

Describe the bug When using jetcd with authentication enabled, if the cached JWT token becomes invalid because it's revision has become obsolete and etcd responds with "auth: revision in header is old" error, jetcd fails to refresh the token and instead reuses the same one until it expires normally.

To Reproduce

  1. Configure jetcd to use JWT token authentication
  2. Make a request to etcd to obtain a JWT token for the given credentials
  3. Update the etcd auth database revision by adding a new role/user/permission etc....

Expected behavior The instantiated jetcd client continues to work but instead on every subsequent request the error "io.grpc.StatusRuntimeException: UNKNOWN: auth: revision in header is old" is returned

Additional context Seems like the auth token validity logic must be updated in io.etcd.jetcd.support.Errors: public static boolean isInvalidTokenError(Status status) { return (status.getCode() == Code.UNAUTHENTICATED || status.getCode() == Code.UNKNOWN) && "etcdserver: invalid auth token".equals(status.getDescription()); } An extra check for "auth: revision in header is old" should be added here.

After I added this my tests worked fine.

lburgazzoli commented 3 months ago

@Rmarian, do you have time to work on a pr to fix the issue ?

Rmarian commented 3 months ago

@lburgazzoli yes, I already have the fix locally. I can open a PR soon.

Rmarian commented 3 months ago

@lburgazzoli So in the end the PR fix is a bit different than what I initially planned.

It turned out that the fact that etcd was returning "auth: revision in header is old" was a bug in 3.5.0 since fixed by https://github.com/etcd-io/etcd/pull/13308

However even after testing with 3.5.10 I got another failure "etcdserver: revision of auth store is old".

So I added this error to the retry condition as well and now all seems fine.

Looks like in the original fix, https://github.com/etcd-io/etcd/pull/13308, the condition was not added as re-tryable but I don't know why.