chore: x/authz audit changes (#14120)

* chore: x/authz audit changes

* nits

* tests

* Update x/authz/keeper/msg_server_test.go

Co-authored-by: Likhita Polavarapu <78951027+likhita-809@users.noreply.github.com>

Co-authored-by: Likhita Polavarapu <78951027+likhita-809@users.noreply.github.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
This commit is contained in:
atheeshp 2022-12-06 18:33:04 +05:30 committed by GitHub
parent 4a7e3f0522
commit 8a2c93fb7d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 281 additions and 16 deletions

View File

@ -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
}

View File

@ -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
}

View File

@ -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 {

View File

@ -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() {

View File

@ -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)

View File

@ -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)
}
})
}
}