Skip to content

Commit

Permalink
refactor: authz audit changes (backport #16631) (#16647)
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Jun 22, 2023
1 parent 7539520 commit e61ce3c
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 11 deletions.
2 changes: 1 addition & 1 deletion proto/cosmos/authz/v1beta1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ message QueryGranterGrantsResponse {
cosmos.base.query.v1beta1.PageResponse pagination = 2;
}

// QueryGranteeGrantsRequest is the request type for the Query/IssuedGrants RPC method.
// QueryGranteeGrantsRequest is the request type for the Query/GranteeGrants RPC method.
message QueryGranteeGrantsRequest {
string grantee = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];

Expand Down
12 changes: 6 additions & 6 deletions proto/cosmos/authz/v1beta1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,8 @@ message MsgGrant {
cosmos.authz.v1beta1.Grant grant = 3 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true];
}

// MsgExecResponse defines the Msg/MsgExecResponse response type.
message MsgExecResponse {
repeated bytes results = 1;
}
// MsgGrantResponse defines the Msg/MsgGrant response type.
message MsgGrantResponse {}

// MsgExec attempts to execute the provided messages using
// authorizations granted to the grantee. Each message should have only
Expand All @@ -63,8 +61,10 @@ message MsgExec {
repeated google.protobuf.Any msgs = 2 [(cosmos_proto.accepts_interface) = "cosmos.base.v1beta1.Msg"];
}

// MsgGrantResponse defines the Msg/MsgGrant response type.
message MsgGrantResponse {}
// MsgExecResponse defines the Msg/MsgExecResponse response type.
message MsgExecResponse {
repeated bytes results = 1;
}

// MsgRevoke revokes any authorization with the provided sdk.Msg type on the
// granter's account with that has been granted to the grantee.
Expand Down
2 changes: 1 addition & 1 deletion x/authz/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ $ %s query %s grants cosmos1skjw.. cosmos1skjwj.. %s
return err
}
msgAuthorized := ""
if len(args) >= 3 {
if len(args) == 3 {
msgAuthorized = args[2]
}
pageReq, err := client.ReadPageRequest(cmd.Flags())
Expand Down
73 changes: 73 additions & 0 deletions x/authz/client/cli/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,79 @@ func (s *CLITestSuite) TestQueryAuthorization() {
}
}

func (s *CLITestSuite) TestQueryGranteeGrants() {
val := testutil.CreateKeyringAccounts(s.T(), s.kr, 1)

grantee := s.grantee[0]
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()
require := s.Require()

_, err := authzclitestutil.CreateGrant(
s.clientCtx,
[]string{
grantee.String(),
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()),
},
)
s.Require().NoError(err)

testCases := []struct {
name string
args []string
expectErr bool
expectedErr string
}{
{
"invalid address",
[]string{
"invalid-address",
fmt.Sprintf("--%s=json", flags.FlagOutput),
},
true,
"decoding bech32 failed",
},
{
"valid case",
[]string{
grantee.String(),
fmt.Sprintf("--%s=json", flags.FlagOutput),
},
false,
"",
},
{
"valid case with pagination",
[]string{
grantee.String(),
"--limit=2",
fmt.Sprintf("--%s=json", flags.FlagOutput),
},
false,
"",
},
}
for _, tc := range testCases {
s.Run(tc.name, func() {
cmd := cli.GetQueryGranteeGrants(addresscodec.NewBech32Codec("cosmos"))
out, err := clitestutil.ExecTestCLICmd(s.clientCtx, cmd, tc.args)
if tc.expectErr {
require.Error(err)
require.Contains(out.String(), tc.expectedErr)
} else {
require.NoError(err)
var grants authz.QueryGranteeGrantsResponse
require.NoError(s.clientCtx.Codec.UnmarshalJSON(out.Bytes(), &grants))
}
})
}
}

func (s *CLITestSuite) TestQueryGranterGrants() {
val := testutil.CreateKeyringAccounts(s.T(), s.kr, 1)

Expand Down
25 changes: 24 additions & 1 deletion x/authz/client/cli/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
sdkmath "cosmossdk.io/math"

_ "cosmossdk.io/api/cosmos/authz/v1beta1"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
Expand Down Expand Up @@ -229,6 +230,19 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
true,
"invalid separator index",
},
{
"Invalid spend limit",
[]string{
grantee.String(),
"send",
fmt.Sprintf("--%s=0stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()),
fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
},
true,
"spend-limit should be greater than zero",
},
{
"Invalid expiration time",
[]string{
Expand Down Expand Up @@ -306,7 +320,7 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
"invalid denom",
},
{
"invalid bond denon for tx redelegate authorization",
"invalid bond denom for tx redelegate authorization",
[]string{
grantee.String(),
"redelegate",
Expand Down Expand Up @@ -337,6 +351,15 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() {
true,
"invalid decimal coin expression",
},
{
"invalid authorization type",
[]string{
grantee.String(),
"invalid authz type",
},
true,
"invalid authorization type",
},
{
"Valid tx send authorization",
[]string{
Expand Down
2 changes: 1 addition & 1 deletion x/authz/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"github.com/cosmos/cosmos-sdk/x/authz"
)

// InitGenesis new authz genesis
// InitGenesis initializes new authz genesis
func (k Keeper) InitGenesis(ctx sdk.Context, data *authz.GenesisState) {
now := ctx.BlockTime()
for _, entry := range data.Authorization {
Expand Down
5 changes: 4 additions & 1 deletion x/authz/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,12 @@ func (suite *GenesisTestSuite) TestImportExportGenesis() {
// TODO, recheck!
// Clear keeper
suite.keeper.DeleteGrant(suite.ctx, granteeAddr, granterAddr, grant.MsgTypeURL())
newGenesis := suite.keeper.ExportGenesis(suite.ctx)
suite.Require().NotEqual(genesis, newGenesis)
suite.Require().Empty(newGenesis)

suite.keeper.InitGenesis(suite.ctx, genesis)
newGenesis := suite.keeper.ExportGenesis(suite.ctx)
newGenesis = suite.keeper.ExportGenesis(suite.ctx)
suite.Require().Equal(genesis, newGenesis)
}

Expand Down
1 change: 1 addition & 0 deletions x/authz/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/authz"
Expand Down

0 comments on commit e61ce3c

Please sign in to comment.