merge fix/restore-sets-lastrenewal (8063b88634) #94
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/restore-sets-lastrenewal"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
defaultClient.Restore() assigned c.authSecret and the client token but never assigned c.lastRenewal, leaving it at its zero value. lastRenewal has only two other writers -- Login() and the renewal watcher -- and neither runs on the restore path. checkExpiry() consumes lastRenewal as an ABSOLUTE timestamp: ts := time.Unix(c.lastRenewal, 0).Add(horizon) return time.Now().After(ts), nil With lastRenewal == 0 the horizon is computed from the Unix epoch, i.e. 1970-01-01 + TTL, so time.Now().After(ts) is unconditionally true for any realistic TTL. Every client restored from storage was reported expired. The sentinel that would have caught this is bypassed: the exported CheckExpiry() explicitly rejects lastRenewal == 0 with "never logged in", but Validate() calls the unexported checkExpiry() directly and so never sees the guard. Consequence: with Persist enabled, an operator restart runs RestoreAll -> restoreClient -> NewClientFromStorageEntry -> Restore() and caches the client. The next Get() calls Validate(), which reports "client token expired", evicts the entry from the cache and performs a full re-login. The client-persistence feature silently never took effect across restarts. Anchor lastRenewal to time.Now() at restore, matching Login(). The storage entry carries no renewal timestamp of its own, so the restore instant is the only available anchor and is the conservative choice (it can only under-estimate remaining TTL, never over-estimate it). Adds client_test.go, the first test in this package beyond utils_test.go. The tests are offline: Restore() and checkExpiry() are local state transitions, and a non-renewable auth secret starts no lifetime watcher, so no fake server is needed. Coverage: lastRenewal is set, a freshly restored 24h token is not expired via both checkExpiry() and CheckExpiry(), Validate() succeeds on a restored client, and an elapsed TTL is still correctly reported as expired.