refactor: use CheckAuthorization instead of IsAuthorized

changelog.md

@@ -25,6 +25,7 @@
 * [2305](https://github.com/zeta-chain/node/pull/2305) - add new messages `MsgAddAuthorization` and `MsgRemoveAuthorization` that can be used to update the authorization list
 * [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.
 * [2312](https://github.com/zeta-chain/node/pull/2312) - add queries `ShowAuthorization` and `ListAuthorizations`
+* [2319](https://github.com/zeta-chain/node/pull/2319) - use `CheckAuthorization` function in all messages .
 
 ### Refactor
 

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.ErrorContains(t, err, types.ErrUnauthorized.Error())
 	})
 
 	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)

x/authority/keeper/msg_server_remove_authorization.go

@@ -18,11 +18,10 @@ func (k msgServer) RemoveAuthorization(
 ) (*types.MsgRemoveAuthorizationResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	if !k.IsAuthorized(ctx, msg.Creator, types.PolicyType_groupAdmin) {
-		return nil, errorsmod.Wrap(
-			types.ErrUnauthorized,
-			"RemoveAuthorization can only be executed by the admin policy account",
-		)
+	// check if the caller is authorized to remove an authorization
+	err := k.CheckAuthorization(ctx, msg)
+	if err != nil {
+		return nil, errorsmod.Wrap(types.ErrUnauthorized, err.Error())
 	}
 
 	// check if the authorization list exists, we can return early if there is no list.
@@ -32,7 +31,7 @@ func (k msgServer) RemoveAuthorization(
 	}
 
 	// check if the authorization exists, we can return early if the authorization does not exist.
-	_, err := authorizationList.GetAuthorizedPolicy(msg.MsgUrl)
+	_, err = authorizationList.GetAuthorizedPolicy(msg.MsgUrl)
 	if err != nil {
 		return nil, errorsmod.Wrap(err, fmt.Sprintf("msg url %s", msg.MsgUrl))
 	}