Skip to content

Commit 8a2c93f

Browse files
atheeshplikhita-809tac0turtle
authored
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>
1 parent 4a7e3f0 commit 8a2c93f

File tree

6 files changed

+281
-16
lines changed

6 files changed

+281
-16
lines changed

‎x/authz/keeper/grpc_query.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616

1717
var _ authz.QueryServer = Keeper{}
1818

19-
// Authorizations implements the Query/Grants gRPC method.
19+
// Grants implements the Query/Grants gRPC method.
2020
// It returns grants for a granter-grantee pair. If msg type URL is set, it returns grants only for that msg type.
2121
func (k Keeper) Grants(c context.Context, req *authz.QueryGrantsRequest) (*authz.QueryGrantsResponse, error) {
2222
if req == nil {
@@ -173,6 +173,7 @@ func (k Keeper) GranteeGrants(c context.Context, req *authz.QueryGranteeGrantsRe
173173
}, func() *authz.Grant {
174174
return &authz.Grant{}
175175
})
176+
176177
if err != nil {
177178
return nil, err
178179
}

‎x/authz/keeper/grpc_query_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,11 +302,11 @@ func (suite *TestSuite) TestGRPCQueryGranteeGrants() {
302302
}
303303
}
304304

305-
func (suite *TestSuite) createSendAuthorization(a1, a2 sdk.AccAddress) authz.Authorization {
305+
func (suite *TestSuite) createSendAuthorization(grantee, granter sdk.AccAddress) authz.Authorization {
306306
exp := suite.ctx.BlockHeader().Time.Add(time.Hour)
307307
newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100))
308308
authorization := &banktypes.SendAuthorization{SpendLimit: newCoins}
309-
err := suite.authzKeeper.SaveGrant(suite.ctx, a1, a2, authorization, &exp)
309+
err := suite.authzKeeper.SaveGrant(suite.ctx, grantee, granter, authorization, &exp)
310310
suite.Require().NoError(err)
311311
return authorization
312312
}

