From 9d68d5ef187620f739b92e897dbe5e07b1d44e76 Mon Sep 17 00:00:00 2001 From: Jim Date: Sat, 12 Oct 2024 16:10:34 -0400 Subject: [PATCH] feat (cache): impl. soft-delete of users in cache Previously, when all the auth tokens for a user where deleted, we deleted the user along with all the cached resources for that user. This can create a lot of churn in the cache, especially if the auth tokens have a short TTL (like 8 hrs), which can cause all the resources to be reloaded every morning when the user logs in. This change introduces a soft-delete of users in the cache. When all the auth tokens for a user are deleted, but there's still a valid refresh token that's less than 20 days old, we mark the user as deleted in the cache and do not return the user in the list of users returned by the cache, but we still keep the user in the cache (all all their cached resources). This way, when the user logs in again, we can just mark the user as active again and not have to reload all the resources. If the refresh token is older than 20 days, we delete the user from the cache along with all their cached resources. --- .../internal/cache/refresh_test.go | 8 +- .../cache/repository_refresh_token_test.go | 5 +- .../internal/cache/repository_token.go | 42 +- .../internal/cache/repository_token_test.go | 359 +++++++++++++++++- internal/clientcache/internal/db/db.go | 2 +- internal/clientcache/internal/db/schema.sql | 49 ++- internal/db/option.go | 12 + internal/db/option_test.go | 11 + internal/db/read_writer.go | 12 +- 9 files changed, 469 insertions(+), 31 deletions(-) diff --git a/internal/clientcache/internal/cache/refresh_test.go b/internal/clientcache/internal/cache/refresh_test.go index 3895376132..6f43e6b028 100644 --- a/internal/clientcache/internal/cache/refresh_test.go +++ b/internal/clientcache/internal/cache/refresh_test.go @@ -104,7 +104,7 @@ func testResolvableAliasStaticResourceRetrievalFunc(inFunc func(ctx context.Cont // testNoRefreshRetrievalFunc simulates a controller that doesn't support refresh // since it does not return any refresh token. -func testNoRefreshRetrievalFunc[T any](t *testing.T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { +func testNoRefreshRetrievalFunc[T any](_ *testing.T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { return func(_ context.Context, _, _ string, _ RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { return nil, nil, "", ErrRefreshNotSupported } @@ -113,7 +113,7 @@ func testNoRefreshRetrievalFunc[T any](t *testing.T) func(context.Context, strin // testErroringForRefreshTokenRetrievalFunc returns a refresh token error when // the refresh token is not empty. This is useful for testing behavior when // the refresh token has expired or is otherwise invalid. -func testErroringForRefreshTokenRetrievalFunc[T any](t *testing.T, ret []T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { +func testErroringForRefreshTokenRetrievalFunc[T any](_ *testing.T, ret []T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { return func(ctx context.Context, s1, s2 string, refToken RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { if refToken != "" { return nil, nil, "", api.ErrInvalidListToken @@ -158,7 +158,7 @@ func testStaticResourceRetrievalFuncForId[T any](t *testing.T, ret [][]T, remove // since it does not return any refresh token. This is for retrieval // functions that require an id be provided for listing purposes like when // listing resolvable aliases. -func testNoRefreshRetrievalFuncForId[T any](t *testing.T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { +func testNoRefreshRetrievalFuncForId[T any](_ *testing.T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { return func(_ context.Context, _, _, _ string, _ RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { return nil, nil, "", ErrRefreshNotSupported } @@ -169,7 +169,7 @@ func testNoRefreshRetrievalFuncForId[T any](t *testing.T) func(context.Context, // the refresh token has expired or is otherwise invalid. This is for retrieval // functions that require an id be provided for listing purposes like when // listing resolvable aliases. -func testErroringForRefreshTokenRetrievalFuncForId[T any](t *testing.T, ret []T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { +func testErroringForRefreshTokenRetrievalFuncForId[T any](_ *testing.T, ret []T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { return func(ctx context.Context, s1, s2, s3 string, refToken RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) { if refToken != "" { return nil, nil, "", api.ErrInvalidListToken diff --git a/internal/clientcache/internal/cache/repository_refresh_token_test.go b/internal/clientcache/internal/cache/repository_refresh_token_test.go index e2c67ccabe..cb092b6912 100644 --- a/internal/clientcache/internal/cache/repository_refresh_token_test.go +++ b/internal/clientcache/internal/cache/repository_refresh_token_test.go @@ -189,7 +189,7 @@ func TestLookupRefreshToken(t *testing.T) { }) t.Run("unknown user", func(t *testing.T) { - got, err := r.lookupRefreshToken(ctx, &user{Id: "unkonwnUser", Address: "addr"}, targetResourceType) + got, err := r.lookupRefreshToken(ctx, &user{Id: "unknownUser", Address: "addr"}, targetResourceType) assert.NoError(t, err) assert.Empty(t, got) }) @@ -209,10 +209,11 @@ func TestLookupRefreshToken(t *testing.T) { require.NoError(t, r.rw.Create(ctx, known)) before := time.Now().Truncate(time.Millisecond).UTC() - r.rw.DoTx(ctx, 1, db.ExpBackoff{}, func(r db.Reader, w db.Writer) error { + _, err := r.rw.DoTx(ctx, 1, db.ExpBackoff{}, func(r db.Reader, w db.Writer) error { require.NoError(t, upsertRefreshToken(ctx, w, known, targetResourceType, token)) return nil }) + require.NoError(t, err) got, err := r.lookupRefreshToken(ctx, known, targetResourceType) assert.NoError(t, err) diff --git a/internal/clientcache/internal/cache/repository_token.go b/internal/clientcache/internal/cache/repository_token.go index 055a1b7e32..2732584597 100644 --- a/internal/clientcache/internal/cache/repository_token.go +++ b/internal/clientcache/internal/cache/repository_token.go @@ -75,7 +75,8 @@ func upsertUserAndAuthToken(ctx context.Context, reader db.Reader, writer db.Wri } var users []*user - if err := reader.SearchWhere(ctx, &users, "true", []any{}, db.WithLimit(-1)); err != nil { + // we only want users that have not been soft deleted + if err := reader.SearchWhere(ctx, &users, "true", []any{}, db.WithLimit(-1), db.WithTable(activeUserTableName)); err != nil { return errors.Wrap(ctx, err, op) } if len(users) <= usersLimit { @@ -382,6 +383,8 @@ func cleanExpiredOrOrphanedAuthTokens(ctx context.Context, writer db.Writer, idT return nil } +const activeUserTableName = "user_active" // users that have not been soft deleted + // lookupUser returns a user if one is present in the repository or nil if not. func (r *Repository) lookupUser(ctx context.Context, id string) (*user, error) { const op = "cache.(Repository).lookupUser" @@ -390,7 +393,8 @@ func (r *Repository) lookupUser(ctx context.Context, id string) (*user, error) { return nil, errors.New(ctx, errors.InvalidParameter, op, "empty id") } ret := &user{Id: id} - if err := r.rw.LookupById(ctx, ret); err != nil { + // we only want users that have NOT been soft deleted + if err := r.rw.LookupById(ctx, ret, db.WithTable(activeUserTableName)); err != nil { if errors.IsNotFoundError(err) { return nil, nil } @@ -403,7 +407,8 @@ func (r *Repository) lookupUser(ctx context.Context, id string) (*user, error) { func (r *Repository) listUsers(ctx context.Context) ([]*user, error) { const op = "cache.(Repository).listUsers" var ret []*user - if err := r.rw.SearchWhere(ctx, &ret, "true", nil); err != nil { + // we only want users that have NOT been soft deleted + if err := r.rw.SearchWhere(ctx, &ret, "true", nil, db.WithTable(activeUserTableName)); err != nil { return nil, errors.Wrap(ctx, err, op) } return ret, nil @@ -482,16 +487,31 @@ func deleteUser(ctx context.Context, w db.Writer, u *user) (int, error) { case u.Id == "": return db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "missing id") } - // TODO(https://github.com/go-gorm/gorm/issues/4879): Use the - // writer.Delete() function once the gorm bug is fixed. Until then - // the gorm driver for sqlite has an error which wont execute a - // delete correctly. as a work around we manually execute the - // query here. - n, err := w.Exec(ctx, "delete from user where id = ?", []any{u.Id}) + const ( + // delete the user if they don't have any refresh tokens which are + // newer than 20 days (the refresh token expiration time) + deleteStmt = "delete from user where id = ? and id not in (select user_id from refresh_token where DATETIME('now', '-20 days') < datetime(create_time) )" + + // fallback to soft deleting the user + softDeleteStmt = "update user set deleted_at = (strftime('%Y-%m-%d %H:%M:%f','now')) where id = ?" + ) + // see if we should delete the user + rowsAffected, err := w.Exec(ctx, deleteStmt, []any{u.Id}) + switch { + case err != nil: + return db.NoRowsAffected, errors.Wrap(ctx, err, op) + case rowsAffected > 0: + // if we deleted the user, so we're done. + return rowsAffected, nil + } + + // fallback to soft delete + rowsAffected, err = w.Exec(ctx, softDeleteStmt, []any{u.Id}) if err != nil { - err = errors.Wrap(ctx, err, op) + return db.NoRowsAffected, errors.Wrap(ctx, err, op) } - return n, err + + return rowsAffected, nil } // user is a gorm model for the user table. It represents a user diff --git a/internal/clientcache/internal/cache/repository_token_test.go b/internal/clientcache/internal/cache/repository_token_test.go index 7124e805ad..ed3541ac3f 100644 --- a/internal/clientcache/internal/cache/repository_token_test.go +++ b/internal/clientcache/internal/cache/repository_token_test.go @@ -5,14 +5,21 @@ package cache import ( "context" + "database/sql/driver" + stderrors "errors" "fmt" "sync" "testing" "time" + "github.com/hashicorp/boundary/api/aliases" "github.com/hashicorp/boundary/api/authtokens" + "github.com/hashicorp/boundary/api/sessions" + "github.com/hashicorp/boundary/api/targets" cachedb "github.com/hashicorp/boundary/internal/clientcache/internal/db" "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/go-dbw" + "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/exp/maps" @@ -610,7 +617,7 @@ func TestRepository_LookupToken(t *testing.T) { }) } -func TestRepository_lookupUpser(t *testing.T) { +func TestRepository_lookupUpUser(t *testing.T) { ctx := context.Background() s, err := cachedb.Open(ctx) require.NoError(t, err) @@ -650,6 +657,250 @@ func TestRepository_lookupUpser(t *testing.T) { assert.NoError(t, err) assert.Equal(t, &user{Id: at.UserId, Address: addr}, u) }) + t.Run("soft-deleted", func(t *testing.T) { + at2 := &authtokens.AuthToken{ + Id: "at_2", + Token: "at_2_token", + UserId: "u_2", + ExpirationTime: time.Now().Add(1 * time.Minute), // not expired is required for this test + } + kt2 := KeyringToken{ + TokenName: "t2", + KeyringType: "k2", + AuthTokenId: at2.Id, + } + addr2 := "address2" + boundaryAuthTokens2 := []*authtokens.AuthToken{at2} + atMap2 := map[ringToken]*authtokens.AuthToken{ + {kt2.KeyringType, kt2.TokenName}: at2, + } + m := &sync.Map{} + r2, err := NewRepository(ctx, s, m, mapBasedAuthTokenKeyringLookup(atMap2), sliceBasedAuthTokenBoundaryReader(boundaryAuthTokens2)) + require.NoError(t, err) + assert.NoError(t, r2.AddKeyringToken(ctx, addr2, kt2)) + + rs, err := NewRefreshService(ctx, r2, hclog.NewNullLogger(), 0, 0) + require.NoError(t, err) + + retTargets := []*targets.Target{ + target("1"), + target("2"), + target("3"), + target("4"), + } + opts := []Option{ + WithAliasRetrievalFunc(testResolvableAliasStaticResourceRetrievalFunc(testStaticResourceRetrievalFuncForId[*aliases.Alias](t, nil, nil))), + WithSessionRetrievalFunc(testSessionStaticResourceRetrievalFunc(testStaticResourceRetrievalFunc[*sessions.Session](t, nil, nil))), + WithTargetRetrievalFunc(testTargetStaticResourceRetrievalFunc(testStaticResourceRetrievalFunc[*targets.Target](t, + [][]*targets.Target{ + retTargets[:3], + retTargets[3:], + }, + [][]string{ + nil, + {retTargets[0].Id, retTargets[1].Id}, + }, + ))), + } + assert.NoError(t, rs.RefreshForSearch(ctx, at2.Id, Targets, opts...)) + // Now load up a few resources and a token, and trying again should + // see the RefreshForSearch update more fields. + assert.NoError(t, rs.Refresh(ctx, opts...)) + cachedTargets, err := r.ListTargets(ctx, at2.Id) + assert.NoError(t, err) + assert.ElementsMatch(t, retTargets[:3], cachedTargets.Targets) + + // should be found in cache (user_active) + u2, err := r2.lookupUser(ctx, at2.UserId) + assert.NoError(t, err) + assert.Equal(t, &user{Id: at2.UserId, Address: addr2}, u2) + u2, err = r2.lookupUser(ctx, at2.UserId) + assert.NoError(t, err) + assert.Equal(t, &user{Id: at2.UserId, Address: addr2}, u2) + + // should be found in underlying user table as well + tu, err := testLookupUser(t, s, at2.UserId) + assert.NoError(t, err) + assert.Equal(t, &testUser{Id: at2.UserId, Address: addr2, DeletedAt: infinityValue}, tu) + + // there better be some refresh tokens + tks, err := r2.listRefreshTokens(ctx, u2) + assert.NoError(t, err) + assert.NotEmpty(t, tks) + + // now delete the user's auth_token and be sure the user is still found + // in the cache (table == "user" and not in "user_active") + err = r2.deleteKeyringToken(ctx, kt2) + require.NoError(t, err) + + currentTks, err := r2.listTokens(ctx, u2) + require.NoError(t, err) + assert.Empty(t, currentTks) + + // should no longer be an active user + u2, err = r2.lookupUser(ctx, tu.Id) + assert.NoError(t, err) + assert.Empty(t, u2) + + // should still be found in underlying user table + tu, err = testLookupUser(t, s, tu.Id) + assert.NoError(t, err) + assert.Equal(t, &testUser{Id: tu.Id, Address: tu.Address, DeletedAt: tu.DeletedAt}, tu) + }) + t.Run("hard-deleted", func(t *testing.T) { + at3 := &authtokens.AuthToken{ + Id: "at_3", + Token: "at_3_token", + UserId: "u_3", + ExpirationTime: time.Now().Add(1 * time.Minute), // not expired is required for this test + } + kt3 := KeyringToken{ + TokenName: "t3", + KeyringType: "k3", + AuthTokenId: at3.Id, + } + addr3 := "address3" + boundaryAuthTokens3 := []*authtokens.AuthToken{at3} + atMap3 := map[ringToken]*authtokens.AuthToken{ + {kt3.KeyringType, kt3.TokenName}: at3, + } + m := &sync.Map{} + r3, err := NewRepository(ctx, s, m, mapBasedAuthTokenKeyringLookup(atMap3), sliceBasedAuthTokenBoundaryReader(boundaryAuthTokens3)) + require.NoError(t, err) + assert.NoError(t, r3.AddKeyringToken(ctx, addr3, kt3)) + + // should be found in cache (user_active) + u3, err := r3.lookupUser(ctx, at3.UserId) + assert.NoError(t, err) + assert.Equal(t, &user{Id: at3.UserId, Address: addr3}, u3) + u3, err = r3.lookupUser(ctx, at3.UserId) + assert.NoError(t, err) + assert.Equal(t, &user{Id: at3.UserId, Address: addr3}, u3) + + // should be found in underlying user table as well + tu, err := testLookupUser(t, s, at3.UserId) + assert.NoError(t, err) + assert.Equal(t, &testUser{Id: at3.UserId, Address: addr3, DeletedAt: infinityValue}, tu) + + // there better be some refresh tokens + tks, err := r3.listRefreshTokens(ctx, u3) + assert.NoError(t, err) + assert.Empty(t, tks) + + // now delete the user's auth_token and be sure the user is not found + // in the cache (not in either the "user" or "user_active" tables) + err = r3.deleteKeyringToken(ctx, kt3) + require.NoError(t, err) + + currentTks, err := r3.listTokens(ctx, u3) + require.NoError(t, err) + assert.Empty(t, currentTks) + + // should no longer be an active user + u3, err = r3.lookupUser(ctx, tu.Id) + assert.NoError(t, err) + assert.Empty(t, u3) + + // should not be found in underlying user table + _, err = testLookupUser(t, s, tu.Id) + assert.Error(t, err) + assert.ErrorIs(t, err, dbw.ErrRecordNotFound) + }) +} + +// infinityValue represents a time.Time that is infinity +var infinityValue = infinityDate{ + Time: time.Time{}, + IsInfinity: true, +} + +// negInfinityValue represents a time.Time that is negative infinity +var negInfinityValue = infinityDate{ + Time: time.Time{}, + IsNegInfinity: true, +} + +// infinityDate is used to represent a time.Time that can be infinity, neg +// infinity or a regular time.Time +type infinityDate struct { + Time time.Time + IsInfinity bool + IsNegInfinity bool +} + +// sqliteDatetimeLayout defines the format for sqlite datetime ('YYYY-MM-DD HH:MM:SS.SSS') +const sqliteDatetimeLayout = "2006-01-02 15:04:05.999" + +// Scan implements the sql.Scanner interface for infinityDate +func (d *infinityDate) Scan(value any) error { + switch v := value.(type) { + case string: + if v == "infinity" { + d.IsInfinity = true + d.IsNegInfinity = false + return nil + } else if v == "-infinity" { + d.IsNegInfinity = true + d.IsInfinity = false + return nil + } else { + parsedTime, err := time.Parse(sqliteDatetimeLayout, v) + if err != nil { + return err + } + d.Time = parsedTime + d.IsInfinity = false + d.IsNegInfinity = false + return nil + } + case time.Time: + d.Time = v + d.IsInfinity = false + d.IsNegInfinity = false + return nil + } + return stderrors.New("unsupported data type for Date") +} + +// Value implements the driver.Valuer interface for infinityDate +func (d infinityDate) Value() (driver.Value, error) { + if d.IsInfinity { + return "infinity", nil + } else if d.IsNegInfinity { + return "-infinity", nil + } + return d.Time.Format(sqliteDatetimeLayout), nil +} + +// testUser is used by testLookupUser to lookup a user from the database and +// supports returing the user's DeletedAt time (soft delete). +type testUser struct { + Id string + Address string + DeletedAt infinityDate +} + +// testLookupUser is a helper function to lookup a user from the database in the +// underlying user table. +func testLookupUser(t *testing.T, conn any, id string) (*testUser, error) { + t.Helper() + var rw db.Reader + switch v := conn.(type) { + case *db.DB: + rw = db.New(v) + case db.Reader: + rw = v + } + u := &testUser{ + Id: id, + } + err := rw.LookupById(context.Background(), u, db.WithTable("user")) + switch { + case err == nil: + return u, nil + default: + return &testUser{}, err + } } func TestRepository_RemoveStaleTokens(t *testing.T) { @@ -863,4 +1114,110 @@ func TestUpsertUserAndAuthToken(t *testing.T) { return nil }) require.NoError(t, err) + t.Run("hard-and-soft-delete-oldest-user", func(t *testing.T) { + boundaryAuthTokens := make([]*authtokens.AuthToken, 0, usersLimit) + atMap := map[ringToken]*authtokens.AuthToken{} + m := &sync.Map{} + + // create usersLimit users to simulate the case where the user limit is + // reached. The Tx is required because upsertUserAndAuthToken requires + // an inflight transaction. + _, err = rw.DoTx(ctx, 1, db.ExpBackoff{}, func(txReader db.Reader, txWriter db.Writer) error { + for i := 1; i <= usersLimit; i++ { + u := &user{ + Id: fmt.Sprintf("u_%d", i), + Address: fmt.Sprintf("address_%d", i), + } + at := &authtokens.AuthToken{ + Id: fmt.Sprintf("at_%d", i), + Token: fmt.Sprintf("at_%d_token", i), + UserId: u.Id, + } + boundaryAuthTokens = append(boundaryAuthTokens, at) + atMap[ringToken{fmt.Sprintf("k_%d", i), fmt.Sprintf("t_%d", i)}] = at + err := upsertUserAndAuthToken(ctx, txReader, txWriter, u.Address, at) + require.NoError(t, err) + + } + return nil + }) + // verify that all the initial users were added + repo, err := NewRepository(ctx, s, m, mapBasedAuthTokenKeyringLookup(atMap), sliceBasedAuthTokenBoundaryReader(boundaryAuthTokens)) + require.NoError(t, err) + for i := 1; i <= usersLimit; i++ { + userId := fmt.Sprintf("u_%d", i) + foundUser, err := repo.lookupUser(ctx, userId) + require.NoError(t, err) + _, err = testLookupUser(t, s, foundUser.Id) + assert.NoError(t, err) + } + + { + // setup is done. Let's add a new user and verify that the oldest + // user is hard deleted + _, err = rw.DoTx(ctx, 1, db.ExpBackoff{}, func(txReader db.Reader, txWriter db.Writer) error { + // add a new user, which should trigger the hard deletion of the oldest user + newUser := &user{ + Id: "u_new", + Address: "address_new", + } + newUserAt := &authtokens.AuthToken{ + Id: "at_new", + Token: "at_new_token", + UserId: newUser.Id, + } + err := upsertUserAndAuthToken(ctx, txReader, txWriter, newUser.Address, newUserAt) + require.NoError(t, err) + return nil + }) + require.NoError(t, err) + + // verify that the oldest user was hard deleted + foundUser, err := repo.lookupUser(ctx, "u_1") + assert.NoError(t, err) + assert.Empty(t, foundUser) + foundTestUser, err := testLookupUser(t, s, "u_1") + assert.Error(t, err) + assert.Equal(t, &testUser{}, foundTestUser) + } + { + // Let's add a refresh token for the oldest user and then new user + // and verify that the oldest user is soft deleted + rt := &refreshToken{ + UserId: "u_2", + ResourceType: "target", + RefreshToken: "rt_2", + CreateTime: time.Now().Add(-24 * time.Hour), + UpdateTime: time.Now().Add(-24 * time.Hour), + } + err = repo.rw.Create(ctx, rt) + require.NoError(t, err) + + _, err = rw.DoTx(ctx, 1, db.ExpBackoff{}, func(txReader db.Reader, txWriter db.Writer) error { + // add a new user, which should trigger the soft deletion of the oldest user + newUser := &user{ + Id: "u_new_2", + Address: "address_new_2", + } + newUserAt := &authtokens.AuthToken{ + Id: "at_new_2", + Token: "at_new_token_2", + UserId: newUser.Id, + } + err := upsertUserAndAuthToken(ctx, txReader, txWriter, newUser.Address, newUserAt) + require.NoError(t, err) + return nil + }) + require.NoError(t, err) + + // verify that the oldest user was soft deleted + foundUser, err := repo.lookupUser(ctx, "u_2") + assert.NoError(t, err) + assert.Empty(t, foundUser) + // should not find the user in the underlying user table + foundTestUser, err := testLookupUser(t, s, "u_2") + assert.NoError(t, err) + assert.NotEqual(t, &testUser{}, foundTestUser) + } + }) } diff --git a/internal/clientcache/internal/db/db.go b/internal/clientcache/internal/db/db.go index 9063cf3725..2c4c029666 100644 --- a/internal/clientcache/internal/db/db.go +++ b/internal/clientcache/internal/db/db.go @@ -146,7 +146,7 @@ type schema struct { const ( schemaTableName = "schema_version" - schemaCurrentVersion = "v0.0.2" + schemaCurrentVersion = "v0.0.4" ) // TableName returns the table name diff --git a/internal/clientcache/internal/db/schema.sql b/internal/clientcache/internal/db/schema.sql index 3806637716..dd5edf8e94 100644 --- a/internal/clientcache/internal/db/schema.sql +++ b/internal/clientcache/internal/db/schema.sql @@ -34,7 +34,7 @@ when end; -insert into schema_version(version) values('v0.0.2'); +insert into schema_version(version) values('v0.0.4'); -- user contains the boundary user information for the boundary user that owns -- the information in the cache. @@ -44,9 +44,17 @@ create table if not exists user ( check (length(id) > 0), -- The address of the boundary instance that this user id comes from address text not null - check (length(address) > 0) + check (length(address) > 0), + -- deleted indicate if the user has been soft-deleted when the all + -- auth_tokens associated with the user are deleted. + deleted_at timestamp not null default 'infinity' ); +-- user_active is a view that contains only the active users in the cache. This +-- view is used to prevent the cache from syncing data for users that have been +-- soft-deleted. +create view user_active as select * from user where deleted_at = 'infinity'; + -- Contains the known resource types contained in the boundary client cache create table if not exists resource_type_enm( string text not null primary key @@ -111,19 +119,46 @@ create table if not exists auth_token ( ); -- *delete_orphaned_users triggers delete a user when it no longer has any --- auth tokens associated with them +-- auth tokens associated with them and they no longer have any refresh tokens +-- that are less than 20 days old. This is to prevent the cache from syncing +-- data for users that are no longer active. create trigger token_update_delete_orphaned_users after update on auth_token begin -delete from user +-- delete users that no longer have any auth tokens associated with them +-- and they have no refresh tokens that are newer (less) than 20 days old. +delete from user +where + id not in (select user_id from auth_token) and + id not in (select user_id from refresh_token where DATETIME('now', '-20 days') < datetime(create_time) ); + +-- soft delete users that no longer have any auth tokens associated with them +-- and they haven't been previously soft deleted +-- and they no longer have any refresh tokens that are newer (greater) than 20 days old. +update user set deleted_at = (strftime('%Y-%m-%d %H:%M:%f','now')) where - id not in (select user_id from auth_token); + id not in (select user_id from auth_token) and + deleted_at = 'infinity' and + id not in (select user_id from refresh_token where DATETIME('now', '-20 days') > datetime(create_time)); + end; create trigger token_delete_delete_orphaned_users after delete on auth_token begin -delete from user +-- delete users that no longer have any auth tokens associated with them +-- and they have no refresh tokens that are newer (less) than 20 days old. +delete from user +where + id not in (select user_id from auth_token) and + id not in (select user_id from refresh_token where DATETIME('now', '-20 days') < datetime(create_time) ); + +-- soft delete users that no longer have any auth tokens associated with them +-- and they haven't been previously soft deleted +-- and they no longer have any refresh tokens that are newer (greater) than 20 days old. +update user set deleted_at = (strftime('%Y-%m-%d %H:%M:%f','now')) where - id not in (select user_id from auth_token); + id not in (select user_id from auth_token) and + deleted_at = 'infinity' and + id not in (select user_id from refresh_token where DATETIME('now', '-20 days') > datetime(create_time)); end; create table if not exists keyring_token ( diff --git a/internal/db/option.go b/internal/db/option.go index f590fc6af0..c260c4aa37 100644 --- a/internal/db/option.go +++ b/internal/db/option.go @@ -137,6 +137,9 @@ func getDbwOptions(ctx context.Context, rw *Db, i any, opType OpType, opt ...Opt if opts.withRowsAffected != nil { dbwOpts = append(dbwOpts, dbw.WithReturnRowsAffected(opts.withRowsAffected)) } + if opts.withTable != "" { + dbwOpts = append(dbwOpts, dbw.WithTable(opts.withTable)) + } return dbwOpts, nil } @@ -181,6 +184,8 @@ type Options struct { withOnConflict *OnConflict withRowsAffected *int64 + + withTable string } type oplogOpts struct { @@ -205,6 +210,13 @@ func getDefaultOptions() Options { } } +// WithTable provides an optional table name for the operation. +func WithTable(name string) Option { + return func(o *Options) { + o.withTable = name + } +} + // WithLookup enables a lookup. func WithLookup(enable bool) Option { return func(o *Options) { diff --git a/internal/db/option_test.go b/internal/db/option_test.go index e953b2644e..99feb9659e 100644 --- a/internal/db/option_test.go +++ b/internal/db/option_test.go @@ -255,4 +255,15 @@ func Test_getOpts(t *testing.T) { testOpts.withRowsAffected = &rowsAffected assert.Equal(opts, testOpts) }) + t.Run("WithTable", func(t *testing.T) { + assert := assert.New(t) + // test default of "" + opts := GetOpts() + testOpts := getDefaultOptions() + assert.Equal(opts, testOpts) + + opts = GetOpts(WithTable("foo")) + testOpts.withTable = "foo" + assert.Equal(opts, testOpts) + }) } diff --git a/internal/db/read_writer.go b/internal/db/read_writer.go index 29f3da7b5e..91d63e7b8c 100644 --- a/internal/db/read_writer.go +++ b/internal/db/read_writer.go @@ -471,14 +471,15 @@ func (rw *Db) IsTx(_ context.Context) bool { } // LookupByPublicId will lookup resource by its public_id or private_id, which -// must be unique. WithDebug is the only valid option, all other options are ignored. +// must be unique. WithTable and WithDebug are the only valid options, all other +// options are ignored. func (rw *Db) LookupById(ctx context.Context, resourceWithIder any, opt ...Option) error { const op = "db.LookupById" if rw.underlying == nil { return errors.New(ctx, errors.InvalidParameter, op, "missing underlying db") } opts := GetOpts(opt...) - if err := dbw.New(rw.underlying.wrapped.Load()).LookupBy(ctx, resourceWithIder, dbw.WithDebug(opts.withDebug)); err != nil { + if err := dbw.New(rw.underlying.wrapped.Load()).LookupBy(ctx, resourceWithIder, dbw.WithDebug(opts.withDebug), dbw.WithTable(opts.withTable)); err != nil { var errOpts []errors.Option if errors.Is(err, dbw.ErrRecordNotFound) { // Not found is a common workflow in the application layer during lookup, suppress @@ -491,20 +492,21 @@ func (rw *Db) LookupById(ctx context.Context, resourceWithIder any, opt ...Optio } // LookupByPublicId will lookup resource by its public_id, which must be unique. -// WithDebug is supported. +// WithTable and WithDebug are supported. func (rw *Db) LookupByPublicId(ctx context.Context, resource ResourcePublicIder, opt ...Option) error { return rw.LookupById(ctx, resource, opt...) } // LookupWhere will lookup the first resource using a where clause with -// parameters (it only returns the first one). WithDebug is supported. +// parameters (it only returns the first one). WithTable and WithDebug are +// supported. func (rw *Db) LookupWhere(ctx context.Context, resource any, where string, args []any, opt ...Option) error { const op = "db.LookupWhere" if rw.underlying == nil { return errors.New(ctx, errors.InvalidParameter, op, "missing underlying db") } opts := GetOpts(opt...) - if err := dbw.New(rw.underlying.wrapped.Load()).LookupWhere(ctx, resource, where, args, dbw.WithDebug(opts.withDebug)); err != nil { + if err := dbw.New(rw.underlying.wrapped.Load()).LookupWhere(ctx, resource, where, args, dbw.WithDebug(opts.withDebug), dbw.WithTable(opts.withTable)); err != nil { var errOpts []errors.Option if errors.Is(err, dbw.ErrRecordNotFound) { // Not found is a common workflow in the application layer during lookup, suppress