Skip to content

[sql-51] session: handle removed linked account #1134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion session/sql_migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/davecgh/go-spew/spew"
"github.com/lightninglabs/lightning-terminal/accounts"
"github.com/lightninglabs/lightning-terminal/db/sqlc"
"github.com/lightningnetwork/lnd/fn"
"github.com/lightningnetwork/lnd/sqldb"
"github.com/pmezard/go-difflib/difflib"
"go.etcd.io/bbolt"
Expand Down Expand Up @@ -147,6 +148,7 @@ func migrateSessionsToSQLAndValidate(ctx context.Context,
overrideSessionTimeZone(kvSession)
overrideSessionTimeZone(migratedSession)
overrideMacaroonRecipe(kvSession, migratedSession)
overrideRemovedAccount(kvSession, migratedSession)

if !reflect.DeepEqual(kvSession, migratedSession) {
diff := difflib.UnifiedDiff{
Expand Down Expand Up @@ -196,7 +198,17 @@ func migrateSingleSessionToSQL(ctx context.Context,
var acctDBID int64
acctDBID, err = tx.GetAccountIDByAlias(ctx, acctAlias)
if errors.Is(err, sql.ErrNoRows) {
err = accounts.ErrAccNotFound
// If we can't find the account in the SQL store, it
// most likely means that the user deleted the account
// with the "litcli accounts remove" command after the
// session was created. We therefore can't link the
// SQL session to the account, and we therefore just
// leave the acctID as sql.Null
log.Warnf("Unable to find account %v in SQL store, "+
"skipping linking session %x to account",
acctAlias, session.ID)
err = nil

return
} else if err != nil {
return
Expand Down Expand Up @@ -394,3 +406,21 @@ func overrideMacaroonRecipe(kvSession *Session, migratedSession *Session) {
}
}
}

// overrideRemovedAccount modifies the kvSession to remove the account ID if the
// migrated session does not have an account ID, while the kvSession has one.
// This happens when the account was not found in the SQL store during the
// migration, which can occur if the account was deleted after the session
// was created.
func overrideRemovedAccount(kvSession *Session, migratedSession *Session) {
if migratedSession.AccountID.IsSome() {
return
}

// There was a linked account in the kv session, but not in the
// migrated session. We therefore remove the account ID from the
// kvSession.
kvSession.AccountID.WhenSome(func(alias accounts.AccountID) {
kvSession.AccountID = fn.None[accounts.AccountID]()
})
}
58 changes: 58 additions & 0 deletions session/sql_migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ func TestSessionsStoreMigration(t *testing.T) {
// kvSession has such a recipe stored.
overrideMacaroonRecipe(kvSession, sqlSession)

// We also need to override the account ID if the
// kvSession has an account ID set, while the SQL
// session doesn't.
overrideRemovedAccount(kvSession, sqlSession)

assertEqualSessions(t, kvSession, sqlSession)
}

Expand Down Expand Up @@ -291,6 +296,49 @@ func TestSessionsStoreMigration(t *testing.T) {
require.NoError(t, err)
},
},
{
name: "one session with a deleted linked account",
populateDB: func(t *testing.T, store *BoltStore,
acctStore accounts.Store) {

// Create an account with balance
acct, err := acctStore.NewAccount(
ctx, 1234, time.Now().Add(time.Hour),
"",
)
require.NoError(t, err)
require.False(t, acct.HasExpired())

// For now, we manually add the account caveat
// for bbolt compatibility.
accountCaveat := checkers.Condition(
macaroons.CondLndCustom,
fmt.Sprintf("%s %x",
accounts.CondAccount,
acct.ID[:],
),
)

sessCaveats := []macaroon.Caveat{
{
Id: []byte(accountCaveat),
},
}

_, err = store.NewSession(
ctx, "test", TypeMacaroonAccount,
time.Unix(1000, 0), "",
WithAccount(acct.ID),
WithMacaroonRecipe(sessCaveats, nil),
)
require.NoError(t, err)

// Now delete the account, which represents
// that the user ran "litcli accounts remove".
err = acctStore.RemoveAccount(ctx, acct.ID)
require.NoError(t, err)
},
},
{
name: "linked session",
populateDB: func(t *testing.T, store *BoltStore,
Expand Down Expand Up @@ -538,6 +586,16 @@ func randomizedSessions(t *testing.T, kvStore *BoltStore,
}
}

// When an account is set, we also remove the account in 50% of
// the cases. This simulates that the user ran "litcli accounts
// remove" after creating the session.
activeSess.AccountID.WhenSome(func(id accounts.AccountID) {
if rand.Intn(2) == 0 {
err = accountsStore.RemoveAccount(ctx, id)
}
})
require.NoError(t, err)

// Finally, we shift the active session to a random state.
// As the state we set may be a state that's no longer set
// through the current code base, or be an illegal state
Expand Down
Loading