From 3308537dea3e1adf5a97f4bd58ffe28605b8e9b4 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Mon, 3 Jun 2024 19:31:54 -0400 Subject: [PATCH 1/8] add function to check authorization --- testutil/sample/authority.go | 20 +++ x/authority/keeper/authorization_list.go | 47 ++++++ x/authority/keeper/authorization_list_test.go | 155 ++++++++++++++++++ x/authority/keeper/policies.go | 14 -- x/authority/types/errors.go | 11 +- x/authority/types/policies.go | 12 ++ x/authority/types/policies_test.go | 131 +++++++++++++++ 7 files changed, 373 insertions(+), 17 deletions(-) diff --git a/testutil/sample/authority.go b/testutil/sample/authority.go index c7e3b7e6b8..759bea949a 100644 --- a/testutil/sample/authority.go +++ b/testutil/sample/authority.go @@ -3,6 +3,7 @@ package sample import ( "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/zeta-chain/zetacore/pkg/chains" authoritytypes "github.com/zeta-chain/zetacore/x/authority/types" ) @@ -65,3 +66,22 @@ func Authorization() authoritytypes.Authorization { AuthorizedPolicy: authoritytypes.PolicyType_groupOperational, } } + +func MultipleSignerMessage() sdk.Msg { + return &TestMessage{} +} + +type TestMessage struct{} + +var _ sdk.Msg = &TestMessage{} + +func (m *TestMessage) Reset() {} +func (m *TestMessage) String() string { return "TestMessage" } +func (m *TestMessage) ProtoMessage() {} +func (m *TestMessage) ValidateBasic() error { return nil } +func (m *TestMessage) GetSigners() []sdk.AccAddress { + return []sdk.AccAddress{ + sdk.MustAccAddressFromBech32(AccAddress()), + sdk.MustAccAddressFromBech32(AccAddress()), + } +} diff --git a/x/authority/keeper/authorization_list.go b/x/authority/keeper/authorization_list.go index 0e7fba9163..6c3692138c 100644 --- a/x/authority/keeper/authorization_list.go +++ b/x/authority/keeper/authorization_list.go @@ -1,12 +1,17 @@ package keeper import ( + "fmt" + + "cosmossdk.io/errors" "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/zeta-chain/zetacore/x/authority/types" ) +// TODO : Refactor this file to authorization_list.go + // SetAuthorizationList sets the authorization list to the store.It returns an error if the list is invalid. func (k Keeper) SetAuthorizationList(ctx sdk.Context, list types.AuthorizationList) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.AuthorizationListKey)) @@ -24,3 +29,45 @@ func (k Keeper) GetAuthorizationList(ctx sdk.Context) (val types.AuthorizationLi k.cdc.MustUnmarshal(b, &val) return val, true } + +// IsAuthorized checks if the address is authorized for the given policy type +func (k Keeper) IsAuthorized(ctx sdk.Context, address string, policyType types.PolicyType) bool { + policies, found := k.GetPolicies(ctx) + if !found { + return false + } + for _, policy := range policies.Items { + if policy.Address == address && policy.PolicyType == policyType { + return true + } + } + return false +} + +// CheckAuthorization checks if the signer is authorized to sign the message +func (k Keeper) CheckAuthorization(ctx sdk.Context, msg sdk.Msg) error { + // Policy transactions must have only one signer + if len(msg.GetSigners()) != 1 { + return errors.Wrap(types.ErrSigners, fmt.Sprintf("msg: %v", sdk.MsgTypeURL(msg))) + } + signer := msg.GetSigners()[0].String() + msgURL := sdk.MsgTypeURL(msg) + authorizationsList, found := k.GetAuthorizationList(ctx) + if !found { + return types.ErrAuthorizationListNotFound + } + policyRequired, err := authorizationsList.GetAuthorizedPolicy(msgURL) + if err != nil { + return errors.Wrap(types.ErrAuthorizationNotFound, fmt.Sprintf("msg: %v", msgURL)) + } + //// TODO : check for empty policy + //if policyRequired == types.PolicyType_groupOperational { + // return errors.Wrap(types.ErrMsgNotAuthorized, fmt.Sprintf("msg: %v", sdk.MsgTypeURL(msg))) + //} + policies, found := k.GetPolicies(ctx) + if !found { + return errors.Wrap(types.ErrPoliciesNotFound, fmt.Sprintf("msg: %v", msgURL)) + } + + return policies.CheckSigner(signer, policyRequired) +} diff --git a/x/authority/keeper/authorization_list_test.go b/x/authority/keeper/authorization_list_test.go index 008be61f25..2dddb2f0f4 100644 --- a/x/authority/keeper/authorization_list_test.go +++ b/x/authority/keeper/authorization_list_test.go @@ -3,7 +3,9 @@ package keeper_test import ( "testing" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + lightclienttypes "github.com/zeta-chain/zetacore/x/lightclient/types" keepertest "github.com/zeta-chain/zetacore/testutil/keeper" "github.com/zeta-chain/zetacore/testutil/sample" @@ -47,3 +49,156 @@ func TestKeeper_SetAuthorizationList(t *testing.T) { require.Equal(t, newAuthorizationList, list) }) } + +func TestKeeper_CheckAuthorization(t *testing.T) { + t.Run("successfully check authorization", func(t *testing.T) { + k, ctx := keepertest.AuthorityKeeper(t) + signer := sample.AccAddress() + msg := lightclienttypes.MsgDisableHeaderVerification{ + Creator: signer, + } + authorizationList := types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: sdk.MsgTypeURL(&msg), + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + }, + } + policies := types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupOperational, + }, + }, + } + + k.SetPolicies(ctx, policies) + k.SetAuthorizationList(ctx, authorizationList) + + err := k.CheckAuthorization(ctx, &msg) + require.NoError(t, err) + }) + + t.Run("unable to check authorization with multiple signers", func(t *testing.T) { + k, ctx := keepertest.AuthorityKeeper(t) + signer := sample.AccAddress() + msg := sample.MultipleSignerMessage() + authorizationList := types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: sdk.MsgTypeURL(msg), + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + }, + } + policies := types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupOperational, + }, + }, + } + k.SetPolicies(ctx, policies) + k.SetAuthorizationList(ctx, authorizationList) + + err := k.CheckAuthorization(ctx, msg) + require.ErrorIs(t, err, types.ErrSigners) + }) + + t.Run("unable to check authorization with no authorization list", func(t *testing.T) { + k, ctx := keepertest.AuthorityKeeper(t) + signer := sample.AccAddress() + msg := lightclienttypes.MsgDisableHeaderVerification{ + Creator: signer, + } + + policies := types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupOperational, + }, + }, + } + k.SetPolicies(ctx, policies) + + err := k.CheckAuthorization(ctx, &msg) + require.ErrorIs(t, err, types.ErrAuthorizationListNotFound) + }) + + t.Run("unable to check authorization with no policies", func(t *testing.T) { + k, ctx := keepertest.AuthorityKeeper(t) + signer := sample.AccAddress() + msg := lightclienttypes.MsgDisableHeaderVerification{ + Creator: signer, + } + authorizationList := types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: sdk.MsgTypeURL(&msg), + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + }, + } + k.SetAuthorizationList(ctx, authorizationList) + + err := k.CheckAuthorization(ctx, &msg) + require.ErrorIs(t, err, types.ErrPoliciesNotFound) + }) + + t.Run("unable to check authorization when the required authorization doesnt exist", func(t *testing.T) { + k, ctx := keepertest.AuthorityKeeper(t) + signer := sample.AccAddress() + msg := lightclienttypes.MsgDisableHeaderVerification{ + Creator: signer, + } + authorizationList := types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: "/zetachain.zetacore.observer.MsgDisableCCTX", + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + }, + } + policies := types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupOperational, + }, + }, + } + k.SetPolicies(ctx, policies) + k.SetAuthorizationList(ctx, authorizationList) + + err := k.CheckAuthorization(ctx, &msg) + require.ErrorIs(t, err, types.ErrAuthorizationNotFound) + }) + + t.Run("unable to check authorization when check signer fails", func(t *testing.T) { + k, ctx := keepertest.AuthorityKeeper(t) + signer := sample.AccAddress() + msg := lightclienttypes.MsgDisableHeaderVerification{ + Creator: signer, + } + authorizationList := types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: sdk.MsgTypeURL(&msg), + AuthorizedPolicy: types.PolicyType_groupOperational, + }, + }, + } + policies := types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupAdmin, + }, + }, + } + k.SetPolicies(ctx, policies) + k.SetAuthorizationList(ctx, authorizationList) + + err := k.CheckAuthorization(ctx, &msg) + require.ErrorIs(t, err, types.ErrSignerDoesntMatch) + }) +} diff --git a/x/authority/keeper/policies.go b/x/authority/keeper/policies.go index 448f922749..cd04a23d34 100644 --- a/x/authority/keeper/policies.go +++ b/x/authority/keeper/policies.go @@ -24,17 +24,3 @@ func (k Keeper) GetPolicies(ctx sdk.Context) (val types.Policies, found bool) { k.cdc.MustUnmarshal(b, &val) return val, true } - -// IsAuthorized checks if the address is authorized for the given policy type -func (k Keeper) IsAuthorized(ctx sdk.Context, address string, policyType types.PolicyType) bool { - policies, found := k.GetPolicies(ctx) - if !found { - return false - } - for _, policy := range policies.Items { - if policy.Address == address && policy.PolicyType == policyType { - return true - } - } - return false -} diff --git a/x/authority/types/errors.go b/x/authority/types/errors.go index 88b44b9887..2fbc3928d6 100644 --- a/x/authority/types/errors.go +++ b/x/authority/types/errors.go @@ -3,7 +3,12 @@ package types import errorsmod "cosmossdk.io/errors" var ( - ErrUnauthorized = errorsmod.Register(ModuleName, 1102, "sender not authorized") - ErrInvalidAuthorizationList = errorsmod.Register(ModuleName, 1103, "invalid authorization list") - ErrAuthorizationNotFound = errorsmod.Register(ModuleName, 1104, "authorization not found") + ErrUnauthorized = errorsmod.Register(ModuleName, 1102, "sender not authorized") + ErrInvalidAuthorizationList = errorsmod.Register(ModuleName, 1103, "invalid authorization list") + ErrAuthorizationNotFound = errorsmod.Register(ModuleName, 1104, "authorization not found") + ErrAuthorizationListNotFound = errorsmod.Register(ModuleName, 1105, "authorization list not found") + ErrSigners = errorsmod.Register(ModuleName, 1106, "policy transactions must have only one signer") + ErrMsgNotAuthorized = errorsmod.Register(ModuleName, 1107, "msg type is not authorized") + ErrPoliciesNotFound = errorsmod.Register(ModuleName, 1108, "policies not found") + ErrSignerDoesntMatch = errorsmod.Register(ModuleName, 1109, "signer doesn't match required policy") ) diff --git a/x/authority/types/policies.go b/x/authority/types/policies.go index 28a97594ce..e7719f6e5e 100644 --- a/x/authority/types/policies.go +++ b/x/authority/types/policies.go @@ -3,6 +3,7 @@ package types import ( "fmt" + "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -55,3 +56,14 @@ func (p Policies) Validate() error { return nil } + +// CheckSigner checks if the signer is authorized for the given policy type +func (p Policies) CheckSigner(signer string, policyRequired PolicyType) error { + for _, policy := range p.Items { + if policy.Address == signer && policy.PolicyType == policyRequired { + return nil + } + } + return errors.Wrap(ErrSignerDoesntMatch, fmt.Sprintf("signer: %s, policy required for message: %s ", + signer, policyRequired.String())) +} diff --git a/x/authority/types/policies_test.go b/x/authority/types/policies_test.go index 7f8d2ff52c..57749b35f6 100644 --- a/x/authority/types/policies_test.go +++ b/x/authority/types/policies_test.go @@ -130,3 +130,134 @@ func TestPolicies_Validate(t *testing.T) { }) } } + +func TestPolicies_CheckSigner(t *testing.T) { + signer := sample.AccAddress() + tt := []struct { + name string + policies types.Policies + signer string + policyRequired types.PolicyType + expectedErr error + }{ + { + name: "successfully check signer for policyType groupEmergency", + policies: types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupEmergency, + }, + { + Address: signer, + PolicyType: types.PolicyType_groupAdmin, + }, + { + Address: signer, + PolicyType: types.PolicyType_groupOperational, + }, + }, + }, + signer: signer, + policyRequired: types.PolicyType_groupEmergency, + expectedErr: nil, + }, + { + name: "successfully check signer for policyType groupOperational", + policies: types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupEmergency, + }, + { + Address: signer, + PolicyType: types.PolicyType_groupAdmin, + }, + { + Address: signer, + PolicyType: types.PolicyType_groupOperational, + }, + }, + }, + signer: signer, + policyRequired: types.PolicyType_groupOperational, + expectedErr: nil, + }, + { + name: "successfully check signer for policyType groupAdmin", + policies: types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupEmergency, + }, + { + Address: signer, + PolicyType: types.PolicyType_groupAdmin, + }, + { + Address: signer, + PolicyType: types.PolicyType_groupOperational, + }, + }, + }, + signer: signer, + policyRequired: types.PolicyType_groupAdmin, + expectedErr: nil, + }, + { + name: "signer not found", + policies: types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupEmergency, + }, + { + Address: signer, + PolicyType: types.PolicyType_groupAdmin, + }, + { + Address: signer, + PolicyType: types.PolicyType_groupOperational, + }, + }, + }, + signer: sample.AccAddress(), + policyRequired: types.PolicyType_groupEmergency, + expectedErr: types.ErrSignerDoesntMatch, + }, + { + name: "policy required not found", + policies: types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupAdmin, + }, + { + Address: sample.AccAddress(), + PolicyType: types.PolicyType_groupOperational, + }, + }, + }, + signer: signer, + policyRequired: types.PolicyType_groupEmergency, + expectedErr: types.ErrSignerDoesntMatch, + }, + { + name: "empty policies", + policies: types.Policies{}, + signer: signer, + policyRequired: types.PolicyType_groupEmergency, + expectedErr: types.ErrSignerDoesntMatch, + }, + } + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + err := tc.policies.CheckSigner(tc.signer, tc.policyRequired) + require.ErrorIs(t, err, tc.expectedErr) + }) + } +} From 8dcd8aece71c15f6bce5258a4e19e421a56682b3 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Mon, 3 Jun 2024 21:36:29 -0400 Subject: [PATCH 2/8] modify comments --- x/authority/keeper/authorization_list.go | 2 -- x/authority/types/authorizations.go | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x/authority/keeper/authorization_list.go b/x/authority/keeper/authorization_list.go index 6c3692138c..ab3cd1ced3 100644 --- a/x/authority/keeper/authorization_list.go +++ b/x/authority/keeper/authorization_list.go @@ -10,8 +10,6 @@ import ( "github.com/zeta-chain/zetacore/x/authority/types" ) -// TODO : Refactor this file to authorization_list.go - // SetAuthorizationList sets the authorization list to the store.It returns an error if the list is invalid. func (k Keeper) SetAuthorizationList(ctx sdk.Context, list types.AuthorizationList) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefix(types.AuthorizationListKey)) diff --git a/x/authority/types/authorizations.go b/x/authority/types/authorizations.go index 0de98a768e..7beb52834b 100644 --- a/x/authority/types/authorizations.go +++ b/x/authority/types/authorizations.go @@ -6,6 +6,8 @@ import ( "cosmossdk.io/errors" ) +// TODO : Refactor this file to authorization_list.go + var ( // OperationPolicyMessages keeps track of the message URLs that can, by default, only be executed by operational policy address OperationPolicyMessages = []string{ From 0461829379d826e7dece4db5b3f8ae37a3fba162 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Mon, 3 Jun 2024 21:40:49 -0400 Subject: [PATCH 3/8] add changelog --- changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.md b/changelog.md index f0f94999ef..d89bc3bc45 100644 --- a/changelog.md +++ b/changelog.md @@ -22,6 +22,7 @@ * [2275](https://github.com/zeta-chain/node/pull/2275) - add ChainInfo singleton state variable in authority * [2291](https://github.com/zeta-chain/node/pull/2291) - initialize cctx gateway interface * [2289](https://github.com/zeta-chain/node/pull/2289) - add an authorization list to keep track of all authorizations on the chain +* [2313](https://github.com/zeta-chain/node/pull/2313) - add `CheckAuthorization` function to replace the `IsAuthorized` function. The new function uses the authorization list to verify the signer's authorization. ### Refactor From c10bac16b11b40ef294121a91425c1eec791ccca Mon Sep 17 00:00:00 2001 From: Tanmay Date: Tue, 4 Jun 2024 14:43:19 -0400 Subject: [PATCH 4/8] rebase develop --- testutil/sample/authority.go | 1 + x/authority/keeper/authorization_list.go | 11 ++++--- x/authority/keeper/authorization_list_test.go | 30 ++++++++++++++++++- ...uthorizations.go => authorization_list.go} | 0 ...ons_test.go => authorization_list_test.go} | 0 x/authority/types/errors.go | 1 + 6 files changed, 38 insertions(+), 5 deletions(-) rename x/authority/types/{authorizations.go => authorization_list.go} (100%) rename x/authority/types/{authorizations_test.go => authorization_list_test.go} (100%) diff --git a/testutil/sample/authority.go b/testutil/sample/authority.go index 759bea949a..24c49d3123 100644 --- a/testutil/sample/authority.go +++ b/testutil/sample/authority.go @@ -4,6 +4,7 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/zeta-chain/zetacore/pkg/chains" authoritytypes "github.com/zeta-chain/zetacore/x/authority/types" ) diff --git a/x/authority/keeper/authorization_list.go b/x/authority/keeper/authorization_list.go index ab3cd1ced3..a28eb24ede 100644 --- a/x/authority/keeper/authorization_list.go +++ b/x/authority/keeper/authorization_list.go @@ -43,13 +43,16 @@ func (k Keeper) IsAuthorized(ctx sdk.Context, address string, policyType types.P } // CheckAuthorization checks if the signer is authorized to sign the message +// It uses both the authorization list and the policies to check if the signer is authorized func (k Keeper) CheckAuthorization(ctx sdk.Context, msg sdk.Msg) error { // Policy transactions must have only one signer if len(msg.GetSigners()) != 1 { return errors.Wrap(types.ErrSigners, fmt.Sprintf("msg: %v", sdk.MsgTypeURL(msg))) } + signer := msg.GetSigners()[0].String() msgURL := sdk.MsgTypeURL(msg) + authorizationsList, found := k.GetAuthorizationList(ctx) if !found { return types.ErrAuthorizationListNotFound @@ -58,10 +61,10 @@ func (k Keeper) CheckAuthorization(ctx sdk.Context, msg sdk.Msg) error { if err != nil { return errors.Wrap(types.ErrAuthorizationNotFound, fmt.Sprintf("msg: %v", msgURL)) } - //// TODO : check for empty policy - //if policyRequired == types.PolicyType_groupOperational { - // return errors.Wrap(types.ErrMsgNotAuthorized, fmt.Sprintf("msg: %v", sdk.MsgTypeURL(msg))) - //} + if policyRequired == types.PolicyType_groupEmpty { + return errors.Wrap(types.ErrInvalidPolicyType, fmt.Sprintf("Empty policy for msg: %v", msgURL)) + } + policies, found := k.GetPolicies(ctx) if !found { return errors.Wrap(types.ErrPoliciesNotFound, fmt.Sprintf("msg: %v", msgURL)) diff --git a/x/authority/keeper/authorization_list_test.go b/x/authority/keeper/authorization_list_test.go index 2dddb2f0f4..ef3c5ae660 100644 --- a/x/authority/keeper/authorization_list_test.go +++ b/x/authority/keeper/authorization_list_test.go @@ -5,11 +5,11 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" - lightclienttypes "github.com/zeta-chain/zetacore/x/lightclient/types" keepertest "github.com/zeta-chain/zetacore/testutil/keeper" "github.com/zeta-chain/zetacore/testutil/sample" "github.com/zeta-chain/zetacore/x/authority/types" + lightclienttypes "github.com/zeta-chain/zetacore/x/lightclient/types" ) func TestKeeper_GetAuthorizationList(t *testing.T) { @@ -201,4 +201,32 @@ func TestKeeper_CheckAuthorization(t *testing.T) { err := k.CheckAuthorization(ctx, &msg) require.ErrorIs(t, err, types.ErrSignerDoesntMatch) }) + + t.Run("unable to check authorization when the required policy is empty", func(t *testing.T) { + k, ctx := keepertest.AuthorityKeeper(t) + signer := sample.AccAddress() + msg := lightclienttypes.MsgDisableHeaderVerification{ + Creator: signer, + } + authorizationList := types.AuthorizationList{Authorizations: []types.Authorization{ + { + MsgUrl: sdk.MsgTypeURL(&msg), + AuthorizedPolicy: types.PolicyType_groupEmpty, + }, + }, + } + policies := types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupOperational, + }, + }, + } + k.SetPolicies(ctx, policies) + k.SetAuthorizationList(ctx, authorizationList) + + err := k.CheckAuthorization(ctx, &msg) + require.ErrorIs(t, err, types.ErrInvalidPolicyType) + }) } diff --git a/x/authority/types/authorizations.go b/x/authority/types/authorization_list.go similarity index 100% rename from x/authority/types/authorizations.go rename to x/authority/types/authorization_list.go diff --git a/x/authority/types/authorizations_test.go b/x/authority/types/authorization_list_test.go similarity index 100% rename from x/authority/types/authorizations_test.go rename to x/authority/types/authorization_list_test.go diff --git a/x/authority/types/errors.go b/x/authority/types/errors.go index 2fbc3928d6..776132d525 100644 --- a/x/authority/types/errors.go +++ b/x/authority/types/errors.go @@ -11,4 +11,5 @@ var ( ErrMsgNotAuthorized = errorsmod.Register(ModuleName, 1107, "msg type is not authorized") ErrPoliciesNotFound = errorsmod.Register(ModuleName, 1108, "policies not found") ErrSignerDoesntMatch = errorsmod.Register(ModuleName, 1109, "signer doesn't match required policy") + ErrInvalidPolicyType = errorsmod.Register(ModuleName, 1110, "invalid policy type") ) From cf3cf0ac3953efe591867040c480851de4c6d4e6 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Tue, 4 Jun 2024 18:53:05 -0400 Subject: [PATCH 5/8] rebase develop --- x/authority/types/authorization_list.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/authority/types/authorization_list.go b/x/authority/types/authorization_list.go index 8ec0fa7d00..848b1da1d2 100644 --- a/x/authority/types/authorization_list.go +++ b/x/authority/types/authorization_list.go @@ -6,8 +6,6 @@ import ( "cosmossdk.io/errors" ) -// TODO : Refactor this file to authorization_list.go - var ( // OperationPolicyMessages keeps track of the message URLs that can, by default, only be executed by operational policy address OperationPolicyMessages = []string{ From 8778140ab0c503886b096f42b09a8109323b4460 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Wed, 5 Jun 2024 10:13:49 -0400 Subject: [PATCH 6/8] fix comments --- testutil/sample/authority.go | 1 + x/authority/keeper/authorization_list.go | 3 +- x/authority/keeper/authorization_list_test.go | 63 +++++++++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/testutil/sample/authority.go b/testutil/sample/authority.go index 24c49d3123..c32f1621e4 100644 --- a/testutil/sample/authority.go +++ b/testutil/sample/authority.go @@ -72,6 +72,7 @@ func MultipleSignerMessage() sdk.Msg { return &TestMessage{} } +// TestMessage is a sample message which has two signers instead of one. This is used to test cases when we have checks for number of signers such as authorized transactions. type TestMessage struct{} var _ sdk.Msg = &TestMessage{} diff --git a/x/authority/keeper/authorization_list.go b/x/authority/keeper/authorization_list.go index a28eb24ede..24255bd089 100644 --- a/x/authority/keeper/authorization_list.go +++ b/x/authority/keeper/authorization_list.go @@ -47,7 +47,7 @@ func (k Keeper) IsAuthorized(ctx sdk.Context, address string, policyType types.P func (k Keeper) CheckAuthorization(ctx sdk.Context, msg sdk.Msg) error { // Policy transactions must have only one signer if len(msg.GetSigners()) != 1 { - return errors.Wrap(types.ErrSigners, fmt.Sprintf("msg: %v", sdk.MsgTypeURL(msg))) + return errors.Wrapf(types.ErrSigners, "msg: %v", sdk.MsgTypeURL(msg)) } signer := msg.GetSigners()[0].String() @@ -57,6 +57,7 @@ func (k Keeper) CheckAuthorization(ctx sdk.Context, msg sdk.Msg) error { if !found { return types.ErrAuthorizationListNotFound } + policyRequired, err := authorizationsList.GetAuthorizedPolicy(msgURL) if err != nil { return errors.Wrap(types.ErrAuthorizationNotFound, fmt.Sprintf("msg: %v", msgURL)) diff --git a/x/authority/keeper/authorization_list_test.go b/x/authority/keeper/authorization_list_test.go index ef3c5ae660..6fd2357d74 100644 --- a/x/authority/keeper/authorization_list_test.go +++ b/x/authority/keeper/authorization_list_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "fmt" "testing" sdk "github.com/cosmos/cosmos-sdk/types" @@ -80,6 +81,68 @@ func TestKeeper_CheckAuthorization(t *testing.T) { require.NoError(t, err) }) + t.Run("successfully check authorization against large authorization list", func(t *testing.T) { + k, ctx := keepertest.AuthorityKeeper(t) + signer := sample.AccAddress() + msg := lightclienttypes.MsgDisableHeaderVerification{ + Creator: signer, + } + authorizationList := types.DefaultAuthorizationsList() + // Add 300 more authorizations to the list + for i := 0; i < 100; i++ { + authorizationList.Authorizations = append(authorizationList.Authorizations, sample.AuthorizationList(fmt.Sprintf("sample%d", i)).Authorizations...) + } + policies := types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupEmergency, + }, + }, + } + + k.SetPolicies(ctx, policies) + k.SetAuthorizationList(ctx, authorizationList) + + err := k.CheckAuthorization(ctx, &msg) + require.NoError(t, err) + + list, found := k.GetAuthorizationList(ctx) + require.True(t, found) + require.Equal(t, authorizationList, list) + }) + + t.Run("check authorization against fails against large authorization list", func(t *testing.T) { + k, ctx := keepertest.AuthorityKeeper(t) + signer := sample.AccAddress() + msg := lightclienttypes.MsgDisableHeaderVerification{ + Creator: signer, + } + authorizationList := types.AuthorizationList{} + // Add 300 more authorizations to the list + for i := 0; i < 100; i++ { + authorizationList.Authorizations = append(authorizationList.Authorizations, sample.AuthorizationList(fmt.Sprintf("sample%d", i)).Authorizations...) + } + policies := types.Policies{ + Items: []*types.Policy{ + { + Address: signer, + PolicyType: types.PolicyType_groupEmergency, + }, + }, + } + + k.SetPolicies(ctx, policies) + k.SetAuthorizationList(ctx, authorizationList) + + err := k.CheckAuthorization(ctx, &msg) + require.ErrorIs(t, err, types.ErrAuthorizationNotFound) + + list, found := k.GetAuthorizationList(ctx) + require.True(t, found) + require.Equal(t, authorizationList, list) + }) + t.Run("unable to check authorization with multiple signers", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) signer := sample.AccAddress() From c2a8bf49c65241f0997ff25d6fb3eba9de8b7882 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Wed, 5 Jun 2024 10:28:53 -0400 Subject: [PATCH 7/8] generate files --- x/authority/keeper/authorization_list_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x/authority/keeper/authorization_list_test.go b/x/authority/keeper/authorization_list_test.go index 6fd2357d74..0e8ef69b5c 100644 --- a/x/authority/keeper/authorization_list_test.go +++ b/x/authority/keeper/authorization_list_test.go @@ -90,7 +90,9 @@ func TestKeeper_CheckAuthorization(t *testing.T) { authorizationList := types.DefaultAuthorizationsList() // Add 300 more authorizations to the list for i := 0; i < 100; i++ { - authorizationList.Authorizations = append(authorizationList.Authorizations, sample.AuthorizationList(fmt.Sprintf("sample%d", i)).Authorizations...) + authorizationList.Authorizations = append( + authorizationList.Authorizations, + sample.AuthorizationList(fmt.Sprintf("sample%d", i)).Authorizations...) } policies := types.Policies{ Items: []*types.Policy{ @@ -121,7 +123,9 @@ func TestKeeper_CheckAuthorization(t *testing.T) { authorizationList := types.AuthorizationList{} // Add 300 more authorizations to the list for i := 0; i < 100; i++ { - authorizationList.Authorizations = append(authorizationList.Authorizations, sample.AuthorizationList(fmt.Sprintf("sample%d", i)).Authorizations...) + authorizationList.Authorizations = append( + authorizationList.Authorizations, + sample.AuthorizationList(fmt.Sprintf("sample%d", i)).Authorizations...) } policies := types.Policies{ Items: []*types.Policy{ From fe9590a52cbd574fd233eb533cb5532fd1085467 Mon Sep 17 00:00:00 2001 From: Tanmay Date: Thu, 6 Jun 2024 09:48:05 -0400 Subject: [PATCH 8/8] comments 3 --- testutil/sample/authority.go | 20 ++++++++----------- x/authority/keeper/authorization_list_test.go | 2 +- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/testutil/sample/authority.go b/testutil/sample/authority.go index c32f1621e4..be9ca2c555 100644 --- a/testutil/sample/authority.go +++ b/testutil/sample/authority.go @@ -68,20 +68,16 @@ func Authorization() authoritytypes.Authorization { } } -func MultipleSignerMessage() sdk.Msg { - return &TestMessage{} -} - -// TestMessage is a sample message which has two signers instead of one. This is used to test cases when we have checks for number of signers such as authorized transactions. -type TestMessage struct{} +// MultipleSignerMessage is a sample message which has two signers instead of one. This is used to test cases when we have checks for number of signers such as authorized transactions. +type MultipleSignerMessage struct{} -var _ sdk.Msg = &TestMessage{} +var _ sdk.Msg = &MultipleSignerMessage{} -func (m *TestMessage) Reset() {} -func (m *TestMessage) String() string { return "TestMessage" } -func (m *TestMessage) ProtoMessage() {} -func (m *TestMessage) ValidateBasic() error { return nil } -func (m *TestMessage) GetSigners() []sdk.AccAddress { +func (m *MultipleSignerMessage) Reset() {} +func (m *MultipleSignerMessage) String() string { return "MultipleSignerMessage" } +func (m *MultipleSignerMessage) ProtoMessage() {} +func (m *MultipleSignerMessage) ValidateBasic() error { return nil } +func (m *MultipleSignerMessage) GetSigners() []sdk.AccAddress { return []sdk.AccAddress{ sdk.MustAccAddressFromBech32(AccAddress()), sdk.MustAccAddressFromBech32(AccAddress()), diff --git a/x/authority/keeper/authorization_list_test.go b/x/authority/keeper/authorization_list_test.go index 0e8ef69b5c..a4c0d0e7bd 100644 --- a/x/authority/keeper/authorization_list_test.go +++ b/x/authority/keeper/authorization_list_test.go @@ -150,7 +150,7 @@ func TestKeeper_CheckAuthorization(t *testing.T) { t.Run("unable to check authorization with multiple signers", func(t *testing.T) { k, ctx := keepertest.AuthorityKeeper(t) signer := sample.AccAddress() - msg := sample.MultipleSignerMessage() + msg := &sample.MultipleSignerMessage{} authorizationList := types.AuthorizationList{Authorizations: []types.Authorization{ { MsgUrl: sdk.MsgTypeURL(msg),