Address review feedback; move storage of these values to the expiration manager

http/logical_test.go

@@ -126,7 +126,6 @@ func TestLogical_StandbyRedirect(t *testing.T) {
 			"orphan":       true,
 			"id":           root,
 			"ttl":          float64(0),
-			"last_renewal_time": float64(0),
 		},
 		"warnings": nilWarnings,
 		"auth":     nil,

vault/expiration.go

@@ -295,6 +295,7 @@ func (m *ExpirationManager) Renew(leaseID string, increment time.Duration) (*log
 	le.Data = resp.Data
 	le.Secret = resp.Secret
 	le.ExpireTime = resp.Secret.ExpirationTime()
+	le.LastRenewalTime = time.Now().UTC()
 	if err := m.persistEntry(le); err != nil {
 		return nil, err
 	}
@@ -346,6 +347,7 @@ func (m *ExpirationManager) RenewToken(source string, token string,
 	// Update the lease entry
 	le.Auth = resp.Auth
 	le.ExpireTime = resp.Auth.ExpirationTime()
+	le.LastRenewalTime = time.Now().UTC()
 	if err := m.persistEntry(le); err != nil {
 		return nil, err
 	}
@@ -424,6 +426,40 @@ func (m *ExpirationManager) RegisterAuth(source string, auth *logical.Auth) erro
 	return nil
 }
 
+// FetchLeaseTimesByToken is a helper function to use token values to compute
+// the leaseID, rather than pushing that logic back into the token store.
+func (m *ExpirationManager) FetchLeaseTimesByToken(source, token string) (*leaseEntry, error) {
+	defer metrics.MeasureSince([]string{"expire", "fetch-lease-times-by-token"}, time.Now())
+
+	// Compute the Lease ID
+	leaseID := path.Join(source, m.tokenStore.SaltID(token))
+	return m.FetchLeaseTimes(leaseID)
+}
+
+// FetchLeaseTimes is used to fetch the issue time, expiration time, and last
+// renewed time of a lease entry. It returns a leaseEntry itself, but with only
+// those values copied over.
+func (m *ExpirationManager) FetchLeaseTimes(leaseID string) (*leaseEntry, error) {
+	defer metrics.MeasureSince([]string{"expire", "fetch-lease-times"}, time.Now())
+
+	// Load the entry
+	le, err := m.loadEntry(leaseID)
+	if err != nil {
+		return nil, err
+	}
+	if le == nil {
+		return nil, nil
+	}
+
+	ret := &leaseEntry{
+		IssueTime:       le.IssueTime,
+		ExpireTime:      le.ExpireTime,
+		LastRenewalTime: le.LastRenewalTime,
+	}
+
+	return ret, nil
+}
+
 // updatePending is used to update a pending invocation for a lease
 func (m *ExpirationManager) updatePending(le *leaseEntry, leaseTotal time.Duration) {
 	m.pendingLock.Lock()
@@ -641,6 +677,7 @@ type leaseEntry struct {
 	Auth            *logical.Auth          `json:"auth"`
 	IssueTime       time.Time              `json:"issue_time"`
 	ExpireTime      time.Time              `json:"expire_time"`
+	LastRenewalTime time.Time              `json:"last_renewal_time"`
 }
 
 // encode is used to JSON encode the lease entry

vault/expiration_test.go

@@ -852,6 +852,7 @@ func TestExpiration_PersistLoadDelete(t *testing.T) {
 		},
 		IssueTime:       time.Now().UTC(),
 		ExpireTime:      time.Now().UTC(),
+		LastRenewalTime: time.Time{}.UTC(),
 	}
 	if err := exp.persistEntry(le); err != nil {
 		t.Fatalf("err: %v", err)
@@ -862,7 +863,7 @@ func TestExpiration_PersistLoadDelete(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 	if !reflect.DeepEqual(out, le) {
-		t.Fatalf("out: %#v expect: %#v", out, le)
+		t.Fatalf("\nout: %#v\nexpect: %#v\n", out, le)
 	}
 
 	err = exp.deleteEntry("foo/bar/1234")

vault/token_store.go

@@ -274,7 +274,6 @@ type TokenEntry struct {
 	DisplayName  string            // Used for operators to be able to associate with the source
 	NumUses      int               // Used to restrict the number of uses (zero is unlimited). This is to support one-time-tokens (generalized).
 	CreationTime int64             // Time of token creation
-	LastRenewalTime int64             // Time that the token was last renewed; 0 means never renewed
 	TTL          time.Duration     // Duration set when token was created
 }
 
@@ -316,7 +315,8 @@ func (ts *TokenStore) create(entry *TokenEntry) error {
 	return ts.storeCommon(entry, true)
 }
 
-// Store is used to store an updated token entry.
+// Store is used to store an updated token entry without writing the
+// secondary index.
 func (ts *TokenStore) store(entry *TokenEntry) error {
 	defer metrics.MeasureSince([]string{"token", "store"}, time.Now())
 	return ts.storeCommon(entry, false)
@@ -834,7 +834,6 @@ func (ts *TokenStore) handleLookup(
 			"num_uses":      out.NumUses,
 			"orphan":        false,
 			"creation_time": int(out.CreationTime),
-			"last_renewal_time": int(out.LastRenewalTime),
 			"ttl":           int(out.TTL.Seconds()),
 		},
 	}
@@ -843,6 +842,15 @@ func (ts *TokenStore) handleLookup(
 		resp.Data["orphan"] = true
 	}
 
+	// Fetch the last renewal time
+	leaseTimes, err := ts.expiration.FetchLeaseTimesByToken(out.Path, out.ID)
+	if err != nil {
+		return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest
+	}
+	if leaseTimes != nil && !leaseTimes.LastRenewalTime.IsZero() {
+		resp.Data["last_renewal_time"] = leaseTimes.LastRenewalTime.Unix()
+	}
+
 	return resp, nil
 }
 
@@ -882,11 +890,6 @@ func (ts *TokenStore) handleRenew(
 		return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest
 	}
 
-	te.LastRenewalTime = time.Now().Unix()
-	if err := ts.store(te); err != nil {
-		return logical.ErrorResponse("error saving renewed token"), fmt.Errorf("unable to save renewed token: %v", err)
-	}
-
 	// Generate the response
 	resp := &logical.Response{
 		Auth: auth,

vault/token_store_test.go

@@ -869,7 +869,6 @@ func TestTokenStore_HandleRequest_Lookup(t *testing.T) {
 		"orphan":       true,
 		"num_uses":     0,
 		"ttl":          0,
-		"last_renewal_time": 0,
 	}
 
 	if resp.Data["creation_time"].(int) == 0 {
@@ -901,7 +900,6 @@ func TestTokenStore_HandleRequest_Lookup(t *testing.T) {
 		"orphan":       false,
 		"num_uses":     0,
 		"ttl":          3600,
-		"last_renewal_time": 0,
 	}
 
 	if resp.Data["creation_time"].(int) == 0 {
@@ -932,7 +930,7 @@ func TestTokenStore_HandleRequest_Lookup(t *testing.T) {
 		t.Fatalf("bad: %#v", resp)
 	}
 
-	if resp.Data["last_renewal_time"].(int) == 0 {
+	if resp.Data["last_renewal_time"].(int64) == 0 {
 		t.Fatalf("last_renewal_time was zero")
 	}
 }
@@ -998,7 +996,6 @@ func TestTokenStore_HandleRequest_LookupSelf(t *testing.T) {
 		"orphan":       true,
 		"num_uses":     0,
 		"ttl":          0,
-		"last_renewal_time": 0,
 	}
 
 	if resp.Data["creation_time"].(int) == 0 {
@@ -1007,7 +1004,7 @@ func TestTokenStore_HandleRequest_LookupSelf(t *testing.T) {
 	delete(resp.Data, "creation_time")
 
 	if !reflect.DeepEqual(resp.Data, exp) {
-		t.Fatalf("bad: %#v exp: %#v", resp.Data, exp)
+		t.Fatalf("bad:\ngot %#v\nexpected: %#v\n", resp.Data, exp)
 	}
 }