Remove expired auth tokens from cache (#3768)

* Remove expired auth tokens from cache

internal/cmd/commands/search/search_test.go

@@ -50,9 +50,10 @@ func (r *testCommander) ReadTokenFromKeyring(k, a string) *authtokens.AuthToken
 func TestSearch(t *testing.T) {
 	ctx := context.Background()
 	at := &authtokens.AuthToken{
-		Id:     "at_1",
-		UserId: "user_1",
-		Token:  "at_1_token",
+		Id:             "at_1",
+		UserId:         "user_1",
+		Token:          "at_1_token",
+		ExpirationTime: time.Now().Add(time.Minute),
 	}
 	cmd := &testCommander{t: t, at: at}
 

internal/daemon/cache/repository_refresh.go

@@ -102,7 +102,7 @@ func (r *Repository) cleanAndPickAuthTokens(ctx context.Context, u *user) (map[A
 // targets from a boundary address.
 func (r *Repository) Refresh(ctx context.Context, opt ...Option) error {
 	const op = "cache.(Repository).Refresh"
-	if err := r.cleanOrphanedAuthTokens(ctx); err != nil {
+	if err := r.cleanExpiredOrOrphanedAuthTokens(ctx); err != nil {
 		return errors.Wrap(ctx, err, op)
 	}
 

internal/daemon/cache/repository_refresh_test.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/boundary/api/authtokens"
 	"github.com/hashicorp/boundary/api/sessions"
@@ -36,26 +37,30 @@ func TestCleanAndPickTokens(t *testing.T) {
 	boundaryAddr := "address"
 	u1 := &user{Id: "u1", Address: boundaryAddr}
 	at1a := &authtokens.AuthToken{
-		Id:     "at_1a",
-		Token:  "at_1a_token",
-		UserId: u1.Id,
+		Id:             "at_1a",
+		Token:          "at_1a_token",
+		UserId:         u1.Id,
+		ExpirationTime: time.Now().Add(time.Minute),
 	}
 	at1b := &authtokens.AuthToken{
-		Id:     "at_1b",
-		Token:  "at_1b_token",
-		UserId: u1.Id,
+		Id:             "at_1b",
+		Token:          "at_1b_token",
+		UserId:         u1.Id,
+		ExpirationTime: time.Now().Add(time.Minute),
 	}
 
 	keyringOnlyUser := &user{Id: "keyringUser", Address: boundaryAddr}
 	keyringAuthToken1 := &authtokens.AuthToken{
-		Id:     "at_2a",
-		Token:  "at_2a_token",
-		UserId: keyringOnlyUser.Id,
+		Id:             "at_2a",
+		Token:          "at_2a_token",
+		UserId:         keyringOnlyUser.Id,
+		ExpirationTime: time.Now().Add(time.Minute),
 	}
 	keyringAuthToken2 := &authtokens.AuthToken{
-		Id:     "at_2b",
-		Token:  "at_2b_token",
-		UserId: keyringOnlyUser.Id,
+		Id:             "at_2b",
+		Token:          "at_2b_token",
+		UserId:         keyringOnlyUser.Id,
+		ExpirationTime: time.Now().Add(time.Minute),
 	}
 	boundaryAuthTokens := []*authtokens.AuthToken{at1a, keyringAuthToken1, at1b, keyringAuthToken2}
 	atMap := make(map[ringToken]*authtokens.AuthToken)
@@ -149,10 +154,10 @@ func TestRefresh(t *testing.T) {
 	boundaryAddr := "address"
 	u := &user{Id: "u1", Address: boundaryAddr}
 	at := &authtokens.AuthToken{
-		Id:           "at_1",
-		Token:        "at_1_token",
-		UserId:       u.Id,
-		AuthMethodId: "am_1",
+		Id:             "at_1",
+		Token:          "at_1_token",
+		UserId:         u.Id,
+		ExpirationTime: time.Now().Add(time.Minute),
 	}
 
 	boundaryAuthTokens := []*authtokens.AuthToken{at}

internal/daemon/cache/repository_token.go

@@ -62,10 +62,11 @@ func upsertUserAndAuthToken(ctx context.Context, reader db.Reader, writer db.Wri
 			Id:               at.Id,
 			UserId:           at.UserId,
 			LastAccessedTime: time.Now(),
+			ExpirationTime:   at.ExpirationTime,
 		}
 		onConflict := &db.OnConflict{
 			Target: db.Columns{"id"},
-			Action: db.SetColumns([]string{"last_accessed_time"}),
+			Action: db.SetColumns([]string{"last_accessed_time", "expiration_time"}),
 		}
 		if err := writer.Create(ctx, st, db.WithOnConflict(onConflict)); err != nil {
 			return errors.Wrap(ctx, err, op)
@@ -303,7 +304,7 @@ func (r *Repository) deleteKeyringToken(ctx context.Context, kt KeyringToken) er
 
 		switch n {
 		case 1:
-			if err := cleanOrphanedAuthTokens(ctx, writer, r.idToKeyringlessAuthToken); err != nil {
+			if err := cleanExpiredOrOrphanedAuthTokens(ctx, writer, r.idToKeyringlessAuthToken); err != nil {
 				return errors.Wrap(ctx, err, op)
 			}
 			return nil
@@ -319,23 +320,23 @@ func (r *Repository) deleteKeyringToken(ctx context.Context, kt KeyringToken) er
 	return nil
 }
 
-// cleanAuthTokens removes all tokens which are older than the staleness limit
+// cleanExpiredOrOrphanedAuthTokens removes all tokens which are older than the staleness limit
 // or does not have either a keyring or keyringless reference to it.
-func (r *Repository) cleanOrphanedAuthTokens(ctx context.Context) error {
-	const op = "cache.Repository.cleanOrphanedAuthTokens"
+func (r *Repository) cleanExpiredOrOrphanedAuthTokens(ctx context.Context) error {
+	const op = "cache.Repository.cleanExpiredOrOrphanedAuthTokens"
 	_, err := r.rw.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(reader db.Reader, writer db.Writer) error {
-		if err := cleanOrphanedAuthTokens(ctx, writer, r.idToKeyringlessAuthToken); err != nil {
+		if err := cleanExpiredOrOrphanedAuthTokens(ctx, writer, r.idToKeyringlessAuthToken); err != nil {
 			return errors.Wrap(ctx, err, op)
 		}
 		return nil
 	})
 	return err
 }
 
-// cleanAuthTokens removes all tokens which are older than the staleness limit
+// cleanExpiredOrOrphanedAuthTokens removes all tokens which are older than the staleness limit
 // or does not have either a keyring or keyringless reference to it.
-func cleanOrphanedAuthTokens(ctx context.Context, writer db.Writer, idToKeyringlessAuthToken *sync.Map) error {
-	const op = "cache.cleanAuthTokens"
+func cleanExpiredOrOrphanedAuthTokens(ctx context.Context, writer db.Writer, idToKeyringlessAuthToken *sync.Map) error {
+	const op = "cache.cleanExpiredOrOrphanedAuthTokens"
 	switch {
 	// TODO: Add check here to see if a transaction is in flight.
 	case util.IsNil(writer):
@@ -354,6 +355,12 @@ func cleanOrphanedAuthTokens(ctx context.Context, writer db.Writer, idToKeyringl
 	delete from auth_token
 	where
 		last_accessed_time < @last_accessed_time
+	or
+		-- sqlite stores expiration_time as a string in a format that might not match
+		-- what is being used by current_timestamp.  Using datetime() makes the
+		-- formats match which allow the string comparison performed here to work
+		-- the same as a time comparison.
+		datetime(expiration_time) < current_timestamp
 	or
 	%s
 	`
@@ -497,6 +504,7 @@ type AuthToken struct {
 	Id               string    `gorm:"primaryKey"`
 	UserId           string    `gorm:"default:null"`
 	LastAccessedTime time.Time `gorm:"default:(strftime('%Y-%m-%d %H:%M:%f','now'))"`
+	ExpirationTime   time.Time
 }
 
 func (*AuthToken) TableName() string {

internal/daemon/cache/repository_token_test.go

@@ -618,35 +618,51 @@ func TestRepository_RemoveStaleTokens(t *testing.T) {
 		Address: "address",
 	}
 	at1 := &authtokens.AuthToken{
-		Id:     "at_1",
-		Token:  "at_1_token",
-		UserId: u.Id,
+		Id:             "at_1",
+		Token:          "at_1_token",
+		UserId:         u.Id,
+		ExpirationTime: time.Now().Add(1 * time.Minute),
 	}
 	kt1 := KeyringToken{
 		TokenName:   "t1",
 		KeyringType: "k1",
 		AuthTokenId: at1.Id,
 	}
 	at2 := &authtokens.AuthToken{
-		Id:     "at_2",
-		Token:  "at_2_token",
-		UserId: u.Id,
+		Id:             "at_2",
+		Token:          "at_2_token",
+		UserId:         u.Id,
+		ExpirationTime: time.Now().Add(1 * time.Minute),
 	}
 	kt2 := KeyringToken{
 		TokenName:   "t2",
 		KeyringType: "k2",
 		AuthTokenId: "at_2",
 	}
+	// this auth token expired a minute ago
+	at3 := &authtokens.AuthToken{
+		Id:             "at_3",
+		Token:          "at_3_token",
+		UserId:         u.Id,
+		ExpirationTime: time.Now().Add(-(1 * time.Minute)),
+	}
+	kt3 := KeyringToken{
+		TokenName:   "t3",
+		KeyringType: "k3",
+		AuthTokenId: "at_3",
+	}
 
-	boundaryAuthTokens := []*authtokens.AuthToken{at1, at2}
+	boundaryAuthTokens := []*authtokens.AuthToken{at1, at2, at3}
 	atMap := map[ringToken]*authtokens.AuthToken{
 		{kt1.KeyringType, kt1.TokenName}: at1,
 		{kt2.KeyringType, kt2.TokenName}: at2,
+		{kt3.KeyringType, kt3.TokenName}: at3,
 	}
 	r, err := NewRepository(ctx, s, &sync.Map{}, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(boundaryAuthTokens))
 	require.NoError(t, err)
 	assert.NoError(t, r.AddKeyringToken(ctx, u.Address, kt1))
 	assert.NoError(t, r.AddKeyringToken(ctx, u.Address, kt2))
+	assert.NoError(t, r.AddKeyringToken(ctx, u.Address, kt3))
 
 	staleTime := time.Now().Add(-(tokenStalenessLimit + 1*time.Hour))
 	freshTime := time.Now().Add(-(tokenStalenessLimit - 1*time.Hour))
@@ -657,6 +673,12 @@ func TestRepository_RemoveStaleTokens(t *testing.T) {
 	}
 	_, err = r.rw.Update(ctx, freshAt, []string{"LastAccessedTime"}, nil)
 	require.NoError(t, err)
+	anotherFreshAt := &AuthToken{
+		Id:               at3.Id,
+		LastAccessedTime: freshTime,
+	}
+	_, err = r.rw.Update(ctx, anotherFreshAt, []string{"LastAccessedTime"}, nil)
+	require.NoError(t, err)
 
 	staleAt := &AuthToken{
 		Id:               at2.Id,
@@ -667,9 +689,9 @@ func TestRepository_RemoveStaleTokens(t *testing.T) {
 
 	lAt, err := r.listTokens(ctx, u)
 	assert.NoError(t, err)
-	assert.Len(t, lAt, 2)
+	assert.Len(t, lAt, 3)
 
-	assert.NoError(t, r.cleanOrphanedAuthTokens(ctx))
+	assert.NoError(t, r.cleanExpiredOrOrphanedAuthTokens(ctx))
 
 	lAt, err = r.listTokens(ctx, u)
 	assert.NoError(t, err)

internal/daemon/cache/schema.sql

@@ -25,7 +25,8 @@ create table if not exists auth_token (
   -- the last time this the auth token was used on this machine to access
   -- boundary outside of the context of the cache.
   last_accessed_time timestamp not null
-    default (strftime('%Y-%m-%d %H:%M:%f','now'))
+    default (strftime('%Y-%m-%d %H:%M:%f','now')),
+  expiration_time timestamp not null
 );
 
 -- *delete_orphaned_users triggers delete a user when it no longer has any