chore: x/authz audit changes

x/authz/keeper/grpc_query.go

@@ -16,7 +16,7 @@ import (
 
 var _ authz.QueryServer = Keeper{}
 
-// Authorizations implements the Query/Grants gRPC method.
+// Grants implements the Query/Grants gRPC method.
 // It returns grants for a granter-grantee pair. If msg type URL is set, it returns grants only for that msg type.
 func (k Keeper) Grants(c context.Context, req *authz.QueryGrantsRequest) (*authz.QueryGrantsResponse, error) {
 	if req == nil {
@@ -173,6 +173,7 @@ func (k Keeper) GranteeGrants(c context.Context, req *authz.QueryGranteeGrantsRe
 	}, func() *authz.Grant {
 		return &authz.Grant{}
 	})
+
 	if err != nil {
 		return nil, err
 	}

x/authz/keeper/grpc_query_test.go

@@ -302,11 +302,11 @@ func (suite *TestSuite) TestGRPCQueryGranteeGrants() {
 	}
 }
 
-func (suite *TestSuite) createSendAuthorization(a1, a2 sdk.AccAddress) authz.Authorization {
+func (suite *TestSuite) createSendAuthorization(grantee, granter sdk.AccAddress) authz.Authorization {
 	exp := suite.ctx.BlockHeader().Time.Add(time.Hour)
 	newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100))
 	authorization := &banktypes.SendAuthorization{SpendLimit: newCoins}
-	err := suite.authzKeeper.SaveGrant(suite.ctx, a1, a2, authorization, &exp)
+	err := suite.authzKeeper.SaveGrant(suite.ctx, grantee, granter, authorization, &exp)
 	suite.Require().NoError(err)
 	return authorization
 }

x/authz/keeper/keeper.go

