refactor: use CheckAuthorization instead of IsAuthorized (#2319)

cmd/zetacored/parsers_test.go

@@ -6,11 +6,10 @@ import (
 	"os"
 	"testing"
 
-	"github.com/cometbft/cometbft/crypto"
-	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/require"
 
 	"github.com/zeta-chain/zetacore/app"
+	"github.com/zeta-chain/zetacore/testutil/sample"
 )
 
 func TestParsefileToObserverMapper(t *testing.T) {
@@ -20,31 +19,38 @@ func TestParsefileToObserverMapper(t *testing.T) {
 		require.NoError(t, err)
 	}(t, file)
 	app.SetConfig()
-	createObserverList(file)
+
+	observerAddress := sample.AccAddress()
+	commonGrantAddress := sample.AccAddress()
+	validatorAddress := sample.AccAddress()
+
+	createObserverList(file, observerAddress, commonGrantAddress, validatorAddress)
 	obsListReadFromFile, err := ParsefileToObserverDetails(file)
 	require.NoError(t, err)
 	for _, obs := range obsListReadFromFile {
+		require.Equal(
+			t,
+			obs.ObserverAddress,
+			observerAddress,
+		)
 		require.Equal(
 			t,
 			obs.ZetaClientGranteeAddress,
-			sdk.AccAddress(crypto.AddressHash([]byte("ObserverGranteeAddress"))).String(),
+			commonGrantAddress,
 		)
 	}
 }
 
-func createObserverList(fp string) {
+func createObserverList(fp string, observerAddress, commonGrantAddress, validatorAddress string) {
 	var listReader []ObserverInfoReader
-	commonGrantAddress := sdk.AccAddress(crypto.AddressHash([]byte("ObserverGranteeAddress")))
-	observerAddress := sdk.AccAddress(crypto.AddressHash([]byte("ObserverAddress")))
-	validatorAddress := sdk.ValAddress(crypto.AddressHash([]byte("ValidatorAddress")))
 	info := ObserverInfoReader{
-		ObserverAddress:           observerAddress.String(),
-		ZetaClientGranteeAddress:  commonGrantAddress.String(),
-		StakingGranteeAddress:     commonGrantAddress.String(),
+		ObserverAddress:           observerAddress,
+		ZetaClientGranteeAddress:  commonGrantAddress,
+		StakingGranteeAddress:     commonGrantAddress,
 		StakingMaxTokens:          "100000000",
-		StakingValidatorAllowList: []string{validatorAddress.String()},
+		StakingValidatorAllowList: []string{validatorAddress},
 		SpendMaxTokens:            "100000000",
-		GovGranteeAddress:         commonGrantAddress.String(),
+		GovGranteeAddress:         commonGrantAddress,
 		ZetaClientGranteePubKey:   "zetapub1addwnpepqggtjvkmj6apcqr6ynyc5edxf2mpf5fxp2d3kwupemxtfwvg6gm7qv79fw0",
 	}
 	listReader = append(listReader, info)

testutil/keeper/authority.go

@@ -74,9 +74,9 @@ func AuthorityKeeper(t testing.TB) (*keeper.Keeper, sdk.Context) {
 	return &k, ctx
 }
 
-// MockIsAuthorized mocks the IsAuthorized method of an authority keeper mock
-func MockIsAuthorized(m *mock.Mock, address string, policyType types.PolicyType, isAuthorized bool) {
-	m.On("IsAuthorized", mock.Anything, address, policyType).Return(isAuthorized).Once()
+// MockCheckAuthorization mocks the CheckAuthorization method of the authority keeper.
+func MockCheckAuthorization(m *mock.Mock, msg sdk.Msg, authorizationResult error) {
+	m.On("CheckAuthorization", mock.Anything, msg).Return(authorizationResult).Once()
 }
 
 func SetAdminPolicies(ctx sdk.Context, ak *keeper.Keeper) string {

x/authority/keeper/authorization_list.go

@@ -28,22 +28,7 @@ func (k Keeper) GetAuthorizationList(ctx sdk.Context) (val types.AuthorizationLi
 	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
-// It uses both the authorization list and the policies to check if the signer is authorized
+// CheckAuthorization 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 {

x/authority/keeper/msg_server_add_authorization.go

@@ -17,11 +17,10 @@ func (k msgServer) AddAuthorization(
 ) (*types.MsgAddAuthorizationResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	if !k.IsAuthorized(ctx, msg.Creator, types.PolicyType_groupAdmin) {
-		return nil, errorsmod.Wrap(
-			types.ErrUnauthorized,
-			"AddAuthorization can only be executed by the admin policy account",
-		)
+	// check if the caller is authorized to add an authorization
+	err := k.CheckAuthorization(ctx, msg)
+	if err != nil {
+		return nil, errorsmod.Wrap(types.ErrUnauthorized, err.Error())
 	}
 
 	authorizationList, found := k.GetAuthorizationList(ctx)
@@ -31,7 +30,7 @@ func (k msgServer) AddAuthorization(
 	authorizationList.SetAuthorization(types.Authorization{MsgUrl: msg.MsgUrl, AuthorizedPolicy: msg.AuthorizedPolicy})
 
 	// validate the authorization list after adding the authorization as a precautionary measure.
-	err := authorizationList.Validate()
+	err = authorizationList.Validate()
 	if err != nil {
 		return nil, errorsmod.Wrap(err, "authorization list is invalid")
 	}

x/authority/keeper/msg_server_add_authorization_test.go

@@ -14,6 +14,10 @@ import (
 
 func TestMsgServer_AddAuthorization(t *testing.T) {
 	const url = "/zetachain.zetacore.sample.ABC"
+	var AddAuthorization = types.Authorization{
+		MsgUrl:           "/zetachain.zetacore.authority.MsgAddAuthorization",
+		AuthorizedPolicy: types.PolicyType_groupAdmin,
+	}
 	t.Run("successfully add authorization of type admin to existing authorization list", func(t *testing.T) {
 		k, ctx := keepertest.AuthorityKeeper(t)
 		admin := keepertest.SetAdminPolicies(ctx, k)
@@ -86,35 +90,41 @@ func TestMsgServer_AddAuthorization(t *testing.T) {
 		require.Equal(t, prevLen+1, len(authorizationList.Authorizations))
 	})
 
-	t.Run("successfully add authorization to empty authorization list", func(t *testing.T) {
-		k, ctx := keepertest.AuthorityKeeper(t)
-		admin := keepertest.SetAdminPolicies(ctx, k)
-		k.SetAuthorizationList(ctx, types.AuthorizationList{})
-		msgServer := keeper.NewMsgServerImpl(*k)
-
-		msg := &types.MsgAddAuthorization{
-			Creator:          admin,
-			MsgUrl:           url,
-			AuthorizedPolicy: types.PolicyType_groupAdmin,
-		}
-
-		_, err := msgServer.AddAuthorization(sdk.WrapSDKContext(ctx), msg)
-		require.NoError(t, err)
-
-		authorizationList, found := k.GetAuthorizationList(ctx)
-		require.True(t, found)
-		policy, err := authorizationList.GetAuthorizedPolicy(url)
-		require.NoError(t, err)
-		require.Equal(t, types.PolicyType_groupAdmin, policy)
-		require.Equal(t, 1, len(authorizationList.Authorizations))
-	})
+	t.Run(
+		"successfully add authorization to list containing only authorization for AddAuthorization",
+		func(t *testing.T) {
+			k, ctx := keepertest.AuthorityKeeper(t)
+			admin := keepertest.SetAdminPolicies(ctx, k)
+			k.SetAuthorizationList(ctx, types.AuthorizationList{
+				Authorizations: []types.Authorization{
+					AddAuthorization,
+				},
+			})
+			msgServer := keeper.NewMsgServerImpl(*k)
+
+			msg := &types.MsgAddAuthorization{
+				Creator:          admin,
+				MsgUrl:           url,
+				AuthorizedPolicy: types.PolicyType_groupAdmin,
+			}
+
+			_, err := msgServer.AddAuthorization(sdk.WrapSDKContext(ctx), msg)
+			require.NoError(t, err)
+
+			authorizationList, found := k.GetAuthorizationList(ctx)
+			require.True(t, found)
+			policy, err := authorizationList.GetAuthorizedPolicy(url)
+			require.NoError(t, err)
+			require.Equal(t, types.PolicyType_groupAdmin, policy)
+			require.Equal(t, 2, len(authorizationList.Authorizations))
+		},
+	)
 
-	t.Run("successfully set authorization when list is not found ", func(t *testing.T) {
+	t.Run("unable to add authorization to empty authorization list", func(t *testing.T) {
 		k, ctx := keepertest.AuthorityKeeper(t)
 		admin := keepertest.SetAdminPolicies(ctx, k)
+		k.SetAuthorizationList(ctx, types.AuthorizationList{})
 		msgServer := keeper.NewMsgServerImpl(*k)
-		authorizationList, found := k.GetAuthorizationList(ctx)
-		require.False(t, found)
 
 		msg := &types.MsgAddAuthorization{
 			Creator:          admin,
@@ -123,14 +133,7 @@ func TestMsgServer_AddAuthorization(t *testing.T) {
 		}
 
 		_, err := msgServer.AddAuthorization(sdk.WrapSDKContext(ctx), msg)
-		require.NoError(t, err)
-
-		authorizationList, found = k.GetAuthorizationList(ctx)
-		require.True(t, found)
-		policy, err := authorizationList.GetAuthorizedPolicy(url)
-		require.NoError(t, err)
-		require.Equal(t, types.PolicyType_groupAdmin, policy)
-		require.Equal(t, 1, len(authorizationList.Authorizations))
+		require.ErrorIs(t, err, types.ErrUnauthorized)
 	})
 
 	t.Run("update existing authorization", func(t *testing.T) {
@@ -141,6 +144,7 @@ func TestMsgServer_AddAuthorization(t *testing.T) {
 				MsgUrl:           "/zetachain.zetacore.sample.ABC",
 				AuthorizedPolicy: types.PolicyType_groupOperational,
 			},
+			AddAuthorization,
 		},
 		}
 		k.SetAuthorizationList(ctx, authorizationList)
@@ -198,6 +202,7 @@ func TestMsgServer_AddAuthorization(t *testing.T) {
 				MsgUrl:           url,
 				AuthorizedPolicy: types.PolicyType_groupOperational,
 			},
+			AddAuthorization,
 		}}
 		k.SetAuthorizationList(ctx, authorizationList)
 		prevLen := len(authorizationList.Authorizations)