‎x/authz/keeper/keeper.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,13 @@ func (k Keeper) SaveGrant(ctx sdk.Context, grantee, granter sdk.AccAddress, auth
177177
if oldGrant, found := k.getGrant(ctx, skey); found {
178178
oldExp = oldGrant.Expiration
179179
}
180+
180181
if oldExp != nil && (expiration == nil || !oldExp.Equal(*expiration)) {
181182
if err = k.removeFromGrantQueue(ctx, skey, granter, grantee, *oldExp); err != nil {
182183
return err
183184
}
184185
}
186+
185187
// If the expiration didn't change, then we don't remove it and we should not insert again
186188
if expiration != nil && (oldExp == nil || !oldExp.Equal(*expiration)) {
187189
if err = k.insertIntoGrantQueue(ctx, granter, grantee, msgType, *expiration); err != nil {

‎x/authz/keeper/keeper_test.go‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
tmtime "github.com/tendermint/tendermint/types/time"
1111

1212
"github.com/cosmos/cosmos-sdk/baseapp"
13-
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
1413
"github.com/cosmos/cosmos-sdk/testutil"
1514
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
1615
sdk "github.com/cosmos/cosmos-sdk/types"
@@ -33,15 +32,15 @@ var (
3332
type TestSuite struct {
3433
suite.Suite
3534

36-
ctx sdk.Context
37-
addrs []sdk.AccAddress
38-
authzKeeper authzkeeper.Keeper
39-
accountKeeper *authztestutil.MockAccountKeeper
40-
bankKeeper *authztestutil.MockBankKeeper
41-
interfaceRegistry codectypes.InterfaceRegistry
42-
baseApp *baseapp.BaseApp
43-
encCfg moduletestutil.TestEncodingConfig
44-
queryClient authz.QueryClient
35+
ctx sdk.Context
36+
addrs []sdk.AccAddress
37+
authzKeeper authzkeeper.Keeper
38+
accountKeeper *authztestutil.MockAccountKeeper
39+
bankKeeper *authztestutil.MockBankKeeper
40+
baseApp *baseapp.BaseApp
41+
encCfg moduletestutil.TestEncodingConfig
42+
queryClient authz.QueryClient
43+
msgSrvr authz.MsgServer
4544
}
4645

4746
func (s *TestSuite) SetupTest() {
@@ -75,7 +74,7 @@ func (s *TestSuite) SetupTest() {
7574
queryClient := authz.NewQueryClient(queryHelper)
7675
s.queryClient = queryClient
7776

78-
s.queryClient = queryClient
77+
s.msgSrvr = s.authzKeeper
7978
}
8079

8180
func (s *TestSuite) TestKeeper() {

‎x/authz/keeper/msg_server.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010

1111
var _ authz.MsgServer = Keeper{}
1212

13-
// GrantAuthorization implements the MsgServer.Grant method to create a new grant.
13+
// Grant implements the MsgServer.Grant method to create a new grant.
1414
func (k Keeper) Grant(goCtx context.Context, msg *authz.MsgGrant) (*authz.MsgGrantResponse, error) {
1515
ctx := sdk.UnwrapSDKContext(goCtx)
1616
grantee, err := sdk.AccAddressFromBech32(msg.Grantee)
@@ -48,7 +48,7 @@ func (k Keeper) Grant(goCtx context.Context, msg *authz.MsgGrant) (*authz.MsgGra
4848
return &authz.MsgGrantResponse{}, nil
4949
}
5050

51-
// RevokeAuthorization implements the MsgServer.Revoke method.
51+
// Revoke implements the MsgServer.Revoke method.
5252
func (k Keeper) Revoke(goCtx context.Context, msg *authz.MsgRevoke) (*authz.MsgRevokeResponse, error) {
5353
ctx := sdk.UnwrapSDKContext(goCtx)
5454
grantee, err := sdk.AccAddressFromBech32(msg.Grantee)

‎x/authz/keeper/msg_server_test.go‎

Lines changed: 263 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,263 @@
1+
package keeper_test
2+
3+
import (
4+
"time"
5+
6+
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
7+
sdk "github.com/cosmos/cosmos-sdk/types"
8+
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
9+
"github.com/cosmos/cosmos-sdk/x/authz"
10+
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
11+
12+
"github.com/golang/mock/gomock"
13+
)
14+
15+
func (suite *TestSuite) createAccounts(accs int) []sdk.AccAddress {
16+
addrs := simtestutil.CreateIncrementalAccounts(2)
17+
suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[0]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[0])).AnyTimes()
18+
suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[1]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[1])).AnyTimes()
19+
return addrs
20+
}
21+
22+
func (suite *TestSuite) TestGrant() {
23+
ctx := suite.ctx.WithBlockTime(time.Now())
24+
addrs := suite.createAccounts(2)
25+
curBlockTime := ctx.BlockTime()
26+
27+
oneHour := curBlockTime.Add(time.Hour)
28+
oneYear := curBlockTime.AddDate(1, 0, 0)
29+
30+
coins := sdk.NewCoins(sdk.NewCoin("steak", sdk.NewInt(10)))
31+
32+
grantee, granter := addrs[0], addrs[1]
33+
34+
testCases := []struct {
35+
name string
36+
malleate func() *authz.MsgGrant
37+
expErr bool
38+
errMsg string
39+
}{
40+
{
41+
name: "invalid granter",
42+
malleate: func() *authz.MsgGrant {
43+
grant, err := authz.NewGrant(curBlockTime, banktypes.NewSendAuthorization(coins, nil), &oneYear)
44+
suite.Require().NoError(err)
45+
return &authz.MsgGrant{
46+
Granter: "invalid",
47+
Grantee: grantee.String(),
48+
Grant: grant,
49+
}
50+
},
51+
expErr: true,
52+
errMsg: "invalid bech32 string",
53+
},
54+
{
55+
name: "invalid grantee",
56+
malleate: func() *authz.MsgGrant {
57+
grant, err := authz.NewGrant(curBlockTime, banktypes.NewSendAuthorization(coins, nil), &oneYear)
58+
suite.Require().NoError(err)
59+
return &authz.MsgGrant{
60+
Granter: granter.String(),
61+
Grantee: "invalid",
62+
Grant: grant,
63+
}
64+
},
65+
expErr: true,
66+
errMsg: "invalid bech32 string",
67+
},
68+
{
69+
name: "grantee account does not exist on chain: valid grant",
70+
malleate: func() *authz.MsgGrant {
71+
newAcc := sdk.AccAddress("valid")
72+
suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), newAcc).Return(nil).AnyTimes()
73+
acc := authtypes.NewBaseAccountWithAddress(newAcc)
74+
suite.accountKeeper.EXPECT().NewAccountWithAddress(gomock.Any(), newAcc).Return(acc).AnyTimes()
75+
suite.accountKeeper.EXPECT().SetAccount(gomock.Any(), acc).Return()
76+
77+
grant, err := authz.NewGrant(curBlockTime, banktypes.NewSendAuthorization(coins, nil), &oneYear)
78+
suite.Require().NoError(err)
79+
return &authz.MsgGrant{
80+
Granter: granter.String(),
81+
Grantee: newAcc.String(),
82+
Grant: grant,
83+
}
84+
},
85+
},
86+
{
87+
name: "valid grant",
88+
malleate: func() *authz.MsgGrant {
89+
grant, err := authz.NewGrant(curBlockTime, banktypes.NewSendAuthorization(coins, nil), &oneYear)
90+
suite.Require().NoError(err)
91+
return &authz.MsgGrant{
92+
Granter: granter.String(),
93+
Grantee: grantee.String(),
94+
Grant: grant,
95+
}
96+
},
97+
},
98+
{
99+
name: "valid grant, same grantee, granter pair but different msgType",
100+
malleate: func() *authz.MsgGrant {
101+
g, err := authz.NewGrant(curBlockTime, banktypes.NewSendAuthorization(coins, nil), &oneHour)
102+
suite.Require().NoError(err)
103+
_, err = suite.msgSrvr.Grant(suite.ctx, &authz.MsgGrant{
104+
Granter: granter.String(),
105+
Grantee: grantee.String(),
106+
Grant: g,
107+
})
108+
suite.Require().NoError(err)
109+
110+
grant, err := authz.NewGrant(curBlockTime, authz.NewGenericAuthorization("/cosmos.bank.v1beta1.MsgUpdateParams"), &oneHour)
111+
suite.Require().NoError(err)
112+
return &authz.MsgGrant{
113+
Granter: granter.String(),
114+
Grantee: grantee.String(),
115+
Grant: grant,
116+
}
117+
},
118+
},
119+
}
120+
121+
for _, tc := range testCases {
122+
suite.Run(tc.name, func() {
123+
_, err := suite.msgSrvr.Grant(suite.ctx, tc.malleate())
124+
if tc.expErr {
125+
suite.Require().Error(err)
126+
suite.Require().Contains(err.Error(), tc.errMsg)
127+
} else {
128+
suite.Require().NoError(err)
129+
}
130+
})
131+
}
132+
}
133+
134+
func (suite *TestSuite) TestRevoke() {
135+
addrs := suite.createAccounts(2)
136+
137+
grantee, granter := addrs[0], addrs[1]
138+
139+
testCases := []struct {
140+
name string
141+
malleate func() *authz.MsgRevoke
142+
expErr bool
143+
errMsg string
144+
}{
145+
{
146+
name: "invalid granter",
147+
malleate: func() *authz.MsgRevoke {
148+
return &authz.MsgRevoke{
149+
Granter: "invalid",
150+
Grantee: grantee.String(),
151+
MsgTypeUrl: bankSendAuthMsgType,
152+
}
153+
},
154+
expErr: true,
155+
errMsg: "invalid bech32 string",
156+
},
157+
{
158+
name: "invalid grantee",
159+
malleate: func() *authz.MsgRevoke {
160+
return &authz.MsgRevoke{
161+
Granter: granter.String(),
162+
Grantee: "invalid",
163+
MsgTypeUrl: bankSendAuthMsgType,
164+
}
165+
},
166+
expErr: true,
167+
errMsg: "invalid bech32 string",
168+
},
169+
{
170+
name: "valid grant",
171+
malleate: func() *authz.MsgRevoke {
172+
suite.createSendAuthorization(grantee, granter)
173+
174+
return &authz.MsgRevoke{
175+
Granter: granter.String(),
176+
Grantee: grantee.String(),
177+
MsgTypeUrl: bankSendAuthMsgType,
178+
}
179+
},
180+
},
181+
{
182+
name: "no existing grant to revoke",
183+
malleate: func() *authz.MsgRevoke {
184+
return &authz.MsgRevoke{
185+
Granter: granter.String(),
186+
Grantee: grantee.String(),
187+
MsgTypeUrl: bankSendAuthMsgType,
188+
}
189+
},
190+
expErr: true,
191+
errMsg: "authorization not found",
192+
},
193+
}
194+
195+
for _, tc := range testCases {
196+
suite.Run(tc.name, func() {
197+
_, err := suite.msgSrvr.Revoke(suite.ctx, tc.malleate())
198+
if tc.expErr {
199+
suite.Require().Error(err)
200+
suite.Require().Contains(err.Error(), tc.errMsg)
201+
} else {
202+
suite.Require().NoError(err)
203+
}
204+
})
205+
}
206+
}
207+
208+
func (suite *TestSuite) TestExec() {
209+
addrs := suite.createAccounts(2)
210+
211+
grantee, granter := addrs[0], addrs[1]
212+
coins := sdk.NewCoins(sdk.NewCoin("steak", sdk.NewInt(10)))
213+
214+
msg := &banktypes.MsgSend{
215+
FromAddress: granter.String(),
216+
ToAddress: grantee.String(),
217+
Amount: coins,
218+
}
219+
220+
testCases := []struct {
221+
name string
222+
malleate func() authz.MsgExec
223+
expErr bool
224+
errMsg string
225+
}{
226+
{
227+
name: "invalid grantee",
228+
malleate: func() authz.MsgExec {
229+
return authz.NewMsgExec(sdk.AccAddress{}, []sdk.Msg{msg})
230+
},
231+
expErr: true,
232+
errMsg: "empty address string is not allowed",
233+
},
234+
{
235+
name: "no existing grant",
236+
malleate: func() authz.MsgExec {
237+
return authz.NewMsgExec(grantee, []sdk.Msg{msg})
238+
},
239+
expErr: true,
240+
errMsg: "authorization not found",
241+
},
242+
{
243+
name: "valid case",
244+
malleate: func() authz.MsgExec {
245+
suite.createSendAuthorization(grantee, granter)
246+
return authz.NewMsgExec(grantee, []sdk.Msg{msg})
247+
},
248+
},
249+
}
250+
251+
for _, tc := range testCases {
252+
suite.Run(tc.name, func() {
253+
req := tc.malleate()
254+
_, err := suite.msgSrvr.Exec(suite.ctx, &req)
255+
if tc.expErr {
256+
suite.Require().Error(err)
257+
suite.Require().Contains(err.Error(), tc.errMsg)
258+
} else {
259+
suite.Require().NoError(err)
260+
}
261+
})
262+
}
263+
}

0 commit comments

Comments
 (0)