@@ -177,11 +177,13 @@ func (k Keeper) SaveGrant(ctx sdk.Context, grantee, granter sdk.AccAddress, auth
 	if oldGrant, found := k.getGrant(ctx, skey); found {
 		oldExp = oldGrant.Expiration
 	}
+
 	if oldExp != nil && (expiration == nil || !oldExp.Equal(*expiration)) {
 		if err = k.removeFromGrantQueue(ctx, skey, granter, grantee, *oldExp); err != nil {
 			return err
 		}
 	}
+
 	// If the expiration didn't change, then we don't remove it and we should not insert again
 	if expiration != nil && (oldExp == nil || !oldExp.Equal(*expiration)) {
 		if err = k.insertIntoGrantQueue(ctx, granter, grantee, msgType, *expiration); err != nil {

x/authz/keeper/keeper_test.go

@@ -10,7 +10,6 @@ import (
 	tmtime "github.com/tendermint/tendermint/types/time"
 
 	"github.com/cosmos/cosmos-sdk/baseapp"
-	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 	"github.com/cosmos/cosmos-sdk/testutil"
 	simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -33,15 +32,15 @@ var (
 type TestSuite struct {
 	suite.Suite
 
-	ctx               sdk.Context
-	addrs             []sdk.AccAddress
-	authzKeeper       authzkeeper.Keeper
-	accountKeeper     *authztestutil.MockAccountKeeper
-	bankKeeper        *authztestutil.MockBankKeeper
-	interfaceRegistry codectypes.InterfaceRegistry
-	baseApp           *baseapp.BaseApp
-	encCfg            moduletestutil.TestEncodingConfig
-	queryClient       authz.QueryClient
+	ctx           sdk.Context
+	addrs         []sdk.AccAddress
+	authzKeeper   authzkeeper.Keeper
+	accountKeeper *authztestutil.MockAccountKeeper
+	bankKeeper    *authztestutil.MockBankKeeper
+	baseApp       *baseapp.BaseApp
+	encCfg        moduletestutil.TestEncodingConfig
+	queryClient   authz.QueryClient
+	msgSrvr       authz.MsgServer
 }
 
 func (s *TestSuite) SetupTest() {
@@ -75,7 +74,7 @@ func (s *TestSuite) SetupTest() {
 	queryClient := authz.NewQueryClient(queryHelper)
 	s.queryClient = queryClient
 
-	s.queryClient = queryClient
+	s.msgSrvr = s.authzKeeper
 }
 
 func (s *TestSuite) TestKeeper() {

x/authz/keeper/msg_server.go

@@ -10,7 +10,7 @@ import (
 
 var _ authz.MsgServer = Keeper{}
 
-// GrantAuthorization implements the MsgServer.Grant method to create a new grant.
+// Grant implements the MsgServer.Grant method to create a new grant.
 func (k Keeper) Grant(goCtx context.Context, msg *authz.MsgGrant) (*authz.MsgGrantResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 	grantee, err := sdk.AccAddressFromBech32(msg.Grantee)
@@ -48,7 +48,7 @@ func (k Keeper) Grant(goCtx context.Context, msg *authz.MsgGrant) (*authz.MsgGra
 	return &authz.MsgGrantResponse{}, nil
 }
 
-// RevokeAuthorization implements the MsgServer.Revoke method.
+// Revoke implements the MsgServer.Revoke method.
 func (k Keeper) Revoke(goCtx context.Context, msg *authz.MsgRevoke) (*authz.MsgRevokeResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 	grantee, err := sdk.AccAddressFromBech32(msg.Grantee)

x/authz/keeper/msg_server_test.go

@@ -0,0 +1,263 @@
+package keeper_test
+
+import (
+	"time"
+
+	simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	"github.com/cosmos/cosmos-sdk/x/authz"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+
+	"github.com/golang/mock/gomock"
+)
+
+func (suite *TestSuite) createAccounts(accs int) []sdk.AccAddress {
+	addrs := simtestutil.CreateIncrementalAccounts(2)
+	suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[0]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[0])).AnyTimes()
+	suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[1]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[1])).AnyTimes()
+	return addrs
+}
+
+func (suite *TestSuite) TestGrant() {
+	ctx := suite.ctx.WithBlockTime(time.Now())
+	addrs := suite.createAccounts(2)
+	curBlockTime := ctx.BlockTime()
+
+	oneHour := curBlockTime.Add(time.Hour)
+	oneYear := curBlockTime.AddDate(1, 0, 0)
+
+	coins := sdk.NewCoins(sdk.NewCoin("steak", sdk.NewInt(10)))
+
+	grantee, granter := addrs[0], addrs[1]
+
+	testCases := []struct {
+		name     string
+		malleate func() *authz.MsgGrant
+		expErr   bool
+		errMsg   string
+	}{
+		{
+			name: "invalid granter",
+			malleate: func() *authz.MsgGrant {
+				grant, err := authz.NewGrant(curBlockTime, banktypes.NewSendAuthorization(coins, nil), &oneYear)
+				suite.Require().NoError(err)
+				return &authz.MsgGrant{
+					Granter: "invalid",
+					Grantee: grantee.String(),
+					Grant:   grant,
+				}
+			},
+			expErr: true,
+			errMsg: "invalid bech32 string",
+		},
+		{
+			name: "invalid grantee",
+			malleate: func() *authz.MsgGrant {
+				grant, err := authz.NewGrant(curBlockTime, banktypes.NewSendAuthorization(coins, nil), &oneYear)
+				suite.Require().NoError(err)
+				return &authz.MsgGrant{
+					Granter: granter.String(),
+					Grantee: "invalid",
+					Grant:   grant,
+				}
+			},
+			expErr: true,
+			errMsg: "invalid bech32 string",
+		},
+		{
+			name: "grantee account does not exist on chain: valid grant",
+			malleate: func() *authz.MsgGrant {
+				newAcc := sdk.AccAddress("valid")
+				suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), newAcc).Return(nil).AnyTimes()
+				acc := authtypes.NewBaseAccountWithAddress(newAcc)
+				suite.accountKeeper.EXPECT().NewAccountWithAddress(gomock.Any(), newAcc).Return(acc).AnyTimes()
+				suite.accountKeeper.EXPECT().SetAccount(gomock.Any(), acc).Return()
+
+				grant, err := authz.NewGrant(curBlockTime, banktypes.NewSendAuthorization(coins, nil), &oneYear)
+				suite.Require().NoError(err)
+				return &authz.MsgGrant{
+					Granter: granter.String(),
+					Grantee: newAcc.String(),
+					Grant:   grant,
+				}
+			},
+		},
+		{
+			name: "valid grant",
+			malleate: func() *authz.MsgGrant {
+				grant, err := authz.NewGrant(curBlockTime, banktypes.NewSendAuthorization(coins, nil), &oneYear)
+				suite.Require().NoError(err)
+				return &authz.MsgGrant{
+					Granter: granter.String(),
+					Grantee: grantee.String(),
+					Grant:   grant,
+				}
+			},
+		},
+		{
+			name: "valid grant, same grantee, granter pair but different msgType",
+			malleate: func() *authz.MsgGrant {
+				g, err := authz.NewGrant(curBlockTime, banktypes.NewSendAuthorization(coins, nil), &oneHour)
+				suite.Require().NoError(err)
+				_, err = suite.msgSrvr.Grant(suite.ctx, &authz.MsgGrant{
+					Granter: granter.String(),
+					Grantee: grantee.String(),
+					Grant:   g,
+				})
+				suite.Require().NoError(err)
+
+				grant, err := authz.NewGrant(curBlockTime, authz.NewGenericAuthorization("/cosmos.bank.v1beta1.MsgUpdateParams"), &oneHour)
+				suite.Require().NoError(err)
+				return &authz.MsgGrant{
+					Granter: granter.String(),
+					Grantee: grantee.String(),
+					Grant:   grant,
+				}
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			_, err := suite.msgSrvr.Grant(suite.ctx, tc.malleate())
+			if tc.expErr {
+				suite.Require().Error(err)
+				suite.Require().Contains(err.Error(), tc.errMsg)
+			} else {
+				suite.Require().NoError(err)
+			}
+		})
+	}
+}
+
+func (suite *TestSuite) TestRevoke() {
+	addrs := suite.createAccounts(2)
+
+	grantee, granter := addrs[0], addrs[1]
+
+	testCases := []struct {
+		name     string
+		malleate func() *authz.MsgRevoke
+		expErr   bool
+		errMsg   string
+	}{
+		{
+			name: "invalid granter",
+			malleate: func() *authz.MsgRevoke {
+				return &authz.MsgRevoke{
+					Granter:    "invalid",
+					Grantee:    grantee.String(),
+					MsgTypeUrl: bankSendAuthMsgType,
+				}
+			},
+			expErr: true,
+			errMsg: "invalid bech32 string",
+		},
+		{
+			name: "invalid grantee",
+			malleate: func() *authz.MsgRevoke {
+				return &authz.MsgRevoke{
+					Granter:    granter.String(),
+					Grantee:    "invalid",
+					MsgTypeUrl: bankSendAuthMsgType,
+				}
+			},
+			expErr: true,
+			errMsg: "invalid bech32 string",
+		},
+		{
+			name: "valid grant",
+			malleate: func() *authz.MsgRevoke {
+				suite.createSendAuthorization(grantee, granter)
+
+				return &authz.MsgRevoke{
+					Granter:    granter.String(),
+					Grantee:    grantee.String(),
+					MsgTypeUrl: bankSendAuthMsgType,
+				}
+			},
+		},
+		{
+			name: "no existing grant to revoke",
+			malleate: func() *authz.MsgRevoke {
+				return &authz.MsgRevoke{
+					Granter:    granter.String(),
+					Grantee:    grantee.String(),
+					MsgTypeUrl: bankSendAuthMsgType,
+				}
+			},
+			expErr: true,
+			errMsg: "authorization not found",
+		},
+	}
+
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			_, err := suite.msgSrvr.Revoke(suite.ctx, tc.malleate())
+			if tc.expErr {
+				suite.Require().Error(err)
+				suite.Require().Contains(err.Error(), tc.errMsg)
+			} else {
+				suite.Require().NoError(err)
+			}
+		})
+	}
+}
+
+func (suite *TestSuite) TestExec() {
+	addrs := suite.createAccounts(2)
+
+	grantee, granter := addrs[0], addrs[1]
+	coins := sdk.NewCoins(sdk.NewCoin("steak", sdk.NewInt(10)))
+
+	msg := &banktypes.MsgSend{
+		FromAddress: granter.String(),
+		ToAddress:   grantee.String(),
+		Amount:      coins,
+	}
+
+	testCases := []struct {
+		name     string
+		malleate func() authz.MsgExec
+		expErr   bool
+		errMsg   string
+	}{
+		{
+			name: "invalid grantee",
+			malleate: func() authz.MsgExec {
+				return authz.NewMsgExec(sdk.AccAddress{}, []sdk.Msg{msg})
+			},
+			expErr: true,
+			errMsg: "empty address string is not allowed",
+		},
+		{
+			name: "no existing grant",
+			malleate: func() authz.MsgExec {
+				return authz.NewMsgExec(grantee, []sdk.Msg{msg})
+			},
+			expErr: true,
+			errMsg: "authorization not found",
+		},
+		{
+			name: "valid case",
+			malleate: func() authz.MsgExec {
+				suite.createSendAuthorization(grantee, granter)
+				return authz.NewMsgExec(grantee, []sdk.Msg{msg})
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		suite.Run(tc.name, func() {
+			req := tc.malleate()
+			_, err := suite.msgSrvr.Exec(suite.ctx, &req)
+			if tc.expErr {
+				suite.Require().Error(err)
+				suite.Require().Contains(err.Error(), tc.errMsg)
+			} else {
+				suite.Require().NoError(err)
+			}
+		})
+	}
+}