From 580ce701a21ce89cf7ec3a6f7beb553558c8fdb0 Mon Sep 17 00:00:00 2001 From: Viktor Torstensson Date: Wed, 10 Sep 2025 11:53:43 +0200 Subject: [PATCH 1/4] firewalldb: update kvdb `assertEqualActions` The upcoming commit will update the `AddActionReq` struct to include an extra field which the `kvdb` actions store will ignore. Therefore the `assertEqualActions` for the `kvdb` version will need to be update to ignore this field. In preparation for that change, we also do another optimization of the `assertEqualActions` function under kvdb builds, to not mutate the passed action references. --- firewalldb/test_kvdb.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/firewalldb/test_kvdb.go b/firewalldb/test_kvdb.go index c3cd4533a..c6255b05f 100644 --- a/firewalldb/test_kvdb.go +++ b/firewalldb/test_kvdb.go @@ -5,10 +5,8 @@ package firewalldb import ( "testing" - "github.com/lightninglabs/lightning-terminal/accounts" "github.com/lightninglabs/lightning-terminal/session" "github.com/lightningnetwork/lnd/clock" - "github.com/lightningnetwork/lnd/fn" "github.com/stretchr/testify/require" ) @@ -59,8 +57,9 @@ func newDBFromPathWithSessions(t *testing.T, dbPath string, func assertEqualActions(t *testing.T, expected, got *Action) { // Accounts are not explicitly linked in our bbolt DB implementation. + actualAccountID := got.AccountID got.AccountID = expected.AccountID require.Equal(t, expected, got) - got.AccountID = fn.None[accounts.AccountID]() + got.AccountID = actualAccountID } From 651cc678fcc726d872f3911fee8e38c671fb60b8 Mon Sep 17 00:00:00 2001 From: Viktor Torstensson Date: Mon, 15 Sep 2025 11:26:36 +0200 Subject: [PATCH 2/4] multi: persist full mac root key in sql actions db When migrating the actions store from kvdb to sql, we will update the existing actions to include the full mac root key, instead of just the last 4 bytes (currently called `MacaroonIdentifier`). In order to do so, we change the sql implementation of the `actions` store to persist the full mac root key, instead of just the last 4 bytes. As no production data in the sql actions store exists for users yet, it's fine for us to change this without having to address old sql actions which only stored the last 4 bytes. Note though that since old actions stored in the kvdb implementation only have the last 4 bytes of the mac root key persisted, we will only ever persist the last 4 byte of the mac root key ID for kvdb actions. When the actions are later read back from the kvdb store, the first 4 bytes of the mac root key ID will be padded with zeroes to make up the full 8 bytes. As no call site currently utilizes the full 8 bytes of the mac root key ID, this is okay for now. When we later deprecate and remove the kvdb implementation, we can then update the rest of `litd` to also use the full mac root key ID. --- firewall/request_logger.go | 19 ++++++++++++++++--- firewalldb/actions.go | 9 +++++++++ firewalldb/actions_kvdb.go | 6 ++++++ firewalldb/actions_sql.go | 22 ++++++++++++++++++---- firewalldb/actions_test.go | 16 ++++++++++++++++ firewalldb/test_kvdb.go | 20 +++++++++++++++++++- 6 files changed, 84 insertions(+), 8 deletions(-) diff --git a/firewall/request_logger.go b/firewall/request_logger.go index b28b1092a..df4292133 100644 --- a/firewall/request_logger.go +++ b/firewall/request_logger.go @@ -7,6 +7,7 @@ import ( "sync" "github.com/lightninglabs/lightning-terminal/firewalldb" + litmac "github.com/lightninglabs/lightning-terminal/macaroons" mid "github.com/lightninglabs/lightning-terminal/rpcmiddleware" "github.com/lightninglabs/lightning-terminal/session" "github.com/lightningnetwork/lnd/fn" @@ -182,14 +183,25 @@ func (r *RequestLogger) Intercept(ctx context.Context, func (r *RequestLogger) addNewAction(ctx context.Context, ri *RequestInfo, withPayloadData bool) error { - var macaroonID fn.Option[[4]byte] + var ( + rootKeyID fn.Option[uint64] + macaroonID fn.Option[[4]byte] + ) + if ri.Macaroon != nil { var err error - macID, err := session.IDFromMacaroon(ri.Macaroon) + + fullRootKeyID, err := litmac.RootKeyIDFromMacaroon( + ri.Macaroon, + ) if err != nil { - return fmt.Errorf("could not extract ID from macaroon") + return fmt.Errorf("could not extract root key ID from "+ + "macaroon: %w", err) } + macID := session.IDFromMacRootKeyID(fullRootKeyID) + + rootKeyID = fn.Some(fullRootKeyID) macaroonID = fn.Some([4]byte(macID)) } @@ -197,6 +209,7 @@ func (r *RequestLogger) addNewAction(ctx context.Context, ri *RequestInfo, SessionID: ri.SessionID, AccountID: ri.AccountID, MacaroonIdentifier: macaroonID, + MacaroonRootKeyID: rootKeyID, RPCMethod: ri.URI, } diff --git a/firewalldb/actions.go b/firewalldb/actions.go index 1d0c8c36f..d9349b38a 100644 --- a/firewalldb/actions.go +++ b/firewalldb/actions.go @@ -39,6 +39,15 @@ type AddActionReq struct { // If no macaroon was used for the action, then this will not be set. MacaroonIdentifier fn.Option[[4]byte] + // MacaroonRootKeyID is the uint64 / full 8 bytes of the root key ID of + // the macaroon used to perform the action. + // If no macaroon was used for the action, then this will not be set. + // + // NOTE: for our BoltDB impl, only the lower 32 bits / last 4 bytes of + // this uint64 are stored. When read back, the upper 32 bits / first 4 + // bytes are zeroed. + MacaroonRootKeyID fn.Option[uint64] + // SessionID holds the optional session ID of the session that this // action was performed with. // diff --git a/firewalldb/actions_kvdb.go b/firewalldb/actions_kvdb.go index b2dddd36c..24047f8fd 100644 --- a/firewalldb/actions_kvdb.go +++ b/firewalldb/actions_kvdb.go @@ -596,7 +596,13 @@ func DeserializeAction(r io.Reader, sessionID session.ID) (*Action, error) { return nil, err } + // Since the kvdb only persists 4 bytes for the macaroon root key ID, we + // first cast it to a uint32, and then to a uint64, effectively padding + // the first 4 bytes with zeroes. + rootKeyID := uint64(binary.BigEndian.Uint32(sessionID[:])) + action.MacaroonIdentifier = fn.Some([4]byte(sessionID)) + action.MacaroonRootKeyID = fn.Some(rootKeyID) action.SessionID = fn.Some(sessionID) action.ActorName = string(actor) action.FeatureName = string(featureName) diff --git a/firewalldb/actions_sql.go b/firewalldb/actions_sql.go index 75c9d0a6d..e575de4e2 100644 --- a/firewalldb/actions_sql.go +++ b/firewalldb/actions_sql.go @@ -3,6 +3,7 @@ package firewalldb import ( "context" "database/sql" + "encoding/binary" "errors" "fmt" "math" @@ -140,8 +141,11 @@ func (s *SQLDB) AddAction(ctx context.Context, } var macID []byte - req.MacaroonIdentifier.WhenSome(func(id [4]byte) { - macID = id[:] + req.MacaroonRootKeyID.WhenSome(func(rootKeyID uint64) { + rootKeyBytes := make([]byte, 8) + binary.BigEndian.PutUint64(rootKeyBytes[:], rootKeyID) + + macID = rootKeyBytes }) id, err := db.InsertAction(ctx, sqlc.InsertActionParams{ @@ -393,14 +397,24 @@ func unmarshalAction(ctx context.Context, db SQLActionQueries, legacyAcctID = fn.Some(acctID) } + // Note that we export the full 8 byte macaroon root key ID in the sql + // actions DB, while the kvdb version persists and exports stored the + // last 4 bytes only. var macID fn.Option[[4]byte] - if len(dbAction.MacaroonIdentifier) > 0 { - macID = fn.Some([4]byte(dbAction.MacaroonIdentifier)) + var macRootKeyID fn.Option[uint64] + if len(dbAction.MacaroonIdentifier) >= 4 { + dbMacID := dbAction.MacaroonIdentifier + macID = fn.Some([4]byte(dbMacID[len(dbMacID)-4:])) + + if len(dbAction.MacaroonIdentifier) >= 8 { + macRootKeyID = fn.Some(binary.BigEndian.Uint64(dbMacID)) + } } return &Action{ AddActionReq: AddActionReq{ MacaroonIdentifier: macID, + MacaroonRootKeyID: macRootKeyID, AccountID: legacyAcctID, SessionID: legacySessID, ActorName: dbAction.ActorName.String, diff --git a/firewalldb/actions_test.go b/firewalldb/actions_test.go index 69990c1da..54256ab27 100644 --- a/firewalldb/actions_test.go +++ b/firewalldb/actions_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/lightninglabs/lightning-terminal/accounts" + litmac "github.com/lightninglabs/lightning-terminal/macaroons" "github.com/lightninglabs/lightning-terminal/session" "github.com/lightningnetwork/lnd/clock" "github.com/lightningnetwork/lnd/fn" @@ -60,10 +61,13 @@ func TestActionStorage(t *testing.T) { acct1, err := accountsDB.NewAccount(ctx, 0, time.Time{}, "foo") require.NoError(t, err) + sess1RootKeyID := litmac.NewSuperMacaroonRootKeyID(sess1.ID) + action1Req := &AddActionReq{ SessionID: fn.Some(sess1.ID), AccountID: fn.Some(acct1.ID), MacaroonIdentifier: fn.Some([4]byte(sess1.ID)), + MacaroonRootKeyID: fn.Some(sess1RootKeyID), ActorName: "Autopilot", FeatureName: "auto-fees", Trigger: "fee too low", @@ -79,9 +83,12 @@ func TestActionStorage(t *testing.T) { State: ActionStateDone, } + sess2RootKeyID := litmac.NewSuperMacaroonRootKeyID(sess2.ID) + action2Req := &AddActionReq{ SessionID: fn.Some(sess2.ID), MacaroonIdentifier: fn.Some([4]byte(sess2.ID)), + MacaroonRootKeyID: fn.Some(sess2RootKeyID), ActorName: "Autopilot", FeatureName: "rebalancer", Trigger: "channels not balanced", @@ -213,8 +220,11 @@ func TestListActions(t *testing.T) { addAction := func(sessionID [4]byte) { actionIds++ + sessRootKeyID := litmac.NewSuperMacaroonRootKeyID(sessionID) + actionReq := &AddActionReq{ MacaroonIdentifier: fn.Some(sessionID), + MacaroonRootKeyID: fn.Some(sessRootKeyID), ActorName: "Autopilot", FeatureName: fmt.Sprintf("%d", actionIds), Trigger: "fee too low", @@ -424,9 +434,12 @@ func TestListGroupActions(t *testing.T) { ) require.NoError(t, err) + sess1RootKeyID := litmac.NewSuperMacaroonRootKeyID(sess1.ID) + action1Req := &AddActionReq{ SessionID: fn.Some(sess1.ID), MacaroonIdentifier: fn.Some([4]byte(sess1.ID)), + MacaroonRootKeyID: fn.Some(sess1RootKeyID), ActorName: "Autopilot", FeatureName: "auto-fees", Trigger: "fee too low", @@ -442,9 +455,12 @@ func TestListGroupActions(t *testing.T) { State: ActionStateDone, } + sess2RootKeyID := litmac.NewSuperMacaroonRootKeyID(sess2.ID) + action2Req := &AddActionReq{ SessionID: fn.Some(sess2.ID), MacaroonIdentifier: fn.Some([4]byte(sess2.ID)), + MacaroonRootKeyID: fn.Some(sess2RootKeyID), ActorName: "Autopilot", FeatureName: "rebalancer", Trigger: "channels not balanced", diff --git a/firewalldb/test_kvdb.go b/firewalldb/test_kvdb.go index c6255b05f..6b2f05c95 100644 --- a/firewalldb/test_kvdb.go +++ b/firewalldb/test_kvdb.go @@ -3,10 +3,12 @@ package firewalldb import ( + "encoding/binary" "testing" "github.com/lightninglabs/lightning-terminal/session" "github.com/lightningnetwork/lnd/clock" + "github.com/lightningnetwork/lnd/fn" "github.com/stretchr/testify/require" ) @@ -59,7 +61,23 @@ func assertEqualActions(t *testing.T, expected, got *Action) { // Accounts are not explicitly linked in our bbolt DB implementation. actualAccountID := got.AccountID got.AccountID = expected.AccountID - require.Equal(t, expected, got) + // As the kvdb implementation only stores the last 4 bytes Macaroon Root + // Key ID, we pad it with 4 zero bytes when comparing. + expectedMacRootKey := expected.MacaroonRootKeyID + + expectedMacRootKey.WhenSome(func(rootID uint64) { + // Remove the 4 byte prefix of the actual Macaroon Root Key ID. + sessID := session.IDFromMacRootKeyID(rootID) + + // Recreate the full 8 byte Macaroon Root Key ID (represented as + // a uint64) by padding the first 4 bytes with zeroes. + expected.MacaroonRootKeyID = fn.Some( + uint64(binary.BigEndian.Uint32(sessID[:])), + ) + }) + + require.Equal(t, expected, got) got.AccountID = actualAccountID + expected.MacaroonRootKeyID = expectedMacRootKey } From 01bc36ca41a4e4f1eb1cd063fbc19ac9bbffaa3f Mon Sep 17 00:00:00 2001 From: Viktor Torstensson Date: Wed, 1 Oct 2025 15:13:44 +0200 Subject: [PATCH 3/4] multi: remove `AddActionReq` `MacaroonIdentifier` As the `MacaroonRootKeyID` field of the `AddActionReq` struct also contains the 4 bytes of the `MacaroonIdentifier`, we change all call sites to instead use the last 4 bytes of the `MacaroonRootKeyID` field. As the `MacaroonIdentifier` field therefore becomes redundant, we also remove it. --- firewall/request_logger.go | 17 +++++--------- firewalldb/actions.go | 9 ++------ firewalldb/actions_kvdb.go | 8 +++---- firewalldb/actions_sql.go | 13 ++++------- firewalldb/actions_test.go | 45 ++++++++++++++++++-------------------- session_rpcserver.go | 4 ++-- 6 files changed, 38 insertions(+), 58 deletions(-) diff --git a/firewall/request_logger.go b/firewall/request_logger.go index df4292133..2af278311 100644 --- a/firewall/request_logger.go +++ b/firewall/request_logger.go @@ -9,7 +9,6 @@ import ( "github.com/lightninglabs/lightning-terminal/firewalldb" litmac "github.com/lightninglabs/lightning-terminal/macaroons" mid "github.com/lightninglabs/lightning-terminal/rpcmiddleware" - "github.com/lightninglabs/lightning-terminal/session" "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/macaroons" @@ -184,8 +183,7 @@ func (r *RequestLogger) addNewAction(ctx context.Context, ri *RequestInfo, withPayloadData bool) error { var ( - rootKeyID fn.Option[uint64] - macaroonID fn.Option[[4]byte] + rootKeyID fn.Option[uint64] ) if ri.Macaroon != nil { @@ -198,19 +196,14 @@ func (r *RequestLogger) addNewAction(ctx context.Context, ri *RequestInfo, return fmt.Errorf("could not extract root key ID from "+ "macaroon: %w", err) } - - macID := session.IDFromMacRootKeyID(fullRootKeyID) - rootKeyID = fn.Some(fullRootKeyID) - macaroonID = fn.Some([4]byte(macID)) } actionReq := &firewalldb.AddActionReq{ - SessionID: ri.SessionID, - AccountID: ri.AccountID, - MacaroonIdentifier: macaroonID, - MacaroonRootKeyID: rootKeyID, - RPCMethod: ri.URI, + SessionID: ri.SessionID, + AccountID: ri.AccountID, + MacaroonRootKeyID: rootKeyID, + RPCMethod: ri.URI, } if withPayloadData { diff --git a/firewalldb/actions.go b/firewalldb/actions.go index d9349b38a..ddaae65be 100644 --- a/firewalldb/actions.go +++ b/firewalldb/actions.go @@ -34,11 +34,6 @@ const ( // It contains all the information that is needed to create a new Action in the // ActionStateInit State. type AddActionReq struct { - // MacaroonIdentifier is a 4 byte identifier created from the last 4 - // bytes of the root key ID of the macaroon used to perform the action. - // If no macaroon was used for the action, then this will not be set. - MacaroonIdentifier fn.Option[[4]byte] - // MacaroonRootKeyID is the uint64 / full 8 bytes of the root key ID of // the macaroon used to perform the action. // If no macaroon was used for the action, then this will not be set. @@ -52,8 +47,8 @@ type AddActionReq struct { // action was performed with. // // NOTE: for our BoltDB impl, this is not persisted in any way, and we - // populate it by casting the macaroon ID to a session.ID and so is not - // guaranteed to be linked to an existing session. + // populate it by casting the MacaroonRootKeyID to a session.ID and so + // is not guaranteed to be linked to an existing session. SessionID fn.Option[session.ID] // AccountID holds the optional account ID of the account that this diff --git a/firewalldb/actions_kvdb.go b/firewalldb/actions_kvdb.go index 24047f8fd..75dc30426 100644 --- a/firewalldb/actions_kvdb.go +++ b/firewalldb/actions_kvdb.go @@ -59,10 +59,11 @@ func (db *BoltDB) AddAction(ctx context.Context, req *AddActionReq) (ActionLocator, error) { // If no macaroon is provided, then an empty 4-byte array is used as the - // macaroon ID. + // macaroon ID. Note that the kvdb implementation only stores the last + // 4 bytes of the macaroon root key ID. var macaroonID [4]byte - req.MacaroonIdentifier.WhenSome(func(id [4]byte) { - macaroonID = id + req.MacaroonRootKeyID.WhenSome(func(rootID uint64) { + macaroonID = session.IDFromMacRootKeyID(rootID) }) // If the new action links to a session, the session must exist. @@ -601,7 +602,6 @@ func DeserializeAction(r io.Reader, sessionID session.ID) (*Action, error) { // the first 4 bytes with zeroes. rootKeyID := uint64(binary.BigEndian.Uint32(sessionID[:])) - action.MacaroonIdentifier = fn.Some([4]byte(sessionID)) action.MacaroonRootKeyID = fn.Some(rootKeyID) action.SessionID = fn.Some(sessionID) action.ActorName = string(actor) diff --git a/firewalldb/actions_sql.go b/firewalldb/actions_sql.go index e575de4e2..9e2fa63bd 100644 --- a/firewalldb/actions_sql.go +++ b/firewalldb/actions_sql.go @@ -400,20 +400,15 @@ func unmarshalAction(ctx context.Context, db SQLActionQueries, // Note that we export the full 8 byte macaroon root key ID in the sql // actions DB, while the kvdb version persists and exports stored the // last 4 bytes only. - var macID fn.Option[[4]byte] var macRootKeyID fn.Option[uint64] - if len(dbAction.MacaroonIdentifier) >= 4 { - dbMacID := dbAction.MacaroonIdentifier - macID = fn.Some([4]byte(dbMacID[len(dbMacID)-4:])) - - if len(dbAction.MacaroonIdentifier) >= 8 { - macRootKeyID = fn.Some(binary.BigEndian.Uint64(dbMacID)) - } + if len(dbAction.MacaroonIdentifier) >= 8 { + macRootKeyID = fn.Some( + binary.BigEndian.Uint64(dbAction.MacaroonIdentifier), + ) } return &Action{ AddActionReq: AddActionReq{ - MacaroonIdentifier: macID, MacaroonRootKeyID: macRootKeyID, AccountID: legacyAcctID, SessionID: legacySessID, diff --git a/firewalldb/actions_test.go b/firewalldb/actions_test.go index 54256ab27..b3ee2f785 100644 --- a/firewalldb/actions_test.go +++ b/firewalldb/actions_test.go @@ -66,7 +66,6 @@ func TestActionStorage(t *testing.T) { action1Req := &AddActionReq{ SessionID: fn.Some(sess1.ID), AccountID: fn.Some(acct1.ID), - MacaroonIdentifier: fn.Some([4]byte(sess1.ID)), MacaroonRootKeyID: fn.Some(sess1RootKeyID), ActorName: "Autopilot", FeatureName: "auto-fees", @@ -86,15 +85,14 @@ func TestActionStorage(t *testing.T) { sess2RootKeyID := litmac.NewSuperMacaroonRootKeyID(sess2.ID) action2Req := &AddActionReq{ - SessionID: fn.Some(sess2.ID), - MacaroonIdentifier: fn.Some([4]byte(sess2.ID)), - MacaroonRootKeyID: fn.Some(sess2RootKeyID), - ActorName: "Autopilot", - FeatureName: "rebalancer", - Trigger: "channels not balanced", - Intent: "balance", - RPCMethod: "SendToRoute", - RPCParamsJson: []byte("hops, amount"), + SessionID: fn.Some(sess2.ID), + MacaroonRootKeyID: fn.Some(sess2RootKeyID), + ActorName: "Autopilot", + FeatureName: "rebalancer", + Trigger: "channels not balanced", + Intent: "balance", + RPCMethod: "SendToRoute", + RPCParamsJson: []byte("hops, amount"), } action2 := &Action{ @@ -223,7 +221,6 @@ func TestListActions(t *testing.T) { sessRootKeyID := litmac.NewSuperMacaroonRootKeyID(sessionID) actionReq := &AddActionReq{ - MacaroonIdentifier: fn.Some(sessionID), MacaroonRootKeyID: fn.Some(sessRootKeyID), ActorName: "Autopilot", FeatureName: fmt.Sprintf("%d", actionIds), @@ -246,11 +243,13 @@ func TestListActions(t *testing.T) { assertActions := func(dbActions []*Action, al []*action) { require.Len(t, dbActions, len(al)) for i, a := range al { - mID, err := dbActions[i].MacaroonIdentifier.UnwrapOrErr( - fmt.Errorf("macaroon identifier is none"), + rID, err := dbActions[i].MacaroonRootKeyID.UnwrapOrErr( + fmt.Errorf("macaroon root key is none"), ) require.NoError(t, err) - require.EqualValues(t, a.sessionID, mID) + require.EqualValues( + t, a.sessionID, session.IDFromMacRootKeyID(rID), + ) require.Equal(t, a.actionID, dbActions[i].FeatureName) } } @@ -438,7 +437,6 @@ func TestListGroupActions(t *testing.T) { action1Req := &AddActionReq{ SessionID: fn.Some(sess1.ID), - MacaroonIdentifier: fn.Some([4]byte(sess1.ID)), MacaroonRootKeyID: fn.Some(sess1RootKeyID), ActorName: "Autopilot", FeatureName: "auto-fees", @@ -458,15 +456,14 @@ func TestListGroupActions(t *testing.T) { sess2RootKeyID := litmac.NewSuperMacaroonRootKeyID(sess2.ID) action2Req := &AddActionReq{ - SessionID: fn.Some(sess2.ID), - MacaroonIdentifier: fn.Some([4]byte(sess2.ID)), - MacaroonRootKeyID: fn.Some(sess2RootKeyID), - ActorName: "Autopilot", - FeatureName: "rebalancer", - Trigger: "channels not balanced", - Intent: "balance", - RPCMethod: "SendToRoute", - RPCParamsJson: []byte("hops, amount"), + SessionID: fn.Some(sess2.ID), + MacaroonRootKeyID: fn.Some(sess2RootKeyID), + ActorName: "Autopilot", + FeatureName: "rebalancer", + Trigger: "channels not balanced", + Intent: "balance", + RPCMethod: "SendToRoute", + RPCParamsJson: []byte("hops, amount"), } action2 := &Action{ diff --git a/session_rpcserver.go b/session_rpcserver.go index b88cea053..6bebb73da 100644 --- a/session_rpcserver.go +++ b/session_rpcserver.go @@ -818,8 +818,8 @@ func (s *sessionRpcServer) ListActions(ctx context.Context, }) var macID [4]byte - a.MacaroonIdentifier.WhenSome(func(id [4]byte) { - macID = id + a.MacaroonRootKeyID.WhenSome(func(rootID uint64) { + macID = session.IDFromMacRootKeyID(rootID) }) resp[i] = &litrpc.Action{ From bd59605516c64dceb02206e182d94aa8fcd920e3 Mon Sep 17 00:00:00 2001 From: Viktor Torstensson Date: Fri, 3 Oct 2025 11:35:45 +0200 Subject: [PATCH 4/4] multi: add `AddActionReq` `MacaroonId` helper func Add helper method to `AddActionReq` returns the 4 byte macaroon ID that is derived from the MacaroonRootKeyID. Using the helper removes some code repetition at call sites, and makes the intended usage clearer. --- firewalldb/actions.go | 12 ++++++++++++ firewalldb/actions_kvdb.go | 5 +---- firewalldb/actions_test.go | 6 +----- session_rpcserver.go | 5 +---- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/firewalldb/actions.go b/firewalldb/actions.go index ddaae65be..9a4e350be 100644 --- a/firewalldb/actions.go +++ b/firewalldb/actions.go @@ -84,6 +84,18 @@ type AddActionReq struct { RPCParamsJson []byte } +// MacaroonId returns the 4 byte macaroon ID that is derived from the +// MacaroonRootKeyID. If the MacaroonRootKeyID is not set, then this will return +// an empty 4 byte array. +func (a *AddActionReq) MacaroonId() [4]byte { + var macID [4]byte + a.MacaroonRootKeyID.WhenSome(func(rootID uint64) { + macID = session.IDFromMacRootKeyID(rootID) + }) + + return macID +} + // Action represents an RPC call made through the firewall. type Action struct { AddActionReq diff --git a/firewalldb/actions_kvdb.go b/firewalldb/actions_kvdb.go index 75dc30426..adf58eb02 100644 --- a/firewalldb/actions_kvdb.go +++ b/firewalldb/actions_kvdb.go @@ -61,10 +61,7 @@ func (db *BoltDB) AddAction(ctx context.Context, // If no macaroon is provided, then an empty 4-byte array is used as the // macaroon ID. Note that the kvdb implementation only stores the last // 4 bytes of the macaroon root key ID. - var macaroonID [4]byte - req.MacaroonRootKeyID.WhenSome(func(rootID uint64) { - macaroonID = session.IDFromMacRootKeyID(rootID) - }) + macaroonID := req.MacaroonId() // If the new action links to a session, the session must exist. // For the bbolt impl of the store, this is our best effort attempt diff --git a/firewalldb/actions_test.go b/firewalldb/actions_test.go index b3ee2f785..8aa70509f 100644 --- a/firewalldb/actions_test.go +++ b/firewalldb/actions_test.go @@ -243,12 +243,8 @@ func TestListActions(t *testing.T) { assertActions := func(dbActions []*Action, al []*action) { require.Len(t, dbActions, len(al)) for i, a := range al { - rID, err := dbActions[i].MacaroonRootKeyID.UnwrapOrErr( - fmt.Errorf("macaroon root key is none"), - ) - require.NoError(t, err) require.EqualValues( - t, a.sessionID, session.IDFromMacRootKeyID(rID), + t, a.sessionID, dbActions[i].MacaroonId(), ) require.Equal(t, a.actionID, dbActions[i].FeatureName) } diff --git a/session_rpcserver.go b/session_rpcserver.go index 6bebb73da..0d56bd79b 100644 --- a/session_rpcserver.go +++ b/session_rpcserver.go @@ -817,10 +817,7 @@ func (s *sessionRpcServer) ListActions(ctx context.Context, sessionID = id }) - var macID [4]byte - a.MacaroonRootKeyID.WhenSome(func(rootID uint64) { - macID = session.IDFromMacRootKeyID(rootID) - }) + macID := a.MacaroonId() resp[i] = &litrpc.Action{ SessionId: sessionID[:],