Skip to content

Commit

Permalink
chore: x/authz audit changes (cosmos#14120)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
3 people committed Dec 6, 2022
1 parent 4a7e3f0 commit 8a2c93f
Show file tree
Hide file tree
Showing 6 changed files with 281 additions and 16 deletions.
3 changes: 2 additions & 1 deletion x/authz/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions x/authz/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 2 additions & 0 deletions x/authz/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 10 additions & 11 deletions x/authz/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions x/authz/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
263 changes: 263 additions & 0 deletions x/authz/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}

0 comments on commit 8a2c93f

Please sign in to comment.